Upgrade boring to 5.1 by cjpatton · Pull Request #2446 · cloudflare/quiche

cjpatton · 2026-04-23T23:13:33Z

Bumps the boring workspace dependency from 4.3 to 5.1 and pins the quiche/deps/boringssl submodule to match boring-sys 5.1.0. Updates test expectations and CI accordingly (see commit messages for details).
Reviewers are encouraged to review these commit-by-commit. The changes were driven mostly by CI feedback.

cjpatton · 2026-04-27T19:08:51Z

+    let out = std::process::Command::new("git")
+        .arg("apply")
+        .arg("--verbose")
+        .arg(&patch)
+        .current_dir(&patched)
+        .output()
+        .expect("running `git apply` for boring-pq.patch");


AI: Claude preferred to use git to apply the patch here. In the boring crate, we just use apply:
https://github.com/cloudflare/boring/blob/ac3469371c4da185da51647a54687edc3260be48/boring-sys/build/main.rs#L483

I don't have a strong preference. Ideally we use as much of boring-sys's build system as we can, but ultimately we still need to copy-paste it here.

ghedo · 2026-04-28T08:55:14Z

@@ -0,0 +1,706 @@
+From cb5689e091f515fc8a42ceaff08d702333e505ed Mon Sep 17 00:00:00 2001


TBH I don't think duplicating the boring patch here is the way forward.

Incidentally we've been working on removing the vendored BoringSSL from this repo and default to boring crate in #2080 which is currently blocked due to some arch-specific build failures which #2142 was meant to solve, but that in turn is blocked on test failures that the new change causes 🙈

Unfortunately I gaven't had time to spend on that in a while, but if it's something you have cycle for I think getting that completed would significantly simplify this PR as well.

It looks like #2142 is meant to solve a similar problem we encountered here. It looks like you're using QEMU there. We gave this a try, and what we found is that emulation slows things down so much that it actually causes tests to flake. See commit messages for details:
a27e8ac

What we could do is adjust the test expectations to account for varint's sensitivity to measured timings.

I'd be happy to drop vendored boringSSL as part of this PR.

I've tacked on a commit that drops the vendored boringSSL: 974059a

Note: I don't have write permission for this repo, so I can't push to #2080.

Could you create a clone PR based on #2080 or suggest a merge to the branch used by #2080?

I created #2457 with the cross image change, #2458 to remove the OpenSSL backend, and updated #2080 (which now passes CI).

It'd be better to get those merged first, since this PR is quite large, but I think we'll still want something like #2446 (comment) as well.

These are now merged, so this PR needs rebase (a few commits can be dropped as well).

Thanks @ghedo! I've rebased this PR and was able to drop a few commits relevant to them. I tacked on a couple of minor fixes needed for boring 5 here: 7843a71

ghedo · 2026-04-28T09:01:39Z

 anyhow = { version = "1" }
 assert_matches = { version = "1" }
-boring = { version = "4.3" }
+boring = { version = "5.1.0" }


I deally we'd allow both v4 and v5 (e.g. version = ">=4.21,<6") to make migration easier, and avoid blocking any application still on v4 from updating quiche until the full chain of dependencies is updated too.

I guess for RPK we'd need to provide both implementations based on which boring version is used, which I think would be fine in the short term, any other missing APIs?

Forcing downstream applications to bump to boring 5 before than can take this change to quiche seems like a way easier path. Otherwise we'll end up with a lot of hacks in cruft in quiche that will make maintenance even more complex than it already is.

What would dual-support of boring v4+v5 in quiche look like? In my experience, maintaining support for both the old and new version makes it more likely for downstream users to adopt new version over time, even if it would require more initial work upfront from upstream maintainers.

I'm guessing that moving away from the submodule is a possible pre-req.

Could support for boring 5.1 be built as a feature?

@antoniovicente do you mean a feature for quiche? I think it would be possible, but it we would have to add a lot of exceptions throughout the code and build infra. Off the top of my head:

The RPK api has changed from 4 to 5. Shouldn't be too hard to support both.

Many tests are sensitive to packet sizes and counts. These differ between 4 and 5 because PQ is enabled by default in the latter. We would either (a) set the test expectations based on which version of boring is used or (b) disable PQ in all tests that are sensitive to this. There are perhaps 10s of such test, so neither option would be too bad.

The prerequisites for building BoringSSL have changed from 4 to 5. It's perhaps the case that the prerequisites for 5 are sufficient for 4, but I haven't tested this. I'm not an expert here, but I'm worried about the maintenance burden this could add to quiche.

I can give this a try if we really really need it. My intuition is that this would add a lot of complexity that won't be worth it in the end, but I'm open to being wrong.

I was just asking how to implement support for multiple versions of boring suggested by others.

Seems like part of the problem is that boring is a leaky abstraction.

Seems like part of the problem is that boring is a leaky abstraction.

