tls: allow obvious key/passphrase combinations

Passphrase is now used whether keys are provided singly, in an array of string/buffer, or an array of object, where it used to be ignored in some argument combinations. Specifically, these now work as expected: key: [encryptedPem], passphrase: 'passphrase' and key: [{pem: encryptedPem}] passphrase: 'passphrase' and key: [{pem: unencryptedPem}] PR-URL: nodejs/node#10294 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
michielbaird · Jul 19, 2017 · 822b6c4 · 822b6c4
1 parent 7dcb8e6
commit 822b6c4
Show file tree

Hide file tree

Showing 4 changed files with 97 additions and 30 deletions.
diff --git a/doc/api/tls.md b/doc/api/tls.md
@@ -891,12 +891,13 @@ added: v0.11.13
     individually. PFX is usually encrypted, if it is, `passphrase` will be used
     to decrypt it.
   * `key` {string|string[]|Buffer|Buffer[]|Object[]} Optional private keys in
-    PEM format. Single keys will be decrypted with `passphrase` if necessary.
-    Multiple keys, probably using different algorithms, can be provided either
-    as an array of unencrypted key strings or buffers, or an array of objects in
-    the form `{pem: <string|buffer>, passphrase: <string>}`. The object form can
-    only occur in an array, and it _must_ include a passphrase, even if key is
-    not encrypted.
+    PEM format. PEM allows the option of private keys being encrypted. Encrypted
+    keys will be decrypted with `options.passphrase`.  Multiple keys using
+    different algorithms can be provided either as an array of unencrypted key
+    strings or buffers, or an array of objects in the form `{pem:
+    <string|buffer>[, passphrase: <string>]}`. The object form can only occur in
+    an array. `object.passphrase` is optional. Encrypted keys will be decrypted
+    with `object.passphrase` if provided, or `options.passphrase` if it is not.
   * `passphrase` {string} Optional shared passphrase used for a single private
     key and/or a PFX.
   * `cert` {string|string[]|Buffer|Buffer[]} Optional cert chains in PEM format.

diff --git a/lib/_tls_common.js b/lib/_tls_common.js
@@ -78,17 +78,11 @@ exports.createSecureContext = function createSecureContext(options, context) {
     if (Array.isArray(options.key)) {
       for (i = 0; i < options.key.length; i++) {
         const key = options.key[i];
-        if (key.passphrase)
-          c.context.setKey(key.pem, key.passphrase);
-        else
-          c.context.setKey(key);
+        const passphrase = key.passphrase || options.passphrase;
+        c.context.setKey(key.pem || key, passphrase);
       }
     } else {
-      if (options.passphrase) {
-        c.context.setKey(options.key, options.passphrase);
-      } else {
-        c.context.setKey(options.key);
-      }
+      c.context.setKey(options.key, options.passphrase);
     }
   }
 

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -446,7 +446,10 @@ void SecureContext::SetKey(const FunctionCallbackInfo<Value>& args) {
   }
 
   if (len == 2) {
-    THROW_AND_RETURN_IF_NOT_STRING(args[1], "Pass phrase");
+    if (args[1]->IsUndefined() || args[1]->IsNull())
+      len = 1;
+    else
+      THROW_AND_RETURN_IF_NOT_STRING(args[1], "Pass phrase");
   }
 
   BIO *bio = LoadBIO(env, args[0]);

