CSHARP-697: Keep SecureStrings secure and support non-ASCII characters in passwords.

rstam · rstam · commit 0e636da12f33 · 2013-03-08T12:06:49.000-05:00
diff --git a/MongoDB.Driver/Communication/Security/MongoCRAuthenticationProtocol.cs b/MongoDB.Driver/Communication/Security/MongoCRAuthenticationProtocol.cs
@@ -48,7 +48,7 @@ public void Authenticate(MongoConnection connection, MongoCredential credential)
             }
 
             var nonce = commandResult.Response["nonce"].AsString;
-            var passwordDigest = MongoUtils.Hash(credential.Username + ":mongo:" + ((PasswordEvidence)credential.Evidence).Password);
+            var passwordDigest = ((PasswordEvidence)credential.Evidence).ComputeMongoCRPasswordDigest(credential.Username);
             var digest = MongoUtils.Hash(nonce + credential.Username + passwordDigest);
             var authenticateCommand = new CommandDocument
                 {
diff --git a/MongoDB.Driver/MongoCredential.cs b/MongoDB.Driver/MongoCredential.cs
@@ -15,8 +15,8 @@
 
 using System;
 using System.Linq;
+using System.Runtime.InteropServices;
 using System.Security;
-using System.Security.Principal;
 
 namespace MongoDB.Driver
 {
@@ -83,14 +83,29 @@ public string Mechanism
         /// Gets the password.
         /// </summary>
         [Obsolete("Use Evidence instead.")]
+        [SecuritySafeCritical]
         public string Password
         {
             get
             {
                 var passwordEvidence = _evidence as PasswordEvidence;
                 if (passwordEvidence != null)
                 {
-                    return passwordEvidence.Password;
+                    var secureString = passwordEvidence.SecurePassword;
+                    if (secureString == null || secureString.Length == 0)
+                    {
+                        return "";
+                    }
+
+                    var bstr = Marshal.SecureStringToBSTR(secureString);
+                    try
+                    {
+                        return Marshal.PtrToStringBSTR(bstr);
+                    }
+                    finally
+                    {
+                        Marshal.ZeroFreeBSTR(bstr);
+                    }
                 }
 
                 return null;
diff --git a/MongoDB.Driver/MongoUser.cs b/MongoDB.Driver/MongoUser.cs
@@ -46,7 +46,7 @@ public MongoUser(string username, PasswordEvidence password, bool isReadOnly)
                 throw new ArgumentNullException("password");
             }
             _username = username;
-            _passwordHash = HashPassword(username, password.Password);
+            _passwordHash = HashPassword(username, password);
             _isReadOnly = isReadOnly;
         }
 
@@ -147,6 +147,17 @@ public static bool Equals(MongoUser lhs, MongoUser rhs)
             return lhs.Equals(rhs);
         }
 
+        /// <summary>
+        /// Calculates the password hash.
+        /// </summary>
+        /// <param name="username">The username.</param>
+        /// <param name="password">The password.</param>
+        /// <returns>The password hash.</returns>
+        public static string HashPassword(string username, PasswordEvidence password)
+        {
+            return password.ComputeMongoCRPasswordDigest(username);
+        }
+
         /// <summary>
         /// Calculates the password hash.
         /// </summary>
@@ -155,7 +166,7 @@ public static bool Equals(MongoUser lhs, MongoUser rhs)
         /// <returns>The password hash.</returns>
         public static string HashPassword(string username, string password)
         {
-            return MongoUtils.Hash(username + ":mongo:" + password);
+            return HashPassword(username, new PasswordEvidence(password));
         }
 
         // public methods
diff --git a/MongoDB.Driver/PasswordEvidence.cs b/MongoDB.Driver/PasswordEvidence.cs
@@ -14,12 +14,11 @@
 */
 
 using System;
-using System.Collections.Generic;
-using System.Linq;
 using System.Runtime.InteropServices;
 using System.Security;
 using System.Security.Cryptography;
 using System.Text;
+using MongoDB.Bson;
 
 namespace MongoDB.Driver
 {
@@ -30,7 +29,7 @@ public sealed class PasswordEvidence : MongoIdentityEvidence
     {
         // private fields
         private readonly SecureString _securePassword;
-        private readonly string _digest;
+        private readonly string _digest; // used to implement Equals without referring to the SecureString
 
         // constructors
         /// <summary>
@@ -56,14 +55,6 @@ public PasswordEvidence(string password)
         /// <summary>
         /// Gets the password.
         /// </summary>
-        public string Password
-        {
-            get { return CreateString(_securePassword); }
-        }
-
-        /// <summary>
-        /// Gets the secure password.
-        /// </summary>
         public SecureString SecurePassword
         {
             get { return _securePassword; }
@@ -95,6 +86,24 @@ public override int GetHashCode()
             return _digest.GetHashCode();
         }
 
+        // internal methods
+        /// <summary>
+        /// Computes the MONGODB-CR password digest.
+        /// </summary>
+        /// <param name="username">The username.</param>
+        /// <returns></returns>
+        internal string ComputeMongoCRPasswordDigest(string username)
+        {
+            using (var md5 = MD5.Create())
+            {
+                var encoding = new UTF8Encoding(false, true);
+                var prefixBytes = encoding.GetBytes(username + ":mongo:");
+                md5.TransformBlock(prefixBytes, 0, prefixBytes.Length, null, 0);
+                TransformFinalBlock(md5, _securePassword);
+                return BsonUtils.ToHexString(md5.Hash);
+            }
+        }
+
         // private static methods
         private static SecureString CreateSecureString(string str)
         {
@@ -112,56 +121,54 @@ private static SecureString CreateSecureString(string str)
             return null;
         }
 
-        [SecuritySafeCritical]
-        private static string CreateString(SecureString secureString)
+        /// <summary>
+        /// Computes the hash value of the secured string 
+        /// </summary>
+        private static string GenerateDigest(SecureString secureString)
         {
-            IntPtr strPtr = IntPtr.Zero;
-            if (secureString == null || secureString.Length == 0)
-            {
-                return string.Empty;
-            }
-
-            try
+            using (var sha256 = new SHA256Managed())
             {
-                strPtr = Marshal.SecureStringToBSTR(secureString);
-                return Marshal.PtrToStringBSTR(strPtr);
-            }
-            finally
-            {
-                if (strPtr != IntPtr.Zero)
-                {
-                    Marshal.ZeroFreeBSTR(strPtr);
-                }
+                TransformFinalBlock(sha256, secureString);
+                return BsonUtils.ToHexString(sha256.Hash);
             }
         }
 
-        /// <summary>
-        /// Computes the hash value of the secured string 
-        /// </summary>
         [SecuritySafeCritical]
-        private static string GenerateDigest(SecureString secureString)
+        private static void TransformFinalBlock(HashAlgorithm hash, SecureString secureString)
         {
-            IntPtr unmanagedRef = Marshal.SecureStringToBSTR(secureString);
-            // stored with 0's in between each character...
-            byte[] bytes = new byte[secureString.Length * 2];
-            var byteArrayHandle = GCHandle.Alloc(bytes, GCHandleType.Pinned);
-            Marshal.Copy(unmanagedRef, bytes, 0, secureString.Length * 2);
-            using (var SHA256 = new SHA256Managed())
+            var bstr = Marshal.SecureStringToBSTR(secureString);
+            try
             {
+                var passwordChars = new char[secureString.Length];
+                var passwordCharsHandle = GCHandle.Alloc(passwordChars, GCHandleType.Pinned);
                 try
                 {
-                    return Convert.ToBase64String(SHA256.ComputeHash(bytes));
+                    Marshal.Copy(bstr, passwordChars, 0, passwordChars.Length);
+
+                    var passwordBytes = new byte[secureString.Length * 3]; // worst case for UTF16 to UTF8 encoding
+                    var passwordBytesHandle = GCHandle.Alloc(passwordBytes, GCHandleType.Pinned);
+                    try
+                    {
+                        var encoding = new UTF8Encoding(false, true);
+                        var length = encoding.GetBytes(passwordChars, 0, passwordChars.Length, passwordBytes, 0);
+                        hash.TransformFinalBlock(passwordBytes, 0, length);
+                    }
+                    finally
+                    {
+                        Array.Clear(passwordBytes, 0, passwordBytes.Length);
+                        passwordBytesHandle.Free();
+                    }
                 }
                 finally
                 {
-                    for (int i = 0; i < bytes.Length; i++)
-                    {
-                        bytes[i] = (byte)'\0';
-                    }
-                    byteArrayHandle.Free();
-                    Marshal.ZeroFreeBSTR(unmanagedRef);
+                    Array.Clear(passwordChars, 0, passwordChars.Length);
+                    passwordCharsHandle.Free();
                 }
             }
+            finally
+            {
+                Marshal.ZeroFreeBSTR(bstr);
+            }
         }
     }
 }
diff --git a/MongoDB.DriverUnitTests/MongoClientSettingsTests.cs b/MongoDB.DriverUnitTests/MongoClientSettingsTests.cs
@@ -245,7 +245,7 @@ public void TestFromMongoConnectionStringBuilder()
             Assert.AreEqual(1, settings.Credentials.Count());
             Assert.AreEqual(builder.Username, settings.Credentials.Single().Username);
             Assert.AreEqual("admin", settings.Credentials.Single().Source);
-            Assert.AreEqual(builder.Password, ((PasswordEvidence)settings.Credentials.Single().Evidence).Password);
+            Assert.AreEqual(new PasswordEvidence(builder.Password), settings.Credentials.Single().Evidence);
             Assert.AreEqual(builder.GuidRepresentation, settings.GuidRepresentation);
             Assert.AreEqual(builder.IPv6, settings.IPv6);
             Assert.AreEqual(builder.MaxConnectionIdleTime, settings.MaxConnectionIdleTime);
@@ -286,7 +286,7 @@ public void TestFromUrl()
             Assert.AreEqual(url.Username, settings.Credentials.Single().Username);
             Assert.AreEqual(url.AuthenticationMechanism, settings.Credentials.Single().Mechanism);
             Assert.AreEqual(url.AuthenticationSource, settings.Credentials.Single().Source);
-            Assert.AreEqual(url.Password, ((PasswordEvidence)settings.Credentials.Single().Evidence).Password);
+            Assert.AreEqual(new PasswordEvidence(url.Password), settings.Credentials.Single().Evidence);
             Assert.AreEqual(url.GuidRepresentation, settings.GuidRepresentation);
             Assert.AreEqual(url.IPv6, settings.IPv6);
             Assert.AreEqual(url.MaxConnectionIdleTime, settings.MaxConnectionIdleTime);
diff --git a/MongoDB.DriverUnitTests/MongoCredentialTests.cs b/MongoDB.DriverUnitTests/MongoCredentialTests.cs
@@ -26,7 +26,7 @@ public void TestCreateCredential()
         {
             var credential = MongoCredential.CreateMongoCRCredential("db", "username", "password");
             Assert.AreEqual("username", credential.Username);
-            Assert.AreEqual("password", ((PasswordEvidence)credential.Evidence).Password);
+            Assert.AreEqual(new PasswordEvidence("password"), credential.Evidence);
         }
 
         [Test]
@@ -60,5 +60,14 @@ public void TestEquals()
             Assert.IsFalse(null != n);
             Assert.IsTrue(c != d);
         }
