Repository navigation
tls.connect() options ciphers no longer accept null as a valid value in node v15.3.0 #36292
Description
- Version:
node v15.3.0
- Platform:
Reproduced on Linux 5.9.0-3-amd64 #1 SMP Debian 5.9.9-1 (2020-11-19) x86_64 GNU/Linux - but probably applicable on all platform
- Subsystem:
tls.connect() options
What steps will reproduce the bug?
Before version 15.3.0 tls.connect (also accepted by https.request() ) option value null was accepted as falsy value for the cipthers option.
As of version 15.3.0, passing option.ciphers = null throw an error.
case.js (tweaked from https://nodejs.org/api/https.html#https_https_request_options_callback )
const https = require('https');
const options = {
hostname: 'nodejs.org',
port: 443,
path: '/en/',
ciphers: null,
method: 'GET'
};
const req = https.request(options, (res) => {
console.log('statusCode:', res.statusCode);
console.log('headers:', res.headers);
res.on('data', (d) => {
process.stdout.write(d);
});
});
req.on('error', (e) => {
console.error(e);
});
req.end();
You get a connection that end up with 200 OK using this v15.2.1 dockerfile
FROM node:15.2.1-buster-slim
COPY ./case.js ./
CMD node case.js
but it will throw an error with a v15.3.0 dockerfile
FROM node:15.3.0-buster-slim
COPY ./case.js ./
CMD node case.js
How often does it reproduce? Is there a required condition?
throw an error 100% of the time on v15.3.0 with options.ciphers = null
What is the expected behavior?
I have no doubt it is a changing behavior, but i don't know what was the expected behavior of an undocumented cipher option value in the first place either. i just know that it used to work.
What do you see instead?
behavior changed, it now throw an error :
node:internal/validators:123
throw new ERR_INVALID_ARG_TYPE(name, 'string', value);
^
TypeError [ERR_INVALID_ARG_TYPE]: The "options.ciphers" property must be of type string. Received null
at new NodeError (node:internal/errors:278:15)
at validateString (node:internal/validators:123:11)
at Object.createSecureContext (node:_tls_common:267:5)
at Object.connect (node:_tls_wrap:1581:48)
at Agent.createConnection (node:https:129:22)
at Agent.createSocket (node:_http_agent:323:26)
at Agent.addRequest (node:_http_agent:274:10)
at new ClientRequest (node:_http_client:318:16)
at Object.request (node:https:313:10)
at Object.<anonymous> (/case.js:11:19) {
code: 'ERR_INVALID_ARG_TYPE'
Additional information
- I stumbled upon this because it breaks real world npm module elasticsearch-legacy-client on v15.3.0 build (see https://github.com/elastic/elasticsearch-js-legacy/blob/16.x/src/lib/host.js#L44 )
- the new version's error message is clear and seems well-adapted to the given context.
- this change seem to be introduced by https://github.com/nodejs/node/pull/35368/files
The point of this report is to warn about this non-obvious breaking behavior change and not to say it's not acceptable/legit api change.
ps: Thanks to @jasnell for encouraging me to write an issue 👍
Activity
T1B0 commentedon Nov 27, 2020
cc @spalger for the https://github.com/elastic/elasticsearch-js-legacy/ part ^^
Trott commentedon Nov 29, 2020
I did a bisect and the commit that introduced this issue is 35274cb.
jasnell commentedon Nov 29, 2020
Yep, already planning on working on it on Monday.
Trott commentedon Nov 29, 2020
It's a one-line fix so I hope I'm not ruining your Monday plans/strategy.: #36318
tmadeira commentedon Dec 1, 2020
I think the same fix is needed for
dhparam !== undefined,crl !== undefined,sessionIdContext !== undefined,pfx !== undefined. Even settingciphershere I'm getting "Error: Unable to load PFX certificate" in a code that used to work before node v15.3.0.tls: permit null as a cipher value
Trott commentedon Dec 3, 2020
Reopening based on #36292 (comment). /ping @jasnell
tls: permit null as a cipher value
fix: issue with node > 14. See: nodejs/node#36292
CallMeLaNN commentedon Dec 10, 2021
Can we look into this. This is the only thing left before I can switch to latest LTS due to
Error: Unable to load PFX certificateThe APNS library https://github.com/parse-community/node-apn expect
pfx: nullif I configure to use pem cert. This is reproduceable in 16.13.1:The
lib/_tls_common.jsstructure from the PR above probably changed. I guess the remaining fixes should be in this file right?node/lib/internal/tls/secure-context.js
Line 264 in ecf4114
2 remaining items
mcollina commentedon Dec 14, 2021
Where does it do this? I could not find a reference to
pfxin that library.I see no problem in allowing
nullfor pfx, go for the PR!CallMeLaNN commentedon Dec 14, 2021
https://github.com/parse-community/node-apn/blob/master/doc/provider.markdown
Basically it passes their
optionstohttp2.connect(address, options)but somehowoptions.pfxis always null.How about
dhparam,crl,sessionIdContextas mentioned #36292 (comment)?I'm not sure of those options, but the pattern is very similar. Let me know if I can include those options in the PR.
mcollina commentedon Dec 14, 2021
I would just include them all, thanks.
fholzer commentedon Dec 14, 2021
In case anyone got here looking for a workaround, but for some reason you don't have the luxury of being able to set
pfxtoundefined: As an alternative you can setpfxto an empty array[]. That resolved the issue for me and my elasticsearch client. (So the elasticsearch client options would look like this:{ "ssl": { "pfx": [] } })tls: permit null as a pfx value
tls: permit null as an options value
alolis commentedon Dec 16, 2021
Thanks @fholzer , you saved the day.
tls: permit null as a pfx value
tls: permit null as a pfx value
tls: permit null as a pfx value
tls: permit null as a pfx value
fholzer commentedon Nov 29, 2022
To anyone still finding this when using the elasticsearch client...
Your options:
{ "ssl": { "pfx": [] } }option to ES client: tls.connect() options ciphers no longer accept null as a valid value in node v15.3.0 #36292 (comment)