Skip to content

Commit 257c8c8

Browse files
CSHARP-2928: Use Ordinal string comparison in ScramShaAuthenticator.
1 parent 067940d commit 257c8c8

File tree

2 files changed

+99
-45
lines changed

2 files changed

+99
-45
lines changed

src/MongoDB.Driver.Core/Core/Authentication/ScramShaAuthenticator.cs

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

1616
using System;
17-
using System.Collections.Generic;
18-
using System.IO;
1917
using System.Security.Cryptography;
2018
using System.Text;
2119
using MongoDB.Bson.IO;
20+
#if NET452
2221
using MongoDB.Driver.Core.Authentication.Vendored;
22+
#endif
2323
using MongoDB.Driver.Core.Connections;
2424
using MongoDB.Driver.Core.Misc;
2525

@@ -168,7 +168,6 @@ private string PrepUsername(string username)
168168

169169
private class ClientFirst : ISaslStep
170170
{
171-
172171
private readonly byte[] _bytesToSendToServer;
173172
private readonly string _clientFirstMessageBare;
174173
private readonly UsernamePasswordCredential _credential;
@@ -211,7 +210,7 @@ public ISaslStep Transition(SaslConversation conversation, byte[] bytesReceivedF
211210
var map = SaslMapParser.Parse(serverFirstMessage);
212211

213212
var r = map['r'];
214-
if (!r.StartsWith(_rPrefix))
213+
if (!r.StartsWith(_rPrefix, StringComparison.Ordinal))
215214
{
216215
throw new MongoAuthenticationException(conversation.ConnectionId, message: "Server sent an invalid nonce.");
217216
}

tests/MongoDB.Driver.Core.Tests/Core/Authentication/ScramSha256AuthenticatorTests.cs

Lines changed: 96 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -27,19 +27,26 @@
2727
using Xunit;
2828
using MongoDB.Driver.Core.Connections;
2929
using MongoDB.Bson.TestHelpers.XunitExtensions;
30+
3031
namespace MongoDB.Driver.Core.Authentication
3132
{
3233
public class ScramSha256AuthenticatorTests
3334
{
34-
private static readonly UsernamePasswordCredential __credential
35-
= new UsernamePasswordCredential("source", "user", "pencil");
35+
// private constants
36+
private const string _clientNonce = "rOprNGfwEbeRWgbNEkqO";
37+
private const int _iterationCount = 4096;
38+
private const string _serverNonce = "%hvYDpWUa2RaTCAfuxFIlj)hNlF$k0";
39+
private const string _serverSalt = "W22ZaJ0SNY7soEsUEjb6gQ==";
40+
41+
// private static
42+
private static readonly UsernamePasswordCredential __credential = new UsernamePasswordCredential("source", "user", "pencil");
3643
private static readonly ClusterId __clusterId = new ClusterId();
3744
private static readonly ServerId __serverId = new ServerId(__clusterId, new DnsEndPoint("localhost", 27017));
3845
private static readonly ConnectionDescription __description = new ConnectionDescription(
3946
new ConnectionId(__serverId),
4047
new IsMasterResult(new BsonDocument("ok", 1).Add("ismaster", 1)),
4148
new BuildInfoResult(new BsonDocument("version", "4.0.0")));
42-
49+
4350
/*
4451
* This is a simple example of a SCRAM-SHA-256 authentication exchange. The username
4552
* 'user' and password 'pencil' are being used, with a client nonce of "rOprNGfwEbeRWgbNEkqO"
@@ -49,42 +56,42 @@ private static readonly UsernamePasswordCredential __credential
4956
* S: v=6rriTRBi23WpRR/wtup+mMhUZUn/dB5nLTJRsjl95G4=
5057
*/
5158

52-
private const string _clientNonce = "rOprNGfwEbeRWgbNEkqO";
53-
private const int _iterationCount = 4096;
54-
private const string _serverNonce = "%hvYDpWUa2RaTCAfuxFIlj)hNlF$k0";
55-
private const string _serverSalt = "W22ZaJ0SNY7soEsUEjb6gQ==";
56-
5759
private static readonly string _clientRequest1 = $"n,,n=user,r={_clientNonce}";
5860
private static readonly string _serverResponse1 =
5961
$"r={_clientNonce}{_serverNonce},s={_serverSalt},i={_iterationCount}";
6062
private static readonly string _clientRequest2 =
6163
$"c=biws,r={_clientNonce}{_serverNonce},p=dHzbZapWIk4jUhN+Ute9ytag9zjfMHgsqmmiz7AndVQ=";
6264
private static readonly string _serverReponse2 = "v=6rriTRBi23WpRR/wtup+mMhUZUn/dB5nLTJRsjl95G4=";
63-
65+
6466
private static void Authenticate(
6567
ScramSha256Authenticator authenticator,
6668
IConnection connection,
67-
ConnectionDescription description,
6869
bool async)
6970
{
7071
if (async)
7172
{
72-
authenticator.AuthenticateAsync(connection, __description, CancellationToken.None).GetAwaiter().GetResult();
73+
authenticator
74+
.AuthenticateAsync(connection, __description, CancellationToken.None)
75+
.GetAwaiter()
76+
.GetResult();
77+
}
78+
else
79+
{
80+
authenticator.Authenticate(connection, __description, CancellationToken.None);
7381
}
74-
authenticator.Authenticate(connection, __description, CancellationToken.None);
7582
}
76-
83+
7784
/* In response, the server sends a "server-first-message" containing the
7885
* user's iteration count i and the user's salt, and appends its own
7986
* nonce to the client-specified one. */
8087
private static string CreateSaslStartReply(
81-
string clientSaslStart,
82-
string serverNonce,
83-
string serverSalt,
88+
string clientSaslStart,
89+
string serverNonce,
90+
string serverSalt,
8491
int iterationCount)
8592
{
8693
// strip expected GS2 header of "n,," before parsing map
87-
var clientNonce = SaslMapParser.Parse(clientSaslStart.Substring(3))['r'];
94+
var clientNonce = SaslMapParser.Parse(clientSaslStart.Substring(3))['r'];
8895
return $"r={clientNonce}{serverNonce},s={serverSalt},i={iterationCount}";
8996
}
9097

@@ -93,15 +100,15 @@ private static string PoisonSaslMessage(string message, string poison)
93100
{
94101
return message.Substring(0, message.Length - poison.Length) + poison;
95102
}
96-
103+
97104
private static string ToUtf8Base64(string s)
98105
{
99106
return Convert.ToBase64String((System.Text.Encoding.UTF8.GetBytes(s)));
100107
}
101-
108+
102109
[Fact]
103110
public void Constructor_should_throw_an_ArgumentNullException_when_credential_is_null()
104-
{
111+
{
105112
var exception = Record.Exception(() => new ScramSha256Authenticator(null));
106113
exception.Should().BeOfType<ArgumentNullException>();
107114
}
@@ -117,12 +124,12 @@ public void Authenticate_should_throw_an_AuthenticationException_when_authentica
117124
var connection = new MockConnection(__serverId);
118125
connection.EnqueueReplyMessage(reply);
119126

120-
var act = async
127+
var act = async
121128
? () => subject.AuthenticateAsync(connection, __description, CancellationToken.None).GetAwaiter().GetResult()
122-
: (Action) (() => subject.Authenticate(connection, __description, CancellationToken.None));
129+
: (Action)(() => subject.Authenticate(connection, __description, CancellationToken.None));
123130

124131
var exception = Record.Exception(act);
125-
132+
126133
exception.Should().BeOfType<MongoAuthenticationException>();
127134
}
128135

@@ -136,20 +143,20 @@ public void Authenticate_should_throw_when_server_provides_invalid_r_value(
136143
var poisonedSaslStart = PoisonSaslMessage(message: _clientRequest1, poison: "bluePill");
137144
var poisonedSaslStartReply = CreateSaslStartReply(poisonedSaslStart, _serverNonce, _serverSalt, _iterationCount);
138145
var poisonedSaslStartReplyMessage = MessageHelper.BuildReply(RawBsonDocumentHelper.FromJson(
139-
@"{conversationId: 1, " +
146+
@"{conversationId: 1, " +
140147
$" payload: BinData(0,\"{ToUtf8Base64(poisonedSaslStartReply)}\")," +
141148
@" done: false,
142149
ok: 1}"));
143150

144151
var connection = new MockConnection(__serverId);
145152
connection.EnqueueReplyMessage(poisonedSaslStartReplyMessage);
146153

147-
var act = async
154+
var act = async
148155
? () => subject.AuthenticateAsync(connection, __description, CancellationToken.None).GetAwaiter().GetResult()
149-
: (Action) (() => subject.Authenticate(connection, __description, CancellationToken.None));
156+
: (Action)(() => subject.Authenticate(connection, __description, CancellationToken.None));
150157

151158
var exception = Record.Exception(act);
152-
159+
153160
exception.Should().BeOfType<MongoAuthenticationException>();
154161
}
155162

@@ -160,17 +167,17 @@ public void Authenticate_should_throw_when_server_provides_invalid_serverSignatu
160167
{
161168
var randomStringGenerator = new ConstantRandomStringGenerator(_clientNonce);
162169
var subject = new ScramSha256Authenticator(__credential, randomStringGenerator);
163-
170+
164171
var saslStartReply = CreateSaslStartReply(_clientRequest1, _serverNonce, _serverSalt, _iterationCount);
165-
var poisonedSaslContinueReply = PoisonSaslMessage(message: _serverReponse2, poison: "redApple");
172+
var poisonedSaslContinueReply = PoisonSaslMessage(message: _serverReponse2, poison: "redApple");
166173
var saslStartReplyMessage = MessageHelper.BuildReply(RawBsonDocumentHelper.FromJson(
167-
@"{conversationId: 1, " +
174+
@"{conversationId: 1, " +
168175
$" payload: BinData(0,\"{ToUtf8Base64(saslStartReply)}\")," +
169176
@" done: false,
170177
ok: 1}"));
171178
var poisonedSaslContinueReplyMessage = MessageHelper.BuildReply(RawBsonDocumentHelper.FromJson(
172-
@"{conversationId: 1, "+
173-
$" payload: BinData(0,\"{ToUtf8Base64(poisonedSaslContinueReply)}\")," +
179+
@"{conversationId: 1, " +
180+
$" payload: BinData(0,\"{ToUtf8Base64(poisonedSaslContinueReply)}\")," +
174181
@" done: true,
175182
ok: 1}"));
176183

@@ -187,7 +194,7 @@ public void Authenticate_should_throw_when_server_provides_invalid_serverSignatu
187194
{
188195
act = () => subject.Authenticate(connection, __description, CancellationToken.None);
189196
}
190-
197+
191198
var exception = Record.Exception(act);
192199

193200
exception.Should().BeOfType<MongoAuthenticationException>();
@@ -202,13 +209,13 @@ public void Authenticate_should_not_throw_when_authentication_succeeds(
202209
var subject = new ScramSha256Authenticator(__credential, randomStringGenerator);
203210

204211
var saslStartReply = MessageHelper.BuildReply<RawBsonDocument>(RawBsonDocumentHelper.FromJson(
205-
@"{conversationId: 1," +
212+
@"{conversationId: 1," +
206213
$" payload: BinData(0,\"{ToUtf8Base64(_serverResponse1)}\")," +
207214
@" done: false,
208215
ok: 1}"));
209216
var saslContinueReply = MessageHelper.BuildReply<RawBsonDocument>(RawBsonDocumentHelper.FromJson(
210217
@"{conversationId: 1," +
211-
$" payload: BinData(0,\"{ToUtf8Base64(_serverReponse2)}\")," +
218+
$" payload: BinData(0,\"{ToUtf8Base64(_serverReponse2)}\")," +
212219
@" done: true,
213220
ok: 1}"));
214221

@@ -241,24 +248,24 @@ public void Authenticate_should_not_throw_when_authentication_succeeds(
241248
actualRequestId1.Should().BeInRange(actualRequestId0 + 1, actualRequestId0 + 11);
242249

243250
sentMessages[0].Should().Be(
244-
@"{opcode: ""query""," +
251+
@"{opcode: ""query""," +
245252
$" requestId: {actualRequestId0}," +
246253
@" database: ""source"",
247254
collection: ""$cmd"",
248255
batchSize: -1,
249256
slaveOk: true,
250257
query: {saslStart: 1,
251-
mechanism: ""SCRAM-SHA-256""," +
258+
mechanism: ""SCRAM-SHA-256""," +
252259
$" payload: new BinData(0, \"{ToUtf8Base64(_clientRequest1)}\")}}}}");
253260
sentMessages[1].Should().Be(
254-
@"{opcode: ""query""," +
261+
@"{opcode: ""query""," +
255262
$" requestId: {actualRequestId1}," +
256263
@" database: ""source"",
257264
collection: ""$cmd"",
258265
batchSize: -1,
259266
slaveOk: true,
260267
query: {saslContinue: 1,
261-
conversationId: 1, " +
268+
conversationId: 1, " +
262269
$" payload: new BinData(0, \"{ToUtf8Base64(_clientRequest2)}\")}}}}");
263270
}
264271

@@ -302,6 +309,54 @@ public void Authenticate_should_use_cache(
302309
subject._cache()._cacheKey().Should().NotBe(null);
303310
subject._cache()._cachedEntry().Should().NotBe(null);
304311
}
312+
313+
#if !NETCOREAPP1_1
314+
[Theory]
315+
[ParameterAttributeData]
316+
public void Authenticate_should_work_regardless_of_culture(
317+
[Values("da-DK", "en-US")] string name,
318+
[Values(false, true)] bool async)
319+
{
320+
SetCultureAndResetAfterTest(name, () =>
321+
{
322+
var randomStringGenerator = new ConstantRandomStringGenerator("a");
323+
324+
// ScramSha1Authenticator will have exactly the same code paths
325+
var subject = new ScramSha256Authenticator(__credential, randomStringGenerator);
326+
var mockConnection = new MockConnection();
327+
328+
var payload1 = $"r=aa,s={_serverSalt},i=1";
329+
var serverResponse1 = $"{{ ok : 1, payload : BinData(0,\"{ToUtf8Base64(payload1)}\"), done : true, conversationId : 1 }}";
330+
var serverResponseRawDocument1 = RawBsonDocumentHelper.FromJson(serverResponse1);
331+
var serverResponseMessage1 = MessageHelper.BuildReply(serverResponseRawDocument1);
332+
333+
var payload2 = $"v=v1wZS02d7kZVSzuKoB7TuI+jIpSsKvnQUkU9Oqj2t+w=";
334+
var serverResponse2 = $"{{ ok : 1, payload : BinData(0,\"{ToUtf8Base64(payload2)}\"), done : true }}";
335+
var serverResponseRawDocument2 = RawBsonDocumentHelper.FromJson(serverResponse2);
336+
var serverResponseMessage2 = MessageHelper.BuildReply(serverResponseRawDocument2);
337+
338+
mockConnection.EnqueueReplyMessage(serverResponseMessage1);
339+
mockConnection.EnqueueReplyMessage(serverResponseMessage2);
340+
341+
Authenticate(subject, mockConnection, async);
342+
});
343+
344+
void SetCultureAndResetAfterTest(string cultureName, Action test)
345+
{
346+
var originalCulture = Thread.CurrentThread.CurrentCulture;
347+
Thread.CurrentThread.CurrentCulture = new System.Globalization.CultureInfo(cultureName);
348+
349+
try
350+
{
351+
test();
352+
}
353+
finally
354+
{
355+
Thread.CurrentThread.CurrentCulture = originalCulture;
356+
}
357+
}
358+
}
359+
#endif
305360
}
306361

307362
internal static class ScramShaAuthenticatorReflector
@@ -313,10 +368,10 @@ public static ScramCache _cache(this ScramShaAuthenticator obj) =>
313368

314369
internal static class ScramCacheReflector
315370
{
316-
public static ScramCacheKey _cacheKey (this ScramCache obj) =>
371+
public static ScramCacheKey _cacheKey(this ScramCache obj) =>
317372
(ScramCacheKey)Reflector.GetFieldValue(obj, nameof(_cacheKey));
318373

319-
public static ScramCacheEntry _cachedEntry (this ScramCache obj) =>
374+
public static ScramCacheEntry _cachedEntry(this ScramCache obj) =>
320375
(ScramCacheEntry)Reflector.GetFieldValue(obj, nameof(_cachedEntry));
321376
}
322377
}

0 commit comments

Comments
 (0)