From bafa0f72b5e4da22053ca5e2396e2635eeac9c7a Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Mon, 20 Mar 2023 03:59:11 -0400 Subject: [PATCH 01/21] Removed Scalar::{from_bits, from_bytes_clamped}; all constructible scalars are now reduced mod l --- src/edwards.rs | 56 +++++-- src/montgomery.rs | 30 ++++ src/scalar.rs | 379 ++++++++++++++++++++++------------------------ 3 files changed, 252 insertions(+), 213 deletions(-) diff --git a/src/edwards.rs b/src/edwards.rs index 06ce8ce5d..2bbd20f73 100644 --- a/src/edwards.rs +++ b/src/edwards.rs @@ -728,6 +728,36 @@ impl EdwardsPoint { scalar * constants::ED25519_BASEPOINT_TABLE } } + + /// Scalar multiplication using the low 255 bits of a little-endian 256-bit integer, `clamping` + /// it's value to be in range + /// + /// **n ∈ 2^254 + 8\*{0, 1, 2, 3, . . ., 2^251 − 1}** + /// + /// # Explanation of `clamping` + /// + /// For Curve25519, h = 8, and multiplying by 8 is the same as a binary left-shift by 3 bits. + /// If you take a secret scalar value between 2^251 and 2^252 – 1 and left-shift by 3 bits + /// then you end up with a 255-bit number with the most significant bit set to 1 and + /// the least-significant three bits set to 0. + /// + /// The Curve25519 clamping operation takes **an arbitrary 256-bit random value** and + /// clears the most-significant bit (making it a 255-bit number), sets the next bit, and then + /// clears the 3 least-significant bits. In other words, it directly creates a scalar value that is + /// in the right form and pre-multiplied by the cofactor. + /// + /// See for details + pub fn mul_clamped(self, bytes: [u8; 32]) -> Self { + // This is the only place we construct a Scalar that is not reduced mod l. All our + // multiplication routines are defined up to and including 2^255 - 1, and clamping is + // guaranteed to return something within this range. Further, we don't do any reduction or + // arithmetic with this clamped value, so there's no issues arising from the fact that the + // curve point is not necessarily in the prime-order subgroup. + let s = Scalar { + bytes: crate::scalar::clamp(bytes), + }; + s * self + } } // ------------------------------------------------------------------------ @@ -1289,6 +1319,19 @@ mod test { 0x2b, 0x42, ]); + /// The largest valid scalar (not mod l). Remember for NAF computations, the top bit has to be + // 0. So the largest integer a scalar can hold is 2^255 - 1. Addition and subtraction are + // broken on unreduced scalars. The only thing you can do with this is multiplying with a curve + // point (and actually also scalar-scalar multiplication, but that's just a quirk of our + // implementation). + static LARGEST_UNREDUCED_SCALAR: Scalar = Scalar { + bytes: [ + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0x7f, + ], + }; + /// Test round-trip decompression for the basepoint. #[test] fn basepoint_decompression_compression() { @@ -1470,11 +1513,7 @@ mod test { #[test] fn basepoint_tables_unreduced_scalar() { let P = &constants::ED25519_BASEPOINT_POINT; - let a = Scalar::from_bits([ - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, - 0xFF, 0xFF, 0xFF, 0xFF, - ]); + let a = LARGEST_UNREDUCED_SCALAR; let table_radix16 = EdwardsBasepointTableRadix16::create(P); let table_radix32 = EdwardsBasepointTableRadix32::create(P); @@ -1617,16 +1656,11 @@ mod test { // A single iteration of a consistency check for MSM. #[cfg(feature = "alloc")] fn multiscalar_consistency_iter(n: usize) { - use core::iter; let mut rng = rand::thread_rng(); // Construct random coefficients x0, ..., x_{n-1}, // followed by some extra hardcoded ones. - let xs = (0..n) - .map(|_| Scalar::random(&mut rng)) - // The largest scalar allowed by the type system, 2^255-1 - .chain(iter::once(Scalar::from_bits([0xff; 32]))) - .collect::>(); + let xs = (0..n).map(|_| Scalar::random(&mut rng)).collect::>(); let check = xs.iter().map(|xi| xi * xi).sum::(); // Construct points G_i = x_i * B diff --git a/src/montgomery.rs b/src/montgomery.rs index 7bb4a294f..92db27b9c 100644 --- a/src/montgomery.rs +++ b/src/montgomery.rs @@ -123,6 +123,36 @@ impl MontgomeryPoint { EdwardsPoint::mul_base(scalar).to_montgomery() } + /// Scalar multiplication using the low 255 bits of a little-endian 256-bit integer, `clamping` + /// it's value to be in range + /// + /// **n ∈ 2^254 + 8\*{0, 1, 2, 3, . . ., 2^251 − 1}** + /// + /// # Explanation of `clamping` + /// + /// For Curve25519, h = 8, and multiplying by 8 is the same as a binary left-shift by 3 bits. + /// If you take a secret scalar value between 2^251 and 2^252 – 1 and left-shift by 3 bits + /// then you end up with a 255-bit number with the most significant bit set to 1 and + /// the least-significant three bits set to 0. + /// + /// The Curve25519 clamping operation takes **an arbitrary 256-bit random value** and + /// clears the most-significant bit (making it a 255-bit number), sets the next bit, and then + /// clears the 3 least-significant bits. In other words, it directly creates a scalar value that is + /// in the right form and pre-multiplied by the cofactor. + /// + /// See for details + pub fn mul_clamped(self, bytes: [u8; 32]) -> Self { + // This is the only place we construct a Scalar that is not reduced mod l. All our + // multiplication routines are defined up to and including 2^255 - 1, and clamping is + // guaranteed to return something within this range. Further, we don't do any reduction or + // arithmetic with this clamped value, so there's no issues arising from the fact that the + // curve point is not necessarily in the prime-order subgroup. + let s = Scalar { + bytes: crate::scalar::clamp(bytes), + }; + s * self + } + /// View this `MontgomeryPoint` as an array of bytes. pub const fn as_bytes(&self) -> &[u8; 32] { &self.0 diff --git a/src/scalar.rs b/src/scalar.rs index 58c71e4d9..89b3b742f 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -110,34 +110,6 @@ //! See also `Scalar::hash_from_bytes` and `Scalar::from_hash` that //! reduces a \\(512\\)-bit integer, if the optional `digest` feature //! has been enabled. -//! -//! Finally, to create a `Scalar` with a specific bit-pattern -//! (e.g., for compatibility with X/Ed25519 -//! ["clamping"](https://github.com/isislovecruft/ed25519-dalek/blob/f790bd2ce/src/ed25519.rs#L349)), -//! use [`Scalar::from_bits`]. This constructs a scalar with exactly -//! the bit pattern given, without any assurances as to reduction -//! modulo the group order: -//! -//! ``` -//! use curve25519_dalek::scalar::Scalar; -//! -//! let l_plus_two_bytes: [u8; 32] = [ -//! 0xef, 0xd3, 0xf5, 0x5c, 0x1a, 0x63, 0x12, 0x58, -//! 0xd6, 0x9c, 0xf7, 0xa2, 0xde, 0xf9, 0xde, 0x14, -//! 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, -//! 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, -//! ]; -//! let a: Scalar = Scalar::from_bits(l_plus_two_bytes); -//! -//! let two: Scalar = Scalar::ONE + Scalar::ONE; -//! -//! assert!(a != two); // the scalar is not reduced (mod l)… -//! assert!(! bool::from(a.is_canonical())); // …and therefore is not canonical. -//! assert!(a.reduce() == two); // if we were to reduce it manually, it would be. -//! ``` -//! -//! The resulting `Scalar` has exactly the specified bit pattern, -//! **except for the highest bit, which will be set to 0**. use core::borrow::Borrow; use core::cmp::{Eq, PartialEq}; @@ -211,8 +183,7 @@ cfg_if! { } } -/// The `Scalar` struct holds an integer \\(s < 2\^{255} \\) which -/// represents an element of \\(\mathbb Z / \ell\\). +/// The `Scalar` struct holds an element of \\(\mathbb Z / \ell\mathbb Z \\). #[allow(clippy::derive_hash_xor_eq)] #[derive(Copy, Clone, Hash)] pub struct Scalar { @@ -221,14 +192,23 @@ pub struct Scalar { /// /// # Invariant /// - /// The integer representing this scalar must be bounded above by \\(2\^{255}\\), or - /// equivalently the high bit of `bytes[31]` must be zero. + /// The integer representing this scalar is less than \\(2\^{255} - 19 \\), i.e., it represents + /// a canonical representative of an element of \\( \mathbb Z / \ell\mathbb Z \\). + /// + /// Note that this implies the high bit of `bytes[31]` is zero, which ensures that there is + /// room for a carry bit when computing a NAF representation. + /// + /// # Note + /// + /// In some unit tests, and one specific use case of the `mul_clamped` for `EdwardsPoint` and + /// `MontgomeryPoint`, this will not be reduced mod l. This is not an issue. We implement curve + /// point scalar multiplication for any choice of `bytes`, so long as the top bit is 0. /// - /// This ensures that there is room for a carry bit when computing a NAF representation. - // - // XXX This is pub(crate) so we can write literal constants. - // Alternatively we could make the Scalar constructors `const fn`s and use those instead. - // See dalek-cryptography/curve25519-dalek#493 + /// This is the only thing you can do safely with an unreduced scalar. Addition and subtraction + /// are NOT correct when using unreduced scalars. Multiplication is correct, but this is only + /// due to a quirk of our implementation, and not guaranteed to hold in general. + /// + /// It is not possible to construct an unreduced `Scalar` from the publicly-facing API. pub(crate) bytes: [u8; 32], } @@ -257,54 +237,13 @@ impl Scalar { /// # Return /// /// - `Some(s)`, where `s` is the `Scalar` corresponding to `bytes`, - /// if `bytes` is a canonical byte representation; + /// 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::from_bits(bytes); + let candidate = Scalar { bytes }; CtOption::new(candidate, high_bit_unset & candidate.is_canonical()) } - - /// Construct a `Scalar` from the low 255 bits of a 256-bit integer. - /// - /// This function is intended for applications like X25519 which - /// require specific bit-patterns when performing scalar - /// multiplication. - pub const fn from_bits(bytes: [u8; 32]) -> Scalar { - let mut s = Scalar { bytes }; - // Ensure that s < 2^255 by masking the high bit - s.bytes[31] &= 0b0111_1111; - - s - } - - /// Construct a `Scalar` from the low 255 bits of a little-endian 256-bit integer - /// `clamping` it's value to be in range - /// - /// **n ∈ 2^254 + 8\*{0, 1, 2, 3, . . ., 2^251 − 1}** - /// - /// # Explanation of `clamping` - /// - /// For Curve25519, h = 8, and multiplying by 8 is the same as a binary left-shift by 3 bits. - /// If you take a secret scalar value between 2^251 and 2^252 – 1 and left-shift by 3 bits - /// then you end up with a 255-bit number with the most significant bit set to 1 and - /// the least-significant three bits set to 0. - /// - /// The Curve25519 clamping operation takes **an arbitrary 256-bit random value** and - /// clears the most-significant bit (making it a 255-bit number), sets the next bit, and then - /// clears the 3 least-significant bits. In other words, it directly creates a scalar value that is - /// in the right form and pre-multiplied by the cofactor. - /// - /// See for details - pub const fn from_bits_clamped(bytes: [u8; 32]) -> Scalar { - let mut s = Scalar { bytes }; - - s.bytes[0] &= 0b1111_1000; - s.bytes[31] &= 0b0111_1111; - s.bytes[31] |= 0b0100_0000; - - s - } } impl Debug for Scalar { @@ -364,15 +303,9 @@ impl<'a, 'b> Add<&'b Scalar> for &'a Scalar { type Output = Scalar; #[allow(non_snake_case)] fn add(self, _rhs: &'b Scalar) -> Scalar { - // The UnpackedScalar::add function produces reduced outputs - // if the inputs are reduced. However, these inputs may not - // be reduced -- they might come from Scalar::from_bits. So - // after computing the sum, we explicitly reduce it mod l - // before repacking. - let sum = UnpackedScalar::add(&self.unpack(), &_rhs.unpack()); - let sum_R = UnpackedScalar::mul_internal(&sum, &constants::R); - let sum_mod_l = UnpackedScalar::montgomery_reduce(&sum_R); - sum_mod_l.pack() + // The UnpackedScalar::add function produces reduced outputs if the inputs are reduced. By + // the Scalar invariant property, this is always the case. + UnpackedScalar::add(&self.unpack(), &_rhs.unpack()).pack() } } @@ -390,16 +323,9 @@ impl<'a, 'b> Sub<&'b Scalar> for &'a Scalar { type Output = Scalar; #[allow(non_snake_case)] fn sub(self, rhs: &'b Scalar) -> Scalar { - // The UnpackedScalar::sub function requires reduced inputs - // and produces reduced output. However, these inputs may not - // be reduced -- they might come from Scalar::from_bits. So - // we explicitly reduce the inputs. - let self_R = UnpackedScalar::mul_internal(&self.unpack(), &constants::R); - let self_mod_l = UnpackedScalar::montgomery_reduce(&self_R); - let rhs_R = UnpackedScalar::mul_internal(&rhs.unpack(), &constants::R); - let rhs_mod_l = UnpackedScalar::montgomery_reduce(&rhs_R); - - UnpackedScalar::sub(&self_mod_l, &rhs_mod_l).pack() + // The UnpackedScalar::sub function produces reduced outputs if the inputs are reduced. By + // the Scalar invariant property, this is always the case. + UnpackedScalar::sub(&self.unpack(), &rhs.unpack()).pack() } } @@ -691,10 +617,13 @@ impl Scalar { /// let s = Scalar::from_hash(h); /// /// println!("{:?}", s.to_bytes()); - /// assert!(s == Scalar::from_bits([ 21, 88, 208, 252, 63, 122, 210, 152, - /// 154, 38, 15, 23, 16, 167, 80, 150, - /// 192, 221, 77, 226, 62, 25, 224, 148, - /// 239, 48, 176, 10, 185, 69, 168, 11, ])); + /// assert!( + /// s.to_bytes(), + /// [ 21, 88, 208, 252, 63, 122, 210, 152, + /// 154, 38, 15, 23, 16, 167, 80, 150, + /// 192, 221, 77, 226, 62, 25, 224, 148, + /// 239, 48, 176, 10, 185, 69, 168, 11, ], + /// )); /// # } /// ``` pub fn from_hash(hash: D) -> Scalar @@ -1153,21 +1082,9 @@ impl Scalar { x_mod_l.pack() } - /// Check whether this `Scalar` is the canonical representative mod \\(\ell\\). - /// - /// ``` - /// # use curve25519_dalek::scalar::Scalar; - /// # use subtle::ConditionallySelectable; - /// # fn main() { - /// // 2^255 - 1, since `from_bits` clears the high bit - /// let _2_255_minus_1 = Scalar::from_bits([0xff;32]); - /// assert!(! bool::from(_2_255_minus_1.is_canonical())); - /// - /// let reduced = _2_255_minus_1.reduce(); - /// assert!(bool::from(reduced.is_canonical())); - /// # } - /// ``` - pub fn is_canonical(&self) -> Choice { + /// Check whether this `Scalar` is the canonical representative mod \\(\ell\\). This is not + /// public because any `Scalar` that is publicly observed is reduced, by the invariant. + fn is_canonical(&self) -> Choice { self.ct_eq(&self.reduce()) } } @@ -1264,6 +1181,31 @@ fn read_le_u64_into(src: &[u8], dst: &mut [u64]) { } } +/// Fixed-base scalar multiplication using the low 255 bits of a little-endian 256-bit integer, +/// `clamping` it's value to be in range +/// +/// **n ∈ 2^254 + 8\*{0, 1, 2, 3, . . ., 2^251 − 1}** +/// +/// # Explanation of `clamping` +/// +/// For Curve25519, h = 8, and multiplying by 8 is the same as a binary left-shift by 3 bits. +/// If you take a secret scalar value between 2^251 and 2^252 – 1 and left-shift by 3 bits +/// then you end up with a 255-bit number with the most significant bit set to 1 and +/// the least-significant three bits set to 0. +/// +/// The Curve25519 clamping operation takes **an arbitrary 256-bit random value** and +/// clears the most-significant bit (making it a 255-bit number), sets the next bit, and then +/// clears the 3 least-significant bits. In other words, it directly creates a scalar value that is +/// in the right form and pre-multiplied by the cofactor. +/// +/// See for details +pub fn clamp(mut bytes: [u8; 32]) -> [u8; 32] { + bytes[0] &= 0b1111_1000; + bytes[31] &= 0b0111_1111; + bytes[31] |= 0b0100_0000; + bytes +} + #[cfg(test)] mod test { use super::*; @@ -1297,6 +1239,19 @@ mod test { ], }; + /// The largest valid scalar (not mod l). Remember for NAF computations, the top bit has to be + // 0. So the largest integer a scalar can hold is 2^255 - 1. You cannot do addition or + // subtraction with this Scalar, since those require inputs to be reduced. You can do + // multiplication though. This property is not relevant for our API, but it is relevant for the + // tests below. + static LARGEST_UNREDUCED_SCALAR: Scalar = Scalar { + bytes: [ + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0x7f, + ], + }; + /// x*y = 5690045403673944803228348699031245560686958845067437804563560795922180092780 static X_TIMES_Y: Scalar = Scalar { bytes: [ @@ -1336,27 +1291,19 @@ mod test { 0, 0, 0, 0, 15, 0, 0, 0, 0, 15, 0, 0, 0, 0, 15, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, ]; - static LARGEST_ED25519_S: Scalar = Scalar { + const BASEPOINT_ORDER_MINUS_ONE: Scalar = Scalar { bytes: [ - 0xf8, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0x7f, + 0xec, 0xd3, 0xf5, 0x5c, 0x1a, 0x63, 0x12, 0x58, 0xd6, 0x9c, 0xf7, 0xa2, 0xde, 0xf9, + 0xde, 0x14, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x10, ], }; - static CANONICAL_LARGEST_ED25519_S_PLUS_ONE: Scalar = Scalar { - bytes: [ - 0x7e, 0x34, 0x47, 0x75, 0x47, 0x4a, 0x7f, 0x97, 0x23, 0xb6, 0x3a, 0x8b, 0xe9, 0x2a, - 0xe7, 0x6d, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0x0f, - ], - }; - - static CANONICAL_LARGEST_ED25519_S_MINUS_ONE: Scalar = Scalar { + static LARGEST_ED25519_S: Scalar = Scalar { bytes: [ - 0x7c, 0x34, 0x47, 0x75, 0x47, 0x4a, 0x7f, 0x97, 0x23, 0xb6, 0x3a, 0x8b, 0xe9, 0x2a, - 0xe7, 0x6d, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0x0f, + 0xf8, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0x7f, ], }; @@ -1460,58 +1407,34 @@ mod test { #[test] fn add_reduces() { - // Check that the addition works - assert_eq!( - (LARGEST_ED25519_S + Scalar::ONE).reduce(), - CANONICAL_LARGEST_ED25519_S_PLUS_ONE - ); - // Check that the addition reduces - assert_eq!( - LARGEST_ED25519_S + Scalar::ONE, - CANONICAL_LARGEST_ED25519_S_PLUS_ONE - ); + // Check that addition wraps around the modulus + assert_eq!(BASEPOINT_ORDER_MINUS_ONE + Scalar::ONE, Scalar::ZERO); } #[test] fn sub_reduces() { - // Check that the subtraction works - assert_eq!( - (LARGEST_ED25519_S - Scalar::ONE).reduce(), - CANONICAL_LARGEST_ED25519_S_MINUS_ONE - ); - // Check that the subtraction reduces - assert_eq!( - LARGEST_ED25519_S - Scalar::ONE, - CANONICAL_LARGEST_ED25519_S_MINUS_ONE - ); + // Check that subtraction wraps around the modulus + assert_eq!(Scalar::ZERO - Scalar::ONE, BASEPOINT_ORDER_MINUS_ONE); } + /* #[test] fn quarkslab_scalar_overflow_does_not_occur() { - // Check that manually-constructing large Scalars with - // from_bits cannot produce incorrect results. + // Check that manually-constructing large Scalars cannot produce incorrect results. // - // The from_bits function is required to implement X/Ed25519, - // while all other methods of constructing a Scalar produce - // reduced Scalars. However, this "invariant loophole" allows - // constructing large scalars which are not reduced mod l. + // The EdwardsPoint::mul_clamped function is required to implement X/Ed25519, while all + // other methods of constructing a Scalar produce reduced Scalars. However, this "invariant + // loophole" allows constructing large scalars which are not reduced mod l. // - // This issue was discovered independently by both Jack - // "str4d" Grigg (issue #238), who noted that reduction was - // not performed on addition, and Laurent Grémy & Nicolas - // Surbayrole of Quarkslab, who noted that it was possible to - // cause an overflow and compute incorrect results. + // This issue was discovered independently by both Jack "str4d" Grigg (issue #238), who + // noted that reduction was not performed on addition, and Laurent Grémy & Nicolas + // Surbayrole of Quarkslab, who noted that it was possible to cause an overflow and compute + // incorrect results. // // This test is adapted from the one suggested by Quarkslab. - let large_bytes = [ - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0x7f, - ]; - - let a = Scalar::from_bytes_mod_order(large_bytes); - let b = Scalar::from_bits(large_bytes); + let a = Scalar::from_bytes_mod_order(LARGEST_VALID_SCALAR.bytes); + let b = LARGEST_VALID_SCALAR; assert_eq!(a, b.reduce()); @@ -1532,6 +1455,7 @@ mod test { assert_eq!(minus_a_3, -a_3); assert_eq!(minus_b_3, -b_3); } + */ #[test] fn impl_add() { @@ -1825,8 +1749,9 @@ mod test { // from the produced representation precisely. let cases = (2..100) .map(|s| Scalar::from(s as u64).invert()) - // The largest unreduced scalar, s = 2^255-1 - .chain(iter::once(Scalar::from_bits([0xff; 32]))); + // The largest unreduced scalar, s = 2^255-1. This is not reduced mod l. Scalar mult + // still works though. + .chain(iter::once(LARGEST_UNREDUCED_SCALAR)); for scalar in cases { test_pippenger_radix_iter(scalar, 6); @@ -1900,37 +1825,87 @@ mod test { #[test] fn test_scalar_clamp() { let input = A_SCALAR.bytes; - let expected = Scalar { - bytes: [ - 0x18, 0x0e, 0x97, 0x8a, 0x90, 0xf6, 0x62, 0x2d, 0x37, 0x47, 0x02, 0x3f, 0x8a, 0xd8, - 0x26, 0x4d, 0xa7, 0x58, 0xaa, 0x1b, 0x88, 0xe0, 0x40, 0xd1, 0x58, 0x9e, 0x7b, 0x7f, - 0x23, 0x76, 0xef, 0x49, - ], - }; - let actual = Scalar::from_bits_clamped(input); + let expected = [ + 0x18, 0x0e, 0x97, 0x8a, 0x90, 0xf6, 0x62, 0x2d, 0x37, 0x47, 0x02, 0x3f, 0x8a, 0xd8, + 0x26, 0x4d, 0xa7, 0x58, 0xaa, 0x1b, 0x88, 0xe0, 0x40, 0xd1, 0x58, 0x9e, 0x7b, 0x7f, + 0x23, 0x76, 0xef, 0x49, + ]; + let actual = clamp(input); assert_eq!(actual, expected); - let expected = Scalar { - bytes: [ - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0x40, - ], - }; - let actual = Scalar::from_bits_clamped([0; 32]); + let expected = [ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0x40, + ]; + let actual = clamp([0; 32]); assert_eq!(expected, actual); - let expected = Scalar { - bytes: [ - 0xf8, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0x7f, - ], - }; - let actual = Scalar::from_bits_clamped([0xff; 32]); + let expected = [ + 0xf8, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0x7f, + ]; + let actual = clamp([0xff; 32]); assert_eq!(actual, expected); - assert_eq!( - LARGEST_ED25519_S.bytes, - Scalar::from_bits_clamped(LARGEST_ED25519_S.bytes).bytes - ) + assert_eq!(LARGEST_ED25519_S.bytes, clamp(LARGEST_ED25519_S.bytes)); + } + + /* + #[test] + fn test_bad_arithmetic1() { + use rand::{RngCore, SeedableRng}; + let mut rng = rand::rngs::StdRng::seed_from_u64(2u64); + + // Make a random clamped scalar + let s = { + let mut bytes = [0u8; 32]; + rng.fill_bytes(&mut bytes); + Scalar::from_bits_clamped(bytes) + }; + + // Make a random point + let point = { + let mut bytes = [0u8; 32]; + rng.fill_bytes(&mut bytes); + crate::edwards::CompressedEdwardsY::from_slice(&bytes) + .unwrap() + .decompress() + .unwrap() + }; + + assert_eq!(s * point, s.reduce() * point); + + //assert_eq!(max_scalar.bytes, reduced.bytes); + } + + #[test] + fn test_bad_arithmetic2() { + use rand::{RngCore, SeedableRng}; + let mut rng = rand::rngs::StdRng::seed_from_u64(2u64); + + // Make two random scalars that are reduced mod l + let (s1, s2) = { + let mut bytes1 = [0u8; 32]; + let mut bytes2 = [0u8; 32]; + rng.fill_bytes(&mut bytes1); + rng.fill_bytes(&mut bytes2); + ( + Scalar::from_bytes_mod_order(bytes1), + Scalar::from_bytes_mod_order(bytes2), + ) + }; + + // Make a random point + let point = { + let mut bytes = [0u8; 32]; + rng.fill_bytes(&mut bytes); + crate::edwards::CompressedEdwardsY::from_slice(&bytes) + .unwrap() + .decompress() + .unwrap() + }; + + assert_eq!((s1 + s2) * point, s1 * point + s2 * point); } + */ } From b2862f2a822d6d5f874c29ae5a47108b21153b5d Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Mon, 20 Mar 2023 04:41:24 -0400 Subject: [PATCH 02/21] Made Scalar::reduce() not pub; fixed test warning --- src/scalar.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/scalar.rs b/src/scalar.rs index 8b57c7ec3..5b65b2599 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -1075,7 +1075,7 @@ impl Scalar { /// Reduce this `Scalar` modulo \\(\ell\\). #[allow(non_snake_case)] - pub fn reduce(&self) -> Scalar { + fn reduce(&self) -> Scalar { let x = self.unpack(); let xR = UnpackedScalar::mul_internal(&x, &constants::R); let x_mod_l = UnpackedScalar::montgomery_reduce(&xR); @@ -1240,10 +1240,11 @@ mod test { }; /// The largest valid scalar (not mod l). Remember for NAF computations, the top bit has to be - // 0. So the largest integer a scalar can hold is 2^255 - 1. You cannot do addition or - // subtraction with this Scalar, since those require inputs to be reduced. You can do - // multiplication though. This property is not relevant for our API, but it is relevant for the - // tests below. + // 0. So the largest integer a scalar can hold is 2^255 - 1. Addition and subtraction are + // broken on unreduced scalars. The only thing you can do with this is multiplying with a curve + // point (and actually also scalar-scalar multiplication, but that's just a quirk of our + // implementation). + #[cfg(feature = "precomputed-tables")] static LARGEST_UNREDUCED_SCALAR: Scalar = Scalar { bytes: [ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, From 9867c6c581443e04bd648e1674562cab3315290c Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Mon, 20 Mar 2023 04:41:42 -0400 Subject: [PATCH 03/21] Added benches for scalar add/sub/mul --- benches/dalek_benchmarks.rs | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/benches/dalek_benchmarks.rs b/benches/dalek_benchmarks.rs index aa16d0f09..9148faafe 100644 --- a/benches/dalek_benchmarks.rs +++ b/benches/dalek_benchmarks.rs @@ -300,11 +300,34 @@ mod montgomery_benches { mod scalar_benches { use super::*; - fn scalar_inversion(c: &mut BenchmarkGroup) { + fn scalar_arith(c: &mut BenchmarkGroup) { + let mut rng = thread_rng(); + c.bench_function("Scalar inversion", |b| { let s = Scalar::from(897987897u64).invert(); b.iter(|| s.invert()); }); + c.bench_function("Scalar addition", |b| { + b.iter_batched( + || (Scalar::random(&mut rng), Scalar::random(&mut rng)), + |(a, b)| a + b, + BatchSize::SmallInput, + ); + }); + c.bench_function("Scalar subtraction", |b| { + b.iter_batched( + || (Scalar::random(&mut rng), Scalar::random(&mut rng)), + |(a, b)| a - b, + BatchSize::SmallInput, + ); + }); + c.bench_function("Scalar multiplication", |b| { + b.iter_batched( + || (Scalar::random(&mut rng), Scalar::random(&mut rng)), + |(a, b)| a * b, + BatchSize::SmallInput, + ); + }); } fn batch_scalar_inversion(c: &mut BenchmarkGroup) { @@ -329,7 +352,7 @@ mod scalar_benches { let mut c = Criterion::default(); let mut g = c.benchmark_group("scalar benches"); - scalar_inversion(&mut g); + scalar_arith(&mut g); batch_scalar_inversion(&mut g); } } From ef4285f2090416b70e5fd58db72a713e2a494541 Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Mon, 20 Mar 2023 04:47:41 -0400 Subject: [PATCH 04/21] Fixed build warning --- src/edwards.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/edwards.rs b/src/edwards.rs index 5242811a0..810bcf5fa 100644 --- a/src/edwards.rs +++ b/src/edwards.rs @@ -1324,6 +1324,7 @@ mod test { // broken on unreduced scalars. The only thing you can do with this is multiplying with a curve // point (and actually also scalar-scalar multiplication, but that's just a quirk of our // implementation). + #[cfg(feature = "precomputed-tables")] static LARGEST_UNREDUCED_SCALAR: Scalar = Scalar { bytes: [ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, From 3d16f1250daf932a97f2cef5e476f9f5bb4fdc9e Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Mon, 20 Mar 2023 04:51:16 -0400 Subject: [PATCH 05/21] Fixed doctests --- src/scalar.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/scalar.rs b/src/scalar.rs index 5b65b2599..86bc3eb25 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -617,13 +617,13 @@ impl Scalar { /// let s = Scalar::from_hash(h); /// /// println!("{:?}", s.to_bytes()); - /// assert!( + /// assert_eq!( /// s.to_bytes(), /// [ 21, 88, 208, 252, 63, 122, 210, 152, /// 154, 38, 15, 23, 16, 167, 80, 150, /// 192, 221, 77, 226, 62, 25, 224, 148, /// 239, 48, 176, 10, 185, 69, 168, 11, ], - /// )); + /// ); /// # } /// ``` pub fn from_hash(hash: D) -> Scalar From a299343892baeb51d39be639f0e9c95fec620e77 Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Mon, 20 Mar 2023 14:32:19 -0400 Subject: [PATCH 06/21] Docs --- src/edwards.rs | 4 ++-- src/montgomery.rs | 4 ++-- src/scalar.rs | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/edwards.rs b/src/edwards.rs index 810bcf5fa..7e614cd46 100644 --- a/src/edwards.rs +++ b/src/edwards.rs @@ -730,11 +730,11 @@ impl EdwardsPoint { } /// Scalar multiplication using the low 255 bits of a little-endian 256-bit integer, `clamping` - /// it's value to be in range + /// its value to be in range /// /// **n ∈ 2^254 + 8\*{0, 1, 2, 3, . . ., 2^251 − 1}** /// - /// # Explanation of `clamping` + /// # Explanation of _clamping_ /// /// For Curve25519, h = 8, and multiplying by 8 is the same as a binary left-shift by 3 bits. /// If you take a secret scalar value between 2^251 and 2^252 – 1 and left-shift by 3 bits diff --git a/src/montgomery.rs b/src/montgomery.rs index 92db27b9c..0f78fd26a 100644 --- a/src/montgomery.rs +++ b/src/montgomery.rs @@ -124,11 +124,11 @@ impl MontgomeryPoint { } /// Scalar multiplication using the low 255 bits of a little-endian 256-bit integer, `clamping` - /// it's value to be in range + /// its value to be in range /// /// **n ∈ 2^254 + 8\*{0, 1, 2, 3, . . ., 2^251 − 1}** /// - /// # Explanation of `clamping` + /// # Explanation of _clamping_ /// /// For Curve25519, h = 8, and multiplying by 8 is the same as a binary left-shift by 3 bits. /// If you take a secret scalar value between 2^251 and 2^252 – 1 and left-shift by 3 bits diff --git a/src/scalar.rs b/src/scalar.rs index 86bc3eb25..62a953646 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -1181,12 +1181,12 @@ fn read_le_u64_into(src: &[u8], dst: &mut [u64]) { } } -/// Fixed-base scalar multiplication using the low 255 bits of a little-endian 256-bit integer, -/// `clamping` it's value to be in range +/// Clamps the given little-endian representation of a 32-byte integer. Clamping the value puts it +/// in the range: /// /// **n ∈ 2^254 + 8\*{0, 1, 2, 3, . . ., 2^251 − 1}** /// -/// # Explanation of `clamping` +/// # Explanation of _clamping_ /// /// For Curve25519, h = 8, and multiplying by 8 is the same as a binary left-shift by 3 bits. /// If you take a secret scalar value between 2^251 and 2^252 – 1 and left-shift by 3 bits From 5d1aae2b391d38c3a3abd7776edbe6499cc1dc8d Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Mon, 20 Mar 2023 15:46:21 -0400 Subject: [PATCH 07/21] Added EdwardsPoint::mul_base_clamped and gated Scalar::from_bits behind legacy_compatibility --- Cargo.toml | 1 + src/edwards.rs | 12 ++++++++++-- src/montgomery.rs | 4 ++-- src/scalar.rs | 26 +++++++++++++++++++++----- 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index dfa53f0d7..c19cc1b47 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -66,6 +66,7 @@ packed_simd = { version = "0.3.4", package = "packed_simd_2", features = ["into_ default = ["alloc", "precomputed-tables", "zeroize"] alloc = ["zeroize?/alloc"] precomputed-tables = [] +legacy_compatibility = [] [profile.dev] opt-level = 2 diff --git a/src/edwards.rs b/src/edwards.rs index 7e614cd46..e0b165251 100644 --- a/src/edwards.rs +++ b/src/edwards.rs @@ -118,7 +118,7 @@ use zeroize::Zeroize; use crate::constants; use crate::field::FieldElement; -use crate::scalar::Scalar; +use crate::scalar::{clamp_integer, Scalar}; use crate::montgomery::MontgomeryPoint; @@ -754,10 +754,18 @@ impl EdwardsPoint { // arithmetic with this clamped value, so there's no issues arising from the fact that the // curve point is not necessarily in the prime-order subgroup. let s = Scalar { - bytes: crate::scalar::clamp(bytes), + bytes: clamp_integer(bytes), }; s * self } + + /// A fixed-base version of [`Self::mul_clamped`]. + pub fn mul_base_clamped(bytes: [u8; 32]) -> Self { + let s = Scalar { + bytes: clamp_integer(bytes), + }; + Self::mul_base(&s) + } } // ------------------------------------------------------------------------ diff --git a/src/montgomery.rs b/src/montgomery.rs index 0f78fd26a..2d888ce02 100644 --- a/src/montgomery.rs +++ b/src/montgomery.rs @@ -57,7 +57,7 @@ use core::{ use crate::constants::{APLUS2_OVER_FOUR, MONTGOMERY_A, MONTGOMERY_A_NEG}; use crate::edwards::{CompressedEdwardsY, EdwardsPoint}; use crate::field::FieldElement; -use crate::scalar::Scalar; +use crate::scalar::{clamp_integer, Scalar}; use crate::traits::Identity; @@ -148,7 +148,7 @@ impl MontgomeryPoint { // arithmetic with this clamped value, so there's no issues arising from the fact that the // curve point is not necessarily in the prime-order subgroup. let s = Scalar { - bytes: crate::scalar::clamp(bytes), + bytes: clamp_integer(bytes), }; s * self } diff --git a/src/scalar.rs b/src/scalar.rs index 62a953646..de187b120 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -244,6 +244,19 @@ impl Scalar { let candidate = Scalar { bytes }; CtOption::new(candidate, high_bit_unset & candidate.is_canonical()) } + + /// Construct a `Scalar` from the low 255 bits of a 256-bit integer. This breaks the invariant + /// that scalars are always reduced. **Scalar arithmetic does not work** on scalars produced + /// from this function. You may only use the output of this for `EdwardsPoint::mul` and + /// `EdwardsPoint::vartime_double_scalar_mul_basepoint` + #[cfg(feature = "legacy_compatibility")] + pub const fn from_bits(bytes: [u8; 32]) -> Scalar { + let mut s = Scalar { bytes }; + // Ensure that s < 2^255 by masking the high bit + s.bytes[31] &= 0b0111_1111; + + s + } } impl Debug for Scalar { @@ -1199,7 +1212,7 @@ fn read_le_u64_into(src: &[u8], dst: &mut [u64]) { /// in the right form and pre-multiplied by the cofactor. /// /// See for details -pub fn clamp(mut bytes: [u8; 32]) -> [u8; 32] { +pub fn clamp_integer(mut bytes: [u8; 32]) -> [u8; 32] { bytes[0] &= 0b1111_1000; bytes[31] &= 0b0111_1111; bytes[31] |= 0b0100_0000; @@ -1831,24 +1844,27 @@ mod test { 0x26, 0x4d, 0xa7, 0x58, 0xaa, 0x1b, 0x88, 0xe0, 0x40, 0xd1, 0x58, 0x9e, 0x7b, 0x7f, 0x23, 0x76, 0xef, 0x49, ]; - let actual = clamp(input); + let actual = clamp_integer(input); assert_eq!(actual, expected); let expected = [ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x40, ]; - let actual = clamp([0; 32]); + let actual = clamp_integer([0; 32]); assert_eq!(expected, actual); let expected = [ 0xf8, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f, ]; - let actual = clamp([0xff; 32]); + let actual = clamp_integer([0xff; 32]); assert_eq!(actual, expected); - assert_eq!(LARGEST_ED25519_S.bytes, clamp(LARGEST_ED25519_S.bytes)); + assert_eq!( + LARGEST_ED25519_S.bytes, + clamp_integer(LARGEST_ED25519_S.bytes) + ); } /* From c170939ba931b5ebf6ab09235a5e98dc14f5bf0b Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Tue, 21 Mar 2023 02:56:27 -0400 Subject: [PATCH 08/21] Added unit test for Mul impl on unreduced Scalars --- src/scalar.rs | 44 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/src/scalar.rs b/src/scalar.rs index de187b120..35db637e5 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -406,7 +406,10 @@ impl<'de> Deserialize<'de> for Scalar { type Value = Scalar; fn expecting(&self, formatter: &mut ::core::fmt::Formatter) -> ::core::fmt::Result { - formatter.write_str("a valid point in Edwards y + sign format") + formatter.write_str( + "a sequence of 32 bytes whose little-endian interpretation is less than the \ + basepoint order ℓ", + ) } fn visit_seq(self, mut seq: A) -> Result @@ -1867,6 +1870,45 @@ mod test { ); } + // Check that a * b == a.reduce() * a.reduce() for ANY scalars a,b, even ones out of range and + // with the high bit set. In old versions of ed25519-dalek, it used to be the case that a was + // reduced and b was clamped an unreduced. This should not affect computation. + #[test] + fn test_mul_reduction_invariance() { + use rand::RngCore; + let mut rng = rand::thread_rng(); + + for _ in 0..10 { + // Also define c that's clamped. We'll make sure that clamping doesn't affect + // computation + let (a, b, c) = { + let mut a_bytes = [0u8; 32]; + let mut b_bytes = [0u8; 32]; + let mut c_bytes = [0u8; 32]; + rng.fill_bytes(&mut a_bytes); + rng.fill_bytes(&mut b_bytes); + rng.fill_bytes(&mut c_bytes); + ( + Scalar { bytes: a_bytes }, + Scalar { bytes: b_bytes }, + Scalar { + bytes: clamp_integer(c_bytes), + }, + ) + }; + + // Make sure this is the same product no matter how you cut it + let reduced_mul_ab = a.reduce() * b.reduce(); + let reduced_mul_ac = a.reduce() * c.reduce(); + assert_eq!(a * b, reduced_mul_ab); + assert_eq!(a.reduce() * b, reduced_mul_ab); + assert_eq!(a * b.reduce(), reduced_mul_ab); + assert_eq!(a * c, reduced_mul_ac); + assert_eq!(a.reduce() * c, reduced_mul_ac); + assert_eq!(a * c.reduce(), reduced_mul_ac); + } + } + /* #[test] fn test_bad_arithmetic1() { From 1934019e3af35bccaa8676a8d98fdcc2ff71104d Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Mon, 27 Mar 2023 01:40:43 -0400 Subject: [PATCH 09/21] Added Montgomery::mul_base_clamped; tweaked docs --- src/edwards.rs | 37 +++++++++++++------------------------ src/montgomery.rs | 43 ++++++++++++++++++++----------------------- src/scalar.rs | 3 ++- 3 files changed, 35 insertions(+), 48 deletions(-) diff --git a/src/edwards.rs b/src/edwards.rs index e0b165251..9c4a9611e 100644 --- a/src/edwards.rs +++ b/src/edwards.rs @@ -729,38 +729,27 @@ impl EdwardsPoint { } } - /// Scalar multiplication using the low 255 bits of a little-endian 256-bit integer, `clamping` - /// its value to be in range - /// - /// **n ∈ 2^254 + 8\*{0, 1, 2, 3, . . ., 2^251 − 1}** - /// - /// # Explanation of _clamping_ - /// - /// For Curve25519, h = 8, and multiplying by 8 is the same as a binary left-shift by 3 bits. - /// If you take a secret scalar value between 2^251 and 2^252 – 1 and left-shift by 3 bits - /// then you end up with a 255-bit number with the most significant bit set to 1 and - /// the least-significant three bits set to 0. - /// - /// The Curve25519 clamping operation takes **an arbitrary 256-bit random value** and - /// clears the most-significant bit (making it a 255-bit number), sets the next bit, and then - /// clears the 3 least-significant bits. In other words, it directly creates a scalar value that is - /// in the right form and pre-multiplied by the cofactor. - /// - /// See for details + /// Multiply this point by `clamp_integer(bytes)`. For a description of clamping, see + /// [`clamp_integer`]. pub fn mul_clamped(self, bytes: [u8; 32]) -> Self { - // This is the only place we construct a Scalar that is not reduced mod l. All our - // multiplication routines are defined up to and including 2^255 - 1, and clamping is - // guaranteed to return something within this range. Further, we don't do any reduction or - // arithmetic with this clamped value, so there's no issues arising from the fact that the - // curve point is not necessarily in the prime-order subgroup. + // We have to construct a Scalar that is not reduced mod l, which breaks its invariant. + // However, all our scalar-point multiplication routines are defined for all values of + // `bytes` up to and including 2^255 - 1, and clamping is guaranteed to return something + // within this range. Further, we don't do any reduction or arithmetic with this clamped + // value, so there's no issues arising from the fact that the curve point is not + // necessarily in the prime-order subgroup. let s = Scalar { bytes: clamp_integer(bytes), }; s * self } - /// A fixed-base version of [`Self::mul_clamped`]. + /// Multiply the basepoint by `clamp_integer(bytes)`. For a description of clamping, see + /// [`clamp_integer`]. pub fn mul_base_clamped(bytes: [u8; 32]) -> Self { + // See reasoning in Self::mul_clamped why it is OK to make an unreduced Scalar here. We + // note that basepoint multiplication is also defined for all values of `bytes` up to and + // including 2^255 - 1. let s = Scalar { bytes: clamp_integer(bytes), }; diff --git a/src/montgomery.rs b/src/montgomery.rs index 2d888ce02..353b0bfe9 100644 --- a/src/montgomery.rs +++ b/src/montgomery.rs @@ -123,36 +123,33 @@ impl MontgomeryPoint { EdwardsPoint::mul_base(scalar).to_montgomery() } - /// Scalar multiplication using the low 255 bits of a little-endian 256-bit integer, `clamping` - /// its value to be in range - /// - /// **n ∈ 2^254 + 8\*{0, 1, 2, 3, . . ., 2^251 − 1}** - /// - /// # Explanation of _clamping_ - /// - /// For Curve25519, h = 8, and multiplying by 8 is the same as a binary left-shift by 3 bits. - /// If you take a secret scalar value between 2^251 and 2^252 – 1 and left-shift by 3 bits - /// then you end up with a 255-bit number with the most significant bit set to 1 and - /// the least-significant three bits set to 0. - /// - /// The Curve25519 clamping operation takes **an arbitrary 256-bit random value** and - /// clears the most-significant bit (making it a 255-bit number), sets the next bit, and then - /// clears the 3 least-significant bits. In other words, it directly creates a scalar value that is - /// in the right form and pre-multiplied by the cofactor. - /// - /// See for details + /// Multiply this point by `clamp_integer(bytes)`. For a description of clamping, see + /// [`clamp_integer`]. pub fn mul_clamped(self, bytes: [u8; 32]) -> Self { - // This is the only place we construct a Scalar that is not reduced mod l. All our - // multiplication routines are defined up to and including 2^255 - 1, and clamping is - // guaranteed to return something within this range. Further, we don't do any reduction or - // arithmetic with this clamped value, so there's no issues arising from the fact that the - // curve point is not necessarily in the prime-order subgroup. + // We have to construct a Scalar that is not reduced mod l, which breaks its invariant. + // However, all our scalar-point multiplication routines are defined for all values of + // `bytes` up to and including 2^255 - 1, and clamping is guaranteed to return something + // within this range. Further, we don't do any reduction or arithmetic with this clamped + // value, so there's no issues arising from the fact that the curve point is not + // necessarily in the prime-order subgroup. let s = Scalar { bytes: clamp_integer(bytes), }; s * self } + /// Multiply the basepoint by `clamp_integer(bytes)`. For a description of clamping, see + /// [`clamp_integer`]. + pub fn mul_base_clamped(bytes: [u8; 32]) -> Self { + // See reasoning in Self::mul_clamped why it is OK to make an unreduced Scalar here. We + // note that basepoint multiplication is also defined for all values of `bytes` up to and + // including 2^255 - 1. + let s = Scalar { + bytes: clamp_integer(bytes), + }; + Self::mul_base(&s) + } + /// View this `MontgomeryPoint` as an array of bytes. pub const fn as_bytes(&self) -> &[u8; 32] { &self.0 diff --git a/src/scalar.rs b/src/scalar.rs index 35db637e5..9ee1583d4 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -1214,7 +1214,8 @@ fn read_le_u64_into(src: &[u8], dst: &mut [u64]) { /// clears the 3 least-significant bits. In other words, it directly creates a scalar value that is /// in the right form and pre-multiplied by the cofactor. /// -/// See for details +/// See [here](https://neilmadden.blog/2020/05/28/whats-the-curve25519-clamping-all-about/) for +/// more details. pub fn clamp_integer(mut bytes: [u8; 32]) -> [u8; 32] { bytes[0] &= 0b1111_1000; bytes[31] &= 0b0111_1111; From dcd30784e229c41c58b42a05df61a1d9de8f1a6b Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Mon, 27 Mar 2023 04:58:14 -0400 Subject: [PATCH 10/21] Added BasepointTable::mul_base_clamped; did lots of docs --- .../serial/scalar_mul/variable_base.rs | 1 + src/edwards.rs | 72 ++++++++------ src/montgomery.rs | 65 +++++++++---- src/scalar.rs | 93 +++++++++++-------- src/traits.rs | 14 ++- 5 files changed, 160 insertions(+), 85 deletions(-) diff --git a/src/backend/serial/scalar_mul/variable_base.rs b/src/backend/serial/scalar_mul/variable_base.rs index 513904137..1de84bc4d 100644 --- a/src/backend/serial/scalar_mul/variable_base.rs +++ b/src/backend/serial/scalar_mul/variable_base.rs @@ -16,6 +16,7 @@ pub(crate) fn mul(point: &EdwardsPoint, scalar: &Scalar) -> EdwardsPoint { // s = s_0 + s_1*16^1 + ... + s_63*16^63, // // with `-8 ≤ s_i < 8` for `0 ≤ i < 63` and `-8 ≤ s_63 ≤ 8`. + // This decomposition requires s < 2^255, which is guaranteed by Scalar invariant #1. let scalar_digits = scalar.as_radix_16(); // Compute s*P as // diff --git a/src/edwards.rs b/src/edwards.rs index 9c4a9611e..61583b130 100644 --- a/src/edwards.rs +++ b/src/edwards.rs @@ -44,8 +44,8 @@ //! //! ## Scalars //! -//! Scalars are represented by the [`Scalar`] struct. To construct a scalar with a specific bit -//! pattern, see [`Scalar::from_bits`]. +//! Scalars are represented by the [`Scalar`] struct. To construct a scalar, see +//! [`Scalar::from_canonical_bytes`] or [`Scalar::from_bytes_mod_order_wide`]. //! //! ## Scalar Multiplication //! @@ -732,12 +732,13 @@ impl EdwardsPoint { /// Multiply this point by `clamp_integer(bytes)`. For a description of clamping, see /// [`clamp_integer`]. pub fn mul_clamped(self, bytes: [u8; 32]) -> Self { - // We have to construct a Scalar that is not reduced mod l, which breaks its invariant. - // However, all our scalar-point multiplication routines are defined for all values of - // `bytes` up to and including 2^255 - 1, and clamping is guaranteed to return something - // within this range. Further, we don't do any reduction or arithmetic with this clamped - // value, so there's no issues arising from the fact that the curve point is not - // necessarily in the prime-order subgroup. + // We have to construct a Scalar that is not reduced mod l, which breaks scalar invariant + // #2. But #2 is not necessary for correctness of variable-base multiplication. All that + // needs to hold is invariant #1, i.e., the scalar is less than 2^255. This is guaranteed + // by clamping. + // Further, we don't do any reduction or arithmetic with this clamped value, so there's no + // issues arising from the fact that the curve point is not necessarily in the prime-order + // subgroup. let s = Scalar { bytes: clamp_integer(bytes), }; @@ -748,8 +749,8 @@ impl EdwardsPoint { /// [`clamp_integer`]. pub fn mul_base_clamped(bytes: [u8; 32]) -> Self { // See reasoning in Self::mul_clamped why it is OK to make an unreduced Scalar here. We - // note that basepoint multiplication is also defined for all values of `bytes` up to and - // including 2^255 - 1. + // note that fixed-base multiplication is also defined for all values of `bytes` less than + // 2^255. let s = Scalar { bytes: clamp_integer(bytes), }; @@ -902,7 +903,7 @@ macro_rules! impl_basepoint_table { /// /// Normally, the radix-256 tables would allow for only 32 additions per scalar /// multiplication. However, due to the fact that standardised definitions of - /// legacy protocols—such as x25519—require allowing unreduced 255-bit scalar + /// legacy protocols—such as x25519—require allowing unreduced 255-bit scalars /// invariants, when converting such an unreduced scalar's representation to /// radix-\\(2^{8}\\), we cannot guarantee the carry bit will fit in the last /// coefficient (the coefficients are `i8`s). When, \\(w\\), the power-of-2 of @@ -1251,8 +1252,7 @@ impl Debug for EdwardsPoint { #[cfg(test)] mod test { use super::*; - use crate::field::FieldElement; - use crate::scalar::Scalar; + use crate::{field::FieldElement, scalar::Scalar}; use subtle::ConditionallySelectable; #[cfg(feature = "alloc")] @@ -1316,20 +1316,6 @@ mod test { 0x2b, 0x42, ]); - /// The largest valid scalar (not mod l). Remember for NAF computations, the top bit has to be - // 0. So the largest integer a scalar can hold is 2^255 - 1. Addition and subtraction are - // broken on unreduced scalars. The only thing you can do with this is multiplying with a curve - // point (and actually also scalar-scalar multiplication, but that's just a quirk of our - // implementation). - #[cfg(feature = "precomputed-tables")] - static LARGEST_UNREDUCED_SCALAR: Scalar = Scalar { - bytes: [ - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0x7f, - ], - }; - /// Test round-trip decompression for the basepoint. #[test] fn basepoint_decompression_compression() { @@ -1506,12 +1492,13 @@ mod test { assert_eq!(aP128, aP256); } - /// Check a unreduced scalar multiplication by the basepoint tables. + /// Check unreduced scalar multiplication by the basepoint tables is the same no matter what + /// radix the table is. #[cfg(feature = "precomputed-tables")] #[test] fn basepoint_tables_unreduced_scalar() { let P = &constants::ED25519_BASEPOINT_POINT; - let a = LARGEST_UNREDUCED_SCALAR; + let a = crate::scalar::test::LARGEST_UNREDUCED_SCALAR; let table_radix16 = EdwardsBasepointTableRadix16::create(P); let table_radix32 = EdwardsBasepointTableRadix32::create(P); @@ -1552,6 +1539,33 @@ mod test { assert_eq!(bp16.compress(), BASE16_CMPRSSD); } + /// Check that mul_base_clamped and mul_clamped agree + #[test] + fn mul_base_clamped() { + let mut csprng = rand_core::OsRng; + + // Test agreement on a large integer. Even after clamping, this is not reduced mod l. + let a_bytes = [0xff; 32]; + assert_eq!( + EdwardsPoint::mul_base_clamped(a_bytes), + constants::ED25519_BASEPOINT_POINT.mul_clamped(a_bytes) + ); + + // Test agreement on random integers + for _ in 0..100 { + use rand_core::RngCore; + + // This will be reduced mod l with probability l / 2^256 ≈ 6.25% + let mut a_bytes = [0u8; 32]; + csprng.fill_bytes(&mut a_bytes); + + assert_eq!( + EdwardsPoint::mul_base_clamped(a_bytes), + constants::ED25519_BASEPOINT_POINT.mul_clamped(a_bytes) + ); + } + } + #[test] #[cfg(feature = "alloc")] fn impl_sum() { diff --git a/src/montgomery.rs b/src/montgomery.rs index 353b0bfe9..5f4033487 100644 --- a/src/montgomery.rs +++ b/src/montgomery.rs @@ -126,12 +126,13 @@ impl MontgomeryPoint { /// Multiply this point by `clamp_integer(bytes)`. For a description of clamping, see /// [`clamp_integer`]. pub fn mul_clamped(self, bytes: [u8; 32]) -> Self { - // We have to construct a Scalar that is not reduced mod l, which breaks its invariant. - // However, all our scalar-point multiplication routines are defined for all values of - // `bytes` up to and including 2^255 - 1, and clamping is guaranteed to return something - // within this range. Further, we don't do any reduction or arithmetic with this clamped - // value, so there's no issues arising from the fact that the curve point is not - // necessarily in the prime-order subgroup. + // We have to construct a Scalar that is not reduced mod l, which breaks scalar invariant + // #2. But #2 is not necessary for correctness of variable-base multiplication. All that + // needs to hold is invariant #1, i.e., the scalar is less than 2^255. This is guaranteed + // by clamping. + // Further, we don't do any reduction or arithmetic with this clamped value, so there's no + // issues arising from the fact that the curve point is not necessarily in the prime-order + // subgroup. let s = Scalar { bytes: clamp_integer(bytes), }; @@ -142,8 +143,8 @@ impl MontgomeryPoint { /// [`clamp_integer`]. pub fn mul_base_clamped(bytes: [u8; 32]) -> Self { // See reasoning in Self::mul_clamped why it is OK to make an unreduced Scalar here. We - // note that basepoint multiplication is also defined for all values of `bytes` up to and - // including 2^255 - 1. + // note that fixed-base multiplication is also defined for all values of `bytes` less than + // 2^255. let s = Scalar { bytes: clamp_integer(bytes), }; @@ -369,6 +370,9 @@ impl<'a, 'b> Mul<&'b Scalar> for &'a MontgomeryPoint { W: FieldElement::ONE, }; + // NOTE: The below swap-double-add routine skips the first iteration, i.e., it assumes the + // MSB of `scalar` is 0. This is allowed, since it follows from Scalar invariant #1. + // Go through the bits from most to least significant, using a sliding window of 2 let mut bits = scalar.bits_le().rev(); let mut prev_bit = bits.next().unwrap(); @@ -418,8 +422,7 @@ mod test { #[cfg(feature = "alloc")] use alloc::vec::Vec; - #[cfg(feature = "rand_core")] - use rand_core::OsRng; + use rand_core::RngCore; #[test] fn identity_in_different_coordinates() { @@ -503,18 +506,44 @@ mod test { } #[test] - #[cfg(feature = "rand_core")] fn montgomery_ladder_matches_edwards_scalarmult() { - let mut csprng: OsRng = OsRng; + let mut csprng = rand_core::OsRng; + + for _ in 0..100 { + let s: Scalar = Scalar::random(&mut csprng); + let p_edwards = EdwardsPoint::mul_base(&s); + let p_montgomery: MontgomeryPoint = p_edwards.to_montgomery(); + + let expected = s * p_edwards; + let result = s * p_montgomery; - let s: Scalar = Scalar::random(&mut csprng); - let p_edwards = EdwardsPoint::mul_base(&s); - let p_montgomery: MontgomeryPoint = p_edwards.to_montgomery(); + assert_eq!(result, expected.to_montgomery()) + } + } - let expected = s * p_edwards; - let result = s * p_montgomery; + /// Check that mul_base_clamped and mul_clamped agree + #[test] + fn mul_base_clamped() { + let mut csprng = rand_core::OsRng; - assert_eq!(result, expected.to_montgomery()) + // Test agreement on a large integer. Even after clamping, this is not reduced mod l. + let a_bytes = [0xff; 32]; + assert_eq!( + MontgomeryPoint::mul_base_clamped(a_bytes), + constants::X25519_BASEPOINT.mul_clamped(a_bytes) + ); + + // Test agreement on random integers + for _ in 0..100 { + // This will be reduced mod l with probability l / 2^256 ≈ 6.25% + let mut a_bytes = [0u8; 32]; + csprng.fill_bytes(&mut a_bytes); + + assert_eq!( + MontgomeryPoint::mul_base_clamped(a_bytes), + constants::X25519_BASEPOINT.mul_clamped(a_bytes) + ); + } } #[cfg(feature = "alloc")] diff --git a/src/scalar.rs b/src/scalar.rs index 9ee1583d4..c8fce1292 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -190,25 +190,38 @@ pub struct Scalar { /// `bytes` is a little-endian byte encoding of an integer representing a scalar modulo the /// group order. /// - /// # Invariant + /// # Invariant #1 /// - /// The integer representing this scalar is less than \\(2\^{255} - 19 \\), i.e., it represents - /// a canonical representative of an element of \\( \mathbb Z / \ell\mathbb Z \\). + /// The integer representing this scalar is less than \\(2\^{255}\\). That is, the most + /// significant bit of `bytes[31]` is 0. + /// + /// This is required for `EdwardsPoint` variable- and fixed-base multiplication, because most + /// integers above 2^255 are unrepresentable in our radix-16 NAF (see [`Self::as_radix_16`]). + /// The invariant is also required because our `MontgomeryPoint` multiplication assumes the MSB + /// is 0 (see `MontgomeryPoint::mul`). + /// + /// # Invariant #2 (weak) /// - /// Note that this implies the high bit of `bytes[31]` is zero, which ensures that there is - /// room for a carry bit when computing a NAF representation. + /// The integer representing this scalar is less than \\(2\^{255} - 19 \\), i.e., it represents + /// a canonical representative of an element of \\( \mathbb Z / \ell\mathbb Z \\). This is + /// stronger than invariant #1. It also sometimes has to be broken. /// - /// # Note + /// This invariant is deliberately broken in the implementation of `EdwardsPoint::{mul_clamped, + /// mul_base_clamped}`, `MontgomeryPoint::{mul_clamped, mul_base_clamped}`, and + /// `BasepointTable::mul_base_clamped`. This is not an issue though. As mentioned above, + /// scalar-point multiplication is defined for any choice of `bytes` that satisfies invariant + /// #1. Since clamping guarantees invariant #1 is satisfied, these operations are well defined. /// - /// In some unit tests, and one specific use case of the `mul_clamped` for `EdwardsPoint` and - /// `MontgomeryPoint`, this will not be reduced mod l. This is not an issue. We implement curve - /// point scalar multiplication for any choice of `bytes`, so long as the top bit is 0. + /// Note: Scalar-point mult is the _only_ thing you can do safely with an unreduced scalar. + /// Scalar-scalar addition and subtraction are NOT correct when using unreduced scalars. + /// Multiplication is correct, but this is only due to a quirk of our implementation, and not + /// guaranteed to hold in general in the future. /// - /// This is the only thing you can do safely with an unreduced scalar. Addition and subtraction - /// are NOT correct when using unreduced scalars. Multiplication is correct, but this is only - /// due to a quirk of our implementation, and not guaranteed to hold in general. + /// Note: It is not possible to construct an unreduced `Scalar` from the public API unless the + /// `legacy_compatibility` is enabled (thus making `Scalar::from_bits` public). Thus, for all + /// public non-legacy uses, invariant #2 + /// always holds. /// - /// It is not possible to construct an unreduced `Scalar` from the publicly-facing API. pub(crate) bytes: [u8; 32], } @@ -248,11 +261,12 @@ impl Scalar { /// Construct a `Scalar` from the low 255 bits of a 256-bit integer. This breaks the invariant /// that scalars are always reduced. **Scalar arithmetic does not work** on scalars produced /// from this function. You may only use the output of this for `EdwardsPoint::mul` and - /// `EdwardsPoint::vartime_double_scalar_mul_basepoint` + /// `EdwardsPoint::vartime_double_scalar_mul_basepoint`. **Do not use this function** unless + /// you absolutely have to. #[cfg(feature = "legacy_compatibility")] pub const fn from_bits(bytes: [u8; 32]) -> Scalar { let mut s = Scalar { bytes }; - // Ensure that s < 2^255 by masking the high bit + // Ensure invariant #1 holds. That is, make s < 2^255 by masking the high bit. s.bytes[31] &= 0b0111_1111; s @@ -317,7 +331,7 @@ impl<'a, 'b> Add<&'b Scalar> for &'a Scalar { #[allow(non_snake_case)] fn add(self, _rhs: &'b Scalar) -> Scalar { // The UnpackedScalar::add function produces reduced outputs if the inputs are reduced. By - // the Scalar invariant property, this is always the case. + // Scalar invariant #1, this is always the case. UnpackedScalar::add(&self.unpack(), &_rhs.unpack()).pack() } } @@ -337,7 +351,7 @@ impl<'a, 'b> Sub<&'b Scalar> for &'a Scalar { #[allow(non_snake_case)] fn sub(self, rhs: &'b Scalar) -> Scalar { // The UnpackedScalar::sub function produces reduced outputs if the inputs are reduced. By - // the Scalar invariant property, this is always the case. + // Scalar invariant #1, this is always the case. UnpackedScalar::sub(&self.unpack(), &rhs.unpack()).pack() } } @@ -833,7 +847,7 @@ impl Scalar { /// /// The length of the NAF is at most one more than the length of /// the binary representation of \\(k\\). This is why the - /// `Scalar` type maintains an invariant that the top bit is + /// `Scalar` type maintains an invariant (invariant #1) that the top bit is /// \\(0\\), so that the NAF of a scalar has at most 256 digits. /// /// Intuitively, this is like a binary expansion, except that we @@ -952,6 +966,10 @@ impl Scalar { /// a = a\_0 + a\_1 16\^1 + \cdots + a_{63} 16\^{63}, /// $$ /// with \\(-8 \leq a_i < 8\\) for \\(0 \leq i < 63\\) and \\(-8 \leq a_{63} \leq 8\\). + /// + /// The largest value that can be decomposed like this is just over \\(2^{255}\\). Thus, in + /// order to not error, the top bit MUST NOT be set, i.e., `Self` MUST be less than + /// \\(2^{255}\\). pub(crate) fn as_radix_16(&self) -> [i8; 64] { debug_assert!(self[31] <= 127); let mut output = [0i8; 64]; @@ -994,10 +1012,7 @@ impl Scalar { debug_assert!(w <= 8); let digits_count = match w { - 4 => (256 + w - 1) / w, - 5 => (256 + w - 1) / w, - 6 => (256 + w - 1) / w, - 7 => (256 + w - 1) / w, + 4..=7 => (256 + w - 1) / w, // See comment in to_radix_2w on handling the terminal carry. 8 => (256 + w - 1) / w + 1_usize, _ => panic!("invalid radix parameter"), @@ -1007,14 +1022,18 @@ impl Scalar { digits_count } - /// Creates a representation of a Scalar in radix 32, 64, 128 or 256 for use with the Pippenger algorithm. - /// For lower radix, use `to_radix_16`, which is used by the Straus multi-scalar multiplication. - /// Higher radixes are not supported to save cache space. Radix 256 is near-optimal even for very - /// large inputs. + /// Creates a representation of a Scalar in radix \\( 2^w \\) with \\(w = 4, 5, 6, 7, 8\\) for + /// use with the Pippenger algorithm. Higher radixes are not supported to save cache space. + /// Radix 256 is near-optimal even for very large inputs. /// - /// Radix below 32 or above 256 is prohibited. + /// Radix below 16 or above 256 is prohibited. /// This method returns digits in a fixed-sized array, excess digits are zeroes. /// + /// For radix 16, `Self` must be less than \\(2^{255}\\). This is because most integers larger + /// than \\(2^{255}\\) are unrepresentable in the form described below for \\(w = 4\\). This + /// would be true for \\(w = 8\\) as well, but it is compensated for by increasing the size + /// hint by 1. + /// /// ## Scalar representation /// /// Radix \\(2\^w\\), with \\(n = ceil(256/w)\\) coefficients in \\([-(2\^w)/2,(2\^w)/2)\\), @@ -1068,12 +1087,12 @@ impl Scalar { digits[i] = ((coef as i64) - (carry << w) as i64) as i8; } - // When w < 8, we can fold the final carry onto the last digit d, + // When 4 < w < 8, we can fold the final carry onto the last digit d, // because d < 2^w/2 so d + carry*2^w = d + 1*2^w < 2^(w+1) < 2^8. // // When w = 8, we can't fit carry*2^w into an i8. This should // not happen anyways, because the final carry will be 0 for - // reduced scalars, but the Scalar invariant allows 255-bit scalars. + // reduced scalars, but Scalar invariant #1 allows 255-bit scalars. // To handle this, we expand the size_hint by 1 when w=8, // and accumulate the final carry onto another digit. match w { @@ -1099,7 +1118,7 @@ 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 the invariant. + /// public because any `Scalar` that is publicly observed is reduced, by scalar invariant #2. fn is_canonical(&self) -> Choice { self.ct_eq(&self.reduce()) } @@ -1224,7 +1243,7 @@ pub fn clamp_integer(mut bytes: [u8; 32]) -> [u8; 32] { } #[cfg(test)] -mod test { +pub(crate) mod test { use super::*; use crate::constants; @@ -1256,13 +1275,13 @@ mod test { ], }; - /// The largest valid scalar (not mod l). Remember for NAF computations, the top bit has to be - // 0. So the largest integer a scalar can hold is 2^255 - 1. Addition and subtraction are - // broken on unreduced scalars. The only thing you can do with this is multiplying with a curve - // point (and actually also scalar-scalar multiplication, but that's just a quirk of our - // implementation). + /// The largest scalar that satisfies invariant #1, i.e., the largest scalar with the top bit + /// set to 0. Since this scalar violates invariant #2, i.e., it's greater than the modulus `l`, + /// addition and subtraction are broken. The only thing you can do with this is scalar-point + /// multiplication (and actually also scalar-scalar multiplication, but that's just a quirk of + /// our implementation). #[cfg(feature = "precomputed-tables")] - static LARGEST_UNREDUCED_SCALAR: Scalar = Scalar { + pub(crate) static LARGEST_UNREDUCED_SCALAR: Scalar = Scalar { bytes: [ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, diff --git a/src/traits.rs b/src/traits.rs index 31ba9bea9..a742a2dde 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -17,7 +17,7 @@ use core::borrow::Borrow; use subtle; -use crate::scalar::Scalar; +use crate::scalar::{clamp_integer, Scalar}; // ------------------------------------------------------------------------ // Public Traits @@ -61,6 +61,18 @@ pub trait BasepointTable { /// Multiply a `scalar` by this precomputed basepoint table, in constant time. fn mul_base(&self, scalar: &Scalar) -> Self::Point; + + /// Multiply `clamp_integer(bytes)` by this precomputed basepoint table, in constant time. For + /// a description of clamping, see [`clamp_integer`]. + fn mul_base_clamped(&self, bytes: [u8; 32]) -> Self::Point { + // Basepoint multiplication is defined for all values of `bytes` up to and including + // 2^255 - 1. The limit comes from the fact that scalar.as_radix_16() doesn't work for + // most scalars larger than 2^255. + let s = Scalar { + bytes: clamp_integer(bytes), + }; + self.mul_base(&s) + } } /// A trait for constant-time multiscalar multiplication without precomputation. From 25ee96cca1e744fc6d8a5d7a7cfed6d40eb3857e Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Mon, 27 Mar 2023 05:01:56 -0400 Subject: [PATCH 11/21] Removed invalid scalar arithmetic test; this functionality is no longer supported --- src/scalar.rs | 99 --------------------------------------------------- 1 file changed, 99 deletions(-) diff --git a/src/scalar.rs b/src/scalar.rs index c8fce1292..9cdfedf33 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -1454,46 +1454,6 @@ pub(crate) mod test { assert_eq!(Scalar::ZERO - Scalar::ONE, BASEPOINT_ORDER_MINUS_ONE); } - /* - #[test] - fn quarkslab_scalar_overflow_does_not_occur() { - // Check that manually-constructing large Scalars cannot produce incorrect results. - // - // The EdwardsPoint::mul_clamped function is required to implement X/Ed25519, while all - // other methods of constructing a Scalar produce reduced Scalars. However, this "invariant - // loophole" allows constructing large scalars which are not reduced mod l. - // - // This issue was discovered independently by both Jack "str4d" Grigg (issue #238), who - // noted that reduction was not performed on addition, and Laurent Grémy & Nicolas - // Surbayrole of Quarkslab, who noted that it was possible to cause an overflow and compute - // incorrect results. - // - // This test is adapted from the one suggested by Quarkslab. - - let a = Scalar::from_bytes_mod_order(LARGEST_VALID_SCALAR.bytes); - let b = LARGEST_VALID_SCALAR; - - assert_eq!(a, b.reduce()); - - let a_3 = a + a + a; - let b_3 = b + b + b; - - assert_eq!(a_3, b_3); - - let neg_a = -a; - let neg_b = -b; - - assert_eq!(neg_a, neg_b); - - let minus_a_3 = Scalar::ZERO - a - a - a; - let minus_b_3 = Scalar::ZERO - b - b - b; - - assert_eq!(minus_a_3, minus_b_3); - assert_eq!(minus_a_3, -a_3); - assert_eq!(minus_b_3, -b_3); - } - */ - #[test] fn impl_add() { let two = Scalar::from(2u64); @@ -1928,63 +1888,4 @@ pub(crate) mod test { assert_eq!(a * c.reduce(), reduced_mul_ac); } } - - /* - #[test] - fn test_bad_arithmetic1() { - use rand::{RngCore, SeedableRng}; - let mut rng = rand::rngs::StdRng::seed_from_u64(2u64); - - // Make a random clamped scalar - let s = { - let mut bytes = [0u8; 32]; - rng.fill_bytes(&mut bytes); - Scalar::from_bits_clamped(bytes) - }; - - // Make a random point - let point = { - let mut bytes = [0u8; 32]; - rng.fill_bytes(&mut bytes); - crate::edwards::CompressedEdwardsY::from_slice(&bytes) - .unwrap() - .decompress() - .unwrap() - }; - - assert_eq!(s * point, s.reduce() * point); - - //assert_eq!(max_scalar.bytes, reduced.bytes); - } - - #[test] - fn test_bad_arithmetic2() { - use rand::{RngCore, SeedableRng}; - let mut rng = rand::rngs::StdRng::seed_from_u64(2u64); - - // Make two random scalars that are reduced mod l - let (s1, s2) = { - let mut bytes1 = [0u8; 32]; - let mut bytes2 = [0u8; 32]; - rng.fill_bytes(&mut bytes1); - rng.fill_bytes(&mut bytes2); - ( - Scalar::from_bytes_mod_order(bytes1), - Scalar::from_bytes_mod_order(bytes2), - ) - }; - - // Make a random point - let point = { - let mut bytes = [0u8; 32]; - rng.fill_bytes(&mut bytes); - crate::edwards::CompressedEdwardsY::from_slice(&bytes) - .unwrap() - .decompress() - .unwrap() - }; - - assert_eq!((s1 + s2) * point, s1 * point + s2 * point); - } - */ } From be9fc8fb9135df2415ef3fe690fa3c96e7273a12 Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Mon, 27 Mar 2023 05:05:39 -0400 Subject: [PATCH 12/21] Made clamp_integer() const --- src/scalar.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/scalar.rs b/src/scalar.rs index 9cdfedf33..2ae58a3d0 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -1235,7 +1235,7 @@ fn read_le_u64_into(src: &[u8], dst: &mut [u64]) { /// /// See [here](https://neilmadden.blog/2020/05/28/whats-the-curve25519-clamping-all-about/) for /// more details. -pub fn clamp_integer(mut bytes: [u8; 32]) -> [u8; 32] { +pub const fn clamp_integer(mut bytes: [u8; 32]) -> [u8; 32] { bytes[0] &= 0b1111_1000; bytes[31] &= 0b0111_1111; bytes[31] |= 0b0100_0000; @@ -1336,13 +1336,8 @@ pub(crate) mod test { ], }; - static LARGEST_ED25519_S: Scalar = Scalar { - bytes: [ - 0xf8, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0x7f, - ], - }; + /// The largest clamped integer + static LARGEST_CLAMPED_INTEGER: [u8; 32] = clamp_integer(LARGEST_UNREDUCED_SCALAR.bytes); #[test] fn fuzzer_testcase_reduction() { @@ -1845,8 +1840,8 @@ pub(crate) mod test { assert_eq!(actual, expected); assert_eq!( - LARGEST_ED25519_S.bytes, - clamp_integer(LARGEST_ED25519_S.bytes) + LARGEST_CLAMPED_INTEGER, + clamp_integer(LARGEST_CLAMPED_INTEGER) ); } From fc86a64ea60b5097531036a6f5672481b69b8624 Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Mon, 27 Mar 2023 05:14:14 -0400 Subject: [PATCH 13/21] Fixed build; updated readme and changelog --- CHANGELOG.md | 2 ++ README.md | 2 ++ src/scalar.rs | 1 - 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 788c116c2..eefd63986 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,9 +22,11 @@ major series. whenever using `EdwardsBasepointTable` or `RistrettoBasepointTable` * `Scalar::from_canonical_bytes` now returns `CtOption` * `Scalar::is_canonical` now returns `Choice` +* Remove `Scalar::from_bits` and `Scalar::from_bits_clamped` #### Other changes +* Add `EdwardsPoint::{mul_base, mul_base_clamped}`, `MontgomeryPoint::{mul_base, mul_base_clamped}`, and `BasepointTable::mul_base_clamped` * Add `precomputed-tables` feature * Update Maintenance Policies for SemVer * Migrate documentation to docs.rs hosted diff --git a/README.md b/README.md index 44ce31aad..0cc6164b2 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,7 @@ curve25519-dalek = "4.0.0-rc.1" | `rand_core` | | Enables `Scalar::random` and `RistrettoPoint::random`. This is an optional dependency whose version is not subject to SemVer. See [below](#public-api-semver-exemptions) for more details. | | `digest` | | Enables `RistrettoPoint::{from_hash, hash_from_bytes}` and `Scalar::{from_hash, hash_from_bytes}`. This is an optional dependency whose version is not subject to SemVer. See [below](#public-api-semver-exemptions) for more details. | | `serde` | | Enables `serde` serialization/deserialization for all the point and scalar types. | +| `legacy_compatibility`| | Enables `Scalar::from_bits`, which allows the user to build unreduced scalars whose arithmetic is broken. Do not use this unless you know what you're doing. | To disable the default features when using `curve25519-dalek` as a dependency, add `default-features = false` to the dependency in your `Cargo.toml`. To @@ -78,6 +79,7 @@ latest breaking changes are below: * Replace methods `Scalar::{zero, one}` with constants `Scalar::{ZERO, ONE}` * `Scalar::from_canonical_bytes` now returns `CtOption` * `Scalar::is_canonical` now returns `Choice` +* Remove `Scalar::from_bits` and `Scalar::from_bits_clamped` * Deprecate `EdwardsPoint::hash_from_bytes` and rename it `EdwardsPoint::nonspec_map_to_curve` * Require including a new trait, `use curve25519_dalek::traits::BasepointTable` diff --git a/src/scalar.rs b/src/scalar.rs index 2ae58a3d0..718b59612 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -1280,7 +1280,6 @@ pub(crate) mod test { /// addition and subtraction are broken. The only thing you can do with this is scalar-point /// multiplication (and actually also scalar-scalar multiplication, but that's just a quirk of /// our implementation). - #[cfg(feature = "precomputed-tables")] pub(crate) static LARGEST_UNREDUCED_SCALAR: Scalar = Scalar { bytes: [ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, From 1663cb60ed9f2f872bf48926acf1e184c10f5804 Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Mon, 27 Mar 2023 05:19:23 -0400 Subject: [PATCH 14/21] Wibble --- src/scalar.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/scalar.rs b/src/scalar.rs index 718b59612..3c079fe33 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -1216,12 +1216,12 @@ fn read_le_u64_into(src: &[u8], dst: &mut [u64]) { } } -/// Clamps the given little-endian representation of a 32-byte integer. Clamping the value puts it -/// in the range: +/// _Clamps_ the given little-endian representation of a 32-byte integer. Clamping the value puts +/// it in the range: /// /// **n ∈ 2^254 + 8\*{0, 1, 2, 3, . . ., 2^251 − 1}** /// -/// # Explanation of _clamping_ +/// # Explanation of clamping /// /// For Curve25519, h = 8, and multiplying by 8 is the same as a binary left-shift by 3 bits. /// If you take a secret scalar value between 2^251 and 2^252 – 1 and left-shift by 3 bits From 742bf6033e2b289064fab6c141add8f2bbc314c7 Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Mon, 27 Mar 2023 05:26:53 -0400 Subject: [PATCH 15/21] Clarify changelog items --- CHANGELOG.md | 3 ++- README.md | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eefd63986..234c31094 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,8 @@ major series. whenever using `EdwardsBasepointTable` or `RistrettoBasepointTable` * `Scalar::from_canonical_bytes` now returns `CtOption` * `Scalar::is_canonical` now returns `Choice` -* Remove `Scalar::from_bits` and `Scalar::from_bits_clamped` +* Gate `Scalar::from_bits` behind the `legacy_compatibility` feature +* Remove `Scalar::from_bits_clamped` #### Other changes diff --git a/README.md b/README.md index 0cc6164b2..788e10795 100644 --- a/README.md +++ b/README.md @@ -79,7 +79,8 @@ latest breaking changes are below: * Replace methods `Scalar::{zero, one}` with constants `Scalar::{ZERO, ONE}` * `Scalar::from_canonical_bytes` now returns `CtOption` * `Scalar::is_canonical` now returns `Choice` -* Remove `Scalar::from_bits` and `Scalar::from_bits_clamped` +* Gate `Scalar::from_bits` behind the `legacy_compatibility` feature +* Remove `Scalar::from_bits_clamped` * Deprecate `EdwardsPoint::hash_from_bytes` and rename it `EdwardsPoint::nonspec_map_to_curve` * Require including a new trait, `use curve25519_dalek::traits::BasepointTable` From a7a52571c97e605dfad1935f18ed6220f42541fa Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Tue, 28 Mar 2023 16:37:00 -0400 Subject: [PATCH 16/21] Added BasepointTable::mul_base_clamped to tests --- src/edwards.rs | 26 +++++++++++++++++++++++--- src/scalar.rs | 10 ++++++---- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/edwards.rs b/src/edwards.rs index 61583b130..0310fd58f 100644 --- a/src/edwards.rs +++ b/src/edwards.rs @@ -1261,6 +1261,8 @@ mod test { #[cfg(feature = "precomputed-tables")] use crate::constants::ED25519_BASEPOINT_TABLE; + use rand_core::RngCore; + /// X coordinate of the basepoint. /// = 15112221349535400772501151409588531511454012693041857206046113283949847762202 static BASE_X_COORD_BYTES: [u8; 32] = [ @@ -1544,17 +1546,31 @@ mod test { fn mul_base_clamped() { let mut csprng = rand_core::OsRng; - // Test agreement on a large integer. Even after clamping, this is not reduced mod l. + // Make a random curve point in the curve. Give it torsion to make things interesting. + let random_point = { + let mut b = [0u8; 32]; + csprng.fill_bytes(&mut b); + EdwardsPoint::mul_base_clamped(b) + constants::EIGHT_TORSION[1] + }; + // Make a basepoint table from the random point. We'll use this with mul_base_clamped + let random_table = EdwardsBasepointTableRadix256::create(&random_point); + + // Now test scalar mult. agreement on the default basepoint as well as random_point + + // Test that mul_base_clamped and mul_clamped agree on a large integer. Even after + // clamping, this integer is not reduced mod l. let a_bytes = [0xff; 32]; assert_eq!( EdwardsPoint::mul_base_clamped(a_bytes), constants::ED25519_BASEPOINT_POINT.mul_clamped(a_bytes) ); + assert_eq!( + random_table.mul_base_clamped(a_bytes), + random_point.mul_clamped(a_bytes) + ); // Test agreement on random integers for _ in 0..100 { - use rand_core::RngCore; - // This will be reduced mod l with probability l / 2^256 ≈ 6.25% let mut a_bytes = [0u8; 32]; csprng.fill_bytes(&mut a_bytes); @@ -1563,6 +1579,10 @@ mod test { EdwardsPoint::mul_base_clamped(a_bytes), constants::ED25519_BASEPOINT_POINT.mul_clamped(a_bytes) ); + assert_eq!( + random_table.mul_base_clamped(a_bytes), + random_point.mul_clamped(a_bytes) + ); } } diff --git a/src/scalar.rs b/src/scalar.rs index 3c079fe33..70ba14872 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -264,6 +264,7 @@ impl Scalar { /// `EdwardsPoint::vartime_double_scalar_mul_basepoint`. **Do not use this function** unless /// you absolutely have to. #[cfg(feature = "legacy_compatibility")] + #[deprecated(since = "4.0.0", note = "blah")] pub const fn from_bits(bytes: [u8; 32]) -> Scalar { let mut s = Scalar { bytes }; // Ensure invariant #1 holds. That is, make s < 2^255 by masking the high bit. @@ -1250,6 +1251,8 @@ pub(crate) mod test { #[cfg(feature = "alloc")] use alloc::vec::Vec; + use rand::RngCore; + /// x = 2238329342913194256032495932344128051776374960164957527413114840482143558222 pub static X: Scalar = Scalar { bytes: [ @@ -1844,12 +1847,11 @@ pub(crate) mod test { ); } - // Check that a * b == a.reduce() * a.reduce() for ANY scalars a,b, even ones out of range and - // with the high bit set. In old versions of ed25519-dalek, it used to be the case that a was - // reduced and b was clamped an unreduced. This should not affect computation. + // Check that a * b == a.reduce() * a.reduce() for ANY scalars a,b, even ones that violate + // invariant #1, i.e., a,b > 2^255. Old versions of ed25519-dalek did multiplication where a + // was reduced and b was clamped and unreduced. This checks that that was always well-defined. #[test] fn test_mul_reduction_invariance() { - use rand::RngCore; let mut rng = rand::thread_rng(); for _ in 0..10 { From ec9a5d19e62a81ecb34be12bf9c070ad1e56fb94 Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Tue, 28 Mar 2023 16:48:16 -0400 Subject: [PATCH 17/21] Added proper deprecation notice to Scalar::from_bits; added legacy_compatibility to Makefile and docsrs flags --- Cargo.toml | 2 +- Makefile | 2 +- src/scalar.rs | 10 +++++++--- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c19cc1b47..f20863807 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,7 +28,7 @@ rustdoc-args = [ "--cfg", "docsrs", ] rustc-args = ["--cfg", "curve25519_dalek_backend=\"simd\""] -features = ["serde", "rand_core", "digest"] +features = ["serde", "rand_core", "digest", "legacy_compatibility"] [dev-dependencies] sha2 = { version = "0.10", default-features = false } diff --git a/Makefile b/Makefile index b263cf753..3b41b1756 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -FEATURES := serde rand_core digest +FEATURES := serde rand_core digest legacy_compatibility export RUSTFLAGS := --cfg=curve25519_dalek_backend="simd" export RUSTDOCFLAGS := \ diff --git a/src/scalar.rs b/src/scalar.rs index 70ba14872..3b71be84a 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -259,12 +259,16 @@ impl Scalar { } /// Construct a `Scalar` from the low 255 bits of a 256-bit integer. This breaks the invariant - /// that scalars are always reduced. **Scalar arithmetic does not work** on scalars produced - /// from this function. You may only use the output of this for `EdwardsPoint::mul` and + /// that scalars are always reduced. Scalar-scalar arithmetic, i.e., addition, subtraction, + /// multiplication, **does not work** on scalars produced from this function. You may only use + /// the output of this function for `EdwardsPoint::mul`, `MontgomeryPoint::mul`, and /// `EdwardsPoint::vartime_double_scalar_mul_basepoint`. **Do not use this function** unless /// you absolutely have to. #[cfg(feature = "legacy_compatibility")] - #[deprecated(since = "4.0.0", note = "blah")] + #[deprecated( + since = "4.0.0", + note = "This constructor outputs scalars with undefined scalar-scalar arithmetic. See docs." + )] pub const fn from_bits(bytes: [u8; 32]) -> Scalar { let mut s = Scalar { bytes }; // Ensure invariant #1 holds. That is, make s < 2^255 by masking the high bit. From 90ff2cb68ec772730b41fb6c2b31c739630f10db Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Tue, 28 Mar 2023 16:50:59 -0400 Subject: [PATCH 18/21] Docs wibble --- src/backend/serial/u32/scalar.rs | 3 ++- src/scalar.rs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/backend/serial/u32/scalar.rs b/src/backend/serial/u32/scalar.rs index 2703078a1..8ae126b1e 100644 --- a/src/backend/serial/u32/scalar.rs +++ b/src/backend/serial/u32/scalar.rs @@ -18,7 +18,8 @@ use zeroize::Zeroize; use crate::constants; -/// The `Scalar29` struct represents an element in ℤ/lℤ as 9 29-bit limbs +/// The `Scalar29` struct represents an element in \\(\mathbb{Z} / \ell\mathbb{Z}\\) as 9 29-bit +/// limbs #[derive(Copy, Clone)] pub struct Scalar29(pub [u32; 9]); diff --git a/src/scalar.rs b/src/scalar.rs index 3b71be84a..829f56021 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -574,7 +574,7 @@ impl Scalar { /// /// # Returns /// - /// A random scalar within ℤ/lℤ. + /// A random scalar within \\(\mathbb{Z} / \ell\mathbb{Z}\\). /// /// # Example /// From a64d72a16f4f23af7eb5843b340c88dd0029bff6 Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Tue, 28 Mar 2023 16:52:56 -0400 Subject: [PATCH 19/21] Fixed test CI --- src/edwards.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/edwards.rs b/src/edwards.rs index 0310fd58f..fae296f66 100644 --- a/src/edwards.rs +++ b/src/edwards.rs @@ -1547,12 +1547,14 @@ mod test { let mut csprng = rand_core::OsRng; // Make a random curve point in the curve. Give it torsion to make things interesting. + #[cfg(feature = "precomputed-tables")] let random_point = { let mut b = [0u8; 32]; csprng.fill_bytes(&mut b); EdwardsPoint::mul_base_clamped(b) + constants::EIGHT_TORSION[1] }; // Make a basepoint table from the random point. We'll use this with mul_base_clamped + #[cfg(feature = "precomputed-tables")] let random_table = EdwardsBasepointTableRadix256::create(&random_point); // Now test scalar mult. agreement on the default basepoint as well as random_point @@ -1564,6 +1566,7 @@ mod test { EdwardsPoint::mul_base_clamped(a_bytes), constants::ED25519_BASEPOINT_POINT.mul_clamped(a_bytes) ); + #[cfg(feature = "precomputed-tables")] assert_eq!( random_table.mul_base_clamped(a_bytes), random_point.mul_clamped(a_bytes) @@ -1579,6 +1582,7 @@ mod test { EdwardsPoint::mul_base_clamped(a_bytes), constants::ED25519_BASEPOINT_POINT.mul_clamped(a_bytes) ); + #[cfg(feature = "precomputed-tables")] assert_eq!( random_table.mul_base_clamped(a_bytes), random_point.mul_clamped(a_bytes) From 2854d4e60c550f076ed714af21983076df630958 Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Tue, 28 Mar 2023 16:55:16 -0400 Subject: [PATCH 20/21] Added Scalar::reduce removal to changelog --- CHANGELOG.md | 2 +- README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 234c31094..f8ba149e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ major series. * `Scalar::from_canonical_bytes` now returns `CtOption` * `Scalar::is_canonical` now returns `Choice` * Gate `Scalar::from_bits` behind the `legacy_compatibility` feature -* Remove `Scalar::from_bits_clamped` +* Remove `Scalar::from_bits_clamped` and `Scalar::reduce` #### Other changes diff --git a/README.md b/README.md index 788e10795..bbff7acb4 100644 --- a/README.md +++ b/README.md @@ -80,7 +80,7 @@ latest breaking changes are below: * `Scalar::from_canonical_bytes` now returns `CtOption` * `Scalar::is_canonical` now returns `Choice` * Gate `Scalar::from_bits` behind the `legacy_compatibility` feature -* Remove `Scalar::from_bits_clamped` +* Remove `Scalar::from_bits_clamped` and `Scalar::reduce` * Deprecate `EdwardsPoint::hash_from_bytes` and rename it `EdwardsPoint::nonspec_map_to_curve` * Require including a new trait, `use curve25519_dalek::traits::BasepointTable` From 239ffa82b9a727af7b5263b25e46f6d2b6b9e6f0 Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Tue, 28 Mar 2023 16:56:50 -0400 Subject: [PATCH 21/21] Changelog wibble --- CHANGELOG.md | 4 ++-- README.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8ba149e6..0e3a97c49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,8 +22,8 @@ major series. whenever using `EdwardsBasepointTable` or `RistrettoBasepointTable` * `Scalar::from_canonical_bytes` now returns `CtOption` * `Scalar::is_canonical` now returns `Choice` -* Gate `Scalar::from_bits` behind the `legacy_compatibility` feature -* Remove `Scalar::from_bits_clamped` and `Scalar::reduce` +* Remove `Scalar::from_bytes_clamped` and `Scalar::reduce` +* Deprecate and feature-gate `Scalar::from_bits` behind `legacy_compatibility` #### Other changes diff --git a/README.md b/README.md index bbff7acb4..9b14bb9f6 100644 --- a/README.md +++ b/README.md @@ -79,8 +79,8 @@ latest breaking changes are below: * Replace methods `Scalar::{zero, one}` with constants `Scalar::{ZERO, ONE}` * `Scalar::from_canonical_bytes` now returns `CtOption` * `Scalar::is_canonical` now returns `Choice` -* Gate `Scalar::from_bits` behind the `legacy_compatibility` feature -* Remove `Scalar::from_bits_clamped` and `Scalar::reduce` +* Remove `Scalar::from_bytes_clamped` and `Scalar::reduce` +* Deprecate and feature-gate `Scalar::from_bits` behind `legacy_compatibility` * Deprecate `EdwardsPoint::hash_from_bytes` and rename it `EdwardsPoint::nonspec_map_to_curve` * Require including a new trait, `use curve25519_dalek::traits::BasepointTable`