diff --git a/curve25519-dalek/Cargo.toml b/curve25519-dalek/Cargo.toml index 2da4d3b33..57ebefa00 100644 --- a/curve25519-dalek/Cargo.toml +++ b/curve25519-dalek/Cargo.toml @@ -50,7 +50,7 @@ required-features = ["alloc", "rand_core"] cfg-if = "1" rand_core = { version = "0.6.4", default-features = false, optional = true } digest = { version = "0.10", default-features = false, optional = true } -subtle = { version = "2.3.0", default-features = false } +subtle = { version = "2.4", default-features = false } serde = { version = "1.0", default-features = false, optional = true, features = ["derive"] } zeroize = { version = "1", default-features = false, optional = true } diff --git a/curve25519-dalek/benches/dalek_benchmarks.rs b/curve25519-dalek/benches/dalek_benchmarks.rs index 9148faafe..0f8f6fe8a 100644 --- a/curve25519-dalek/benches/dalek_benchmarks.rs +++ b/curve25519-dalek/benches/dalek_benchmarks.rs @@ -1,6 +1,6 @@ #![allow(non_snake_case)] -use rand::{rngs::OsRng, thread_rng}; +use rand::{thread_rng, Rng}; use criterion::{ criterion_main, measurement::Measurement, BatchSize, BenchmarkGroup, BenchmarkId, Criterion, @@ -249,7 +249,7 @@ mod ristretto_benches { BenchmarkId::new("Batch Ristretto double-and-encode", *batch_size), &batch_size, |b, &&size| { - let mut rng = OsRng; + let mut rng = thread_rng(); let points: Vec = (0..size) .map(|_| RistrettoPoint::random(&mut rng)) .collect(); @@ -336,7 +336,7 @@ mod scalar_benches { BenchmarkId::new("Batch scalar inversion", *batch_size), &batch_size, |b, &&size| { - let mut rng = OsRng; + let mut rng = thread_rng(); let scalars: Vec = (0..size).map(|_| Scalar::random(&mut rng)).collect(); b.iter(|| { @@ -347,6 +347,24 @@ mod scalar_benches { ); } } + fn scalar_from_canonical_bytes(c: &mut BenchmarkGroup) { + let bytes = [0xFF; 32]; + c.bench_function("Max scalar from_canonical_bytes", move |b| { + b.iter(|| Scalar::from_canonical_bytes(bytes)) + }); + let bytes = [0u8; 32]; + c.bench_function("Zero scalar from_canonical_bytes", move |b| { + b.iter(|| Scalar::from_canonical_bytes(bytes)) + }); + c.bench_function("Rand scalar from_canonical_bytes", |bench| { + let mut rng = thread_rng(); + bench.iter_batched( + || rng.gen(), + Scalar::from_canonical_bytes, + BatchSize::SmallInput, + ); + }); + } pub(crate) fn scalar_benches() { let mut c = Criterion::default(); @@ -354,6 +372,7 @@ mod scalar_benches { scalar_arith(&mut g); batch_scalar_inversion(&mut g); + scalar_from_canonical_bytes(&mut g); } } diff --git a/curve25519-dalek/src/scalar.rs b/curve25519-dalek/src/scalar.rs index 6634c88fc..d4ab4e4b2 100644 --- a/curve25519-dalek/src/scalar.rs +++ b/curve25519-dalek/src/scalar.rs @@ -132,10 +132,8 @@ use digest::generic_array::typenum::U64; #[cfg(feature = "digest")] use digest::Digest; -use subtle::Choice; -use subtle::ConditionallySelectable; -use subtle::ConstantTimeEq; -use subtle::CtOption; +use subtle::{Choice, CtOption}; +use subtle::{ConditionallySelectable, ConstantTimeEq, ConstantTimeGreater}; #[cfg(feature = "zeroize")] use zeroize::Zeroize; @@ -253,9 +251,8 @@ impl Scalar { /// if `bytes` is a canonical byte representation modulo the group order \\( \ell \\); /// - `None` if `bytes` is not a canonical byte representation. pub fn from_canonical_bytes(bytes: [u8; 32]) -> CtOption { - let high_bit_unset = (bytes[31] >> 7).ct_eq(&0); let candidate = Scalar { bytes }; - CtOption::new(candidate, high_bit_unset & candidate.is_canonical()) + CtOption::new(candidate, candidate.is_canonical()) } /// Construct a `Scalar` from the low 255 bits of a 256-bit integer. This breaks the invariant @@ -1125,7 +1122,15 @@ impl Scalar { /// Check whether this `Scalar` is the canonical representative mod \\(\ell\\). This is not /// public because any `Scalar` that is publicly observed is reduced, by scalar invariant #2. fn is_canonical(&self) -> Choice { - self.ct_eq(&self.reduce()) + let mut over = Choice::from(0); + let mut under = Choice::from(0); + for (this, l) in self.unpack().0.iter().zip(&constants::L.0).rev() { + let gt = this.ct_gt(l); + let eq = this.ct_eq(l); + under |= (!gt & !eq) & !over; + over |= gt; + } + under } } @@ -1650,15 +1655,42 @@ pub(crate) mod test { 0, 0, 128, ]; - assert!(bool::from( - Scalar::from_canonical_bytes(canonical_bytes).is_some() - )); - assert!(bool::from( - Scalar::from_canonical_bytes(non_canonical_bytes_because_unreduced).is_none() - )); - assert!(bool::from( - Scalar::from_canonical_bytes(non_canonical_bytes_because_highbit).is_none() - )); + let canonical_l_minus_one = [ + 237, 211, 245, 92, 26, 99, 18, 88, 214, 156, 247, 162, 222, 249, 222, 20, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 15, + ]; + let canonical_zero = [0u8; 32]; + let canonical_255_minus_1 = [ + 132, 52, 71, 117, 71, 74, 127, 151, 35, 182, 58, 139, 233, 42, 231, 109, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 15, + ]; + let non_canonical_l = [ + 237, 211, 245, 92, 26, 99, 18, 88, 214, 156, 247, 162, 222, 249, 222, 20, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 16, + ]; + let non_canonical_l_plus_one = [ + 237, 211, 245, 92, 26, 99, 18, 88, 214, 156, 247, 162, 222, 249, 222, 20, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 17, + ]; + let non_canonical_full = [0xFF; 32]; + let non_canonical_255_minus_1 = { + let mut non_canonical_255_minus_1 = [0xFF; 32]; + non_canonical_255_minus_1[31] = 0b0111_1111; + non_canonical_255_minus_1 + }; + + let from_canonical_option = |b| Option::::from(Scalar::from_canonical_bytes(b)); + + assert!(from_canonical_option(canonical_bytes).is_some()); + assert!(from_canonical_option(canonical_l_minus_one).is_some()); + assert!(from_canonical_option(canonical_zero).is_some()); + assert!(from_canonical_option(canonical_255_minus_1).is_some()); + assert!(from_canonical_option(non_canonical_bytes_because_unreduced).is_none()); + assert!(from_canonical_option(non_canonical_bytes_because_highbit).is_none()); + assert!(from_canonical_option(non_canonical_l).is_none()); + assert!(from_canonical_option(non_canonical_l_plus_one).is_none()); + assert!(from_canonical_option(non_canonical_full).is_none()); + assert!(from_canonical_option(non_canonical_255_minus_1).is_none()); } #[test]