0

Is there any way to provide a SecureString as an argument to the ProtectData.Protect method without first converting it to a regular string?

Currently I have the below code to perform this operation. I've used SecureString to hold the password from its input / the only point at which it becomes a string is in the method call below. That hopefully reduces the life of the insecure string, but is there any way to further improve this?

This logic won't be available in my mail app, but rather in a helper app, the sole point of which is to take input and return encrypted output / the lifespan of which will be minutes, most of which will be spent awaiting the user's input (so with no password in memory).

public string EncryptString(SecureString secureUnencryptedString)
{
    if (secureUnencryptedString == null) return null;
    return EncryptString(secureUnencryptedString.ToInsecureString());
}

private string EncryptString(string insecureUnencryptedString)
{
    Debug.Assert(insecureUnencryptedString != null);
    byte[] encryptedData = ProtectedData.Protect
    (
        Encoding.Unicode.GetBytes(insecureUnencryptedString),
        entropy,
        scope
    );
    return Convert.ToBase64String(encryptedData);
}

//...
//made internal so anyone using my library won't think this should be used elsewhere
internal static class SecureStringExtension
{
    internal static string ToInsecureString(this SecureString value)
    {
        if (value == null) return null;
        IntPtr valuePtr = IntPtr.Zero;
        try
        {
            valuePtr = Marshal.SecureStringToGlobalAllocUnicode(value);
            return Marshal.PtrToStringUni(valuePtr);
        }
        finally
        {
            Marshal.ZeroFreeGlobalAllocUnicode(valuePtr);
        }
    }
}

Is there an option to use the SecureString as an argument directly, rather than first having to convert it to a string?

Failing that, would this be one of the few situations where it's justifiable to call System.GC.Collect() (i.e. in a finally block on the EncryptString method) to improve the chances of this insecure string being removed from memory? e.g.

public string EncryptString(SecureString secureUnencryptedString)
{
    if (secureUnencryptedString == null) return null;
    try
    {
        return EncryptString(secureUnencryptedString.ToInsecureString());
    }
    finally
    {
        GC.Collect(); //doesn't guarantee cleanup, but improves the chances / helps minimise the potential lifetime of the insecure string
    }
}
JohnLBevan
  • 22,735
  • 13
  • 96
  • 178
  • 1
    The insecure string is one of the last things to have been allocated in the managed heap, so will be close to the current "high water mark". When the GC collects, it *compacts*. The chance that those bytes will be left alone are quite high, because it's unlikely that something else will need to be moved into their space. – Damien_The_Unbeliever Nov 02 '18 at 10:50
  • Thanks @Damien_The_Unbeliever. Would simply overwriting the string help? e.g. setting `insecureUnencryptedString = null;` before the end of the method / something like that? – JohnLBevan Nov 02 '18 at 10:52
  • 1
    I'd instead by looking at the [unmanaged API](https://learn.microsoft.com/en-us/windows/desktop/seccng/cng-dpapi) and avoid letting it near the managed heap at all. – Damien_The_Unbeliever Nov 02 '18 at 10:53

0 Answers0