From 57307d739e0699454effccc0dd3015287fab398e Mon Sep 17 00:00:00 2001 From: Christopher Patton Date: Thu, 13 Mar 2025 09:47:56 -0700 Subject: [PATCH] Remove "fips-no-compat", decouple "fips-compat" from "fips" Modify the "fips" feature so that it no longer implies "fips-compat". The latter is no longer needed for recent builds of boringSSL; users who need older builds will need to enable "fips-compat" explicitly. Also, remove the "fipps-no-compat" feature, as it's now equivalent to "fips". --- .github/workflows/ci.yml | 4 ++-- boring-sys/build/main.rs | 2 +- boring/Cargo.toml | 21 ++++++--------------- boring/src/fips.rs | 12 ++---------- boring/src/lib.rs | 2 +- boring/src/ssl/mod.rs | 12 ++++++------ boring/src/ssl/test/mod.rs | 6 +++--- hyper-boring/Cargo.toml | 10 ---------- tokio-boring/Cargo.toml | 10 ---------- 9 files changed, 21 insertions(+), 58 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0910a482..f798c632 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -263,7 +263,7 @@ jobs: working-directory: ${{ runner.temp }}/llvm/bin run: ln -s clang clang++-12 - name: Run tests - run: cargo test --features fips + run: cargo test --features fips,fips-compat - name: Test boring-sys cargo publish (FIPS) # Running `cargo publish --dry-run` tests two things: # @@ -338,7 +338,7 @@ jobs: - name: Set CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_LINKER run: echo "CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_LINKER=${{ matrix.target }}-gcc" >> $GITHUB_ENV - name: Build for ${{ matrix.target }} - run: cargo build --target ${{ matrix.target }} --all-targets --features fips + run: cargo build --target ${{ matrix.target }} --all-targets --features fips,fips-compat test-features: name: Test features diff --git a/boring-sys/build/main.rs b/boring-sys/build/main.rs index 06756a7d..5af2df76 100644 --- a/boring-sys/build/main.rs +++ b/boring-sys/build/main.rs @@ -760,7 +760,7 @@ fn main() { "des.h", "dtls1.h", "hkdf.h", - #[cfg(not(any(feature = "fips", feature = "fips-no-compat")))] + #[cfg(not(feature = "fips"))] "hpke.h", "hmac.h", "hrss.h", diff --git a/boring/Cargo.toml b/boring/Cargo.toml index 491b1702..52de7814 100644 --- a/boring/Cargo.toml +++ b/boring/Cargo.toml @@ -19,22 +19,13 @@ rustdoc-args = ["--cfg", "docsrs"] [features] # Controlling the build -# Use a FIPS-validated version of BoringSSL. This feature sets "fips-compat". -fips = ["fips-compat", "boring-sys/fips"] +# Use a FIPS-validated version of BoringSSL. Note that depending on how old the +# version you're using, is, you may also need `fips-compat`. +fips = ["boring-sys/fips"] -# Use a FIPS build of BoringSSL, but don't set "fips-compat". -# -# As of boringSSL commit a430310d6563c0734ddafca7731570dfb683dc19, we no longer -# need to make exceptions for the types of BufLen, ProtosLen, and ValueLen, -# which means the "fips-compat" feature is no longer needed. -# -# TODO(cjpatton) Delete this feature and modify "fips" so that it doesn't imply -# "fips-compat". -fips-no-compat = ["boring-sys/fips"] - -# Build with compatibility for the BoringSSL FIPS version, without enabling the -# `fips` feature itself (useful e.g. if `fips-link-precompiled` is used with an -# older BoringSSL version). +# Build with compatibility for older versions of boringSSL, primarily +# fips-20220613. This feature doesn't enable `fips` itself, which is useful if, +# e.g., you use `fips-link-precompiled`. fips-compat = [] # Link with precompiled FIPS-validated `bcm.o` module. diff --git a/boring/src/fips.rs b/boring/src/fips.rs index 29dbfae0..de28f260 100644 --- a/boring/src/fips.rs +++ b/boring/src/fips.rs @@ -14,16 +14,8 @@ pub fn enabled() -> bool { #[test] fn is_enabled() { - #[cfg(any( - feature = "fips", - feature = "fips-no-compat", - feature = "fips-link-precompiled" - ))] + #[cfg(any(feature = "fips", feature = "fips-link-precompiled"))] assert!(enabled()); - #[cfg(not(any( - feature = "fips", - feature = "fips-no-compat", - feature = "fips-link-precompiled" - )))] + #[cfg(not(any(feature = "fips", feature = "fips-link-precompiled")))] assert!(!enabled()); } diff --git a/boring/src/lib.rs b/boring/src/lib.rs index 7c870c5d..1b23edac 100644 --- a/boring/src/lib.rs +++ b/boring/src/lib.rs @@ -128,7 +128,7 @@ pub mod error; pub mod ex_data; pub mod fips; pub mod hash; -#[cfg(not(any(feature = "fips", feature = "fips-no-compat")))] +#[cfg(not(feature = "fips"))] pub mod hpke; pub mod memcmp; pub mod nid; diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index 250c4d08..c4979f97 100644 --- a/boring/src/ssl/mod.rs +++ b/boring/src/ssl/mod.rs @@ -104,7 +104,7 @@ pub use self::async_callbacks::{ pub use self::connector::{ ConnectConfiguration, SslAcceptor, SslAcceptorBuilder, SslConnector, SslConnectorBuilder, }; -#[cfg(not(any(feature = "fips", feature = "fips-no-compat")))] +#[cfg(not(feature = "fips"))] pub use self::ech::{SslEchKeys, SslEchKeysRef}; pub use self::error::{Error, ErrorCode, HandshakeError}; @@ -112,7 +112,7 @@ mod async_callbacks; mod bio; mod callbacks; mod connector; -#[cfg(not(any(feature = "fips", feature = "fips-no-compat")))] +#[cfg(not(feature = "fips"))] mod ech; mod error; mod mut_only; @@ -714,7 +714,7 @@ impl SslCurve { pub const X25519: SslCurve = SslCurve(ffi::SSL_CURVE_X25519 as _); - #[cfg(not(any(feature = "fips", feature = "fips-no-compat")))] + #[cfg(not(feature = "fips"))] pub const X25519_KYBER768_DRAFT00: SslCurve = SslCurve(ffi::SSL_CURVE_X25519_KYBER768_DRAFT00 as _); @@ -759,7 +759,7 @@ impl SslCurve { ffi::SSL_CURVE_SECP384R1 => Some(ffi::NID_secp384r1), ffi::SSL_CURVE_SECP521R1 => Some(ffi::NID_secp521r1), ffi::SSL_CURVE_X25519 => Some(ffi::NID_X25519), - #[cfg(not(any(feature = "fips", feature = "fips-no-compat")))] + #[cfg(not(feature = "fips"))] ffi::SSL_CURVE_X25519_KYBER768_DRAFT00 => Some(ffi::NID_X25519Kyber768Draft00), #[cfg(feature = "pq-experimental")] ffi::SSL_CURVE_X25519_KYBER768_DRAFT00_OLD => Some(ffi::NID_X25519Kyber768Draft00Old), @@ -2010,7 +2010,7 @@ impl SslContextBuilder { /// ECHConfigs to allow stale DNS caches to update. Unlike most `SSL_CTX` APIs, this function /// is safe to call even after the `SSL_CTX` has been associated with connections on various /// threads. - #[cfg(not(any(feature = "fips", feature = "fips-no-compat")))] + #[cfg(not(feature = "fips"))] #[corresponds(SSL_CTX_set1_ech_keys)] pub fn set_ech_keys(&self, keys: &SslEchKeys) -> Result<(), ErrorStack> { unsafe { cvt(ffi::SSL_CTX_set1_ech_keys(self.as_ptr(), keys.as_ptr())).map(|_| ()) } @@ -2267,7 +2267,7 @@ impl SslContextRef { /// ECHConfigs to allow stale DNS caches to update. Unlike most `SSL_CTX` APIs, this function /// is safe to call even after the `SSL_CTX` has been associated with connections on various /// threads. - #[cfg(not(any(feature = "fips", feature = "fips-no-compat")))] + #[cfg(not(feature = "fips"))] #[corresponds(SSL_CTX_set1_ech_keys)] pub fn set_ech_keys(&self, keys: &SslEchKeys) -> Result<(), ErrorStack> { unsafe { cvt(ffi::SSL_CTX_set1_ech_keys(self.as_ptr(), keys.as_ptr())).map(|_| ()) } diff --git a/boring/src/ssl/test/mod.rs b/boring/src/ssl/test/mod.rs index 69d0dbf6..4566b73c 100644 --- a/boring/src/ssl/test/mod.rs +++ b/boring/src/ssl/test/mod.rs @@ -21,13 +21,13 @@ use crate::ssl::{ use crate::x509::verify::X509CheckFlags; use crate::x509::{X509Name, X509}; -#[cfg(not(any(feature = "fips", feature = "fips-no-compat")))] +#[cfg(not(feature = "fips"))] use super::CompliancePolicy; mod cert_compressor; mod cert_verify; mod custom_verify; -#[cfg(not(any(feature = "fips", feature = "fips-no-compat")))] +#[cfg(not(feature = "fips"))] mod ech; mod private_key_method; mod server; @@ -990,7 +990,7 @@ fn test_get_ciphers() { } #[test] -#[cfg(not(any(feature = "fips", feature = "fips-no-compat")))] +#[cfg(not(feature = "fips"))] fn test_set_compliance() { let mut ctx = SslContext::builder(SslMethod::tls()).unwrap(); ctx.set_compliance_policy(CompliancePolicy::FIPS_202205) diff --git a/hyper-boring/Cargo.toml b/hyper-boring/Cargo.toml index 7c9921f2..a6ed3180 100644 --- a/hyper-boring/Cargo.toml +++ b/hyper-boring/Cargo.toml @@ -23,16 +23,6 @@ runtime = ["hyper_old/runtime"] # Use a FIPS-validated version of boringssl. fips = ["tokio-boring/fips"] -# Use a FIPS build of BoringSSL, but don't set "fips-compat". -# -# As of boringSSL commit a430310d6563c0734ddafca7731570dfb683dc19, we no longer -# need to make exceptions for the types of BufLen, ProtosLen, and ValueLen, -# which means the "fips-compat" feature is no longer needed. -# -# TODO(cjpatton) Delete this feature and modify "fips" so that it doesn't imply -# "fips-compat". -fips-no-compat = ["tokio-boring/fips-no-compat"] - # Link with precompiled FIPS-validated `bcm.o` module. fips-link-precompiled = ["tokio-boring/fips-link-precompiled"] diff --git a/tokio-boring/Cargo.toml b/tokio-boring/Cargo.toml index 2aed8fb5..8a475515 100644 --- a/tokio-boring/Cargo.toml +++ b/tokio-boring/Cargo.toml @@ -19,16 +19,6 @@ rustdoc-args = ["--cfg", "docsrs"] # Use a FIPS-validated version of boringssl. fips = ["boring/fips", "boring-sys/fips"] -# Use a FIPS build of BoringSSL, but don't set "fips-compat". -# -# As of boringSSL commit a430310d6563c0734ddafca7731570dfb683dc19, we no longer -# need to make exceptions for the types of BufLen, ProtosLen, and ValueLen, -# which means the "fips-compat" feature is no longer needed. -# -# TODO(cjpatton) Delete this feature and modify "fips" so that it doesn't imply -# "fips-compat". -fips-no-compat = ["boring/fips-no-compat"] - # Link with precompiled FIPS-validated `bcm.o` module. fips-link-precompiled = ["boring/fips-link-precompiled", "boring-sys/fips-link-precompiled"]