Remove the "kx-*" features

The "kx-*" features control default key exchange preferences. Its
implementation requires disabling APIs for manually setting curve
preferences via `set_curves()` or `set_curves_list()`.

In practice, most teams need to be able to override default preferences
at runtime anyway, which means these features were never really used.
This commit gets rid of them, thereby reducing some complexity in the
API.

Author: cjpatton

.github/workflows/ci.yml

@@ -363,8 +363,6 @@ jobs:
       name: Run `underscore-wildcards` 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
     - run: cargo test --features pq-experimental,underscore-wildcards
       name: Run `pq-experimental,underscore-wildcards` tests
     - run: cargo test --features rpk,underscore-wildcards

boring/Cargo.toml

@@ -44,28 +44,6 @@ pq-experimental = ["boring-sys/pq-experimental"]
 # those for `pq-experimental` feature apply.
 underscore-wildcards = ["boring-sys/underscore-wildcards"]
 
-# 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

@@ -747,16 +747,12 @@ impl SslCurve {
         }
     }
 
-    // We need to allow dead_code here because `SslRef::set_curves` is conditionally compiled
-    // against the absence of the `kx-safe-default` feature and thus this function is never used.
-    //
     // **NOTE**: This function only exists because the version of boringssl we currently use does
     // not expose SSL_CTX_set1_group_ids. Because `SslRef::curve()` returns the public SSL_GROUP id
     // as opposed to the internal NID, but `SslContextBuilder::set_curves()` requires the internal
     // NID, we need this mapping in place to avoid breaking changes to the public API. Once the
     // underlying boringssl version is upgraded, this should be removed in favor of the new
     // SSL_CTX_set1_group_ids API.
-    #[allow(dead_code)]
     pub fn nid(&self) -> Option<SslCurveNid> {
         match self.0 {
             ffi::SSL_GROUP_SECP224R1 => Some(ffi::NID_secp224r1),
@@ -2017,11 +2013,6 @@ impl SslContextBuilder {
     }
 
     /// Sets the context's supported 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"))]
     #[corresponds(SSL_CTX_set1_curves_list)]
     pub fn set_curves_list(&mut self, curves: &str) -> Result<(), ErrorStack> {
         let curves = CString::new(curves).map_err(ErrorStack::internal_error)?;
@@ -2035,12 +2026,7 @@ impl SslContextBuilder {
     }
 
     /// Sets the context's supported 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.
     #[corresponds(SSL_CTX_set1_curves)]
-    #[cfg(not(feature = "kx-safe-default"))]
     pub fn set_curves(&mut self, curves: &[SslCurve]) -> Result<(), ErrorStack> {
         let curves: Vec<i32> = curves
             .iter()
@@ -2915,40 +2901,6 @@ impl SslRef {
         }
     }
 
-    #[cfg(feature = "kx-safe-default")]
-    fn client_set_default_curves_list(&mut self) {
-        let curves = if cfg!(feature = "kx-client-pq-preferred") {
-            if cfg!(feature = "kx-client-nist-required") {
-                "P256Kyber768Draft00:P-256:P-384:P-521"
-            } else {
-                "X25519MLKEM768:X25519Kyber768Draft00:X25519:P256Kyber768Draft00:P-256:P-384:P-521"
-            }
-        } else if cfg!(feature = "kx-client-pq-supported") {
-            if cfg!(feature = "kx-client-nist-required") {
-                "P-256:P-384:P-521:P256Kyber768Draft00"
-            } else {
-                "X25519:P-256:P-384:P-521:X25519MLKEM768:X25519Kyber768Draft00:P256Kyber768Draft00"
-            }
-        } else {
-            if cfg!(feature = "kx-client-nist-required") {
-                "P-256:P-384:P-521"
-            } else {
-                "X25519:P-256:P-384:P-521"
-            }
-        };
-
-        self.set_curves_list(curves)
-            .expect("invalid default client curves list");
-    }
-
-    #[cfg(feature = "kx-safe-default")]
-    fn server_set_default_curves_list(&mut self) {
-        self.set_curves_list(
-            "X25519MLKEM768:X25519Kyber768Draft00:P256Kyber768Draft00:X25519:P-256:P-384",
-        )
-        .expect("invalid default server curves list");
-    }
-
     /// Returns the [`SslCurve`] used for this `SslRef`.
     #[corresponds(SSL_get_curve_id)]
     #[must_use]
@@ -4341,9 +4293,6 @@ where
     pub fn setup_connect(mut self) -> MidHandshakeSslStream<S> {
         self.set_connect_state();
 
-        #[cfg(feature = "kx-safe-default")]
-        self.inner.ssl.client_set_default_curves_list();
-
         MidHandshakeSslStream {
             stream: self.inner,
             error: Error {
@@ -4373,9 +4322,6 @@ where
     pub fn setup_accept(mut self) -> MidHandshakeSslStream<S> {
         self.set_accept_state();
 
-        #[cfg(feature = "kx-safe-default")]
-        self.inner.ssl.server_set_default_curves_list();
-
         MidHandshakeSslStream {
             stream: self.inner,
             error: Error {

boring/src/ssl/test/mod.rs

@@ -952,30 +952,6 @@ fn sni_callback_swapped_ctx() {
     assert!(CALLED_BACK.load(Ordering::SeqCst));
 }
 
-#[cfg(feature = "kx-safe-default")]
-#[test]
-fn client_set_default_curves_list() {
-    let ssl_ctx = crate::ssl::SslContextBuilder::new(SslMethod::tls())
-        .unwrap()
-        .build();
-    let mut ssl = Ssl::new(&ssl_ctx).unwrap();
-
-    // Panics if Kyber768 missing in boringSSL.
-    ssl.client_set_default_curves_list();
-}
-
-#[cfg(feature = "kx-safe-default")]
-#[test]
-fn server_set_default_curves_list() {
-    let ssl_ctx = crate::ssl::SslContextBuilder::new(SslMethod::tls())
-        .unwrap()
-        .build();
-    let mut ssl = Ssl::new(&ssl_ctx).unwrap();
-
-    // Panics if Kyber768 missing in boringSSL.
-    ssl.server_set_default_curves_list();
-}
-
 #[test]
 fn get_curve() {
     let server = Server::builder().build();
@@ -994,7 +970,6 @@ fn get_curve_name() {
     assert_eq!(SslCurve::X25519.name(), Some("X25519"));
 }
 
-#[cfg(not(feature = "kx-safe-default"))]
 #[test]
 fn set_curves() {
     let mut ctx = SslContext::builder(SslMethod::tls()).unwrap();