RTG-5132 Bump BoringSSL - #524
Conversation
Most of our patches required work: 1. pq: (a) Upstream removed Kyber. We keep X25519Kyber768 and P256Kyber768 by implementing Kyber768 on top of ML-KEM-768. (b) Improve documentation of what's currently supported. 2. allow-crl-extensions-bad-version: Properly document what the patch actually does. Also improve its tests. 3. relax-cert-validation: Upstream removed enforce_rsa_key_usage; inline the behaviour instead.
Lekensteyn
left a comment
Lekensteyn
left a comment
There was a problem hiding this comment.
LGTM. I was worried about the impact of the RSA Key Usage not relaxing TLS 1.3, but that was already the status quo before.
What wasn't immediately obvious to me, and explains why the reported kind of breakage is not more widespread: X.509 certificates with RSA keys pass the key usage check either (1) when the requested key usage bit is set, or (2) when the Key Usage extension is missing.
The last case should cover the majority of RSA keys, like the one generated with openssl req -new -x509 -keyout server.pem -out server.pem -sha256 -nodes -subj /
Lekensteyn
left a comment
Lekensteyn
left a comment
There was a problem hiding this comment.
While trying to apply the boring-pq patch on boringssl, I noticed some more minor issues. See comments.
| @@ -141,21 +157,30 @@ index 055b3e025..09d6508f4 100644 | |||
| +OPENSSL_EXPORT void SSL_CTX_use_second_keyshare(SSL_CTX *ctx, int enabled); | |||
There was a problem hiding this comment.
Unit tests (part of ninja run_tests) fail unless this is applied:
--- a/include/openssl/prefix_symbols.h
+++ b/include/openssl/prefix_symbols.h
@@ -2102,2 +2102,3 @@
#pragma redefine_extname SSL_CTX_use_psk_identity_hint BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_CTX_use_psk_identity_hint)
+#pragma redefine_extname SSL_CTX_use_second_keyshare BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_CTX_use_second_keyshare)
#pragma redefine_extname SSL_ECH_KEYS_add BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_ECH_KEYS_add)
@@ -2451,2 +2452,3 @@
#pragma redefine_extname SSL_use_psk_identity_hint BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_use_psk_identity_hint)
+#pragma redefine_extname SSL_use_second_keyshare BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_use_second_keyshare)
#pragma redefine_extname SSL_used_hello_retry_request BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_used_hello_retry_request)
@@ -5239,2 +5241,3 @@
#define SSL_CTX_use_psk_identity_hint BORINGSSL_ADD_PREFIX(SSL_CTX_use_psk_identity_hint)
+#define SSL_CTX_use_second_keyshare BORINGSSL_ADD_PREFIX(SSL_CTX_use_second_keyshare)
#define SSL_ECH_KEYS_add BORINGSSL_ADD_PREFIX(SSL_ECH_KEYS_add)
@@ -5588,2 +5591,3 @@
#define SSL_use_psk_identity_hint BORINGSSL_ADD_PREFIX(SSL_use_psk_identity_hint)
+#define SSL_use_second_keyshare BORINGSSL_ADD_PREFIX(SSL_use_second_keyshare)
#define SSL_used_hello_retry_request BORINGSSL_ADD_PREFIX(SSL_used_hello_retry_request)This possibly changed in commit google/boringssl@d32c35e (committed 2026-01-27, part of 0.20260204.0).
There was a problem hiding this comment.
To prevent this From line from changing after patch changes, I used --zero-commit:
git format-patch --zero-commit --no-numbered --no-signature
There was a problem hiding this comment.
Let's prepend SSL_GROUP_X25519_MLKEM768 here to ensure it is enabled when using SSL_set_compliance_policy(ssl, ssl_compliance_policy_fips_202205).
Alternatively, we stop patching this and require our users to move away from this API.
There was a problem hiding this comment.
We have users of the API
There was a problem hiding this comment.
This note is obsolete and needs to be removed, X25519MLKEM768 is already enabled by default since 0.20260713.0 (commit date 2026-07-09) via google/boringssl@e748fac
| index 53fd5ea99..e21047412 100644 | ||
| --- a/ssl/test/runner/common.go | ||
| +++ b/ssl/test/runner/common.go | ||
| @@ -2186,7 +2186,7 @@ type ProtocolBugs struct { |
There was a problem hiding this comment.
This diff below seems incorrect and should be removed:
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -2186,7 +2186,7 @@ type ProtocolBugs struct {
FailIfHelloRetryRequested bool
// FailIfPostQuantumOffered will cause a server to reject a ClientHello if
- // post-quantum curves are supported.
+ // post-quantum curves are not supported.
FailIfPostQuantumOffered bool
// ExpectKeyShares, if not nil, lists (in order) the curves that a ClientHelloThe previous comment was correct. If this flag is set, the peer will reject a Client Hello if PQ curves are included (supported).
Enable X25519MLKEM768 in the fips202205 compliance profile, previously we added P256Kyber768Draft00 to it as well. Update commit message to reflect that X25519MLKEM768 is already enabled by default. Update prefix_symbols.h for the added `SSL_use_second_keyshare` API, otherwise `ninja run_tests` fails. Remove unnecessary fuzzer and test changes (`-curves`, comments). Remove NotJustMLKEMKeyShare case, this was a wrongly resolved merge conflict, NotJustMLKEMKeyShare-X25519MLKEM768 already covers it.
2 participants
Most of our patches required work:
pq: (a) Upstream removed Kyber. We keep X25519Kyber768 and P256Kyber768 by implementing Kyber768 on top of ML-KEM-768.
(b) Improve documentation of what's currently supported.
allow-crl-extensions-bad-version: Properly document what the patch actually does. Also improve its tests.
relax-cert-validation: Upstream removed enforce_rsa_key_usage; inline the behaviour instead.
I tested the Kyber key exchanges of the new BoringSSL against the old both as client and server. I didn't add a Go test for it to keep the patch small (not wanting to bring in a Kyber implementation.)