From 806cd627741813e38d098c39f1c65694e34bb185 Mon Sep 17 00:00:00 2001
From: Matt Broadstone <mbroadst@mongodb.com>
Date: Fri, 6 Dec 2019 16:39:32 -0500
Subject: [PATCH] fix(scram): verify server digest, ensuring mutual
 authentication

NODE-2376
---
 lib/core/auth/scram.js                  | 48 +++++++++++++++++++++----
 test/unit/core/scram_iterations.test.js | 47 ++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 6 deletions(-)

diff --git a/lib/core/auth/scram.js b/lib/core/auth/scram.js
index ac8853eb33..b65a076cea 100644
--- a/lib/core/auth/scram.js
+++ b/lib/core/auth/scram.js
@@ -19,7 +19,6 @@ try {
 var parsePayload = function(payload) {
   var dict = {};
   var parts = payload.split(',');
-
   for (var i = 0; i < parts.length; i++) {
     var valueParts = parts[i].split('=');
     dict[valueParts[0]] = valueParts[1];
@@ -105,6 +104,23 @@ function HI(data, salt, iterations, cryptoMethod) {
   return saltedData;
 }
 
+function compareDigest(lhs, rhs) {
+  if (lhs.length !== rhs.length) {
+    return false;
+  }
+
+  if (typeof crypto.timingSafeEqual === 'function') {
+    return crypto.timingSafeEqual(lhs, rhs);
+  }
+
+  let result = 0;
+  for (let i = 0; i < lhs.length; i++) {
+    result |= lhs[i] ^ rhs[i];
+  }
+
+  return result === 0;
+}
+
 /**
  * Creates a new ScramSHA authentication mechanism
  * @class
@@ -179,9 +195,19 @@ class ScramSHA extends AuthProvider {
 
       const payload = Buffer.isBuffer(r.payload) ? new Binary(r.payload) : r.payload;
       const dict = parsePayload(payload.value());
+
       const iterations = parseInt(dict.i, 10);
+      if (iterations && iterations < 4096) {
+        callback(new MongoError(`Server returned an invalid iteration count ${iterations}`), false);
+        return;
+      }
+
       const salt = dict.s;
       const rnonce = dict.r;
+      if (rnonce.startsWith('nonce')) {
+        callback(new MongoError(`Server returned an invalid nonce: ${rnonce}`), false);
+        return;
+      }
 
       // Set up start of proof
       const withoutProof = `c=biws,r=${rnonce}`;
@@ -192,18 +218,17 @@ class ScramSHA extends AuthProvider {
         cryptoMethod
       );
 
-      if (iterations && iterations < 4096) {
-        const error = new MongoError(`Server returned an invalid iteration count ${iterations}`);
-        return callback(error, false);
-      }
-
       const clientKey = HMAC(cryptoMethod, saltedPassword, 'Client Key');
+      const serverKey = HMAC(cryptoMethod, saltedPassword, 'Server Key');
       const storedKey = H(cryptoMethod, clientKey);
       const authMessage = [firstBare, payload.value().toString('base64'), withoutProof].join(',');
 
       const clientSignature = HMAC(cryptoMethod, storedKey, authMessage);
       const clientProof = `p=${xor(clientKey, clientSignature)}`;
       const clientFinal = [withoutProof, clientProof].join(',');
+
+      const serverSignature = HMAC(cryptoMethod, serverKey, authMessage);
+
       const saslContinueCmd = {
         saslContinue: 1,
         conversationId: r.conversationId,
@@ -211,6 +236,17 @@ class ScramSHA extends AuthProvider {
       };
 
       sendAuthCommand(connection, `${db}.$cmd`, saslContinueCmd, (err, r) => {
+        if (r && typeof r.ok === 'number' && r.ok === 0) {
+          callback(err, r);
+          return;
+        }
+
+        const parsedResponse = parsePayload(r.payload.value());
+        if (!compareDigest(Buffer.from(parsedResponse.v, 'base64'), serverSignature)) {
+          callback(new MongoError('Server returned an invalid signature'));
+          return;
+        }
+
         if (!r || r.done !== false) {
           return callback(err, r);
         }
diff --git a/test/unit/core/scram_iterations.test.js b/test/unit/core/scram_iterations.test.js
index 29ea25ff5c..47188c8563 100644
--- a/test/unit/core/scram_iterations.test.js
+++ b/test/unit/core/scram_iterations.test.js
@@ -65,4 +65,51 @@ describe('SCRAM Iterations Tests', function() {
 
     client.connect();
   });
+
+  it('should error if server digest is invalid', 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) {
+        return request.reply({
+          ok: 1,
+          done: false,
+          payload: Buffer.from('v=bWFsaWNpb3VzbWFsaWNpb3VzVzV')
+        });
+      }
+    });
+
+    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(/Server returned an invalid signature/);
+
+      client.destroy(done);
+    });
+
+    client.connect();
+  });
 });