Skip to content

Commit

Permalink
tls: allow obvious key/passphrase combinations
Browse files Browse the repository at this point in the history 
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: #10294
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
@sam-github @cjihrig
sam-github authored and cjihrig committed Dec 20, 2016
1 parent 2fbb998 commit 164ee0f
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 30 deletions.
13 changes: 7 additions & 6 deletions doc/api/tls.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -893,12 +893,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.
Expand Down
12 changes: 3 additions & 9 deletions lib/_tls_common.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/node_crypto.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,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]);
Expand Down
97 changes: 83 additions & 14 deletions test/parallel/test-tls-passphrase.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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() {}));
Expand All @@ -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,
Expand All @@ -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() {}));
Expand All @@ -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'}],
Expand All @@ -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() {}));
Expand All @@ -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,
Expand All @@ -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/);

0 comments on commit 164ee0f

Please sign in to comment.