rust-openssl / rust-openssl/rust-openssl
Building with BoringSSL should set ossl110, etc.
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 1.6k
- Forks
- 841
- Avg merge
- 6h 51m
- Merged PRs (30d)
- 4
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:
- 0194e3f9decf0820615ce5b70f26433ac15eaba7
- 0257e2611d01127607b724a043642b01adf41706
- 2fe8b94066f1063ec78b0502052e4558379514a0
- 65a75a818f280ed578e9e68f7d6c1ca203b10e6f (the dh.rs bugfix)
- 3f68c0e5c77bd27ece67eb589ac71fc734fffe5b
- d3e557cf4836e49d97f38dcf2b349b8e7c30d9a8
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_rustsuffixes 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
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing the BoringSSL and OpenSSL cfg handling around openssl/src/rsa.rs and openssl/src/dsa.rs, then inspect the related bindings and callback naming in bssl-sys. Map the existing cfg(ossl110, boringssl) cases and unsupported features before changing the compatibility structure. Done means BoringSSL follows its advertised OpenSSL target without requiring scattered special cases.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- build-system, cryptography
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100