From df9d8ee6cbc2163d3adea0063fe8dff5d4844ef2 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Thu, 15 Dec 2016 12:47:36 -0800 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/10294 Reviewed-By: Fedor Indutny Reviewed-By: Ben Noordhuis --- doc/api/tls.md | 13 ++-- lib/_tls_common.js | 12 +--- src/node_crypto.cc | 5 +- test/parallel/test-tls-passphrase.js | 97 ++++++++++++++++++++++++---- 4 files changed, 97 insertions(+), 30 deletions(-) diff --git a/doc/api/tls.md b/doc/api/tls.md index 0e47ddac03..f83ed1df3e 100644 --- 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: , passphrase: }`. 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: + [, passphrase: ]}`. 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 index e161a48486..107c3bb2ea 100644 --- 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 index 96f731f45e..db01533988 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -446,7 +446,10 @@ void SecureContext::SetKey(const FunctionCallbackInfo& 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 index 319c3511dc..4630fe236d 100644 --- a/test/parallel/test-tls-passphrase.js +++ b/test/parallel/test-tls-passphrase.js @@ -51,13 +51,12 @@ 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], @@ -65,7 +64,6 @@ server.listen(0, common.mustCall(function() { 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,13 +99,12 @@ 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()], @@ -115,7 +112,6 @@ server.listen(0, common.mustCall(function() { 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,23 +161,22 @@ 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() {})); @@ -173,7 +184,7 @@ server.listen(0, 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/);