Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for building boringssl with bindgen #1831

Merged
merged 1 commit into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ jobs:
- false
library:
- name: boringssl
version: 5697a9202615925696f8dc7f4e286d44d474769e
version: 93e8d4463d59d671e9c5c6171226341f04b07907
- name: openssl
version: vendored
- name: openssl
Expand Down Expand Up @@ -215,10 +215,6 @@ jobs:
library:
name: libressl
version: 3.7.0
exclude:
- library:
name: boringssl
bindgen: true
name: ${{ matrix.target }}-${{ matrix.library.name }}-${{ matrix.library.version }}-${{ matrix.bindgen }}
runs-on: ubuntu-latest
env:
Expand Down Expand Up @@ -311,24 +307,34 @@ jobs:
make install_sw
;;
"boringssl")
sed -i rust/CMakeLists.txt -e '1s%^%include_directories(../include)\n%'
cpu=`echo ${{ matrix.target }} | cut -d - -f 1`
mkdir build
cd build

echo "set(CMAKE_SYSTEM_NAME Linux)" > toolchain.cmake
echo "set(CMAKE_SYSTEM_PROCESSOR $cpu)" >> toolchain.cmake
echo "set(triple ${{ matrix.target }})" >> toolchain.cmake
echo 'set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} '$OS_FLAGS '" CACHE STRING "c++ flags")' >> toolchain.cmake
echo 'set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} '$OS_FLAGS '" CACHE STRING "c flags")' >> toolchain.cmake
echo 'set(CMAKE_ASM_FLAGS "${CMAKE_ASM_FLAGS} '$OS_FLAGS '" CACHE STRING "asm flags")' >> toolchain.cmake
cmake -DRUST_BINDINGS="${{ matrix.target }}" -B $OPENSSL_DIR -DCMAKE_TOOLCHAIN_FILE=toolchain.cmake
make -C $OPENSSL_DIR

cmake .. -DCMAKE_POSITION_INDEPENDENT_CODE=ON -DRUST_BINDINGS="${{ matrix.target }}" -DCMAKE_INSTALL_PREFIX="${OPENSSL_DIR}" -DCMAKE_TOOLCHAIN_FILE=toolchain.cmake
make -j "$(nproc)"
make install

# Copy stuff around so it's all as the build system expects.
cp -r rust/ "$OPENSSL_DIR/rust"
mkdir -p "$OPENSSL_DIR/crypto/"
mkdir -p "$OPENSSL_DIR/ssl/"
cp "$OPENSSL_DIR/lib/libcrypto.a" "$OPENSSL_DIR/crypto/"
cp "$OPENSSL_DIR/lib/libssl.a" "$OPENSSL_DIR/ssl/"
esac

if: matrix.library.version != 'vendored' && !steps.openssl-cache.outputs.cache-hit
- run: |
mkdir -p .cargo
echo '[patch.crates-io]' > .cargo/config.toml
echo 'bssl-sys = { path = "'$OPENSSL_DIR'/rust" }' >> .cargo/config.toml
if: matrix.library.name == 'boringssl'
if: matrix.library.name == 'boringssl' && !matrix.bindgen
- uses: actions/cache@v3
with:
path: ~/.cargo/registry/index
Expand All @@ -350,14 +356,14 @@ jobs:
if [[ "${{ matrix.library.version }}" == "vendored" ]]; then
features="--features vendored"
fi
if [[ "${{ matrix.bindgen }}" == "true" ]]; then
if [[ "${{ matrix.bindgen }}" == "true" && "${{ matrix.library.name }}" != "boringssl" ]]; then
features="$features --features bindgen"
fi
cargo run --manifest-path=systest/Cargo.toml --target ${{ matrix.target }} $features
if: matrix.library.name != 'boringssl'
- name: Test openssl
run: |
if [[ "${{ matrix.library.name }}" == "boringssl" ]]; then
if [[ "${{ matrix.library.name }}" == "boringssl" && "${{ matrix.bindgen }}" != "true" ]]; then
features="--features unstable_boringssl"
fi
if [[ "${{ matrix.library.version }}" == "vendored" ]]; then
Expand Down
2 changes: 1 addition & 1 deletion openssl-sys/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ libc = "0.2"
bssl-sys = { version = "0.1.0", optional = true }

