* curve: add `EdwardsPoint::compress_batch` and `::random`
We've had various requests to implement batch point compression for
`EdwardsPoint`, e.g. #705.
We can leverage `FieldElement::batch_invert` to implement it, which
results in a fairly significant speedup.
The name `EdwardsPoint::compress_batch` has been chosen to match
`RistrettoPoint::double_and_compress_batch`.
For benchmarking, randomized `EdwardsPoint`s have been used. To obtain
these, an inherent `EdwardsPoint::random` has been extracted from the
existing `Group::random` implementation, which uses rejection sampling.
`Group::random` has been updated to call the inherent
`EdwardsPoint::random`. This avoids a `group` dependency just to run the
batch compression benchmarks.
The following benchmark results have been obtained:
edwards benches/EdwardsPoint compression
time: [3.5029 µs 3.5098 µs 3.5171 µs]
edwards benches/Batch EdwardsPoint compression/1
time: [3.6698 µs 3.6758 µs 3.6817 µs]
edwards benches/Batch EdwardsPoint compression/2
time: [3.8410 µs 3.8461 µs 3.8516 µs]
edwards benches/Batch EdwardsPoint compression/4
time: [4.1534 µs 4.1961 µs 4.2558 µs]
edwards benches/Batch EdwardsPoint compression/8
time: [4.8466 µs 4.8533 µs 4.8600 µs]
edwards benches/Batch EdwardsPoint compression/16
time: [6.1216 µs 6.1315 µs 6.1410 µs]
As you can see, it affords a fairly significant speedup, batch
compressing 16 points in less time than the standard point compression
algorithm would take to compress 2 in a row.
The build is currently broken because it's been stabilized:
error: the feature `avx512_target_feature` has been stable since 1.89.0-nightly and no longer requires an attribute to enable
* Make AVX512IFMA opt-in backend
* Updated README to have backend info
* Added entry to changelog
---------
Co-authored-by: Michael Rosenberg <michael@mrosenberg.pub>
Alternative to #659/#661 and #662 which leverages `subtle::Choice` and
`subtle::ConditionallySelectable` as the optimization barriers.
Really the previous masking was there to conditionally add the scalar
field modulus on underflow, so instead of that, we can conditionally
select zero or the modulus using a `Choice` constructed from the
underflow bit.
Replaces the security mitigation added in #659 and #661 for
masking-related timing variability which used an inline `black_box`
using the recently added `subtle::BlackBox` newtype (see
dalek-cryptography/subtle#123)
Internally `BlackBox` uses a volatile read by default (i.e. same
strategy which was used before) or when the `core_hint_black_box`
feature of `subtle` is enabled, it uses `core::hint::black_box`
(whose documentation was recently updated to reflect the nuances of
potential cryptographic use, see rust-lang/rust#126703)
This PR goes ahead and uses `BlackBox` for both `mask` and
`underflow_mask` where previously it was only used on `underflow_mask`.
The general pattern of bitwise masking inside a loop seems worrisome for
the optimizer potentially inserting branches in the future.
Below are godbolt inspections of the generated assembly, which are free
of the `jns` instructions originally spotted in #659/#661:
- 32-bit (read_volatile): https://godbolt.org/z/TKo9fqza4
- 32-bit (hint::black_box): https://godbolt.org/z/caoMxYbET
- 64-bit (read_volatile): https://godbolt.org/z/PM6zKjj1f
- 64-bit (hint::black_box): https://godbolt.org/z/nseaPvdWv
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
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 <alexander.wagner@aisec.fraunhofer.de> and
Lea Themint <lea.thiemt@tum.de> using their DATA tool:
https://github.com/Fraunhofer-AISEC/DATA
Co-authored-by: Tony Arcieri <bascule@gmail.com>
* Fix nightly build
* Add nightly feature constraint so AVX-512 requires either x86 or x86_64
Co-authored-by: Tony Arcieri <bascule@gmail.com>
* fmt
---------
Co-authored-by: Michael Rosenberg <micro@fastmail.com>
Co-authored-by: Tony Arcieri <bascule@gmail.com>
Co-authored-by: Michael Rosenberg <michael@mrosenberg.pub>
* Mark constants::BASEPOINT_ORDER_PRIVATE deprecated from pub API
* Move all BASEPOINT_ORDER use private internally
Co-authored-by: Tony Arcieri <bascule@gmail.com>
* Fix CHANGELOG for 4.1.1
---------
Co-authored-by: Tony Arcieri <bascule@gmail.com>
We have a lot of backend types leaking via the public API, including
e.g. `FieldElement51`:
https://docs.rs/curve25519-dalek/latest/curve25519_dalek/backend/serial/u64/field/struct.FieldElement51.html
At the very least, these types shouldn't be visible in the rustdoc.
This PR hides them from the docs, but ideally we would hide them
completely from the public API (which might technically be considered a
breaking change, but IMO leaking them at all is a bug).
Adds a lints section to the top of lib.rs with the following:
#![warn(
clippy::unwrap_used,
missing_docs,
rust_2018_idioms,
unused_lifetimes,
unused_qualifications
)]
`warn` is used instead of `deny` to prevent the lints from firing during
local development, however we already configure `-D warnings` in CI so
if any lint fails on checked-in code, it will cause a CI failure.
This commit also fixes or explicitly allows any current violations of
these lints. The main ones were:
- `clippy::unwrap_used`: replaces usages of `unwrap` with `expect`
- `rust_2018_idioms`: no implicit lifetimes, which were present on
usages of `core::fmt::Formatter`
There is occasionally [a need](https://github.com/dalek-cryptography/curve25519-dalek/pull/519#issuecomment-1637770888) to multiply a non-prime-order Montgomery point by an integer. There's currently no way to do this, since our only methods are multiplication by `Scalar` (doesn't make sense in the non-prime-order case), and `MontgomeryPoint::mul_base_clamped` clamps the integer before multiplying.
This defines `MontgomeryPoint::mul_bits_be`, which takes a big-endian representation of an integer and multiplies the point by that integer. Its usage is not recommended by default, but it is also not so unsafe as to be gated behind a `hazmat` feature.