+
+        [Test]
+        public void TestPassword()
+        {
+            var credentials = MongoCredential.CreateMongoCRCredential("database", "username", "password");
+#pragma warning disable 618
+            Assert.AreEqual("password", credentials.Password);
+#pragma warning restore
+        }
     }
 }
diff --git a/MongoDB.DriverUnitTests/MongoServerSettingsTests.cs b/MongoDB.DriverUnitTests/MongoServerSettingsTests.cs
@@ -260,7 +260,7 @@ public void TestFromClientSettings()
             Assert.AreEqual(url.Username, settings.Credentials.Single().Username);
             Assert.AreEqual(url.AuthenticationMechanism, settings.Credentials.Single().Mechanism);
             Assert.AreEqual(url.AuthenticationSource, settings.Credentials.Single().Source);
-            Assert.AreEqual(url.Password, ((PasswordEvidence)settings.Credentials.Single().Evidence).Password);
+            Assert.AreEqual(new PasswordEvidence(builder.Password), settings.Credentials.Single().Evidence);
             Assert.AreEqual(url.GuidRepresentation, settings.GuidRepresentation);
             Assert.AreEqual(url.IPv6, settings.IPv6);
             Assert.AreEqual(url.MaxConnectionIdleTime, settings.MaxConnectionIdleTime);
