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

Merge upstream v4.6.0 into libsignal branch #25

Closed
wants to merge 109 commits into from

Conversation

jrose-signal
Copy link

No description provided.

eaufavor and others added 30 commits September 19, 2023 22:04
While not commonly used, P-521 is a perfectly safe choice of key
exchange algorithm.
These APIs allow more SslStream to be used more flexibly
These lists are hardcoded and the calls have no business failing in the first place.
These two new kinds of methods immediately return a MidHandshakeSslStream
instead of actually initiating a handshake. This greatly simplifies
loops around MidHandshakeSslStream::WouldBlock.
This encapsulates a bit better the unsafety of task context
management to invoke async code from inside boring.
To handle lifetimes better and allow returning a &mut SslRef from
the client hello struct passed to the closure from
SslContextBuilder::set_select_certificate_callback, we make
the ClientHello struct itself own a reference to the FFI
client hello struct.
We introduce tokio_boring::SslContextBuilderExt, with 2 methods:

* set_async_select_certificate_callback
* set_async_private_key_method
In boringssl, FIPS_mode_set is more or less useless, and
it doesn't even set an error stack at all on failure,
so there is no point using it instead of FIPS_mode.
This commit separates X509VerifyResult::OK from the rest
of the codes that actually represent errors, using
a Result type as usual.
Using a struct improves navigation of the build script,
as we can rely on rust-analyzer to help us check how
a feature flag or an environment variable is used,
as opposed to grepping for multiple env::var calls
or #[cfg] attributes.

This commit also removes some obsolete blocks of code
related to the now defunct ndk-old-gcc and fuzzing features.
The current directory from a build script executed by cargo
is always the manifest dir, so we may as well only use
the manifest dir.
This means BORING_SSL_PRECOMPILED_BCM_O is now
BORING_BSSL_PRECOMPILED_BCM_O.

Prefix BORING_BSSL_ has been chosen because that's the
one that is used the most among all the variables
the build script uses.
Builds using feature fips or fips-link-precompiled now
read variables prefixed by BORING_BSSL_FIPS_ instead of
BORING_BSSL_. This helps complex builds where build dependencies
also use boring, where we may not want to use fips there.

Without those separate variables, the boring build for the
build dependencies end up relying on e.g. BORING_BSSL_PATH,
causing errors if this path is a boring checkout intended for
fips builds, while the fips feature isn't enabled for
the build dependency.
Feature rpk in boring doesn't do anything unless you
explicitly use `SslAcceptor::rpk` or `SslContext::rpk_builder`,
and neither of these types are directly reachable if the
user depends only on tokio-boring or hyper-boring, which
means you still need to explicitly depend on the boring crate
to use RPK, in which case you can enable the feature there.
Feature no-patches is ever only useful when setting other env variables
BORING_BSSL{,_FIPS}{,_SOURCE}_PATH, and it has no impact on the APIs
provided by any of the boring crates, so we may as well make it an env
variable itself so downstream users have less features to propagate
across their own crate graph.
nox and others added 23 commits January 3, 2024 19:37
For now it has a single associated constant, X509Flags::TRUSTED_FIRST.
This feature expects a recent boringssl checkout (such as the one
found in boring-sys/deps/boringssl), so it should not be using
the same bindings as the fips feature, which are based on
boring-sys/deps/boringssl-fips, which is older and with a different
API.
We need to add `/build/crypto` and `/build/ssl` to the library search
path to handle the case where we pass `BORING_BSSL_SOURCE_PATH` when
building without enabling any fips features. Otherwise, non bazel
commits will not work because `/build/` itself will not contain any
crypto libraries to link with
When passing BORING_BSSL_FIPS_PATH, you need to add /lib/ to the search
path, and when passing BORING_BSSL_PATH you need to add /crypto/ and
/ssl/ to the search path.
While setting curves should be restricted by the kx-safe-default
feature, reading the curve is allowed.
When establishing new TLS sessions, servers may send multiple session
tickets (RFC8446 4.6.1). hyper-boring caches tickets without placing a
limit on how many tickets are cached. This leads to unbounded growth of
hyper-boring's cache and leaves clients vulnerable to malicious servers
who might send many session tickets to exhaust a client's available
memory.

This change bounds the cache to a default of 8 tickets.
@jrose-signal jrose-signal changed the title Merge upstream v4.0.0 into libsignal branch Merge upstream v4.6.0 into libsignal branch Apr 10, 2024
chore: Release boring-sys version 4.6.0
@jrose-signal
Copy link
Author

Diff-of-diffs reviewed by @moiseev-signal. However, we shouldn't actually merge this into the libsignal branch, because past versions of libsignal depend on that branch, which means we can't put source-breaking changes in it without breaking cargo update. Instead, we're going to (1) make this the new main branch, (2) tag it as signal-v4.6.0, (3) update in libsignal by pointing to the tag instead of a branch, and (4) leaving a note in the libsignal branch about why it's not getting updated anymore.

@jrose-signal jrose-signal deleted the jrose/merge-v4.0 branch April 11, 2024 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.