Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Rfc2898DeriveBytes + PBKDF2 + SecureString is it possible to use a secure string instead of a string?

I've a function GetPassword, that returns a SecureString type.

When I pass this secure string to Rfc2898DeriveBytes to generate a key, Visual Studio shows an error. My limited knowledge tells me that it is because Rfc2898DeriveBytes accepts only a string and not a secure string. Is there a workaround to this?

//read the password from terminal
Console.Write("Insert password");
securePwd = myCryptography.GetPassword();

//dont know why the salt is initialized like this
byte[] salt = new byte[] { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0xF1, 0xF0, 0xEE, 0x21, 0x22, 0x45 };
 try
 {   //PBKDF2 standard 
     Rfc2898DeriveBytes key = new Rfc2898DeriveBytes(securePwd, salt, iterationsPwd);
like image 656
NoobTom Avatar asked Mar 16 '12 08:03

NoobTom


3 Answers

I found it interesting that the Rfc2898DeriveBytes class does not support a SecureString overload for passing the password used in deriving the key.

WPF allows for handling passwords as SecureString objects with the PasswordBox control. It seemed like such a waste that the added security that this control offers was lost due to the fact we could not pass in a SecureString to the constructor. However, erickson brought up the excellent point of using the byte[] instead of the string overload as it is relatively easier to properly manage the contents of a byte[] in memory than a string.

Using erickson's suggestion as inspiration I came up with the following wrapper which should allow for using the value of the password protected by SecureString with minimal exposure of the plaintext value in memory.

private byte[] DeriveKey(SecureString password, byte[] salt, int iterations, int keyByteLength)
{
    IntPtr ptr = Marshal.SecureStringToBSTR(password);
    byte[] passwordByteArray = null;
    try
    {
        int length = Marshal.ReadInt32(ptr, -4);
        passwordByteArray = new byte[length];
        GCHandle handle = GCHandle.Alloc(passwordByteArray, GCHandleType.Pinned);
        try
        {
            for (int i = 0; i < length; i++)
            {
                passwordByteArray[i] = Marshal.ReadByte(ptr, i);
            }

            using (var rfc2898 = new Rfc2898DeriveBytes(passwordByteArray, salt, iterations))
            {
                return rfc2898.GetBytes(keyByteLength);
            }
        }
        finally
        {
            Array.Clear(passwordByteArray, 0, passwordByteArray.Length);  
            handle.Free();
        }
    }
    finally
    {
        Marshal.ZeroFreeBSTR(ptr);
    }
}

This approach leverages the fact that BSTR is a pointer pointing to the first character of the data string with a four byte length prefix.

Important points:

  • By wrapping Rfc2898DeriveBytes in a using statement it ensures that it is disposed in a deterministic manner. This is important as it has a internal HMACSHA1 object which is a KeyedHashAlgorithm and needs to have the copy of the key (password) it possesses to be zeroed out in the call to Dispose. See Reference Source for full details.
  • As soon as we are done with the BSTR we zero it out and free it via ZeroFreeBSTR.
  • Lastly, we zero out (clear) of our copy of the password.
  • Update: Added pinning of the byte[]. As discussed in the comments of this answer, if the byte[] is not pinned then the garbage collector could relocate the object during collection and we would be left with no way to zero out the original copy.

This should keep the plaintext password in memory for the shortest amount of time and not weaken the gains of using SecureString too much. Although, if the attacker has access to RAM you probably have bigger problems. Another point is that we can only only manage our own copies of the password, the API we are using could very well mismanage (not zero out/clear) their copies. To the best of my knowledge this is not the case with Rfc2898DeriveBytes, although their copy of the byte[] key (password) is not pinned and therefore traces of the array may hang around if it was moved in the heap before being zeroed out. The message here is that code can look secure, but problems may lie underneath.

If anyone finds any serious holes in this implementation, please let me know.

like image 131
Derek W Avatar answered Sep 28 '22 00:09

Derek W


Apparently, you can violate the protection afforded by SecureString and expose its internal state via the Marshal.SecureStringToBSTR() function.

Rather than creating a String out of the result, copy the content to a Byte[] to pass to Rfc2898DeriveBytes. Creating a String would prevent you from destroying the password information, allowing it to hang out in the heap indefinitely, or get paged to disk, which in turn increases the chances that an attacker can find it. Instead, you should destroy the password as soon as you are finished using it, by filling the array with zeros. For the same reason, you should also assign a zero to each element of the BSTR as you copy it to the Byte[].

Salt should be randomly selected for each hashed password, not a fixed, predictable value, otherwise a pre-computed dictionary attack is possible. You should iterate many tens of thousands of times in order to prevent brute force attacks.

like image 44
erickson Avatar answered Sep 28 '22 00:09

erickson


After doing some research and looking at previous answers on stackoverflow mentioning SecureString, that answer is almost certainly: "No". Only the creators of the API can accept SecureString and handle it correctly internally. And they can only do that with help of the platform.

If you - as a user - could retrieve the plain text String, you would have negated most of the advantages of using SecureString in the first place. It would even be a bit dangerous as you would create secure looking code, that would not actually be secure at all (edit: at least not when it comes to protecting in-memory data).

like image 45
Maarten Bodewes Avatar answered Sep 28 '22 01:09

Maarten Bodewes