fix(NODE-3118): keyAltNames option not serialized

bindings/node/lib/clientEncryption.js

@@ -9,43 +9,6 @@ module.exports = function(modules) {
   const StateMachine = modules.stateMachine.StateMachine;
   const cryptoCallbacks = require('./cryptoCallbacks');
 
-  function sanitizeDataKeyOptions(bson, options) {
-    options = Object.assign({}, options);
-
-    // To avoid using libbson inside the bindings, we pre-serialize
-    // any keyAltNames here.
-    if (options.keyAltNames) {
-      if (!Array.isArray(options.keyAltNames)) {
-        throw new TypeError(
-          `Option "keyAltNames" must be an array of string, but was of type ${typeof options.keyAltNames}.`
-        );
-      }
-      const serializedKeyAltNames = [];
-      for (let i = 0; i < options.keyAltNames.length; i += 1) {
-        const item = options.keyAltNames[i];
-        const itemType = typeof item;
-        if (itemType !== 'string') {
-          throw new TypeError(
-            `Option "keyAltNames" must be an array of string, but item at index ${i} was of type ${itemType} `
-          );
-        }
-
-        serializedKeyAltNames.push(bson.serialize({ keyAltName: item }));
-      }
-
-      options.keyAltNames = serializedKeyAltNames;
-    } else if (options.keyAltNames == null) {
-      // If keyAltNames is null or undefined, we can assume the intent of
-      // the user is to not pass in the value. B/c Nan::Has will still
-      // register a value of null or undefined as present as long
-      // as the key is present, we delete it off of the options
-      // object here.
-      delete options.keyAltNames;
-    }
-
-    return options;
-  }
-
   /**
    * @typedef {object} KMSProviders
    * @description Configuration options that are used by specific KMS providers during key generation, encryption, and decryption.
@@ -202,12 +165,37 @@ module.exports = function(modules) {
      * });
      */
     createDataKey(provider, options, callback) {
-      if (typeof options === 'function') (callback = options), (options = {});
+      if (typeof options === 'function') {
+        callback = options;
+        options = {};
+      }
+      if (typeof options === 'undefined') {
+        options = {};
+      }
 
       const bson = this._bson;
-      options = sanitizeDataKeyOptions(bson, options);
-      const dataKeyBson = bson.serialize(Object.assign({ provider }, options.masterKey));
-      const context = this._mongoCrypt.makeDataKeyContext(dataKeyBson);
+
+      const dataKey = Object.assign({ provider }, options.masterKey);
+
+      let keyAltNames;
+      if (Array.isArray(options.keyAltNames) && options.keyAltNames.length > 0) {
+        keyAltNames = options.keyAltNames.map((keyAltName, i) => {
+          if (typeof keyAltName !== 'string') {
+            throw new TypeError(
+              `Option "keyAltNames" must be an array of string, but item at index ${i} was of type ${typeof keyAltName}`
+            );
+          }
+
+          return bson.serialize({ keyAltName });
+        });
+      } else if (options.keyAltNames && !Array.isArray(options.keyAltNames)) {
+        throw new TypeError(
+          `Option "keyAltNames" must be an array of string, but was of type ${typeof options.keyAltNames}.`
+        );
+      }
+
+      const dataKeyBson = bson.serialize(dataKey);
+      const context = this._mongoCrypt.makeDataKeyContext(dataKeyBson, { keyAltNames });
       const stateMachine = new StateMachine({ bson });
 
       return promiseOrCallback(callback, cb => {
@@ -223,7 +211,7 @@ module.exports = function(modules) {
           this._keyVaultClient
             .db(dbName)
             .collection(collectionName)
-            .insertOne(dataKey, { w: 'majority' }, (err, result) => {
+            .insertOne(dataKey, { writeConcern: { w: 'majority' } }, (err, result) => {
               if (err) {
                 cb(err, null);
                 return;

bindings/node/lib/cryptoCallbacks.js

@@ -87,9 +87,16 @@ function signRsaSha256Hook(key, input, output) {
   let result;
   try {
     const signer = crypto.createSign('sha256WithRSAEncryption');
-    const privateKey = Buffer.from(
-      `-----BEGIN PRIVATE KEY-----\n${key.toString('base64')}\n-----END PRIVATE KEY-----\n`
-    );
+    const privateKeyString = `-----BEGIN PRIVATE KEY-----\n${key.toString(
+      'base64'
+    )}\n-----END PRIVATE KEY-----\n`;
+
+    let privateKey;
+    if (crypto.createPrivateKey) {
+      privateKey = crypto.createPrivateKey(privateKeyString);
+    } else {
+      privateKey = Buffer.from(privateKeyString);
+    }
 
     result = signer
       .update(input)

bindings/node/lib/stateMachine.js

@@ -2,7 +2,11 @@
 
 module.exports = function(modules) {
   const tls = require('tls');
-  const MongoTimeoutError = modules.mongodb.MongoTimeoutError;
+
+  // Try first to import 4.x name, fallback to 3.x name
+  const MongoNetworkTimeoutError =
+    modules.mongodb.MongoNetworkTimeoutError || modules.mongodb.MongoTimeoutError;
+
   const common = require('./common');
   const debug = common.debug;
   const databaseNamespace = common.databaseNamespace;
@@ -59,7 +63,7 @@ module.exports = function(modules) {
    * @ignore
    * @callback StateMachine~fetchKeysCallback
    * @param {Error} [err] If present, indicates that fetching the keys failed with the given error
-   * @param {object[]} [result] If present, is all the keys from the keyVault colleciton that matched the given filter
+   * @param {object[]} [result] If present, is all the keys from the keyVault collection that matched the given filter
    */
 
   /**
@@ -116,7 +120,7 @@ module.exports = function(modules) {
             if (err) {
               // If we are not bypassing spawning, then we should retry once on a MongoTimeoutError (server selection error)
               if (
-                err instanceof MongoTimeoutError &&
+                err instanceof MongoNetworkTimeoutError &&
                 mongocryptdManager &&
                 !mongocryptdManager.bypassSpawn
               ) {

bindings/node/package.json

@@ -38,15 +38,16 @@
     "prebuild-install": "6.1.2"
   },
   "devDependencies": {
-    "bson": "^4.2.3",
+    "bson": "^4.4.0",
     "chai": "^4.3.4",
     "chai-subset": "^1.6.0",
+    "clang-format": "^1.5.0",
     "dmd-clear": "^0.1.2",
     "eslint": "^4.19.1",
     "eslint-plugin-prettier": "^2.7.0",
     "jsdoc-to-markdown": "^5.0.3",
     "mocha": "^4.1.0",
-    "mongodb": "^3.5.9",
+    "mongodb": "^3.6.9",
     "node-gyp": "^5.1.1",
     "prebuild": "^10.0.1",
     "prettier": "^1.19.1",

bindings/node/src/mongocrypt.cc

@@ -674,6 +674,43 @@ NAN_METHOD(MongoCrypt::MakeDataKeyContext) {
         return;
     }
 
+    if (info[1]->IsObject()) {
+        v8::Local<v8::Object> options = Nan::To<v8::Object>(info[1]).ToLocalChecked();
+        auto ALT_NAMES_KEY = Nan::New("keyAltNames").ToLocalChecked();
+        if (Nan::Has(options, ALT_NAMES_KEY).FromMaybe(false)) {
+            v8::Local<v8::Value> keyAltNames = Nan::Get(options, ALT_NAMES_KEY).ToLocalChecked();
+
+            if (!(keyAltNames->IsNull() || keyAltNames->IsUndefined())) {
+                if (!keyAltNames->IsArray()) {
+                    Nan::ThrowTypeError("keyAltNames must be an array of strings");
+                    return;
+                }
+
+                v8::Local<v8::Array> keyAltNamesArray = v8::Local<v8::Array>::Cast(keyAltNames);
+                uint32_t keyAltNamesLength = keyAltNamesArray->Length();
+                for (uint32_t i = 0; i < keyAltNamesLength; i += 1) {
+                    if (Nan::Has(keyAltNamesArray, i).FromMaybe(false)) {
+                        v8::Local<v8::Object> keyAltName =
+                            Nan::To<v8::Object>(Nan::Get(keyAltNamesArray, i).ToLocalChecked())
+                                .ToLocalChecked();
+                        if (!node::Buffer::HasInstance(keyAltName)) {
+                            // We should never get here
+                            Nan::ThrowTypeError("Serialized keyAltName must be a Buffer");
+                            return;
+                        }
+
+                        std::unique_ptr<mongocrypt_binary_t, MongoCryptBinaryDeleter> binary(
+                            BufferToBinary(keyAltName));
+                        if (!mongocrypt_ctx_setopt_key_alt_name(context.get(), binary.get())) {
+                            Nan::ThrowTypeError(errorStringFromStatus(context.get()));
+                            return;
+                        }
+                    }
+                }
+            }
+        }
+    }
+
     if (!mongocrypt_ctx_datakey_init(context.get())) {
         Nan::ThrowTypeError(errorStringFromStatus(context.get()));
         return;

bindings/node/test/autoEncrypter.test.js

@@ -4,11 +4,8 @@ const fs = require('fs');
 const BSON = require('bson');
 const EJSON = require('bson').EJSON;
 const sinon = require('sinon');
-const mongodb = Object.assign({}, require('mongodb'), {
-  // TODO: once this is actually defined, use the real one
-  MongoTimeoutError: class MongoTimeoutError {}
-});
-const MongoTimeoutError = mongodb.MongoTimeoutError;
+const mongodb = require('mongodb');
+const MongoNetworkTimeoutError = mongodb.MongoNetworkTimeoutError || mongodb.MongoTimeoutError;
 const stateMachine = require('../lib/stateMachine')({ mongodb });
 const StateMachine = stateMachine.StateMachine;
 const MongocryptdManager = require('../lib/mongocryptdManager').MongocryptdManager;
@@ -277,12 +274,12 @@ describe('AutoEncrypter', function() {
       });
     });
 
-    it('should restore the mongocryptd and retry once if a MongoTimeoutError is experienced', function(done) {
+    it('should restore the mongocryptd and retry once if a MongoNetworkTimeoutError is experienced', function(done) {
       let called = false;
       StateMachine.prototype.markCommand.callsFake((client, ns, filter, callback) => {
         if (!called) {
           called = true;
-          callback(new MongoTimeoutError('msg'));
+          callback(new MongoNetworkTimeoutError('msg'));
           return;
         }
 
@@ -313,12 +310,12 @@ describe('AutoEncrypter', function() {
       });
     });
 
-    it('should propagate error if MongoTimeoutError is experienced twice in a row', function(done) {
+    it('should propagate error if MongoNetworkTimeoutError is experienced twice in a row', function(done) {
       let counter = 2;
       StateMachine.prototype.markCommand.callsFake((client, ns, filter, callback) => {
         if (counter) {
           counter -= 1;
-          callback(new MongoTimeoutError('msg'));
+          callback(new MongoNetworkTimeoutError('msg'));
           return;
         }
 
@@ -343,7 +340,7 @@ describe('AutoEncrypter', function() {
 
         this.mc.encrypt('test.test', TEST_COMMAND, err => {
           expect(localMcdm.spawn).to.have.been.calledOnce;
-          expect(err).to.be.an.instanceof(MongoTimeoutError);
+          expect(err).to.be.an.instanceof(MongoNetworkTimeoutError);
           done();
         });
       });
@@ -423,7 +420,7 @@ describe('AutoEncrypter', function() {
 
     it('should not spawn a mongocryptd or retry on a server selection error', function(done) {
       let called = false;
-      const timeoutError = new MongoTimeoutError('msg');
+      const timeoutError = new MongoNetworkTimeoutError('msg');
       StateMachine.prototype.markCommand.callsFake((client, ns, filter, callback) => {
         if (!called) {
           called = true;