Skip to content

Commit

Permalink
Fix crash in socket.upgradeTLS (oven-sh#13884)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Jarred-Sumner
Jarred-Sumner authored Sep 12, 2024
1 parent c319794 commit 2064876
Show file tree
Hide file tree
Showing 8 changed files with 284 additions and 81 deletions.
6 changes: 6 additions & 0 deletions packages/bun-usockets/src/crypto/openssl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1986,6 +1986,12 @@ struct us_internal_ssl_socket_t *us_internal_ssl_socket_wrap_with_tls(
struct us_socket_context_t *context = us_create_bun_socket_context(
1, old_context->loop, sizeof(struct us_wrapped_socket_context_t),
options);

// Handle SSL context creation failure
if (UNLIKELY(!context)) {
return NULL;
}

struct us_internal_ssl_socket_context_t *tls_context =
(struct us_internal_ssl_socket_context_t *)context;

Expand Down
2 changes: 1 addition & 1 deletion packages/bun-usockets/src/libusockets.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ void *us_socket_context_get_native_handle(int ssl, us_socket_context_r context);
struct us_socket_context_t *us_create_socket_context(int ssl, us_loop_r loop,
int ext_size, struct us_socket_context_options_t options) nonnull_fn_decl;
struct us_socket_context_t *us_create_bun_socket_context(int ssl, struct us_loop_t *loop,
int ext_size, struct us_bun_socket_context_options_t options) nonnull_fn_decl;
int ext_size, struct us_bun_socket_context_options_t options);

/* Delete resources allocated at creation time (will call unref now and only free when ref count == 0). */
void us_socket_context_free(int ssl, us_socket_context_r context) nonnull_fn_decl;
Expand Down
17 changes: 17 additions & 0 deletions src/boringssl.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,3 +206,20 @@ pub fn checkServerIdentity(
}
return false;
}

const JSC = bun.JSC;
pub fn ERR_toJS(globalThis: *JSC.JSGlobalObject, err_code: u32) JSC.JSValue {
var outbuf: [128 + 1 + "BoringSSL ".len]u8 = undefined;
@memset(&outbuf, 0);
outbuf[0.."BoringSSL ".len].* = "BoringSSL ".*;
const message_buf = outbuf["BoringSSL ".len..];

_ = boring.ERR_error_string_n(err_code, message_buf, message_buf.len);

const error_message: []const u8 = bun.sliceTo(outbuf[0..], 0);
if (error_message.len == "BoringSSL ".len) {
return globalThis.ERR_BORINGSSL("An unknown BoringSSL error occurred: {d}", .{err_code}).toJS();
}

return globalThis.ERR_BORINGSSL("{s}", .{error_message}).toJS();
}
14 changes: 1 addition & 13 deletions src/bun.js/api/BunObject.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1800,19 +1800,7 @@ pub const Crypto = struct {
};

pub fn createCryptoError(globalThis: *JSC.JSGlobalObject, err_code: u32) JSValue {
var outbuf: [128 + 1 + "BoringSSL error: ".len]u8 = undefined;
@memset(&outbuf, 0);
outbuf[0.."BoringSSL error: ".len].* = "BoringSSL error: ".*;
const message_buf = outbuf["BoringSSL error: ".len..];

_ = BoringSSL.ERR_error_string_n(err_code, message_buf, message_buf.len);

const error_message: []const u8 = bun.sliceTo(outbuf[0..], 0);
if (error_message.len == "BoringSSL error: ".len) {
return ZigString.static("Unknown BoringSSL error").toErrorInstance(globalThis);
}

return ZigString.fromUTF8(error_message).toErrorInstance(globalThis);
return BoringSSL.ERR_toJS(globalThis, err_code);
}
const unknown_password_algorithm_message = "unknown algorithm, expected one of: \"bcrypt\", \"argon2id\", \"argon2d\", \"argon2i\" (default is \"argon2id\")";

