rust-openssl / rust-openssl/rust-openssl

Building with BoringSSL should set ossl110, etc.

Open
#1,944 5 comments 0 reactions 0 assignees View on GitHub

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_rust suffixes on function callbacks which we may need to replicate in bssl-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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.