From 8fd11f9e35c1af568bc4f28acd3b12e5dc9d7f4a Mon Sep 17 00:00:00 2001 From: CallMeLaNN Date: Fri, 10 Dec 2021 20:52:41 +0800 Subject: [PATCH 1/2] 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 --- lib/internal/tls/secure-context.js | 2 +- .../parallel/test-tls-connect-secure-context.js | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/internal/tls/secure-context.js b/lib/internal/tls/secure-context.js index 50a68df092c981..05329417ea51a8 100644 --- a/lib/internal/tls/secure-context.js +++ b/lib/internal/tls/secure-context.js @@ -261,7 +261,7 @@ function configSecureContext(context, options = {}, name = 'options') { context.setSessionIdContext(sessionIdContext); } - if (pfx !== undefined) { + if (pfx != null) { if (ArrayIsArray(pfx)) { ArrayPrototypeForEach(pfx, (val) => { const raw = val.buf ? val.buf : val; diff --git a/test/parallel/test-tls-connect-secure-context.js b/test/parallel/test-tls-connect-secure-context.js index afa98cf3313b6f..78f7e1685beb1d 100644 --- a/test/parallel/test-tls-connect-secure-context.js +++ b/test/parallel/test-tls-connect-secure-context.js @@ -23,3 +23,20 @@ connect({ assert.ifError(err); return cleanup(); }); + +connect({ + client: { + servername: 'agent1', + secureContext: tls.createSecureContext({ + ca: keys.agent1.ca, + pfx: null, + }), + }, + server: { + cert: keys.agent1.cert, + key: keys.agent1.key, + }, +}, function(err, pair, cleanup) { + assert.ifError(err); + return cleanup(); +}); From f474aa5f722c477c59a618b87a8de42f718fd779 Mon Sep 17 00:00:00 2001 From: CallMeLaNN Date: Wed, 15 Dec 2021 15:01:36 +0800 Subject: [PATCH 2/2] tls: permit null as an options value Allow the expected null along with undefined for options value. This is to avoid breaking change when upgrading v14 to v16 and 3rd party library passing null to options Fixes: https://github.com/nodejs/node/issues/36292 --- lib/internal/tls/secure-context.js | 40 ++++++++++--------- .../test-tls-connect-secure-context.js | 11 +++++ 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/lib/internal/tls/secure-context.js b/lib/internal/tls/secure-context.js index 05329417ea51a8..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 != null) { + 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 78f7e1685beb1d..31941656c09a05 100644 --- a/test/parallel/test-tls-connect-secure-context.js +++ b/test/parallel/test-tls-connect-secure-context.js @@ -29,7 +29,18 @@ connect({ 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: {