Skip to content

Commit ed6f099

Browse files
cirospaciariautofix-ci[bot]
cirospaciari
and
autofix-ci[bot]
authored
fix(tls) fix ciphers (#21545)
### What does this PR do? Uses same ciphers than node.js for compatibility and do the same error checking on empty ciphers Fixes #9425 Fixes #21518 Fixes #19859 Fixes #18980 You can see more about redis ciphers here https://redis.io/docs/latest/operate/rs/security/encryption/tls/ciphers/ this should fix redis related ciphers issues ### How did you verify your code works? Tests --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent 258a2a2 commit ed6f099

File tree

13 files changed

+447
-53
lines changed

13 files changed

+447
-53
lines changed

‎packages/bun-usockets/src/crypto/default_ciphers.h

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// TLSv1.3 suites start with TLS_, and are the OpenSSL defaults, see:
2+
// https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_ciphersuites.html
3+
#ifndef DEFAULT_CIPHER_LIST
4+
#define DEFAULT_CIPHER_LIST \
5+
"ECDHE-RSA-AES128-GCM-SHA256:" \
6+
"ECDHE-ECDSA-AES128-GCM-SHA256:" \
7+
"ECDHE-RSA-AES256-GCM-SHA384:" \
8+
"ECDHE-ECDSA-AES256-GCM-SHA384:" \
9+
"ECDHE-RSA-AES128-SHA256:" \
10+
"ECDHE-RSA-AES256-SHA384:" \
11+
"HIGH:" \
12+
"!aNULL:" \
13+
"!eNULL:" \
14+
"!EXPORT:" \
15+
"!DES:" \
16+
"!RC4:" \
17+
"!MD5:" \
18+
"!PSK:" \
19+
"!SRP:" \
20+
"!CAMELLIA"
21+
#endif
22+
23+
// BoringSSL does not support legacy DHE ciphers and dont support SSL_CTX_set_cipher_list (see https://github.com/envoyproxy/envoy/issues/8848#issuecomment-548672667)
24+
// Node.js full list bellow
25+
26+
// In node.js they filter TLS_* ciphers and use SSL_CTX_set_cipher_list (TODO: Electron has a patch https://github.com/nodejs/node/issues/25890)
27+
// if passed to SSL_CTX_set_cipher_list it will be filtered out and not used in BoringSSL
28+
// "TLS_AES_256_GCM_SHA384:" \
29+
// "TLS_CHACHA20_POLY1305_SHA256:" \
30+
// "TLS_AES_128_GCM_SHA256:" \
31+
32+
// Supported by BoringSSL:
33+
// "ECDHE-RSA-AES128-GCM-SHA256:" \
34+
// "ECDHE-ECDSA-AES128-GCM-SHA256:" \
35+
// "ECDHE-RSA-AES256-GCM-SHA384:" \
36+
// "ECDHE-ECDSA-AES256-GCM-SHA384:" \
37+
// "ECDHE-RSA-AES128-SHA256:" \
38+
// "ECDHE-RSA-AES256-SHA384:" \
39+
40+
// Not supported by BoringSSL:
41+
// "ECDHE-RSA-AES256-SHA256:" \
42+
// "DHE-RSA-AES128-GCM-SHA256:" \
43+
// "DHE-RSA-AES128-SHA256:" \
44+
// "DHE-RSA-AES256-SHA384:" \
45+
// "DHE-RSA-AES256-SHA256:" \
46+
47+
48+
// Also present in Node.js and supported by BoringSSL:
49+
// "HIGH:" \
50+
// "!aNULL:" \
51+
// "!eNULL:" \
52+
// "!EXPORT:" \
53+
// "!DES:" \
54+
// "!RC4:" \
55+
// "!MD5:" \
56+
// "!PSK:" \
57+
// "!SRP:" \
58+
// "!CAMELLIA"

‎packages/bun-usockets/src/crypto/openssl.c

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ void *sni_find(void *sni, const char *hostname);
4545
#endif
4646

4747
#include "./root_certs_header.h"
48-
48+
#include "./default_ciphers.h"
4949
struct loop_ssl_data {
5050
char *ssl_read_input, *ssl_read_output;
5151
unsigned int ssl_read_input_length;
@@ -848,21 +848,24 @@ create_ssl_context_from_options(struct us_socket_context_options_t options) {
848848
return NULL;
849849
}
850850

851-
/* OWASP Cipher String 'A+'
852-
* (https://www.owasp.org/index.php/TLS_Cipher_String_Cheat_Sheet) */
853-
if (SSL_CTX_set_cipher_list(
854-
ssl_context,
855-
"DHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-"
856-
"AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256") != 1) {
851+
if (!SSL_CTX_set_cipher_list(ssl_context, DEFAULT_CIPHER_LIST)) {
857852
free_ssl_context(ssl_context);
858853
return NULL;
859854
}
860855
}
861856

862857
if (options.ssl_ciphers) {
863-
if (SSL_CTX_set_cipher_list(ssl_context, options.ssl_ciphers) != 1) {
864-
free_ssl_context(ssl_context);
865-
return NULL;
858+
if (!SSL_CTX_set_cipher_list(ssl_context, options.ssl_ciphers)) {
859+
unsigned long ssl_err = ERR_get_error();
860+
if (!(strlen(options.ssl_ciphers) == 0 && ERR_GET_REASON(ssl_err) == SSL_R_NO_CIPHER_MATCH)) {
861+
// TLS1.2 ciphers were deliberately cleared, so don't consider
862+
// SSL_R_NO_CIPHER_MATCH to be an error (this is how _set_cipher_suites()
863+
// works). If the user actually sets a value (like "no-such-cipher"), then
864+
// that's actually an error.
865+
free_ssl_context(ssl_context);
866+
return NULL;
867+
}
868+
ERR_clear_error();
866869
}
867870
}
868871

@@ -1288,21 +1291,27 @@ SSL_CTX *create_ssl_context_from_bun_options(
12881291
return NULL;
12891292
}
12901293

1291-
/* OWASP Cipher String 'A+'
1292-
* (https://www.owasp.org/index.php/TLS_Cipher_String_Cheat_Sheet) */
1293-
if (SSL_CTX_set_cipher_list(
1294-
ssl_context,
1295-
"DHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-"
1296-
"AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256") != 1) {
1294+
if (!SSL_CTX_set_cipher_list(ssl_context, DEFAULT_CIPHER_LIST)) {
12971295
free_ssl_context(ssl_context);
12981296
return NULL;
12991297
}
13001298
}
13011299

13021300
if (options.ssl_ciphers) {
1303-
if (SSL_CTX_set_cipher_list(ssl_context, options.ssl_ciphers) != 1) {
1304-
free_ssl_context(ssl_context);
1305-
return NULL;
1301+
if (!SSL_CTX_set_cipher_list(ssl_context, options.ssl_ciphers)) {
1302+
unsigned long ssl_err = ERR_get_error();
1303+
if (!(strlen(options.ssl_ciphers) == 0 && ERR_GET_REASON(ssl_err) == SSL_R_NO_CIPHER_MATCH)) {
1304+
char error_msg[256];
1305+
ERR_error_string_n(ERR_peek_last_error(), error_msg, sizeof(error_msg));
1306+
// TLS1.2 ciphers were deliberately cleared, so don't consider
1307+
// SSL_R_NO_CIPHER_MATCH to be an error (this is how _set_cipher_suites()
1308+
// works). If the user actually sets a value (like "no-such-cipher"), then
1309+
// that's actually an error.
1310+
*err = CREATE_BUN_SOCKET_ERROR_INVALID_CIPHERS;
1311+
free_ssl_context(ssl_context);
1312+
return NULL;
1313+
}
1314+
ERR_clear_error();
13061315
}
13071316
}
13081317

‎packages/bun-usockets/src/crypto/root_certs.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "./internal/internal.h"
66
#include <atomic>
77
#include <string.h>
8+
#include "./default_ciphers.h"
89
static const int root_certs_size = sizeof(root_certs) / sizeof(root_certs[0]);
910

1011
extern "C" void BUN__warn__extra_ca_load_failed(const char* filename, const char* error_msg);
@@ -184,3 +185,6 @@ extern "C" X509_STORE *us_get_default_ca_store() {
184185

185186
return store;
186187
}
188+
extern "C" const char *us_get_default_ciphers() {
189+
return DEFAULT_CIPHER_LIST;
190+
}

‎packages/bun-usockets/src/libusockets.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,7 @@ enum create_bun_socket_error_t {
262262
CREATE_BUN_SOCKET_ERROR_LOAD_CA_FILE,
263263
CREATE_BUN_SOCKET_ERROR_INVALID_CA_FILE,
264264
CREATE_BUN_SOCKET_ERROR_INVALID_CA,
265+
CREATE_BUN_SOCKET_ERROR_INVALID_CIPHERS,
265266
};
266267

267268
struct us_socket_context_t *us_create_bun_ssl_socket_context(struct us_loop_t *loop,

‎src/bun.js/api/BunObject.zig

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,6 +1306,15 @@ pub fn getS3DefaultClient(globalThis: *jsc.JSGlobalObject, _: *jsc.JSObject) jsc
13061306
return globalThis.bunVM().rareData().s3DefaultClient(globalThis);
13071307
}
13081308

1309+
pub fn getTLSDefaultCiphers(globalThis: *jsc.JSGlobalObject, _: *jsc.JSObject) jsc.JSValue {
1310+
return globalThis.bunVM().rareData().tlsDefaultCiphers();
1311+
}
1312+
1313+
pub fn setTLSDefaultCiphers(globalThis: *jsc.JSGlobalObject, _: *jsc.JSObject, ciphers: jsc.JSValue) jsc.JSValue {
1314+
globalThis.bunVM().rareData().setTLSDefaultCiphers(ciphers);
1315+
return .js_undefined;
1316+
}
1317+
13091318
pub fn getValkeyDefaultClient(globalThis: *jsc.JSGlobalObject, _: *jsc.JSObject) jsc.JSValue {
13101319
const valkey = jsc.API.Valkey.create(globalThis, &.{.js_undefined}) catch |err| {
13111320
if (err != error.JSError) {

‎src/bun.js/api/server/SSLConfig.zig

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
const SSLConfig = @This();
22

3-
requires_custom_request_ctx: bool = false,
43
server_name: [*c]const u8 = null,
54

65
key_file_name: [*c]const u8 = null,
@@ -10,7 +9,6 @@ ca_file_name: [*c]const u8 = null,
109
dh_params_file_name: [*c]const u8 = null,
1110

1211
passphrase: [*c]const u8 = null,
13-
low_memory_mode: bool = false,
1412

1513
key: ?[][*c]const u8 = null,
1614
key_count: u32 = 0,
@@ -29,6 +27,9 @@ protos: ?[*:0]const u8 = null,
2927
protos_len: usize = 0,
3028
client_renegotiation_limit: u32 = 0,
3129
client_renegotiation_window: u32 = 0,
30+
requires_custom_request_ctx: bool = false,
31+
is_using_default_ciphers: bool = true,
32+
low_memory_mode: bool = false,
3233

3334
const BlobFileContentResult = struct {
3435
data: [:0]const u8,
@@ -168,10 +169,18 @@ pub fn deinit(this: *SSLConfig) void {
168169
"ca_file_name",
169170
"dh_params_file_name",
170171
"passphrase",
171-
"ssl_ciphers",
172172
"protos",
173173
};
174174

175+
if (!this.is_using_default_ciphers) {
176+
if (this.ssl_ciphers) |slice_ptr| {
177+
const slice = std.mem.span(slice_ptr);
178+
if (slice.len > 0) {
179+
bun.freeSensitive(bun.default_allocator, slice);
180+
}
181+
}
182+
}
183+
175184
inline for (fields) |field| {
176185
if (@field(this, field)) |slice_ptr| {
177186
const slice = std.mem.span(slice_ptr);
@@ -452,10 +461,14 @@ pub fn fromJS(vm: *jsc.VirtualMachine, global: *jsc.JSGlobalObject, obj: jsc.JSV
452461
defer sliced.deinit();
453462
if (sliced.len > 0) {
454463
result.ssl_ciphers = try bun.default_allocator.dupeZ(u8, sliced.slice());
464+
result.is_using_default_ciphers = false;
455465
any = true;
456466
result.requires_custom_request_ctx = true;
457467
}
458468
}
469+
if (result.is_using_default_ciphers) {
470+
result.ssl_ciphers = global.bunVM().rareData().tlsDefaultCiphers() orelse null;
471+
}
459472

460473
if (try obj.getTruthy(global, "serverName") orelse try obj.getTruthy(global, "servername")) |server_name| {
461474
var sliced = try server_name.toSlice(global, bun.default_allocator);

‎src/bun.js/bindings/NodeTLS.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,4 +72,17 @@ JSC_DEFINE_HOST_FUNCTION(getExtraCACertificates, (JSC::JSGlobalObject * globalOb
7272
RELEASE_AND_RETURN(scope, JSValue::encode(JSC::objectConstructorFreeze(globalObject, rootCertificates)));
7373
}
7474

75+
extern "C" JSC::EncodedJSValue Bun__getTLSDefaultCiphers(JSC::JSGlobalObject* globalObject, JSC::CallFrame* callFrame);
76+
extern "C" JSC::EncodedJSValue Bun__setTLSDefaultCiphers(JSC::JSGlobalObject* globalObject, JSC::CallFrame* callFrame);
77+
78+
JSC_DEFINE_HOST_FUNCTION(getDefaultCiphers, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame))
79+
{
80+
return Bun__getTLSDefaultCiphers(globalObject, callFrame);
81+
}
82+
83+
JSC_DEFINE_HOST_FUNCTION(setDefaultCiphers, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame))
84+
{
85+
return Bun__setTLSDefaultCiphers(globalObject, callFrame);
86+
}
87+
7588
} // namespace Bun

‎src/bun.js/bindings/NodeTLS.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,7 @@ namespace Bun {
66
BUN_DECLARE_HOST_FUNCTION(Bun__canonicalizeIP);
77
JSC_DECLARE_HOST_FUNCTION(getBundledRootCertificates);
88
JSC_DECLARE_HOST_FUNCTION(getExtraCACertificates);
9+
JSC_DECLARE_HOST_FUNCTION(getDefaultCiphers);
10+
JSC_DECLARE_HOST_FUNCTION(setDefaultCiphers);
911

1012
}

‎src/bun.js/rare_data.zig

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ default_csrf_secret: []const u8 = "",
3939

4040
valkey_context: ValkeyContext = .{},
4141

42+
tls_default_ciphers: ?[:0]const u8 = null,
43+
4244
const PipeReadBuffer = [256 * 1024]u8;
4345
const DIGESTED_HMAC_256_LEN = 32;
4446
pub const AWSSignatureCache = struct {
@@ -421,6 +423,31 @@ pub export fn Bun__Process__getStdinFdType(vm: *jsc.VirtualMachine, fd: i32) Std
421423
}
422424
}
423425

426+
fn setTLSDefaultCiphersFromJS(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.JSError!jsc.JSValue {
427+
const vm = globalThis.bunVM();
428+
const args = callframe.arguments();
429+
const ciphers = if (args.len > 0) args[0] else .js_undefined;
430+
if (!ciphers.isString()) return globalThis.throwInvalidArgumentTypeValue("ciphers", "string", ciphers);
431+
var sliced = try ciphers.toSlice(globalThis, bun.default_allocator);
432+
defer sliced.deinit();
433+
vm.rareData().setTLSDefaultCiphers(sliced.slice());
434+
return .js_undefined;
435+
}
436+
437+
fn getTLSDefaultCiphersFromJS(globalThis: *jsc.JSGlobalObject, _: *jsc.CallFrame) bun.JSError!jsc.JSValue {
438+
const vm = globalThis.bunVM();
439+
const ciphers = vm.rareData().tlsDefaultCiphers() orelse return try bun.String.createUTF8ForJS(globalThis, bun.uws.get_default_ciphers());
440+
441+
return try bun.String.createUTF8ForJS(globalThis, ciphers);
442+
}
443+
444+
comptime {
445+
const js_setTLSDefaultCiphers = jsc.toJSHostFn(setTLSDefaultCiphersFromJS);
446+
@export(&js_setTLSDefaultCiphers, .{ .name = "Bun__setTLSDefaultCiphers" });
447+
const js_getTLSDefaultCiphers = jsc.toJSHostFn(getTLSDefaultCiphersFromJS);
448+
@export(&js_getTLSDefaultCiphers, .{ .name = "Bun__getTLSDefaultCiphers" });
449+
}
450+
424451
pub fn spawnIPCContext(rare: *RareData, vm: *jsc.VirtualMachine) *uws.SocketContext {
425452
if (rare.spawn_ipc_usockets_context) |ctx| {
426453
return ctx;
@@ -466,6 +493,17 @@ pub fn s3DefaultClient(rare: *RareData, globalThis: *jsc.JSGlobalObject) jsc.JSV
466493
};
467494
}
468495

496+
pub fn tlsDefaultCiphers(this: *RareData) ?[:0]const u8 {
497+
return this.tls_default_ciphers orelse null;
498+
}
499+
500+
pub fn setTLSDefaultCiphers(this: *RareData, ciphers: []const u8) void {
501+
if (this.tls_default_ciphers) |old_ciphers| {
502+
bun.default_allocator.free(old_ciphers);
503+
}
504+
this.tls_default_ciphers = bun.default_allocator.dupeZ(u8, ciphers) catch bun.outOfMemory();
505+
}
506+
469507
pub fn defaultCSRFSecret(this: *RareData) []const u8 {
470508
if (this.default_csrf_secret.len == 0) {
471509
const secret = bun.default_allocator.alloc(u8, 16) catch bun.outOfMemory();
@@ -498,6 +536,11 @@ pub fn deinit(this: *RareData) void {
498536
deflate.deinit();
499537
}
500538

539+
if (this.tls_default_ciphers) |ciphers| {
540+
this.tls_default_ciphers = null;
541+
bun.default_allocator.free(ciphers);
542+
}
543+
501544
this.valkey_context.deinit();
502545
}
503546

‎src/deps/uws.zig

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ pub const create_bun_socket_error_t = enum(c_int) {
6969
load_ca_file,
7070
invalid_ca_file,
7171
invalid_ca,
72+
invalid_ciphers,
7273

7374
pub fn toJS(this: create_bun_socket_error_t, globalObject: *jsc.JSGlobalObject) jsc.JSValue {
7475
return switch (this) {
@@ -79,6 +80,7 @@ pub const create_bun_socket_error_t = enum(c_int) {
7980
.load_ca_file => globalObject.ERR(.BORINGSSL, "Failed to load CA file", .{}).toJS(),
8081
.invalid_ca_file => globalObject.ERR(.BORINGSSL, "Invalid CA file", .{}).toJS(),
8182
.invalid_ca => globalObject.ERR(.BORINGSSL, "Invalid CA", .{}).toJS(),
83+
.invalid_ciphers => globalObject.ERR(.BORINGSSL, "Invalid ciphers", .{}).toJS(),
8284
};
8385
}
8486
};
@@ -144,6 +146,14 @@ pub const LIBUS_SOCKET_DESCRIPTOR = switch (bun.Environment.isWindows) {
144146
false => i32,
145147
};
146148

149+
const c = struct {
150+
pub extern fn us_get_default_ciphers() [*:0]const u8;
151+
};
152+
153+
pub fn get_default_ciphers() [:0]const u8 {
154+
return c.us_get_default_ciphers()[0..bun.len(c.us_get_default_ciphers()) :0];
155+
}
156+
147157
const bun = @import("bun");
148158
const Environment = bun.Environment;
149159
const jsc = bun.jsc;

0 commit comments

Comments
 (0)