diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 083ee77c..32aa3afe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -313,6 +313,8 @@ jobs: uses: actions/setup-go@v5 with: go-version: '>=1.22.0' + - name: Install ${{ matrix.target }} toolchain + run: brew tap messense/macos-cross-toolchains && brew install ${{ matrix.target }} && brew link x86_64-unknown-linux-gnu - name: Install Clang-12 uses: KyleMayes/install-llvm-action@v1 with: @@ -320,10 +322,7 @@ jobs: directory: ${{ runner.temp }}/llvm - name: Add clang++-12 link working-directory: ${{ runner.temp }}/llvm/bin - run: ln -s clang clang++-12 - - name: Install ${{ matrix.target }} toolchain - # TODO(rmehra): find a better way to overwrite the python3 version without specifying version - run: brew tap messense/macos-cross-toolchains && brew install --overwrite python@3.12 && brew install ${{ matrix.target }} + run: ln -s clang++ clang++-12 - name: Set BORING_BSSL_FIPS_COMPILER_EXTERNAL_TOOLCHAIN run: echo "BORING_BSSL_FIPS_COMPILER_EXTERNAL_TOOLCHAIN=$(brew --prefix ${{ matrix.target }})/toolchain" >> $GITHUB_ENV shell: bash diff --git a/.github/workflows/semgrep.yml b/.github/workflows/semgrep.yml new file mode 100644 index 00000000..d98c3fbd --- /dev/null +++ b/.github/workflows/semgrep.yml @@ -0,0 +1,23 @@ +on: + pull_request: {} + workflow_dispatch: {} + push: + branches: + - master + schedule: + - cron: "0 0 * * *" +name: Semgrep config +jobs: + semgrep: + name: semgrep/ci + runs-on: ubuntu-latest + env: + SEMGREP_APP_TOKEN: ${{ secrets.SEMGREP_APP_TOKEN }} + SEMGREP_URL: https://cloudflare.semgrep.dev + SEMGREP_APP_URL: https://cloudflare.semgrep.dev + SEMGREP_VERSION_CHECK_URL: https://cloudflare.semgrep.dev/api/check-version + container: + image: semgrep/semgrep + steps: + - uses: actions/checkout@v4 + - run: semgrep ci diff --git a/Cargo.toml b/Cargo.toml index d0c7b420..1262fef6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ members = [ resolver = "2" [workspace.package] -version = "4.10.2" +version = "4.11.0" repository = "https://github.com/cloudflare/boring" edition = "2021" @@ -19,9 +19,9 @@ tag-prefix = "" publish = false [workspace.dependencies] -boring-sys = { package = "rboring-sys", version = "4.10.2", path = "./boring-sys" } -boring = { package = "rboring", version = "4.10.2", path = "./boring" } -tokio-boring = { package = "tokio-rboring", version = "4.10.2", path = "./tokio-boring" } +boring-sys = { package = "rboring-sys", version = "4.11.0", path = "./boring-sys" } +boring = { package = "rboring", version = "4.11.0", path = "./boring" } +tokio-boring = { package = "tokio-rboring", version = "4.11.0", path = "./tokio-boring" } bindgen = { version = "0.70.1", default-features = false, features = ["runtime"] } bytes = "1" @@ -49,3 +49,4 @@ openssl-macros = "0.1.1" tower = "0.4" tower-layer = "0.3" tower-service = "0.3" +autocfg = "1.3.0" diff --git a/RELEASE_NOTES b/RELEASE_NOTES index f1531f74..c205d73b 100644 --- a/RELEASE_NOTES +++ b/RELEASE_NOTES @@ -1,3 +1,11 @@ +4.11.0 +- 2024-10-17 boring-sys: include HPKE header file for bindgen +- 2024-10-17 Add "fips-compat" feature +- 2024-09-25 Create semgrep.yml + +4.10.3 +- 2024-09-21 Set MSRV to 1.70 (#279) + 4.10.2 - 2024-09-18 boring-pq.patch Fix by not updating crypto_test_data.cc diff --git a/boring-sys/Cargo.toml b/boring-sys/Cargo.toml index 337019a4..ddb405bb 100644 --- a/boring-sys/Cargo.toml +++ b/boring-sys/Cargo.toml @@ -77,6 +77,7 @@ pq-experimental = [] underscore-wildcards = [] [build-dependencies] +autocfg = { workspace = true } bindgen = { workspace = true } cmake = { workspace = true } fs_extra = { workspace = true } diff --git a/boring-sys/build/main.rs b/boring-sys/build/main.rs index c0b6f36c..0d7b6c74 100644 --- a/boring-sys/build/main.rs +++ b/boring-sys/build/main.rs @@ -690,6 +690,10 @@ fn main() { } }); + // bindgen 0.70 replaced the run-time layout tests with compile-time ones, + // but they depend on std::mem::offset_of, stabilized in 1.77. + let supports_layout_tests = autocfg::new().probe_rustc_version(1, 77); + let mut builder = bindgen::Builder::default() .rust_target(bindgen::RustTarget::Stable_1_68) // bindgen MSRV is 1.70, so this is enough .derive_copy(true) @@ -704,7 +708,7 @@ fn main() { .generate_comments(true) .fit_macro_constants(false) .size_t_is_usize(true) - .layout_tests(true) + .layout_tests(supports_layout_tests) .prepend_enum_name(true) .blocklist_type("max_align_t") // Not supported by bindgen on all targets, not used by BoringSSL .clang_args(get_extra_clang_args_for_bindgen(&config)) @@ -731,6 +735,8 @@ fn main() { "des.h", "dtls1.h", "hkdf.h", + #[cfg(not(feature = "fips"))] + "hpke.h", "hmac.h", "hrss.h", "md4.h", diff --git a/boring/Cargo.toml b/boring/Cargo.toml index f92473e6..3286b173 100644 --- a/boring/Cargo.toml +++ b/boring/Cargo.toml @@ -19,7 +19,12 @@ rustdoc-args = ["--cfg", "docsrs"] # Controlling the build # Use a FIPS-validated version of boringssl. -fips = ["boring-sys/fips"] +fips = ["fips-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). +fips-compat = [] # Link with precompiled FIPS-validated `bcm.o` module. fips-link-precompiled = ["boring-sys/fips-link-precompiled"] diff --git a/boring/src/bio.rs b/boring/src/bio.rs index b6d52e09..a0c882a2 100644 --- a/boring/src/bio.rs +++ b/boring/src/bio.rs @@ -19,9 +19,9 @@ impl<'a> Drop for MemBioSlice<'a> { impl<'a> MemBioSlice<'a> { pub fn new(buf: &'a [u8]) -> Result, ErrorStack> { - #[cfg(not(feature = "fips"))] + #[cfg(not(feature = "fips-compat"))] type BufLen = isize; - #[cfg(feature = "fips")] + #[cfg(feature = "fips-compat")] type BufLen = libc::c_int; ffi::init(); diff --git a/boring/src/ssl/callbacks.rs b/boring/src/ssl/callbacks.rs index f592d9d2..f41108d5 100644 --- a/boring/src/ssl/callbacks.rs +++ b/boring/src/ssl/callbacks.rs @@ -64,6 +64,34 @@ where unsafe { raw_custom_verify_callback(ssl, out_alert, callback) } } +pub(super) unsafe extern "C" fn raw_cert_verify( + x509_ctx: *mut ffi::X509_STORE_CTX, + _arg: *mut c_void, +) -> c_int +where + F: Fn(&mut X509StoreContextRef) -> bool + 'static + Sync + Send, +{ + // SAFETY: boring provides valid inputs. + let ctx = unsafe { X509StoreContextRef::from_ptr_mut(x509_ctx) }; + + let ssl_idx = X509StoreContext::ssl_idx().expect("BUG: store context ssl index missing"); + let verify_idx = SslContext::cached_ex_index::(); + + let verify = ctx + .ex_data(ssl_idx) + .expect("BUG: store context missing ssl") + .ssl_context() + .ex_data(verify_idx) + .expect("BUG: verify callback missing"); + + // SAFETY: The callback won't outlive the context it's associated with + // because there is no way to get a mutable reference to the `SslContext`, + // so the callback can't replace itself. + let verify = unsafe { &*(verify as *const F) }; + + verify(ctx) as c_int +} + pub(super) unsafe extern "C" fn ssl_raw_custom_verify( ssl: *mut ffi::SSL, out_alert: *mut u8, diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index cd8a59eb..9ca8f234 100644 --- a/boring/src/ssl/mod.rs +++ b/boring/src/ssl/mod.rs @@ -777,10 +777,10 @@ impl SslCurve { /// A compliance policy. #[derive(Debug, Copy, Clone, PartialEq, Eq)] -#[cfg(not(feature = "fips"))] +#[cfg(not(feature = "fips-compat"))] pub struct CompliancePolicy(ffi::ssl_compliance_policy_t); -#[cfg(not(feature = "fips"))] +#[cfg(not(feature = "fips-compat"))] impl CompliancePolicy { /// Does nothing, however setting this does not undo other policies, so trying to set this is an error. pub const NONE: Self = Self(ffi::ssl_compliance_policy_t::ssl_compliance_policy_none); @@ -1003,6 +1003,49 @@ impl SslContextBuilder { self.ctx.as_ptr() } + /// Registers a certificate verification callback that replaces the default verification + /// process. + /// + /// The callback returns true if the certificate chain is valid, and false if not. + /// A viable verification result value (either `Ok(())` or an `Err(X509VerifyError)`) must be + /// reflected in the error member of `X509StoreContextRef`, which can be done by calling + /// `X509StoreContextRef::set_error`. However, the callback's return value determines + /// whether the chain is accepted or not. + /// + /// *Warning*: Providing a complete verification procedure is a complex task. See + /// https://docs.openssl.org/master/man3/SSL_CTX_set_cert_verify_callback/#notes for more + /// information. + /// + /// TODO: Add the ability to unset the callback by either adding a new function or wrapping the + /// callback in an `Option`. + /// + /// # Panics + /// + /// This method panics if this `SslContext` is associated with a RPK context. + #[corresponds(SSL_CTX_set_cert_verify_callback)] + pub fn set_cert_verify_callback(&mut self, callback: F) + where + F: Fn(&mut X509StoreContextRef) -> bool + 'static + Sync + Send, + { + #[cfg(feature = "rpk")] + assert!(!self.is_rpk, "This API is not supported for RPK"); + + // NOTE(jlarisch): Q: Why don't we wrap the callback in an Arc, since + // `set_verify_callback` does? + // A: I don't think that Arc is necessary, and I don't think one is necessary here. + // There's no way to get a mutable reference to the `Ssl` or `SslContext`, which + // is what you need to register a new callback. + // See the NOTE in `ssl_raw_verify` for confirmation. + self.replace_ex_data(SslContext::cached_ex_index::(), callback); + unsafe { + ffi::SSL_CTX_set_cert_verify_callback( + self.as_ptr(), + Some(raw_cert_verify::), + ptr::null_mut(), + ); + } + } + /// Configures the certificate verification method for new connections. #[corresponds(SSL_CTX_set_verify)] pub fn set_verify(&mut self, mode: SslVerifyMode) { @@ -1472,7 +1515,7 @@ impl SslContextBuilder { #[corresponds(SSL_CTX_set_alpn_protos)] pub fn set_alpn_protos(&mut self, protocols: &[u8]) -> Result<(), ErrorStack> { unsafe { - #[cfg_attr(not(feature = "fips"), allow(clippy::unnecessary_cast))] + #[cfg_attr(not(feature = "fips-compat"), allow(clippy::unnecessary_cast))] { assert!(protocols.len() <= ProtosLen::MAX as usize); } @@ -1816,7 +1859,7 @@ impl SslContextBuilder { /// version of BoringSSL which doesn't yet include these APIs. /// Once the submoduled fips commit is upgraded, these gates can be removed. #[corresponds(SSL_CTX_set_permute_extensions)] - #[cfg(not(feature = "fips"))] + #[cfg(not(feature = "fips-compat"))] pub fn set_permute_extensions(&mut self, enabled: bool) { unsafe { ffi::SSL_CTX_set_permute_extensions(self.as_ptr(), enabled as _) } } @@ -1891,7 +1934,7 @@ impl SslContextBuilder { /// /// This feature isn't available in the certified version of BoringSSL. #[corresponds(SSL_CTX_set_compliance_policy)] - #[cfg(not(feature = "fips"))] + #[cfg(not(feature = "fips-compat"))] pub fn set_compliance_policy(&mut self, policy: CompliancePolicy) -> Result<(), ErrorStack> { unsafe { cvt_0i(ffi::SSL_CTX_set_compliance_policy(self.as_ptr(), policy.0)).map(|_| ()) } } @@ -2163,9 +2206,9 @@ impl SslContextRef { #[derive(Debug)] pub struct GetSessionPendingError; -#[cfg(not(feature = "fips"))] +#[cfg(not(feature = "fips-compat"))] type ProtosLen = usize; -#[cfg(feature = "fips")] +#[cfg(feature = "fips-compat")] type ProtosLen = libc::c_uint; /// Information about the state of a cipher. @@ -2886,7 +2929,7 @@ impl SslRef { /// Note: This is gated to non-fips because the fips feature builds with a separate /// version of BoringSSL which doesn't yet include these APIs. /// Once the submoduled fips commit is upgraded, these gates can be removed. - #[cfg(not(feature = "fips"))] + #[cfg(not(feature = "fips-compat"))] pub fn set_permute_extensions(&mut self, enabled: bool) { unsafe { ffi::SSL_set_permute_extensions(self.as_ptr(), enabled as _) } } @@ -2897,7 +2940,7 @@ impl SslRef { #[corresponds(SSL_set_alpn_protos)] pub fn set_alpn_protos(&mut self, protocols: &[u8]) -> Result<(), ErrorStack> { unsafe { - #[cfg_attr(not(feature = "fips"), allow(clippy::unnecessary_cast))] + #[cfg_attr(not(feature = "fips-compat"), allow(clippy::unnecessary_cast))] { assert!(protocols.len() <= ProtosLen::MAX as usize); } diff --git a/boring/src/ssl/test/cert_verify.rs b/boring/src/ssl/test/cert_verify.rs new file mode 100644 index 00000000..929db48c --- /dev/null +++ b/boring/src/ssl/test/cert_verify.rs @@ -0,0 +1,99 @@ +use crate::hash::MessageDigest; +use crate::ssl::test::Server; +use crate::ssl::SslVerifyMode; + +#[test] +fn error_when_trusted_but_callback_returns_false() { + let mut server = Server::builder(); + server.should_error(); + let server = server.build(); + let mut client = server.client_with_root_ca(); + client.ctx().set_verify(SslVerifyMode::PEER); + client.ctx().set_cert_verify_callback(|x509| { + // The cert is OK + assert!(x509.verify_cert().unwrap()); + assert!(x509.current_cert().is_some()); + assert!(x509.verify_result().is_ok()); + // But we return false + false + }); + + client.connect_err(); +} + +#[test] +fn no_error_when_untrusted_but_callback_returns_true() { + let server = Server::builder().build(); + let mut client = server.client(); + client.ctx().set_verify(SslVerifyMode::PEER); + client.ctx().set_cert_verify_callback(|x509| { + // The cert is not OK + assert!(!x509.verify_cert().unwrap()); + assert!(x509.current_cert().is_some()); + assert!(x509.verify_result().is_err()); + // But we return true + true + }); + + client.connect(); +} + +#[test] +fn no_error_when_trusted_and_callback_returns_true() { + let server = Server::builder().build(); + let mut client = server.client_with_root_ca(); + client.ctx().set_verify(SslVerifyMode::PEER); + client.ctx().set_cert_verify_callback(|x509| { + // The cert is OK + assert!(x509.verify_cert().unwrap()); + assert!(x509.current_cert().is_some()); + assert!(x509.verify_result().is_ok()); + // And we return true + true + }); + client.connect(); +} + +#[test] +fn callback_receives_correct_certificate() { + let server = Server::builder().build(); + let mut client = server.client(); + let expected = "59172d9313e84459bcff27f967e79e6e9217e584"; + client.ctx().set_verify(SslVerifyMode::PEER); + client.ctx().set_cert_verify_callback(move |x509| { + assert!(!x509.verify_cert().unwrap()); + assert!(x509.current_cert().is_some()); + assert!(x509.verify_result().is_err()); + let cert = x509.current_cert().unwrap(); + let digest = cert.digest(MessageDigest::sha1()).unwrap(); + assert_eq!(hex::encode(digest), expected); + true + }); + + client.connect(); +} + +#[test] +fn callback_receives_correct_chain() { + let server = Server::builder().build(); + let mut client = server.client_with_root_ca(); + let leaf_sha1 = "59172d9313e84459bcff27f967e79e6e9217e584"; + let root_sha1 = "c0cbdf7cdd03c9773e5468e1f6d2da7d5cbb1875"; + client.ctx().set_verify(SslVerifyMode::PEER); + client.ctx().set_cert_verify_callback(move |x509| { + assert!(x509.verify_cert().unwrap()); + assert!(x509.current_cert().is_some()); + assert!(x509.verify_result().is_ok()); + let chain = x509.chain().unwrap(); + assert!(chain.len() == 2); + let leaf_cert = chain.get(0).unwrap(); + let leaf_digest = leaf_cert.digest(MessageDigest::sha1()).unwrap(); + assert_eq!(hex::encode(leaf_digest), leaf_sha1); + let root_cert = chain.get(1).unwrap(); + let root_digest = root_cert.digest(MessageDigest::sha1()).unwrap(); + assert_eq!(hex::encode(root_digest), root_sha1); + true + }); + + client.connect(); +} diff --git a/boring/src/ssl/test/mod.rs b/boring/src/ssl/test/mod.rs index f3b0fd29..6010cf98 100644 --- a/boring/src/ssl/test/mod.rs +++ b/boring/src/ssl/test/mod.rs @@ -24,6 +24,7 @@ use crate::x509::{X509Name, X509}; #[cfg(not(feature = "fips"))] use super::CompliancePolicy; +mod cert_verify; mod custom_verify; mod private_key_method; mod server; diff --git a/boring/src/x509/mod.rs b/boring/src/x509/mod.rs index 34896d6a..af1d5040 100644 --- a/boring/src/x509/mod.rs +++ b/boring/src/x509/mod.rs @@ -981,9 +981,9 @@ impl X509NameBuilder { } } -#[cfg(not(feature = "fips"))] +#[cfg(not(feature = "fips-compat"))] type ValueLen = isize; -#[cfg(feature = "fips")] +#[cfg(feature = "fips-compat")] type ValueLen = i32; foreign_type_and_impl_send_sync! { diff --git a/boring/src/x509/tests/trusted_first.rs b/boring/src/x509/tests/trusted_first.rs index 951d1da5..9823072f 100644 --- a/boring/src/x509/tests/trusted_first.rs +++ b/boring/src/x509/tests/trusted_first.rs @@ -15,7 +15,7 @@ fn test_verify_cert() { assert_eq!(Ok(()), verify(&leaf, &[&root1], &[&intermediate], |_| {})); - #[cfg(not(feature = "fips"))] + #[cfg(not(feature = "fips-compat"))] assert_eq!( Ok(()), verify( @@ -26,7 +26,7 @@ fn test_verify_cert() { ) ); - #[cfg(feature = "fips")] + #[cfg(feature = "fips-compat")] assert_eq!( Err(X509VerifyError::CERT_HAS_EXPIRED), verify(