From 67b4478137b140d5dc6693f5ae96a4577b59b12e Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sat, 19 Oct 2024 01:14:13 -0700 Subject: [PATCH] Fixes #14333 (#14679) --- packages/bun-usockets/src/context.c | 5 +- packages/bun-usockets/src/crypto/openssl.c | 27 ++- packages/bun-usockets/src/internal/internal.h | 2 +- packages/bun-usockets/src/libusockets.h | 2 +- packages/bun-uws/src/App.h | 19 +- src/bake/DevServer.zig | 5 +- src/bun.js/api/BunObject.zig | 123 +++------- src/bun.js/api/server.zig | 224 ++++++++++++------ src/deps/libuwsockets.cpp | 10 +- src/deps/uws.zig | 14 +- test/js/bun/http/bun-serve-ssl.test.ts | 152 ++++++++++++ 11 files changed, 396 insertions(+), 187 deletions(-) create mode 100644 test/js/bun/http/bun-serve-ssl.test.ts diff --git a/packages/bun-usockets/src/context.c b/packages/bun-usockets/src/context.c index 664f7dabdd477e..2f83ec72228f1d 100644 --- a/packages/bun-usockets/src/context.c +++ b/packages/bun-usockets/src/context.c @@ -212,12 +212,13 @@ void us_socket_context_add_server_name(int ssl, struct us_socket_context_t *cont } #endif } -void us_bun_socket_context_add_server_name(int ssl, struct us_socket_context_t *context, const char *hostname_pattern, struct us_bun_socket_context_options_t options, void *user) { +int us_bun_socket_context_add_server_name(int ssl, struct us_socket_context_t *context, const char *hostname_pattern, struct us_bun_socket_context_options_t options, void *user) { #ifndef LIBUS_NO_SSL if (ssl) { - us_bun_internal_ssl_socket_context_add_server_name((struct us_internal_ssl_socket_context_t *) context, hostname_pattern, options, user); + return us_bun_internal_ssl_socket_context_add_server_name((struct us_internal_ssl_socket_context_t *) context, hostname_pattern, options, user); } #endif + return 0; } /* Remove SNI context */ diff --git a/packages/bun-usockets/src/crypto/openssl.c b/packages/bun-usockets/src/crypto/openssl.c index 2c0420109595b4..ddd2504fa6f5da 100644 --- a/packages/bun-usockets/src/crypto/openssl.c +++ b/packages/bun-usockets/src/crypto/openssl.c @@ -855,6 +855,11 @@ create_ssl_context_from_options(struct us_socket_context_options_t options) { } } + if (ERR_peek_error() != 0) { + free_ssl_context(ssl_context); + return NULL; + } + /* This must be free'd with free_ssl_context, not SSL_CTX_free */ return ssl_context; } @@ -1106,6 +1111,8 @@ int us_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) { SSL_CTX *create_ssl_context_from_bun_options( struct us_bun_socket_context_options_t options, enum create_bun_socket_error_t *err) { + ERR_clear_error(); + /* Create the context */ SSL_CTX *ssl_context = SSL_CTX_new(TLS_method()); @@ -1211,6 +1218,9 @@ SSL_CTX *create_ssl_context_from_bun_options( return NULL; } + // It may return spurious errors here. + ERR_clear_error(); + if (options.reject_unauthorized) { SSL_CTX_set_verify(ssl_context, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, @@ -1336,7 +1346,7 @@ void us_internal_ssl_socket_context_add_server_name( } } -void us_bun_internal_ssl_socket_context_add_server_name( +int us_bun_internal_ssl_socket_context_add_server_name( struct us_internal_ssl_socket_context_t *context, const char *hostname_pattern, struct us_bun_socket_context_options_t options, void *user) { @@ -1344,6 +1354,9 @@ void us_bun_internal_ssl_socket_context_add_server_name( /* Try and construct an SSL_CTX from options */ enum create_bun_socket_error_t err = CREATE_BUN_SOCKET_ERROR_NONE; SSL_CTX *ssl_context = create_ssl_context_from_bun_options(options, &err); + if (ssl_context == NULL) { + return -1; + } /* Attach the user data to this context */ if (1 != SSL_CTX_set_ex_data(ssl_context, 0, user)) { @@ -1351,15 +1364,15 @@ void us_bun_internal_ssl_socket_context_add_server_name( printf("CANNOT SET EX DATA!\n"); abort(); #endif + return -1; } - /* We do not want to hold any nullptr's in our SNI tree */ - if (ssl_context) { - if (sni_add(context->sni, hostname_pattern, ssl_context)) { - /* If we already had that name, ignore */ - free_ssl_context(ssl_context); - } + if (sni_add(context->sni, hostname_pattern, ssl_context)) { + /* If we already had that name, ignore */ + free_ssl_context(ssl_context); } + + return 0; } void us_internal_ssl_socket_context_on_server_name( diff --git a/packages/bun-usockets/src/internal/internal.h b/packages/bun-usockets/src/internal/internal.h index abc24a4e8300c2..6c3ce73906ba02 100644 --- a/packages/bun-usockets/src/internal/internal.h +++ b/packages/bun-usockets/src/internal/internal.h @@ -302,7 +302,7 @@ void us_internal_ssl_socket_context_add_server_name( us_internal_ssl_socket_context_r context, const char *hostname_pattern, struct us_socket_context_options_t options, void *user); -void us_bun_internal_ssl_socket_context_add_server_name( +int us_bun_internal_ssl_socket_context_add_server_name( us_internal_ssl_socket_context_r context, const char *hostname_pattern, struct us_bun_socket_context_options_t options, void *user); diff --git a/packages/bun-usockets/src/libusockets.h b/packages/bun-usockets/src/libusockets.h index e4a568cea1ca9f..6c93a24ee76468 100644 --- a/packages/bun-usockets/src/libusockets.h +++ b/packages/bun-usockets/src/libusockets.h @@ -234,7 +234,7 @@ unsigned short us_socket_context_timestamp(int ssl, us_socket_context_r context) /* Adds SNI domain and cert in asn1 format */ void us_socket_context_add_server_name(int ssl, us_socket_context_r context, const char *hostname_pattern, struct us_socket_context_options_t options, void *user); -void us_bun_socket_context_add_server_name(int ssl, us_socket_context_r context, const char *hostname_pattern, struct us_bun_socket_context_options_t options, void *user); +int us_bun_socket_context_add_server_name(int ssl, us_socket_context_r context, const char *hostname_pattern, struct us_bun_socket_context_options_t options, void *user); void us_socket_context_remove_server_name(int ssl, us_socket_context_r context, const char *hostname_pattern); void us_socket_context_on_server_name(int ssl, us_socket_context_r context, void (*cb)(us_socket_context_r context, const char *hostname)); void *us_socket_server_name_userdata(int ssl, us_socket_r s); diff --git a/packages/bun-uws/src/App.h b/packages/bun-uws/src/App.h index be91e146d68323..0a44054bf95860 100644 --- a/packages/bun-uws/src/App.h +++ b/packages/bun-uws/src/App.h @@ -106,14 +106,17 @@ struct TemplatedApp { /* Server name */ - TemplatedApp &&addServerName(std::string hostname_pattern, SocketContextOptions options = {}) { + TemplatedApp &&addServerName(std::string hostname_pattern, SocketContextOptions options = {}, bool *success = nullptr) { /* Do nothing if not even on SSL */ if constexpr (SSL) { /* First we create a new router for this domain */ auto *domainRouter = new HttpRouter::RouterData>(); - us_bun_socket_context_add_server_name(SSL, (struct us_socket_context_t *) httpContext, hostname_pattern.c_str(), options, domainRouter); + int result = us_bun_socket_context_add_server_name(SSL, (struct us_socket_context_t *) httpContext, hostname_pattern.c_str(), options, domainRouter); + if (success) { + *success = result == 0; + } } return std::move(*this); @@ -238,6 +241,18 @@ struct TemplatedApp { httpContext = HttpContext::create(Loop::get(), options); } + TemplatedApp(HttpContext &context) { + httpContext = &context; + } + + static TemplatedApp* create(SocketContextOptions options = {}) { + auto* httpContext = HttpContext::create(Loop::get(), options); + if (!httpContext) { + return nullptr; + } + return new TemplatedApp(*httpContext); + } + bool constructorFailed() { return !httpContext; } diff --git a/src/bake/DevServer.zig b/src/bake/DevServer.zig index 6c19d2893f8eb7..8464d24bd7ac1f 100644 --- a/src/bake/DevServer.zig +++ b/src/bake/DevServer.zig @@ -168,7 +168,10 @@ pub fn init(options: Options) !*DevServer { else null; - const app = App.create(.{}); + const app = App.create(.{}) orelse { + Output.prettyErrorln("Failed to create app", .{}); + return error.AppInitialization; + }; const separate_ssr_graph = if (options.framework.server_components) |sc| sc.separate_ssr_graph else false; diff --git a/src/bun.js/api/BunObject.zig b/src/bun.js/api/BunObject.zig index 7ccdc67e6f42ff..d272114f1f1c50 100644 --- a/src/bun.js/api/BunObject.zig +++ b/src/bun.js/api/BunObject.zig @@ -3334,8 +3334,6 @@ pub fn serve( break :brk config; }; - var exception_value: *JSC.JSValue = undefined; - if (config.allow_hot) { if (globalObject.bunVM().hotMap()) |hot| { if (config.id.len == 0) { @@ -3370,98 +3368,43 @@ pub fn serve( } } - // Listen happens on the next tick! - // This is so we can return a Server object - if (config.ssl_config != null) { - if (config.development) { - var server = JSC.API.DebugHTTPSServer.init(config, globalObject.ptr()); - exception_value = &server.thisObject; - server.listen(); - if (!server.thisObject.isEmpty()) { - exception_value.unprotect(); - globalObject.throwValue(server.thisObject); - server.thisObject = JSC.JSValue.zero; - server.deinit(); - return .zero; - } - const obj = server.toJS(globalObject); - obj.protect(); - - server.thisObject = obj; - - if (config.allow_hot) { - if (globalObject.bunVM().hotMap()) |hot| { - hot.insert(config.id, server); - } - } - return obj; - } else { - var server = JSC.API.HTTPSServer.init(config, globalObject.ptr()); - exception_value = &server.thisObject; - server.listen(); - if (!exception_value.isEmpty()) { - exception_value.unprotect(); - globalObject.throwValue(exception_value.*); - server.thisObject = JSC.JSValue.zero; - server.deinit(); - return .zero; - } - const obj = server.toJS(globalObject); - obj.protect(); - server.thisObject = obj; - - if (config.allow_hot) { - if (globalObject.bunVM().hotMap()) |hot| { - hot.insert(config.id, server); - } - } - return obj; - } - } else { - if (config.development) { - var server = JSC.API.DebugHTTPServer.init(config, globalObject.ptr()); - exception_value = &server.thisObject; - server.listen(); - if (!exception_value.isEmpty()) { - exception_value.unprotect(); - globalObject.throwValue(exception_value.*); - server.thisObject = JSC.JSValue.zero; - server.deinit(); - return .zero; - } - const obj = server.toJS(globalObject); - obj.protect(); - server.thisObject = obj; + switch (config.ssl_config != null) { + inline else => |has_ssl_config| { + switch (config.development) { + inline else => |development| { + const ServerType = comptime switch (development) { + true => switch (has_ssl_config) { + true => JSC.API.DebugHTTPSServer, + false => JSC.API.DebugHTTPServer, + }, + false => switch (has_ssl_config) { + true => JSC.API.HTTPSServer, + false => JSC.API.HTTPServer, + }, + }; - if (config.allow_hot) { - if (globalObject.bunVM().hotMap()) |hot| { - hot.insert(config.id, server); - } - } - return obj; - } else { - var server = JSC.API.HTTPServer.init(config, globalObject.ptr()); - exception_value = &server.thisObject; - server.listen(); - if (!exception_value.isEmpty()) { - exception_value.unprotect(); - globalObject.throwValue(exception_value.*); - server.thisObject = JSC.JSValue.zero; - server.deinit(); - return .zero; - } - const obj = server.toJS(globalObject); - obj.protect(); + var server = ServerType.init(config, globalObject); + if (globalObject.hasException()) { + return .zero; + } + server.listen(); + if (globalObject.hasException()) { + return .zero; + } + const obj = server.toJS(globalObject); + obj.protect(); - server.thisObject = obj; + server.thisObject = obj; - if (config.allow_hot) { - if (globalObject.bunVM().hotMap()) |hot| { - hot.insert(config.id, server); - } + if (config.allow_hot) { + if (globalObject.bunVM().hotMap()) |hot| { + hot.insert(config.id, server); + } + } + return obj; + }, } - return obj; - } + }, } unreachable; diff --git a/src/bun.js/api/server.zig b/src/bun.js/api/server.zig index a5350abd3a3f19..0efe72a66b9d22 100644 --- a/src/bun.js/api/server.zig +++ b/src/bun.js/api/server.zig @@ -1460,30 +1460,32 @@ pub const ServerConfig = struct { return; } while (value_iter.next()) |item| { - if (SSLConfig.inJS(vm, global, item, exception)) |ssl_config| { - if (args.ssl_config == null) { - args.ssl_config = ssl_config; - } else { - if (ssl_config.server_name == null or std.mem.span(ssl_config.server_name).len == 0) { - var config = ssl_config; - defer config.deinit(); - JSC.throwInvalidArguments("SNI tls object must have a serverName", .{}, global, exception); - return; - } - if (args.sni == null) { - args.sni = bun.BabyList(SSLConfig).initCapacity(bun.default_allocator, value_iter.len - 1) catch bun.outOfMemory(); - } + var ssl_config = SSLConfig.inJS(vm, global, item, exception) orelse { + if (exception.* != null) { + return; + } - args.sni.?.push(bun.default_allocator, ssl_config) catch bun.outOfMemory(); + if (global.hasException()) { + return; } - } - if (exception.* != null) { - return; - } + // Backwards-compatibility; we ignored empty tls objects. + continue; + }; - if (global.hasException()) { - return; + if (args.ssl_config == null) { + args.ssl_config = ssl_config; + } else { + if (ssl_config.server_name == null or std.mem.span(ssl_config.server_name).len == 0) { + defer ssl_config.deinit(); + JSC.throwInvalidArguments("SNI tls object must have a serverName", .{}, global, exception); + return; + } + if (args.sni == null) { + args.sni = bun.BabyList(SSLConfig).initCapacity(bun.default_allocator, value_iter.len - 1) catch bun.outOfMemory(); + } + + args.sni.?.push(bun.default_allocator, ssl_config) catch bun.outOfMemory(); } } } else { @@ -5800,7 +5802,8 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp listener: ?*App.ListenSocket = null, thisObject: JSC.JSValue = JSC.JSValue.zero, - app: *App = undefined, + /// Potentially null before listen() is called, and once .destroy() is called. + app: ?*App = null, vm: *JSC.VirtualMachine = undefined, globalThis: *JSGlobalObject, base_url_string_for_joining: string = "", @@ -5812,7 +5815,6 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp listen_callback: JSC.AnyTask = undefined, allocator: std.mem.Allocator, poll_ref: Async.KeepAlive = .{}, - temporary_url_buffer: std.ArrayListUnmanaged(u8) = .{}, cached_hostname: bun.String = bun.String.empty, @@ -5854,7 +5856,7 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp return JSValue.jsNumber(0); } - return JSValue.jsNumber((this.app.num_subscribers(topic.slice()))); + return JSValue.jsNumber((this.app.?.num_subscribers(topic.slice()))); } pub usingnamespace NamespaceType; @@ -5900,7 +5902,7 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp if (this.config.websocket == null) return JSValue.jsNumber(0); - const app = this.app; + const app = this.app.?; if (topic.len == 0) { httplog("publish() topic invalid", .{}); @@ -6124,7 +6126,7 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp pub fn onReloadFromZig(this: *ThisServer, new_config: *ServerConfig, globalThis: *JSC.JSGlobalObject) void { httplog("onReload", .{}); - this.app.clearRoutes(); + this.app.?.clearRoutes(); // only reload those two if (this.config.onRequest != new_config.onRequest) { @@ -6572,7 +6574,7 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp ws.handler.app = null; } this.flags.terminated = true; - this.app.close(); + this.app.?.close(); } } @@ -6595,7 +6597,7 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp if (!this.flags.terminated) { this.flags.terminated = true; - this.app.close(); + this.app.?.close(); } const task = bun.default_allocator.create(JSC.AnyTask) catch unreachable; @@ -6609,7 +6611,11 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp this.all_closed_promise.deinit(); this.config.deinit(); - this.app.destroy(); + if (this.app) |app| { + this.app = null; + app.destroy(); + } + this.destroy(); } @@ -6645,7 +6651,8 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp noinline fn onListenFailed(this: *ThisServer) void { httplog("onListenFailed", .{}); - this.unref(); + + const globalThis = this.globalThis; var error_instance = JSC.JSValue.zero; var output_buf: [4096]u8 = undefined; @@ -6698,7 +6705,7 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp if (written > 0) { const message = output_buf[0..written]; - error_instance = this.globalThis.createErrorInstance("OpenSSL {s}", .{message}); + error_instance = globalThis.createErrorInstance("OpenSSL {s}", .{message}); BoringSSL.ERR_clear_error(); } } @@ -6715,7 +6722,7 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp .message = bun.String.init(std.fmt.bufPrint(&output_buf, "permission denied {s}:{d}", .{ tcp.hostname orelse "0.0.0.0", tcp.port }) catch "Failed to start server"), .code = bun.String.static("EACCES"), .syscall = bun.String.static("listen"), - }).toErrorInstance(this.globalThis); + }).toErrorInstance(globalThis); break :error_set; } } @@ -6723,7 +6730,7 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp .message = bun.String.init(std.fmt.bufPrint(&output_buf, "Failed to start server. Is port {d} in use?", .{tcp.port}) catch "Failed to start server"), .code = bun.String.static("EADDRINUSE"), .syscall = bun.String.static("listen"), - }).toErrorInstance(this.globalThis); + }).toErrorInstance(globalThis); } }, .unix => |unix| { @@ -6733,27 +6740,20 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp .message = bun.String.init(std.fmt.bufPrint(&output_buf, "Failed to listen on unix socket {}", .{bun.fmt.QuotedFormatter{ .text = unix }}) catch "Failed to start server"), .code = bun.String.static("EADDRINUSE"), .syscall = bun.String.static("listen"), - }).toErrorInstance(this.globalThis); + }).toErrorInstance(globalThis); }, else => |e| { var sys_err = bun.sys.Error.fromCode(e, .listen); sys_err.path = unix; - error_instance = sys_err.toJSC(this.globalThis); + error_instance = sys_err.toJSC(globalThis); }, } }, } } - // store the exception in here - // toErrorInstance clones the string error_instance.ensureStillAlive(); - error_instance.protect(); - this.thisObject = error_instance; - - // reference it in stack memory - this.thisObject.ensureStillAlive(); - return; + globalThis.throwValue(error_instance); } pub fn onListen(this: *ThisServer, socket: ?*App.ListenSocket) void { @@ -7082,19 +7082,20 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp } fn setRoutes(this: *ThisServer) void { + const app = this.app.?; if (this.config.static_routes.items.len > 0) { this.config.applyStaticRoutes( ssl_enabled, AnyServer.from(this), - this.app, + app, ); } if (this.config.websocket) |*websocket| { websocket.globalObject = this.globalThis; - websocket.handler.app = this.app; + websocket.handler.app = app; websocket.handler.flags.ssl = ssl_enabled; - this.app.ws( + app.ws( "/*", this, 0, @@ -7102,61 +7103,113 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp ); } - this.app.any("/*", *ThisServer, this, onRequest); + app.any("/*", *ThisServer, this, onRequest); if (comptime debug_mode) { - this.app.get("/bun:info", *ThisServer, this, onBunInfoRequest); + app.get("/bun:info", *ThisServer, this, onBunInfoRequest); if (this.config.inspector) { JSC.markBinding(@src()); - Bun__addInspector(ssl_enabled, this.app, this.globalThis); + Bun__addInspector(ssl_enabled, app, this.globalThis); } - this.app.get("/src:/*", *ThisServer, this, onSrcRequest); + app.get("/src:/*", *ThisServer, this, onSrcRequest); } } pub fn listen(this: *ThisServer) void { httplog("listen", .{}); + var app: *App = undefined; + const globalThis = this.globalThis; if (ssl_enabled) { BoringSSL.load(); const ssl_config = this.config.ssl_config orelse @panic("Assertion failure: ssl_config"); const ssl_options = ssl_config.asUSockets(); - this.app = App.create(ssl_options); + + app = App.create(ssl_options) orelse { + if (!globalThis.hasException()) { + if (!throwSSLErrorIfNecessary(globalThis)) { + globalThis.throw("Failed to create HTTP server", .{}); + } + } + + this.app = null; + this.deinit(); + return; + }; + + this.app = app; this.setRoutes(); + // add serverName to the SSL context using default ssl options - if (ssl_config.server_name != null) { - const servername_len = std.mem.span(ssl_config.server_name).len; - if (servername_len > 0) { - this.app.addServerNameWithOptions(ssl_config.server_name, ssl_options); - this.app.domain(ssl_config.server_name[0..servername_len :0]); + if (ssl_config.server_name) |server_name_ptr| { + const server_name: [:0]const u8 = std.mem.span(server_name_ptr); + if (server_name.len > 0) { + app.addServerNameWithOptions(server_name, ssl_options) catch { + if (!globalThis.hasException()) { + if (!throwSSLErrorIfNecessary(globalThis)) { + globalThis.throw("Failed to add serverName: {s}", .{server_name}); + } + } + + this.deinit(); + return; + }; + if (throwSSLErrorIfNecessary(globalThis)) { + this.deinit(); + return; + } + + app.domain(server_name); + if (throwSSLErrorIfNecessary(globalThis)) { + this.deinit(); + return; + } + + // Ensure the routes are set for that domain name. this.setRoutes(); } } // apply SNI routes if any - if (this.config.sni) |sni| { - for (sni.slice()) |sni_ssl_config| { - const sni_servername_len = std.mem.span(sni_ssl_config.server_name).len; - if (sni_servername_len > 0) { - this.app.addServerNameWithOptions(sni_ssl_config.server_name, sni_ssl_config.asUSockets()); - this.app.domain(sni_ssl_config.server_name[0..sni_servername_len :0]); + if (this.config.sni) |*sni| { + for (sni.slice()) |*sni_ssl_config| { + const sni_servername: [:0]const u8 = std.mem.span(sni_ssl_config.server_name); + if (sni_servername.len > 0) { + app.addServerNameWithOptions(sni_servername, sni_ssl_config.asUSockets()) catch { + if (!globalThis.hasException()) { + if (!throwSSLErrorIfNecessary(globalThis)) { + globalThis.throw("Failed to add serverName: {s}", .{sni_servername}); + } + } + + this.deinit(); + return; + }; + + app.domain(sni_servername); + + if (throwSSLErrorIfNecessary(globalThis)) { + this.deinit(); + return; + } + + // Ensure the routes are set for that domain name. this.setRoutes(); } } } } else { - this.app = App.create(.{}); - this.setRoutes(); - } - - this.ref(); + app = App.create(.{}) orelse { + if (!globalThis.hasException()) { + globalThis.throw("Failed to create HTTP server", .{}); + } + this.deinit(); + return; + }; + this.app = app; - // Starting up an HTTP server is a good time to GC - if (this.vm.aggressive_garbage_collection == .aggressive) { - this.vm.autoGarbageCollect(); - } else { - this.vm.eventLoop().performGC(); + this.setRoutes(); } switch (this.config.address) { @@ -7175,7 +7228,7 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp } } - this.app.listenWithConfig(*ThisServer, this, onListen, .{ + app.listenWithConfig(*ThisServer, this, onListen, .{ .port = tcp.port, .host = host, .options = if (this.config.reuse_port) 0 else 1, @@ -7183,7 +7236,7 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp }, .unix => |unix| { - this.app.listenOnUnixSocket( + app.listenOnUnixSocket( *ThisServer, this, onListen, @@ -7192,6 +7245,20 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp ); }, } + + if (globalThis.hasException()) { + this.deinit(); + return; + } + + this.ref(); + + // Starting up an HTTP server is a good time to GC + if (this.vm.aggressive_garbage_collection == .aggressive) { + this.vm.autoGarbageCollect(); + } else { + this.vm.eventLoop().performGC(); + } } }; } @@ -7304,3 +7371,14 @@ comptime { _ = Server__setIdleTimeout; } } + +fn throwSSLErrorIfNecessary(globalThis: *JSC.JSGlobalObject) bool { + const err_code = BoringSSL.ERR_get_error(); + if (err_code != 0) { + defer BoringSSL.ERR_clear_error(); + globalThis.throwValue(JSC.API.Bun.Crypto.createCryptoError(globalThis, err_code)); + return true; + } + + return false; +} diff --git a/src/deps/libuwsockets.cpp b/src/deps/libuwsockets.cpp index bc9ff248f8c23d..54973d7bc60188 100644 --- a/src/deps/libuwsockets.cpp +++ b/src/deps/libuwsockets.cpp @@ -20,7 +20,7 @@ extern "C" uWS::SocketContextOptions socket_context_options; memcpy(&socket_context_options, &options, sizeof(uWS::SocketContextOptions)); - return (uws_app_t *)new uWS::SSLApp(socket_context_options); + return (uws_app_t *)uWS::SSLApp::create(socket_context_options); } return (uws_app_t *)new uWS::App(); @@ -530,23 +530,25 @@ extern "C" uwsApp->addServerName(hostname_pattern); } } - void uws_add_server_name_with_options( + int uws_add_server_name_with_options( int ssl, uws_app_t *app, const char *hostname_pattern, struct us_bun_socket_context_options_t options) { uWS::SocketContextOptions sco; memcpy(&sco, &options, sizeof(uWS::SocketContextOptions)); + bool success = false; if (ssl) { uWS::SSLApp *uwsApp = (uWS::SSLApp *)app; - uwsApp->addServerName(hostname_pattern, sco); + uwsApp->addServerName(hostname_pattern, sco, &success); } else { uWS::App *uwsApp = (uWS::App *)app; - uwsApp->addServerName(hostname_pattern, sco); + uwsApp->addServerName(hostname_pattern, sco, &success); } + return !success; } void uws_missing_server_name(int ssl, uws_app_t *app, diff --git a/src/deps/uws.zig b/src/deps/uws.zig index defe1e3d1220a7..f44f02f5c7f4ab 100644 --- a/src/deps/uws.zig +++ b/src/deps/uws.zig @@ -3208,8 +3208,8 @@ pub fn NewApp(comptime ssl: bool) type { return uws_app_close(ssl_flag, @as(*uws_app_s, @ptrCast(this))); } - pub fn create(opts: us_bun_socket_context_options_t) *ThisApp { - return @as(*ThisApp, @ptrCast(uws_create_app(ssl_flag, opts))); + pub fn create(opts: us_bun_socket_context_options_t) ?*ThisApp { + return @ptrCast(uws_create_app(ssl_flag, opts)); } pub fn destroy(app: *ThisApp) void { return uws_app_destroy(ssl_flag, @as(*uws_app_s, @ptrCast(app))); @@ -3454,8 +3454,10 @@ pub fn NewApp(comptime ssl: bool) type { pub fn addServerName(app: *ThisApp, hostname_pattern: [*:0]const u8) void { return uws_add_server_name(ssl_flag, @as(*uws_app_t, @ptrCast(app)), hostname_pattern); } - pub fn addServerNameWithOptions(app: *ThisApp, hostname_pattern: [*:0]const u8, opts: us_bun_socket_context_options_t) void { - return uws_add_server_name_with_options(ssl_flag, @as(*uws_app_t, @ptrCast(app)), hostname_pattern, opts); + pub fn addServerNameWithOptions(app: *ThisApp, hostname_pattern: [*:0]const u8, opts: us_bun_socket_context_options_t) !void { + if (uws_add_server_name_with_options(ssl_flag, @as(*uws_app_t, @ptrCast(app)), hostname_pattern, opts) != 0) { + return error.FailedToAddServerName; + } } pub fn missingServerName(app: *ThisApp, handler: uws_missing_server_handler, user_data: ?*anyopaque) void { return uws_missing_server_name(ssl_flag, @as(*uws_app_t, @ptrCast(app)), handler, user_data); @@ -3882,7 +3884,7 @@ extern fn uws_res_prepare_for_sendfile(ssl: i32, res: *uws_res) void; extern fn uws_res_get_native_handle(ssl: i32, res: *uws_res) *Socket; extern fn uws_res_get_remote_address(ssl: i32, res: *uws_res, dest: *[*]const u8) usize; extern fn uws_res_get_remote_address_as_text(ssl: i32, res: *uws_res, dest: *[*]const u8) usize; -extern fn uws_create_app(ssl: i32, options: us_bun_socket_context_options_t) *uws_app_t; +extern fn uws_create_app(ssl: i32, options: us_bun_socket_context_options_t) ?*uws_app_t; extern fn uws_app_destroy(ssl: i32, app: *uws_app_t) void; extern fn uws_app_get(ssl: i32, app: *uws_app_t, pattern: [*c]const u8, handler: uws_method_handler, user_data: ?*anyopaque) void; extern fn uws_app_post(ssl: i32, app: *uws_app_t, pattern: [*c]const u8, handler: uws_method_handler, user_data: ?*anyopaque) void; @@ -3912,7 +3914,7 @@ extern fn uws_publish(ssl: i32, app: *uws_app_t, topic: [*c]const u8, topic_leng extern fn uws_get_native_handle(ssl: i32, app: *anyopaque) ?*anyopaque; extern fn uws_remove_server_name(ssl: i32, app: *uws_app_t, hostname_pattern: [*c]const u8) void; extern fn uws_add_server_name(ssl: i32, app: *uws_app_t, hostname_pattern: [*c]const u8) void; -extern fn uws_add_server_name_with_options(ssl: i32, app: *uws_app_t, hostname_pattern: [*c]const u8, options: us_bun_socket_context_options_t) void; +extern fn uws_add_server_name_with_options(ssl: i32, app: *uws_app_t, hostname_pattern: [*c]const u8, options: us_bun_socket_context_options_t) i32; extern fn uws_missing_server_name(ssl: i32, app: *uws_app_t, handler: uws_missing_server_handler, user_data: ?*anyopaque) void; extern fn uws_filter(ssl: i32, app: *uws_app_t, handler: uws_filter_handler, user_data: ?*anyopaque) void; extern fn uws_ws(ssl: i32, app: *uws_app_t, ctx: *anyopaque, pattern: [*]const u8, pattern_len: usize, id: usize, behavior: *const WebSocketBehavior) void; diff --git a/test/js/bun/http/bun-serve-ssl.test.ts b/test/js/bun/http/bun-serve-ssl.test.ts new file mode 100644 index 00000000000000..6e1bdee5849f50 --- /dev/null +++ b/test/js/bun/http/bun-serve-ssl.test.ts @@ -0,0 +1,152 @@ +import { describe, expect, test } from "bun:test"; +import privateKey from "../../third_party/jsonwebtoken/priv.pem" with { type: "text" }; +import publicKey from "../../third_party/jsonwebtoken/pub.pem" with { type: "text" }; +import { tls } from "harness"; + +describe("Bun.serve SSL validations", () => { + const fixtures = [ + { + label: "invalid key", + tls: { + key: privateKey.slice(100), + cert: publicKey, + }, + }, + { + label: "invalid key #2", + tls: { + key: privateKey.slice(0, -20), + cert: publicKey, + }, + }, + { + label: "invalid cert", + tls: { + key: privateKey, + cert: publicKey.slice(0, -40), + }, + }, + { + label: "invalid cert #2", + tls: [ + { + key: privateKey, + cert: publicKey, + serverName: "error-mc-erroryface.com", + }, + { + key: privateKey, + cert: publicKey.slice(0, -40), + serverName: "error-mc-erroryface.co.uk", + }, + ], + }, + { + label: "invalid serverName: missing serverName", + tls: [ + { + key: privateKey, + cert: publicKey, + serverName: "hello.com", + }, + { + key: privateKey, + cert: publicKey, + }, + ], + }, + { + label: "invalid serverName: empty serverName", + tls: [ + { + key: privateKey, + cert: publicKey, + serverName: "hello.com", + }, + { + key: privateKey, + cert: publicKey, + serverName: "", + }, + ], + }, + ]; + for (const development of [true, false]) { + for (const fixture of fixtures) { + test(`${fixture.label} ${development ? "development" : "production"}`, () => { + expect(() => { + Bun.serve({ + port: 0, + tls: fixture.tls, + fetch: () => new Response("Hello, world!"), + development, + }); + }).toThrow(); + }); + } + } + + const validFixtures = [ + { + label: "valid", + tls: { + key: privateKey, + cert: publicKey, + }, + }, + { + label: "valid 2", + tls: [ + { + key: privateKey, + cert: publicKey, + serverName: "localhost", + }, + { + key: privateKey, + cert: publicKey, + serverName: "localhost2.com", + }, + ], + }, + ]; + for (const development of [true, false]) { + for (const fixture of validFixtures) { + test(`${fixture.label} ${development ? "development" : "production"}`, async () => { + using server = Bun.serve({ + port: 0, + tls: fixture.tls, + fetch: () => new Response("Hello, world!"), + development, + }); + expect(server.url).toBeDefined(); + expect().pass(); + let serverNames = Array.isArray(fixture.tls) ? fixture.tls.map(({ serverName }) => serverName) : ["localhost"]; + + for (const serverName of serverNames) { + const res = await fetch(server.url, { + headers: { + Host: serverName, + }, + tls: { + rejectUnauthorized: false, + }, + keepAlive: false, + }); + expect(res.status).toBe(200); + expect(await res.text()).toBe("Hello, world!"); + } + + const res = await fetch(server.url, { + headers: { + Host: "badhost.com", + }, + tls: { + rejectUnauthorized: false, + }, + keepAlive: false, + }); + }); + } + } +});