[build-dependencies]
bindgen = { version = "0.60.1", optional = true }
bindgen = { version = "0.64.0", optional = true, features = ["experimental"] }
cc = "1.0"
openssl-src = { version = "111", optional = true }
pkg-config = "0.3.9"
Expand Down
21 changes: 15 additions & 6 deletions openssl-sys/build/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ mod cfgs;
mod find_normal;
#[cfg(feature = "vendored")]
mod find_vendored;
#[cfg(feature = "bindgen")]
mod run_bindgen;

#[derive(PartialEq)]
Expand All @@ -32,6 +31,7 @@ enum Version {
Openssl11x,
Openssl10x,
Libressl,
Boringssl,
}

fn env_inner(name: &str) -> Option<OsString> {
Expand Down Expand Up @@ -67,10 +67,9 @@ fn find_openssl(target: &str) -> (Vec<PathBuf>, PathBuf) {
fn check_ssl_kind() {
if cfg!(feature = "unstable_boringssl") {
println!("cargo:rustc-cfg=boringssl");
println!("cargo:boringssl=true");
// BoringSSL does not have any build logic, exit early
std::process::exit(0);
} else {
println!("cargo:rustc-cfg=openssl");
}
}

Expand Down Expand Up @@ -146,8 +145,12 @@ fn check_rustc_versions() {
#[allow(clippy::let_and_return)]
fn postprocess(include_dirs: &[PathBuf]) -> Version {
let version = validate_headers(include_dirs);
#[cfg(feature = "bindgen")]
run_bindgen::run(&include_dirs);

// Never run bindgen for BoringSSL, if it was needed we already ran it.
if version != Version::Boringssl {
#[cfg(feature = "bindgen")]
run_bindgen::run(&include_dirs);
}

version
}
Expand Down Expand Up @@ -235,9 +238,15 @@ See rust-openssl documentation for more information:
}

if is_boringssl {
panic!("BoringSSL detected, but `unstable_boringssl` feature wasn't specified.")
println!("cargo:rustc-cfg=boringssl");
println!("cargo:boringssl=true");
run_bindgen::run_boringssl(include_dirs);
return Version::Boringssl;
}

// We set this for any non-BoringSSL lib.
println!("cargo:rustc-cfg=openssl");

for enabled in &enabled {
println!("cargo:rustc-cfg=osslconf=\"{}\"", enabled);
}
Expand Down
117 changes: 112 additions & 5 deletions openssl-sys/build/run_bindgen.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
#[cfg(feature = "bindgen")]
use bindgen::callbacks::{MacroParsingBehavior, ParseCallbacks};
use bindgen::RustTarget;
use std::env;
#[cfg(feature = "bindgen")]
use bindgen::{MacroTypeVariation, RustTarget};
use std::io::Write;
use std::path::PathBuf;
#[cfg(not(feature = "bindgen"))]
use std::process;
use std::{env, fs};

const INCLUDES: &str = "
#include <openssl/aes.h>
#include <openssl/asn1.h>
#include <openssl/bio.h>
#include <openssl/comp.h>
#include <openssl/conf.h>
#include <openssl/crypto.h>
#include <openssl/dh.h>
Expand All @@ -17,7 +21,6 @@ const INCLUDES: &str = "
#include <openssl/evp.h>
#include <openssl/hmac.h>
#include <openssl/objects.h>
#include <openssl/ocsp.h>
#include <openssl/opensslv.h>
#include <openssl/pem.h>
#include <openssl/pkcs12.h>
Expand All @@ -35,10 +38,15 @@ const INCLUDES: &str = "
// this must be included after ssl.h for libressl!
#include <openssl/srtp.h>

#if !defined(LIBRESSL_VERSION_NUMBER)
#if !defined(LIBRESSL_VERSION_NUMBER) && !defined(OPENSSL_IS_BORINGSSL)
#include <openssl/cms.h>
#endif

#if !defined(OPENSSL_IS_BORINGSSL)
#include <openssl/comp.h>
#include <openssl/ocsp.h>
#endif

#if !defined(LIBRESSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER >= 0x10100000
#include <openssl/kdf.h>
#endif
Expand All @@ -48,6 +56,7 @@ const INCLUDES: &str = "
#endif
";

#[cfg(feature = "bindgen")]
pub fn run(include_dirs: &[PathBuf]) {
let out_dir = PathBuf::from(env::var_os("OUT_DIR").unwrap());

Expand Down Expand Up @@ -94,9 +103,107 @@ pub fn run(include_dirs: &[PathBuf]) {
.unwrap();
}

#[cfg(feature = "bindgen")]
pub fn run_boringssl(include_dirs: &[PathBuf]) {
let out_dir = PathBuf::from(env::var_os("OUT_DIR").unwrap());
let mut builder = bindgen::builder()
.rust_target(RustTarget::Stable_1_47)
.ctypes_prefix("::libc")
.derive_default(false)
.enable_function_attribute_detection()
.size_t_is_usize(true)
.default_macro_constant_type(MacroTypeVariation::Signed)
.rustified_enum("point_conversion_form_t")
.allowlist_file(".*/openssl/[^/]+\\.h")
.wrap_static_fns(true)
.wrap_static_fns_path(out_dir.join("boring_static_wrapper").display().to_string())
.layout_tests(false)
.header_contents("includes.h", INCLUDES);

for include_dir in include_dirs {
builder = builder
.clang_arg("-I")
.clang_arg(include_dir.display().to_string());
}

builder
.generate()
.unwrap()
.write_to_file(out_dir.join("bindgen.rs"))
.unwrap();

fs::File::create(out_dir.join("boring_static_wrapper.h"))
.expect("Failed to create boring_static_wrapper.h")
.write_all(INCLUDES.as_bytes())
.expect("Failed to write contents to boring_static_wrapper.h");

cc::Build::new()
.file(out_dir.join("boring_static_wrapper.c"))
.includes(include_dirs)
.flag("-include")
.flag(
&out_dir
.join("boring_static_wrapper.h")
.display()
.to_string(),
)
.compile("boring_static_wrapper");
}

#[cfg(not(feature = "bindgen"))]
pub fn run_boringssl(include_dirs: &[PathBuf]) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems a bit unfortunate to be duplicating all of the bindgen flags here and above. If we end up having to tweak them in the future we should probably refactor.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and yes, makes sense.

let out_dir = PathBuf::from(env::var_os("OUT_DIR").unwrap());

fs::File::create(out_dir.join("boring_static_wrapper.h"))
.expect("Failed to create boring_static_wrapper.h")
.write_all(INCLUDES.as_bytes())
.expect("Failed to write contents to boring_static_wrapper.h");

let mut bindgen_cmd = process::Command::new("bindgen");
bindgen_cmd
.arg("-o")
.arg(out_dir.join("bindgen.rs"))
.arg("--rust-target=1.47")
.arg("--ctypes-prefix=::libc")
.arg("--no-derive-default")
.arg("--enable-function-attribute-detection")
.arg("--size_t-is-usize")
.arg("--default-macro-constant-type=signed")
.arg("--rustified-enum=point_conversion_form_t")
.arg("--allowlist-file=.*/openssl/[^/]+\\.h")
.arg("--experimental")
.arg("--wrap-static-fns")
Copy link
Contributor

@davidben davidben Mar 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on, it doesn't look like this flag works correctly in bindgen yet, so from BoringSSL's perspective, this bindings strategy is unsupported. I'd love to see a version of it that works, but bindgen isn't there yet.
https://boringssl-review.googlesource.com/c/boringssl/+/56505/8/rust/CMakeLists.txt#37

.arg("--wrap-static-fns-path")
.arg(out_dir.join("boring_static_wrapper").display().to_string())
.arg("--no-layout-tests")
.arg(out_dir.join("boring_static_wrapper.h"))
.arg("--")
.arg(format!("--target={}", env::var("TARGET").unwrap()));

for include_dir in include_dirs {
bindgen_cmd.arg("-I").arg(include_dir.display().to_string());
}

let result = bindgen_cmd.status().expect("bindgen failed to execute");
assert!(result.success());

cc::Build::new()
.file(out_dir.join("boring_static_wrapper.c"))
.includes(include_dirs)
.flag("-include")
.flag(
&out_dir
.join("boring_static_wrapper.h")
.display()
.to_string(),
)
.compile("boring_static_wrapper");
}

#[derive(Debug)]
struct OpensslCallbacks;

#[cfg(feature = "bindgen")]
impl ParseCallbacks for OpensslCallbacks {
// for now we'll continue hand-writing constants
fn will_parse_macro(&self, _name: &str) -> MacroParsingBehavior {
Expand Down
18 changes: 16 additions & 2 deletions openssl-sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,25 @@
extern crate libc;
pub use libc::*;

#[cfg(boringssl)]
#[cfg(feature = "unstable_boringssl")]
extern crate bssl_sys;
#[cfg(boringssl)]
#[cfg(feature = "unstable_boringssl")]
pub use bssl_sys::*;

#[cfg(all(boringssl, not(feature = "unstable_boringssl")))]
#[path = "."]
mod boringssl {
include!(concat!(env!("OUT_DIR"), "/bindgen.rs"));

pub fn init() {
unsafe {
CRYPTO_library_init();
}
}
}
#[cfg(all(boringssl, not(feature = "unstable_boringssl")))]
pub use boringssl::*;

#[cfg(openssl)]
#[path = "."]
mod openssl {
Expand Down
2 changes: 1 addition & 1 deletion openssl/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ fn main() {
println!("cargo:rustc-cfg=libressl");
}

if env::var("CARGO_FEATURE_UNSTABLE_BORINGSSL").is_ok() {
if env::var("DEP_OPENSSL_BORINGSSL").is_ok() {
println!("cargo:rustc-cfg=boringssl");
return;
}
Expand Down
4 changes: 2 additions & 2 deletions openssl/src/bio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl<'a> MemBioSlice<'a> {
let bio = unsafe {
cvt_p(BIO_new_mem_buf(
buf.as_ptr() as *const _,
buf.len() as c_int,
buf.len() as crate::SLenType,
))?
};

Expand Down Expand Up @@ -74,7 +74,7 @@ impl MemBio {
}

cfg_if! {
if #[cfg(ossl102)] {
if #[cfg(any(ossl102, boringssl))] {
use ffi::BIO_new_mem_buf;
} else {
#[allow(bad_style)]
Expand Down
2 changes: 1 addition & 1 deletion openssl/src/dh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ where
}

cfg_if! {
if #[cfg(any(ossl110, libressl270))] {
if #[cfg(any(ossl110, libressl270, boringssl))] {
use ffi::{DH_set0_pqg, DH_get0_pqg, DH_get0_key, DH_set0_key};
} else {
#[allow(bad_style)]
Expand Down
11 changes: 8 additions & 3 deletions openssl/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,19 +297,24 @@ impl fmt::Debug for Error {
}

impl fmt::Display for Error {
// On BoringSSL ERR_GET_{LIB,FUNC,REASON} are `unsafe`, but on
// OpenSSL/LibreSSL they're safe.
#[allow(unused_unsafe)]
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(fmt, "error:{:08X}", self.code())?;
match self.library() {
Some(l) => write!(fmt, ":{}", l)?,
None => write!(fmt, ":lib({})", ffi::ERR_GET_LIB(self.code()))?,
None => write!(fmt, ":lib({})", unsafe { ffi::ERR_GET_LIB(self.code()) })?,
}
match self.function() {
Some(f) => write!(fmt, ":{}", f)?,
None => write!(fmt, ":func({})", ffi::ERR_GET_FUNC(self.code()))?,
None => write!(fmt, ":func({})", unsafe { ffi::ERR_GET_FUNC(self.code()) })?,
}
match self.reason() {
Some(r) => write!(fmt, ":{}", r)?,
None => write!(fmt, ":reason({})", ffi::ERR_GET_REASON(self.code()))?,
None => write!(fmt, ":reason({})", unsafe {
ffi::ERR_GET_REASON(self.code())
})?,
}
write!(
fmt,
Expand Down
Loading