diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8ee8b46c..46e621e7 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/boring/Cargo.toml b/boring/Cargo.toml index caac99f2..bc9dba22 100644 --- a/boring/Cargo.toml +++ b/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 } diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index 49788c3e..302f99ba 100644 --- a/boring/src/ssl/mod.rs +++ b/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 { 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 = 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 { 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 { self.set_accept_state(); - #[cfg(feature = "kx-safe-default")] - self.inner.ssl.server_set_default_curves_list(); - MidHandshakeSslStream { stream: self.inner, error: Error { diff --git a/boring/src/ssl/test/mod.rs b/boring/src/ssl/test/mod.rs index 6ac6ca75..0a4f6243 100644 --- a/boring/src/ssl/test/mod.rs +++ b/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();