Skip to content
2 changes: 1 addition & 1 deletion curve25519-dalek/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ curve25519-dalek = ">= 5.0, < 5.2"
| `digest` | | Enables `RistrettoPoint::{from_hash, hash_from_bytes}` and `Scalar::{from_hash, hash_from_bytes}`. This is an optional dependency whose version is not subject to SemVer. See [below](#public-api-semver-exemptions) for more details. |
| `serde` | | Enables `serde` serialization/deserialization for all the point and scalar types. |
| `legacy_compatibility`| | Enables `Scalar::from_bits`, which allows the user to build unreduced scalars whose arithmetic is broken. Do not use this unless you know what you're doing. |
| `group` | | Enables external `group` and `ff` crate traits. |
| `group` | | Enables external `group` and `ff` crate traits, exposing an implementation of the Ed25519 field. |
| `group-bits` | | Enables `group` and impls `ff::PrimeFieldBits` for `Scalar`. |

To disable the default features when using `curve25519-dalek` as a dependency,
Expand Down
284 changes: 283 additions & 1 deletion curve25519-dalek/src/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ impl ConstantTimeEq for FieldElement {

impl FieldElement {
/// Load a `FieldElement` from 64 bytes, by reducing modulo q.
#[cfg(feature = "digest")]
#[cfg(any(feature = "digest", feature = "group"))]
pub(crate) fn from_bytes_wide(bytes: &[u8; 64]) -> Self {
let mut fl = [0u8; 32];
let mut gl = [0u8; 32];
Expand Down Expand Up @@ -410,6 +410,288 @@ impl FieldElement {
}
}

#[cfg(feature = "group")]
mod group {
use super::FieldElement;
use core::{
fmt::Debug,
iter::{Product, Sum},
ops::{Add, AddAssign, Mul, MulAssign, Neg, Sub, SubAssign},
};
use ff::{Field, FromUniformBytes, PrimeField};
use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption};

/// A `FieldElement` represents an element of the field
/// \\( \mathbb Z / (2\^{255} - 19)\\).
///
/// The `FieldElement` type is an alias for one of the platform-specific
/// implementations. Its size and internals are not guaranteed to have
/// any specific properties and are not covered by semver.
#[derive(Clone, Copy, PartialEq, Eq)]
pub struct FfFieldElement(FieldElement);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about adding another field element type, especially with a name like FfFieldElement (Finite field field element?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a thin wrapper around the existing FieldElement into the ff backend. It's exported publicly as FieldElement, but as it's a distinct type, it obviously needed its own name internally. The alternative would be this in its own module (as now), with the underlying FieldElement imported under the alias UnderlyingFieldElement, which an earlier draft of mine did. I'm happy to make that change, though I'll note I don't substantially believe this to be new.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry. I realize now my comment doesn't iterate over the primary issue. FieldElement does not satisfy the ff API and will not without invasive changes and performance regressions, due to its policy of lazy reduction.

A new type, isolating from the rest of dalek while ensuring proper fulfillment of the intended API, truly seemed optimal.

Alternatively, exposing everything under a hazmat feature and then a wrapper crate may define this ff-compliant type with a pin to an exact version of dalek, including patch, would work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are so many field element types in this codebase I worry about introducing another, especially one with the same name as a different FieldElement type (but only when re-exported), it just seems very confusing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heard. While I can point out it's a higher-level on the stack and feature-gated, it'd be better to discuss solutions.

How about it's own file, plus the nomenclature Ff25519 or anything of similar distance? The alternatives would be:

  1. Exposing the unsafe FieldElement type, under a hazmat feature, letting crates define this ff wrapper itself. I'm fine with that but consider it more damaging than any complexity of this patch.
  2. Not exposing the 2**255-19 field. As someone who's maintained my own field impl, and am now planning to maintain my own fork of dalek with this patch if not upstreamed, I'd continue as I have but be disappointed. I truly believe this represents a real-world use and this patch can be done incredibly non-intrusively.


impl Default for FfFieldElement {
fn default() -> Self {
FfFieldElement(FieldElement::ZERO)
}
}

impl Debug for FfFieldElement {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(
f,
"FfFieldElement{{\n\tbytes: {:?},\n}}",
&self.0.to_bytes()
)
}
}

impl From<u64> for FfFieldElement {
fn from(a: u64) -> Self {
// Portable method to convert a u64 to a FfFieldElement,
// regardless of the internal representation
let mut bytes = [0; 32];
bytes[..8].copy_from_slice(&a.to_le_bytes());
Self(FieldElement::from_bytes(&bytes))
}
}

impl Add<&FfFieldElement> for FfFieldElement {
type Output = Self;
fn add(self, other: &Self) -> Self {
let unreduced = &self.0 + &other.0;
// Force a reduction
Self(FieldElement::from_bytes(&unreduced.to_bytes()))
}
}
#[allow(clippy::op_ref)]
impl Add for FfFieldElement {
type Output = Self;
fn add(self, other: Self) -> Self {
self + &other
}
}
impl AddAssign for FfFieldElement {
fn add_assign(&mut self, other: Self) {
*self = *self + other;
}
}
impl AddAssign<&FfFieldElement> for FfFieldElement {
fn add_assign(&mut self, other: &Self) {
*self = *self + other;
}
}

impl Sub<&FfFieldElement> for FfFieldElement {
type Output = Self;
fn sub(self, other: &Self) -> Self {
let unreduced = &self.0 - &other.0;
// Force a reduction
Self(FieldElement::from_bytes(&unreduced.to_bytes()))
}
}
#[allow(clippy::op_ref)]
impl Sub for FfFieldElement {
type Output = Self;
fn sub(self, other: Self) -> Self {
self - &other
}
}
impl SubAssign for FfFieldElement {
fn sub_assign(&mut self, other: Self) {
*self = *self - other;
}
}
impl SubAssign<&FfFieldElement> for FfFieldElement {
fn sub_assign(&mut self, other: &Self) {
*self = *self - other;
}
}

impl Neg for FfFieldElement {
type Output = Self;
fn neg(mut self) -> Self {
self.0.negate();
Self(FieldElement::from_bytes(&self.0.to_bytes()))
}
}

impl Mul<&FfFieldElement> for FfFieldElement {
type Output = Self;
fn mul(self, other: &Self) -> Self {
let unreduced = &self.0 * &other.0;
// Force a reduction
Self(FieldElement::from_bytes(&unreduced.to_bytes()))
}
}
#[allow(clippy::op_ref)]
impl Mul for FfFieldElement {
type Output = Self;
fn mul(self, other: Self) -> Self {
self * &other
}
}
impl MulAssign for FfFieldElement {
fn mul_assign(&mut self, other: Self) {
*self = *self * other;
}
}
impl MulAssign<&FfFieldElement> for FfFieldElement {
fn mul_assign(&mut self, other: &Self) {
*self = *self * other;
}
}

impl Sum for FfFieldElement {
fn sum<I: Iterator<Item = FfFieldElement>>(iter: I) -> Self {
let mut res = FfFieldElement::ZERO;
for item in iter {
res += item;
}
res
}
}
impl<'a> Sum<&'a FfFieldElement> for FfFieldElement {
fn sum<I: Iterator<Item = &'a FfFieldElement>>(iter: I) -> Self {
iter.copied().sum()
}
}

impl Product<FfFieldElement> for FfFieldElement {
fn product<I: Iterator<Item = FfFieldElement>>(iter: I) -> Self {
let mut res = FfFieldElement::ONE;
for item in iter {
res *= item;
}
res
}
}
impl<'a> Product<&'a FfFieldElement> for FfFieldElement {
fn product<I: Iterator<Item = &'a FfFieldElement>>(iter: I) -> Self {
iter.copied().product()
}
}

impl ConstantTimeEq for FfFieldElement {
fn ct_eq(&self, other: &Self) -> Choice {
self.0.ct_eq(&other.0)
}
}

impl ConditionallySelectable for FfFieldElement {
fn conditional_select(a: &Self, b: &Self, choice: Choice) -> Self {
Self(<_>::conditional_select(&a.0, &b.0, choice))
}
}

#[cfg(feature = "zeroize")]
impl zeroize::Zeroize for FfFieldElement {
fn zeroize(&mut self) {
self.0.zeroize();
}
}

impl Field for FfFieldElement {
const ZERO: Self = Self(FieldElement::ZERO);
const ONE: Self = Self(FieldElement::ONE);

fn try_from_rng<R: rand_core::TryRngCore + ?Sized>(rng: &mut R) -> Result<Self, R::Error> {
let mut bytes = [0; 64];
rng.try_fill_bytes(&mut bytes)?;
Ok(Self(FieldElement::from_bytes_wide(&bytes)))
}

fn square(&self) -> Self {
*self * self
}

fn double(&self) -> Self {
*self + self
}

fn invert(&self) -> CtOption<Self> {
CtOption::new(Self(self.0.invert()), !self.is_zero())
}

fn sqrt_ratio(num: &Self, div: &Self) -> (Choice, Self) {
let res = FieldElement::sqrt_ratio_i(&num.0, &div.0);
(res.0, Self(res.1))
}
}

impl PrimeField for FfFieldElement {
type Repr = [u8; 32];

fn from_repr(repr: Self::Repr) -> CtOption<Self> {
let res = Self(FieldElement::from_bytes(&repr));
CtOption::new(res, repr.ct_eq(&res.0.to_bytes()))
}

fn from_repr_vartime(repr: Self::Repr) -> Option<Self> {
Self::from_repr(repr).into()
}

fn to_repr(&self) -> Self::Repr {
self.0.to_bytes()
}

fn is_odd(&self) -> Choice {
Choice::from(self.0.to_bytes()[0] & 1)
}

const MODULUS: &'static str =
"0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffed";
const NUM_BITS: u32 = 255;
const CAPACITY: u32 = 254;

const TWO_INV: Self = Self(FieldElement::from_bytes(&[
247, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255,
255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 63,
]));
const MULTIPLICATIVE_GENERATOR: Self = Self(FieldElement::from_bytes(&[
2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0,
]));
const S: u32 = 2;
const ROOT_OF_UNITY: Self = Self(FieldElement::from_bytes(&[
176, 160, 14, 74, 39, 27, 238, 196, 120, 228, 47, 173, 6, 24, 67, 47, 167, 215, 251,
61, 153, 0, 77, 43, 11, 223, 193, 79, 128, 36, 131, 43,
]));
const ROOT_OF_UNITY_INV: Self = Self(FieldElement::from_bytes(&[
61, 95, 241, 181, 216, 228, 17, 59, 135, 27, 208, 82, 249, 231, 188, 208, 88, 40, 4,
194, 102, 255, 178, 212, 244, 32, 62, 176, 127, 219, 124, 84,
]));
const DELTA: Self = Self(FieldElement::from_bytes(&[
16, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0,
]));
}

#[cfg(feature = "group-bits")]
impl ff::PrimeFieldBits for FfFieldElement {
type ReprBits = [u8; 32];

fn to_le_bits(&self) -> ff::FieldBits<Self::ReprBits> {
self.to_repr().into()
}

fn char_le_bits() -> ff::FieldBits<Self::ReprBits> {
[
237, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255,
255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 127,
]
.into()
}
}
impl FromUniformBytes<64> for FfFieldElement {
fn from_uniform_bytes(bytes: &[u8; 64]) -> Self {
Self(FieldElement::from_bytes_wide(bytes))
}
}
}
#[cfg(feature = "group")]
pub use group::FfFieldElement;

#[cfg(test)]
mod test {
use crate::field::*;
Expand Down
2 changes: 2 additions & 0 deletions curve25519-dalek/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ pub(crate) mod window;
pub use crate::{
edwards::EdwardsPoint, montgomery::MontgomeryPoint, ristretto::RistrettoPoint, scalar::Scalar,
};
#[cfg(feature = "group")]
pub use field::FfFieldElement as FieldElement;

// Build time diagnostics for validation
#[cfg(curve25519_dalek_diagnostics = "build")]
Expand Down