From 4af7c384fef06c8305d85abde5fb935a9e4f172a Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 2 Mar 2025 21:20:49 -0500 Subject: [PATCH 01/27] implement KeyInit for SigningKey --- ed25519-dalek/src/signing.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ed25519-dalek/src/signing.rs b/ed25519-dalek/src/signing.rs index f3c1053b2..3f505c4ab 100644 --- a/ed25519-dalek/src/signing.rs +++ b/ed25519-dalek/src/signing.rs @@ -20,7 +20,7 @@ use rand_core::CryptoRngCore; #[cfg(feature = "serde")] use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use sha2::Sha512; +use sha2::{digest::{consts::U32, crypto_common::KeySizeUser, KeyInit}, Sha512}; use subtle::{Choice, ConstantTimeEq}; use curve25519_dalek::{ @@ -536,6 +536,16 @@ impl SigningKey { } } +impl KeySizeUser for SigningKey { + type KeySize = U32; +} + +impl KeyInit for SigningKey { + fn new(key: &sha2::digest::Key) -> Self { + Self::from_bytes(key.as_ref()) + } +} + impl AsRef for SigningKey { fn as_ref(&self) -> &VerifyingKey { &self.verifying_key From 878ade7e2532df75961d6e2a4e69e07bbf5d4e32 Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Tue, 4 Mar 2025 19:11:05 -0500 Subject: [PATCH 02/27] move behind "digest" feature --- ed25519-dalek/src/signing.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ed25519-dalek/src/signing.rs b/ed25519-dalek/src/signing.rs index 3f505c4ab..c3c2c8b3c 100644 --- a/ed25519-dalek/src/signing.rs +++ b/ed25519-dalek/src/signing.rs @@ -20,11 +20,15 @@ use rand_core::CryptoRngCore; #[cfg(feature = "serde")] use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use sha2::{digest::{consts::U32, crypto_common::KeySizeUser, KeyInit}, Sha512}; +use sha2::Sha512; use subtle::{Choice, ConstantTimeEq}; use curve25519_dalek::{ - digest::{generic_array::typenum::U64, Digest}, + digest::{ + crypto_common::KeySizeUser, + generic_array::typenum::{U32, U64}, + Digest, KeyInit, + }, edwards::{CompressedEdwardsY, EdwardsPoint}, scalar::Scalar, }; @@ -536,10 +540,12 @@ impl SigningKey { } } +#[cfg(feature = "digest")] impl KeySizeUser for SigningKey { type KeySize = U32; } +#[cfg(feature = "digest")] impl KeyInit for SigningKey { fn new(key: &sha2::digest::Key) -> Self { Self::from_bytes(key.as_ref()) From 1a6ea176d4e984e23b5989c5cd6d0c254415f1ed Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Tue, 4 Mar 2025 19:14:23 -0500 Subject: [PATCH 03/27] implement KeySizeUser for VerifyingKey --- ed25519-dalek/src/verifying.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/ed25519-dalek/src/verifying.rs b/ed25519-dalek/src/verifying.rs index 2bb40ebd7..1ac5d5c77 100644 --- a/ed25519-dalek/src/verifying.rs +++ b/ed25519-dalek/src/verifying.rs @@ -13,7 +13,11 @@ use core::fmt::Debug; use core::hash::{Hash, Hasher}; use curve25519_dalek::{ - digest::{generic_array::typenum::U64, Digest}, + digest::{ + crypto_common::KeySizeUser, + generic_array::typenum::{U32, U64}, + Digest, + }, edwards::{CompressedEdwardsY, EdwardsPoint}, montgomery::MontgomeryPoint, scalar::Scalar, @@ -109,6 +113,11 @@ impl From for VerifyingKey { } } +#[cfg(feature = "digest")] +impl KeySizeUser for VerifyingKey { + type KeySize = U32; +} + impl VerifyingKey { /// Convert this public key to a byte array. #[inline] From 0fcca09c8e6b2675c0ac84b130483b5861f5a4d3 Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sat, 8 Mar 2025 21:03:53 -0500 Subject: [PATCH 04/27] move imports behind "digest" feature --- ed25519-dalek/src/signing.rs | 9 ++++----- ed25519-dalek/src/verifying.rs | 9 ++++----- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/ed25519-dalek/src/signing.rs b/ed25519-dalek/src/signing.rs index c3c2c8b3c..291a706c7 100644 --- a/ed25519-dalek/src/signing.rs +++ b/ed25519-dalek/src/signing.rs @@ -20,15 +20,14 @@ use rand_core::CryptoRngCore; #[cfg(feature = "serde")] use serde::{Deserialize, Deserializer, Serialize, Serializer}; +#[cfg(feature = "digest")] +use curve25519_dalek::digest::{crypto_common::KeySizeUser, typenum::U32, KeyInit}; + use sha2::Sha512; use subtle::{Choice, ConstantTimeEq}; use curve25519_dalek::{ - digest::{ - crypto_common::KeySizeUser, - generic_array::typenum::{U32, U64}, - Digest, KeyInit, - }, + digest::{generic_array::typenum::U64, Digest}, edwards::{CompressedEdwardsY, EdwardsPoint}, scalar::Scalar, }; diff --git a/ed25519-dalek/src/verifying.rs b/ed25519-dalek/src/verifying.rs index 1ac5d5c77..991869ef4 100644 --- a/ed25519-dalek/src/verifying.rs +++ b/ed25519-dalek/src/verifying.rs @@ -9,15 +9,14 @@ //! ed25519 public keys. +#[cfg(feature = "digest")] +use curve25519_dalek::digest::{crypto_common::KeySizeUser, typenum::U32}; + use core::fmt::Debug; use core::hash::{Hash, Hasher}; use curve25519_dalek::{ - digest::{ - crypto_common::KeySizeUser, - generic_array::typenum::{U32, U64}, - Digest, - }, + digest::{generic_array::typenum::U64, Digest}, edwards::{CompressedEdwardsY, EdwardsPoint}, montgomery::MontgomeryPoint, scalar::Scalar, From 8898298b6fd2c5fc6ed980c0bde7dfb759bb1d0a Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sat, 8 Mar 2025 21:08:28 -0500 Subject: [PATCH 05/27] update "digest" feature description --- ed25519-dalek/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ed25519-dalek/README.md b/ed25519-dalek/README.md index 364d08538..5381629e3 100644 --- a/ed25519-dalek/README.md +++ b/ed25519-dalek/README.md @@ -22,7 +22,7 @@ This crate is `#[no_std]` compatible with `default-features = false`. | `zeroize` | ✓ | Implements `Zeroize` and `ZeroizeOnDrop` for `SigningKey` | | `rand_core` | | Enables `SigningKey::generate` | | `batch` | | Enables `verify_batch` for verifying many signatures quickly. Also enables `rand_core`. | -| `digest` | | Enables `Context`, `SigningKey::{with_context, sign_prehashed}` and `VerifyingKey::{with_context, verify_prehashed, verify_prehashed_strict}` for Ed25519ph prehashed signatures | +| `digest` | | Enables `Context`, `SigningKey::{with_context, sign_prehashed}` and `VerifyingKey::{with_context, verify_prehashed, verify_prehashed_strict}` for Ed25519ph prehashed signatures. Also implements `KeySizeUser` for `SigningKey` and `VerifyingKey`, and implements `KeyInit` for `SigningKey`. | | `asm` | | Enables assembly optimizations in the SHA-512 compression functions | | `pkcs8` | | Enables [PKCS#8](https://en.wikipedia.org/wiki/PKCS_8) serialization/deserialization for `SigningKey` and `VerifyingKey` | | `pem` | | Enables PEM serialization support for PKCS#8 private keys and SPKI public keys. Also enables `alloc`. | From 3d36f7224199fd4fc973898d74e33ce01e00562b Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 23 Mar 2025 21:12:37 -0400 Subject: [PATCH 06/27] fix import statements --- ed25519-dalek/src/signing.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ed25519-dalek/src/signing.rs b/ed25519-dalek/src/signing.rs index 291a706c7..29b53679d 100644 --- a/ed25519-dalek/src/signing.rs +++ b/ed25519-dalek/src/signing.rs @@ -21,7 +21,7 @@ use rand_core::CryptoRngCore; use serde::{Deserialize, Deserializer, Serialize, Serializer}; #[cfg(feature = "digest")] -use curve25519_dalek::digest::{crypto_common::KeySizeUser, typenum::U32, KeyInit}; +use curve25519_dalek::digest::{crypto_common::{KeySizeUser, KeyInit, Key}, typenum::U32}; use sha2::Sha512; use subtle::{Choice, ConstantTimeEq}; @@ -546,7 +546,7 @@ impl KeySizeUser for SigningKey { #[cfg(feature = "digest")] impl KeyInit for SigningKey { - fn new(key: &sha2::digest::Key) -> Self { + fn new(key: &Key) -> Self { Self::from_bytes(key.as_ref()) } } From 0f8392fbfb4103084eddcb1acd03f310da9f7a6b Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sat, 26 Apr 2025 22:07:58 -0400 Subject: [PATCH 07/27] first batch of trait implementations --- curve25519-dalek/Cargo.toml | 4 +- curve25519-dalek/src/ed25519.rs | 24 ++++++++++ curve25519-dalek/src/edwards.rs | 77 ++++++++++++++++++++++++--------- curve25519-dalek/src/lib.rs | 3 ++ 4 files changed, 86 insertions(+), 22 deletions(-) create mode 100644 curve25519-dalek/src/ed25519.rs diff --git a/curve25519-dalek/Cargo.toml b/curve25519-dalek/Cargo.toml index fc35264bb..a5833794a 100644 --- a/curve25519-dalek/Cargo.toml +++ b/curve25519-dalek/Cargo.toml @@ -54,6 +54,7 @@ digest = { version = "0.10", default-features = false, optional = true } 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 } +elliptic-curve = { version = "0.13", optional = true } [target.'cfg(target_arch = "x86_64")'.dependencies] cpufeatures = "0.2.6" @@ -66,8 +67,9 @@ default = ["alloc", "precomputed-tables", "zeroize"] alloc = ["zeroize?/alloc"] precomputed-tables = [] legacy_compatibility = [] -group = ["dep:group", "rand_core"] +group = ["dep:group", "rand_core", "digest"] group-bits = ["group", "ff/bits"] +elliptic-curve = ["group", "dep:ff", "dep:elliptic-curve"] [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" } diff --git a/curve25519-dalek/src/ed25519.rs b/curve25519-dalek/src/ed25519.rs new file mode 100644 index 000000000..6a988d159 --- /dev/null +++ b/curve25519-dalek/src/ed25519.rs @@ -0,0 +1,24 @@ +use elliptic_curve::{bigint::U256, consts::U32, Curve, CurveArithmetic, FieldBytesEncoding}; + +use crate::{constants::BASEPOINT_ORDER_PRIVATE, edwards::CompressedEdwardsY, EdwardsPoint, Scalar}; + +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, PartialOrd, Ord, Hash)] +pub struct Ed25519; + +impl Curve for Ed25519 { + type FieldBytesSize = U32; + + type Uint = U256; + + const ORDER: Self::Uint = U256::from_le_slice(&BASEPOINT_ORDER_PRIVATE.bytes); +} + +impl CurveArithmetic for Ed25519 { + type AffinePoint = CompressedEdwardsY; + + type ProjectivePoint = EdwardsPoint; + + type Scalar = Scalar; +} + +impl FieldBytesEncoding for U256 {} diff --git a/curve25519-dalek/src/edwards.rs b/curve25519-dalek/src/edwards.rs index d6672bd3e..c964a6b8f 100644 --- a/curve25519-dalek/src/edwards.rs +++ b/curve25519-dalek/src/edwards.rs @@ -104,7 +104,8 @@ use core::ops::{Mul, MulAssign}; use cfg_if::cfg_if; #[cfg(feature = "digest")] -use digest::{generic_array::typenum::U64, Digest}; +use digest::{generic_array::typenum::{U64, U32}, Digest}; +use zeroize::DefaultIsZeroes; #[cfg(feature = "group")] use { @@ -248,6 +249,47 @@ impl TryFrom<&[u8]> for CompressedEdwardsY { } } +// ------------------------------------------------------------------------ +// Constant-time assignment +// ------------------------------------------------------------------------ + +impl ConditionallySelectable for CompressedEdwardsY { + fn conditional_select(a: &CompressedEdwardsY, b: &CompressedEdwardsY, choice: Choice) -> CompressedEdwardsY { + CompressedEdwardsY( + <[u8; 32]>::conditional_select(&a.0, &b.0, choice) + ) + } +} + +// ------------------------------------------------------------------------ +// Affine Coordinate +// ----------------------------------------------------------------------- + +#[cfg(feature = "elliptic-curve")] +impl elliptic_curve::point::AffineCoordinates for CompressedEdwardsY { + type FieldRepr = digest::generic_array::GenericArray; + + fn x(&self) -> Self::FieldRepr { + // QUESTION: here we assume that the CompressedEdwardsY valid, and it won't panic in dbg mode. + // We should either change the CompressedEdwardsY API to now allow instancing a + // `CompressedEdwardsY` that is invalid, or clearly document that case somewhere. + // How should we handle this? + let (is_valid, mut X, _, _) = decompress::step_1(self); + debug_assert!(bool::from(is_valid)); + + // FieldElement::sqrt_ratio_i always returns the nonnegative square root, + // so we negate according to the supplied sign bit. + let compressed_sign_bit = Choice::from(self.as_bytes()[31] >> 7); + X.conditional_negate(compressed_sign_bit); + X.as_bytes().into() + } + + fn y_is_odd(&self) -> Choice { + // TODO: here I assume that the Y is encoded in little-endian, which I should check. + Choice::from(self.as_bytes()[0] & 0x01) + } +} + // ------------------------------------------------------------------------ // Serde support // ------------------------------------------------------------------------ @@ -428,24 +470,10 @@ impl Default for EdwardsPoint { // ------------------------------------------------------------------------ #[cfg(feature = "zeroize")] -impl Zeroize for CompressedEdwardsY { - /// Reset this `CompressedEdwardsY` to the compressed form of the identity element. - fn zeroize(&mut self) { - self.0.zeroize(); - self.0[0] = 1; - } -} +impl DefaultIsZeroes for CompressedEdwardsY {} #[cfg(feature = "zeroize")] -impl Zeroize for EdwardsPoint { - /// Reset this `EdwardsPoint` to the identity element. - fn zeroize(&mut self) { - self.X.zeroize(); - self.Y = FieldElement::ONE; - self.Z = FieldElement::ONE; - self.T.zeroize(); - } -} +impl DefaultIsZeroes for EdwardsPoint {} // ------------------------------------------------------------------------ // Validity checks (for debugging, not CT) @@ -1322,10 +1350,10 @@ impl group::Group for EdwardsPoint { #[cfg(feature = "group")] impl GroupEncoding for EdwardsPoint { - type Repr = [u8; 32]; + type Repr = digest::generic_array::GenericArray; fn from_bytes(bytes: &Self::Repr) -> CtOption { - let repr = CompressedEdwardsY(*bytes); + let repr = CompressedEdwardsY(<[u8; 32]>::from(*bytes)); let (is_valid_y_coord, X, Y, Z) = decompress::step_1(&repr); CtOption::new(decompress::step_2(&repr, X, Y, Z), is_valid_y_coord) } @@ -1336,7 +1364,14 @@ impl GroupEncoding for EdwardsPoint { } fn to_bytes(&self) -> Self::Repr { - self.compress().to_bytes() + self.compress().to_bytes().into() + } +} + +#[cfg(feature = "elliptic-curve")] +impl elliptic_curve::ops::MulByGenerator for EdwardsPoint { + fn mul_by_generator(scalar: &Self::Scalar) -> Self { + Self::mul_base(scalar) } } @@ -1589,7 +1624,7 @@ impl GroupEncoding for SubgroupPoint { } fn to_bytes(&self) -> Self::Repr { - self.0.compress().to_bytes() + self.0.compress().to_bytes().into() } } diff --git a/curve25519-dalek/src/lib.rs b/curve25519-dalek/src/lib.rs index d8666453c..8db950450 100644 --- a/curve25519-dalek/src/lib.rs +++ b/curve25519-dalek/src/lib.rs @@ -109,3 +109,6 @@ pub use crate::{ // Build time diagnostics for validation #[cfg(curve25519_dalek_diagnostics = "build")] mod diagnostics; + +#[cfg(feature = "elliptic-curve")] +mod ed25519; From a4c10d81e45b1e50ce9d0ac67ae71615bdf15c1f Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 27 Apr 2025 11:18:23 -0400 Subject: [PATCH 08/27] fix: FieldElement trait for CurveArithmetic --- curve25519-dalek/src/scalar.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/curve25519-dalek/src/scalar.rs b/curve25519-dalek/src/scalar.rs index 6afd74eef..9ab9435b7 100644 --- a/curve25519-dalek/src/scalar.rs +++ b/curve25519-dalek/src/scalar.rs @@ -1251,12 +1251,15 @@ impl Field for Scalar { } } -#[cfg(feature = "group")] +#[cfg(all(feature = "group", feature = "digest"))] impl PrimeField for Scalar { - type Repr = [u8; 32]; + // DISCUSSION: it sucks that we have to use this type but that's what `ArithmeticCurve` + // requires. This is only a minor version bump since this trait was already behind the "group" + // feature. + type Repr = digest::generic_array::GenericArray; fn from_repr(repr: Self::Repr) -> CtOption { - Self::from_canonical_bytes(repr) + Self::from_canonical_bytes(repr.into()) } fn from_repr_vartime(repr: Self::Repr) -> Option { @@ -1265,7 +1268,7 @@ impl PrimeField for Scalar { return None; } - let candidate = Scalar { bytes: repr }; + let candidate = Scalar { bytes: repr.into() }; if candidate == candidate.reduce() { Some(candidate) @@ -1275,7 +1278,7 @@ impl PrimeField for Scalar { } fn to_repr(&self) -> Self::Repr { - self.to_bytes() + self.to_bytes().into() } fn is_odd(&self) -> Choice { @@ -1328,7 +1331,7 @@ impl PrimeFieldBits for Scalar { type ReprBits = [u8; 32]; fn to_le_bits(&self) -> FieldBits { - self.to_repr().into() + <[u8; 32]>::from(self.to_repr()).into() } fn char_le_bits() -> FieldBits { @@ -1980,7 +1983,7 @@ pub(crate) mod test { ); } - #[cfg(feature = "group")] + #[cfg(all(feature = "group", feature = "digest"))] #[test] fn ff_impls() { assert!(bool::from(Scalar::ZERO.is_even())); @@ -1996,10 +1999,10 @@ pub(crate) mod test { assert!([X, -X].contains(&x_sq.sqrt().unwrap())); assert_eq!(Scalar::from_repr_vartime(X.to_repr()), Some(X)); - assert_eq!(Scalar::from_repr_vartime([0xff; 32]), None); + assert_eq!(Scalar::from_repr_vartime([0xff; 32].into()), None); assert_eq!(Scalar::from_repr(X.to_repr()).unwrap(), X); - assert!(bool::from(Scalar::from_repr([0xff; 32]).is_none())); + assert!(bool::from(Scalar::from_repr([0xff; 32].into()).is_none())); } #[test] From c0a313b48b98188a99299df5ac89c8e3316e5a14 Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 27 Apr 2025 11:23:55 -0400 Subject: [PATCH 09/27] remove unecessary digest requirements --- curve25519-dalek/src/scalar.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/curve25519-dalek/src/scalar.rs b/curve25519-dalek/src/scalar.rs index 9ab9435b7..082c06eaf 100644 --- a/curve25519-dalek/src/scalar.rs +++ b/curve25519-dalek/src/scalar.rs @@ -1251,7 +1251,7 @@ impl Field for Scalar { } } -#[cfg(all(feature = "group", feature = "digest"))] +#[cfg(feature = "group")] impl PrimeField for Scalar { // DISCUSSION: it sucks that we have to use this type but that's what `ArithmeticCurve` // requires. This is only a minor version bump since this trait was already behind the "group" @@ -1983,7 +1983,7 @@ pub(crate) mod test { ); } - #[cfg(all(feature = "group", feature = "digest"))] + #[cfg(feature = "group")] #[test] fn ff_impls() { assert!(bool::from(Scalar::ZERO.is_even())); From 8cf3f00c6ce342bf65eece3f1a621f1a4c264fb2 Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 27 Apr 2025 11:49:23 -0400 Subject: [PATCH 10/27] impl ShrAssign for Scalar --- curve25519-dalek/src/scalar.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/curve25519-dalek/src/scalar.rs b/curve25519-dalek/src/scalar.rs index 082c06eaf..b205e0dd8 100644 --- a/curve25519-dalek/src/scalar.rs +++ b/curve25519-dalek/src/scalar.rs @@ -1208,6 +1208,29 @@ impl UnpackedScalar { } } +// ---------------------------------------------------------------------- +// Elliptic Curve Arithmetic traits +// ---------------------------------------------------------------------- + +// QUESTION: This trait is needed for `CurveArithmetic`, hence why it is bounded behind the +// "elliptic-curve" feature. If this is a fine trait to expose to the user, we can consider +// removing the feature flag and implement all of its variants (with u8, &u8, u16, etc.) and the +// non-assign `Shr` trait. We probably don't want to impl `Shl` as this can overflow the scalar +// and break the invariants. If we don't want to expose this trait, we can also make it +// `#[doc(hidden)]`. +// +// QUESTION: I did not use constant-time because other elliptic-curves dont +// (e.g., https://docs.rs/p256/0.13.2/src/p256/arithmetic/scalar.rs.html#426-432). +// Is constant-time necessary? +#[cfg(feature = "elliptic-curve")] +impl core::ops::ShrAssign for Scalar { + fn shr_assign(&mut self, rhs: usize) { + use elliptic_curve::bigint::Encoding; + let repr = elliptic_curve::bigint::U256::from_le_bytes(self.bytes); + self.bytes = (repr.shr_vartime(rhs)).to_le_bytes(); + } +} + #[cfg(feature = "group")] impl Field for Scalar { const ZERO: Self = Self::ZERO; From f5365ed192f4468ad7a491da3f718962676eca4a Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 27 Apr 2025 12:26:38 -0400 Subject: [PATCH 11/27] impl Reduce for Scalar --- curve25519-dalek/src/edwards.rs | 1 - curve25519-dalek/src/scalar.rs | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/curve25519-dalek/src/edwards.rs b/curve25519-dalek/src/edwards.rs index c964a6b8f..7642a3584 100644 --- a/curve25519-dalek/src/edwards.rs +++ b/curve25519-dalek/src/edwards.rs @@ -285,7 +285,6 @@ impl elliptic_curve::point::AffineCoordinates for CompressedEdwardsY { } fn y_is_odd(&self) -> Choice { - // TODO: here I assume that the Y is encoded in little-endian, which I should check. Choice::from(self.as_bytes()[0] & 0x01) } } diff --git a/curve25519-dalek/src/scalar.rs b/curve25519-dalek/src/scalar.rs index b205e0dd8..e0708edbe 100644 --- a/curve25519-dalek/src/scalar.rs +++ b/curve25519-dalek/src/scalar.rs @@ -1231,6 +1231,23 @@ impl core::ops::ShrAssign for Scalar { } } +#[cfg(feature = "elliptic-curve")] +impl elliptic_curve::ops::Reduce for Scalar { + type Bytes = elliptic_curve::generic_array::GenericArray< + u8, + elliptic_curve::generic_array::typenum::U32, + >; + + fn reduce(n: elliptic_curve::bigint::U256) -> Self { + use elliptic_curve::bigint::Encoding; + Scalar::from_bytes_mod_order(n.to_le_bytes()) + } + + fn reduce_bytes(bytes: &Self::Bytes) -> Self { + Scalar::from_bytes_mod_order(<[u8; 32]>::from(*bytes)) + } +} + #[cfg(feature = "group")] impl Field for Scalar { const ZERO: Self = Self::ZERO; From 9579a5bb6d1e95adb58a6a24a50c1a2fdbc870eb Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 27 Apr 2025 12:28:25 -0400 Subject: [PATCH 12/27] impl Ord for Scalar impl Ord for Scalar --- curve25519-dalek/src/scalar.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/curve25519-dalek/src/scalar.rs b/curve25519-dalek/src/scalar.rs index e0708edbe..6ddb74b9e 100644 --- a/curve25519-dalek/src/scalar.rs +++ b/curve25519-dalek/src/scalar.rs @@ -191,7 +191,7 @@ cfg_if! { /// The `Scalar` struct holds an element of \\(\mathbb Z / \ell\mathbb Z \\). #[allow(clippy::derived_hash_with_manual_eq)] -#[derive(Copy, Clone, Hash)] +#[derive(Copy, Clone, Hash, PartialOrd, Ord)] pub struct Scalar { /// `bytes` is a little-endian byte encoding of an integer representing a scalar modulo the /// group order. From 15a8763c4852bcff12b8d5e90538b9930d695e2d Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 27 Apr 2025 12:55:03 -0400 Subject: [PATCH 13/27] impl IsHigh for Scalar --- curve25519-dalek/src/scalar.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/curve25519-dalek/src/scalar.rs b/curve25519-dalek/src/scalar.rs index 6ddb74b9e..21b12647a 100644 --- a/curve25519-dalek/src/scalar.rs +++ b/curve25519-dalek/src/scalar.rs @@ -140,6 +140,7 @@ use digest::Digest; use subtle::Choice; use subtle::ConditionallySelectable; +use subtle::ConstantTimeGreater; use subtle::ConstantTimeEq; use subtle::CtOption; @@ -1248,6 +1249,19 @@ impl elliptic_curve::ops::Reduce for Scalar { } } +// QUESTION: Even though the traits asks for where &self is _greater that or equal_ to `n / 2`, +// every implementation I have looked at (e.g., +// https://docs.rs/p256/latest/src/p256/arithmetic/scalar.rs.html#413-415) uses `ct_gt`. +// I decided to do the same, it this correct? +impl elliptic_curve::scalar::IsHigh for Scalar { + fn is_high(&self) -> Choice { + use elliptic_curve::bigint::{Encoding, U256}; + U256::from_le_bytes(self.bytes).ct_gt( + &U256::from_le_bytes((constants::BASEPOINT_ORDER_PRIVATE * Self::TWO_INV ).bytes) + ) + } +} + #[cfg(feature = "group")] impl Field for Scalar { const ZERO: Self = Self::ZERO; From 3b72a93d5b0fc05244dd411f280cf886f0007d63 Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 27 Apr 2025 13:10:36 -0400 Subject: [PATCH 14/27] implement trivial AsRef for Scalar --- curve25519-dalek/src/scalar.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/curve25519-dalek/src/scalar.rs b/curve25519-dalek/src/scalar.rs index 21b12647a..e45d8f83e 100644 --- a/curve25519-dalek/src/scalar.rs +++ b/curve25519-dalek/src/scalar.rs @@ -560,6 +560,11 @@ impl Zeroize for Scalar { } } +impl AsRef for Scalar { + fn as_ref(&self) -> &Scalar { + self + } +} impl Scalar { /// The scalar \\( 0 \\). pub const ZERO: Self = Self { bytes: [0u8; 32] }; From 9dbf0779d969e625f548a92585849f7882282d39 Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 27 Apr 2025 15:51:25 -0400 Subject: [PATCH 15/27] implementation typechecks --- curve25519-dalek/src/edwards.rs | 111 ++++++++++++++++++++++++++++++-- curve25519-dalek/src/scalar.rs | 95 ++++++++++++++++++++++----- 2 files changed, 183 insertions(+), 23 deletions(-) diff --git a/curve25519-dalek/src/edwards.rs b/curve25519-dalek/src/edwards.rs index 7642a3584..420d96a99 100644 --- a/curve25519-dalek/src/edwards.rs +++ b/curve25519-dalek/src/edwards.rs @@ -813,6 +813,24 @@ impl EdwardsPoint { } } +// ------------------------------------------------------------------------ +// Elliptic curve traits +// ------------------------------------------------------------------------ + +#[cfg(feature = "elliptic-curve")] +impl elliptic_curve::ops::LinearCombination for EdwardsPoint { + fn lincomb(x: &Self, k: &Self::Scalar, y: &Self, l: &Self::Scalar) -> Self { + EdwardsPoint::multiscalar_mul([k, l], [x, y]) + } +} +#[cfg(feature = "elliptic-curve")] +impl elliptic_curve::ops::MulByGenerator for EdwardsPoint { + fn mul_by_generator(scalar: &Self::Scalar) -> Self { + Self::mul_base(scalar) + } +} + + // ------------------------------------------------------------------------ // Multiscalar Multiplication impls // ------------------------------------------------------------------------ @@ -1312,6 +1330,15 @@ impl Debug for EdwardsPoint { // group traits // ------------------------------------------------------------------------ +#[cfg(feature = "group")] +impl group::Curve for EdwardsPoint { + type AffineRepr = CompressedEdwardsY; + + fn to_affine(&self) -> Self::AffineRepr { + self.compress() + } +} + // Use the full trait path to avoid Group::identity overlapping Identity::identity in the // rest of the module (e.g. tests). #[cfg(feature = "group")] @@ -1367,13 +1394,6 @@ impl GroupEncoding for EdwardsPoint { } } -#[cfg(feature = "elliptic-curve")] -impl elliptic_curve::ops::MulByGenerator for EdwardsPoint { - fn mul_by_generator(scalar: &Self::Scalar) -> Self { - Self::mul_base(scalar) - } -} - /// A `SubgroupPoint` represents a point on the Edwards form of Curve25519, that is /// guaranteed to be in the prime-order subgroup. #[cfg(feature = "group")] @@ -1648,6 +1668,83 @@ impl CofactorGroup for EdwardsPoint { } } +// ------------------------------------------------------------------------ +// Interop between CompressedEdwardsY and EdwardsPoint for group traits +// ------------------------------------------------------------------------ + +// Again, here we assume throughout that CompressedEdwardsY is a valid point. + +impl From for EdwardsPoint { + fn from(value: CompressedEdwardsY) -> Self { + let (_, X, Y, Z) = decompress::step_1(&value); + decompress::step_2(&value, X, Y, Z) + } +} + +impl From<&CompressedEdwardsY> for EdwardsPoint { + fn from(value: &CompressedEdwardsY) -> Self { + let (_, X, Y, Z) = decompress::step_1(value); + decompress::step_2(value, X, Y, Z) + } +} + + +impl From for CompressedEdwardsY { + fn from(value: EdwardsPoint) -> Self { + value.compress() + } +} + +impl From<&EdwardsPoint> for CompressedEdwardsY { + fn from(value: &EdwardsPoint) -> Self { + value.compress() + } +} + +impl Add<&CompressedEdwardsY> for &EdwardsPoint { + type Output = EdwardsPoint; + + fn add(self, other: &CompressedEdwardsY) -> EdwardsPoint { + self + EdwardsPoint::from(other) + } +} + +define_add_variants!( + LHS = EdwardsPoint, + RHS = CompressedEdwardsY, + Output = EdwardsPoint +); + +impl AddAssign<&CompressedEdwardsY> for EdwardsPoint { + fn add_assign(&mut self, rhs: &CompressedEdwardsY) { + *self += EdwardsPoint::from(rhs); + } +} + +define_add_assign_variants!(LHS = EdwardsPoint, RHS = CompressedEdwardsY); + +impl Sub<&CompressedEdwardsY> for &EdwardsPoint { + type Output = EdwardsPoint; + + fn sub(self, other: &CompressedEdwardsY) -> EdwardsPoint { + self - EdwardsPoint::from(other) + } +} + +define_sub_variants!( + LHS = EdwardsPoint, + RHS = CompressedEdwardsY, + Output = EdwardsPoint +); + +impl SubAssign<&CompressedEdwardsY> for EdwardsPoint { + fn sub_assign(&mut self, rhs: &CompressedEdwardsY) { + *self -= EdwardsPoint::from(rhs); + } +} + +define_sub_assign_variants!(LHS = EdwardsPoint, RHS = CompressedEdwardsY); + // ------------------------------------------------------------------------ // Tests // ------------------------------------------------------------------------ diff --git a/curve25519-dalek/src/scalar.rs b/curve25519-dalek/src/scalar.rs index e45d8f83e..014d5559d 100644 --- a/curve25519-dalek/src/scalar.rs +++ b/curve25519-dalek/src/scalar.rs @@ -134,18 +134,18 @@ use rand_core::RngCore; use rand_core::CryptoRngCore; #[cfg(feature = "digest")] -use digest::generic_array::typenum::U64; +use digest::generic_array::{GenericArray, typenum::{U64, U32}}; #[cfg(feature = "digest")] use digest::Digest; use subtle::Choice; use subtle::ConditionallySelectable; -use subtle::ConstantTimeGreater; use subtle::ConstantTimeEq; +use subtle::ConstantTimeGreater; use subtle::CtOption; #[cfg(feature = "zeroize")] -use zeroize::Zeroize; +use zeroize::DefaultIsZeroes; use crate::backend; use crate::constants; @@ -192,7 +192,7 @@ cfg_if! { /// The `Scalar` struct holds an element of \\(\mathbb Z / \ell\mathbb Z \\). #[allow(clippy::derived_hash_with_manual_eq)] -#[derive(Copy, Clone, Hash, PartialOrd, Ord)] +#[derive(Copy, Clone, Hash)] pub struct Scalar { /// `bytes` is a little-endian byte encoding of an integer representing a scalar modulo the /// group order. @@ -554,17 +554,14 @@ impl From for Scalar { } #[cfg(feature = "zeroize")] -impl Zeroize for Scalar { - fn zeroize(&mut self) { - self.bytes.zeroize(); - } -} +impl DefaultIsZeroes for Scalar {} impl AsRef for Scalar { fn as_ref(&self) -> &Scalar { self } } + impl Scalar { /// The scalar \\( 0 \\). pub const ZERO: Self = Self { bytes: [0u8; 32] }; @@ -837,7 +834,7 @@ impl Scalar { } #[cfg(feature = "zeroize")] - Zeroize::zeroize(&mut scratch); + zeroize::Zeroize::zeroize(&mut scratch); ret } @@ -1239,9 +1236,9 @@ impl core::ops::ShrAssign for Scalar { #[cfg(feature = "elliptic-curve")] impl elliptic_curve::ops::Reduce for Scalar { - type Bytes = elliptic_curve::generic_array::GenericArray< + type Bytes = GenericArray< u8, - elliptic_curve::generic_array::typenum::U32, + U32, >; fn reduce(n: elliptic_curve::bigint::U256) -> Self { @@ -1258,12 +1255,78 @@ impl elliptic_curve::ops::Reduce for Scalar { // every implementation I have looked at (e.g., // https://docs.rs/p256/latest/src/p256/arithmetic/scalar.rs.html#413-415) uses `ct_gt`. // I decided to do the same, it this correct? +#[cfg(feature = "elliptic-curve")] impl elliptic_curve::scalar::IsHigh for Scalar { fn is_high(&self) -> Choice { use elliptic_curve::bigint::{Encoding, U256}; - U256::from_le_bytes(self.bytes).ct_gt( - &U256::from_le_bytes((constants::BASEPOINT_ORDER_PRIVATE * Self::TWO_INV ).bytes) - ) + U256::from_le_bytes(self.bytes).ct_gt(&U256::from_le_bytes( + (constants::BASEPOINT_ORDER_PRIVATE * Self::TWO_INV).bytes, + )) + } +} + +impl elliptic_curve::ops::Invert for Scalar { + type Output = CtOption; + + fn invert(&self) -> Self::Output { + let invert = self.invert(); + CtOption::new(invert, !self.is_zero()) + } +} + +impl PartialOrd for Scalar { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for Scalar { + fn cmp(&self, other: &Self) -> core::cmp::Ordering { + // TODO: + todo!() + } +} + +#[cfg(feature = "elliptic-curve")] +impl From for elliptic_curve::bigint::U256 { + fn from(value: Scalar) -> Self { + use elliptic_curve::bigint::Encoding; + elliptic_curve::bigint::U256::from_le_bytes(value.bytes) + } +} + +#[cfg(feature = "elliptic-curve")] +impl From> for Scalar { + fn from(value: elliptic_curve::ScalarPrimitive) -> Self { + // TODO + todo!() + } +} + +#[cfg(feature = "elliptic-curve")] +impl From for elliptic_curve::ScalarPrimitive { + fn from(value: Scalar) -> Self { + // TODO + todo!() + } +} + +#[cfg(feature = "digest")] +impl From for GenericArray { + fn from(value: Scalar) -> Self { + value.bytes.into() + } +} + +#[cfg(feature = "elliptic-curve")] +impl elliptic_curve::scalar::FromUintUnchecked for Scalar { + type Uint = elliptic_curve::bigint::U256; + + fn from_uint_unchecked(uint: Self::Uint) -> Self { + use elliptic_curve::bigint::Encoding; + Self { + bytes: uint.to_le_bytes(), + } } } @@ -1315,7 +1378,7 @@ impl PrimeField for Scalar { // DISCUSSION: it sucks that we have to use this type but that's what `ArithmeticCurve` // requires. This is only a minor version bump since this trait was already behind the "group" // feature. - type Repr = digest::generic_array::GenericArray; + type Repr = GenericArray; fn from_repr(repr: Self::Repr) -> CtOption { Self::from_canonical_bytes(repr.into()) From 0841636c24f916a73f3730b3f48a395e7ed7f08d Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 27 Apr 2025 16:20:26 -0400 Subject: [PATCH 16/27] complete implementations --- curve25519-dalek/src/scalar.rs | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/curve25519-dalek/src/scalar.rs b/curve25519-dalek/src/scalar.rs index 014d5559d..a18fbdd48 100644 --- a/curve25519-dalek/src/scalar.rs +++ b/curve25519-dalek/src/scalar.rs @@ -122,6 +122,7 @@ use core::ops::{Sub, SubAssign}; use cfg_if::cfg_if; +use elliptic_curve::bigint::Encoding; #[cfg(feature = "group")] use group::ff::{Field, FromUniformBytes, PrimeField}; #[cfg(feature = "group-bits")] @@ -134,7 +135,10 @@ use rand_core::RngCore; use rand_core::CryptoRngCore; #[cfg(feature = "digest")] -use digest::generic_array::{GenericArray, typenum::{U64, U32}}; +use digest::generic_array::{ + typenum::{U32, U64}, + GenericArray, +}; #[cfg(feature = "digest")] use digest::Digest; @@ -1236,10 +1240,7 @@ impl core::ops::ShrAssign for Scalar { #[cfg(feature = "elliptic-curve")] impl elliptic_curve::ops::Reduce for Scalar { - type Bytes = GenericArray< - u8, - U32, - >; + type Bytes = GenericArray; fn reduce(n: elliptic_curve::bigint::U256) -> Self { use elliptic_curve::bigint::Encoding; @@ -1282,8 +1283,9 @@ impl PartialOrd for Scalar { impl Ord for Scalar { fn cmp(&self, other: &Self) -> core::cmp::Ordering { - // TODO: - todo!() + elliptic_curve::bigint::U256::from_le_bytes(self.bytes).cmp( + &elliptic_curve::bigint::U256::from_le_bytes(other.bytes) + ) } } @@ -1298,16 +1300,21 @@ impl From for elliptic_curve::bigint::U256 { #[cfg(feature = "elliptic-curve")] impl From> for Scalar { fn from(value: elliptic_curve::ScalarPrimitive) -> Self { - // TODO - todo!() + use elliptic_curve::bigint::Encoding; + Scalar { + bytes: value.to_uint().to_le_bytes(), + } } } #[cfg(feature = "elliptic-curve")] impl From for elliptic_curve::ScalarPrimitive { - fn from(value: Scalar) -> Self { - // TODO - todo!() + fn from(mut value: Scalar) -> Self { + value.bytes.reverse(); + let bytes: GenericArray<_, _> = value.bytes.into(); + + elliptic_curve::ScalarPrimitive::from_bytes(&bytes) + .expect("Scalar should have valid bytes") } } From 72ba9e755202a6aca597f4520581dd0d03b2dcd6 Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 27 Apr 2025 18:40:16 -0400 Subject: [PATCH 17/27] fix: things not being behind feature flag --- curve25519-dalek/src/ed25519.rs | 2 ++ curve25519-dalek/src/scalar.rs | 12 +++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/curve25519-dalek/src/ed25519.rs b/curve25519-dalek/src/ed25519.rs index 6a988d159..074a8570c 100644 --- a/curve25519-dalek/src/ed25519.rs +++ b/curve25519-dalek/src/ed25519.rs @@ -2,6 +2,8 @@ use elliptic_curve::{bigint::U256, consts::U32, Curve, CurveArithmetic, FieldByt use crate::{constants::BASEPOINT_ORDER_PRIVATE, edwards::CompressedEdwardsY, EdwardsPoint, Scalar}; +/// QUESTION: I don't know where to put this singleton. Maybe in the crate's root? +/// Otherwise, I thought of something like "singleton" or "curve". #[derive(Clone, Copy, Debug, Default, Eq, PartialEq, PartialOrd, Ord, Hash)] pub struct Ed25519; diff --git a/curve25519-dalek/src/scalar.rs b/curve25519-dalek/src/scalar.rs index a18fbdd48..2e73b0e9b 100644 --- a/curve25519-dalek/src/scalar.rs +++ b/curve25519-dalek/src/scalar.rs @@ -122,7 +122,6 @@ use core::ops::{Sub, SubAssign}; use cfg_if::cfg_if; -use elliptic_curve::bigint::Encoding; #[cfg(feature = "group")] use group::ff::{Field, FromUniformBytes, PrimeField}; #[cfg(feature = "group-bits")] @@ -1224,7 +1223,11 @@ impl UnpackedScalar { // removing the feature flag and implement all of its variants (with u8, &u8, u16, etc.) and the // non-assign `Shr` trait. We probably don't want to impl `Shl` as this can overflow the scalar // and break the invariants. If we don't want to expose this trait, we can also make it -// `#[doc(hidden)]`. +// `#[doc(hidden)]`. For reference, crates in the `RustCrypto/elliptic-curves` repository +// implement: +// - `Shr for &Scalar` +// - `Shr for Scalar` +// - `ShrAssign for Scalar` // // QUESTION: I did not use constant-time because other elliptic-curves dont // (e.g., https://docs.rs/p256/0.13.2/src/p256/arithmetic/scalar.rs.html#426-432). @@ -1252,7 +1255,7 @@ impl elliptic_curve::ops::Reduce for Scalar { } } -// QUESTION: Even though the traits asks for where &self is _greater that or equal_ to `n / 2`, +// QUESTION: Even though the trait asks for "&self is _greater that or equal_ to `n / 2`", // every implementation I have looked at (e.g., // https://docs.rs/p256/latest/src/p256/arithmetic/scalar.rs.html#413-415) uses `ct_gt`. // I decided to do the same, it this correct? @@ -1266,6 +1269,7 @@ impl elliptic_curve::scalar::IsHigh for Scalar { } } +#[cfg(feature = "elliptic-curve")] impl elliptic_curve::ops::Invert for Scalar { type Output = CtOption; @@ -1275,12 +1279,14 @@ impl elliptic_curve::ops::Invert for Scalar { } } +#[cfg(feature = "elliptic-curve")] impl PartialOrd for Scalar { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) } } +#[cfg(feature = "elliptic-curve")] impl Ord for Scalar { fn cmp(&self, other: &Self) -> core::cmp::Ordering { elliptic_curve::bigint::U256::from_le_bytes(self.bytes).cmp( From 49f41fa5d4812e499a46032777f5b0d8c70efad0 Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 27 Apr 2025 18:51:54 -0400 Subject: [PATCH 18/27] remove comment --- curve25519-dalek/src/scalar.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/curve25519-dalek/src/scalar.rs b/curve25519-dalek/src/scalar.rs index 2e73b0e9b..36a0c324a 100644 --- a/curve25519-dalek/src/scalar.rs +++ b/curve25519-dalek/src/scalar.rs @@ -1388,9 +1388,6 @@ impl Field for Scalar { #[cfg(feature = "group")] impl PrimeField for Scalar { - // DISCUSSION: it sucks that we have to use this type but that's what `ArithmeticCurve` - // requires. This is only a minor version bump since this trait was already behind the "group" - // feature. type Repr = GenericArray; fn from_repr(repr: Self::Repr) -> CtOption { From a3c3f520a19b45b331e852ce22d3e0d168cdffc2 Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 27 Apr 2025 18:54:55 -0400 Subject: [PATCH 19/27] fix: clippy lints --- curve25519-dalek/src/scalar.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/curve25519-dalek/src/scalar.rs b/curve25519-dalek/src/scalar.rs index 36a0c324a..fba4d33cb 100644 --- a/curve25519-dalek/src/scalar.rs +++ b/curve25519-dalek/src/scalar.rs @@ -144,7 +144,6 @@ use digest::Digest; use subtle::Choice; use subtle::ConditionallySelectable; use subtle::ConstantTimeEq; -use subtle::ConstantTimeGreater; use subtle::CtOption; #[cfg(feature = "zeroize")] @@ -1263,6 +1262,7 @@ impl elliptic_curve::ops::Reduce for Scalar { impl elliptic_curve::scalar::IsHigh for Scalar { fn is_high(&self) -> Choice { use elliptic_curve::bigint::{Encoding, U256}; + use subtle::ConstantTimeGreater; U256::from_le_bytes(self.bytes).ct_gt(&U256::from_le_bytes( (constants::BASEPOINT_ORDER_PRIVATE * Self::TWO_INV).bytes, )) From d82f71e37bef39de23c38e299b4d33e680814a34 Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 27 Apr 2025 18:56:18 -0400 Subject: [PATCH 20/27] fix comment wording --- curve25519-dalek/src/edwards.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/curve25519-dalek/src/edwards.rs b/curve25519-dalek/src/edwards.rs index 420d96a99..9e2d36894 100644 --- a/curve25519-dalek/src/edwards.rs +++ b/curve25519-dalek/src/edwards.rs @@ -271,8 +271,8 @@ impl elliptic_curve::point::AffineCoordinates for CompressedEdwardsY { fn x(&self) -> Self::FieldRepr { // QUESTION: here we assume that the CompressedEdwardsY valid, and it won't panic in dbg mode. - // We should either change the CompressedEdwardsY API to now allow instancing a - // `CompressedEdwardsY` that is invalid, or clearly document that case somewhere. + // We should either change the CompressedEdwardsY API to not allow instancing a + // `CompressedEdwardsY` that is invalid, or use another type. // How should we handle this? let (is_valid, mut X, _, _) = decompress::step_1(self); debug_assert!(bool::from(is_valid)); From 5d8774f85f688c3b7b812ae0453d9dc3e4ad71d3 Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 27 Apr 2025 18:58:54 -0400 Subject: [PATCH 21/27] standardize Zeroize impls --- curve25519-dalek/src/edwards.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/curve25519-dalek/src/edwards.rs b/curve25519-dalek/src/edwards.rs index 9e2d36894..279addb3c 100644 --- a/curve25519-dalek/src/edwards.rs +++ b/curve25519-dalek/src/edwards.rs @@ -105,7 +105,6 @@ use cfg_if::cfg_if; #[cfg(feature = "digest")] use digest::{generic_array::typenum::{U64, U32}, Digest}; -use zeroize::DefaultIsZeroes; #[cfg(feature = "group")] use { @@ -122,7 +121,7 @@ use subtle::ConditionallySelectable; use subtle::ConstantTimeEq; #[cfg(feature = "zeroize")] -use zeroize::Zeroize; +use zeroize::DefaultIsZeroes; use crate::constants; @@ -1587,11 +1586,7 @@ impl ConditionallySelectable for SubgroupPoint { } #[cfg(all(feature = "group", feature = "zeroize"))] -impl Zeroize for SubgroupPoint { - fn zeroize(&mut self) { - self.0.zeroize(); - } -} +impl DefaultIsZeroes for SubgroupPoint {} #[cfg(feature = "group")] impl group::Group for SubgroupPoint { From 6d6a91ce62d018e6d8aed0acc74249240be32ea8 Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 27 Apr 2025 19:02:20 -0400 Subject: [PATCH 22/27] fix: comment wording --- curve25519-dalek/src/edwards.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/curve25519-dalek/src/edwards.rs b/curve25519-dalek/src/edwards.rs index 279addb3c..b9e360a17 100644 --- a/curve25519-dalek/src/edwards.rs +++ b/curve25519-dalek/src/edwards.rs @@ -1667,7 +1667,8 @@ impl CofactorGroup for EdwardsPoint { // Interop between CompressedEdwardsY and EdwardsPoint for group traits // ------------------------------------------------------------------------ -// Again, here we assume throughout that CompressedEdwardsY is a valid point. +// Again, we assume throughout that CompressedEdwardsY is a valid point (this is not what we +// want, just something that somewhat works until we know what to do). impl From for EdwardsPoint { fn from(value: CompressedEdwardsY) -> Self { From aa1bdba79d58468c3dbcebc8d5fccfd299a93bbe Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 27 Apr 2025 19:04:21 -0400 Subject: [PATCH 23/27] fix: test compilation --- curve25519-dalek/src/scalar.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/curve25519-dalek/src/scalar.rs b/curve25519-dalek/src/scalar.rs index fba4d33cb..dc5150739 100644 --- a/curve25519-dalek/src/scalar.rs +++ b/curve25519-dalek/src/scalar.rs @@ -1289,8 +1289,9 @@ impl PartialOrd for Scalar { #[cfg(feature = "elliptic-curve")] impl Ord for Scalar { fn cmp(&self, other: &Self) -> core::cmp::Ordering { - elliptic_curve::bigint::U256::from_le_bytes(self.bytes).cmp( - &elliptic_curve::bigint::U256::from_le_bytes(other.bytes) + use elliptic_curve::bigint::{Encoding, U256}; + U256::from_le_bytes(self.bytes).cmp( + &U256::from_le_bytes(other.bytes) ) } } From 4cb744fd703383531697949bfdc6bd4dc5ffe929 Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 8 Jun 2025 10:59:00 -0400 Subject: [PATCH 24/27] clean comment --- curve25519-dalek/src/ed25519.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/curve25519-dalek/src/ed25519.rs b/curve25519-dalek/src/ed25519.rs index 074a8570c..521e2b266 100644 --- a/curve25519-dalek/src/ed25519.rs +++ b/curve25519-dalek/src/ed25519.rs @@ -3,7 +3,6 @@ use elliptic_curve::{bigint::U256, consts::U32, Curve, CurveArithmetic, FieldByt use crate::{constants::BASEPOINT_ORDER_PRIVATE, edwards::CompressedEdwardsY, EdwardsPoint, Scalar}; /// QUESTION: I don't know where to put this singleton. Maybe in the crate's root? -/// Otherwise, I thought of something like "singleton" or "curve". #[derive(Clone, Copy, Debug, Default, Eq, PartialEq, PartialOrd, Ord, Hash)] pub struct Ed25519; From bff18e4601c4cf47b61233f658e1252cd8d910d7 Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 8 Jun 2025 13:11:55 -0400 Subject: [PATCH 25/27] add `EdwardsAffinePoint` for `elliptic-curve` trait implementations --- curve25519-dalek/src/ed25519.rs | 6 +- curve25519-dalek/src/edwards.rs | 190 ++++++++++++++++++++++---------- 2 files changed, 137 insertions(+), 59 deletions(-) diff --git a/curve25519-dalek/src/ed25519.rs b/curve25519-dalek/src/ed25519.rs index 521e2b266..dc88b7383 100644 --- a/curve25519-dalek/src/ed25519.rs +++ b/curve25519-dalek/src/ed25519.rs @@ -1,8 +1,8 @@ use elliptic_curve::{bigint::U256, consts::U32, Curve, CurveArithmetic, FieldBytesEncoding}; -use crate::{constants::BASEPOINT_ORDER_PRIVATE, edwards::CompressedEdwardsY, EdwardsPoint, Scalar}; +use crate::{constants::BASEPOINT_ORDER_PRIVATE, edwards::AffineEdwardsPoint, EdwardsPoint, Scalar}; -/// QUESTION: I don't know where to put this singleton. Maybe in the crate's root? +/// QUESTION: I don't know where to put this singleton. Maybe in the crate root? #[derive(Clone, Copy, Debug, Default, Eq, PartialEq, PartialOrd, Ord, Hash)] pub struct Ed25519; @@ -15,7 +15,7 @@ impl Curve for Ed25519 { } impl CurveArithmetic for Ed25519 { - type AffinePoint = CompressedEdwardsY; + type AffinePoint = AffineEdwardsPoint; type ProjectivePoint = EdwardsPoint; diff --git a/curve25519-dalek/src/edwards.rs b/curve25519-dalek/src/edwards.rs index b9e360a17..2edda76cd 100644 --- a/curve25519-dalek/src/edwards.rs +++ b/curve25519-dalek/src/edwards.rs @@ -104,7 +104,13 @@ use core::ops::{Mul, MulAssign}; use cfg_if::cfg_if; #[cfg(feature = "digest")] -use digest::{generic_array::typenum::{U64, U32}, Digest}; +use digest::{ + generic_array::typenum::U64, + Digest, +}; + +#[cfg(feature = "elliptic-curve")] +use digest::generic_array::typenum::U32; #[cfg(feature = "group")] use { @@ -248,43 +254,20 @@ impl TryFrom<&[u8]> for CompressedEdwardsY { } } -// ------------------------------------------------------------------------ -// Constant-time assignment -// ------------------------------------------------------------------------ - -impl ConditionallySelectable for CompressedEdwardsY { - fn conditional_select(a: &CompressedEdwardsY, b: &CompressedEdwardsY, choice: Choice) -> CompressedEdwardsY { - CompressedEdwardsY( - <[u8; 32]>::conditional_select(&a.0, &b.0, choice) - ) - } -} - // ------------------------------------------------------------------------ // Affine Coordinate // ----------------------------------------------------------------------- #[cfg(feature = "elliptic-curve")] -impl elliptic_curve::point::AffineCoordinates for CompressedEdwardsY { +impl elliptic_curve::point::AffineCoordinates for AffineEdwardsPoint { type FieldRepr = digest::generic_array::GenericArray; fn x(&self) -> Self::FieldRepr { - // QUESTION: here we assume that the CompressedEdwardsY valid, and it won't panic in dbg mode. - // We should either change the CompressedEdwardsY API to not allow instancing a - // `CompressedEdwardsY` that is invalid, or use another type. - // How should we handle this? - let (is_valid, mut X, _, _) = decompress::step_1(self); - debug_assert!(bool::from(is_valid)); - - // FieldElement::sqrt_ratio_i always returns the nonnegative square root, - // so we negate according to the supplied sign bit. - let compressed_sign_bit = Choice::from(self.as_bytes()[31] >> 7); - X.conditional_negate(compressed_sign_bit); - X.as_bytes().into() + self.x.as_bytes().into() } fn y_is_odd(&self) -> Choice { - Choice::from(self.as_bytes()[0] & 0x01) + Choice::from(self.y.as_bytes()[0] & 1) } } @@ -415,6 +398,13 @@ pub struct EdwardsPoint { pub(crate) T: FieldElement, } +/// Represents a point on the Edwards form of Curve25519 in affine coordinates. +#[derive(Copy, Clone, Debug)] +pub struct AffineEdwardsPoint { + pub(crate) x: FieldElement, + pub(crate) y: FieldElement, +} + // ------------------------------------------------------------------------ // Constructors // ------------------------------------------------------------------------ @@ -463,6 +453,21 @@ impl Default for EdwardsPoint { } } +impl Identity for AffineEdwardsPoint { + fn identity() -> Self { + Self { + x: FieldElement::ZERO, + y: FieldElement::ONE, + } + } +} + +impl Default for AffineEdwardsPoint { + fn default() -> Self { + Self::identity() + } +} + // ------------------------------------------------------------------------ // Zeroize implementations for wiping points from memory // ------------------------------------------------------------------------ @@ -473,6 +478,9 @@ impl DefaultIsZeroes for CompressedEdwardsY {} #[cfg(feature = "zeroize")] impl DefaultIsZeroes for EdwardsPoint {} +#[cfg(feature = "zeroize")] +impl DefaultIsZeroes for AffineEdwardsPoint {} + // ------------------------------------------------------------------------ // Validity checks (for debugging, not CT) // ------------------------------------------------------------------------ @@ -486,6 +494,12 @@ impl ValidityCheck for EdwardsPoint { } } +impl ValidityCheck for AffineEdwardsPoint { + fn is_valid(&self) -> bool { + self.as_projective().is_valid() + } +} + // ------------------------------------------------------------------------ // Constant-time assignment // ------------------------------------------------------------------------ @@ -501,6 +515,15 @@ impl ConditionallySelectable for EdwardsPoint { } } +impl ConditionallySelectable for AffineEdwardsPoint { + fn conditional_select(a: &Self, b: &Self, choice: Choice) -> Self { + AffineEdwardsPoint { + x: FieldElement::conditional_select(&a.x, &b.x, choice), + y: FieldElement::conditional_select(&a.y, &b.y, choice), + } + } +} + // ------------------------------------------------------------------------ // Equality // ------------------------------------------------------------------------ @@ -526,6 +549,20 @@ impl PartialEq for EdwardsPoint { impl Eq for EdwardsPoint {} +impl ConstantTimeEq for AffineEdwardsPoint { + fn ct_eq(&self, other: &Self) -> Choice { + self.x.ct_eq(&other.x) & self.y.ct_eq(&other.y) + } +} + +impl PartialEq for AffineEdwardsPoint { + fn eq(&self, other: &Self) -> bool { + self.ct_eq(other).into() + } +} + +impl Eq for AffineEdwardsPoint {} + // ------------------------------------------------------------------------ // Point conversions // ------------------------------------------------------------------------ @@ -588,6 +625,14 @@ impl EdwardsPoint { MontgomeryPoint(u.as_bytes()) } + /// Map this point to its affine representation. + pub fn as_affine(&self) -> AffineEdwardsPoint { + let recip = self.Z.invert(); + let x = &self.X * &recip; + let y = &self.Y * &recip; + AffineEdwardsPoint { x, y } + } + /// Compress this point to `CompressedEdwardsY` format. pub fn compress(&self) -> CompressedEdwardsY { let recip = self.Z.invert(); @@ -633,6 +678,32 @@ impl EdwardsPoint { } } +impl AffineEdwardsPoint { + /// Convert the representation of this point from affine + /// coordinates to projective coordinates. + /// + /// Free. + pub(crate) const fn as_projective(&self) -> ProjectivePoint { + ProjectivePoint { + X: self.x, + Y: self.y, + Z: FieldElement::ONE, + } + } + + /// Convert to the \\( \mathbb P\^3 \\) model. + /// + /// This costs \\(1 \mathrm M\\). + pub(crate) fn as_extended(&self) -> EdwardsPoint { + EdwardsPoint { + X: self.x, + Y: self.y, + Z: FieldElement::ONE, + T: &self.x * &self.y, + } + } +} + // ------------------------------------------------------------------------ // Doubling // ------------------------------------------------------------------------ @@ -829,7 +900,6 @@ impl elliptic_curve::ops::MulByGenerator for EdwardsPoint { } } - // ------------------------------------------------------------------------ // Multiscalar Multiplication impls // ------------------------------------------------------------------------ @@ -1331,10 +1401,10 @@ impl Debug for EdwardsPoint { #[cfg(feature = "group")] impl group::Curve for EdwardsPoint { - type AffineRepr = CompressedEdwardsY; + type AffineRepr = AffineEdwardsPoint; fn to_affine(&self) -> Self::AffineRepr { - self.compress() + self.as_affine() } } @@ -1664,82 +1734,79 @@ impl CofactorGroup for EdwardsPoint { } // ------------------------------------------------------------------------ -// Interop between CompressedEdwardsY and EdwardsPoint for group traits +// Interop between AffineEdwardsPoint and EdwardsPoint for group traits // ------------------------------------------------------------------------ // Again, we assume throughout that CompressedEdwardsY is a valid point (this is not what we // want, just something that somewhat works until we know what to do). -impl From for EdwardsPoint { - fn from(value: CompressedEdwardsY) -> Self { - let (_, X, Y, Z) = decompress::step_1(&value); - decompress::step_2(&value, X, Y, Z) +impl From for EdwardsPoint { + fn from(value: AffineEdwardsPoint) -> Self { + value.as_extended() } } -impl From<&CompressedEdwardsY> for EdwardsPoint { - fn from(value: &CompressedEdwardsY) -> Self { - let (_, X, Y, Z) = decompress::step_1(value); - decompress::step_2(value, X, Y, Z) +impl From<&AffineEdwardsPoint> for EdwardsPoint { + fn from(value: &AffineEdwardsPoint) -> Self { + value.as_extended() } } - -impl From for CompressedEdwardsY { +impl From for AffineEdwardsPoint { fn from(value: EdwardsPoint) -> Self { - value.compress() + value.as_affine() } } -impl From<&EdwardsPoint> for CompressedEdwardsY { +impl From<&EdwardsPoint> for AffineEdwardsPoint { fn from(value: &EdwardsPoint) -> Self { - value.compress() + value.as_affine() } } -impl Add<&CompressedEdwardsY> for &EdwardsPoint { +impl Add<&AffineEdwardsPoint> for &EdwardsPoint { type Output = EdwardsPoint; - fn add(self, other: &CompressedEdwardsY) -> EdwardsPoint { + fn add(self, other: &AffineEdwardsPoint) -> EdwardsPoint { self + EdwardsPoint::from(other) } } define_add_variants!( LHS = EdwardsPoint, - RHS = CompressedEdwardsY, + RHS = AffineEdwardsPoint, Output = EdwardsPoint ); -impl AddAssign<&CompressedEdwardsY> for EdwardsPoint { - fn add_assign(&mut self, rhs: &CompressedEdwardsY) { +impl AddAssign<&AffineEdwardsPoint> for EdwardsPoint { + fn add_assign(&mut self, rhs: &AffineEdwardsPoint) { *self += EdwardsPoint::from(rhs); } } -define_add_assign_variants!(LHS = EdwardsPoint, RHS = CompressedEdwardsY); +define_add_assign_variants!(LHS = EdwardsPoint, RHS = AffineEdwardsPoint); -impl Sub<&CompressedEdwardsY> for &EdwardsPoint { +impl Sub<&AffineEdwardsPoint> for &EdwardsPoint { type Output = EdwardsPoint; - fn sub(self, other: &CompressedEdwardsY) -> EdwardsPoint { + fn sub(self, other: &AffineEdwardsPoint) -> EdwardsPoint { self - EdwardsPoint::from(other) } } define_sub_variants!( LHS = EdwardsPoint, - RHS = CompressedEdwardsY, + RHS = AffineEdwardsPoint, Output = EdwardsPoint ); -impl SubAssign<&CompressedEdwardsY> for EdwardsPoint { - fn sub_assign(&mut self, rhs: &CompressedEdwardsY) { +impl SubAssign<&AffineEdwardsPoint> for EdwardsPoint { + fn sub_assign(&mut self, rhs: &AffineEdwardsPoint) { *self -= EdwardsPoint::from(rhs); } } -define_sub_assign_variants!(LHS = EdwardsPoint, RHS = CompressedEdwardsY); +define_sub_assign_variants!(LHS = EdwardsPoint, RHS = AffineEdwardsPoint); // ------------------------------------------------------------------------ // Tests @@ -1827,6 +1894,17 @@ mod test { assert_eq!(bp.compress(), constants::ED25519_BASEPOINT_COMPRESSED); } + /// Test round-trip `EdwardsPoint` <-> `AffineEdwardsPoint` for the basepoint + #[test] + fn basepoint_affine_roundtrip() { + let bp = constants::ED25519_BASEPOINT_POINT; + let affine = bp.as_affine(); + assert!(affine.is_valid()); + let bp_rt = EdwardsPoint::from(affine); + assert!(bp_rt.is_valid()); + assert_eq!(bp, bp_rt); + } + /// Test sign handling in decompression #[test] fn decompression_sign_handling() { From 48962e7c2a5aa1bb6cd6de59acb48226cbf117ba Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 8 Jun 2025 14:34:06 -0400 Subject: [PATCH 26/27] Revert "add `EdwardsAffinePoint` for `elliptic-curve` trait implementations" This reverts commit bff18e4601c4cf47b61233f658e1252cd8d910d7. --- curve25519-dalek/src/ed25519.rs | 6 +- curve25519-dalek/src/edwards.rs | 190 ++++++++++---------------------- 2 files changed, 59 insertions(+), 137 deletions(-) diff --git a/curve25519-dalek/src/ed25519.rs b/curve25519-dalek/src/ed25519.rs index dc88b7383..521e2b266 100644 --- a/curve25519-dalek/src/ed25519.rs +++ b/curve25519-dalek/src/ed25519.rs @@ -1,8 +1,8 @@ use elliptic_curve::{bigint::U256, consts::U32, Curve, CurveArithmetic, FieldBytesEncoding}; -use crate::{constants::BASEPOINT_ORDER_PRIVATE, edwards::AffineEdwardsPoint, EdwardsPoint, Scalar}; +use crate::{constants::BASEPOINT_ORDER_PRIVATE, edwards::CompressedEdwardsY, EdwardsPoint, Scalar}; -/// QUESTION: I don't know where to put this singleton. Maybe in the crate root? +/// QUESTION: I don't know where to put this singleton. Maybe in the crate's root? #[derive(Clone, Copy, Debug, Default, Eq, PartialEq, PartialOrd, Ord, Hash)] pub struct Ed25519; @@ -15,7 +15,7 @@ impl Curve for Ed25519 { } impl CurveArithmetic for Ed25519 { - type AffinePoint = AffineEdwardsPoint; + type AffinePoint = CompressedEdwardsY; type ProjectivePoint = EdwardsPoint; diff --git a/curve25519-dalek/src/edwards.rs b/curve25519-dalek/src/edwards.rs index 2edda76cd..b9e360a17 100644 --- a/curve25519-dalek/src/edwards.rs +++ b/curve25519-dalek/src/edwards.rs @@ -104,13 +104,7 @@ use core::ops::{Mul, MulAssign}; use cfg_if::cfg_if; #[cfg(feature = "digest")] -use digest::{ - generic_array::typenum::U64, - Digest, -}; - -#[cfg(feature = "elliptic-curve")] -use digest::generic_array::typenum::U32; +use digest::{generic_array::typenum::{U64, U32}, Digest}; #[cfg(feature = "group")] use { @@ -254,20 +248,43 @@ impl TryFrom<&[u8]> for CompressedEdwardsY { } } +// ------------------------------------------------------------------------ +// Constant-time assignment +// ------------------------------------------------------------------------ + +impl ConditionallySelectable for CompressedEdwardsY { + fn conditional_select(a: &CompressedEdwardsY, b: &CompressedEdwardsY, choice: Choice) -> CompressedEdwardsY { + CompressedEdwardsY( + <[u8; 32]>::conditional_select(&a.0, &b.0, choice) + ) + } +} + // ------------------------------------------------------------------------ // Affine Coordinate // ----------------------------------------------------------------------- #[cfg(feature = "elliptic-curve")] -impl elliptic_curve::point::AffineCoordinates for AffineEdwardsPoint { +impl elliptic_curve::point::AffineCoordinates for CompressedEdwardsY { type FieldRepr = digest::generic_array::GenericArray; fn x(&self) -> Self::FieldRepr { - self.x.as_bytes().into() + // QUESTION: here we assume that the CompressedEdwardsY valid, and it won't panic in dbg mode. + // We should either change the CompressedEdwardsY API to not allow instancing a + // `CompressedEdwardsY` that is invalid, or use another type. + // How should we handle this? + let (is_valid, mut X, _, _) = decompress::step_1(self); + debug_assert!(bool::from(is_valid)); + + // FieldElement::sqrt_ratio_i always returns the nonnegative square root, + // so we negate according to the supplied sign bit. + let compressed_sign_bit = Choice::from(self.as_bytes()[31] >> 7); + X.conditional_negate(compressed_sign_bit); + X.as_bytes().into() } fn y_is_odd(&self) -> Choice { - Choice::from(self.y.as_bytes()[0] & 1) + Choice::from(self.as_bytes()[0] & 0x01) } } @@ -398,13 +415,6 @@ pub struct EdwardsPoint { pub(crate) T: FieldElement, } -/// Represents a point on the Edwards form of Curve25519 in affine coordinates. -#[derive(Copy, Clone, Debug)] -pub struct AffineEdwardsPoint { - pub(crate) x: FieldElement, - pub(crate) y: FieldElement, -} - // ------------------------------------------------------------------------ // Constructors // ------------------------------------------------------------------------ @@ -453,21 +463,6 @@ impl Default for EdwardsPoint { } } -impl Identity for AffineEdwardsPoint { - fn identity() -> Self { - Self { - x: FieldElement::ZERO, - y: FieldElement::ONE, - } - } -} - -impl Default for AffineEdwardsPoint { - fn default() -> Self { - Self::identity() - } -} - // ------------------------------------------------------------------------ // Zeroize implementations for wiping points from memory // ------------------------------------------------------------------------ @@ -478,9 +473,6 @@ impl DefaultIsZeroes for CompressedEdwardsY {} #[cfg(feature = "zeroize")] impl DefaultIsZeroes for EdwardsPoint {} -#[cfg(feature = "zeroize")] -impl DefaultIsZeroes for AffineEdwardsPoint {} - // ------------------------------------------------------------------------ // Validity checks (for debugging, not CT) // ------------------------------------------------------------------------ @@ -494,12 +486,6 @@ impl ValidityCheck for EdwardsPoint { } } -impl ValidityCheck for AffineEdwardsPoint { - fn is_valid(&self) -> bool { - self.as_projective().is_valid() - } -} - // ------------------------------------------------------------------------ // Constant-time assignment // ------------------------------------------------------------------------ @@ -515,15 +501,6 @@ impl ConditionallySelectable for EdwardsPoint { } } -impl ConditionallySelectable for AffineEdwardsPoint { - fn conditional_select(a: &Self, b: &Self, choice: Choice) -> Self { - AffineEdwardsPoint { - x: FieldElement::conditional_select(&a.x, &b.x, choice), - y: FieldElement::conditional_select(&a.y, &b.y, choice), - } - } -} - // ------------------------------------------------------------------------ // Equality // ------------------------------------------------------------------------ @@ -549,20 +526,6 @@ impl PartialEq for EdwardsPoint { impl Eq for EdwardsPoint {} -impl ConstantTimeEq for AffineEdwardsPoint { - fn ct_eq(&self, other: &Self) -> Choice { - self.x.ct_eq(&other.x) & self.y.ct_eq(&other.y) - } -} - -impl PartialEq for AffineEdwardsPoint { - fn eq(&self, other: &Self) -> bool { - self.ct_eq(other).into() - } -} - -impl Eq for AffineEdwardsPoint {} - // ------------------------------------------------------------------------ // Point conversions // ------------------------------------------------------------------------ @@ -625,14 +588,6 @@ impl EdwardsPoint { MontgomeryPoint(u.as_bytes()) } - /// Map this point to its affine representation. - pub fn as_affine(&self) -> AffineEdwardsPoint { - let recip = self.Z.invert(); - let x = &self.X * &recip; - let y = &self.Y * &recip; - AffineEdwardsPoint { x, y } - } - /// Compress this point to `CompressedEdwardsY` format. pub fn compress(&self) -> CompressedEdwardsY { let recip = self.Z.invert(); @@ -678,32 +633,6 @@ impl EdwardsPoint { } } -impl AffineEdwardsPoint { - /// Convert the representation of this point from affine - /// coordinates to projective coordinates. - /// - /// Free. - pub(crate) const fn as_projective(&self) -> ProjectivePoint { - ProjectivePoint { - X: self.x, - Y: self.y, - Z: FieldElement::ONE, - } - } - - /// Convert to the \\( \mathbb P\^3 \\) model. - /// - /// This costs \\(1 \mathrm M\\). - pub(crate) fn as_extended(&self) -> EdwardsPoint { - EdwardsPoint { - X: self.x, - Y: self.y, - Z: FieldElement::ONE, - T: &self.x * &self.y, - } - } -} - // ------------------------------------------------------------------------ // Doubling // ------------------------------------------------------------------------ @@ -900,6 +829,7 @@ impl elliptic_curve::ops::MulByGenerator for EdwardsPoint { } } + // ------------------------------------------------------------------------ // Multiscalar Multiplication impls // ------------------------------------------------------------------------ @@ -1401,10 +1331,10 @@ impl Debug for EdwardsPoint { #[cfg(feature = "group")] impl group::Curve for EdwardsPoint { - type AffineRepr = AffineEdwardsPoint; + type AffineRepr = CompressedEdwardsY; fn to_affine(&self) -> Self::AffineRepr { - self.as_affine() + self.compress() } } @@ -1734,79 +1664,82 @@ impl CofactorGroup for EdwardsPoint { } // ------------------------------------------------------------------------ -// Interop between AffineEdwardsPoint and EdwardsPoint for group traits +// Interop between CompressedEdwardsY and EdwardsPoint for group traits // ------------------------------------------------------------------------ // Again, we assume throughout that CompressedEdwardsY is a valid point (this is not what we // want, just something that somewhat works until we know what to do). -impl From for EdwardsPoint { - fn from(value: AffineEdwardsPoint) -> Self { - value.as_extended() +impl From for EdwardsPoint { + fn from(value: CompressedEdwardsY) -> Self { + let (_, X, Y, Z) = decompress::step_1(&value); + decompress::step_2(&value, X, Y, Z) } } -impl From<&AffineEdwardsPoint> for EdwardsPoint { - fn from(value: &AffineEdwardsPoint) -> Self { - value.as_extended() +impl From<&CompressedEdwardsY> for EdwardsPoint { + fn from(value: &CompressedEdwardsY) -> Self { + let (_, X, Y, Z) = decompress::step_1(value); + decompress::step_2(value, X, Y, Z) } } -impl From for AffineEdwardsPoint { + +impl From for CompressedEdwardsY { fn from(value: EdwardsPoint) -> Self { - value.as_affine() + value.compress() } } -impl From<&EdwardsPoint> for AffineEdwardsPoint { +impl From<&EdwardsPoint> for CompressedEdwardsY { fn from(value: &EdwardsPoint) -> Self { - value.as_affine() + value.compress() } } -impl Add<&AffineEdwardsPoint> for &EdwardsPoint { +impl Add<&CompressedEdwardsY> for &EdwardsPoint { type Output = EdwardsPoint; - fn add(self, other: &AffineEdwardsPoint) -> EdwardsPoint { + fn add(self, other: &CompressedEdwardsY) -> EdwardsPoint { self + EdwardsPoint::from(other) } } define_add_variants!( LHS = EdwardsPoint, - RHS = AffineEdwardsPoint, + RHS = CompressedEdwardsY, Output = EdwardsPoint ); -impl AddAssign<&AffineEdwardsPoint> for EdwardsPoint { - fn add_assign(&mut self, rhs: &AffineEdwardsPoint) { +impl AddAssign<&CompressedEdwardsY> for EdwardsPoint { + fn add_assign(&mut self, rhs: &CompressedEdwardsY) { *self += EdwardsPoint::from(rhs); } } -define_add_assign_variants!(LHS = EdwardsPoint, RHS = AffineEdwardsPoint); +define_add_assign_variants!(LHS = EdwardsPoint, RHS = CompressedEdwardsY); -impl Sub<&AffineEdwardsPoint> for &EdwardsPoint { +impl Sub<&CompressedEdwardsY> for &EdwardsPoint { type Output = EdwardsPoint; - fn sub(self, other: &AffineEdwardsPoint) -> EdwardsPoint { + fn sub(self, other: &CompressedEdwardsY) -> EdwardsPoint { self - EdwardsPoint::from(other) } } define_sub_variants!( LHS = EdwardsPoint, - RHS = AffineEdwardsPoint, + RHS = CompressedEdwardsY, Output = EdwardsPoint ); -impl SubAssign<&AffineEdwardsPoint> for EdwardsPoint { - fn sub_assign(&mut self, rhs: &AffineEdwardsPoint) { +impl SubAssign<&CompressedEdwardsY> for EdwardsPoint { + fn sub_assign(&mut self, rhs: &CompressedEdwardsY) { *self -= EdwardsPoint::from(rhs); } } -define_sub_assign_variants!(LHS = EdwardsPoint, RHS = AffineEdwardsPoint); +define_sub_assign_variants!(LHS = EdwardsPoint, RHS = CompressedEdwardsY); // ------------------------------------------------------------------------ // Tests @@ -1894,17 +1827,6 @@ mod test { assert_eq!(bp.compress(), constants::ED25519_BASEPOINT_COMPRESSED); } - /// Test round-trip `EdwardsPoint` <-> `AffineEdwardsPoint` for the basepoint - #[test] - fn basepoint_affine_roundtrip() { - let bp = constants::ED25519_BASEPOINT_POINT; - let affine = bp.as_affine(); - assert!(affine.is_valid()); - let bp_rt = EdwardsPoint::from(affine); - assert!(bp_rt.is_valid()); - assert_eq!(bp, bp_rt); - } - /// Test sign handling in decompression #[test] fn decompression_sign_handling() { From 780590230bfa57daa06c54fe9438363ecd3ff006 Mon Sep 17 00:00:00 2001 From: Charles Edward Gagnon Date: Sun, 8 Jun 2025 14:53:01 -0400 Subject: [PATCH 27/27] fixup implementation with new affine point --- curve25519-dalek/src/ed25519.rs | 4 +- curve25519-dalek/src/edwards.rs | 92 +++++++------------------- curve25519-dalek/src/edwards/affine.rs | 13 ++++ 3 files changed, 40 insertions(+), 69 deletions(-) diff --git a/curve25519-dalek/src/ed25519.rs b/curve25519-dalek/src/ed25519.rs index 521e2b266..f46f05373 100644 --- a/curve25519-dalek/src/ed25519.rs +++ b/curve25519-dalek/src/ed25519.rs @@ -1,6 +1,6 @@ use elliptic_curve::{bigint::U256, consts::U32, Curve, CurveArithmetic, FieldBytesEncoding}; -use crate::{constants::BASEPOINT_ORDER_PRIVATE, edwards::CompressedEdwardsY, EdwardsPoint, Scalar}; +use crate::{constants::BASEPOINT_ORDER_PRIVATE, edwards::affine::AffinePoint, EdwardsPoint, Scalar}; /// QUESTION: I don't know where to put this singleton. Maybe in the crate's root? #[derive(Clone, Copy, Debug, Default, Eq, PartialEq, PartialOrd, Ord, Hash)] @@ -15,7 +15,7 @@ impl Curve for Ed25519 { } impl CurveArithmetic for Ed25519 { - type AffinePoint = CompressedEdwardsY; + type AffinePoint = AffinePoint; type ProjectivePoint = EdwardsPoint; diff --git a/curve25519-dalek/src/edwards.rs b/curve25519-dalek/src/edwards.rs index d57064886..4e2023558 100644 --- a/curve25519-dalek/src/edwards.rs +++ b/curve25519-dalek/src/edwards.rs @@ -93,7 +93,7 @@ // affine and projective cakes and eat both of them too. #![allow(non_snake_case)] -mod affine; +pub(crate) mod affine; use cfg_if::cfg_if; use core::array::TryFromSliceError; @@ -253,46 +253,6 @@ impl TryFrom<&[u8]> for CompressedEdwardsY { } } -// ------------------------------------------------------------------------ -// Constant-time assignment -// ------------------------------------------------------------------------ - -impl ConditionallySelectable for CompressedEdwardsY { - fn conditional_select(a: &CompressedEdwardsY, b: &CompressedEdwardsY, choice: Choice) -> CompressedEdwardsY { - CompressedEdwardsY( - <[u8; 32]>::conditional_select(&a.0, &b.0, choice) - ) - } -} - -// ------------------------------------------------------------------------ -// Affine Coordinate -// ----------------------------------------------------------------------- - -#[cfg(feature = "elliptic-curve")] -impl elliptic_curve::point::AffineCoordinates for CompressedEdwardsY { - type FieldRepr = digest::generic_array::GenericArray; - - fn x(&self) -> Self::FieldRepr { - // QUESTION: here we assume that the CompressedEdwardsY valid, and it won't panic in dbg mode. - // We should either change the CompressedEdwardsY API to not allow instancing a - // `CompressedEdwardsY` that is invalid, or use another type. - // How should we handle this? - let (is_valid, mut X, _, _) = decompress::step_1(self); - debug_assert!(bool::from(is_valid)); - - // FieldElement::sqrt_ratio_i always returns the nonnegative square root, - // so we negate according to the supplied sign bit. - let compressed_sign_bit = Choice::from(self.as_bytes()[31] >> 7); - X.conditional_negate(compressed_sign_bit); - X.as_bytes().into() - } - - fn y_is_odd(&self) -> Choice { - Choice::from(self.as_bytes()[0] & 0x01) - } -} - // ------------------------------------------------------------------------ // Serde support // ------------------------------------------------------------------------ @@ -1404,10 +1364,10 @@ impl Debug for EdwardsPoint { #[cfg(feature = "group")] impl group::Curve for EdwardsPoint { - type AffineRepr = CompressedEdwardsY; + type AffineRepr = AffinePoint; fn to_affine(&self) -> Self::AffineRepr { - self.compress() + EdwardsPoint::to_affine(*self) } } @@ -1736,76 +1696,74 @@ impl CofactorGroup for EdwardsPoint { // Again, we assume throughout that CompressedEdwardsY is a valid point (this is not what we // want, just something that somewhat works until we know what to do). -impl From for EdwardsPoint { - fn from(value: CompressedEdwardsY) -> Self { - let (_, X, Y, Z) = decompress::step_1(&value); - decompress::step_2(&value, X, Y, Z) +impl From for EdwardsPoint { + fn from(value: AffinePoint) -> Self { + value.to_edwards() } } -impl From<&CompressedEdwardsY> for EdwardsPoint { - fn from(value: &CompressedEdwardsY) -> Self { - let (_, X, Y, Z) = decompress::step_1(value); - decompress::step_2(value, X, Y, Z) +impl From<&AffinePoint> for EdwardsPoint { + fn from(value: &AffinePoint) -> Self { + value.to_edwards() } } -impl From for CompressedEdwardsY { +impl From for AffinePoint { fn from(value: EdwardsPoint) -> Self { - value.compress() + value.to_affine() } } -impl From<&EdwardsPoint> for CompressedEdwardsY { +impl From<&EdwardsPoint> for AffinePoint { fn from(value: &EdwardsPoint) -> Self { - value.compress() + value.to_affine() } } -impl Add<&CompressedEdwardsY> for &EdwardsPoint { +impl Add<&AffinePoint> for &EdwardsPoint { type Output = EdwardsPoint; - fn add(self, other: &CompressedEdwardsY) -> EdwardsPoint { + fn add(self, other: &AffinePoint) -> EdwardsPoint { self + EdwardsPoint::from(other) } } define_add_variants!( LHS = EdwardsPoint, - RHS = CompressedEdwardsY, + RHS = AffinePoint, Output = EdwardsPoint ); -impl AddAssign<&CompressedEdwardsY> for EdwardsPoint { - fn add_assign(&mut self, rhs: &CompressedEdwardsY) { +impl AddAssign<&AffinePoint> for EdwardsPoint { + fn add_assign(&mut self, rhs: &AffinePoint) { *self += EdwardsPoint::from(rhs); } } -define_add_assign_variants!(LHS = EdwardsPoint, RHS = CompressedEdwardsY); +define_add_assign_variants!(LHS = EdwardsPoint, RHS = AffinePoint); -impl Sub<&CompressedEdwardsY> for &EdwardsPoint { +impl Sub<&AffinePoint> for &EdwardsPoint { type Output = EdwardsPoint; - fn sub(self, other: &CompressedEdwardsY) -> EdwardsPoint { + fn sub(self, other: &AffinePoint) -> EdwardsPoint { self - EdwardsPoint::from(other) } } define_sub_variants!( LHS = EdwardsPoint, - RHS = CompressedEdwardsY, + RHS = AffinePoint, Output = EdwardsPoint ); -impl SubAssign<&CompressedEdwardsY> for EdwardsPoint { - fn sub_assign(&mut self, rhs: &CompressedEdwardsY) { +impl SubAssign<&AffinePoint> for EdwardsPoint { + fn sub_assign(&mut self, rhs: &AffinePoint) { *self -= EdwardsPoint::from(rhs); } } -define_sub_assign_variants!(LHS = EdwardsPoint, RHS = CompressedEdwardsY); +define_sub_assign_variants!(LHS = EdwardsPoint, RHS = AffinePoint); // ------------------------------------------------------------------------ // Tests diff --git a/curve25519-dalek/src/edwards/affine.rs b/curve25519-dalek/src/edwards/affine.rs index aa68d8eae..524444920 100644 --- a/curve25519-dalek/src/edwards/affine.rs +++ b/curve25519-dalek/src/edwards/affine.rs @@ -93,6 +93,19 @@ impl Mul<&AffinePoint> for Scalar { } } +#[cfg(feature = "elliptic-curve")] +impl elliptic_curve::point::AffineCoordinates for AffinePoint { + type FieldRepr = digest::generic_array::GenericArray; + + fn x(&self) -> Self::FieldRepr { + self.x.to_bytes().into() + } + + fn y_is_odd(&self) -> Choice { + Choice::from(self.y.to_bytes()[0] & 1) + } +} + #[cfg(test)] mod tests { use super::{AffinePoint, EdwardsPoint, Identity};