tls: fix order of setting cipher before setting cert and key

Set the cipher list and cipher suite before anything else
because @SECLEVEL=<n> changes the security level and
that affects subsequent operations.

Fixes: https://github.com/nodejs/node/issues/36655
Fixes: https://github.com/nodejs/node/issues/49549
Refs: https://github.com/orgs/nodejs/discussions/49634
Refs: https://github.com/orgs/nodejs/discussions/46545
Refs: https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_security_level.html
PR-URL: https://github.com/nodejs/node/pull/50186
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
This commit is contained in:
Kumar Rishav 2023-10-16 19:55:36 +00:00 committed by Richard Lau
parent 59b27d6990
commit 1e0b75c3df
No known key found for this signature in database
GPG Key ID: C43CEC45C17AB93C
4 changed files with 68 additions and 22 deletions

View File

@ -148,6 +148,31 @@ function configSecureContext(context, options = kEmptyObject, name = 'options')
ticketKeys,
} = options;
// Set the cipher list and cipher suite before anything else because
// @SECLEVEL=<n> changes the security level and that affects subsequent
// operations.
if (ciphers !== undefined && ciphers !== null)
validateString(ciphers, `${name}.ciphers`);
// Work around an OpenSSL API quirk. cipherList is for TLSv1.2 and below,
// cipherSuites is for TLSv1.3 (and presumably any later versions). TLSv1.3
// cipher suites all have a standard name format beginning with TLS_, so split
// the ciphers and pass them to the appropriate API.
const {
cipherList,
cipherSuites,
} = processCiphers(ciphers, `${name}.ciphers`);
if (cipherSuites !== '')
context.setCipherSuites(cipherSuites);
context.setCiphers(cipherList);
if (cipherList === '' &&
context.getMinProto() < TLS1_3_VERSION &&
context.getMaxProto() > TLS1_2_VERSION) {
context.setMinProto(TLS1_3_VERSION);
}
// Add CA before the cert to be able to load cert's issuer in C++ code.
// NOTE(@jasnell): ca, cert, and key are permitted to be falsy, so do not
// change the checks to !== undefined checks.
@ -218,28 +243,6 @@ function configSecureContext(context, options = kEmptyObject, name = 'options')
}
}
if (ciphers !== undefined && ciphers !== null)
validateString(ciphers, `${name}.ciphers`);
// Work around an OpenSSL API quirk. cipherList is for TLSv1.2 and below,
// cipherSuites is for TLSv1.3 (and presumably any later versions). TLSv1.3
// cipher suites all have a standard name format beginning with TLS_, so split
// the ciphers and pass them to the appropriate API.
const {
cipherList,
cipherSuites,
} = processCiphers(ciphers, `${name}.ciphers`);
if (cipherSuites !== '')
context.setCipherSuites(cipherSuites);
context.setCiphers(cipherList);
if (cipherList === '' &&
context.getMinProto() < TLS1_3_VERSION &&
context.getMaxProto() > TLS1_2_VERSION) {
context.setMinProto(TLS1_3_VERSION);
}
validateString(ecdhCurve, `${name}.ecdhCurve`);
context.setECDHCurve(ecdhCurve);

8
test/fixtures/keys/agent11-cert.pem vendored Normal file
View File

@ -0,0 +1,8 @@
-----BEGIN CERTIFICATE-----
MIIBFjCBwaADAgECAgEBMA0GCSqGSIb3DQEBBQUAMBQxEjAQBgNVBAMTCWxvY2Fs
aG9zdDAeFw0yMzEwMTUxNzQ5MTBaFw0yNDEwMTUxNzQ5MTBaMBQxEjAQBgNVBAMT
CWxvY2FsaG9zdDBcMA0GCSqGSIb3DQEBAQUAA0sAMEgCQQDW9vH7W98zSi1IfoTG
pTjbvXRzmmSG6y5z1S3gvC6+keC5QQkEdIG5vWas1efX5qEPybptRyM34T6aWv+U
uzUJAgMBAAEwDQYJKoZIhvcNAQEFBQADQQAEIwD5mLIALrim6uD39DO/umYDtDIb
TAQmgWdkQrCdCtX0Yp49gJyaq2HtFgsk/cxMoYMYkDtT5a7nwEQu+Xqt
-----END CERTIFICATE-----

9
test/fixtures/keys/agent11-key.pem vendored Normal file
View File

@ -0,0 +1,9 @@
-----BEGIN RSA PRIVATE KEY-----
MIIBOwIBAAJBANb28ftb3zNKLUh+hMalONu9dHOaZIbrLnPVLeC8Lr6R4LlBCQR0
gbm9ZqzV59fmoQ/Jum1HIzfhPppa/5S7NQkCAwEAAQJAaetb6GKoY/lUvre4bLjU
f1Gmo5+bkO8pAGI2LNoMnlETjLjlnvShkqu0kxY96G5Il6VSX4Yjz0D40f4IrlJW
AQIhAPChOjGBlOFcGA/pPmzMcW8jRCLvVubiO9TpiYVhWz45AiEA5LIKsSR8HT9y
eyVNNNkRbNvTrddbvXMBBjj+KwxQrVECIQDjalzHQQJl4lXTY8rdpHJoaNoSckSd
PJ7zYCvaZOKI8QIhALoGbRYMxHySCJBNFlE/pKH06mnE/RXMf2/NWkov+UwRAiAz
ucgBN8xY5KvG3eI78WHdE2B5X0B4EabFXmUlzIrhTA==
-----END RSA PRIVATE KEY-----

View File

@ -0,0 +1,26 @@
'use strict';
const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const assert = require('assert');
const tls = require('tls');
const fixtures = require('../common/fixtures');
{
const options = {
key: fixtures.readKey('agent11-key.pem'),
cert: fixtures.readKey('agent11-cert.pem'),
ciphers: 'DEFAULT'
};
// Should throw error as key is too small because openssl v3 doesn't allow it
assert.throws(() => tls.createServer(options, common.mustNotCall()),
/key too small/i);
// Reducing SECLEVEL to 0 in ciphers retains compatibility with previous versions of OpenSSL like using a small key.
// As ciphers are getting set before the cert and key get loaded.
options.ciphers = 'DEFAULT:@SECLEVEL=0';
assert.ok(tls.createServer(options, common.mustNotCall()));
}