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 4.1.3 #13

Merged
merged 35 commits into from
Jun 19, 2024
Merged

Merge upstream 4.1.3 #13

merged 35 commits into from
Jun 19, 2024

Conversation

jrose-signal
Copy link
Contributor

Clean merge, no changes, libsignal tests continue to pass. However, I had to tweak one of our added benchmarks - see final commit for more info.

nategraf and others added 30 commits October 3, 2023 12:51
The `signature` crate contains unstable, minor version-gated
functionality.

The v2.1 release did not change any of that, and only added new
functionality. So it's safe to relax the requirement for `signature` to
`>=2.0, <2.2`.
Uses `finish_non_exhaustive` in lieu of printing the `secret_key`
component of a `SigningKey`, only showing the corresponding
`verifying_key` field which can be used to identify the public key.

Closes #591
To avoid nightly regressions breaking the build, the CI configuration
has been updated to *only* use nightly for resolving Cargo.lock by using
`cargo update -Z minimal-versions`.

Previously, it was running `cargo check` which would attempt to compile
all of the dependencies and the code, which is why the diagnostic bug
was triggered. By avoiding any kind of code compilation using nightly we
can avoid such regressions in the future.

Additionally, the clippy job has been changed to run on the latest
stable release (1.73.0) rather than nightly, which will prevent future
clippy lints from breaking the build. Instead, they can be addressed
when clippy is updated.
* derive: Bump version to 0.1.1

* Added changelog
This image duplicates the `curve25519-dalek` table entry below.

It also doesn't actually link to anything, making README.md look broken.
Like #582, there is a new release of `signature` (v2.2.0) which contains
no breaking changes from ed25519-dalek's perspective. The main notable
one is it bumps MSRV to 1.60, which so also happens to also be
ed25519-dalek's MSRV.

This commit loosens the version requirement to allow `>=2.0, <2.3` to
allow the `signature` 2.2 series.
* Brought back SigningKey::to_scalar_bytes; added regression test

* Updated SigningKey::to_scalar docs and tests
* Fix nightly build

* Add nightly feature constraint so AVX-512 requires either x86 or x86_64

Co-authored-by: Tony Arcieri <[email protected]>

* fmt

---------

Co-authored-by: Michael Rosenberg <[email protected]>
Co-authored-by: Tony Arcieri <[email protected]>
Co-authored-by: Michael Rosenberg <[email protected]>
Recent nightlies have started emitting a dead code lint
… (#624)

Adds VerifyingKey::to_edwards and a From conversion

See #623
rozbb and others added 5 commits June 18, 2024 19:49
Timing variability of any kind is problematic when working with
potentially secret values such as elliptic curve scalars, and such
issues can potentially leak private keys and other secrets. Such a
problem was recently discovered in `curve25519-dalek`.

The `Scalar52::sub` function contained usage of a mask value inside of a
loop where LLVM saw an opportunity to insert a branch instruction
(`jns` on x86) to conditionally bypass this code section when the mask
value is set to zero, as can be seen in godbolt:

https://godbolt.org/z/PczYj7Pda

A similar problem was recently discovered in the Kyber reference
implementation:

https://groups.google.com/a/list.nist.gov/g/pqc-forum/c/hqbtIGFKIpU/m/cnE3pbueBgAJ

As discussed on that thread, one portable solution, which is also used
in this PR, is to introduce a volatile read as an optimization barrier,
which prevents the compiler from optimizing it away.

The fix can be validated in godbolt here:

https://godbolt.org/z/x8d46Yfah

The problem was discovered and the solution independently verified by
Alexander Wagner <[email protected]> and
Lea Themint <[email protected]> using their DATA tool:

https://github.com/Fraunhofer-AISEC/DATA

Co-authored-by: Tony Arcieri <[email protected]>
Similar security fix to #659, but for the 32-bit backend. See that PR
for more information about the problem. Relevant compiler outputs (thanks to @tarcieri):

Without fix
https://godbolt.org/z/zvaWxzvqv
Notice the `jns` ("jump if not sign") instruction on line 106.

With fix
https://godbolt.org/z/jc9j7eb8E
* Bumped to v4.1.3

* Added recent PRs to changelog
An Elligator benchmark was added for the "lizard" extensions, and it
made sure it was only measuring the Elligator part of the work by
expanding a set of bytes into a FieldElement ahead of time. However,
this requires the FieldElement alias to be pub, and depends on the
elligator_ristretto_flavor helper function being pub as well. Changing
it to test the lizard-based entry point means we're diverging less
from upstream, and it's more realistic anyway.
@jrose-signal jrose-signal mentioned this pull request Jun 18, 2024
@jrose-signal jrose-signal merged commit 7c6d347 into main Jun 19, 2024
48 checks passed
@jrose-signal jrose-signal deleted the jrose/upstream-4.1.3 branch June 19, 2024 00:07
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.