From a0b41707646f2a946175c3d697fa8ad7d21c46c3 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Thu, 22 Oct 2026 11:20:43 +0200 Subject: [PATCH] tls: configure TLS 1.3 ciphers with BoringSSL Translate cipher-suite names into protocol IDs for BoringSSL's TLS 1.3 configuration API. Handle duplicates and unknown names, reject nonempty lists without a supported suite, and implement honorCipherOrder through preference flags. Cover suite restrictions, explicit preference ordering, legacy-only lists, runtime defaults, and command-line overrides. Signed-off-by: Filip Skokan Assisted-by: Codex PR-URL: https://github.com/nodejs/node/pull/66922 Reviewed-By: Antoine du Hamel Reviewed-By: James M Snell --- deps/ncrypto/ncrypto.cc | 52 ++++++- lib/internal/tls/wrap.js | 4 +- test/parallel/test-tls-getcipher.js | 5 +- test/parallel/test-tls-set-ciphers-error.js | 10 +- test/parallel/test-tls-set-ciphers.js | 13 +- test/parallel/test-tls13-ciphers.js | 151 ++++++++++++++++++++ 6 files changed, 215 insertions(+), 20 deletions(-) create mode 100644 test/parallel/test-tls13-ciphers.js diff --git a/deps/ncrypto/ncrypto.cc b/deps/ncrypto/ncrypto.cc index 025f47cd920..284ba456a49 100644 --- a/deps/ncrypto/ncrypto.cc +++ b/deps/ncrypto/ncrypto.cc @@ -5110,13 +5110,57 @@ bool SSLCtxPointer::setGroups(const char* groups) { } bool SSLCtxPointer::setCipherSuites(const char* ciphers) { -#ifndef OPENSSL_IS_BORINGSSL if (!ctx_) return false; +#ifndef OPENSSL_IS_BORINGSSL return SSL_CTX_set_ciphersuites(ctx_.get(), ciphers); #else - // BoringSSL does not allow API config of TLS 1.3 cipher suites. - // We treat this as a non-op. - return true; + constexpr std::array supported = { + SSL_CIPHER_AES_128_GCM_SHA256, + SSL_CIPHER_AES_256_GCM_SHA384, + SSL_CIPHER_CHACHA20_POLY1305_SHA256, + }; + std::vector cipher_ids; + std::string_view remaining(ciphers); + while (!remaining.empty()) { + const size_t end = remaining.find(':'); + std::string_view name = remaining.substr(0, end); + const size_t first = name.find_first_not_of(" \t\n\r\f\v"); + if (first != std::string_view::npos) { + name = name.substr(first); + name = name.substr(0, name.find_last_not_of(" \t\n\r\f\v") + 1); + for (const auto id : supported) { + const SSL_CIPHER* cipher = SSL_get_cipher_by_value(id); + if (name == SSL_CIPHER_standard_name(cipher)) { + // OpenSSL ignores unknown names and accepts duplicates, while + // BoringSSL requires unique protocol IDs. + if (std::find(cipher_ids.begin(), cipher_ids.end(), id) == + cipher_ids.end()) { + cipher_ids.push_back(id); + } + break; + } + } + } + if (end == std::string_view::npos) break; + remaining.remove_prefix(end + 1); + } + if (*ciphers != '\0' && cipher_ids.empty()) { + OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CIPHER_MATCH); + return false; + } + std::vector flags(cipher_ids.size()); + if (!flags.empty() && + (SSL_CTX_get_options(get()) & SSL_OP_CIPHER_SERVER_PREFERENCE) == 0) { + // TLS 1.3 otherwise always uses server preference in BoringSSL. An + // equal-preference group lets the client choose from the configured list. + std::fill(flags.begin(), + flags.end() - 1, + SSL_CIPHER_FLAG_EQUAL_PREFERENCE_WITH_NEXT); + } + // An empty input resets BoringSSL's built-in preference order. The public + // TLS API expands empty cipher strings to Node's defaults before this call. + return SSL_CTX_set1_tls13_ciphers( + get(), cipher_ids.data(), flags.data(), cipher_ids.size()) == 1; #endif } diff --git a/lib/internal/tls/wrap.js b/lib/internal/tls/wrap.js index 65611de32d8..d339c4d774c 100644 --- a/lib/internal/tls/wrap.js +++ b/lib/internal/tls/wrap.js @@ -39,6 +39,8 @@ const { assertCrypto(); +const { openssl_is_boringssl: isBoringSSL } = process.features; + const { setImmediate } = require('timers'); const assert = require('internal/assert'); const crypto = require('crypto'); @@ -1126,7 +1128,7 @@ TLSSocket.prototype.renegotiate = function(options, callback) { this._handle.renegotiate(); } catch (err) { const isBoringSSLRenegotiationUnsupported = - process.features.openssl_is_boringssl && + isBoringSSL && err?.code === 'ERR_SSL_FUNCTION_SHOULD_NOT_HAVE_BEEN_CALLED'; const error = isBoringSSLRenegotiationUnsupported ? new ERR_TLS_RENEGOTIATION_UNSUPPORTED() : err; diff --git a/test/parallel/test-tls-getcipher.js b/test/parallel/test-tls-getcipher.js index cb8f1ba826b..560a1b2a6e2 100644 --- a/test/parallel/test-tls-getcipher.js +++ b/test/parallel/test-tls-getcipher.js @@ -109,10 +109,7 @@ tls.createServer({ rejectUnauthorized: false }, common.mustCall(() => { const cipher = client.getCipher(); - const expectedCipher = isBoringSSL ? - 'TLS_AES_128_GCM_SHA256' : - 'TLS_AES_256_GCM_SHA384'; - assert.strictEqual(cipher.name, expectedCipher); + assert.strictEqual(cipher.name, 'TLS_AES_256_GCM_SHA384'); assert.strictEqual(cipher.standardName, cipher.name); assert.strictEqual(cipher.version, isBoringSSL ? 'TLSv1/SSLv3' : diff --git a/test/parallel/test-tls-set-ciphers-error.js b/test/parallel/test-tls-set-ciphers-error.js index 102186f9e39..3cfc8c391bf 100644 --- a/test/parallel/test-tls-set-ciphers-error.js +++ b/test/parallel/test-tls-set-ciphers-error.js @@ -7,7 +7,7 @@ if (!common.hasCrypto) const assert = require('assert'); const tls = require('tls'); const fixtures = require('../common/fixtures'); -const { hasOpenSSL, isBoringSSL } = require('../common/crypto'); +const { hasOpenSSL } = require('../common/crypto'); { const options = { @@ -21,12 +21,8 @@ const { hasOpenSSL, isBoringSSL } = require('../common/crypto'); assert.throws(() => tls.createServer(options, common.mustNotCall()), /no[_ ]cipher[_ ]match/i); options.ciphers = 'TLS_not_a_cipher'; - if (isBoringSSL) { - tls.createServer(options).close(); - } else { - assert.throws(() => tls.createServer(options, common.mustNotCall()), - /no[_ ]cipher[_ ]match/i); - } + assert.throws(() => tls.createServer(options, common.mustNotCall()), + /no[_ ]cipher[_ ]match/i); } // Cipher name matching is case-sensitive prior to OpenSSL 4.0, and diff --git a/test/parallel/test-tls-set-ciphers.js b/test/parallel/test-tls-set-ciphers.js index 57fb3599112..0562c808b8d 100644 --- a/test/parallel/test-tls-set-ciphers.js +++ b/test/parallel/test-tls-set-ciphers.js @@ -7,9 +7,10 @@ if (!common.hasCrypto) { const { hasOpenSSL, hasFIPS, + isBoringSSL, } = require('../common/crypto'); -if (!hasOpenSSL(3)) { +if (!hasOpenSSL(3) && !isBoringSSL) { common.skip('missing crypto, or OpenSSL version lower than 3'); } @@ -133,10 +134,11 @@ if (hasFIPS(3)) { test('TLS_AES_256_GCM_SHA384', 'TLS_CHACHA20_POLY1305_SHA256', U, expectedTLSAlertError, 'ERR_SSL_NO_SHARED_CIPHER'); - test('AES256-SHA', 'AES256-SHA256', U, expectedTLSAlertError, + // Use TLS 1.2 suites available on both crypto backends. + test('AES256-SHA', 'ECDHE-RSA-AES256-GCM-SHA384', U, expectedTLSAlertError, 'ERR_SSL_NO_SHARED_CIPHER'); test('AES256-SHA:TLS_AES_256_GCM_SHA384', - 'TLS_CHACHA20_POLY1305_SHA256:AES256-SHA256', + 'TLS_CHACHA20_POLY1305_SHA256:ECDHE-RSA-AES256-GCM-SHA384', U, expectedTLSAlertError, 'ERR_SSL_NO_SHARED_CIPHER'); // Cipher order ignored, TLS1.3 chosen before TLS1.2. @@ -154,7 +156,10 @@ if (hasFIPS(3)) { // default, but work. // However, for OpenSSL32 AES_128 is not enabled due to the // default security level - if (!hasOpenSSL(3, 2)) { + if (isBoringSSL) { + // BoringSSL does not implement the TLS 1.3 CCM cipher suites. + test('TLS_AES_128_CCM_8_SHA256', U, U, 'ERR_SSL_NO_CIPHER_MATCH', U); + } else if (!hasOpenSSL(3, 2)) { test('TLS_AES_128_CCM_8_SHA256', U, U, 'ERR_SSL_SSLV3_ALERT_HANDSHAKE_FAILURE', 'ERR_SSL_NO_SHARED_CIPHER'); diff --git a/test/parallel/test-tls13-ciphers.js b/test/parallel/test-tls13-ciphers.js new file mode 100644 index 00000000000..7cf87557e8d --- /dev/null +++ b/test/parallel/test-tls13-ciphers.js @@ -0,0 +1,151 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const { constants } = require('crypto'); +const tls = require('tls'); +const fixtures = require('../common/fixtures'); +const { hasOpenSSL, isBoringSSL } = require('../common/crypto'); +const { spawnSyncAndAssert } = require('../common/child_process'); +const { connect, keys } = require(fixtures.path('tls-connect')); + +const aes128 = 'TLS_AES_128_GCM_SHA256'; +const aes256 = 'TLS_AES_256_GCM_SHA384'; +const tls13Ciphers = [aes128, aes256, 'TLS_CHACHA20_POLY1305_SHA256']; +const legacy = 'ECDHE-RSA-AES256-GCM-SHA384'; + +function test(clientCiphers, serverCiphers, expectedCipher, options = {}) { + connect({ + client: { + rejectUnauthorized: false, + minVersion: 'TLSv1.2', + maxVersion: 'TLSv1.3', + ciphers: clientCiphers, + ...options.client, + }, + server: { + key: keys.agent2.key, + cert: keys.agent2.cert, + minVersion: 'TLSv1.2', + maxVersion: 'TLSv1.3', + ciphers: serverCiphers, + ...options.server, + }, + }, common.mustCall((err, pair, cleanup) => { + if (expectedCipher === undefined) { + assert(err); + assert.strictEqual(pair.server.err.code, 'ERR_SSL_NO_SHARED_CIPHER'); + assert(pair.client.err); + } else { + assert.ifError(err); + const expected = Array.isArray(expectedCipher) ? expectedCipher : [expectedCipher]; + const cipher = pair.client.conn.getCipher().name; + assert(expected.includes(cipher)); + for (const { conn } of [pair.client, pair.server]) { + assert.strictEqual(conn.getCipher().name, cipher); + assert.strictEqual(conn.getProtocol(), + expectedCipher === legacy ? 'TLSv1.2' : 'TLSv1.3'); + } + } + cleanup(); + })); +} + +function testLegacyOnlyCipherLists() { + for (const honorCipherOrder of [true, false]) { + const options = { server: { honorCipherOrder } }; + // A legacy-only list retains the backend's TLS 1.3 defaults. Check that + // each peer can negotiate either AES suite without assuming their order. + test(aes128, legacy, aes128, options); + test(aes256, legacy, aes256, options); + test(legacy, aes128, aes128, options); + test(legacy, aes256, aes256, options); + test(legacy, legacy, tls13Ciphers, options); + } +} + +if (process.argv[2] !== undefined) { + const mode = process.argv[2]; + if (mode === 'runtime-defaults') { + tls.DEFAULT_CIPHERS = aes128; + } else { + assert.strictEqual(tls.DEFAULT_CIPHERS, + mode === 'cli-defaults' ? aes128 : legacy); + } + // Explicit TLS 1.3 defaults are still applied when ciphers is omitted, but + // must not replace the native defaults for an explicit legacy-only list. + test(undefined, undefined, mode === 'cli-legacy-defaults' ? tls13Ciphers : aes128); + testLegacyOnlyCipherLists(); + return; +} + +// Both peers must apply the configured TLS 1.3 cipher list. +test(aes256, undefined, aes256); +test(undefined, aes256, aes256); +test(aes128, aes256, undefined); + +// Duplicate names and trailing whitespace must not cause rejection. +const duplicates = `${aes256} \t:${aes256}`; +test(duplicates, duplicates, aes256); + +// OpenSSL 1.1.1 rejects unknown TLS 1.3 cipher names. OpenSSL 3 and BoringSSL +// ignore them and do not interpret exclusion rules in the TLS 1.3 list. +const list = `TLS_UNKNOWN:!${aes128}:${duplicates}`; +if (hasOpenSSL(3) || isBoringSSL) { + test(list, list, aes256); +} else { + assert.throws(() => tls.createSecureContext({ ciphers: list }), { + code: 'ERR_SSL_NO_CIPHER_MATCH', + }); +} +for (const ciphers of ['TLS_UNKNOWN', `!${aes128}`]) { + assert.throws(() => tls.createSecureContext({ ciphers }), { + code: 'ERR_SSL_NO_CIPHER_MATCH', + }); +} +if (isBoringSSL) { + assert.throws(() => tls.createSecureContext({ + ciphers: 'TLS_AES_128_CCM_SHA256', + }), { + code: 'ERR_SSL_NO_CIPHER_MATCH', + }); +} + +// Honor the server's cipher-order option in TLS 1.3 as well as TLS 1.2. +test(`${aes256}:${aes128}`, `${aes128}:${aes256}`, aes128, { + server: { honorCipherOrder: true }, +}); +test(`${aes256}:${aes128}`, `${aes128}:${aes256}`, aes256, { + server: { honorCipherOrder: false }, +}); +testLegacyOnlyCipherLists(); + +// TLS 1.2 configuration and explicit protocol restrictions remain effective. +const mixed = `${legacy}:${aes256}`; +test(mixed, mixed, aes256); +test(mixed, mixed, legacy, { + client: { maxVersion: 'TLSv1.2' }, + server: { maxVersion: 'TLSv1.2' }, +}); +test(mixed, mixed, legacy, { + server: { secureOptions: constants.SSL_OP_NO_TLSv1_3 }, +}); + +// Empty public cipher strings continue to select Node's defaults. +test('', aes256, aes256); +test(aes256, '', aes256); + +for (const [mode, ciphers] of [ + ['runtime-defaults', undefined], + ['cli-defaults', aes128], + ['cli-legacy-defaults', legacy], +]) { + spawnSyncAndAssert(process.execPath, [ + ...process.execArgv, + ...(ciphers === undefined ? [] : [`--tls-cipher-list=${ciphers}`]), + __filename, + mode, + ], {}); +}