diff --git a/test/parallel/test-tls-passphrase.js b/test/parallel/test-tls-passphrase.js
@@ -51,21 +51,19 @@ server.listen(0, common.mustCall(function() {
   tls.connect({
     port: this.address().port,
     key: rawKey,
-    passphrase: 'passphrase', // Ignored.
+    passphrase: 'ignored',
     cert: cert,
     rejectUnauthorized: false
   }, common.mustCall(function() {}));
 
   // Buffer[]
-  /* XXX(sam) Should work, but its unimplemented ATM.
   tls.connect({
     port: this.address().port,
     key: [passKey],
     passphrase: 'passphrase',
     cert: [cert],
     rejectUnauthorized: false
   }, common.mustCall(function() {}));
-  */
 
   tls.connect({
     port: this.address().port,
@@ -77,7 +75,7 @@ server.listen(0, common.mustCall(function() {
   tls.connect({
     port: this.address().port,
     key: [rawKey],
-    passphrase: 'passphrase', // Ignored.
+    passphrase: 'ignored',
     cert: [cert],
     rejectUnauthorized: false
   }, common.mustCall(function() {}));
@@ -101,21 +99,19 @@ server.listen(0, common.mustCall(function() {
   tls.connect({
     port: this.address().port,
     key: rawKey.toString(),
-    passphrase: 'passphrase', // Ignored.
+    passphrase: 'ignored',
     cert: cert.toString(),
     rejectUnauthorized: false
   }, common.mustCall(function() {}));
 
   // String[]
-  /* XXX(sam) Should work, but its unimplemented ATM.
   tls.connect({
     port: this.address().port,
     key: [passKey.toString()],
     passphrase: 'passphrase',
     cert: [cert.toString()],
     rejectUnauthorized: false
   }, common.mustCall(function() {}));
-  */
 
   tls.connect({
     port: this.address().port,
@@ -127,7 +123,7 @@ server.listen(0, common.mustCall(function() {
   tls.connect({
     port: this.address().port,
     key: [rawKey.toString()],
-    passphrase: 'passphrase', // Ignored.
+    passphrase: 'ignored',
     cert: [cert.toString()],
     rejectUnauthorized: false
   }, common.mustCall(function() {}));
@@ -140,6 +136,22 @@ server.listen(0, common.mustCall(function() {
     rejectUnauthorized: false
   }, common.mustCall(function() {}));
 
+  tls.connect({
+    port: this.address().port,
+    key: [{pem: passKey, passphrase: 'passphrase'}],
+    passphrase: 'ignored',
+    cert: cert,
+    rejectUnauthorized: false
+  }, common.mustCall(function() {}));
+
+  tls.connect({
+    port: this.address().port,
+    key: [{pem: passKey}],
+    passphrase: 'passphrase',
+    cert: cert,
+    rejectUnauthorized: false
+  }, common.mustCall(function() {}));
+
   tls.connect({
     port: this.address().port,
     key: [{pem: passKey.toString(), passphrase: 'passphrase'}],
@@ -149,31 +161,30 @@ server.listen(0, common.mustCall(function() {
 
   tls.connect({
     port: this.address().port,
-    key: [{pem: rawKey, passphrase: 'passphrase'}],
+    key: [{pem: rawKey, passphrase: 'ignored'}],
     cert: cert,
     rejectUnauthorized: false
   }, common.mustCall(function() {}));
 
   tls.connect({
     port: this.address().port,
-    key: [{pem: rawKey.toString(), passphrase: 'passphrase'}],
+    key: [{pem: rawKey.toString(), passphrase: 'ignored'}],
     cert: cert,
     rejectUnauthorized: false
   }, common.mustCall(function() {}));
 
-  /* XXX(sam) Should work, but unimplemented ATM
   tls.connect({
     port: this.address().port,
     key: [{pem: rawKey}],
-    passphrase: 'passphrase',
+    passphrase: 'ignored',
     cert: cert,
     rejectUnauthorized: false
   }, common.mustCall(function() {}));
 
   tls.connect({
     port: this.address().port,
     key: [{pem: rawKey.toString()}],
-    passphrase: 'passphrase',
+    passphrase: 'ignored',
     cert: cert,
     rejectUnauthorized: false
   }, common.mustCall(function() {}));
@@ -191,9 +202,37 @@ server.listen(0, common.mustCall(function() {
     cert: cert,
     rejectUnauthorized: false
   }, common.mustCall(function() {}));
-  */
 })).unref();
 
+// Missing passphrase
+assert.throws(function() {
+  tls.connect({
+    port: server.address().port,
+    key: passKey,
+    cert: cert,
+    rejectUnauthorized: false
+  });
+}, /bad password read/);
+
+assert.throws(function() {
+  tls.connect({
+    port: server.address().port,
+    key: [passKey],
+    cert: cert,
+    rejectUnauthorized: false
+  });
+}, /bad password read/);
+
+assert.throws(function() {
+  tls.connect({
+    port: server.address().port,
+    key: [{pem: passKey}],
+    cert: cert,
+    rejectUnauthorized: false
+  });
+}, /bad password read/);
+
+// Invalid passphrase
 assert.throws(function() {
   tls.connect({
     port: server.address().port,
@@ -203,3 +242,33 @@ assert.throws(function() {
     rejectUnauthorized: false
   });
 }, /bad decrypt/);
+
+assert.throws(function() {
+  tls.connect({
+    port: server.address().port,
+    key: [passKey],
+    passphrase: 'invalid',
+    cert: cert,
+    rejectUnauthorized: false
+  });
+}, /bad decrypt/);
+
+assert.throws(function() {
+  tls.connect({
+    port: server.address().port,
+    key: [{pem: passKey}],
+    passphrase: 'invalid',
+    cert: cert,
+    rejectUnauthorized: false
+  });
+}, /bad decrypt/);
+
+assert.throws(function() {
+  tls.connect({
+    port: server.address().port,
+    key: [{pem: passKey, passphrase: 'invalid'}],
+    passphrase: 'passphrase', // Valid but unused
+    cert: cert,
+    rejectUnauthorized: false
+  });
+}, /bad decrypt/);