Skip to content

Commit 0e636da

Browse files
author
rstam
committed
CSHARP-697: Keep SecureStrings secure and support non-ASCII characters in passwords.
1 parent 5ee4b82 commit 0e636da

File tree

7 files changed

+99
-57
lines changed

7 files changed

+99
-57
lines changed

‎MongoDB.Driver/Communication/Security/MongoCRAuthenticationProtocol.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public void Authenticate(MongoConnection connection, MongoCredential credential)
4848
}
4949

5050
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);
5252
var digest = MongoUtils.Hash(nonce + credential.Username + passwordDigest);
5353
var authenticateCommand = new CommandDocument
5454
{

‎MongoDB.Driver/MongoCredential.cs‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515

1616
using System;
1717
using System.Linq;
18+
using System.Runtime.InteropServices;
1819
using System.Security;
19-
using System.Security.Principal;
2020

2121
namespace MongoDB.Driver
2222
{
@@ -83,14 +83,29 @@ public string Mechanism
8383
/// Gets the password.
8484
/// </summary>
8585
[Obsolete("Use Evidence instead.")]
86+
[SecuritySafeCritical]
8687
public string Password
8788
{
8889
get
8990
{
9091
var passwordEvidence = _evidence as PasswordEvidence;
9192
if (passwordEvidence != null)
9293
{
93-
return passwordEvidence.Password;
94+
var secureString = passwordEvidence.SecurePassword;
95+
if (secureString == null || secureString.Length == 0)
96+
{
97+
return "";
98+
}
99+
100+
var bstr = Marshal.SecureStringToBSTR(secureString);
101+
try
102+
{
103+
return Marshal.PtrToStringBSTR(bstr);
104+
}
105+
finally
106+
{
107+
Marshal.ZeroFreeBSTR(bstr);
108+
}
94109
}
95110

96111
return null;

‎MongoDB.Driver/MongoUser.cs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public MongoUser(string username, PasswordEvidence password, bool isReadOnly)
4646
throw new ArgumentNullException("password");
4747
}
4848
_username = username;
49-
_passwordHash = HashPassword(username, password.Password);
49+
_passwordHash = HashPassword(username, password);
5050
_isReadOnly = isReadOnly;
5151
}
5252

@@ -147,6 +147,17 @@ public static bool Equals(MongoUser lhs, MongoUser rhs)
147147
return lhs.Equals(rhs);
148148
}
149149

150+
/// <summary>
151+
/// Calculates the password hash.
152+
/// </summary>
153+
/// <param name="username">The username.</param>
154+
/// <param name="password">The password.</param>
155+
/// <returns>The password hash.</returns>
156+
public static string HashPassword(string username, PasswordEvidence password)
157+
{
158+
return password.ComputeMongoCRPasswordDigest(username);
159+
}
160+
150161
/// <summary>
151162
/// Calculates the password hash.
152163
/// </summary>
@@ -155,7 +166,7 @@ public static bool Equals(MongoUser lhs, MongoUser rhs)
155166
/// <returns>The password hash.</returns>
156167
public static string HashPassword(string username, string password)
157168
{
158-
return MongoUtils.Hash(username + ":mongo:" + password);
169+
return HashPassword(username, new PasswordEvidence(password));
159170
}
160171

161172
// public methods

‎MongoDB.Driver/PasswordEvidence.cs‎

Lines changed: 53 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,11 @@
1414
*/
1515

1616
using System;
17-
using System.Collections.Generic;
18-
using System.Linq;
1917
using System.Runtime.InteropServices;
2018
using System.Security;
2119
using System.Security.Cryptography;
2220
using System.Text;
21+
using MongoDB.Bson;
2322

2423
namespace MongoDB.Driver
2524
{
@@ -30,7 +29,7 @@ public sealed class PasswordEvidence : MongoIdentityEvidence
3029
{
3130
// private fields
3231
private readonly SecureString _securePassword;
33-
private readonly string _digest;
32+
private readonly string _digest; // used to implement Equals without referring to the SecureString
3433

3534
// constructors
3635
/// <summary>
@@ -56,14 +55,6 @@ public PasswordEvidence(string password)
5655
/// <summary>
5756
/// Gets the password.
5857
/// </summary>
59-
public string Password
60-
{
61-
get { return CreateString(_securePassword); }
62-
}
63-
64-
/// <summary>
65-
/// Gets the secure password.
66-
/// </summary>
6758
public SecureString SecurePassword
6859
{
6960
get { return _securePassword; }
@@ -95,6 +86,24 @@ public override int GetHashCode()
9586
return _digest.GetHashCode();
9687
}
9788

89+
// internal methods
90+
/// <summary>
91+
/// Computes the MONGODB-CR password digest.
92+
/// </summary>
93+
/// <param name="username">The username.</param>
94+
/// <returns></returns>
95+
internal string ComputeMongoCRPasswordDigest(string username)
96+
{
97+
using (var md5 = MD5.Create())
98+
{
99+
var encoding = new UTF8Encoding(false, true);
100+
var prefixBytes = encoding.GetBytes(username + ":mongo:");
101+
md5.TransformBlock(prefixBytes, 0, prefixBytes.Length, null, 0);
102+
TransformFinalBlock(md5, _securePassword);
103+
return BsonUtils.ToHexString(md5.Hash);
104+
}
105+
}
106+
98107
// private static methods
99108
private static SecureString CreateSecureString(string str)
100109
{
@@ -112,56 +121,54 @@ private static SecureString CreateSecureString(string str)
112121
return null;
113122
}
114123

115-
[SecuritySafeCritical]
116-
private static string CreateString(SecureString secureString)
124+
/// <summary>
125+
/// Computes the hash value of the secured string
126+
/// </summary>
127+
private static string GenerateDigest(SecureString secureString)
117128
{
118-
IntPtr strPtr = IntPtr.Zero;
119-
if (secureString == null || secureString.Length == 0)
120-
{
121-
return string.Empty;
122-
}
123-
124-
try
129+
using (var sha256 = new SHA256Managed())
125130
{
126-
strPtr = Marshal.SecureStringToBSTR(secureString);
127-
return Marshal.PtrToStringBSTR(strPtr);
128-
}
129-
finally
130-
{
131-
if (strPtr != IntPtr.Zero)
132-
{
133-
Marshal.ZeroFreeBSTR(strPtr);
134-
}
131+
TransformFinalBlock(sha256, secureString);
132+
return BsonUtils.ToHexString(sha256.Hash);
135133
}
136134
}
137135

138-
/// <summary>
139-
/// Computes the hash value of the secured string
140-
/// </summary>
141136
[SecuritySafeCritical]
142-
private static string GenerateDigest(SecureString secureString)
137+
private static void TransformFinalBlock(HashAlgorithm hash, SecureString secureString)
143138
{
144-
IntPtr unmanagedRef = Marshal.SecureStringToBSTR(secureString);
145-
// stored with 0's in between each character...
146-
byte[] bytes = new byte[secureString.Length * 2];
147-
var byteArrayHandle = GCHandle.Alloc(bytes, GCHandleType.Pinned);
148-
Marshal.Copy(unmanagedRef, bytes, 0, secureString.Length * 2);
149-
using (var SHA256 = new SHA256Managed())
139+
var bstr = Marshal.SecureStringToBSTR(secureString);
140+
try
150141
{
142+
var passwordChars = new char[secureString.Length];
143+
var passwordCharsHandle = GCHandle.Alloc(passwordChars, GCHandleType.Pinned);
151144
try
152145
{
153-
return Convert.ToBase64String(SHA256.ComputeHash(bytes));
146+
Marshal.Copy(bstr, passwordChars, 0, passwordChars.Length);
147+
148+
var passwordBytes = new byte[secureString.Length * 3]; // worst case for UTF16 to UTF8 encoding
149+
var passwordBytesHandle = GCHandle.Alloc(passwordBytes, GCHandleType.Pinned);
150+
try
151+
{
152+
var encoding = new UTF8Encoding(false, true);
153+
var length = encoding.GetBytes(passwordChars, 0, passwordChars.Length, passwordBytes, 0);
154+
hash.TransformFinalBlock(passwordBytes, 0, length);
155+
}
156+
finally
157+
{
158+
Array.Clear(passwordBytes, 0, passwordBytes.Length);
159+
passwordBytesHandle.Free();
160+
}
154161
}
155162
finally
156163
{
157-
for (int i = 0; i < bytes.Length; i++)
158-
{
159-
bytes[i] = (byte)'\0';
160-
}
161-
byteArrayHandle.Free();
162-
Marshal.ZeroFreeBSTR(unmanagedRef);
164+
Array.Clear(passwordChars, 0, passwordChars.Length);
165+
passwordCharsHandle.Free();
163166
}
164167
}
168+
finally
169+
{
170+
Marshal.ZeroFreeBSTR(bstr);
171+
}
165172
}
166173
}
167174
}

‎MongoDB.DriverUnitTests/MongoClientSettingsTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ public void TestFromMongoConnectionStringBuilder()
245245
Assert.AreEqual(1, settings.Credentials.Count());
246246
Assert.AreEqual(builder.Username, settings.Credentials.Single().Username);
247247
Assert.AreEqual("admin", settings.Credentials.Single().Source);
248-
Assert.AreEqual(builder.Password, ((PasswordEvidence)settings.Credentials.Single().Evidence).Password);
248+
Assert.AreEqual(new PasswordEvidence(builder.Password), settings.Credentials.Single().Evidence);
249249
Assert.AreEqual(builder.GuidRepresentation, settings.GuidRepresentation);
250250
Assert.AreEqual(builder.IPv6, settings.IPv6);
251251
Assert.AreEqual(builder.MaxConnectionIdleTime, settings.MaxConnectionIdleTime);
@@ -286,7 +286,7 @@ public void TestFromUrl()
286286
Assert.AreEqual(url.Username, settings.Credentials.Single().Username);
287287
Assert.AreEqual(url.AuthenticationMechanism, settings.Credentials.Single().Mechanism);
288288
Assert.AreEqual(url.AuthenticationSource, settings.Credentials.Single().Source);
289-
Assert.AreEqual(url.Password, ((PasswordEvidence)settings.Credentials.Single().Evidence).Password);
289+
Assert.AreEqual(new PasswordEvidence(url.Password), settings.Credentials.Single().Evidence);
290290
Assert.AreEqual(url.GuidRepresentation, settings.GuidRepresentation);
291291
Assert.AreEqual(url.IPv6, settings.IPv6);
292292
Assert.AreEqual(url.MaxConnectionIdleTime, settings.MaxConnectionIdleTime);

‎MongoDB.DriverUnitTests/MongoCredentialTests.cs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public void TestCreateCredential()
2626
{
2727
var credential = MongoCredential.CreateMongoCRCredential("db", "username", "password");
2828
Assert.AreEqual("username", credential.Username);
29-
Assert.AreEqual("password", ((PasswordEvidence)credential.Evidence).Password);
29+
Assert.AreEqual(new PasswordEvidence("password"), credential.Evidence);
3030
}
3131

3232
[Test]
@@ -60,5 +60,14 @@ public void TestEquals()
6060
Assert.IsFalse(null != n);
6161
Assert.IsTrue(c != d);
6262
}
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+
}
6372
}
6473
}

