-
-
Notifications
You must be signed in to change notification settings - Fork 758
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
Building with BoringSSL should set ossl110, etc. #1944
Comments
Yes, I agree we should default to treating BoringSSL as its "equivalent" OpenSSL and opt things out as required, instead of the reverse. I had this on my TODO list and then... forgot about it, because I'm an irresponsible person. |
Oh we should also resolve the |
The RSA and DSA changes will be needed to avoid build breakage soon. The others are mostly tidying up. There's another place around BIO that we'd ideally also switch over, but that depends on resolving the __fixed_rust mess first. This addresses a symptom of sfackler#1944, but not the root cause.
I'm not sure what way you all want to express it in the build (figure I'll leave that to you), but hacking the cfgs in, I see a few compat functions we'll want on the BoringSSL side. I'll take a pass adding in some of those and we can iterate from there. |
Playing with this a bit, the other issue is rust-openssl should still pick up |
Setting ossl110 in the BoringSSL build (see sfackler#1944) causes rust-openssl to expect OCB support. However, OpenSSL already has a feature guard for OCB, which BoringSSL sets. rust-openssl just isn't honoring it. This fixes building against an OpenSSL built with ./config no-ocb
The RSA and DSA changes will be needed to avoid build breakage soon. The others are mostly tidying up. There's another place around BIO that we'd ideally also switch over, but that depends on resolving the __fixed_rust mess first. This addresses a symptom of sfackler#1944, but not the root cause.
BoringSSL defines OpenSSL-compatible OPENSSL_NO_* values to aid porting. This removes a ton of boringssl cfg lines that weren't actually necessary. This addresses part of issue sfackler#1944, though not the version number half.
OpenSSL 1.1.x renamed these functions with an OPENSSL_ prefix. Unfortunately, rust-openssl uses these, losing type-safety, rather than the type-safe macros. It currently expects the old, unprefixed names due to a different bug (sfackler/rust-openssl#1944), but to fix that, we'll need to align with the OpenSSL names. To keep the current version of rust-openssl working, I've preserved the old names that rust-openssl uses, but we should clear these out. Bug: 499 Change-Id: I3be56a54ef503620b92ce8154fafd46b2906ae63 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60505 Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com>
OpenSSL 1.1.x renamed these functions with an OPENSSL_ prefix. Unfortunately, rust-openssl uses these, losing type-safety, rather than the type-safe macros. It currently expects the old, unprefixed names due to a different bug (sfackler/rust-openssl#1944), but to fix that, we'll need to align with the OpenSSL names. To keep the current version of rust-openssl working, I've preserved the old names that rust-openssl uses, but we should clear these out. Bug: 499 Change-Id: I3be56a54ef503620b92ce8154fafd46b2906ae63 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60505 Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com> (cherry picked from commit 99d3c228344ccad4aa29e9ecc2d0f618c9a2b384)
OpenSSL 1.1.x renamed these functions with an OPENSSL_ prefix. Unfortunately, rust-openssl uses these, losing type-safety, rather than the type-safe macros. It currently expects the old, unprefixed names due to a different bug (sfackler/rust-openssl#1944), but to fix that, we'll need to align with the OpenSSL names. To keep the current version of rust-openssl working, I've preserved the old names that rust-openssl uses, but we should clear these out. Bug: 499 Change-Id: I3be56a54ef503620b92ce8154fafd46b2906ae63 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60505 Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com> (cherry picked from commit 99d3c228344ccad4aa29e9ecc2d0f618c9a2b384)
OpenSSL 1.1.x renamed these functions with an OPENSSL_ prefix. Unfortunately, rust-openssl uses these, losing type-safety, rather than the type-safe macros. It currently expects the old, unprefixed names due to a different bug (sfackler/rust-openssl#1944), but to fix that, we'll need to align with the OpenSSL names. To keep the current version of rust-openssl working, I've preserved the old names that rust-openssl uses, but we should clear these out. Bug: 499 Change-Id: I3be56a54ef503620b92ce8154fafd46b2906ae63 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60505 Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com> (cherry picked from commit 99d3c228344ccad4aa29e9ecc2d0f618c9a2b384)
OpenSSL 1.1.x renamed these functions with an OPENSSL_ prefix. Unfortunately, rust-openssl uses these, losing type-safety, rather than the type-safe macros. It currently expects the old, unprefixed names due to a different bug (sfackler/rust-openssl#1944), but to fix that, we'll need to align with the OpenSSL names. To keep the current version of rust-openssl working, I've preserved the old names that rust-openssl uses, but we should clear these out. Bug: 499 Change-Id: I3be56a54ef503620b92ce8154fafd46b2906ae63 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60505 Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com> (cherry picked from commit 99d3c228344ccad4aa29e9ecc2d0f618c9a2b384)
OpenSSL 1.1.x renamed these functions with an OPENSSL_ prefix. Unfortunately, rust-openssl uses these, losing type-safety, rather than the type-safe macros. It currently expects the old, unprefixed names due to a different bug (sfackler/rust-openssl#1944), but to fix that, we'll need to align with the OpenSSL names. To keep the current version of rust-openssl working, I've preserved the old names that rust-openssl uses, but we should clear these out. Bug: 499 Change-Id: I3be56a54ef503620b92ce8154fafd46b2906ae63 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60505 Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com> (cherry picked from commit 99d3c228344ccad4aa29e9ecc2d0f618c9a2b384)
This is yet another symptom of sfackler#1944. Ideally rust-openssl would use the type-safe wrappers, which are the actual public APIs. Right now, rust-openssl's STACK_OF(T) handling is a safety regression from C. This PR doesn't address the safety issue, only works around the BoringSSL porting issue.
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 haveOPENSSL_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: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)__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.)CC @maurer @alex @reaperhulk
The text was updated successfully, but these errors were encountered: