views:

363

answers:

3

Hi guys, I have begun building a secure string type - which i call SecureStringV2 - to extend the existing SecureString type in the .Net framework. This new type is going to add some basic functionality (checking for equality, comparing etc) to the existing type but still maintain the security provided by the SecureString type, that is clear everything out of memory after the types' use. I plan to achieve these features using the Marshal class and hash algorithms. Pointers on how to get this done and done right would be appreciated. Does any of you see any problems with my ideas of implementing this? Thank you :)

Update: This is where my ideas have lead me so far with respect to the core class of the library. take a look and let me know your thoughts.

/// <summary>
///  This class is extension of the SecureString Class in the .Net framework. 
///  It provides checks for equality of multiple SStringV2 instances and maintains
///  the security provided by the SecureString Class
/// </summary>
public class SStringV2 : IEquatable<SStringV2> , IDisposable
{
    private SecureString secureString = new SecureString();
    private Byte[] sStringBytes;
    private String hash = string.Empty;

    /// <summary>
    ///  SStringV2 constructor
    /// </summary>
    /// <param name="confidentialData"></param>
    public SStringV2(ref Char[] confidentialData)
    {
        GCHandle charArrayHandle = GCHandle.Alloc(confidentialData, GCHandleType.Pinned);
        // The unmanaged string splices a zero byte inbetween every two bytes 
        //and at its end  doubling the total number of bytes
        sStringBytes = new Byte[confidentialData.Length*2];
        try
        {
            for (int index = 0; index < confidentialData.Length; ++index)
            {                   
                secureString.AppendChar(confidentialData[index]);
            }
        }
        finally
        {
            ZeroOutSequence.ZeroOutArray(ref confidentialData);
            charArrayHandle.Free();
        }
    }

    /// <summary>
    /// Computes the hash value of the secured string 
    /// </summary>
    private void GenerateHash()
    {
        IntPtr unmanagedRef = Marshal.SecureStringToBSTR(secureString);
        GCHandle byteArrayHandle = GCHandle.Alloc(sStringBytes, GCHandleType.Pinned);
        Marshal.Copy(unmanagedRef, sStringBytes, 0, sStringBytes.Length);
        SHA256Managed SHA256 = new SHA256Managed();

        try
        {
            hash = Convert.ToBase64String(SHA256.ComputeHash(this.sStringBytes));
        }
        finally
        {
            SHA256.Clear();
            ZeroOutSequence.ZeroOutArray(ref sStringBytes);
            byteArrayHandle.Free(); 
            Marshal.ZeroFreeBSTR(unmanagedRef);
        }
    }

    #region IEquatable<SStringV2> Members

    public bool Equals(SStringV2 other)
    {
        if ((this.hash == string.Empty) & ( other.hash == string.Empty))
        { 
            this.GenerateHash();
            other.GenerateHash();
        }
        else if ((this.hash == string.Empty) & !(other.hash == string.Empty))
        {
            this.GenerateHash();
        }
        else if (!(this.hash == string.Empty) & (other.hash == string.Empty))
        {
            other.GenerateHash();
        }

        if (this.hash.Equals(other.hash))
        {
            return true;
        }
            return false;
    }

    #endregion

    #region IDisposable Members

    public void Dispose()
    {
        secureString.Dispose();
        hash = string.Empty;
        GC.SuppressFinalize(this);
    }

    #endregion
}

}

A: 

if you pass a char[] array in you start to lose the benefits of securestring. char[] array could be copied around in memory by the garbage collector before it is cleared.

very true, pinning the char[] array right after passing it in should solve the problem - at least internally - to some extent i believe. This should do it : GCHandle.Alloc(confidentialData, GCHandleType.Pinned);
gogole
it's still wrong. you should only have a method like Append. i actually think securestring is snake oil. i reckon in 99% of cases where ss is used there is a risk that the sensitive data might be inadvertantly written to disk because it is stored somewhere else in the program before it is put in ss.
i see two use cases for ss. 1) protect against leaking secure data to swap. it does an 'ok' job at that with a lot of caveats. 2) protect against an active local attacker. in this case it makes things worse because a local attacker can log all the strings added to SecureString.
i'm sure the improvements i'm making would provide further security and reduce the attack surface.Could we look at things in this perspective and stop worrying about how structures would be moved around by GC.considering things this was would enable us solve the problems of the old implementation.
gogole
Writing the input code in unmanaged languages like C++ should take care of most of our data input worries i believe. Thoughts ?
gogole
+1  A: 

Just one quick note - You're asking for problems mistaking hash equality with data equality - sure the probability of them being different is low but once you hit it it'll be a nightmare :/ (I had the "luck" to debug and fix a custom hash-map implementation that made the same mistake a few years ago so please trust me on this ;) ).

RnR
could you elaborate on this. From my perspective, it is only going to be the SecureString and the Hash that will be kept initialised to a relevant value after the Equals() method is called. if the hashes are equal i believe we can safely generalise that the two instance being compared are equal.
gogole
To show the problem clearly: With strings 100x longer then the hash it's obvious multiple strings of this or greater lenghts have to have the same hashes right? The same is true for shorter strings with a given probability depending on the hash function used - you've been warned ;)
RnR
oh ok, i think i get your perspective of things now.I think there is no cause for alarm since i fed the whole string - i do not extract specific bytes - to the hash function and thus the slightest difference in two strings would certainly produce different hashes. Thanks for the heads up :)
gogole
@gogole: Oh no! Not getting it... How many different strings can there be? (surely more than 2^256). How many different values can the hashing function return? Only 2^256. This means that there will be many different strings that return the same hash...
Arjan Einbu
@Arjan: very true but I'm wondering who or what in this world would go beyond 2^256 characters of confidential data. I guess its better to be paranoid than leave things to chance. If any of you has better ways of checking for equality could you please post it.
gogole
Longer is obvious but shorter is the same as there's no "guarantee" that ABCDE will have a different hash then EDCBA. In fact they had the same hash in MS VC++ once I think - the hash functions are one directional and are hard to reverse - they do not guarantee difference in any way but "chance".
RnR
do you realize that the number of characters for a hash actually exceed the number of alphabets ? (24) taking into account special characters and numbers. This decreases the chances of a hash collision further and makes it more absurd to have a secure data that long.
gogole
My point is that the hash system is not perfect but should do in this scenario.
gogole
@gogole: sorry but it seems you don't know what you're talking about - please read about hash functions, how they work and what they can be used for. Even 2 two letter stings can have the same hash - it's just not very probable, the longer the strings, the bigger the probability of problems - try it
RnR
@RNR: Thanks for the direction, looks like there's a whole lotmore about hashes i do not know. Will research and get back to you.
gogole
@All - A SHA256 hash algorithm has not currently shown to have collisions. That said, collisions are always possible, just extremely unlikely. You might use a SHA512 algorithm to minimize this risk even further, but the chance is so unlikely it's not even worth mentioning.
Doug
A: 

Don't forget that SecureString uses two-way encryption. I don't see any encryption in your class.

Doug