Use features to set key exchange preferences

Overwrite boringSSL's default key exchange preferences with safe
defaults using feature flags:

* "kx-pq-supported" enables support for PQ key exchange algorithms.
  Classical key exchange is still preferred, but will be upgraded to PQ
  if requested.

* "kx-pq-preferred" enables preference for PQ key exchange,
  with fallback to classical key exchange if requested.

* "kx-nist-required" disables non-NIST key exchange.

Each feature implies "kx-safe-default". When this feature is enabled,
don't compile bindings for `SSL_CTX_set1_curves()` and `SslCurve`. This
is to prevent the feature flags from silently overriding curve
preferences chosen by the user.

Ideally we'd allow both: that is, use "kx-*" to set defaults, but still
allow the user to manually override them. However, this doesn't work
because by the time the `SSL_CTX` is constructed, we don't yet know
whether we're the client or server. (The "kx-*" features set different
preferences for each.) If "kx-sfe-default" is set, then the curve
preferences are set just before initiating a TLS handshake
(`SslStreamBuilder::connect()`) or waiting for a TLS handshake
(`SslStreamBuilder::accept()`).

.github/workflows/ci.yml

@@ -238,3 +238,5 @@ jobs:
       name: Run `pq-experimental` tests
     - run: cargo test --features pq-experimental,rpk
       name: Run `pq-experimental,rpk` tests
+    - run: cargo test --features kx-safe-default,pq-experimental
+      name: Run `kx-safe-default` tests

boring-sys/Cargo.toml

@@ -63,7 +63,11 @@ fips-link-precompiled = []
 # Enables Raw public key API (https://datatracker.ietf.org/doc/html/rfc7250)
 rpk = []
 
-# Enables experimental post-quantum crypto (https://blog.cloudflare.com/post-quantum-for-all/)
+# Applies a patch (`patches/boring-pq.patch`) to the boringSSL source code that
+# enables support for PQ key exchange. This feature is necessary in order to
+# compile the bindings for the default branch of boringSSL (`deps/boringssl`).
+# Alternatively, a version of boringSSL that implements the same feature set
+# can be provided by setting `BORING_BSSL_SOURCE_PATH`.
 pq-experimental = []
 
 # Disables git patching of the BoringSSL sources for features like `rpk` and `pq-experimental`, but

boring/Cargo.toml

@@ -16,6 +16,8 @@ features = ["rpk", "pq-experimental"]
 rustdoc-args = ["--cfg", "docsrs"]
 
 [features]
+# Controlling the build
+
 # Use a FIPS-validated version of boringssl.
 fips = ["boring-sys/fips"]
 
@@ -25,7 +27,11 @@ fips-link-precompiled = ["boring-sys/fips-link-precompiled"]
 # Enables Raw public key API (https://datatracker.ietf.org/doc/html/rfc7250)
 rpk = ["boring-sys/rpk"]
 
-# Enables experimental post-quantum crypto (https://blog.cloudflare.com/post-quantum-for-all/)
+# Applies a patch to the boringSSL source code that enables support for PQ key
+# exchange. This feature is necessary in order to compile the bindings for the
+# default branch of boringSSL. Alternatively, a version of boringSSL that
+# implements the same feature set can be provided by setting
+# `BORING_BSSL_SOURCE_PATH`.
 pq-experimental = ["boring-sys/pq-experimental"]
 
 # Disables git patching of the BoringSSL sources for features like `rpk` and `pq-experimental`, but
@@ -36,6 +42,28 @@ pq-experimental = ["boring-sys/pq-experimental"]
 # required patches.
 no-patches = ["boring-sys/no-patches"]
 
+# Controlling key exchange preferences at compile time
+
+# Choose key exchange preferences at compile time. This prevents the user from
+# choosing their own preferences.
+kx-safe-default = []
+
+# Support PQ key exchange. The client will prefer classical key exchange, but
+# will upgrade to PQ key exchange if requested by the server. This is the
+# safest option if you don't know if the peer supports PQ key exchange. This
+# feature implies "kx-safe-default".
+kx-client-pq-supported = ["kx-safe-default"]
+
+# Prefer PQ key exchange. The client will prefer PQ exchange, but fallback to
+# classical key exchange if requested by the server. This is the best option if
+# you know the peer supports PQ key exchange. This feature implies
+# "kx-safe-default" and "kx-client-pq-supported".
+kx-client-pq-preferred = ["kx-safe-default", "kx-client-pq-supported"]
+
+# Disable key exchange involving non-NIST key exchange on the client side.
+# Implies "kx-safe-default".
+kx-client-nist-required = ["kx-safe-default"]
+
 [dependencies]
 bitflags = { workspace = true }
 foreign-types = { workspace = true }

boring/src/ssl/mod.rs

@@ -619,10 +619,12 @@ impl SslSignatureAlgorithm {
 }
 
 /// A TLS Curve.
+#[cfg(not(feature = "kx-safe-default"))]
 #[repr(transparent)]
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub struct SslCurve(c_int);
 
