diff --git a/.claude/commands/upgrade-boringssl.md b/.claude/commands/upgrade-boringssl.md new file mode 100644 index 000000000000..529ae3eae8cd --- /dev/null +++ b/.claude/commands/upgrade-boringssl.md @@ -0,0 +1,100 @@ +--- +description: Upgrade Bun's BoringSSL fork (oven-sh/boringssl) to the latest upstream google/boringssl +--- + +Bun pins BoringSSL by **commit SHA** in `scripts/build/deps/boringssl.ts` (`BORINGSSL_COMMIT`). The build downloads a tarball from `oven-sh/boringssl` at that SHA — there is no submodule and `vendor/boringssl/` is git-ignored. + +The fork carries a small patch set on top of upstream (see "Preserved patches" below). Upgrading means: merge `google/boringssl` into `oven-sh/boringssl@master`, push, then bump the SHA + regenerate source lists in Bun. + +## Steps + +### 1. Clone the fork and merge upstream + +```sh +git clone https://github.com/oven-sh/boringssl.git /tmp/boringssl +cd /tmp/boringssl +git remote add upstream https://github.com/google/boringssl.git +git fetch upstream +git log --oneline $(git merge-base HEAD upstream/main)..HEAD # our patches +git merge upstream/main +``` + +Resolve conflicts **preserving the fork's additions**. Most conflicts are upstream's periodic `|...|` → `` `...` `` doc-comment restyle landing adjacent to a line we added — keep our line + upstream's comment style. For `include/openssl/nid.h`, keep upstream's new NIDs **and** ours (our NID numbers are from OpenSSL's range and don't collide with BoringSSL's sequential allocation). + +### 2. Verify the merged tree builds + +```sh +cmake -B build -GNinja -DCMAKE_BUILD_TYPE=Release +ninja -C build crypto ssl decrepit +``` + +This catches mis-resolved conflicts before they reach Bun's CI. + +### 3. Push to the fork + +The default branch is **`master`** (not `main`). + +```sh +git push origin HEAD:master +NEW_SHA=$(git rev-parse HEAD) +``` + +### 4. Bump Bun + +In the bun repo: + +- `scripts/build/deps/boringssl.ts` — set `BORINGSSL_COMMIT` to `$NEW_SHA`. +- `test/js/node/process/process.test.js` — update the `boringssl:` entry in `expectedVersions` to `$NEW_SHA`. +- Regenerate the source lists (the file's header comment has the exact one-liner). Only `gen/sources.json` is authoritative — diff old vs new and apply the delta: + + ```sh + rm -rf vendor/boringssl # force re-fetch on next build + bun bd --target=clone-boringssl + bun -e 'const j=require("./vendor/boringssl/gen/sources.json"); + const f=l=>l.map(JSON.stringify).join(", "); + for(const k of ["bcm","crypto","ssl","decrepit"]) console.log(k,"\n",f(j[k].srcs)); + console.log("asm\n",f([...j.bcm.asm,...j.crypto.asm])); + console.log("nasm\n",f([...j.bcm.nasm,...j.crypto.nasm]))' + ``` + +### 5. Build and test locally + +```sh +rm -rf vendor/boringssl +bun bd -p 'require("crypto").createHash("sha3-256").update("hi").digest("hex")' +bun bd test test/js/node/crypto/ test/js/bun/crypto/ +bun bd test test/js/node/tls/ test/js/web/fetch/fetch.tls.test.ts +``` + +### 6. Open the Bun PR + +```sh +git checkout -b claude/boringssl- +git commit -am "deps: upgrade BoringSSL to " +git push -u origin HEAD +gh pr create +``` + +Then `bun run ci:watch` and fix anything that turns up. + +## Preserved patches (what conflicts to expect) + +`git diff $(git merge-base HEAD upstream/main)..HEAD --stat` — currently ~35 files, ~550 insertions: + +- **SHA-512/224** — `crypto/fipsmodule/sha/sha512.cc.inc`, `crypto/sha/sha512.cc`, `include/openssl/{sha2,nid,digest}.h` +- **SHA3-224/256/384/512 as `EVP_MD`** — `crypto/digest/digest_extra.cc`, `crypto/fipsmodule/{digest/digests.cc.inc,keccak/*}`, `include/openssl/{digest,nid}.h` +- **HMAC-SHA3** — `crypto/hmac/hmac_test*.{cc,txt}` +- **BLAKE2b-512** — `crypto/blake2/blake2.cc`, `include/openssl/blake2.h` +- **RIPEMD160 in `crypto/` (not `decrepit/`) + `EVP_ripemd160` lookup** — `crypto/ripemd/ripemd.cc` (moved), `crypto/digest/digest_extra.cc`, `include/openssl/digest.h`, `gen/sources.*`, `build.json` +- **`EVP_PBE_validate_scrypt_params`** — `crypto/evp/scrypt.cc`, `include/openssl/evp.h` +- **Electron `SSL_want` / `EVP_CIPHER_do_all_sorted`** — `ssl/ssl_lib.cc` (return `rwstate` directly), `ssl/ssl_test.cc` (drops the corresponding test block), `decrepit/evp/evp_do_all.cc`, `crypto/cipher/get_cipher.cc`, `include/openssl/cipher.h` +- **MLDSA stack-frame pragma** — `crypto/fipsmodule/mldsa/mldsa.cc.inc` + +If upstream upstreams any of these (check `git grep` on `upstream/main` before re-applying), drop the fork's copy. + +## Things that have broken before + +- **`SSL_CTX` / `SSL_ECH_KEYS` / `SSL_CREDENTIAL` made opaque** — Bun's Rust FFI (`src/boringssl_sys/boringssl.rs`) treats them as opaque already, so this is fine, but check `packages/bun-usockets/src/crypto/openssl.c` for any direct field access. +- **`BIO_read`/`BIO_write` error-value narrowing** — can change `SSL_read` error paths over memory BIOs (`SSLWrapper` for TLS-over-duplex). If `node-tls-connect.test.ts` crashes in `flush_pending_events`, see `src/runtime/socket/UpgradedDuplex.rs::teardown` and `WindowsNamedPipe.rs`'s `WRAPPER_BUSY` for the re-entrant-drop guard. +- **Per-handshake allocation churn (PQ key shares)** grows under ASAN quarantine; RSS-delta tests like `tls-connect-socket-churn.test.ts` may need their `isASAN` bound raised. The `sslCtxLiveCount` check is the real regression guard there — if that passes and LSAN is clean, raise the RSS bound. +- **`asn1_string_st` / `GENERAL_NAME_st` layout** — Bun mirrors these in `src/boringssl_sys/boringssl.rs`; diff `include/openssl/{asn1,x509v3}.h` for field changes. diff --git a/scripts/build/deps/boringssl.ts b/scripts/build/deps/boringssl.ts index 0b367a71a445..4b15380e257e 100644 --- a/scripts/build/deps/boringssl.ts +++ b/scripts/build/deps/boringssl.ts @@ -24,7 +24,7 @@ import { quote } from "../shell.ts"; import type { Dependency, DirectBuild } from "../source.ts"; import { depSourceDir } from "../source.ts"; -const BORINGSSL_COMMIT = "0c5fce43b7ed5eb6001487ee48ac65766f5ddcd1"; +const BORINGSSL_COMMIT = "1a41b9025c2c0a37edd07ff10f6944f03e028522"; export const boringssl: Dependency = { name: "boringssl", @@ -131,7 +131,7 @@ const CRYPTO_SRCS = [ "crypto/evp/evp.cc", "crypto/evp/evp_asn1.cc", "crypto/evp/evp_ctx.cc", "crypto/evp/evp_kem.cc", "crypto/evp/p_dh.cc", "crypto/evp/p_dsa.cc", "crypto/evp/p_ec.cc", "crypto/evp/p_ed25519.cc", "crypto/evp/p_hkdf.cc", "crypto/evp/p_mldsa.cc", "crypto/evp/p_mlkem.cc", "crypto/evp/p_rsa.cc", - "crypto/evp/p_x25519.cc", "crypto/evp/pbkdf.cc", "crypto/evp/print.cc", "crypto/evp/scrypt.cc", + "crypto/evp/p_x25519.cc", "crypto/evp/p_xwing.cc", "crypto/evp/pbkdf.cc", "crypto/evp/print.cc", "crypto/evp/scrypt.cc", "crypto/evp/sign.cc", "crypto/ex_data.cc", "crypto/fipsmodule/fips_shared_support.cc", "crypto/fuzzer_mode.cc", "crypto/hpke/hpke.cc", "crypto/hrss/hrss.cc", "crypto/kyber/kyber.cc", "crypto/lhash/lhash.cc", "crypto/md4/md4.cc", "crypto/md5/md5.cc", "crypto/mem.cc", @@ -143,7 +143,7 @@ const CRYPTO_SRCS = [ "crypto/pkcs8/pkcs8_x509.cc", "crypto/poly1305/poly1305.cc", "crypto/poly1305/poly1305_arm.cc", "crypto/poly1305/poly1305_vec.cc", "crypto/pool/pool.cc", "crypto/rand/deterministic.cc", "crypto/rand/fork_detect.cc", "crypto/rand/forkunsafe.cc", "crypto/rand/getentropy.cc", - "crypto/rand/ios.cc", "crypto/rand/passive.cc", "crypto/rand/rand.cc", "crypto/rand/trusty.cc", + "crypto/rand/ios.cc", "crypto/rand/rand.cc", "crypto/rand/trusty.cc", "crypto/rand/urandom.cc", "crypto/rand/windows.cc", "crypto/rc4/rc4.cc", "crypto/refcount.cc", "crypto/ripemd/ripemd.cc", "crypto/rsa/rsa_asn1.cc", "crypto/rsa/rsa_crypt.cc", "crypto/rsa/rsa_extra.cc", "crypto/rsa/rsa_print.cc", "crypto/sha/sha1.cc", diff --git a/src/runtime/socket/UpgradedDuplex.rs b/src/runtime/socket/UpgradedDuplex.rs index 98e481c3a971..13e67a578edf 100644 --- a/src/runtime/socket/UpgradedDuplex.rs +++ b/src/runtime/socket/UpgradedDuplex.rs @@ -502,7 +502,21 @@ impl UpgradedDuplex { // clear the timer self.set_timeout(0); - self.wrapper = None; // Drop runs SSLWrapper teardown + // Neuter in place rather than `self.wrapper = None`: `teardown()` can + // run re-entrantly from `on_close` while a `SSLWrapper::handle_traffic` + // frame is still on the stack with a `*mut Self` into the `Some` + // payload. Assigning `None` to the `Option` runs `Drop` (fine - + // `deinit()` nulls `ssl`/`ctx`) but then memmoves a fresh + // `Option::None` value over the slot, whose payload bytes are stack + // garbage - the in-flight frame's `Self::r(this).ssl` then reads junk + // and `flush_pending_events` UAFs into BoringSSL. `deinit()` alone + // leaves `ssl = None` / `closed_notified = true` readable so those + // guards work; the `Option` is dropped for real when the parent + // `DuplexUpgradeContext` frees on the next tick. See WindowsNamedPipe's + // WRAPPER_BUSY for the sibling pattern. + if let Some(wrapper) = self.wrapper.as_mut() { + wrapper.deinit(); + } self.origin.deinit(); if let Some(callback) = self.on_data_callback.get() { diff --git a/test/js/node/process/process.test.js b/test/js/node/process/process.test.js index 0a1804213eaf..9d261c3861a0 100644 --- a/test/js/node/process/process.test.js +++ b/test/js/node/process/process.test.js @@ -273,7 +273,7 @@ it("process.versions", () => { // These are the ACTUAL commits built into bun (not derived values, so // bumping a dep requires updating this test too). const expectedVersions = { - boringssl: "0c5fce43b7ed5eb6001487ee48ac65766f5ddcd1", + boringssl: "1a41b9025c2c0a37edd07ff10f6944f03e028522", libarchive: "ded82291ab41d5e355831b96b0e1ff49e24d8939", mimalloc: "afb41757285694f832e7a2f164d35f5717457f96", picohttpparser: "066d2b1e9ab820703db0837a7255d92d30f0c9f5", diff --git a/test/js/node/tls/tls-connect-socket-churn.test.ts b/test/js/node/tls/tls-connect-socket-churn.test.ts index 306ffc164b57..484f3b46da23 100644 --- a/test/js/node/tls/tls-connect-socket-churn.test.ts +++ b/test/js/node/tls/tls-connect-socket-churn.test.ts @@ -48,7 +48,11 @@ test("tls.connect churn does not leak SSL_CTX or us_socket_context_t", async () // first verify all bump it). The original regression was ~50 KB/conn of // SSL_CTX; bound at 16 MB so a return of that leak (50 × 50 KB ≈ 2.5 MB on // top of the noise floor) would still trip without flaking on the noise. - const rssBound = isASAN || isDebug ? 64 * 1024 * 1024 : 16 * 1024 * 1024; + // ASAN/debug bound is high because BoringSSL's per-handshake allocation + // churn (PQ key shares, transcript buffers) lands in the ASAN quarantine + // and isn't released by Bun.gc — when this file runs after the rest of the + // tls/ suite the delta hits ~150 MB with zero LSAN-reported per-conn leak. + const rssBound = isASAN || isDebug ? 300 * 1024 * 1024 : 16 * 1024 * 1024; expect(rssAfter - rssBefore).toBeLessThan(rssBound); } finally { server.close();