From c05a33991130d2e64673bf0a3f1eef08de68a27a Mon Sep 17 00:00:00 2001 From: Rushil Mehra Date: Fri, 16 Aug 2024 13:20:02 -0700 Subject: [PATCH 1/4] Support linking with a runtime cpp library As of https://boringssl-review.googlesource.com/c/boringssl/+/66288, libssl allows a C++ runtime dependency. As such, we need to link with a cpp runtime library. Implementation is inspired heavily from https://github.com/google/boringssl/commit/54c956b2e668e11c75f1ee0367f1b3a0ad28eff9. Before releasing this change, we'll need to figure out a way to support this for windows. --- boring-sys/build/config.rs | 2 ++ boring-sys/build/main.rs | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/boring-sys/build/config.rs b/boring-sys/build/config.rs index 7af00f4b..1671981f 100644 --- a/boring-sys/build/config.rs +++ b/boring-sys/build/config.rs @@ -34,6 +34,7 @@ pub(crate) struct Env { pub(crate) opt_level: Option, pub(crate) android_ndk_home: Option, pub(crate) cmake_toolchain_file: Option, + pub(crate) cpp_runtime_lib: Option, } impl Config { @@ -164,6 +165,7 @@ impl Env { opt_level: target_var("OPT_LEVEL"), android_ndk_home: target_var("ANDROID_NDK_HOME").map(Into::into), cmake_toolchain_file: target_var("CMAKE_TOOLCHAIN_FILE").map(Into::into), + cpp_runtime_lib: target_var("BORING_BSSL_RUST_CPPLIB").map(Into::into), } } } diff --git a/boring-sys/build/main.rs b/boring-sys/build/main.rs index 3ff818b3..4e32431c 100644 --- a/boring-sys/build/main.rs +++ b/boring-sys/build/main.rs @@ -1,4 +1,5 @@ use fslock::LockFile; +use std::env; use std::ffi::OsString; use std::fs; use std::io; @@ -636,6 +637,22 @@ fn link_in_precompiled_bcm_o(config: &Config) { .unwrap(); } +fn get_cpp_runtime_lib(config: &Config) -> Option { + if let Some(ref cpp_lib) = config.env.cpp_runtime_lib { + return cpp_lib.clone().into_string().ok(); + } + + // TODO(rmehra): figure out how to do this for windows + if env::var_os("CARGO_CFG_UNIX").is_some() { + match env::var("CARGO_CFG_TARGET_OS").unwrap().as_ref() { + "macos" | "ios" => Some("c++".into()), + _ => Some("stdc++".into()), + } + } else { + None + } +} + fn main() { let config = Config::from_env(); let bssl_dir = built_boring_source_path(&config); @@ -673,6 +690,9 @@ fn main() { link_in_precompiled_bcm_o(&config); } + if let Some(cpp_lib) = get_cpp_runtime_lib(&config) { + println!("cargo:rustc-link-lib={}", cpp_lib); + } println!("cargo:rustc-link-lib=static=crypto"); println!("cargo:rustc-link-lib=static=ssl"); From 33b511331b6cccf42bc75058af075e4ef9fbd697 Mon Sep 17 00:00:00 2001 From: Rushil Mehra Date: Fri, 16 Aug 2024 13:22:59 -0700 Subject: [PATCH 2/4] Fix bug with accessing memzero'd X509StoreContext in tests As of https://boringssl-review.googlesource.com/c/boringssl/+/64141, X509_STORE_CTX_cleanup will zero the memory allocated to the X509_STORE_CTX. Because X509StoreContextRef::init invokes X509_STORE_CTX_cleanup once the with_context closure has finished, calling X509StoreContextRef::verify_result (or any API really) is going to be invalid because memory has been zerod out. This is a pretty big footgun, so maybe we should consider screaming a bit louder for this case. --- boring/src/x509/tests/trusted_first.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/boring/src/x509/tests/trusted_first.rs b/boring/src/x509/tests/trusted_first.rs index 9823072f..d79ff2e3 100644 --- a/boring/src/x509/tests/trusted_first.rs +++ b/boring/src/x509/tests/trusted_first.rs @@ -93,12 +93,12 @@ fn verify( let mut store_ctx = X509StoreContext::new().unwrap(); - let _ = store_ctx.init(&trusted, cert, &untrusted, |ctx| { - configure(ctx.verify_param_mut()); - ctx.verify_cert().unwrap(); + store_ctx + .init(&trusted, cert, &untrusted, |ctx| { + configure(ctx.verify_param_mut()); + ctx.verify_cert().unwrap(); - Ok(()) - }); - - store_ctx.verify_result() + Ok(ctx.verify_result()) + }) + .expect("failed to obtain X509VerifyResult") } From baede6c0af5fe9ca5d07501ac5b81df2c333299c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leo=20Bl=C3=B6cher?= Date: Tue, 12 Nov 2024 14:01:30 +0100 Subject: [PATCH 3/4] Remove INVALID_CALL from mid-handshake error message Mid-handshake errors that occur before certificate verification currently look like this: ``` TLS handshake failed: cert verification failed - Invalid certificate verification context [WRONG_VERSION_NUMBER] ``` Despite no certificate even being received yet, the error complains about a failed verification. The cause here is that `cert verification failed` is only omitted if the verification result is `OK`. The default in BoringSSL before verification runs is `INVALID_CALL`, however. `INVALID_CALL` is set/returned in these places: - https://github.com/google/boringssl/blob/44b3df6f03d85c901767250329c571db405122d5/src/ssl/internal.h#L3904 - https://github.com/google/boringssl/blob/44b3df6f03d85c901767250329c571db405122d5/src/ssl/ssl_session.cc#L396 - https://github.com/google/boringssl/blob/44b3df6f03d85c901767250329c571db405122d5/src/ssl/ssl_x509.cc#L713 It is not used anywhere else as a verification result code. To improve the error message, this commit adds `INVALID_CALL` as a verification result for which no additional error is dislayed. --- boring/src/ssl/error.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/boring/src/ssl/error.rs b/boring/src/ssl/error.rs index 014eb188..a17243df 100644 --- a/boring/src/ssl/error.rs +++ b/boring/src/ssl/error.rs @@ -1,4 +1,5 @@ use crate::ffi; +use crate::x509::X509VerifyError; use libc::c_int; use std::error; use std::error::Error as StdError; @@ -206,7 +207,9 @@ fn fmt_mid_handshake_error( } match s.ssl().verify_result() { - Ok(()) => write!(f, "{}", prefix)?, + // INVALID_CALL is returned if no verification took place, + // such as before a cert is sent. + Ok(()) | Err(X509VerifyError::INVALID_CALL) => write!(f, "{}", prefix)?, Err(verify) => write!(f, "{}: cert verification failed - {}", prefix, verify)?, } From 796afe16371df928f8590e4a1c232b4c3fbdc613 Mon Sep 17 00:00:00 2001 From: Rushil Mehra Date: Wed, 31 Jul 2024 00:52:31 -0700 Subject: [PATCH 4/4] Allow dead_code instead of disabling clippy entirely for bindgen --- boring-sys/src/lib.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/boring-sys/src/lib.rs b/boring-sys/src/lib.rs index b707ec0e..c821b3e5 100644 --- a/boring-sys/src/lib.rs +++ b/boring-sys/src/lib.rs @@ -16,9 +16,11 @@ use std::convert::TryInto; use std::ffi::c_void; use std::os::raw::{c_char, c_int, c_uint, c_ulong}; -#[allow(dead_code)] -#[allow(clippy::all)] -#[rustfmt::skip] +#[allow( + clippy::useless_transmute, + clippy::derive_partial_eq_without_eq, + dead_code +)] mod generated { include!(concat!(env!("OUT_DIR"), "/bindings.rs")); }