@@ -299,7 +299,7 @@ public void TestFromMongoConnectionStringBuilder()
             Assert.AreEqual(1, settings.Credentials.Count());
             Assert.AreEqual(builder.Username, settings.Credentials.Single().Username);
             Assert.AreEqual("admin", settings.Credentials.Single().Source);
-            Assert.AreEqual(builder.Password, ((PasswordEvidence)settings.Credentials.Single().Evidence).Password);
+            Assert.AreEqual(new PasswordEvidence(builder.Password), settings.Credentials.Single().Evidence);
             Assert.AreEqual(builder.GuidRepresentation, settings.GuidRepresentation);
             Assert.AreEqual(builder.IPv6, settings.IPv6);
             Assert.AreEqual(builder.MaxConnectionIdleTime, settings.MaxConnectionIdleTime);
@@ -342,7 +342,7 @@ public void TestFromUrl()
             Assert.AreEqual(url.Username, settings.Credentials.Single().Username);
             Assert.AreEqual(url.AuthenticationMechanism, settings.Credentials.Single().Mechanism);
             Assert.AreEqual(url.AuthenticationSource, settings.Credentials.Single().Source);
-            Assert.AreEqual(url.Password, ((PasswordEvidence)settings.Credentials.Single().Evidence).Password);
+            Assert.AreEqual(new PasswordEvidence(url.Password), settings.Credentials.Single().Evidence);
             Assert.AreEqual(url.GuidRepresentation, settings.GuidRepresentation);
             Assert.AreEqual(url.IPv6, settings.IPv6);
             Assert.AreEqual(url.MaxConnectionIdleTime, settings.MaxConnectionIdleTime);

