From a11ff31bca6b24c4f3902e0cd1db4265aa569a48 Mon Sep 17 00:00:00 2001 From: CallMeLaNN Date: Mon, 27 Dec 2021 22:14:56 +0800 Subject: [PATCH] tls: permit null as a pfx value 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: https://github.com/nodejs/node/issues/36292 PR-URL: https://github.com/nodejs/node/pull/41170 Reviewed-By: Matteo Collina Reviewed-By: Filip Skokan --- lib/internal/tls/secure-context.js | 40 ++++++++++--------- .../test-tls-connect-secure-context.js | 28 +++++++++++++ 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/lib/internal/tls/secure-context.js b/lib/internal/tls/secure-context.js index 50a68df092c981..d5a447adde84b3 100644 --- a/lib/internal/tls/secure-context.js +++ b/lib/internal/tls/secure-context.js @@ -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); } @@ -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 === '') @@ -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); @@ -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, @@ -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); @@ -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)); @@ -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`, @@ -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); } diff --git a/test/parallel/test-tls-connect-secure-context.js b/test/parallel/test-tls-connect-secure-context.js index afa98cf3313b6f..31941656c09a05 100644 --- a/test/parallel/test-tls-connect-secure-context.js +++ b/test/parallel/test-tls-connect-secure-context.js @@ -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(); +});