+#[cfg(not(feature = "kx-safe-default"))]
 impl SslCurve {
     pub const SECP224R1: SslCurve = SslCurve(ffi::NID_secp224r1);
 
@@ -1703,6 +1705,11 @@ impl SslContextBuilder {
     /// This corresponds to [`SSL_CTX_set1_curves`]
     ///
     /// [`SSL_CTX_set1_curves`]: https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#SSL_CTX_set1_curves
+    //
+    // If the "kx-*" flags are used to set key exchange preference, then don't allow the user to
+    // set them here. This ensures we don't override the user's preference without telling them:
+    // when the flags are used, the preferences are set just before connecting or accepting.
+    #[cfg(not(feature = "kx-safe-default"))]
     pub fn set_curves(&mut self, curves: &[SslCurve]) -> Result<(), ErrorStack> {
         unsafe {
             cvt_0i(ffi::SSL_CTX_set1_curves(
@@ -2400,6 +2407,48 @@ impl SslRef {
         unsafe { ErrorCode::from_raw(ffi::SSL_get_error(self.as_ptr(), ret)) }
     }
 
+    #[cfg(feature = "kx-safe-default")]
+    fn set_curves_list(&mut self, curves: &str) -> Result<(), ErrorStack> {
+        let curves = CString::new(curves).unwrap();
+        unsafe {
+            cvt(ffi::SSL_set1_curves_list(
+                self.as_ptr(),
+                curves.as_ptr() as *const _,
+            ))
+            .map(|_| ())
+        }
+    }
+
+    #[cfg(feature = "kx-safe-default")]
+    fn client_set_default_curves_list(&mut self) -> Result<(), ErrorStack> {
+        let curves = if cfg!(feature = "kx-client-pq-preferred") {
+            if cfg!(feature = "kx-client-nist-required") {
+                "P256Kyber768Draft00:P-256:P-384"
+            } else {
+                "X25519Kyber768Draft00:X25519:P256Kyber768Draft00:P-256:P-384"
+            }
+        } else if cfg!(feature = "kx-client-pq-supported") {
+            if cfg!(feature = "kx-client-nist-required") {
+                "P-256:P-384:P256Kyber768Draft00"
+            } else {
+                "X25519:P-256:P-384:X25519Kyber768Draft00:P256Kyber768Draft00"
+            }
+        } else {
+            if cfg!(feature = "kx-client-nist-required") {
+                "P-256:P-384"
+            } else {
+                "X25519:P-256:P-384"
+            }
+        };
+
+        self.set_curves_list(curves)
+    }
+
+    #[cfg(feature = "kx-safe-default")]
+    fn server_set_default_curves_list(&mut self) -> Result<(), ErrorStack> {
+        self.set_curves_list("X25519Kyber768Draft00:P256Kyber768Draft00:X25519:P-256:P-384")
+    }
+
     /// Like [`SslContextBuilder::set_verify`].
     ///
     /// This corresponds to [`SSL_set_verify`].
@@ -3480,6 +3529,10 @@ where
     /// See `Ssl::connect`
     pub fn connect(self) -> Result<SslStream<S>, HandshakeError<S>> {
         let mut stream = self.inner;
+
+        #[cfg(feature = "kx-safe-default")]
+        stream.ssl.client_set_default_curves_list()?;
+
         let ret = unsafe { ffi::SSL_connect(stream.ssl.as_ptr()) };
         if ret > 0 {
             Ok(stream)
@@ -3503,6 +3556,10 @@ where
     /// See `Ssl::accept`
     pub fn accept(self) -> Result<SslStream<S>, HandshakeError<S>> {
         let mut stream = self.inner;
+
+        #[cfg(feature = "kx-safe-default")]
+        stream.ssl.server_set_default_curves_list()?;
+
         let ret = unsafe { ffi::SSL_accept(stream.ssl.as_ptr()) };
         if ret > 0 {
             Ok(stream)

boring/src/ssl/test/mod.rs

@@ -1115,3 +1115,23 @@ fn session_cache_size() {
     let ctx = ctx.build();
     assert_eq!(ctx.session_cache_size(), 1234);
 }
+
+#[cfg(feature = "kx-safe-default")]
+#[test]
+fn client_set_default_curves_list() {
+    let ssl_ctx = SslContextBuilder::new(SslMethod::tls()).unwrap().build();
+    let mut ssl = Ssl::new(&ssl_ctx).unwrap();
+
+    ssl.client_set_default_curves_list()
+        .expect("Failed to set curves list. Is Kyber768 missing in boringSSL?")
+}
+
+#[cfg(feature = "kx-safe-default")]
+#[test]
+fn server_set_default_curves_list() {
+    let ssl_ctx = SslContextBuilder::new(SslMethod::tls()).unwrap().build();
+    let mut ssl = Ssl::new(&ssl_ctx).unwrap();
+
+    ssl.server_set_default_curves_list()
+        .expect("Failed to set curves list. Is Kyber768 missing in boringSSL?")
+}