From f3466bec8f198ac2db943b0cb2648337e4a8b7cd Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Wed, 3 Sep 2025 06:00:52 -0400 Subject: [PATCH 1/2] Add `GroupCanonicalEncoding` `PrimeField::from_repr` required the representation of an element be canonical. `GroupEncoding::from_bytes` (unfortunately) did not, and implementations have not always required points be canonical. Notably, `curve25519-dalek` does not (https://github.com/dalek-cryptography/curve25519-dalek/issues/380). Instead of modifying `GroupEncoding` to state representations must be canonical, which may be a debated change and would be easily overlooked when updating, a new trait is added. This shouldn't be invasive at all, yet allows callers to require a canonical encoding and implementations to declare they offer one. I'm unaware of any library whose `to_bytes` isn't canonical, yet I added a `to_canonical_bytes` for parity with `from_canonical_bytes`. `from_bytes_unchecked` is left without a counterpart as the entire point of this trait is to perform a canonicity check. Descending from `GroupEncoding` (necessary to access `Repr` via) allows the original `from_bytes_unchecked` to still be used. This is necessary as currently, within a generic setting, a bespoke marker trait must be defined and manually implemented, or callers must assume `to_bytes` will be canonical and after decoding, re-encode to check for equivalency. This generally incurs a field inversion which is quite expensive. --- src/lib.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index cd3cf5e..f63f913 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -174,6 +174,19 @@ pub trait GroupEncoding: Sized { fn to_bytes(&self) -> Self::Repr; } +/// `GroupEncoding` with an additional bound that byte representations be canonical. +pub trait GroupCanonicalEncoding: Sized + GroupEncoding { + /// Attempts to deserialize a group element from its canonical encoding. + /// + /// For any returned point, it will be returned if and only if the exact argument `bytes` was + /// passed into this function. This implies checking the coordinate(s) were reduced, and flags, + /// sign bits, etc were minimal. + fn from_canonical_bytes(bytes: &Self::Repr) -> CtOption; + + /// Converts this element into its canonical byte encoding. + fn to_canonical_bytes(&self) -> Self::Repr; +} + /// Affine representation of a point on an elliptic curve that has a defined uncompressed /// encoding. pub trait UncompressedEncoding: Sized { From 4889613a8931eaecd6c3db77599754f6985341ea Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Wed, 3 Sep 2025 12:10:38 -0400 Subject: [PATCH 2/2] `GroupCanonicalEncoding` which is a marker trait --- src/lib.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f63f913..a747488 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,7 +15,7 @@ use core::iter::Sum; use core::ops::{Add, AddAssign, Mul, MulAssign, Neg, Sub, SubAssign}; use ff::PrimeField; use rand_core::{RngCore, TryRngCore}; -use subtle::{Choice, CtOption}; +use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption}; pub mod cofactor; pub mod prime; @@ -174,17 +174,21 @@ pub trait GroupEncoding: Sized { fn to_bytes(&self) -> Self::Repr; } -/// `GroupEncoding` with an additional bound that byte representations be canonical. -pub trait GroupCanonicalEncoding: Sized + GroupEncoding { +/// A marker trait that `::to_bytes()` always yields a canonical encoding. +pub trait GroupCanonicalEncoding: + Sized + Default + ConditionallySelectable + GroupEncoding +{ /// Attempts to deserialize a group element from its canonical encoding. /// - /// For any returned point, it will be returned if and only if the exact argument `bytes` was - /// passed into this function. This implies checking the coordinate(s) were reduced, and flags, - /// sign bits, etc were minimal. - fn from_canonical_bytes(bytes: &Self::Repr) -> CtOption; - - /// Converts this element into its canonical byte encoding. - fn to_canonical_bytes(&self) -> Self::Repr; + /// For any returned point, it will be returned if and only if the exact representation present + /// within `bytes` was passed into this function. This implies checking the coordinate(s) were + /// reduced, and flags, sign bits, etc were minimal. + fn from_canonical_bytes(bytes: &Self::Repr) -> CtOption { + let res = Self::from_bytes(bytes).unwrap_or(Self::default()); + // Safe due to the bound points are always encoded canonically + let canonical = res.to_bytes().as_ref().ct_eq(bytes.as_ref()); + CtOption::new(res, canonical) + } } /// Affine representation of a point on an elliptic curve that has a defined uncompressed