Skip to content

Commit

Permalink
tls: permit null as a pfx value
Browse files Browse the repository at this point in the history 
Allow null along with undefined for pfx value.

This is to avoid breaking change when upgrading v14 to v16 and
3rd party library passing null to pfx

Fixes: #36292

PR-URL: #41170
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
  • Loading branch information
@CallMeLaNN
CallMeLaNN authored Dec 27, 2021
1 parent 9a85efa commit 077c75b
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 18 deletions.
40 changes: 22 additions & 18 deletions lib/internal/tls/secure-context.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ function validateKeyOrCertOption(name, value) {

function setKey(context, key, passphrase, name) {
validateKeyOrCertOption(`${name}.key`, key);
if (passphrase != null)
if (passphrase !== undefined && passphrase !== null)
validateString(passphrase, `${name}.passphrase`);
context.setKey(key, passphrase);
}
Expand Down Expand Up @@ -160,16 +160,20 @@ function configSecureContext(context, options = {}, name = 'options') {
if (ArrayIsArray(key)) {
for (let i = 0; i < key.length; ++i) {
const val = key[i];
// eslint-disable-next-line eqeqeq
const pem = (val != undefined && val.pem !== undefined ? val.pem : val);
setKey(context, pem, val.passphrase || passphrase, name);
const pem = (
val !== undefined && val !== null &&
val.pem !== undefined ? val.pem : val);
const pass = (
val !== undefined && val !== null &&
val.passphrase !== undefined ? val.passphrase : passphrase);
setKey(context, pem, pass, name);
}
} else {
setKey(context, key, passphrase, name);
}
}

if (sigalgs !== undefined) {
if (sigalgs !== undefined && sigalgs !== null) {
validateString(sigalgs, `${name}.sigalgs`);

if (sigalgs === '')
Expand All @@ -178,8 +182,8 @@ function configSecureContext(context, options = {}, name = 'options') {
context.setSigalgs(sigalgs);
}

if (privateKeyIdentifier !== undefined) {
if (privateKeyEngine === undefined) {
if (privateKeyIdentifier !== undefined && privateKeyIdentifier !== null) {
if (privateKeyEngine === undefined || privateKeyEngine === null) {
// Engine is required when privateKeyIdentifier is present
throw new ERR_INVALID_ARG_VALUE(`${name}.privateKeyEngine`,
privateKeyEngine);
Expand All @@ -198,16 +202,16 @@ function configSecureContext(context, options = {}, name = 'options') {
throw new ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED();
} else if (typeof privateKeyIdentifier !== 'string') {
throw new ERR_INVALID_ARG_TYPE(`${name}.privateKeyIdentifier`,
['string', 'undefined'],
['string', 'null', 'undefined'],
privateKeyIdentifier);
} else {
throw new ERR_INVALID_ARG_TYPE(`${name}.privateKeyEngine`,
['string', 'undefined'],
['string', 'null', 'undefined'],
privateKeyEngine);
}
}

if (ciphers != null)
if (ciphers !== undefined && ciphers !== null)
validateString(ciphers, `${name}.ciphers`);

// Work around an OpenSSL API quirk. cipherList is for TLSv1.2 and below,
Expand Down Expand Up @@ -237,14 +241,14 @@ function configSecureContext(context, options = {}, name = 'options') {
validateString(ecdhCurve, `${name}.ecdhCurve`);
context.setECDHCurve(ecdhCurve);

if (dhparam !== undefined) {
if (dhparam !== undefined && dhparam !== null) {
validateKeyOrCertOption(`${name}.dhparam`, dhparam);
const warning = context.setDHParam(dhparam);
if (warning)
process.emitWarning(warning, 'SecurityWarning');
}

if (crl !== undefined) {
if (crl !== undefined && crl !== null) {
if (ArrayIsArray(crl)) {
for (const val of crl) {
validateKeyOrCertOption(`${name}.crl`, val);
Expand All @@ -256,17 +260,17 @@ function configSecureContext(context, options = {}, name = 'options') {
}
}

if (sessionIdContext !== undefined) {
if (sessionIdContext !== undefined && sessionIdContext !== null) {
validateString(sessionIdContext, `${name}.sessionIdContext`);
context.setSessionIdContext(sessionIdContext);
}

if (pfx !== undefined) {
if (pfx !== undefined && pfx !== null) {
if (ArrayIsArray(pfx)) {
ArrayPrototypeForEach(pfx, (val) => {
const raw = val.buf ? val.buf : val;
const pass = val.passphrase || passphrase;
if (pass !== undefined) {
if (pass !== undefined && pass !== null) {
context.loadPKCS12(toBuf(raw), toBuf(pass));
} else {
context.loadPKCS12(toBuf(raw));
Expand All @@ -284,13 +288,13 @@ function configSecureContext(context, options = {}, name = 'options') {
throw new ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED();
else
context.setClientCertEngine(clientCertEngine);
} else if (clientCertEngine !== undefined) {
} else if (clientCertEngine !== undefined && clientCertEngine !== null) {
throw new ERR_INVALID_ARG_TYPE(`${name}.clientCertEngine`,
['string', 'null', 'undefined'],
clientCertEngine);
}

if (ticketKeys !== undefined) {
if (ticketKeys !== undefined && ticketKeys !== null) {
if (!isArrayBufferView(ticketKeys)) {
throw new ERR_INVALID_ARG_TYPE(
`${name}.ticketKeys`,
Expand All @@ -306,7 +310,7 @@ function configSecureContext(context, options = {}, name = 'options') {
context.setTicketKeys(ticketKeys);
}

if (sessionTimeout !== undefined) {
if (sessionTimeout !== undefined && sessionTimeout !== null) {
validateInt32(sessionTimeout, `${name}.sessionTimeout`);
context.setSessionTimeout(sessionTimeout);
}
Expand Down
28 changes: 28 additions & 0 deletions test/parallel/test-tls-connect-secure-context.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,31 @@ connect({
assert.ifError(err);
return cleanup();
});

connect({
client: {
servername: 'agent1',
secureContext: tls.createSecureContext({
ca: keys.agent1.ca,
ciphers: null,
clientCertEngine: null,
crl: null,
dhparam: null,
passphrase: null,
pfx: null,
privateKeyIdentifier: null,
privateKeyEngine: null,
sessionIdContext: null,
sessionTimeout: null,
sigalgs: null,
ticketKeys: null,
}),
},
server: {
cert: keys.agent1.cert,
key: keys.agent1.key,
},
}, function(err, pair, cleanup) {
assert.ifError(err);
return cleanup();
});

0 comments on commit 077c75b

Please sign in to comment.