Yup, that's far. The build system is subject to change as upstream BoringSSL makes changes to their build; That's part of the reason why internally we use a pre-compiled BoringSSL. But if you're consuming boring with its default feature set, then you may have to update your build dependencies from time to time.

antoniovicente · 2026-04-29T22:28:56Z

 anyhow = { version = "1" }
 assert_matches = { version = "1" }
-boring = { version = "4.3" }
+boring = { version = "5.1.0" }


I'm guessing that moving away from the submodule is a possible pre-req.

Could support for boring 5.1 be built as a feature?

Bumps the `boring` workspace dependency from 4.3 to 5.1 and pins the `quiche/deps/boringssl` submodule to match boring-sys` 5.1.0. The main user-visible change is that post-quantum TLS key shares (X25519MLKEM768 and P256Kyber768Draft00) are now offered by default. `boringssl-boring-crate` inherits PQ defaults from `boring-sys` 5.1, which applies `boring-pq.patch` to its vendored BoringSSL. Also, update RPK to use the new credentials API in `boring-sys` 5.1. Test expectations that encoded byte counts or packet counts from handshakes with a single-Initial ClientHello have been updated for the new multi-Initial with the larger ClientHello (containing X25519MLKEM768). A small set of tests that fundamentally require a single-Initial ClientHello (e.g. they drive the handshake one packet at a time) opt out of PQ via a new `test_utils::config_no_pq(version)` helper. `Config::set_curves_list` is added as a `pub(crate)` method so `test_utils` can wire classical curves through the BoringSSL and openssl-quictls backends uniformly. The symbol is a real function on BoringSSL and a header macro on OpenSSL; each backend provides an `SSL_CTX_set1_groups_list` shim so the caller stays backend-agnostic.

BoringSSL 5.x emits C++ symbols (vtables, exception personality, std::optional, etc.) into its static archives. Linking `libquiche.a` into the C example binaries with `cc` therefore fails with undefined references to the C++ runtime. Switch the link step to `$(CXX)` so libc++/libstdc++ is pulled in automatically. Wrap the source with `-x c ... -x none` to keep the C language semantics for the .c file and let clang re-detect the following `.a` archive normally.

boring-sys 5.x's build script unconditionally passes `CMAKE_MSVC_RUNTIME_LIBRARY` for any `target_os == "windows"`. Combined with cmake's default Windows behaviour, this makes cmake look for `cl.exe` even on `*-windows-gnu` targets, where only MinGW gcc is available, and the build fails during compiler detection. Replace the `bwoodsend/setup-winlibs-action` step with a setup that mirrors `boring`'s own CI: - For `x86_64-pc-windows-gnu`, point CC/CXX/include/library at the MSYS2 toolchain preinstalled at `C:\msys64`. - For `i686-pc-windows-gnu`, install a 32-bit MSYS2 environment via `msys2/setup-msys2@v2` (the runner's preinstalled MSYS2 is 64-bit only) and force `CMAKE_GENERATOR="MinGW Makefiles"` so cmake-rs doesn't fall back to "MSYS Makefiles" + cl.exe. Also set `CXXFLAGS=-msse2` for `i686-pc-windows-msvc` to satisfy BoringSSL's x86 SSE2 requirement on that target.

`tests::initial_cwnd` asserts that `tx_cap` falls in `[expected, expected + 1]` after the handshake. For BBR2 in Startup the congestion window grows by exactly `bytes_acked` per ACK, so the post-handshake `tx_cap` equals `initial_cwnd` plus the bytes the server sent during the handshake that the client has acknowledged. That total is sensitive to the encoded length of the ACK frame's `ack_delay` field (a VarInt of microseconds since receipt), which can shift by a byte between architectures. On aarch64 and armv7 (under `cross`'s Docker+QEMU) we observe `tx_cap = expected + 2`, just outside the existing tolerance. Allow a 4-byte upper-bound tolerance and replace the prior `TODO understand` comment with an explanation of what the test is actually measuring.

Two CI jobs broke after the boring 4 -> 5 upgrade because BoringSSL is now built by `boring-sys` rather than by `quiche/src/build.rs`, and a couple of flags we used to set in-tree need to be re-injected from outside the build. 1. fuzz (nightly): BoringSSL 5.x consolidated the older `BORINGSSL_UNSAFE_{DETERMINISTIC,FUZZER}_MODE` defines into a single `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` switch. 2. quiche_multiarch (i686-unknown-linux-gnu): BoringSSL's x86 assembly requires SSE2. `boring-sys` does set `-msse2` via `util/32-bit-toolchain.cmake`, but only when host != target; the `cross` `i686:edge` image is 32-bit-native, so that branch is skipped and the build fails with `x86 assembly requires SSE2`. Add a target-scoped `CFLAGS`/`CXXFLAGS` passthrough in `Cross.toml` so `-msse2 -mfpmath=sse` reaches BoringSSL's cmake build regardless.

cjpatton force-pushed the cjpatton/boring-5 branch 11 times, most recently from 3a23c66 to 5f29272 Compare April 27, 2026 16:26