Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

tls: allow obvious key/passphrase combinations #10294

Closed
wants to merge 1 commit into from
Closed

tls: allow obvious key/passphrase combinations #10294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions doc/api/tls.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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/);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate of the preceding test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, thank you.


// 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/);