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.

Author: cjpatton

.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

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",

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.

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());
 }

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;

boring/src/ssl/mod.rs

@@ -104,15 +104,15 @@ 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};
 
 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(|_| ()) }

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)