Skip to content

Commit d256d1b

Browse files
committed
Ensure server proof has been validated during SCRAM conversation
JAVA-3527
1 parent 95b27cf commit d256d1b

File tree

2 files changed

+87
-7
lines changed

2 files changed

+87
-7
lines changed

driver-core/src/main/com/mongodb/internal/connection/SaslAuthenticator.java

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,14 @@ public Void run() {
6666

6767
res = sendSaslContinue(conversationId, response, connection);
6868
}
69+
if (!saslClient.isComplete()) {
70+
saslClient.evaluateChallenge((res.getBinary("payload")).getData());
71+
if (!saslClient.isComplete()) {
72+
throw new MongoSecurityException(getMongoCredential(),
73+
"SASL protocol error: server completed challenges before client completed responses "
74+
+ getMongoCredential());
75+
}
76+
}
6977
} catch (Exception e) {
7078
throw wrapException(e);
7179
} finally {
@@ -93,7 +101,7 @@ public void onResult(final BsonDocument result, final Throwable t) {
93101
if (t != null) {
94102
callback.onResult(null, wrapException(t));
95103
} else if (result.getBoolean("done").getValue()) {
96-
callback.onResult(null, null);
104+
verifySaslClientComplete(saslClient, result, callback);
97105
} else {
98106
new Continuator(saslClient, result, connection, callback).start();
99107
}
@@ -121,6 +129,26 @@ private void throwIfSaslClientIsNull(final SaslClient saslClient) {
121129
}
122130
}
123131

132+
private void verifySaslClientComplete(final SaslClient saslClient, final BsonDocument result,
133+
final SingleResultCallback<Void> callback) {
134+
if (saslClient.isComplete()) {
135+
callback.onResult(null, null);
136+
} else {
137+
try {
138+
saslClient.evaluateChallenge(result.getBinary("payload").getData());
139+
if (saslClient.isComplete()) {
140+
callback.onResult(null, null);
141+
} else {
142+
callback.onResult(null, new MongoSecurityException(getMongoCredential(),
143+
"SASL protocol error: server completed challenges before client completed responses "
144+
+ getMongoCredential()));
145+
}
146+
} catch (SaslException e) {
147+
callback.onResult(null, wrapException(e));
148+
}
149+
}
150+
}
151+
124152
@Nullable
125153
private Subject getSubject() {
126154
return getMongoCredential().getMechanismProperty(JAVA_SUBJECT_KEY, null);
@@ -202,7 +230,7 @@ public void onResult(final BsonDocument result, final Throwable t) {
202230
callback.onResult(null, wrapException(t));
203231
disposeOfSaslClient(saslClient);
204232
} else if (result.getBoolean("done").getValue()) {
205-
callback.onResult(null, null);
233+
verifySaslClientComplete(saslClient, result, callback);
206234
disposeOfSaslClient(saslClient);
207235
} else {
208236
continueConversation(result);

driver-core/src/test/unit/com/mongodb/internal/connection/ScramShaAuthenticatorSpecification.groovy

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -360,13 +360,65 @@ class ScramShaAuthenticatorSpecification extends Specification {
360360
async << [true, false]
361361
}
362362

363-
def createConnection(List<String> serverResponses) {
363+
def 'should complete authentication when done is set to true prematurely SHA-256'() {
364+
given:
365+
def serverResponses = createMessages('''
366+
S: r=rOprNGfwEbeRWgbNEkqO%hvYDpWUa2RaTCAfuxFIlj)hNlF$k0,s=W22ZaJ0SNY7soEsUEjb6gQ==,i=4096
367+
S: v=6rriTRBi23WpRR/wtup+mMhUZUn/dB5nLTJRsjl95G4=
368+
''').last()
369+
def authenticator = new ScramShaAuthenticator(SHA256_CREDENTIAL, { 'rOprNGfwEbeRWgbNEkqO' }, { 'pencil' })
370+
371+
when:
372+
// server sends done=true on first response, client is not complete after processing response
373+
authenticate(createConnection(serverResponses, 0), authenticator, async)
374+
375+
then:
376+
def e = thrown(MongoSecurityException)
377+
e.getMessage().contains('server completed challenges before client completed responses')
378+
379+
when:
380+
// server sends done=true on second response, client is complete after processing response
381+
authenticate(createConnection(serverResponses, 1), authenticator, async)
382+
383+
then:
384+
noExceptionThrown()
385+
386+
where:
387+
async << [true, false]
388+
}
389+
390+
def 'should throw exception when done is set to true prematurely and server response is invalid SHA-256'() {
391+
given:
392+
def serverResponses = createMessages('''
393+
S: r=rOprNGfwEbeRWgbNEkqO%hvYDpWUa2RaTCAfuxFIlj)hNlF$k0,s=W22ZaJ0SNY7soEsUEjb6gQ==,i=4096
394+
S: v=invalidResponse
395+
''').last()
396+
def authenticator = new ScramShaAuthenticator(SHA256_CREDENTIAL, { 'rOprNGfwEbeRWgbNEkqO' }, { 'pencil' })
397+
398+
when:
399+
// server sends done=true on second response, client throws exception on invalid server response
400+
authenticate(createConnection(serverResponses, 1), authenticator, async)
401+
402+
then:
403+
def e = thrown(MongoSecurityException)
404+
e.getCause() instanceof SaslException
405+
e.getCause().getMessage() == 'Server signature was invalid.'
406+
407+
where:
408+
async << [true, false]
409+
}
410+
411+
def createConnection(List<String> serverResponses, int responseWhereDoneIsTrue = -1) {
364412
TestInternalConnection connection = new TestInternalConnection(serverId)
365-
serverResponses.each {
413+
serverResponses.eachWithIndex { response, index ->
414+
def isDone = (index == responseWhereDoneIsTrue).booleanValue()
366415
connection.enqueueReply(
367-
buildSuccessfulReply("{conversationId: 1, payload: BinData(0, '${encode64(it)}'), done: false, ok: 1}")
368-
) }
369-
connection.enqueueReply(buildSuccessfulReply('{conversationId: 1, done: true, ok: 1}'))
416+
buildSuccessfulReply("{conversationId: 1, payload: BinData(0, '${encode64(response)}'), done: ${isDone}, ok: 1}")
417+
)
418+
}
419+
if (responseWhereDoneIsTrue < 0) {
420+
connection.enqueueReply(buildSuccessfulReply('{conversationId: 1, done: true, ok: 1}'))
421+
}
370422
connection
371423
}
372424

0 commit comments

Comments
 (0)