From 19c7f4a5d5e577adc9cc65a837abef9ed7ebf0a4 Mon Sep 17 00:00:00 2001 From: "pinkforest(she/her)" <36498018+pinkforest@users.noreply.github.com> Date: Fri, 1 Mar 2024 12:56:52 +1100 Subject: [PATCH 01/17] Fix new nightly redundant import lint warns (#638) --- .../src/backend/serial/scalar_mul/pippenger.rs | 1 - .../src/backend/vector/avx2/edwards.rs | 1 - curve25519-dalek/src/edwards.rs | 15 ++++++++------- curve25519-dalek/src/field.rs | 3 --- curve25519-dalek/src/ristretto.rs | 4 +--- curve25519-dalek/src/scalar.rs | 17 ++++++----------- curve25519-dalek/src/traits.rs | 5 ++--- ed25519-dalek/src/batch.rs | 1 - ed25519-dalek/src/signature.rs | 1 - ed25519-dalek/src/verifying.rs | 1 - ed25519-dalek/tests/ed25519.rs | 5 ++--- 11 files changed, 19 insertions(+), 35 deletions(-) diff --git a/curve25519-dalek/src/backend/serial/scalar_mul/pippenger.rs b/curve25519-dalek/src/backend/serial/scalar_mul/pippenger.rs index 9af39e599..f60d9b953 100644 --- a/curve25519-dalek/src/backend/serial/scalar_mul/pippenger.rs +++ b/curve25519-dalek/src/backend/serial/scalar_mul/pippenger.rs @@ -164,7 +164,6 @@ impl VartimeMultiscalarMul for Pippenger { mod test { use super::*; use crate::constants; - use crate::scalar::Scalar; #[test] fn test_vartime_pippenger() { diff --git a/curve25519-dalek/src/backend/vector/avx2/edwards.rs b/curve25519-dalek/src/backend/vector/avx2/edwards.rs index cf6691e83..fd70d7d2f 100644 --- a/curve25519-dalek/src/backend/vector/avx2/edwards.rs +++ b/curve25519-dalek/src/backend/vector/avx2/edwards.rs @@ -35,7 +35,6 @@ #![allow(non_snake_case)] -use core::convert::From; use core::ops::{Add, Neg, Sub}; use subtle::Choice; diff --git a/curve25519-dalek/src/edwards.rs b/curve25519-dalek/src/edwards.rs index accf22776..c49590b39 100644 --- a/curve25519-dalek/src/edwards.rs +++ b/curve25519-dalek/src/edwards.rs @@ -96,7 +96,6 @@ use core::array::TryFromSliceError; use core::borrow::Borrow; use core::fmt::Debug; -use core::iter::Iterator; use core::iter::Sum; use core::ops::{Add, Neg, Sub}; use core::ops::{AddAssign, SubAssign}; @@ -110,10 +109,12 @@ use digest::{generic_array::typenum::U64, Digest}; #[cfg(feature = "group")] use { group::{cofactor::CofactorGroup, prime::PrimeGroup, GroupEncoding}, - rand_core::RngCore, subtle::CtOption, }; +#[cfg(feature = "group")] +use rand_core::RngCore; + use subtle::Choice; use subtle::ConditionallyNegatable; use subtle::ConditionallySelectable; @@ -258,7 +259,7 @@ impl TryFrom<&[u8]> for CompressedEdwardsY { #[cfg(feature = "serde")] use serde::de::Visitor; #[cfg(feature = "serde")] -use serde::{self, Deserialize, Deserializer, Serialize, Serializer}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; #[cfg(feature = "serde")] impl Serialize for EdwardsPoint { @@ -1591,8 +1592,10 @@ impl CofactorGroup for EdwardsPoint { #[cfg(test)] mod test { use super::*; - use crate::{field::FieldElement, scalar::Scalar}; - use subtle::ConditionallySelectable; + + // If `group` is set, then this is already imported in super + #[cfg(not(feature = "group"))] + use rand_core::RngCore; #[cfg(feature = "alloc")] use alloc::vec::Vec; @@ -1600,8 +1603,6 @@ 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] = [ diff --git a/curve25519-dalek/src/field.rs b/curve25519-dalek/src/field.rs index 87058941e..68c9c8b89 100644 --- a/curve25519-dalek/src/field.rs +++ b/curve25519-dalek/src/field.rs @@ -25,8 +25,6 @@ #![allow(unused_qualifications)] -use core::cmp::{Eq, PartialEq}; - use cfg_if::cfg_if; use subtle::Choice; @@ -310,7 +308,6 @@ impl FieldElement { #[cfg(test)] mod test { use crate::field::*; - use subtle::ConditionallyNegatable; /// Random element a of GF(2^255-19), from Sage /// a = 1070314506888354081329385823235218444233221\ diff --git a/curve25519-dalek/src/ristretto.rs b/curve25519-dalek/src/ristretto.rs index dec7ae067..75ba141de 100644 --- a/curve25519-dalek/src/ristretto.rs +++ b/curve25519-dalek/src/ristretto.rs @@ -364,7 +364,7 @@ impl TryFrom<&[u8]> for CompressedRistretto { #[cfg(feature = "serde")] use serde::de::Visitor; #[cfg(feature = "serde")] -use serde::{self, Deserialize, Deserializer, Serialize, Serializer}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; #[cfg(feature = "serde")] impl Serialize for RistrettoPoint { @@ -1277,8 +1277,6 @@ impl Zeroize for RistrettoPoint { mod test { use super::*; use crate::edwards::CompressedEdwardsY; - use crate::scalar::Scalar; - use crate::traits::Identity; use rand_core::OsRng; diff --git a/curve25519-dalek/src/scalar.rs b/curve25519-dalek/src/scalar.rs index 5b9eca1da..1630af5e7 100644 --- a/curve25519-dalek/src/scalar.rs +++ b/curve25519-dalek/src/scalar.rs @@ -112,8 +112,6 @@ //! has been enabled. use core::borrow::Borrow; -use core::cmp::{Eq, PartialEq}; -use core::convert::TryInto; use core::fmt::Debug; use core::iter::{Product, Sum}; use core::ops::Index; @@ -124,13 +122,13 @@ use core::ops::{Sub, SubAssign}; use cfg_if::cfg_if; +#[cfg(feature = "group")] +use group::ff::{Field, FromUniformBytes, PrimeField}; #[cfg(feature = "group-bits")] use group::ff::{FieldBits, PrimeFieldBits}; -#[cfg(feature = "group")] -use { - group::ff::{Field, FromUniformBytes, PrimeField}, - rand_core::RngCore, -}; + +#[cfg(any(test, feature = "group"))] +use rand_core::RngCore; #[cfg(any(test, feature = "rand_core"))] use rand_core::CryptoRngCore; @@ -402,7 +400,7 @@ impl ConditionallySelectable for Scalar { #[cfg(feature = "serde")] use serde::de::Visitor; #[cfg(feature = "serde")] -use serde::{self, Deserialize, Deserializer, Serialize, Serializer}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; #[cfg(feature = "serde")] #[cfg_attr(docsrs, doc(cfg(feature = "serde")))] @@ -1393,13 +1391,10 @@ pub const fn clamp_integer(mut bytes: [u8; 32]) -> [u8; 32] { #[cfg(test)] pub(crate) mod test { use super::*; - use crate::constants; #[cfg(feature = "alloc")] use alloc::vec::Vec; - use rand::RngCore; - /// x = 2238329342913194256032495932344128051776374960164957527413114840482143558222 pub static X: Scalar = Scalar { bytes: [ diff --git a/curve25519-dalek/src/traits.rs b/curve25519-dalek/src/traits.rs index 870dd32f8..322787db5 100644 --- a/curve25519-dalek/src/traits.rs +++ b/curve25519-dalek/src/traits.rs @@ -15,9 +15,8 @@ use core::borrow::Borrow; -use subtle; - use crate::scalar::{clamp_integer, Scalar}; +use subtle::ConstantTimeEq; // ------------------------------------------------------------------------ // Public Traits @@ -41,7 +40,7 @@ pub trait IsIdentity { /// constructor. impl IsIdentity for T where - T: subtle::ConstantTimeEq + Identity, + T: ConstantTimeEq + Identity, { fn is_identity(&self) -> bool { self.ct_eq(&T::identity()).into() diff --git a/ed25519-dalek/src/batch.rs b/ed25519-dalek/src/batch.rs index ed2618d6c..fa79677d5 100644 --- a/ed25519-dalek/src/batch.rs +++ b/ed25519-dalek/src/batch.rs @@ -11,7 +11,6 @@ use alloc::vec::Vec; -use core::convert::TryFrom; use core::iter::once; use curve25519_dalek::constants; diff --git a/ed25519-dalek/src/signature.rs b/ed25519-dalek/src/signature.rs index 36174c8d6..32fde3016 100644 --- a/ed25519-dalek/src/signature.rs +++ b/ed25519-dalek/src/signature.rs @@ -9,7 +9,6 @@ //! An ed25519 signature. -use core::convert::TryFrom; use core::fmt::Debug; use curve25519_dalek::edwards::CompressedEdwardsY; diff --git a/ed25519-dalek/src/verifying.rs b/ed25519-dalek/src/verifying.rs index b53032144..0d154678b 100644 --- a/ed25519-dalek/src/verifying.rs +++ b/ed25519-dalek/src/verifying.rs @@ -9,7 +9,6 @@ //! ed25519 public keys. -use core::convert::TryFrom; use core::fmt::Debug; use core::hash::{Hash, Hasher}; diff --git a/ed25519-dalek/tests/ed25519.rs b/ed25519-dalek/tests/ed25519.rs index 82ac33d76..edab8a814 100644 --- a/ed25519-dalek/tests/ed25519.rs +++ b/ed25519-dalek/tests/ed25519.rs @@ -27,10 +27,11 @@ mod vectors { scalar::Scalar, traits::IsIdentity, }; + + #[cfg(not(feature = "digest"))] use sha2::{digest::Digest, Sha512}; use std::{ - convert::TryFrom, fs::File, io::{BufRead, BufReader}, ops::Neg, @@ -288,8 +289,6 @@ mod vectors { mod integrations { use super::*; use rand::rngs::OsRng; - #[cfg(feature = "digest")] - use sha2::Sha512; use std::collections::HashMap; #[test] From 31ccb6705067d68782cb135e23c79b640a6a06ee Mon Sep 17 00:00:00 2001 From: "pinkforest(she/her)" <36498018+pinkforest@users.noreply.github.com> Date: Sat, 2 Mar 2024 01:35:23 +1100 Subject: [PATCH 02/17] Remove platforms in favor using CARGO_CFG_TARGET_POINTER_WIDTH (#636) --- curve25519-dalek/Cargo.toml | 1 - curve25519-dalek/build.rs | 75 ++++++++++++++++++------------------- 2 files changed, 37 insertions(+), 39 deletions(-) diff --git a/curve25519-dalek/Cargo.toml b/curve25519-dalek/Cargo.toml index 48dcb977d..f8c7c3a8e 100644 --- a/curve25519-dalek/Cargo.toml +++ b/curve25519-dalek/Cargo.toml @@ -38,7 +38,6 @@ rand = "0.8" rand_core = { version = "0.6", default-features = false, features = ["getrandom"] } [build-dependencies] -platforms = "3.0.2" rustc_version = "0.4.0" [[bench]] diff --git a/curve25519-dalek/build.rs b/curve25519-dalek/build.rs index 92d2802cd..97fa28524 100644 --- a/curve25519-dalek/build.rs +++ b/curve25519-dalek/build.rs @@ -9,17 +9,31 @@ enum DalekBits { Dalek64, } +use std::fmt::Formatter; + +impl std::fmt::Display for DalekBits { + fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> { + let w_bits = match self { + DalekBits::Dalek32 => "32", + DalekBits::Dalek64 => "64", + }; + write!(f, "{}", w_bits) + } +} + fn main() { + let target_arch = match std::env::var("CARGO_CFG_TARGET_ARCH") { + Ok(arch) => arch, + _ => "".to_string(), + }; + let curve25519_dalek_bits = match std::env::var("CARGO_CFG_CURVE25519_DALEK_BITS").as_deref() { Ok("32") => DalekBits::Dalek32, Ok("64") => DalekBits::Dalek64, - _ => deterministic::determine_curve25519_dalek_bits(), + _ => deterministic::determine_curve25519_dalek_bits(&target_arch), }; - match curve25519_dalek_bits { - DalekBits::Dalek64 => println!("cargo:rustc-cfg=curve25519_dalek_bits=\"64\""), - DalekBits::Dalek32 => println!("cargo:rustc-cfg=curve25519_dalek_bits=\"32\""), - } + println!("cargo:rustc-cfg=curve25519_dalek_bits=\"{curve25519_dalek_bits}\""); if rustc_version::version_meta() .expect("failed to detect rustc version") @@ -36,11 +50,6 @@ fn main() { println!("cargo:rustc-cfg=allow_unused_unsafe"); } - let target_arch = match std::env::var("CARGO_CFG_TARGET_ARCH") { - Ok(arch) => arch, - _ => "".to_string(), - }; - // Backend overrides / defaults let curve25519_dalek_backend = match std::env::var("CARGO_CFG_CURVE25519_DALEK_BACKEND").as_deref() { @@ -74,11 +83,12 @@ mod deterministic { use super::*; - // Standard Cargo TARGET environment variable of triplet is required - static ERR_MSG_NO_TARGET: &str = "Standard Cargo TARGET environment variable is not set"; + // Custom Rust non-cargo build tooling needs to set CARGO_CFG_TARGET_POINTER_WIDTH + static ERR_MSG_NO_POINTER_WIDTH: &str = + "Standard Cargo TARGET_POINTER_WIDTH environment variable is not set."; - // Custom Non-Rust standard target platforms require explicit settings. - static ERR_MSG_NO_PLATFORM: &str = "Unknown Rust target platform."; + // When either non-32 or 64 TARGET_POINTER_WIDTH detected + static ERR_MSG_UNKNOWN_POINTER_WIDTH: &str = "Unknown TARGET_POINTER_WIDTH detected."; // Warning when the curve25519_dalek_bits cannot be determined fn determine_curve25519_dalek_bits_warning(cause: &str) { @@ -86,41 +96,30 @@ mod deterministic { } // Determine the curve25519_dalek_bits based on Rust standard TARGET triplet - pub(super) fn determine_curve25519_dalek_bits() -> DalekBits { - use platforms::target::PointerWidth; - - // TARGET environment is supplied by Cargo - // https://doc.rust-lang.org/cargo/reference/environment-variables.html - let target_triplet = match std::env::var("TARGET") { - Ok(t) => t, + pub(super) fn determine_curve25519_dalek_bits(target_arch: &String) -> DalekBits { + let target_pointer_width = match std::env::var("CARGO_CFG_TARGET_POINTER_WIDTH") { + Ok(pw) => pw, Err(_) => { - determine_curve25519_dalek_bits_warning(ERR_MSG_NO_TARGET); - return DalekBits::Dalek32; - } - }; - - // platforms crate is the source of truth used to determine the platform - let platform = match platforms::Platform::find(&target_triplet) { - Some(p) => p, - None => { - determine_curve25519_dalek_bits_warning(ERR_MSG_NO_PLATFORM); + determine_curve25519_dalek_bits_warning(ERR_MSG_NO_POINTER_WIDTH); return DalekBits::Dalek32; } }; #[allow(clippy::match_single_binding)] - match platform.target_arch { + match &target_arch { //Issues: 449 and 456 + //TODO: When adding arch defaults use proper types not String match //TODO(Arm): Needs tests + benchmarks to back this up - //platforms::target::Arch::Arm => DalekBits::Dalek64, //TODO(Wasm32): Needs tests + benchmarks to back this up - //platforms::target::Arch::Wasm32 => DalekBits::Dalek64, - _ => match platform.target_pointer_width { - PointerWidth::U64 => DalekBits::Dalek64, - PointerWidth::U32 => DalekBits::Dalek32, + _ => match target_pointer_width.as_ref() { + "64" => DalekBits::Dalek64, + "32" => DalekBits::Dalek32, // Intended default solely for non-32/64 target pointer widths // Otherwise known target platforms only. - _ => DalekBits::Dalek32, + _ => { + determine_curve25519_dalek_bits_warning(ERR_MSG_UNKNOWN_POINTER_WIDTH); + DalekBits::Dalek32 + } }, } } From 858c4ca8ae03d33fe8b71b4504c4d3f5ff5b45c0 Mon Sep 17 00:00:00 2001 From: "pinkforest(she/her)" <36498018+pinkforest@users.noreply.github.com> Date: Fri, 8 Mar 2024 10:58:20 +1100 Subject: [PATCH 03/17] Address new nightly clippy unnecessary qualifications (#639) --- curve25519-dalek/src/backend/mod.rs | 51 +++++++++---------- .../src/backend/serial/curve_models/mod.rs | 8 +-- .../src/backend/serial/fiat_u32/field.rs | 2 +- .../src/backend/serial/fiat_u64/field.rs | 2 +- .../src/backend/serial/u32/field.rs | 2 +- .../src/backend/serial/u32/scalar.rs | 2 +- .../src/backend/serial/u64/field.rs | 2 +- .../src/backend/serial/u64/scalar.rs | 2 +- curve25519-dalek/src/edwards.rs | 10 ++-- curve25519-dalek/src/ristretto.rs | 8 +-- curve25519-dalek/src/scalar.rs | 6 +-- curve25519-dalek/src/window.rs | 6 +-- ed25519-dalek/src/signature.rs | 2 +- ed25519-dalek/src/signing.rs | 4 +- ed25519-dalek/src/verifying.rs | 2 +- 15 files changed, 52 insertions(+), 57 deletions(-) diff --git a/curve25519-dalek/src/backend/mod.rs b/curve25519-dalek/src/backend/mod.rs index 4424e0a53..9ad1dd3de 100644 --- a/curve25519-dalek/src/backend/mod.rs +++ b/curve25519-dalek/src/backend/mod.rs @@ -87,24 +87,24 @@ where match get_selected_backend() { #[cfg(curve25519_dalek_backend = "simd")] BackendKind::Avx2 => - self::vector::scalar_mul::pippenger::spec_avx2::Pippenger::optional_multiscalar_mul::(scalars, points), + vector::scalar_mul::pippenger::spec_avx2::Pippenger::optional_multiscalar_mul::(scalars, points), #[cfg(all(curve25519_dalek_backend = "simd", nightly))] BackendKind::Avx512 => - self::vector::scalar_mul::pippenger::spec_avx512ifma_avx512vl::Pippenger::optional_multiscalar_mul::(scalars, points), + vector::scalar_mul::pippenger::spec_avx512ifma_avx512vl::Pippenger::optional_multiscalar_mul::(scalars, points), BackendKind::Serial => - self::serial::scalar_mul::pippenger::Pippenger::optional_multiscalar_mul::(scalars, points), + serial::scalar_mul::pippenger::Pippenger::optional_multiscalar_mul::(scalars, points), } } #[cfg(feature = "alloc")] pub(crate) enum VartimePrecomputedStraus { #[cfg(curve25519_dalek_backend = "simd")] - Avx2(self::vector::scalar_mul::precomputed_straus::spec_avx2::VartimePrecomputedStraus), + Avx2(vector::scalar_mul::precomputed_straus::spec_avx2::VartimePrecomputedStraus), #[cfg(all(curve25519_dalek_backend = "simd", nightly))] Avx512ifma( - self::vector::scalar_mul::precomputed_straus::spec_avx512ifma_avx512vl::VartimePrecomputedStraus, + vector::scalar_mul::precomputed_straus::spec_avx512ifma_avx512vl::VartimePrecomputedStraus, ), - Scalar(self::serial::scalar_mul::precomputed_straus::VartimePrecomputedStraus), + Scalar(serial::scalar_mul::precomputed_straus::VartimePrecomputedStraus), } #[cfg(feature = "alloc")] @@ -119,12 +119,12 @@ impl VartimePrecomputedStraus { match get_selected_backend() { #[cfg(curve25519_dalek_backend = "simd")] BackendKind::Avx2 => - VartimePrecomputedStraus::Avx2(self::vector::scalar_mul::precomputed_straus::spec_avx2::VartimePrecomputedStraus::new(static_points)), + VartimePrecomputedStraus::Avx2(vector::scalar_mul::precomputed_straus::spec_avx2::VartimePrecomputedStraus::new(static_points)), #[cfg(all(curve25519_dalek_backend = "simd", nightly))] BackendKind::Avx512 => - VartimePrecomputedStraus::Avx512ifma(self::vector::scalar_mul::precomputed_straus::spec_avx512ifma_avx512vl::VartimePrecomputedStraus::new(static_points)), + VartimePrecomputedStraus::Avx512ifma(vector::scalar_mul::precomputed_straus::spec_avx512ifma_avx512vl::VartimePrecomputedStraus::new(static_points)), BackendKind::Serial => - VartimePrecomputedStraus::Scalar(self::serial::scalar_mul::precomputed_straus::VartimePrecomputedStraus::new(static_points)) + VartimePrecomputedStraus::Scalar(serial::scalar_mul::precomputed_straus::VartimePrecomputedStraus::new(static_points)) } } @@ -179,19 +179,16 @@ where match get_selected_backend() { #[cfg(curve25519_dalek_backend = "simd")] BackendKind::Avx2 => { - self::vector::scalar_mul::straus::spec_avx2::Straus::multiscalar_mul::( - scalars, points, - ) + vector::scalar_mul::straus::spec_avx2::Straus::multiscalar_mul::(scalars, points) } #[cfg(all(curve25519_dalek_backend = "simd", nightly))] BackendKind::Avx512 => { - self::vector::scalar_mul::straus::spec_avx512ifma_avx512vl::Straus::multiscalar_mul::< - I, - J, - >(scalars, points) + vector::scalar_mul::straus::spec_avx512ifma_avx512vl::Straus::multiscalar_mul::( + scalars, points, + ) } BackendKind::Serial => { - self::serial::scalar_mul::straus::Straus::multiscalar_mul::(scalars, points) + serial::scalar_mul::straus::Straus::multiscalar_mul::(scalars, points) } } } @@ -209,21 +206,19 @@ where match get_selected_backend() { #[cfg(curve25519_dalek_backend = "simd")] BackendKind::Avx2 => { - self::vector::scalar_mul::straus::spec_avx2::Straus::optional_multiscalar_mul::( + vector::scalar_mul::straus::spec_avx2::Straus::optional_multiscalar_mul::( scalars, points, ) } #[cfg(all(curve25519_dalek_backend = "simd", nightly))] BackendKind::Avx512 => { - self::vector::scalar_mul::straus::spec_avx512ifma_avx512vl::Straus::optional_multiscalar_mul::< + vector::scalar_mul::straus::spec_avx512ifma_avx512vl::Straus::optional_multiscalar_mul::< I, J, >(scalars, points) } BackendKind::Serial => { - self::serial::scalar_mul::straus::Straus::optional_multiscalar_mul::( - scalars, points, - ) + serial::scalar_mul::straus::Straus::optional_multiscalar_mul::(scalars, points) } } } @@ -232,12 +227,12 @@ where pub fn variable_base_mul(point: &EdwardsPoint, scalar: &Scalar) -> EdwardsPoint { match get_selected_backend() { #[cfg(curve25519_dalek_backend = "simd")] - BackendKind::Avx2 => self::vector::scalar_mul::variable_base::spec_avx2::mul(point, scalar), + BackendKind::Avx2 => vector::scalar_mul::variable_base::spec_avx2::mul(point, scalar), #[cfg(all(curve25519_dalek_backend = "simd", nightly))] BackendKind::Avx512 => { - self::vector::scalar_mul::variable_base::spec_avx512ifma_avx512vl::mul(point, scalar) + vector::scalar_mul::variable_base::spec_avx512ifma_avx512vl::mul(point, scalar) } - BackendKind::Serial => self::serial::scalar_mul::variable_base::mul(point, scalar), + BackendKind::Serial => serial::scalar_mul::variable_base::mul(point, scalar), } } @@ -246,11 +241,11 @@ pub fn variable_base_mul(point: &EdwardsPoint, scalar: &Scalar) -> EdwardsPoint pub fn vartime_double_base_mul(a: &Scalar, A: &EdwardsPoint, b: &Scalar) -> EdwardsPoint { match get_selected_backend() { #[cfg(curve25519_dalek_backend = "simd")] - BackendKind::Avx2 => self::vector::scalar_mul::vartime_double_base::spec_avx2::mul(a, A, b), + BackendKind::Avx2 => vector::scalar_mul::vartime_double_base::spec_avx2::mul(a, A, b), #[cfg(all(curve25519_dalek_backend = "simd", nightly))] BackendKind::Avx512 => { - self::vector::scalar_mul::vartime_double_base::spec_avx512ifma_avx512vl::mul(a, A, b) + vector::scalar_mul::vartime_double_base::spec_avx512ifma_avx512vl::mul(a, A, b) } - BackendKind::Serial => self::serial::scalar_mul::vartime_double_base::mul(a, A, b), + BackendKind::Serial => serial::scalar_mul::vartime_double_base::mul(a, A, b), } } diff --git a/curve25519-dalek/src/backend/serial/curve_models/mod.rs b/curve25519-dalek/src/backend/serial/curve_models/mod.rs index d482d721a..1343d3706 100644 --- a/curve25519-dalek/src/backend/serial/curve_models/mod.rs +++ b/curve25519-dalek/src/backend/serial/curve_models/mod.rs @@ -527,7 +527,7 @@ impl<'a> Neg for &'a AffineNielsPoint { // ------------------------------------------------------------------------ impl Debug for ProjectivePoint { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!( f, "ProjectivePoint{{\n\tX: {:?},\n\tY: {:?},\n\tZ: {:?}\n}}", @@ -537,7 +537,7 @@ impl Debug for ProjectivePoint { } impl Debug for CompletedPoint { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!( f, "CompletedPoint{{\n\tX: {:?},\n\tY: {:?},\n\tZ: {:?},\n\tT: {:?}\n}}", @@ -547,7 +547,7 @@ impl Debug for CompletedPoint { } impl Debug for AffineNielsPoint { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!( f, "AffineNielsPoint{{\n\ty_plus_x: {:?},\n\ty_minus_x: {:?},\n\txy2d: {:?}\n}}", @@ -557,7 +557,7 @@ impl Debug for AffineNielsPoint { } impl Debug for ProjectiveNielsPoint { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(f, "ProjectiveNielsPoint{{\n\tY_plus_X: {:?},\n\tY_minus_X: {:?},\n\tZ: {:?},\n\tT2d: {:?}\n}}", &self.Y_plus_X, &self.Y_minus_X, &self.Z, &self.T2d) } diff --git a/curve25519-dalek/src/backend/serial/fiat_u32/field.rs b/curve25519-dalek/src/backend/serial/fiat_u32/field.rs index 94e1f6d36..97695c383 100644 --- a/curve25519-dalek/src/backend/serial/fiat_u32/field.rs +++ b/curve25519-dalek/src/backend/serial/fiat_u32/field.rs @@ -58,7 +58,7 @@ use fiat_crypto::curve25519_32::*; pub struct FieldElement2625(pub(crate) fiat_25519_tight_field_element); impl Debug for FieldElement2625 { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(f, "FieldElement2625({:?})", &(self.0).0[..]) } } diff --git a/curve25519-dalek/src/backend/serial/fiat_u64/field.rs b/curve25519-dalek/src/backend/serial/fiat_u64/field.rs index c871b55c2..2a022e23e 100644 --- a/curve25519-dalek/src/backend/serial/fiat_u64/field.rs +++ b/curve25519-dalek/src/backend/serial/fiat_u64/field.rs @@ -47,7 +47,7 @@ use fiat_crypto::curve25519_64::*; pub struct FieldElement51(pub(crate) fiat_25519_tight_field_element); impl Debug for FieldElement51 { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(f, "FieldElement51({:?})", &(self.0).0[..]) } } diff --git a/curve25519-dalek/src/backend/serial/u32/field.rs b/curve25519-dalek/src/backend/serial/u32/field.rs index 4e0b2133b..7319288a0 100644 --- a/curve25519-dalek/src/backend/serial/u32/field.rs +++ b/curve25519-dalek/src/backend/serial/u32/field.rs @@ -54,7 +54,7 @@ use zeroize::Zeroize; pub struct FieldElement2625(pub(crate) [u32; 10]); impl Debug for FieldElement2625 { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(f, "FieldElement2625({:?})", &self.0[..]) } } diff --git a/curve25519-dalek/src/backend/serial/u32/scalar.rs b/curve25519-dalek/src/backend/serial/u32/scalar.rs index c251e8bbe..2d135d1d4 100644 --- a/curve25519-dalek/src/backend/serial/u32/scalar.rs +++ b/curve25519-dalek/src/backend/serial/u32/scalar.rs @@ -24,7 +24,7 @@ use crate::constants; pub struct Scalar29(pub [u32; 9]); impl Debug for Scalar29 { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(f, "Scalar29: {:?}", &self.0[..]) } } diff --git a/curve25519-dalek/src/backend/serial/u64/field.rs b/curve25519-dalek/src/backend/serial/u64/field.rs index 9659effa1..1263d23e4 100644 --- a/curve25519-dalek/src/backend/serial/u64/field.rs +++ b/curve25519-dalek/src/backend/serial/u64/field.rs @@ -43,7 +43,7 @@ use zeroize::Zeroize; pub struct FieldElement51(pub(crate) [u64; 5]); impl Debug for FieldElement51 { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(f, "FieldElement51({:?})", &self.0[..]) } } diff --git a/curve25519-dalek/src/backend/serial/u64/scalar.rs b/curve25519-dalek/src/backend/serial/u64/scalar.rs index dab80cdce..1cc2df4a0 100644 --- a/curve25519-dalek/src/backend/serial/u64/scalar.rs +++ b/curve25519-dalek/src/backend/serial/u64/scalar.rs @@ -25,7 +25,7 @@ use crate::constants; pub struct Scalar52(pub [u64; 5]); impl Debug for Scalar52 { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(f, "Scalar52: {:?}", &self.0[..]) } } diff --git a/curve25519-dalek/src/edwards.rs b/curve25519-dalek/src/edwards.rs index c49590b39..856fac12f 100644 --- a/curve25519-dalek/src/edwards.rs +++ b/curve25519-dalek/src/edwards.rs @@ -171,7 +171,7 @@ impl ConstantTimeEq for CompressedEdwardsY { } impl Debug for CompressedEdwardsY { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(f, "CompressedEdwardsY: {:?}", self.as_bytes()) } } @@ -302,7 +302,7 @@ impl<'de> Deserialize<'de> for EdwardsPoint { impl<'de> Visitor<'de> for EdwardsPointVisitor { type Value = EdwardsPoint; - fn expecting(&self, formatter: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn expecting(&self, formatter: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { formatter.write_str("a valid point in Edwards y + sign format") } @@ -338,7 +338,7 @@ impl<'de> Deserialize<'de> for CompressedEdwardsY { impl<'de> Visitor<'de> for CompressedEdwardsYVisitor { type Value = CompressedEdwardsY; - fn expecting(&self, formatter: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn expecting(&self, formatter: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { formatter.write_str("32 bytes of data") } @@ -1053,7 +1053,7 @@ macro_rules! impl_basepoint_table { } impl Debug for $name { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(f, "{:?}([\n", stringify!($name))?; for i in 0..32 { write!(f, "\t{:?},\n", &self.0[i])?; @@ -1264,7 +1264,7 @@ impl EdwardsPoint { // ------------------------------------------------------------------------ impl Debug for EdwardsPoint { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!( f, "EdwardsPoint{{\n\tX: {:?},\n\tY: {:?},\n\tZ: {:?},\n\tT: {:?}\n}}", diff --git a/curve25519-dalek/src/ristretto.rs b/curve25519-dalek/src/ristretto.rs index 75ba141de..c9d16aba3 100644 --- a/curve25519-dalek/src/ristretto.rs +++ b/curve25519-dalek/src/ristretto.rs @@ -407,7 +407,7 @@ impl<'de> Deserialize<'de> for RistrettoPoint { impl<'de> Visitor<'de> for RistrettoPointVisitor { type Value = RistrettoPoint; - fn expecting(&self, formatter: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn expecting(&self, formatter: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { formatter.write_str("a valid point in Ristretto format") } @@ -443,7 +443,7 @@ impl<'de> Deserialize<'de> for CompressedRistretto { impl<'de> Visitor<'de> for CompressedRistrettoVisitor { type Value = CompressedRistretto; - fn expecting(&self, formatter: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn expecting(&self, formatter: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { formatter.write_str("32 bytes of data") } @@ -1155,13 +1155,13 @@ impl ConditionallySelectable for RistrettoPoint { // ------------------------------------------------------------------------ impl Debug for CompressedRistretto { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(f, "CompressedRistretto: {:?}", self.as_bytes()) } } impl Debug for RistrettoPoint { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { let coset = self.coset4(); write!( f, diff --git a/curve25519-dalek/src/scalar.rs b/curve25519-dalek/src/scalar.rs index 1630af5e7..5e0d1c96b 100644 --- a/curve25519-dalek/src/scalar.rs +++ b/curve25519-dalek/src/scalar.rs @@ -285,7 +285,7 @@ impl Scalar { } impl Debug for Scalar { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(f, "Scalar{{\n\tbytes: {:?},\n}}", &self.bytes) } } @@ -430,7 +430,7 @@ impl<'de> Deserialize<'de> for Scalar { impl<'de> Visitor<'de> for ScalarVisitor { type Value = Scalar; - fn expecting(&self, formatter: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn expecting(&self, formatter: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { formatter.write_str( "a sequence of 32 bytes whose little-endian interpretation is less than the \ basepoint order ℓ", @@ -831,7 +831,7 @@ impl Scalar { } #[cfg(feature = "zeroize")] - zeroize::Zeroize::zeroize(&mut scratch); + Zeroize::zeroize(&mut scratch); ret } diff --git a/curve25519-dalek/src/window.rs b/curve25519-dalek/src/window.rs index 8c575ee04..43c4b3abb 100644 --- a/curve25519-dalek/src/window.rs +++ b/curve25519-dalek/src/window.rs @@ -83,7 +83,7 @@ macro_rules! impl_lookup_table { } impl Debug for $name { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(f, "{:?}(", stringify!($name))?; for x in self.0.iter() { @@ -193,7 +193,7 @@ impl NafLookupTable5 { } impl Debug for NafLookupTable5 { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(f, "NafLookupTable5({:?})", self.0) } } @@ -240,7 +240,7 @@ impl NafLookupTable8 { #[cfg(any(feature = "precomputed-tables", feature = "alloc"))] impl Debug for NafLookupTable8 { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { writeln!(f, "NafLookupTable8([")?; for i in 0..64 { writeln!(f, "\t{:?},", &self.0[i])?; diff --git a/ed25519-dalek/src/signature.rs b/ed25519-dalek/src/signature.rs index 32fde3016..af8276834 100644 --- a/ed25519-dalek/src/signature.rs +++ b/ed25519-dalek/src/signature.rs @@ -57,7 +57,7 @@ impl Clone for InternalSignature { } impl Debug for InternalSignature { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(f, "Signature( R: {:?}, s: {:?} )", &self.R, &self.s) } } diff --git a/ed25519-dalek/src/signing.rs b/ed25519-dalek/src/signing.rs index e2818fea7..e63d34bb7 100644 --- a/ed25519-dalek/src/signing.rs +++ b/ed25519-dalek/src/signing.rs @@ -543,7 +543,7 @@ impl AsRef for SigningKey { } impl Debug for SigningKey { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { f.debug_struct("SigningKey") .field("verifying_key", &self.verifying_key) .finish_non_exhaustive() // avoids printing `secret_key` @@ -742,7 +742,7 @@ impl<'d> Deserialize<'d> for SigningKey { impl<'de> serde::de::Visitor<'de> for SigningKeyVisitor { type Value = SigningKey; - fn expecting(&self, formatter: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn expecting(&self, formatter: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(formatter, concat!("An ed25519 signing (private) key")) } diff --git a/ed25519-dalek/src/verifying.rs b/ed25519-dalek/src/verifying.rs index 0d154678b..246951b44 100644 --- a/ed25519-dalek/src/verifying.rs +++ b/ed25519-dalek/src/verifying.rs @@ -642,7 +642,7 @@ impl<'d> Deserialize<'d> for VerifyingKey { impl<'de> serde::de::Visitor<'de> for VerifyingKeyVisitor { type Value = VerifyingKey; - fn expecting(&self, formatter: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + fn expecting(&self, formatter: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(formatter, concat!("An ed25519 verifying (public) key")) } From cc3421a22fa7ee1f557cbe9243b450da53bbe962 Mon Sep 17 00:00:00 2001 From: Boyd Kane <33420535+beyarkay@users.noreply.github.com> Date: Sat, 16 Mar 2024 15:43:25 +0200 Subject: [PATCH 04/17] Indicate that the rand_core feature is required (#641) --- ed25519-dalek/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/ed25519-dalek/src/lib.rs b/ed25519-dalek/src/lib.rs index a7cfac488..21d8737ba 100644 --- a/ed25519-dalek/src/lib.rs +++ b/ed25519-dalek/src/lib.rs @@ -21,6 +21,7 @@ #![cfg_attr(feature = "rand_core", doc = "```")] #![cfg_attr(not(feature = "rand_core"), doc = "```ignore")] //! # fn main() { +//! // $ cargo add ed25519_dalek --features rand_core //! use rand::rngs::OsRng; //! use ed25519_dalek::SigningKey; //! use ed25519_dalek::Signature; From 1efe6a93b176c4389b78e81e52b2cf85d728aac6 Mon Sep 17 00:00:00 2001 From: Hiroki Kobayashi <3303362+koba-e964@users.noreply.github.com> Date: Sun, 14 Apr 2024 10:37:33 +0900 Subject: [PATCH 05/17] Fix a minor typo in signing.rs (#649) an -> a --- ed25519-dalek/src/signing.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ed25519-dalek/src/signing.rs b/ed25519-dalek/src/signing.rs index e63d34bb7..8999f50d2 100644 --- a/ed25519-dalek/src/signing.rs +++ b/ed25519-dalek/src/signing.rs @@ -131,7 +131,7 @@ impl SigningKey { /// # Returns /// /// A `Result` whose okay value is an EdDSA [`SigningKey`] or whose error value - /// is an `SignatureError` describing the error that occurred. + /// is a `SignatureError` describing the error that occurred. #[inline] pub fn from_keypair_bytes(bytes: &[u8; 64]) -> Result { let (secret_key, verifying_key) = bytes.split_at(SECRET_KEY_LENGTH); From 9252fa5c0d09054fed4ac4d649e63c40fad7abaf Mon Sep 17 00:00:00 2001 From: "pinkforest(she/her)" <36498018+pinkforest@users.noreply.github.com> Date: Thu, 9 May 2024 23:24:16 +1000 Subject: [PATCH 06/17] Mitigate check-cfg until MSRV 1.77 (#652) --- curve25519-dalek-derive/tests/tests.rs | 9 +-------- curve25519-dalek/src/lib.rs | 2 ++ curve25519-dalek/src/scalar.rs | 2 ++ x25519-dalek/src/lib.rs | 1 - 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/curve25519-dalek-derive/tests/tests.rs b/curve25519-dalek-derive/tests/tests.rs index 1516b3527..dce30c864 100644 --- a/curve25519-dalek-derive/tests/tests.rs +++ b/curve25519-dalek-derive/tests/tests.rs @@ -23,10 +23,6 @@ where a - b } -#[unsafe_target_feature("sse2")] -#[cfg(feature = "dummy")] -fn function_with_cfg() {} - #[unsafe_target_feature("sse2")] #[rustfmt::skip] fn function_with_rustfmt_skip() {} @@ -45,9 +41,6 @@ impl Struct { fn member_function_with_const_arg(self) -> u32 { self.a - N } - - #[cfg(feature = "dummy")] - fn member_function_with_cfg() {} } struct StructWithGenerics @@ -93,7 +86,7 @@ mod inner { } } -#[unsafe_target_feature_specialize("sse2", "avx2", conditional("avx512ifma", disabled))] +#[unsafe_target_feature_specialize("sse2", "avx2")] mod inner_spec { #[for_target_feature("sse2")] const CONST: u32 = 1; diff --git a/curve25519-dalek/src/lib.rs b/curve25519-dalek/src/lib.rs index d8666453c..fecfe888c 100644 --- a/curve25519-dalek/src/lib.rs +++ b/curve25519-dalek/src/lib.rs @@ -42,6 +42,8 @@ unused_lifetimes, unused_qualifications )] +// Requires MSRV 1.77 as it does not allow build.rs gating +#![allow(unexpected_cfgs)] //------------------------------------------------------------------------ // External dependencies: diff --git a/curve25519-dalek/src/scalar.rs b/curve25519-dalek/src/scalar.rs index 5e0d1c96b..6afd74eef 100644 --- a/curve25519-dalek/src/scalar.rs +++ b/curve25519-dalek/src/scalar.rs @@ -1233,10 +1233,12 @@ impl Field for Scalar { } fn sqrt_ratio(num: &Self, div: &Self) -> (Choice, Self) { + #[allow(unused_qualifications)] group::ff::helpers::sqrt_ratio_generic(num, div) } fn sqrt(&self) -> CtOption { + #[allow(unused_qualifications)] group::ff::helpers::sqrt_tonelli_shanks( self, [ diff --git a/x25519-dalek/src/lib.rs b/x25519-dalek/src/lib.rs index 9a5fc1938..7886eeaa5 100644 --- a/x25519-dalek/src/lib.rs +++ b/x25519-dalek/src/lib.rs @@ -15,7 +15,6 @@ // README.md as the crate documentation. #![no_std] -#![cfg_attr(feature = "bench", feature(test))] #![cfg_attr(docsrs, feature(doc_auto_cfg, doc_cfg, doc_cfg_hide))] #![cfg_attr(docsrs, doc(cfg_hide(docsrs)))] #![deny(missing_docs)] From 56bf398d0caed63ef1d1edfbd35eb5335132aba2 Mon Sep 17 00:00:00 2001 From: Isaiah Becker-Mayer Date: Mon, 3 Jun 2024 15:30:13 -0500 Subject: [PATCH 07/17] Updates license field to valid SPDX format (#647) --- curve25519-dalek-derive/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/curve25519-dalek-derive/Cargo.toml b/curve25519-dalek-derive/Cargo.toml index 938144a0f..aed791c78 100644 --- a/curve25519-dalek-derive/Cargo.toml +++ b/curve25519-dalek-derive/Cargo.toml @@ -6,7 +6,7 @@ edition = "2021" repository = "https://github.com/dalek-cryptography/curve25519-dalek" homepage = "https://github.com/dalek-cryptography/curve25519-dalek" documentation = "https://docs.rs/curve25519-dalek-derive" -license = "MIT/Apache-2.0" +license = "MIT OR Apache-2.0" readme = "README.md" description = "curve25519-dalek Derives" From 415892acf1cdf9161bd6a4c99bc2f4cb8fae5e6a Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Tue, 18 Jun 2024 19:49:31 +0200 Subject: [PATCH 08/17] SECURITY: fix timing variability in backend/serial/u64/scalar.rs (#659) Timing variability of any kind is problematic when working with potentially secret values such as elliptic curve scalars, and such issues can potentially leak private keys and other secrets. Such a problem was recently discovered in `curve25519-dalek`. The `Scalar52::sub` function contained usage of a mask value inside of a loop where LLVM saw an opportunity to insert a branch instruction (`jns` on x86) to conditionally bypass this code section when the mask value is set to zero, as can be seen in godbolt: https://godbolt.org/z/PczYj7Pda A similar problem was recently discovered in the Kyber reference implementation: https://groups.google.com/a/list.nist.gov/g/pqc-forum/c/hqbtIGFKIpU/m/cnE3pbueBgAJ As discussed on that thread, one portable solution, which is also used in this PR, is to introduce a volatile read as an optimization barrier, which prevents the compiler from optimizing it away. The fix can be validated in godbolt here: https://godbolt.org/z/x8d46Yfah The problem was discovered and the solution independently verified by Alexander Wagner and Lea Themint using their DATA tool: https://github.com/Fraunhofer-AISEC/DATA Co-authored-by: Tony Arcieri --- curve25519-dalek/src/backend/serial/u64/scalar.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/curve25519-dalek/src/backend/serial/u64/scalar.rs b/curve25519-dalek/src/backend/serial/u64/scalar.rs index 1cc2df4a0..6c0eaf796 100644 --- a/curve25519-dalek/src/backend/serial/u64/scalar.rs +++ b/curve25519-dalek/src/backend/serial/u64/scalar.rs @@ -174,6 +174,14 @@ impl Scalar52 { /// Compute `a - b` (mod l) pub fn sub(a: &Scalar52, b: &Scalar52) -> Scalar52 { + // Optimization barrier to prevent compiler from inserting branch instructions + // TODO(tarcieri): find a better home (or abstraction) for this + fn black_box(value: u64) -> u64 { + // SAFETY: `u64` is a simple integer `Copy` type and `value` lives on the stack so + // a pointer to it will be valid. + unsafe { core::ptr::read_volatile(&value) } + } + let mut difference = Scalar52::ZERO; let mask = (1u64 << 52) - 1; @@ -188,7 +196,9 @@ impl Scalar52 { let underflow_mask = ((borrow >> 63) ^ 1).wrapping_sub(1); let mut carry: u64 = 0; for i in 0..5 { - carry = (carry >> 52) + difference[i] + (constants::L[i] & underflow_mask); + // SECURITY: `black_box` prevents LLVM from inserting a `jns` conditional on x86(_64) + // which can be used to bypass this section when `underflow_mask` is zero. + carry = (carry >> 52) + difference[i] + (constants::L[i] & black_box(underflow_mask)); difference[i] = carry & mask; } From b4f9e4df92a4689fb59e312a21f940ba06ba7013 Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Tue, 18 Jun 2024 13:02:37 -0600 Subject: [PATCH 09/17] SECURITY: fix timing variability in backend/serial/u32/scalar.rs (#661) Similar security fix to #659, but for the 32-bit backend. See that PR for more information about the problem. Relevant compiler outputs (thanks to @tarcieri): Without fix https://godbolt.org/z/zvaWxzvqv Notice the `jns` ("jump if not sign") instruction on line 106. With fix https://godbolt.org/z/jc9j7eb8E --- curve25519-dalek/src/backend/serial/u32/scalar.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/curve25519-dalek/src/backend/serial/u32/scalar.rs b/curve25519-dalek/src/backend/serial/u32/scalar.rs index 2d135d1d4..d3df38c80 100644 --- a/curve25519-dalek/src/backend/serial/u32/scalar.rs +++ b/curve25519-dalek/src/backend/serial/u32/scalar.rs @@ -185,6 +185,14 @@ impl Scalar29 { /// Compute `a - b` (mod l). pub fn sub(a: &Scalar29, b: &Scalar29) -> Scalar29 { + // Optimization barrier to prevent compiler from inserting branch instructions + // TODO(tarcieri): find a better home (or abstraction) for this + fn black_box(value: u32) -> u32 { + // SAFETY: `u32` is a simple integer `Copy` type and `value` lives on the stack so + // a pointer to it will be valid. + unsafe { core::ptr::read_volatile(&value) } + } + let mut difference = Scalar29::ZERO; let mask = (1u32 << 29) - 1; @@ -199,7 +207,7 @@ impl Scalar29 { let underflow_mask = ((borrow >> 31) ^ 1).wrapping_sub(1); let mut carry: u32 = 0; for i in 0..9 { - carry = (carry >> 29) + difference[i] + (constants::L[i] & underflow_mask); + carry = (carry >> 29) + difference[i] + (constants::L[i] & black_box(underflow_mask)); difference[i] = carry & mask; } From 5312a0311ec40df95be953eacfa8a11b9a34bc54 Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Tue, 18 Jun 2024 21:18:51 +0200 Subject: [PATCH 10/17] curve: Bump version to 4.1.3 (#660) * Bumped to v4.1.3 * Added recent PRs to changelog --- curve25519-dalek/CHANGELOG.md | 5 +++++ curve25519-dalek/Cargo.toml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/curve25519-dalek/CHANGELOG.md b/curve25519-dalek/CHANGELOG.md index a4c8452cc..939d3bfba 100644 --- a/curve25519-dalek/CHANGELOG.md +++ b/curve25519-dalek/CHANGELOG.md @@ -5,6 +5,11 @@ major series. ## 4.x series +### 4.1.3 + +* Security: Fix timing leak in Scalar subtraction on u32, u64, fiat_u32, and fiat_u64 backends +* Fix assorted new warnings and lints from rustc and clippy + ### 4.1.2 * Fix nightly SIMD build diff --git a/curve25519-dalek/Cargo.toml b/curve25519-dalek/Cargo.toml index f8c7c3a8e..8e480ada5 100644 --- a/curve25519-dalek/Cargo.toml +++ b/curve25519-dalek/Cargo.toml @@ -4,7 +4,7 @@ name = "curve25519-dalek" # - update CHANGELOG # - update README if required by semver # - if README was updated, also update module documentation in src/lib.rs -version = "4.1.2" +version = "4.1.3" edition = "2021" rust-version = "1.60.0" authors = ["Isis Lovecruft ", From 5b7082bbc8e0b2106ab0d956064f61fa0f393cdc Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Mon, 24 Jun 2024 12:13:54 -0600 Subject: [PATCH 11/17] curve: use `subtle::BlackBox` optimization barrier (#662) Replaces the security mitigation added in #659 and #661 for masking-related timing variability which used an inline `black_box` using the recently added `subtle::BlackBox` newtype (see dalek-cryptography/subtle#123) Internally `BlackBox` uses a volatile read by default (i.e. same strategy which was used before) or when the `core_hint_black_box` feature of `subtle` is enabled, it uses `core::hint::black_box` (whose documentation was recently updated to reflect the nuances of potential cryptographic use, see rust-lang/rust#126703) This PR goes ahead and uses `BlackBox` for both `mask` and `underflow_mask` where previously it was only used on `underflow_mask`. The general pattern of bitwise masking inside a loop seems worrisome for the optimizer potentially inserting branches in the future. Below are godbolt inspections of the generated assembly, which are free of the `jns` instructions originally spotted in #659/#661: - 32-bit (read_volatile): https://godbolt.org/z/TKo9fqza4 - 32-bit (hint::black_box): https://godbolt.org/z/caoMxYbET - 64-bit (read_volatile): https://godbolt.org/z/PM6zKjj1f - 64-bit (hint::black_box): https://godbolt.org/z/nseaPvdWv --- curve25519-dalek/Cargo.toml | 2 +- .../src/backend/serial/u32/scalar.rs | 21 +++++++------------ .../src/backend/serial/u64/scalar.rs | 21 +++++++------------ 3 files changed, 16 insertions(+), 28 deletions(-) diff --git a/curve25519-dalek/Cargo.toml b/curve25519-dalek/Cargo.toml index 8e480ada5..dc3b2b4eb 100644 --- a/curve25519-dalek/Cargo.toml +++ b/curve25519-dalek/Cargo.toml @@ -51,7 +51,7 @@ ff = { version = "0.13", default-features = false, optional = true } group = { version = "0.13", default-features = false, optional = true } rand_core = { version = "0.6.4", default-features = false, optional = true } digest = { version = "0.10", default-features = false, optional = true } -subtle = { version = "2.3.0", default-features = false } +subtle = { version = "2.6.0", default-features = false } serde = { version = "1.0", default-features = false, optional = true, features = ["derive"] } zeroize = { version = "1", default-features = false, optional = true } diff --git a/curve25519-dalek/src/backend/serial/u32/scalar.rs b/curve25519-dalek/src/backend/serial/u32/scalar.rs index d3df38c80..10eadd1bf 100644 --- a/curve25519-dalek/src/backend/serial/u32/scalar.rs +++ b/curve25519-dalek/src/backend/serial/u32/scalar.rs @@ -12,6 +12,7 @@ use core::fmt::Debug; use core::ops::{Index, IndexMut}; +use subtle::BlackBox; #[cfg(feature = "zeroize")] use zeroize::Zeroize; @@ -185,30 +186,24 @@ impl Scalar29 { /// Compute `a - b` (mod l). pub fn sub(a: &Scalar29, b: &Scalar29) -> Scalar29 { - // Optimization barrier to prevent compiler from inserting branch instructions - // TODO(tarcieri): find a better home (or abstraction) for this - fn black_box(value: u32) -> u32 { - // SAFETY: `u32` is a simple integer `Copy` type and `value` lives on the stack so - // a pointer to it will be valid. - unsafe { core::ptr::read_volatile(&value) } - } - let mut difference = Scalar29::ZERO; - let mask = (1u32 << 29) - 1; + let mask = BlackBox::new((1u32 << 29) - 1); // a - b let mut borrow: u32 = 0; for i in 0..9 { borrow = a[i].wrapping_sub(b[i] + (borrow >> 31)); - difference[i] = borrow & mask; + difference[i] = borrow & mask.get(); } // conditionally add l if the difference is negative - let underflow_mask = ((borrow >> 31) ^ 1).wrapping_sub(1); + let underflow_mask = BlackBox::new(((borrow >> 31) ^ 1).wrapping_sub(1)); let mut carry: u32 = 0; for i in 0..9 { - carry = (carry >> 29) + difference[i] + (constants::L[i] & black_box(underflow_mask)); - difference[i] = carry & mask; + // SECURITY: `BlackBox` prevents LLVM from inserting a `jns` conditional on x86(_64) + // which can be used to bypass this section when `underflow_mask` is zero. + carry = (carry >> 29) + difference[i] + (constants::L[i] & underflow_mask.get()); + difference[i] = carry & mask.get(); } difference diff --git a/curve25519-dalek/src/backend/serial/u64/scalar.rs b/curve25519-dalek/src/backend/serial/u64/scalar.rs index 6c0eaf796..ade16f31a 100644 --- a/curve25519-dalek/src/backend/serial/u64/scalar.rs +++ b/curve25519-dalek/src/backend/serial/u64/scalar.rs @@ -13,6 +13,7 @@ use core::fmt::Debug; use core::ops::{Index, IndexMut}; +use subtle::BlackBox; #[cfg(feature = "zeroize")] use zeroize::Zeroize; @@ -174,32 +175,24 @@ impl Scalar52 { /// Compute `a - b` (mod l) pub fn sub(a: &Scalar52, b: &Scalar52) -> Scalar52 { - // Optimization barrier to prevent compiler from inserting branch instructions - // TODO(tarcieri): find a better home (or abstraction) for this - fn black_box(value: u64) -> u64 { - // SAFETY: `u64` is a simple integer `Copy` type and `value` lives on the stack so - // a pointer to it will be valid. - unsafe { core::ptr::read_volatile(&value) } - } - let mut difference = Scalar52::ZERO; - let mask = (1u64 << 52) - 1; + let mask = BlackBox::new((1u64 << 52) - 1); // a - b let mut borrow: u64 = 0; for i in 0..5 { borrow = a[i].wrapping_sub(b[i] + (borrow >> 63)); - difference[i] = borrow & mask; + difference[i] = borrow & mask.get(); } // conditionally add l if the difference is negative - let underflow_mask = ((borrow >> 63) ^ 1).wrapping_sub(1); + let underflow_mask = BlackBox::new(((borrow >> 63) ^ 1).wrapping_sub(1)); let mut carry: u64 = 0; for i in 0..5 { - // SECURITY: `black_box` prevents LLVM from inserting a `jns` conditional on x86(_64) + // SECURITY: `BlackBox` prevents LLVM from inserting a `jns` conditional on x86(_64) // which can be used to bypass this section when `underflow_mask` is zero. - carry = (carry >> 52) + difference[i] + (constants::L[i] & black_box(underflow_mask)); - difference[i] = carry & mask; + carry = (carry >> 52) + difference[i] + (constants::L[i] & underflow_mask.get()); + difference[i] = carry & mask.get(); } difference From 921bd7ced0af512677ee8e1d5b05ba26417696b9 Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Wed, 17 Jul 2024 12:05:46 -0600 Subject: [PATCH 12/17] curve: use `subtle::Choice` for constant-time fixes (#665) Alternative to #659/#661 and #662 which leverages `subtle::Choice` and `subtle::ConditionallySelectable` as the optimization barriers. Really the previous masking was there to conditionally add the scalar field modulus on underflow, so instead of that, we can conditionally select zero or the modulus using a `Choice` constructed from the underflow bit. --- curve25519-dalek/src/backend/serial/u32/scalar.rs | 15 +++++++-------- curve25519-dalek/src/backend/serial/u64/scalar.rs | 15 +++++++-------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/curve25519-dalek/src/backend/serial/u32/scalar.rs b/curve25519-dalek/src/backend/serial/u32/scalar.rs index 10eadd1bf..b0f6bd09f 100644 --- a/curve25519-dalek/src/backend/serial/u32/scalar.rs +++ b/curve25519-dalek/src/backend/serial/u32/scalar.rs @@ -12,7 +12,7 @@ use core::fmt::Debug; use core::ops::{Index, IndexMut}; -use subtle::BlackBox; +use subtle::{Choice, ConditionallySelectable}; #[cfg(feature = "zeroize")] use zeroize::Zeroize; @@ -187,23 +187,22 @@ impl Scalar29 { /// Compute `a - b` (mod l). pub fn sub(a: &Scalar29, b: &Scalar29) -> Scalar29 { let mut difference = Scalar29::ZERO; - let mask = BlackBox::new((1u32 << 29) - 1); + let mask = (1u32 << 29) - 1; // a - b let mut borrow: u32 = 0; for i in 0..9 { borrow = a[i].wrapping_sub(b[i] + (borrow >> 31)); - difference[i] = borrow & mask.get(); + difference[i] = borrow & mask; } // conditionally add l if the difference is negative - let underflow_mask = BlackBox::new(((borrow >> 31) ^ 1).wrapping_sub(1)); let mut carry: u32 = 0; for i in 0..9 { - // SECURITY: `BlackBox` prevents LLVM from inserting a `jns` conditional on x86(_64) - // which can be used to bypass this section when `underflow_mask` is zero. - carry = (carry >> 29) + difference[i] + (constants::L[i] & underflow_mask.get()); - difference[i] = carry & mask.get(); + let underflow = Choice::from((borrow >> 31) as u8); + let addend = u32::conditional_select(&0, &constants::L[i], underflow); + carry = (carry >> 29) + difference[i] + addend; + difference[i] = carry & mask; } difference diff --git a/curve25519-dalek/src/backend/serial/u64/scalar.rs b/curve25519-dalek/src/backend/serial/u64/scalar.rs index ade16f31a..57e2d9b01 100644 --- a/curve25519-dalek/src/backend/serial/u64/scalar.rs +++ b/curve25519-dalek/src/backend/serial/u64/scalar.rs @@ -13,7 +13,7 @@ use core::fmt::Debug; use core::ops::{Index, IndexMut}; -use subtle::BlackBox; +use subtle::{Choice, ConditionallySelectable}; #[cfg(feature = "zeroize")] use zeroize::Zeroize; @@ -176,23 +176,22 @@ impl Scalar52 { /// Compute `a - b` (mod l) pub fn sub(a: &Scalar52, b: &Scalar52) -> Scalar52 { let mut difference = Scalar52::ZERO; - let mask = BlackBox::new((1u64 << 52) - 1); + let mask = (1u64 << 52) - 1; // a - b let mut borrow: u64 = 0; for i in 0..5 { borrow = a[i].wrapping_sub(b[i] + (borrow >> 63)); - difference[i] = borrow & mask.get(); + difference[i] = borrow & mask; } // conditionally add l if the difference is negative - let underflow_mask = BlackBox::new(((borrow >> 63) ^ 1).wrapping_sub(1)); let mut carry: u64 = 0; for i in 0..5 { - // SECURITY: `BlackBox` prevents LLVM from inserting a `jns` conditional on x86(_64) - // which can be used to bypass this section when `underflow_mask` is zero. - carry = (carry >> 52) + difference[i] + (constants::L[i] & underflow_mask.get()); - difference[i] = carry & mask.get(); + let underflow = Choice::from((borrow >> 63) as u8); + let addend = u64::conditional_select(&0, &constants::L[i], underflow); + carry = (carry >> 52) + difference[i] + addend; + difference[i] = carry & mask; } difference From 35e78b21efb61a1de148cbcf4cfe2c3d559fbec0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20W=C3=BClker?= Date: Fri, 26 Jul 2024 04:24:39 +0200 Subject: [PATCH 13/17] Enable unexpected cfgs lint (#656) Previously, the only way to configure this lint was using a build.rs file, but now it can be done using Cargo.toml as well. --- curve25519-dalek/Cargo.toml | 10 ++++++++++ curve25519-dalek/src/backend/vector/ifma/edwards.rs | 2 +- curve25519-dalek/src/backend/vector/ifma/field.rs | 2 +- curve25519-dalek/src/lib.rs | 2 -- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/curve25519-dalek/Cargo.toml b/curve25519-dalek/Cargo.toml index dc3b2b4eb..f0896ba74 100644 --- a/curve25519-dalek/Cargo.toml +++ b/curve25519-dalek/Cargo.toml @@ -71,3 +71,13 @@ group-bits = ["group", "ff/bits"] [target.'cfg(all(not(curve25519_dalek_backend = "fiat"), not(curve25519_dalek_backend = "serial"), target_arch = "x86_64"))'.dependencies] curve25519-dalek-derive = { version = "0.1", path = "../curve25519-dalek-derive" } + +[lints.rust.unexpected_cfgs] +level = "warn" +check-cfg = [ + 'cfg(allow_unused_unsafe)', + 'cfg(curve25519_dalek_backend, values("fiat", "serial", "simd"))', + 'cfg(curve25519_dalek_diagnostics, values("build"))', + 'cfg(curve25519_dalek_bits, values("32", "64"))', + 'cfg(nightly)', +] diff --git a/curve25519-dalek/src/backend/vector/ifma/edwards.rs b/curve25519-dalek/src/backend/vector/ifma/edwards.rs index f8605fe52..3de7030a7 100644 --- a/curve25519-dalek/src/backend/vector/ifma/edwards.rs +++ b/curve25519-dalek/src/backend/vector/ifma/edwards.rs @@ -249,7 +249,7 @@ impl<'a> From<&'a edwards::EdwardsPoint> for NafLookupTable8 { } } -#[cfg(target_feature = "avx512ifma,avx512vl")] +#[cfg(all(target_feature = "avx512ifma", target_feature = "avx512vl"))] #[cfg(test)] mod test { use super::*; diff --git a/curve25519-dalek/src/backend/vector/ifma/field.rs b/curve25519-dalek/src/backend/vector/ifma/field.rs index fa8ce2dd9..ebdfba1bd 100644 --- a/curve25519-dalek/src/backend/vector/ifma/field.rs +++ b/curve25519-dalek/src/backend/vector/ifma/field.rs @@ -629,7 +629,7 @@ impl<'a, 'b> Mul<&'b F51x4Reduced> for &'a F51x4Reduced { } } -#[cfg(target_feature = "avx512ifma,avx512vl")] +#[cfg(all(target_feature = "avx512ifma", target_feature = "avx512vl"))] #[cfg(test)] mod test { use super::*; diff --git a/curve25519-dalek/src/lib.rs b/curve25519-dalek/src/lib.rs index fecfe888c..d8666453c 100644 --- a/curve25519-dalek/src/lib.rs +++ b/curve25519-dalek/src/lib.rs @@ -42,8 +42,6 @@ unused_lifetimes, unused_qualifications )] -// Requires MSRV 1.77 as it does not allow build.rs gating -#![allow(unexpected_cfgs)] //------------------------------------------------------------------------ // External dependencies: From a7a9fffdc98cea50c24b5cdaa0da706b01ed48df Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Tue, 30 Jul 2024 00:11:26 -0500 Subject: [PATCH 14/17] Minor documentation fixes (#671) --- curve25519-dalek/src/edwards.rs | 6 +++--- curve25519-dalek/src/ristretto.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/curve25519-dalek/src/edwards.rs b/curve25519-dalek/src/edwards.rs index 856fac12f..32102887f 100644 --- a/curve25519-dalek/src/edwards.rs +++ b/curve25519-dalek/src/edwards.rs @@ -19,8 +19,8 @@ //! ## Equality Testing //! //! The `EdwardsPoint` struct implements the [`subtle::ConstantTimeEq`] -//! trait for constant-time equality checking, and the Rust `Eq` trait -//! for variable-time equality checking. +//! trait for constant-time equality checking, and also uses this to +//! ensure `Eq` equality checking runs in constant time. //! //! ## Cofactor-related functions //! @@ -438,7 +438,7 @@ impl Zeroize for CompressedEdwardsY { #[cfg(feature = "zeroize")] impl Zeroize for EdwardsPoint { - /// Reset this `CompressedEdwardsPoint` to the identity element. + /// Reset this `EdwardsPoint` to the identity element. fn zeroize(&mut self) { self.X.zeroize(); self.Y = FieldElement::ONE; diff --git a/curve25519-dalek/src/ristretto.rs b/curve25519-dalek/src/ristretto.rs index c9d16aba3..5f4e58264 100644 --- a/curve25519-dalek/src/ristretto.rs +++ b/curve25519-dalek/src/ristretto.rs @@ -76,9 +76,9 @@ //! coordinates without requiring an inversion, so it is much faster. //! //! The `RistrettoPoint` struct implements the -//! `subtle::ConstantTimeEq` trait for constant-time equality -//! checking, and the Rust `Eq` trait for variable-time equality -//! checking. +//! [`subtle::ConstantTimeEq`] trait for constant-time equality +//! checking, and also uses this to ensure `Eq` equality checking +//! runs in constant time. //! //! ## Scalars //! From 79ab6c29bd90c37d89a34a6279b92ae594c44e94 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Tue, 30 Jul 2024 08:51:44 +0300 Subject: [PATCH 15/17] curve: Implement ConditionallySelectable for MontgomeryPoint (#677) --- curve25519-dalek/Cargo.toml | 2 +- curve25519-dalek/src/montgomery.rs | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/curve25519-dalek/Cargo.toml b/curve25519-dalek/Cargo.toml index f0896ba74..c4fb28a38 100644 --- a/curve25519-dalek/Cargo.toml +++ b/curve25519-dalek/Cargo.toml @@ -51,7 +51,7 @@ ff = { version = "0.13", default-features = false, optional = true } group = { version = "0.13", default-features = false, optional = true } rand_core = { version = "0.6.4", default-features = false, optional = true } digest = { version = "0.10", default-features = false, optional = true } -subtle = { version = "2.6.0", default-features = false } +subtle = { version = "2.6.0", default-features = false, features = ["const-generics"]} serde = { version = "1.0", default-features = false, optional = true, features = ["derive"] } zeroize = { version = "1", default-features = false, optional = true } diff --git a/curve25519-dalek/src/montgomery.rs b/curve25519-dalek/src/montgomery.rs index 2be35cdc7..cb5e2fb87 100644 --- a/curve25519-dalek/src/montgomery.rs +++ b/curve25519-dalek/src/montgomery.rs @@ -84,6 +84,12 @@ impl ConstantTimeEq for MontgomeryPoint { } } +impl ConditionallySelectable for MontgomeryPoint { + fn conditional_select(a: &Self, b: &Self, choice: Choice) -> Self { + Self(<[u8; 32]>::conditional_select(&a.0, &b.0, choice)) + } +} + impl PartialEq for MontgomeryPoint { fn eq(&self, other: &MontgomeryPoint) -> bool { self.ct_eq(other).into() From 83a57e591fb6fdd7f879cbf28278737fee1f5479 Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Tue, 30 Jul 2024 01:05:52 -0500 Subject: [PATCH 16/17] curve: Impl `Default` `ConstantTImeEq` and `ConditionallySelectable` for `SubgroupPoint` (#672) --- curve25519-dalek/src/edwards.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/curve25519-dalek/src/edwards.rs b/curve25519-dalek/src/edwards.rs index 32102887f..53ad4fef7 100644 --- a/curve25519-dalek/src/edwards.rs +++ b/curve25519-dalek/src/edwards.rs @@ -1335,7 +1335,7 @@ impl GroupEncoding for EdwardsPoint { /// A `SubgroupPoint` represents a point on the Edwards form of Curve25519, that is /// guaranteed to be in the prime-order subgroup. #[cfg(feature = "group")] -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub struct SubgroupPoint(EdwardsPoint); #[cfg(feature = "group")] @@ -1510,6 +1510,20 @@ impl MulAssign<&Scalar> for SubgroupPoint { #[cfg(feature = "group")] define_mul_assign_variants!(LHS = SubgroupPoint, RHS = Scalar); +#[cfg(feature = "group")] +impl ConstantTimeEq for SubgroupPoint { + fn ct_eq(&self, other: &SubgroupPoint) -> Choice { + self.0.ct_eq(&other.0) + } +} + +#[cfg(feature = "group")] +impl ConditionallySelectable for SubgroupPoint { + fn conditional_select(a: &SubgroupPoint, b: &SubgroupPoint, choice: Choice) -> SubgroupPoint { + SubgroupPoint(EdwardsPoint::conditional_select(&a.0, &b.0, choice)) + } +} + #[cfg(feature = "group")] impl group::Group for SubgroupPoint { type Scalar = Scalar; From 0964f800ab2114a862543ca000291a6e3531c203 Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Tue, 30 Jul 2024 08:43:13 -0500 Subject: [PATCH 17/17] curve: Support MSM #static scalars <= #static points (#668) --- .../serial/scalar_mul/precomputed_straus.rs | 4 +- .../vector/scalar_mul/precomputed_straus.rs | 4 +- curve25519-dalek/src/ristretto.rs | 144 ++++++++++++++++++ curve25519-dalek/src/traits.rs | 22 ++- 4 files changed, 163 insertions(+), 11 deletions(-) diff --git a/curve25519-dalek/src/backend/serial/scalar_mul/precomputed_straus.rs b/curve25519-dalek/src/backend/serial/scalar_mul/precomputed_straus.rs index 711649e21..53116c628 100644 --- a/curve25519-dalek/src/backend/serial/scalar_mul/precomputed_straus.rs +++ b/curve25519-dalek/src/backend/serial/scalar_mul/precomputed_straus.rs @@ -75,7 +75,7 @@ impl VartimePrecomputedMultiscalarMul for VartimePrecomputedStraus { let sp = self.static_lookup_tables.len(); let dp = dynamic_lookup_tables.len(); - assert_eq!(sp, static_nafs.len()); + assert!(sp >= static_nafs.len()); assert_eq!(dp, dynamic_nafs.len()); // We could save some doublings by looking for the highest @@ -99,7 +99,7 @@ impl VartimePrecomputedMultiscalarMul for VartimePrecomputedStraus { } #[allow(clippy::needless_range_loop)] - for i in 0..sp { + for i in 0..static_nafs.len() { let t_ij = static_nafs[i][j]; match t_ij.cmp(&0) { Ordering::Greater => { diff --git a/curve25519-dalek/src/backend/vector/scalar_mul/precomputed_straus.rs b/curve25519-dalek/src/backend/vector/scalar_mul/precomputed_straus.rs index 515b4040c..1f16ab3e1 100644 --- a/curve25519-dalek/src/backend/vector/scalar_mul/precomputed_straus.rs +++ b/curve25519-dalek/src/backend/vector/scalar_mul/precomputed_straus.rs @@ -83,7 +83,7 @@ pub mod spec { let sp = self.static_lookup_tables.len(); let dp = dynamic_lookup_tables.len(); - assert_eq!(sp, static_nafs.len()); + assert!(sp >= static_nafs.len()); assert_eq!(dp, dynamic_nafs.len()); // We could save some doublings by looking for the highest @@ -107,7 +107,7 @@ pub mod spec { } #[allow(clippy::needless_range_loop)] - for i in 0..sp { + for i in 0..static_nafs.len() { let t_ij = static_nafs[i][j]; match t_ij.cmp(&0) { Ordering::Greater => { diff --git a/curve25519-dalek/src/ristretto.rs b/curve25519-dalek/src/ristretto.rs index 5f4e58264..1320bbe48 100644 --- a/curve25519-dalek/src/ristretto.rs +++ b/curve25519-dalek/src/ristretto.rs @@ -1869,4 +1869,148 @@ mod test { assert_eq!(P.compress(), R.compress()); assert_eq!(Q.compress(), R.compress()); } + + #[test] + #[cfg(feature = "alloc")] + fn partial_precomputed_mixed_multiscalar_empty() { + let mut rng = rand::thread_rng(); + + let n_static = 16; + let n_dynamic = 8; + + let static_points = (0..n_static) + .map(|_| RistrettoPoint::random(&mut rng)) + .collect::>(); + + // Use zero scalars + let static_scalars = Vec::new(); + + let dynamic_points = (0..n_dynamic) + .map(|_| RistrettoPoint::random(&mut rng)) + .collect::>(); + + let dynamic_scalars = (0..n_dynamic) + .map(|_| Scalar::random(&mut rng)) + .collect::>(); + + // Compute the linear combination using precomputed multiscalar multiplication + let precomputation = VartimeRistrettoPrecomputation::new(static_points.iter()); + let result_multiscalar = precomputation.vartime_mixed_multiscalar_mul( + &static_scalars, + &dynamic_scalars, + &dynamic_points, + ); + + // Compute the linear combination manually + let mut result_manual = RistrettoPoint::identity(); + for i in 0..static_scalars.len() { + result_manual += static_points[i] * static_scalars[i]; + } + for i in 0..n_dynamic { + result_manual += dynamic_points[i] * dynamic_scalars[i]; + } + + assert_eq!(result_multiscalar, result_manual); + } + + #[test] + #[cfg(feature = "alloc")] + fn partial_precomputed_mixed_multiscalar() { + let mut rng = rand::thread_rng(); + + let n_static = 16; + let n_dynamic = 8; + + let static_points = (0..n_static) + .map(|_| RistrettoPoint::random(&mut rng)) + .collect::>(); + + // Use one fewer scalars + let static_scalars = (0..n_static - 1) + .map(|_| Scalar::random(&mut rng)) + .collect::>(); + + let dynamic_points = (0..n_dynamic) + .map(|_| RistrettoPoint::random(&mut rng)) + .collect::>(); + + let dynamic_scalars = (0..n_dynamic) + .map(|_| Scalar::random(&mut rng)) + .collect::>(); + + // Compute the linear combination using precomputed multiscalar multiplication + let precomputation = VartimeRistrettoPrecomputation::new(static_points.iter()); + let result_multiscalar = precomputation.vartime_mixed_multiscalar_mul( + &static_scalars, + &dynamic_scalars, + &dynamic_points, + ); + + // Compute the linear combination manually + let mut result_manual = RistrettoPoint::identity(); + for i in 0..static_scalars.len() { + result_manual += static_points[i] * static_scalars[i]; + } + for i in 0..n_dynamic { + result_manual += dynamic_points[i] * dynamic_scalars[i]; + } + + assert_eq!(result_multiscalar, result_manual); + } + + #[test] + #[cfg(feature = "alloc")] + fn partial_precomputed_multiscalar() { + let mut rng = rand::thread_rng(); + + let n_static = 16; + + let static_points = (0..n_static) + .map(|_| RistrettoPoint::random(&mut rng)) + .collect::>(); + + // Use one fewer scalars + let static_scalars = (0..n_static - 1) + .map(|_| Scalar::random(&mut rng)) + .collect::>(); + + // Compute the linear combination using precomputed multiscalar multiplication + let precomputation = VartimeRistrettoPrecomputation::new(static_points.iter()); + let result_multiscalar = precomputation.vartime_multiscalar_mul(&static_scalars); + + // Compute the linear combination manually + let mut result_manual = RistrettoPoint::identity(); + for i in 0..static_scalars.len() { + result_manual += static_points[i] * static_scalars[i]; + } + + assert_eq!(result_multiscalar, result_manual); + } + + #[test] + #[cfg(feature = "alloc")] + fn partial_precomputed_multiscalar_empty() { + let mut rng = rand::thread_rng(); + + let n_static = 16; + + let static_points = (0..n_static) + .map(|_| RistrettoPoint::random(&mut rng)) + .collect::>(); + + // Use zero scalars + let static_scalars = Vec::new(); + + // Compute the linear combination using precomputed multiscalar multiplication + let precomputation = VartimeRistrettoPrecomputation::new(static_points.iter()); + let result_multiscalar = precomputation.vartime_multiscalar_mul(&static_scalars); + + // Compute the linear combination manually + let mut result_manual = RistrettoPoint::identity(); + for i in 0..static_scalars.len() { + result_manual += static_points[i] * static_scalars[i]; + } + + assert_eq!(result_multiscalar, result_manual); + } } diff --git a/curve25519-dalek/src/traits.rs b/curve25519-dalek/src/traits.rs index 322787db5..ea7ca3be7 100644 --- a/curve25519-dalek/src/traits.rs +++ b/curve25519-dalek/src/traits.rs @@ -285,7 +285,7 @@ pub trait VartimeMultiscalarMul { /// to be composed into the input iterators. /// /// All methods require that the lengths of the input iterators be -/// known and matching, as if they were `ExactSizeIterator`s. (It +/// known, as if they were `ExactSizeIterator`s. (It /// does not require `ExactSizeIterator` only because that trait is /// broken). pub trait VartimePrecomputedMultiscalarMul: Sized { @@ -306,8 +306,10 @@ pub trait VartimePrecomputedMultiscalarMul: Sized { /// $$ /// where the \\(B_j\\) are the points that were supplied to `new`. /// - /// It is an error to call this function with iterators of - /// inconsistent lengths. + /// It is valid for \\(b_i\\) to have a shorter length than \\(B_j\\). + /// In this case, any "unused" points are ignored in the computation. + /// It is an error to call this function if \\(b_i\\) has a longer + /// length than \\(B_j\\). /// /// The trait bound aims for maximum flexibility: the input must /// be convertable to iterators (`I: IntoIter`), and the @@ -337,8 +339,11 @@ pub trait VartimePrecomputedMultiscalarMul: Sized { /// $$ /// where the \\(B_j\\) are the points that were supplied to `new`. /// - /// It is an error to call this function with iterators of - /// inconsistent lengths. + /// It is valid for \\(b_i\\) to have a shorter length than \\(B_j\\). + /// In this case, any "unused" points are ignored in the computation. + /// It is an error to call this function if \\(b_i\\) has a longer + /// length than \\(B_j\\), or if \\(a_i\\) and \\(A_i\\) do not have + /// the same length. /// /// The trait bound aims for maximum flexibility: the inputs must be /// convertable to iterators (`I: IntoIter`), and the iterator's items @@ -378,8 +383,11 @@ pub trait VartimePrecomputedMultiscalarMul: Sized { /// /// If any of the dynamic points were `None`, return `None`. /// - /// It is an error to call this function with iterators of - /// inconsistent lengths. + /// It is valid for \\(b_i\\) to have a shorter length than \\(B_j\\). + /// In this case, any "unused" points are ignored in the computation. + /// It is an error to call this function if \\(b_i\\) has a longer + /// length than \\(B_j\\), or if \\(a_i\\) and \\(A_i\\) do not have + /// the same length. /// /// This function is particularly useful when verifying statements /// involving compressed points. Accepting `Option` allows