Expand Down
126 changes: 105 additions & 21 deletions src/bun.js/api/bun/socket.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2888,6 +2888,8 @@ fn NewSocket(comptime ssl: bool) type {
callframe: *JSC.CallFrame,
) JSValue {
JSC.markBinding(@src());
const this_js = callframe.this();

if (comptime ssl) {
return JSValue.jsUndefined();
}
Expand All @@ -2904,6 +2906,7 @@ fn NewSocket(comptime ssl: bool) type {
}

var exception: JSC.C.JSValueRef = null;
var success = false;

const opts = args.ptr[0];
if (opts.isEmptyOrUndefinedOrNull() or opts.isBoolean() or !opts.isObject()) {
Expand All @@ -2915,13 +2918,34 @@ fn NewSocket(comptime ssl: bool) type {
globalObject.throw("Expected \"socket\" option", .{});
return .zero;
};
if (globalObject.hasException()) {
return .zero;
}

const handlers = Handlers.fromJS(globalObject, socket_obj, &exception) orelse {
if (!globalObject.hasException() and exception != null) {
globalObject.throwValue(exception.?.value());
}

var handlers = Handlers.fromJS(globalObject, socket_obj, &exception) orelse {
globalObject.throwValue(exception.?.value());
return .zero;
};

if (!globalObject.hasException() and exception != null) {
globalObject.throwValue(exception.?.value());
}

if (globalObject.hasException()) {
return .zero;
}

var ssl_opts: ?JSC.API.ServerConfig.SSLConfig = null;
defer {
if (!success) {
if (ssl_opts) |*ssl_config| {
ssl_config.deinit();
}
}
}

if (opts.getTruthy(globalObject, "tls")) |tls| {
if (tls.isBoolean()) {
Expand All @@ -2931,12 +2955,20 @@ fn NewSocket(comptime ssl: bool) type {
} else {
if (JSC.API.ServerConfig.SSLConfig.inJS(JSC.VirtualMachine.get(), globalObject, tls, &exception)) |ssl_config| {
ssl_opts = ssl_config;
} else if (exception != null) {
return .zero;
}
}
}

if (exception != null) {
if (!globalObject.hasException()) {
globalObject.throwValue(exception.?.value());
}
}

if (globalObject.hasException()) {
return .zero;
}

if (ssl_opts == null) {
globalObject.throw("Expected \"tls\" option", .{});
return .zero;
Expand All @@ -2947,8 +2979,12 @@ fn NewSocket(comptime ssl: bool) type {
default_data = default_data_value;
default_data.ensureStillAlive();
}
if (globalObject.hasException()) {
return .zero;
}

var socket_config = ssl_opts.?;
ssl_opts = null;
defer socket_config.deinit();
const options = socket_config.asUSockets();

Expand All @@ -2959,7 +2995,7 @@ fn NewSocket(comptime ssl: bool) type {

const is_server = this.handlers.is_server;

var handlers_ptr = handlers.vm.allocator.create(Handlers) catch bun.outOfMemory();
var handlers_ptr = bun.default_allocator.create(Handlers) catch bun.outOfMemory();
handlers_ptr.* = handlers;
handlers_ptr.is_server = is_server;
handlers_ptr.protect();
Expand All @@ -2972,11 +3008,11 @@ fn NewSocket(comptime ssl: bool) type {
.protos = if (protos) |p| (bun.default_allocator.dupe(u8, p[0..protos_len]) catch bun.outOfMemory()) else null,
.server_name = if (socket_config.server_name) |server_name| (bun.default_allocator.dupe(u8, server_name[0..bun.len(server_name)]) catch bun.outOfMemory()) else null,
.socket_context = null, // only set after the wrapTLS
.flags = .{
.is_active = false,
},
});

const tls_js_value = tls.getThisValue(globalObject);
TLSSocket.dataSetCached(tls_js_value, globalObject, default_data);

const TCPHandler = NewWrappedHandler(false);

// reconfigure context to use the new wrapper handlers
Expand All @@ -2989,19 +3025,62 @@ fn NewSocket(comptime ssl: bool) type {
WrappedSocket,
TLSHandler,
) orelse {
const err = BoringSSL.ERR_get_error();
defer {
if (err != 0) {
BoringSSL.ERR_clear_error();
}
}
tls.wrapped = .none;

// Reset config to TCP
uws.NewSocketHandler(false).configure(
this.socket.context().?,
true,
*TCPSocket,
struct {
pub const onOpen = NewSocket(false).onOpen;
pub const onClose = NewSocket(false).onClose;
pub const onData = NewSocket(false).onData;
pub const onWritable = NewSocket(false).onWritable;
pub const onTimeout = NewSocket(false).onTimeout;
pub const onConnectError = NewSocket(false).onConnectError;
pub const onEnd = NewSocket(false).onEnd;
pub const onHandshake = NewSocket(false).onHandshake;
},
);

tls.deref();

handlers_ptr.unprotect();
handlers.vm.allocator.destroy(handlers_ptr);
bun.default_allocator.destroy(tls);
bun.default_allocator.destroy(handlers_ptr);

// If BoringSSL gave us an error code, let's use it.
if (err != 0 and !globalObject.hasException()) {
globalObject.throwValue(BoringSSL.ERR_toJS(globalObject, err));
}

// If BoringSSL did not give us an error code, let's throw a generic error.
if (!globalObject.hasException()) {
globalObject.throw("Failed to upgrade socket from TCP -> TLS. Is the TLS config correct?", .{});
}

return JSValue.jsUndefined();
};

// Do not create the JS Wrapper object until _after_ we've validated the TLS config.
// Otherwise, JSC will GC it and the lifetime gets very complicated.
const tls_js_value = tls.getThisValue(globalObject);
TLSSocket.dataSetCached(tls_js_value, globalObject, default_data);

tls.socket = new_socket;
tls.socket_context = new_socket.context(); // owns the new tls context that have a ref from the old one
tls.ref();
const vm = handlers.vm;

var raw_handlers_ptr = handlers.vm.allocator.create(Handlers) catch bun.outOfMemory();
var raw_handlers_ptr = bun.default_allocator.create(Handlers) catch bun.outOfMemory();
raw_handlers_ptr.* = .{
.vm = globalObject.bunVM(),
.vm = vm,
.globalObject = globalObject,
.onOpen = this.handlers.onOpen,
.onClose = this.handlers.onClose,
Expand Down Expand Up @@ -3030,10 +3109,11 @@ fn NewSocket(comptime ssl: bool) type {
raw.ref();

const raw_js_value = raw.getThisValue(globalObject);
if (JSSocketType(ssl).dataGetCached(this.getThisValue(globalObject))) |raw_default_data| {
if (JSSocketType(ssl).dataGetCached(this_js)) |raw_default_data| {
raw_default_data.ensureStillAlive();
TLSSocket.dataSetCached(raw_js_value, globalObject, raw_default_data);
}

// marks both as active
raw.markActive();
// this will keep tls alive until socket.open() is called to start TLS certificate and the handshake process
Expand All @@ -3048,25 +3128,29 @@ fn NewSocket(comptime ssl: bool) type {
ctx.* = .{ .tcp = raw, .tls = tls };
}

// start TLS handshake after we set ext
new_socket.startTLS(!this.handlers.is_server);

//detach and invalidate the old instance
this.socket.detach();
this.deref();
if (this.flags.is_active) {
const vm = this.handlers.vm;
this.poll_ref.disable();
this.flags.is_active = false;
// will free handlers when hits 0 active connections
// the connection can be upgraded inside a handler call so we need to garantee that it will be still alive
this.handlers.markInactive();
this.poll_ref.unref(vm);

this.has_pending_activity.store(false, .release);
}

const array = JSC.JSValue.createEmptyArray(globalObject, 2);
array.putIndex(globalObject, 0, raw_js_value);
array.putIndex(globalObject, 1, tls_js_value);

defer this.deref();

// detach and invalidate the old instance
this.socket.detach();

// start TLS handshake after we set extension on the socket
new_socket.startTLS(!is_server);

success = true;
return array;
}
};
Expand Down
1 change: 1 addition & 0 deletions src/bun.js/bindings/ErrorCode.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,5 @@ export default [
["ERR_FORMDATA_PARSE_ERROR", TypeError, "TypeError"],
["ERR_BODY_ALREADY_USED", Error, "Error"],
["ERR_STREAM_WRAP", Error, "Error"],
["ERR_BORINGSSL", Error, "Error"],
] as ErrorCodeMapping;
6 changes: 3 additions & 3 deletions src/deps/uws.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1938,11 +1938,11 @@ pub const us_bun_socket_context_options_t = extern struct {
ca_file_name: [*c]const u8 = null,
ssl_ciphers: [*c]const u8 = null,
ssl_prefer_low_memory_usage: i32 = 0,
key: [*c][*c]const u8 = null,
key: ?[*]?[*:0]const u8 = null,
key_count: u32 = 0,
cert: [*c][*c]const u8 = null,
cert: ?[*]?[*:0]const u8 = null,
cert_count: u32 = 0,
ca: [*c][*c]const u8 = null,
ca: ?[*]?[*:0]const u8 = null,
ca_count: u32 = 0,
secure_options: u32 = 0,
reject_unauthorized: i32 = 0,
Expand Down
Loading

0 comments on commit 2064876

Please sign in to comment.