From 721b6fca2ed9c50eefe6f8cdab2de5b49ba2fb6a Mon Sep 17 00:00:00 2001 From: Rushil Mehra <84047965+rushilmehra@users.noreply.github.com> Date: Mon, 31 Mar 2025 12:34:29 -0700 Subject: [PATCH] Add fips-precompiled feature to support newer versions of FIPS (#338) Newer versions of FIPS don't need any special casing in our bindings, unlike the submoduled boringssl-fips. In addition, many users currently use FIPS by precompiling BoringSSL with the proper build tools and passing that in to the bindings. Until we adopt the Update Stream pattern for FIPS, there are two main use cases: 1. Passing an unmodified, precompiled FIPS validated version of boringssl (fips-precompiled) 2. Passing a custom source directory of boringssl meant to be linked with a FIPS validated bcm.o. This is mainly useful if you carry custom patches but still want to use a FIPS validated BoringCrypto. (fips-link-precompiled) This commit introduces the `fips-precompiled` feature and removes the `fips-no-compat` feature. --- boring-sys/Cargo.toml | 12 +++++++++- boring-sys/build/config.rs | 24 +++++++++++++++---- boring-sys/build/main.rs | 5 ++-- boring/Cargo.toml | 31 +++++++++++++------------ boring/src/fips.rs | 4 ++-- boring/src/lib.rs | 2 +- boring/src/ssl/mod.rs | 47 +++++++++++++++++++++++++++----------- boring/src/ssl/test/mod.rs | 6 ++--- hyper-boring/Cargo.toml | 2 +- tokio-boring/Cargo.toml | 2 +- 10 files changed, 92 insertions(+), 43 deletions(-) diff --git a/boring-sys/Cargo.toml b/boring-sys/Cargo.toml index c0a45aec..5cd8e214 100644 --- a/boring-sys/Cargo.toml +++ b/boring-sys/Cargo.toml @@ -57,9 +57,19 @@ features = ["rpk", "pq-experimental", "underscore-wildcards"] rustdoc-args = ["--cfg", "docsrs"] [features] -# Use a FIPS-validated version of boringssl. +# Compile boringssl using the FIPS build flag if building boringssl from +# scratch. +# +# See +# https://boringssl.googlesource.com/boringssl/+/master/crypto/fipsmodule/FIPS.md +# for instructions and more details on the boringssl FIPS flag. fips = [] +# Use a precompiled FIPS-validated version of BoringSSL. Meant to be used with +# FIPS-20230428 or newer. Users must set `BORING_BSSL_FIPS_PATH` to use this +# feature, or else the build will fail. +fips-precompiled = [] + # Link with precompiled FIPS-validated `bcm.o` module. fips-link-precompiled = [] diff --git a/boring-sys/build/config.rs b/boring-sys/build/config.rs index 19a63f52..353f4d20 100644 --- a/boring-sys/build/config.rs +++ b/boring-sys/build/config.rs @@ -16,6 +16,7 @@ pub(crate) struct Config { pub(crate) struct Features { pub(crate) fips: bool, + pub(crate) fips_precompiled: bool, pub(crate) fips_link_precompiled: bool, pub(crate) pq_experimental: bool, pub(crate) rpk: bool, @@ -47,11 +48,7 @@ impl Config { let target_os = env::var("CARGO_CFG_TARGET_OS").unwrap(); let features = Features::from_env(); - let env = Env::from_env( - &host, - &target, - features.fips || features.fips_link_precompiled, - ); + let env = Env::from_env(&host, &target, features.is_fips_like()); let mut is_bazel = false; if let Some(src_path) = &env.source_path { @@ -80,6 +77,10 @@ impl Config { panic!("`fips` and `rpk` features are mutually exclusive"); } + if self.features.fips_precompiled && self.features.rpk { + panic!("`fips-precompiled` and `rpk` features are mutually exclusive"); + } + let is_precompiled_native_lib = self.env.path.is_some(); let is_external_native_lib_source = !is_precompiled_native_lib && self.env.source_path.is_none(); @@ -103,15 +104,22 @@ impl Config { ); } + // todo(rmehra): should this even be a restriction? why not let people link a custom bcm.o? + // precompiled boringssl will include libcrypto.a if is_precompiled_native_lib && self.features.fips_link_precompiled { panic!("precompiled BoringSSL was provided, so FIPS configuration can't be applied"); } + + if !is_precompiled_native_lib && self.features.fips_precompiled { + panic!("`fips-precompiled` feature requires `BORING_BSSL_FIPS_PATH` to be set"); + } } } impl Features { fn from_env() -> Self { let fips = env::var_os("CARGO_FEATURE_FIPS").is_some(); + let fips_precompiled = env::var_os("CARGO_FEATURE_FIPS_PRECOMPILED").is_some(); let fips_link_precompiled = env::var_os("CARGO_FEATURE_FIPS_LINK_PRECOMPILED").is_some(); let pq_experimental = env::var_os("CARGO_FEATURE_PQ_EXPERIMENTAL").is_some(); let rpk = env::var_os("CARGO_FEATURE_RPK").is_some(); @@ -119,12 +127,17 @@ impl Features { Self { fips, + fips_precompiled, fips_link_precompiled, pq_experimental, rpk, underscore_wildcards, } } + + pub(crate) fn is_fips_like(&self) -> bool { + self.fips || self.fips_precompiled || self.fips_link_precompiled + } } impl Env { @@ -138,6 +151,7 @@ impl Env { let target_var = |name: &str| { let kind = if host == target { "HOST" } else { "TARGET" }; + // TODO(rmehra): look for just `name` first, as most people just set that var(&format!("{}_{}", name, target)) .or_else(|| var(&format!("{}_{}", name, target_with_underscores))) .or_else(|| var(&format!("{}_{}", kind, name))) diff --git a/boring-sys/build/main.rs b/boring-sys/build/main.rs index 06756a7d..0f0b94c1 100644 --- a/boring-sys/build/main.rs +++ b/boring-sys/build/main.rs @@ -662,13 +662,14 @@ fn main() { let bssl_dir = built_boring_source_path(&config); let build_path = get_boringssl_platform_output_path(&config); - if config.is_bazel || (config.features.fips && config.env.path.is_some()) { + if config.is_bazel || (config.features.is_fips_like() && config.env.path.is_some()) { println!( "cargo:rustc-link-search=native={}/lib/{}", bssl_dir.display(), build_path ); } else { + // todo(rmehra): clean this up, I think these are pretty redundant println!( "cargo:rustc-link-search=native={}/build/crypto/{}", bssl_dir.display(), @@ -760,7 +761,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..f9a3527b 100644 --- a/boring/Cargo.toml +++ b/boring/Cargo.toml @@ -19,24 +19,27 @@ rustdoc-args = ["--cfg", "docsrs"] [features] # Controlling the build -# Use a FIPS-validated version of BoringSSL. This feature sets "fips-compat". +# NOTE: This feature is deprecated. It is needed for the submoduled +# boringssl-fips, which is extremely old and requires modifications to the +# bindings, as some newer APIs don't exist and some function signatures have +# changed. It is highly recommended to use `fips-precompiled` instead. +# +# This feature sets `fips-compat` on behalf of the user to guarantee bindings +# compatibility with the submoduled boringssl-fips. +# +# Use a FIPS-validated version of BoringSSL. fips = ["fips-compat", "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 the submoduled boringssl-fips, without enabling +# the `fips` feature itself (useful e.g. if `fips-link-precompiled` is used +# with an older BoringSSL version). fips-compat = [] +# Use a precompiled FIPS-validated version of BoringSSL. Meant to be used with +# FIPS-20230428 or newer. Users must set `BORING_BSSL_FIPS_PATH` to use this +# feature, or else the build will fail. +fips-precompiled = ["boring-sys/fips-precompiled"] + # Link with precompiled FIPS-validated `bcm.o` module. fips-link-precompiled = ["boring-sys/fips-link-precompiled"] diff --git a/boring/src/fips.rs b/boring/src/fips.rs index 29dbfae0..23fbefdf 100644 --- a/boring/src/fips.rs +++ b/boring/src/fips.rs @@ -16,13 +16,13 @@ pub fn enabled() -> bool { fn is_enabled() { #[cfg(any( feature = "fips", - feature = "fips-no-compat", + feature = "fips-precompiled", feature = "fips-link-precompiled" ))] assert!(enabled()); #[cfg(not(any( feature = "fips", - feature = "fips-no-compat", + feature = "fips-precompiled", feature = "fips-link-precompiled" )))] assert!(!enabled()); diff --git a/boring/src/lib.rs b/boring/src/lib.rs index a54e2bfa..e7b03ddb 100644 --- a/boring/src/lib.rs +++ b/boring/src/lib.rs @@ -135,7 +135,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 3eaa2e83..66a11fbc 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,19 +714,28 @@ impl SslCurve { pub const X25519: SslCurve = SslCurve(ffi::SSL_CURVE_X25519 as _); - #[cfg(not(any(feature = "fips", feature = "fips-no-compat")))] + #[cfg(not(any(feature = "fips", feature = "fips-precompiled")))] pub const X25519_KYBER768_DRAFT00: SslCurve = SslCurve(ffi::SSL_CURVE_X25519_KYBER768_DRAFT00 as _); - #[cfg(all(not(feature = "fips"), feature = "pq-experimental"))] + #[cfg(all( + not(any(feature = "fips", feature = "fips-precompiled")), + feature = "pq-experimental" + ))] pub const X25519_KYBER768_DRAFT00_OLD: SslCurve = SslCurve(ffi::SSL_CURVE_X25519_KYBER768_DRAFT00_OLD as _); - #[cfg(all(not(feature = "fips"), feature = "pq-experimental"))] + #[cfg(all( + not(any(feature = "fips", feature = "fips-precompiled")), + feature = "pq-experimental" + ))] pub const X25519_KYBER512_DRAFT00: SslCurve = SslCurve(ffi::SSL_CURVE_X25519_KYBER512_DRAFT00 as _); - #[cfg(all(not(feature = "fips"), feature = "pq-experimental"))] + #[cfg(all( + not(any(feature = "fips", feature = "fips-precompiled")), + feature = "pq-experimental" + ))] pub const P256_KYBER768_DRAFT00: SslCurve = SslCurve(ffi::SSL_CURVE_P256_KYBER768_DRAFT00 as _); /// Returns the curve name @@ -759,15 +768,27 @@ 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(any(feature = "fips", feature = "fips-precompiled")))] ffi::SSL_CURVE_X25519_KYBER768_DRAFT00 => Some(ffi::NID_X25519Kyber768Draft00), - #[cfg(all(not(feature = "fips"), feature = "pq-experimental"))] + #[cfg(all( + not(any(feature = "fips", feature = "fips-precompiled")), + feature = "pq-experimental" + ))] ffi::SSL_CURVE_X25519_KYBER768_DRAFT00_OLD => Some(ffi::NID_X25519Kyber768Draft00Old), - #[cfg(all(not(feature = "fips"), feature = "pq-experimental"))] + #[cfg(all( + not(any(feature = "fips", feature = "fips-precompiled")), + feature = "pq-experimental" + ))] ffi::SSL_CURVE_X25519_KYBER512_DRAFT00 => Some(ffi::NID_X25519Kyber512Draft00), - #[cfg(all(not(feature = "fips"), feature = "pq-experimental"))] + #[cfg(all( + not(any(feature = "fips", feature = "fips-precompiled")), + feature = "pq-experimental" + ))] ffi::SSL_CURVE_P256_KYBER768_DRAFT00 => Some(ffi::NID_P256Kyber768Draft00), - #[cfg(all(not(feature = "fips"), feature = "pq-experimental"))] + #[cfg(all( + not(any(feature = "fips", feature = "fips-precompiled")), + feature = "pq-experimental" + ))] ffi::SSL_CURVE_X25519_MLKEM768 => Some(ffi::NID_X25519MLKEM768), _ => None, } @@ -2010,7 +2031,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 +2288,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..91f74424 100644 --- a/hyper-boring/Cargo.toml +++ b/hyper-boring/Cargo.toml @@ -31,7 +31,7 @@ fips = ["tokio-boring/fips"] # # TODO(cjpatton) Delete this feature and modify "fips" so that it doesn't imply # "fips-compat". -fips-no-compat = ["tokio-boring/fips-no-compat"] +fips-precompiled = ["tokio-boring/fips-precompiled"] # 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..75c64129 100644 --- a/tokio-boring/Cargo.toml +++ b/tokio-boring/Cargo.toml @@ -27,7 +27,7 @@ fips = ["boring/fips", "boring-sys/fips"] # # TODO(cjpatton) Delete this feature and modify "fips" so that it doesn't imply # "fips-compat". -fips-no-compat = ["boring/fips-no-compat"] +fips-precompiled = ["boring/fips-precompiled"] # Link with precompiled FIPS-validated `bcm.o` module. fips-link-precompiled = ["boring/fips-link-precompiled", "boring-sys/fips-link-precompiled"]