-
Notifications
You must be signed in to change notification settings - Fork 794
Description
In C, BoringSSL defines OPENSSL_VERSION_NUMBER
to match OpenSSL 1.1.1 because, by default, you're expected to use the code you use for OpenSSL 1.1.1. Occasionally things vary from the target version, so we have OPENSSL_IS_BORINGSSL
, but the default is to match 1.1.1.
rust-openssl's initial BoringSSL port wasn't done right. Rather than following this standard pattern, it defaults to giving BoringSSL the code for the very oldest OpenSSL! This has caused a lot of mess over the short period of time this port has existed:
We're still not done. The code below was deprecated years before the port existed, and will break very soon. Also it means rust-openssl loses this fix.
https://github.com/sfackler/rust-openssl/blob/master/openssl/src/rsa.rs#L584
https://github.com/sfackler/rust-openssl/blob/master/openssl/src/dsa.rs#L317
I'll upload a PR to do a point fix there, but that is merely fixing a symptom of a structural flaw in the port. The true fix is for rust-openssl to follow the supported pattern in C. The default for OpenSSL derivatives should be to target the OpenSSL version they advertise.
That'll simplify a lot of the cfg(ossl110, boringssl)
lines. Though we'll need to clean up some tech debt in the process:
- Adding
cfg(not(boringssl))
on features we don't support (bindings libraries are, alas, a pain point because they tend to expose every feature that exists, even if not used in practice) - In some places, we'll probably need to add some compatibility functions to BoringSSL
- There's some mess with
__fixed_rust
suffixes on function callbacks which we may need to replicate inbssl-sys
. (The old names have been deprecated for a while... is it time to unwind that mess?)
(We also have our own BORINGSSL_API_VERSION
define, but that matters less if you only target one BoringSSL. Though @maurer may care about this due to Android.)