Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/http/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1456,7 +1456,7 @@ fn write_to_socket_with_buffer_fallback<const IS_SSL: bool>(
/// than the certificate (`packages/bun-usockets/src/crypto/openssl.c`) when the
/// peer went away. The other one, -71, is a fatal protocol error such as a peer
/// that does not speak TLS. Certificate problems are the positive `X509_V_ERR_*`.
const US_HANDSHAKE_ECONNRESET: i32 = -46;
const US_HANDSHAKE_ECONNRESET: i32 = uws::us_bun_verify_error_t::PEER_DISCONNECTED;

/// Why a TLS handshake that reported failure failed.
pub(crate) fn handshake_failure(error_no: i32) -> crate::Error {
Expand Down
7 changes: 6 additions & 1 deletion src/js/node/_http2_upgrade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,12 @@ function socketHandshake(
const ctx = tlsSocket._ctx;

if (!success) {
const err = verifyError || new Error("TLS handshake failed");
let err: NodeJS.ErrnoException = verifyError || new Error("TLS handshake failed");
// The peer left mid-handshake. Same wording as tlsHandshakeError in net.ts.
if (err.code === "ECONNRESET") {
const { ConnResetException } = require("internal/shared");
err = new ConnResetException("socket hang up");
}
ctx.server.emit("tlsClientError", err, tlsSocket);
tlsSocket.destroy(err);
return;
Expand Down
17 changes: 16 additions & 1 deletion src/uws/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,8 @@ pub mod ssl_wrapper {
HandshakeError,
/// Closed before the handshake finished, or a renegotiation was refused.
Aborted,
/// The peer's close_notify ended the first handshake.
PeerClosed,
}

#[derive(Clone, Copy)]
Expand Down Expand Up @@ -944,6 +946,8 @@ pub mod ssl_wrapper {
(false, us_bun_verify_error_t::default())
}
HandshakeOutcome::Aborted => (false, self.verify_error()),
// Not the X509 verdict and not an empty error: node:tls reads both as an established session.
HandshakeOutcome::PeerClosed => (false, us_bun_verify_error_t::peer_disconnected()),
};
self.flags.set_authorized(success);
// trigger the handshake callback
Expand Down Expand Up @@ -1064,7 +1068,13 @@ pub mod ssl_wrapper {
self.flags.set_received_ssl_shutdown(true);
// 2-step shutdown
let _ = self.shutdown(false);
self.handle_end_of_renegotiation();
// No session will come: report the handshake that never finished, then close.
if self.flags.handshake_state() == HandshakeState::HandshakePending {
self.flags
.set_handshake_state(HandshakeState::HandshakeCompleted);
self.trigger_handshake_callback(HandshakeOutcome::PeerClosed);
}
self.trigger_close_callback();
return false;
}
// as far as I know these are the only errors we want to handle
Expand Down Expand Up @@ -1344,6 +1354,11 @@ pub mod ssl_wrapper {
// ssl_flush_pending_session: handshake/data callbacks first,
// then sessions.
self.flush_pending_events();
} else {
debug_assert!(
self.flags.closed_notified() || self.ssl.get().is_none(),
"update_handshake_state stopped the pass and left the wrapper open"
);
}
}

Expand Down
13 changes: 13 additions & 0 deletions src/uws_sys/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,19 @@ pub struct us_bun_verify_error_t {
impl us_bun_verify_error_t {
/// `X509_V_ERR_HOSTNAME_MISMATCH`, from the in-handshake server identity check (`ERR_TLS_CERT_ALTNAME_INVALID`).
pub const HOSTNAME_MISMATCH: core::ffi::c_int = 62;
/// `error` of [`Self::peer_disconnected`]. Not an X509 code.
pub const PEER_DISCONNECTED: core::ffi::c_int = -46;

/// The peer left before the handshake finished, like `ssl_trigger_handshake_econnreset` in openssl.c.
pub const fn peer_disconnected() -> Self {
Self {
error_no: Self::PEER_DISCONNECTED,
code: c"ECONNRESET".as_ptr(),
reason:
c"Client network socket disconnected before secure TLS connection was established"
.as_ptr(),
}
}
}

impl Default for us_bun_verify_error_t {
Expand Down
39 changes: 39 additions & 0 deletions test/js/bun/http/proxy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3262,6 +3262,45 @@ test("a proxy's own reply to CONNECT never resolves as the https origin's respon
]);
});

test("an https origin that ends the TLS handshake with close_notify fails the tunneled request", async () => {
// The proxy opens the tunnel and keeps it open. The origin answers the ClientHello with a close_notify alert,
// so only the alert says that no TLS session will come.
const closeNotify = Buffer.from([0x15, 0x03, 0x03, 0x00, 0x02, 0x01, 0x00]);
const established = Buffer.from("HTTP/1.1 200 Connection Established\r\n\r\n");
const outcomes = [];
// "with the reply": the alert is in the same write as the reply to CONNECT, ahead of the ClientHello.
for (const alert of ["after the ClientHello", "with the reply"]) {
const sockets: net.Socket[] = [];
const proxy = net.createServer(socket => {
sockets.push(socket);
socket.on("error", () => {});
let chunks = 0;
socket.on("data", () => {
chunks++;
if (chunks === 1)
socket.write(alert === "with the reply" ? Buffer.concat([established, closeNotify]) : established);
else if (chunks === 2 && alert === "after the ClientHello") socket.write(closeNotify);
});
});
await once(proxy.listen(0, "127.0.0.1"), "listening");
try {
outcomes.push(
await fetch("https://origin.invalid/", {
proxy: `http://127.0.0.1:${(proxy.address() as net.AddressInfo).port}`,
keepalive: false,
}).then(
response => ({ resolved: response.status }),
e => ({ code: e.code }),
),
);
} finally {
for (const socket of sockets) socket.destroy();
proxy.close();
}
}
expect(outcomes).toEqual([{ code: "EPROTO" }, { code: "EPROTO" }]);
});

test("invalid TLS options are reported the same through a proxy as directly", async () => {
// Says the tunnel is up; the TLS options are what fails next.
const proxy = net.createServer(socket => {
Expand Down
35 changes: 35 additions & 0 deletions test/js/node/http2/node-http2-upgrade.test.mts
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,41 @@ describe("HTTP/2 upgrade — server TLS options", () => {
});
});

describe("HTTP/2 upgrade — failed TLS handshake", () => {
test("a peer that ends the handshake with close_notify is reported as tlsClientError", async () => {
const h2Server = http2.createSecureServer(TLS);
h2Server.on("error", () => {});
const netServer = net.createServer(socket => {
socket.on("error", () => {});
h2Server.emit("connection", socket);
});
const port = await new Promise<number>(resolve => {
netServer.listen(0, "127.0.0.1", () => resolve((netServer.address() as net.AddressInfo).port));
});
// The alert is the first record, and the peer keeps the connection open behind it.
const peer = net.connect(port, "127.0.0.1", () => {
peer.write(Buffer.from([0x15, 0x03, 0x03, 0x00, 0x02, 0x01, 0x00]));
});
peer.on("error", () => {});
try {
const [err] = await once(h2Server, "tlsClientError");
if (typeof Bun !== "undefined") {
// BoringSSL reads the alert as the peer's close, at every point of the handshake.
assert.deepStrictEqual(
{ code: err.code, message: err.message },
{ code: "ECONNRESET", message: "socket hang up" },
);
} else {
// OpenSSL refuses an alert ahead of the ClientHello.
assert.strictEqual(err.code, "ERR_SSL_UNEXPECTED_MESSAGE");
}
} finally {
peer.destroy();
netServer.close();
}
});
});

if (typeof Bun !== "undefined") {
describe("Node.js compatibility", () => {
test("tests should run on node.js", async () => {
Expand Down
75 changes: 75 additions & 0 deletions test/js/node/tls/node-tls-connect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2118,6 +2118,81 @@ describe("a TLS socket over a Duplex transport reports that transport's error",
});
});

describe("a TLS server wrap over a Duplex transport whose peer ends the handshake with close_notify", () => {
// The peer keeps the transport open, so only the alert says that no handshake will come. BoringSSL reads the
// alert as the peer's close at every point of the handshake, so the wrap reports what it reports for a peer
// that disconnects. Node's OpenSSL refuses an alert ahead of the ClientHello (ERR_SSL_UNEXPECTED_MESSAGE) and
// reads one behind it as a plain end of the stream. The client side of this case runs on both runtimes in
// node-tls-duplex-end-verify.test.ts.
const closeNotify = Buffer.from([0x15, 0x03, 0x03, 0x00, 0x02, 0x01, 0x00]);
const hangUp = { code: "ECONNRESET", message: "socket hang up" };
const serverContext = (options: tls.SecureContextOptions = {}) => ({
isServer: true,
secureContext: tls.createSecureContext({ ...COMMON_CERT_, ...options }),
});
// `onWrite` gets each chunk that the TLS socket writes to the transport.
const makeTransport = (onWrite: (chunk: Buffer) => void = () => {}) =>
new Duplex({
read() {},
write(chunk, _encoding, callback) {
callback();
onWrite(chunk);
},
});
const firstError = async (socket: TLSSocket) => {
const [err] = await once(socket, "error");
return { code: err.code, message: err.message };
};

it("the alert is the first record", async () => {
const transport = makeTransport();
const wrapped = new TLSSocket(transport, serverContext());
const failed = firstError(wrapped);
setImmediate(() => transport.push(closeNotify));
expect(await failed).toEqual(hangUp);
expect(wrapped.destroyed).toBe(true);
transport.destroy();
});

it("the alert follows a ClientHello", async () => {
// A client that is thrown away writes the ClientHello.
const hello = Promise.withResolvers<Buffer>();
const donor = tls.connect({ socket: makeTransport(hello.resolve), rejectUnauthorized: false });
donor.on("error", () => {});
const clientHello = await hello.promise;
donor.destroy();

// The wrap answers the ClientHello with its first flight. The alert is the answer to that flight. In TLS 1.2
// the client's next records are still plaintext, so the plaintext alert is one that the wrap can read.
let answered = false;
const transport: Duplex = makeTransport(() => {
if (answered) return;
answered = true;
setImmediate(() => transport.push(closeNotify));
});
const wrapped = new TLSSocket(transport, serverContext({ maxVersion: "TLSv1.2" }));
const failed = firstError(wrapped);
setImmediate(() => transport.push(clientHello));
expect(await failed).toEqual(hangUp);
expect({ answered, destroyed: wrapped.destroyed }).toEqual({ answered: true, destroyed: true });
transport.destroy();
});

it("a tls.Server reports it as 'tlsClientError'", async () => {
const server = tls.createServer(COMMON_CERT_);
const transport = makeTransport();
const reported = once(server, "tlsClientError");
server.emit("connection", transport);
setImmediate(() => transport.push(closeNotify));
const [err, socket] = await reported;
expect({ code: err.code, message: err.message, destroyed: socket.destroyed }).toEqual({
...hangUp,
destroyed: true,
});
transport.destroy();
});
});

it("delivers 'session' even when the data handler destroys the socket immediately", async () => {
// The TLS1.3 NewSessionTickets ride in the same read pass as the response
// bytes. If the parked session were only flushed after the data dispatch,
Expand Down
Loading
Loading