none
Compare SecureString

    Question

  • I need to compare two secure strings. This is what I have come up with:

    Boolean SecureStringEqual(SecureString s1, SecureString s2)  
    {  
        if (s1 == null)  
        {  
            throw new ArgumentNullException("s1");  
        }  
        if (s2 == null)  
        {  
            throw new ArgumentNullException("s2");  
        }  
     
        if (s1.Length != s2.Length)  
        {  
            return false;  
        }  
     
        IntPtr bstr1 = IntPtr.Zero;  
        IntPtr bstr2 = IntPtr.Zero;  
     
        RuntimeHelpers.PrepareConstrainedRegions();  
     
        try 
        {  
            bstr1 = Marshal.SecureStringToBSTR(s1);  
            bstr2 = Marshal.SecureStringToBSTR(s2);  
     
            unsafe 
            {  
                for (Char* ptr1 = (Char*)bstr1.ToPointer(), ptr2 = (Char*)bstr2.ToPointer();  
                    *ptr1 != 0 && *ptr2 != 0;  
                     ++ptr1, ++ptr2)  
                {  
                    if (*ptr1 != *ptr2)  
                    {  
                        return false;  
                    }  
                }  
            }  
     
            return true;  
        }  
        finally 
        {  
            if (bstr1 != IntPtr.Zero)  
            {  
                Marshal.ZeroFreeBSTR(bstr1);  
            }  
     
            if (bstr2 != IntPtr.Zero)  
            {  
                Marshal.ZeroFreeBSTR(bstr2);  
            }  
        }  
    }  
     

    I have another version that does not require unsafe code and uses memcmp through P/Invoke.

    Is this good enough? Are there better implementations?
    Monday, December 15, 2008 5:32 PM

Answers

  • That's pretty much it. The only way to get (and thus compare) the contents of SecureString is to marshal them out to unmanaged memory. However, rather using unsafe code, which places additional restrictions on your assembly, I would use the P/Invoke options to do the actual comparison.

    And, rather than using memcmp, I would use something like wcscoll for insurance. If I remember correctly, BSTRs are always marshalled out in wide-char mode. If you are comparing strings, then a simple bit-to-bit comparison can have mistakes for different languages. You need to take locale ID (and thus locale rules) into account, and wcscoll will do that.

    But other than that, your code looks pretty good. Also, if you want to avoid manually dealing with CERs, than you can utilize a custom SafeHandle subclass. SafeHandles wrap IntPtrs and provides Critical Finalizers to ensure that the handles get disposed properly, even if the code runs into critical exceptions.
    -Rob Teixeira
    • Marked as answer by Alex Ivanoff Tuesday, December 16, 2008 6:35 AM
    Tuesday, December 16, 2008 1:13 AM

All replies

  • That's pretty much it. The only way to get (and thus compare) the contents of SecureString is to marshal them out to unmanaged memory. However, rather using unsafe code, which places additional restrictions on your assembly, I would use the P/Invoke options to do the actual comparison.

    And, rather than using memcmp, I would use something like wcscoll for insurance. If I remember correctly, BSTRs are always marshalled out in wide-char mode. If you are comparing strings, then a simple bit-to-bit comparison can have mistakes for different languages. You need to take locale ID (and thus locale rules) into account, and wcscoll will do that.

    But other than that, your code looks pretty good. Also, if you want to avoid manually dealing with CERs, than you can utilize a custom SafeHandle subclass. SafeHandles wrap IntPtrs and provides Critical Finalizers to ensure that the handles get disposed properly, even if the code runs into critical exceptions.
    -Rob Teixeira
    • Marked as answer by Alex Ivanoff Tuesday, December 16, 2008 6:35 AM
    Tuesday, December 16, 2008 1:13 AM
  • I actually do want binary comparison since SecureString most of the time represent password.

    As far as SafeHandle goes wouldn't it be nice if Marshal.SecureStringToBSTR return SafeHandle-derived class, not IntPtr. The same goes for other Marshal methods that still operate on IntPtr when they should be SafeHandle.

    Tuesday, December 16, 2008 6:35 AM
  • It would be nice if they overhauled the entire BCL to return and accept SafeHandles when appropriate, but I suspect that won't happen any time soon :-)

    The alternative, however, is that when you create your own custom SafeHandle-derived subclass, you can overload the "imlicit" operator, so that you can implicitly assign an IntPtr to your new custom SafeHandle class. In other words, if your custom subclass is called BstrPointer, then you'd be able to do this:

    BstrPointer myStringPointer = Marshal.SecureStringToBSTR(mySecureString);

    The marshalling infrastructure does this automatically because it has the ability to marshal a 32/64 bit pointer value to a SafeHandle via a custom marshaller. But to assign an IntPtr to a SafeHandle in managed code you'll have to override the implicit operator.
    -Rob Teixeira
    Tuesday, December 16, 2008 5:41 PM