From 67801c7b5c6c9a1b297ea4cbacf593a0553d70c2 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 1 May 2024 13:24:24 -0400 Subject: [PATCH 1/3] Fix building with latest BoringSSL This required two fixes. First, test_verify_param_set_depth_fails_verification needed to be updated. The history here is that OpenSSL 1.1.0 made a backwards-incompatible change to the semantics of the depth limit. Ideally, rust-openssl would have documented the semantics of its own APIs and normalized the behavior, but it instead silently picked up the semantics change. BoringSSL aligned with OpenSSL's new behavior in b251d813ec615e7ef01d82073f94960eb13b1e0a, but since then rust-openssl has codified the old behavior in tests. We need to update some cfgs to reflect this. Second, BoringSSL requires a C++ runtime (we have required a C++ compiler for a long time). This reveals a problem in Cargo's dependency management strategy for externally-built static libraries. Work around this by making some guesses about what library to link in, but see the comment for why this is unsafe. This unsafety appears to be inherent to Cargo and the choice of having Cargo drive cross-language builds, rather than providing hooks for an integrated build system. --- .github/workflows/ci.yml | 2 +- openssl-sys/build/main.rs | 28 ++++++++++++++++++++++++++++ openssl/src/x509/tests.rs | 2 +- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2c46b936c2..fe43c9f791 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -152,7 +152,7 @@ jobs: - false library: - name: boringssl - version: e6489902b7fb692875341b8ab5e57f0515f47bc1 + version: 2db0eb3f96a5756298dcd7f9319e56a98585bd10 - name: openssl version: vendored - name: openssl diff --git a/openssl-sys/build/main.rs b/openssl-sys/build/main.rs index 606acc35c7..70a7a14e02 100644 --- a/openssl-sys/build/main.rs +++ b/openssl-sys/build/main.rs @@ -132,6 +132,34 @@ fn main() { println!("cargo:rustc-link-lib={}={}", kind, lib); } + // libssl in BoringSSL requires the C++ runtime, and static libraries do + // not carry dependency information. On unix-like platforms, the C++ + // runtime and standard library are typically picked up by default via the + // C++ compiler, which has a platform-specific default. (See implementations + // of `GetDefaultCXXStdlibType` in Clang.) Builds may also choose to + // override this and specify their own with `-nostdinc++` and `-nostdlib++` + // flags. Some compilers also provide options like `-stdlib=libc++`. + // + // Typically, such information is carried all the way up the build graph, + // but Cargo is not an integrated cross-language build system, so it cannot + // safely handle any of these situations. As a result, we need to make + // guesses. Getting this wrong may result in symbol conflicts and memory + // errors, but this unsafety is inherent to driving builds with + // externally-built libraries using Cargo. + // + // For now, we guess that the build was made with the defaults. This too is + // difficult because Rust does not expose this information from Clang, but + // try to match the behavior for common platforms. For a more robust option, + // this likely needs to be deferred to the caller with an environment + // variable. + if version == Version::Boringssl && kind == "static" && env::var("CARGO_CFG_UNIX").is_ok() { + let cpp_lib = match env::var("CARGO_CFG_TARGET_OS").unwrap().as_ref() { + "macos" => "c++", + _ => "stdc++", + }; + println!("cargo:rustc-link-lib={}", cpp_lib); + } + // https://github.com/openssl/openssl/pull/15086 if version == Version::Openssl3xx && kind == "static" diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index ae61a2ad34..25c2da0125 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -944,7 +944,7 @@ fn test_verify_param_set_depth_fails_verification() { store_bldr.add_cert(ca).unwrap(); let mut verify_params = X509VerifyParam::new().unwrap(); // OpenSSL 1.1.0+ considers the root certificate to not be part of the chain, while 1.0.2 and LibreSSL do - let expected_depth = if cfg!(any(ossl110)) { 0 } else { 1 }; + let expected_depth = if cfg!(any(ossl110, boringssl)) { 0 } else { 1 }; verify_params.set_depth(expected_depth); store_bldr.set_param(&verify_params).unwrap(); let store = store_bldr.build(); From 5a39798befc5cb972505c811da6af0dff765cdbe Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 2 May 2024 10:18:05 -0400 Subject: [PATCH 2/3] Build OpenSSL et al on 32-bit x86 with -msse2 Newer BoringSSL requires SSE2 on 32-bit x86, but I opted to apply it to all the OpenSSLs that CI builds. This allows the compiler to vectorize random bits of the library, so hopefully it'll run faster on CI. Especially with 32-bit x86 being so register-poor. Additionally, Rust's definition of i686 already assumes SSE2 (see https://github.com/rust-lang/rust/issues/82435), so there's no sense in pretending to test pre-SSE2 targets. --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fe43c9f791..638c041726 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -277,7 +277,7 @@ jobs: ;; "i686-unknown-linux-gnu") OS_COMPILER=linux-elf - OS_FLAGS=-m32 + OS_FLAGS=-m32 -msse2 ;; "arm-unknown-linux-gnueabihf") OS_COMPILER=linux-armv4 From dd1753f5b1b87574a6ba81d13806a0b47e28ac12 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 2 May 2024 10:41:32 -0400 Subject: [PATCH 3/3] Be better at shell --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 638c041726..b25057dd69 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -277,7 +277,7 @@ jobs: ;; "i686-unknown-linux-gnu") OS_COMPILER=linux-elf - OS_FLAGS=-m32 -msse2 + OS_FLAGS="-m32 -msse2" ;; "arm-unknown-linux-gnueabihf") OS_COMPILER=linux-armv4