Original file line number	Diff line number	Diff line change
`@@ -48,7 +48,7 @@ public void Authenticate(MongoConnection connection, MongoCredential credential)`
`48`	`48`	`}`
`49`	`49`
`50`	`50`	`var nonce = commandResult.Response["nonce"].AsString;`
`51`		`- var passwordDigest = MongoUtils.Hash(credential.Username + ":mongo:" + ((PasswordEvidence)credential.Evidence).Password);`
	`51`	`+ var passwordDigest = ((PasswordEvidence)credential.Evidence).ComputeMongoCRPasswordDigest(credential.Username);`
`52`	`52`	`var digest = MongoUtils.Hash(nonce + credential.Username + passwordDigest);`
`53`	`53`	`var authenticateCommand = new CommandDocument`
`54`	`54`	`{`
Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ public void TestCreateCredential()`
`26`	`26`	`{`
`27`	`27`	`var credential = MongoCredential.CreateMongoCRCredential("db", "username", "password");`
`28`	`28`	`Assert.AreEqual("username", credential.Username);`
`29`		`- Assert.AreEqual("password", ((PasswordEvidence)credential.Evidence).Password);`
	`29`	`+ Assert.AreEqual(new PasswordEvidence("password"), credential.Evidence);`
`30`	`30`	`}`
`31`	`31`
`32`	`32`	`[Test]`
`@@ -60,5 +60,14 @@ public void TestEquals()`
`60`	`60`	`Assert.IsFalse(null != n);`
`61`	`61`	`Assert.IsTrue(c != d);`
`62`	`62`	`}`
	`63`	`+`
	`64`	`+ [Test]`
	`65`	`+ public void TestPassword()`
	`66`	`+ {`
	`67`	`+ var credentials = MongoCredential.CreateMongoCRCredential("database", "username", "password");`
	`68`	`+#pragma warning disable 618`
	`69`	`+ Assert.AreEqual("password", credentials.Password);`
	`70`	`+#pragma warning restore`
	`71`	`+ }`
`63`	`72`	`}`
`64`	`73`	`}`