From ba389040de82947e764600c679894e4f6d2a9c2d Mon Sep 17 00:00:00 2001 From: DebugSteven Date: Sun, 3 Mar 2019 17:09:34 -0700 Subject: [PATCH 1/7] implement Zeroize for Scalar and MontgomeryPoint --- Cargo.toml | 4 +++- build.rs | 2 ++ src/lib.rs | 2 ++ src/montgomery.rs | 12 ++++++++++-- src/scalar.rs | 8 ++++++++ 5 files changed, 25 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4be6b1d..f320f49 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,6 +48,7 @@ clear_on_drop = "=0.2.3" subtle = { version = "2.0.0-pre.0", default-features = false } serde = { version = "1.0", optional = true } packed_simd = { version = "0.3.0", features = ["into_bits"], optional = true } +zeroize = { version = "0.5.2", default-features = false } [build-dependencies] rand = { version = "0.6.0", default-features = false } @@ -57,9 +58,10 @@ clear_on_drop = "=0.2.3" subtle = { version = "2.0.0-pre.0", default-features = false } serde = { version = "1.0", optional = true } packed_simd = { version = "0.3.0", features = ["into_bits"], optional = true } +zeroize = { version = "0.5.2", default-features = false } [features] -nightly = ["subtle/nightly", "clear_on_drop/nightly"] +nightly = ["subtle/nightly", "clear_on_drop/nightly", "zeroize/nightly"] default = ["std", "u64_backend"] std = ["alloc", "subtle/std", "rand/std"] alloc = [] diff --git a/build.rs b/build.rs index 284bce7..8747509 100644 --- a/build.rs +++ b/build.rs @@ -16,6 +16,8 @@ extern crate subtle; #[cfg(all(feature = "nightly", feature = "avx2_backend"))] extern crate packed_simd; +extern crate zeroize; + use std::env; use std::fs::File; use std::io::Write; diff --git a/src/lib.rs b/src/lib.rs index 4f52c96..3ab18d9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -54,6 +54,8 @@ extern crate serde; #[cfg(all(test, feature = "serde"))] extern crate bincode; +extern crate zeroize; + // Internal macros. Must come first! #[macro_use] pub(crate) mod macros; diff --git a/src/montgomery.rs b/src/montgomery.rs index da3c9a5..03913d6 100644 --- a/src/montgomery.rs +++ b/src/montgomery.rs @@ -17,7 +17,7 @@ //! Montgomery arithmetic works not on the curve itself, but on the //! \\(u\\)-line, which discards sign information and unifies the curve //! and its quadratic twist. See [_Montgomery curves and their -//! arithmetic_][costello-smith] by Costello and Smith for more details. +//! arithmetic_][costello-smith] by Costello and Smith for more details. //! //! The `MontgomeryPoint` struct contains the affine \\(u\\)-coordinate //! \\(u\_0(P)\\) of a point \\(P\\) on either the curve or the twist. @@ -61,6 +61,8 @@ use subtle::Choice; use subtle::ConditionallySelectable; use subtle::ConstantTimeEq; +use zeroize::Zeroize; + /// Holds the \\(u\\)-coordinate of a point on the Montgomery form of /// Curve25519 or its twist. #[derive(Copy, Clone, Debug)] @@ -90,6 +92,12 @@ impl PartialEq for MontgomeryPoint { impl Eq for MontgomeryPoint {} +impl Zeroize for MontgomeryPoint { + fn zeroize(&mut self) { + self.0.zeroize(); + } +} + impl MontgomeryPoint { /// View this `MontgomeryPoint` as an array of bytes. pub fn as_bytes<'a>(&'a self) -> &'a [u8; 32] { @@ -335,7 +343,7 @@ mod test { #[test] fn montgomery_to_edwards_rejects_twist() { let one = FieldElement::one(); - + // u = 2 corresponds to a point on the twist. let two = MontgomeryPoint((&one+&one).to_bytes()); diff --git a/src/scalar.rs b/src/scalar.rs index 2e4d256..826bd8d 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -160,6 +160,8 @@ use subtle::Choice; use subtle::ConditionallySelectable; use subtle::ConstantTimeEq; +use zeroize::Zeroize; + use backend; use constants; @@ -502,6 +504,12 @@ impl From for Scalar { } } +impl Zeroize for Scalar { + fn zeroize(&mut self) { + self.bytes.zeroize(); + } +} + impl Scalar { /// Return a `Scalar` chosen uniformly at random using a user-provided RNG. /// From 0709a27e46e6837eba2b7e512b4460501f1cccea Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Sat, 26 Oct 2019 04:26:59 +0000 Subject: [PATCH 2/7] Remove test for validity of randomly generated RistrettoPoints. This test is no longer necessary as it originally had a possibility of failure due to the original implementation of the invsqrt() function in the decompression algorithm. The failure with the current API is nonexistent. * FIXES #288. --- src/ristretto.rs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/ristretto.rs b/src/ristretto.rs index 6d53e89..becd8f9 100644 --- a/src/ristretto.rs +++ b/src/ristretto.rs @@ -1322,19 +1322,6 @@ mod test { } } - #[cfg(feature = "rand")] - #[test] - fn random_is_valid() { - let mut rng = OsRng::new().unwrap(); - for _ in 0..100 { - let P = RistrettoPoint::random(&mut rng); - // Check that P is on the curve - assert!(P.0.is_valid()); - // Check that P is in the image of the ristretto map - P.compress(); - } - } - #[test] fn vartime_precomputed_vs_nonprecomputed_multiscalar() { let mut rng = rand::thread_rng(); From ae4bf40e28bddee0f3a6a6b3d7492874c24c2e54 Mon Sep 17 00:00:00 2001 From: Trangar Date: Mon, 28 Oct 2019 11:48:33 +0100 Subject: [PATCH 3/7] Added constants to reduce recalculations in elligator_risteretto_flavor --- src/backend/serial/u32/constants.rs | 15 +++++++++++++++ src/backend/serial/u64/constants.rs | 27 +++++++++++++++++++++++++++ src/ristretto.rs | 10 ++++++---- 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/backend/serial/u32/constants.rs b/src/backend/serial/u32/constants.rs index 559e141..73f353f 100644 --- a/src/backend/serial/u32/constants.rs +++ b/src/backend/serial/u32/constants.rs @@ -18,6 +18,11 @@ use backend::serial::u32::scalar::Scalar29; use edwards::{EdwardsBasepointTable, EdwardsPoint}; use window::{LookupTable, NafLookupTable8}; +/// The value of minus one, equal to `-&FieldElement::one()` +pub(crate) const MINUS_ONE: FieldElement2625 = FieldElement2625([ + 67108844, 33554431, 67108863, 33554431, 67108863, 33554431, 67108863, 33554431, 67108863, 33554431 +]); + /// Edwards `d` value, equal to `-121665/121666 mod p`. pub(crate) const EDWARDS_D: FieldElement2625 = FieldElement2625([ 56195235, 13857412, 51736253, 6949390, 114729, 24766616, 60832955, 30306712, 48412415, 21499315, @@ -28,6 +33,16 @@ pub(crate) const EDWARDS_D2: FieldElement2625 = FieldElement2625([ 45281625, 27714825, 36363642, 13898781, 229458, 15978800, 54557047, 27058993, 29715967, 9444199, ]); +/// One minus edwards `d` value squared, equal to `(1 - (-121665/121666) mod p) pow 2` +pub(crate) const ONE_MINUS_EDWARDS_D_SQUARED: FieldElement2625 = FieldElement2625([ + 6275446, 16937061, 44170319, 29780721, 11667076, 7397348, 39186143, 1766194, 42675006, 672202 +]); + +/// Edwards `d` value minus one squared, equal to `(((-121665/121666) mod p) - 1) pow 2` +pub(crate) const EDWARDS_D_MINUS_ONE_SQUARED: FieldElement2625 = FieldElement2625([ + 15551776, 22456977, 53683765, 23429360, 55212328, 10178283, 40474537, 4729243, 61826754, 23438029 +]); + /// `= sqrt(a*d - 1)`, where `a = -1 (mod p)`, `d` are the Edwards curve parameters. pub(crate) const SQRT_AD_MINUS_ONE: FieldElement2625 = FieldElement2625([ 24849947, 33400850, 43495378, 6347714, 46036536, 32887293, 41837720, 18186727, 66238516, diff --git a/src/backend/serial/u64/constants.rs b/src/backend/serial/u64/constants.rs index 46bc00e..fc8c6da 100644 --- a/src/backend/serial/u64/constants.rs +++ b/src/backend/serial/u64/constants.rs @@ -16,6 +16,15 @@ use backend::serial::u64::scalar::Scalar52; use edwards::{EdwardsBasepointTable, EdwardsPoint}; use window::{LookupTable, NafLookupTable8}; +/// The value of minus one, equal to `-&FieldElement::one()` +pub(crate) const MINUS_ONE: FieldElement51 = FieldElement51([ + 2251799813685228, + 2251799813685247, + 2251799813685247, + 2251799813685247, + 2251799813685247 +]); + /// Edwards `d` value, equal to `-121665/121666 mod p`. pub(crate) const EDWARDS_D: FieldElement51 = FieldElement51([ 929955233495203, @@ -34,6 +43,24 @@ pub(crate) const EDWARDS_D2: FieldElement51 = FieldElement51([ 633789495995903, ]); +/// One minus edwards `d` value squared, equal to `(1 - (-121665/121666) mod p) pow 2` +pub(crate) const ONE_MINUS_EDWARDS_D_SQUARED: FieldElement51 = FieldElement51([ + 1136626929484150, + 1998550399581263, + 496427632559748, + 118527312129759, + 45110755273534 +]); + +/// Edwards `d` value minus one squared, equal to `(((-121665/121666) mod p) - 1) pow 2` +pub(crate) const EDWARDS_D_MINUS_ONE_SQUARED: FieldElement51 = FieldElement51([ + 1507062230895904, + 1572317787530805, + 683053064812840, + 317374165784489, + 1572899562415810 +]); + /// `= sqrt(a*d - 1)`, where `a = -1 (mod p)`, `d` are the Edwards curve parameters. pub(crate) const SQRT_AD_MINUS_ONE: FieldElement51 = FieldElement51([ 2241493124984347, diff --git a/src/ristretto.rs b/src/ristretto.rs index 6d53e89..818da95 100644 --- a/src/ristretto.rs +++ b/src/ristretto.rs @@ -598,14 +598,16 @@ impl RistrettoPoint { /// This method is not public because it's just used for hashing /// to a point -- proper elligator support is deferred for now. pub(crate) fn elligator_ristretto_flavor(r_0: &FieldElement) -> RistrettoPoint { - let (i, d) = (&constants::SQRT_M1, &constants::EDWARDS_D); + let i = &constants::SQRT_M1; + let d = &constants::EDWARDS_D; + let one_minus_d_sq = &constants::ONE_MINUS_EDWARDS_D_SQUARED; + let d_minus_one_sq = &constants::EDWARDS_D_MINUS_ONE_SQUARED; + let mut c = constants::MINUS_ONE; + let one = FieldElement::one(); - let one_minus_d_sq = &one - &d.square(); - let d_minus_one_sq = (d - &one).square(); let r = i * &r_0.square(); let N_s = &(&r + &one) * &one_minus_d_sq; - let mut c = -&one; let D = &(&c - &(d * &r)) * &(&r + d); let (Ns_D_is_sq, mut s) = FieldElement::sqrt_ratio_i(&N_s, &D); From 409ebd94c011472cb2d24bd4f957448d52065ab6 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Mon, 28 Oct 2019 18:00:50 +0000 Subject: [PATCH 4/7] Remove dev-dependency on deprecated rand_os crate. The functionality we were using is now contained in the `rand_core` crate, which we already depend upon. As far as testing code goes, only benchmarks still depend upon `rand`, as they use `thread_rng`. --- Cargo.toml | 1 - benches/dalek_benchmarks.rs | 4 ++-- src/lib.rs | 2 -- src/montgomery.rs | 6 ++---- src/ristretto.rs | 18 +++++++----------- src/scalar.rs | 6 +++--- 6 files changed, 14 insertions(+), 23 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 719a1e1..3b3a480 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,7 +26,6 @@ features = ["nightly", "simd_backend"] travis-ci = { repository = "dalek-cryptography/curve25519-dalek", branch = "master"} [dev-dependencies] -rand_os = "0.2" sha2 = { version = "0.8", default-features = false } bincode = "1" criterion = "0.2" diff --git a/benches/dalek_benchmarks.rs b/benches/dalek_benchmarks.rs index 4518446..89e8e62 100644 --- a/benches/dalek_benchmarks.rs +++ b/benches/dalek_benchmarks.rs @@ -247,7 +247,7 @@ mod ristretto_benches { c.bench_function_over_inputs( "Batch Ristretto double-and-encode", |b, &&size| { - let mut rng = OsRng::new().unwrap(); + let mut rng = OsRng; let points: Vec = (0..size) .map(|_| RistrettoPoint::random(&mut rng)) .collect(); @@ -299,7 +299,7 @@ mod scalar_benches { c.bench_function_over_inputs( "Batch scalar inversion", |b, &&size| { - let mut rng = OsRng::new().unwrap(); + let mut rng = OsRng; let scalars: Vec = (0..size).map(|_| Scalar::random(&mut rng)).collect(); b.iter(|| { let mut s = scalars.clone(); diff --git a/src/lib.rs b/src/lib.rs index 0216628..de30492 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -44,8 +44,6 @@ extern crate packed_simd; extern crate byteorder; pub extern crate digest; extern crate rand_core; -#[cfg(test)] -extern crate rand_os; extern crate zeroize; // Used for traits related to constant-time code. diff --git a/src/montgomery.rs b/src/montgomery.rs index a89de22..75a45fb 100644 --- a/src/montgomery.rs +++ b/src/montgomery.rs @@ -310,8 +310,7 @@ mod test { use constants; use super::*; - #[cfg(feature = "rand")] - use rand_os::OsRng; + use rand_core::OsRng; #[test] #[cfg(feature = "serde")] @@ -380,10 +379,9 @@ mod test { assert_eq!(u18, u18_unred); } - #[cfg(feature = "rand")] #[test] fn montgomery_ladder_matches_edwards_scalarmult() { - let mut csprng: OsRng = OsRng::new().unwrap(); + let mut csprng: OsRng = OsRng; let s: Scalar = Scalar::random(&mut csprng); let p_edwards: EdwardsPoint = &constants::ED25519_BASEPOINT_TABLE * &s; diff --git a/src/ristretto.rs b/src/ristretto.rs index 4ac2b2a..c4b6170 100644 --- a/src/ristretto.rs +++ b/src/ristretto.rs @@ -485,13 +485,13 @@ impl RistrettoPoint { /// ``` /// # extern crate curve25519_dalek; /// # use curve25519_dalek::ristretto::RistrettoPoint; - /// extern crate rand_os; - /// use rand_os::OsRng; + /// extern crate rand_core; + /// use rand_core::OsRng; /// /// # // Need fn main() here in comment so the doctest compiles /// # // See https://doc.rust-lang.org/book/documentation.html#documentation-as-tests /// # fn main() { - /// let mut rng = OsRng::new().unwrap(); + /// let mut rng = OsRng; /// let points: Vec = /// (0..32).map(|_| RistrettoPoint::random(&mut rng)).collect(); /// @@ -1084,8 +1084,7 @@ impl Debug for RistrettoPoint { #[cfg(test)] mod test { - #[cfg(feature = "rand")] - use rand_os::OsRng; + use rand_core::OsRng; use scalar::Scalar; use constants; @@ -1233,10 +1232,9 @@ mod test { } } - #[cfg(feature = "rand")] #[test] fn four_torsion_random() { - let mut rng = OsRng::new().unwrap(); + let mut rng = OsRng; let B = &constants::RISTRETTO_BASEPOINT_TABLE; let P = B * &Scalar::random(&mut rng); let P_coset = P.coset4(); @@ -1296,10 +1294,9 @@ mod test { } } - #[cfg(feature = "rand")] #[test] fn random_roundtrip() { - let mut rng = OsRng::new().unwrap(); + let mut rng = OsRng; let B = &constants::RISTRETTO_BASEPOINT_TABLE; for _ in 0..100 { let P = B * &Scalar::random(&mut rng); @@ -1309,10 +1306,9 @@ mod test { } } - #[cfg(feature = "rand")] #[test] fn double_and_compress_1024_random_points() { - let mut rng = OsRng::new().unwrap(); + let mut rng = OsRng; let points: Vec = (0..1024).map(|_| RistrettoPoint::random(&mut rng)).collect(); diff --git a/src/scalar.rs b/src/scalar.rs index ab2a6d6..bb50363 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -536,15 +536,15 @@ impl Scalar { /// # Example /// /// ``` - /// extern crate rand_os; + /// extern crate rand_core; /// # extern crate curve25519_dalek; /// # /// # fn main() { /// use curve25519_dalek::scalar::Scalar; /// - /// use rand_os::OsRng; + /// use rand_core::OsRng; /// - /// let mut csprng: OsRng = OsRng::new().unwrap(); + /// let mut csprng = OsRng; /// let a: Scalar = Scalar::random(&mut csprng); /// # } pub fn random(rng: &mut R) -> Self { From 4423394ed8d6c6aaad2249b29a6c8f62cdde18a0 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Mon, 28 Oct 2019 18:10:30 +0000 Subject: [PATCH 5/7] Fix warning that a temporary value isn't used. --- src/backend/serial/scalar_mul/variable_base.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/serial/scalar_mul/variable_base.rs b/src/backend/serial/scalar_mul/variable_base.rs index 2471375..c69e48c 100644 --- a/src/backend/serial/scalar_mul/variable_base.rs +++ b/src/backend/serial/scalar_mul/variable_base.rs @@ -25,7 +25,7 @@ pub(crate) fn mul(point: &EdwardsPoint, scalar: &Scalar) -> EdwardsPoint { // We sum right-to-left. // Unwrap first loop iteration to save computing 16*identity - let mut tmp2 = ProjectivePoint::identity(); + let mut tmp2; let mut tmp3 = EdwardsPoint::identity(); let mut tmp1 = &tmp3 + &lookup_table.select(scalar_digits[63]); // Now tmp1 = s_63*P in P1xP1 coords From 2b51978553dcb3a9ff27e3bf895b59671b8bd9f7 Mon Sep 17 00:00:00 2001 From: Henry de Valence Date: Fri, 15 Nov 2019 14:29:14 -0800 Subject: [PATCH 6/7] Remove nightly recommendation now that subtle has stable opt barriers. This bumps the required `subtle` version to `2.2.1`, which contains those changes. --- Cargo.toml | 2 +- README.md | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3b3a480..d63e79a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,7 +39,7 @@ harness = false rand_core = { version = "0.5", default-features = false } byteorder = { version = "^1.2.3", default-features = false, features = ["i128"] } digest = { version = "0.8", default-features = false } -subtle = { version = "2", default-features = false } +subtle = { version = "^2.2.1", default-features = false } serde = { version = "1.0", default-features = false, optional = true, features = ["derive"] } packed_simd = { version = "0.3", features = ["into_bits"], optional = true } zeroize = { version = "1", default-features = false } diff --git a/README.md b/README.md index df534c7..e3363f5 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,8 @@ See `CHANGELOG.md` for more details. # Backends and Features The `nightly` feature enables features available only when using a Rust nightly -compiler. **It is recommended for security**. +compiler. In particular, it is required for rendering documentation and for +the SIMD backends. Curve arithmetic is implemented using one of the following backends: @@ -103,11 +104,10 @@ unless specifically marked as being variable-time code. We believe that our constant-time logic is lowered to constant-time assembly, at least on `x86_64` targets. -As an additional guard against possible future compiler optimizations, the -`nightly` feature places an optimization barrier before every +As an additional guard against possible future compiler optimizations, +the `subtle` crate places an optimization barrier before every conditional move or assignment. More details can be found in [the -documentation for the `subtle` crate][subtle_doc]. This is -recommended, but not required. +documentation for the `subtle` crate][subtle_doc]. Some functionality (e.g., multiscalar multiplication or batch inversion) requires heap allocation for temporary buffers. All @@ -124,9 +124,9 @@ functions in other crates. The implementation is memory-safe, and contains no significant `unsafe` code. The SIMD backend uses `unsafe` internally to call SIMD -intrinsics. These are marked `unsafe` because invoking them on an +intrinsics. These are marked `unsafe` only because invoking them on an inappropriate CPU would cause `SIGILL`, but the entire backend is only -compiled with appropriate `target_feature`s. +compiled with appropriate `target_feature`s, so this cannot occur. # Performance From 69d72f92d60f8c59adb4bee036c5989061183918 Mon Sep 17 00:00:00 2001 From: Henry de Valence Date: Fri, 15 Nov 2019 15:00:15 -0800 Subject: [PATCH 7/7] Bump version to 2.0.0-alpha.2 and update changelog. --- CHANGELOG.md | 6 ++++++ Cargo.toml | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2c2fbc..2652228 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ Entries are listed in reverse chronological order. +## 2.0.0-alpha.2 + +* Require `subtle = ^2.2.1` and remove the note advising nightly Rust, which is + no longer required as of that version of `subtle`. See the `subtle` + changelog for more details. + ## 2.0.0-alpha.1 * Update `README.md` for `2.x` series. diff --git a/Cargo.toml b/Cargo.toml index d63e79a..a20a43c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "curve25519-dalek" -version = "2.0.0-alpha.1" +version = "2.0.0-alpha.2" authors = ["Isis Lovecruft ", "Henry de Valence "] readme = "README.md"