‎MongoDB.DriverUnitTests/MongoServerSettingsTests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ public void TestFromClientSettings()
260260
Assert.AreEqual(url.Username, settings.Credentials.Single().Username);
261261
Assert.AreEqual(url.AuthenticationMechanism, settings.Credentials.Single().Mechanism);
262262
Assert.AreEqual(url.AuthenticationSource, settings.Credentials.Single().Source);
263-
Assert.AreEqual(url.Password, ((PasswordEvidence)settings.Credentials.Single().Evidence).Password);
263+
Assert.AreEqual(new PasswordEvidence(builder.Password), settings.Credentials.Single().Evidence);
264264
Assert.AreEqual(url.GuidRepresentation, settings.GuidRepresentation);
265265
Assert.AreEqual(url.IPv6, settings.IPv6);
266266
Assert.AreEqual(url.MaxConnectionIdleTime, settings.MaxConnectionIdleTime);
@@ -299,7 +299,7 @@ public void TestFromMongoConnectionStringBuilder()
299299
Assert.AreEqual(1, settings.Credentials.Count());
300300
Assert.AreEqual(builder.Username, settings.Credentials.Single().Username);
301301
Assert.AreEqual("admin", settings.Credentials.Single().Source);
302-
Assert.AreEqual(builder.Password, ((PasswordEvidence)settings.Credentials.Single().Evidence).Password);
302+
Assert.AreEqual(new PasswordEvidence(builder.Password), settings.Credentials.Single().Evidence);
303303
Assert.AreEqual(builder.GuidRepresentation, settings.GuidRepresentation);
304304
Assert.AreEqual(builder.IPv6, settings.IPv6);
305305
Assert.AreEqual(builder.MaxConnectionIdleTime, settings.MaxConnectionIdleTime);
@@ -342,7 +342,7 @@ public void TestFromUrl()
342342
Assert.AreEqual(url.Username, settings.Credentials.Single().Username);
343343
Assert.AreEqual(url.AuthenticationMechanism, settings.Credentials.Single().Mechanism);
344344
Assert.AreEqual(url.AuthenticationSource, settings.Credentials.Single().Source);
345-
Assert.AreEqual(url.Password, ((PasswordEvidence)settings.Credentials.Single().Evidence).Password);
345+
Assert.AreEqual(new PasswordEvidence(url.Password), settings.Credentials.Single().Evidence);
346346
Assert.AreEqual(url.GuidRepresentation, settings.GuidRepresentation);
347347
Assert.AreEqual(url.IPv6, settings.IPv6);
348348
Assert.AreEqual(url.MaxConnectionIdleTime, settings.MaxConnectionIdleTime);

0 commit comments

Comments
 (0)