Skip to content

Commit

Permalink
fix: always check for network errors during SCRAM conversation
Browse files Browse the repository at this point in the history
A recent change to verify proper server signatures during SCRAM
conversations introduced a subtle bug where not checking for
network errors could lead to a type error while trying to access
the `payload` of the result.

NODE-2390
  • Loading branch information
mbroadst committed Dec 19, 2019
1 parent e44f553 commit e46a70e
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/core/auth/scram.js
Expand Up @@ -236,7 +236,7 @@ class ScramSHA extends AuthProvider {
};

sendAuthCommand(connection, `${db}.$cmd`, saslContinueCmd, (err, r) => {
if (r && typeof r.ok === 'number' && r.ok === 0) {
if (err || (r && typeof r.ok === 'number' && r.ok === 0)) {
callback(err, r);
return;
}
Expand Down
43 changes: 43 additions & 0 deletions test/unit/core/scram_iterations.test.js
Expand Up @@ -112,4 +112,47 @@ describe('SCRAM Iterations Tests', function() {

client.connect();
});

it('should properly handle network errors on `saslContinue`', function(_done) {
const credentials = new MongoCredentials({
mechanism: 'default',
source: 'db',
username: 'user',
password: 'pencil'
});

let done = e => {
done = () => {};
return _done(e);
};

test.server.setMessageHandler(request => {
const doc = request.document;
if (doc.ismaster) {
return request.reply(Object.assign({}, mock.DEFAULT_ISMASTER));
} else if (doc.saslStart) {
return request.reply({
ok: 1,
done: false,
payload: Buffer.from(
'r=VNnXkRqKflB5+rmfnFiisCWzgDLzez02iRpbvE5mQjMvizb+VkSPRZZ/pDmFzLxq,s=dZTyOb+KZqoeTFdsULiqow==,i=10000'
)
});
} else if (doc.saslContinue) {
request.connection.destroy();
}
});

const client = new Server(Object.assign({}, test.server.address(), { credentials }));
client.on('error', err => {
expect(err).to.not.be.null;
expect(err)
.to.have.property('message')
.that.matches(/failed to connect to server/);

client.destroy(done);
});

client.connect();
});
});

0 comments on commit e46a70e

Please sign in to comment.