FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

tls: fix order of setting cipher before setting cert and key · nodejs/node@d9ea6c1 · GitHub

/ node Public

Commit d9ea6c1

Browse files
kumarrishav
authored and
richardlau
committed
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: #36655 Fixes: #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: #50186 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it>
1 parent 194ff6a commit d9ea6c1

4 files changed

Lines changed: 68 additions & 22 deletions

File tree

‎lib/internal/tls/secure-context.js‎

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,31 @@ function configSecureContext(context, options = kEmptyObject, name = 'options')
144144
ticketKeys,
145145
} = options;
146146

147+
// Set the cipher list and cipher suite before anything else because
148+
// @SECLEVEL=<n> changes the security level and that affects subsequent
149+
// operations.
150+
if (ciphers !== undefined && ciphers !== null)
151+
validateString(ciphers, `${name}.ciphers`);
152+
153+
// Work around an OpenSSL API quirk. cipherList is for TLSv1.2 and below,
154+
// cipherSuites is for TLSv1.3 (and presumably any later versions). TLSv1.3
155+
// cipher suites all have a standard name format beginning with TLS_, so split
156+
// the ciphers and pass them to the appropriate API.
157+
const {
158+
cipherList,
159+
cipherSuites,
160+
} = processCiphers(ciphers, `${name}.ciphers`);
161+
162+
if (cipherSuites !== '')
163+
context.setCipherSuites(cipherSuites);
164+
context.setCiphers(cipherList);
165+
166+
if (cipherList === '' &&
167+
context.getMinProto() < TLS1_3_VERSION &&
168+
context.getMaxProto() > TLS1_2_VERSION) {
169+
context.setMinProto(TLS1_3_VERSION);
170+
}
171+
147172
// Add CA before the cert to be able to load cert's issuer in C++ code.
148173
// NOTE(@jasnell): ca, cert, and key are permitted to be falsy, so do not
149174
// change the checks to !== undefined checks.
@@ -214,28 +239,6 @@ function configSecureContext(context, options = kEmptyObject, name = 'options')
214239
}
215240
}
216241

217-
if (ciphers !== undefined && ciphers !== null)
218-
validateString(ciphers, `${name}.ciphers`);
219-
220-
// Work around an OpenSSL API quirk. cipherList is for TLSv1.2 and below,
221-
// cipherSuites is for TLSv1.3 (and presumably any later versions). TLSv1.3
222-
// cipher suites all have a standard name format beginning with TLS_, so split
223-
// the ciphers and pass them to the appropriate API.
224-
const {
225-
cipherList,
226-
cipherSuites,
227-
} = processCiphers(ciphers, `${name}.ciphers`);
228-
229-
if (cipherSuites !== '')
230-
context.setCipherSuites(cipherSuites);
231-
context.setCiphers(cipherList);
232-
233-
if (cipherList === '' &&
234-
context.getMinProto() < TLS1_3_VERSION &&
235-
context.getMaxProto() > TLS1_2_VERSION) {
236-
context.setMinProto(TLS1_3_VERSION);
237-
}
238-
239242
validateString(ecdhCurve, `${name}.ecdhCurve`);
240243
context.setECDHCurve(ecdhCurve);
241244

‎test/fixtures/keys/agent11-cert.pem‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIIBFjCBwaADAgECAgEBMA0GCSqGSIb3DQEBBQUAMBQxEjAQBgNVBAMTCWxvY2Fs
3+
aG9zdDAeFw0yMzEwMTUxNzQ5MTBaFw0yNDEwMTUxNzQ5MTBaMBQxEjAQBgNVBAMT
4+
CWxvY2FsaG9zdDBcMA0GCSqGSIb3DQEBAQUAA0sAMEgCQQDW9vH7W98zSi1IfoTG
5+
pTjbvXRzmmSG6y5z1S3gvC6+keC5QQkEdIG5vWas1efX5qEPybptRyM34T6aWv+U
6+
uzUJAgMBAAEwDQYJKoZIhvcNAQEFBQADQQAEIwD5mLIALrim6uD39DO/umYDtDIb
7+
TAQmgWdkQrCdCtX0Yp49gJyaq2HtFgsk/cxMoYMYkDtT5a7nwEQu+Xqt
8+
-----END CERTIFICATE-----

‎test/fixtures/keys/agent11-key.pem‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
-----BEGIN RSA PRIVATE KEY-----
2+
MIIBOwIBAAJBANb28ftb3zNKLUh+hMalONu9dHOaZIbrLnPVLeC8Lr6R4LlBCQR0
3+
gbm9ZqzV59fmoQ/Jum1HIzfhPppa/5S7NQkCAwEAAQJAaetb6GKoY/lUvre4bLjU
4+
f1Gmo5+bkO8pAGI2LNoMnlETjLjlnvShkqu0kxY96G5Il6VSX4Yjz0D40f4IrlJW
5+
AQIhAPChOjGBlOFcGA/pPmzMcW8jRCLvVubiO9TpiYVhWz45AiEA5LIKsSR8HT9y
6+
eyVNNNkRbNvTrddbvXMBBjj+KwxQrVECIQDjalzHQQJl4lXTY8rdpHJoaNoSckSd
7+
PJ7zYCvaZOKI8QIhALoGbRYMxHySCJBNFlE/pKH06mnE/RXMf2/NWkov+UwRAiAz
8+
ucgBN8xY5KvG3eI78WHdE2B5X0B4EabFXmUlzIrhTA==
9+
-----END RSA PRIVATE KEY-----

‎test/parallel/test-tls-reduced-SECLEVEL-in-cipher.js‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
'use strict';
2+
const common = require('../common');
3+
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
7+
const assert = require('assert');
8+
const tls = require('tls');
9+
const fixtures = require('../common/fixtures');
10+
11+
{
12+
const options = {
13+
key: fixtures.readKey('agent11-key.pem'),
14+
cert: fixtures.readKey('agent11-cert.pem'),
15+
ciphers: 'DEFAULT'
16+
};
17+
18+
// Should throw error as key is too small because openssl v3 doesn't allow it
19+
assert.throws(() => tls.createServer(options, common.mustNotCall()),
20+
/key too small/i);
21+
22+
// Reducing SECLEVEL to 0 in ciphers retains compatibility with previous versions of OpenSSL like using a small key.
23+
// As ciphers are getting set before the cert and key get loaded.
24+
options.ciphers = 'DEFAULT:@SECLEVEL=0';
25+
assert.ok(tls.createServer(options, common.mustNotCall()));
26+
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL