Skip to content

Commit

Permalink
p384: use canonical form constants (#592)
Browse files Browse the repository at this point in the history 
Now that conversions to/from the Montgomery domain are `const fn` (#590)
it's possible to define constants in their canonical form, rather than
converting them into the Montgomery domain first.

This makes the code clearer and easier to audit.
  • Loading branch information
@tarcieri
tarcieri committed Jun 3, 2022
1 parent d32f8d7 commit 0dbf939
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 58 deletions.
13 changes: 6 additions & 7 deletions p384/src/arithmetic.rs
View file
Expand Up @@ -19,15 +19,14 @@ use self::{
projective::ProjectivePoint,
scalar::Scalar,
};
use crate::U384;

/// a = -3 (0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffeffffffff0000000000000000fffffffc)
///
/// NOTE: field element has been translated into the Montgomery domain.
const CURVE_EQUATION_A: FieldElement = FieldElement(U384::from_be_hex("fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffbfffffffc0000000000000003fffffffc"));
const CURVE_EQUATION_A: FieldElement = FieldElement::ZERO
.sub(&FieldElement::ONE)
.sub(&FieldElement::ONE)
.sub(&FieldElement::ONE);

/// b = b3312fa7 e23ee7e4 988e056b e3f82d19 181d9c6e fe814112
/// 0314088f 5013875a c656398d 8a2ed19d 2a85c8ed d3ec2aef
///
/// NOTE: field element has been translated into the Montgomery domain.
const CURVE_EQUATION_B: FieldElement = FieldElement(U384::from_be_hex("cd08114b604fbff9b62b21f41f022094e3374bee94938ae277f2209b1920022ef729add87a4c32ec081188719d412dcc"));
const CURVE_EQUATION_B: FieldElement =
FieldElement::from_be_hex("b3312fa7e23ee7e4988e056be3f82d19181d9c6efe8141120314088f5013875ac656398d8a2ed19d2a85c8edd3ec2aef");
6 changes: 3 additions & 3 deletions p384/src/arithmetic/affine.rs
View file
Expand Up @@ -5,7 +5,7 @@
use core::ops::{Mul, Neg};

use super::{FieldElement, ProjectivePoint, CURVE_EQUATION_A, CURVE_EQUATION_B, MODULUS};
use crate::{CompressedPoint, EncodedPoint, FieldBytes, NistP384, PublicKey, Scalar, U384};
use crate::{CompressedPoint, EncodedPoint, FieldBytes, NistP384, PublicKey, Scalar};
use elliptic_curve::{
group::{prime::PrimeCurveAffine, GroupEncoding},
sec1::{self, FromEncodedPoint, ToEncodedPoint},
Expand Down Expand Up @@ -66,8 +66,8 @@ impl AffinePoint {
/// NOTE: coordinate field elements have been translated into the Montgomery
/// domain.
pub const GENERATOR: Self = Self {
x: FieldElement(U384::from_be_hex("4d3aadc2299e1513812ff723614ede2b6454868459a30eff879c3afc541b4d6e20e378e2a0d6ce383dd0756649c0b528")),
y: FieldElement(U384::from_be_hex("2b78abc25a15c5e9dd8002263969a840c6c3521968f4ffd98bade7562e83b050a1bfa8bf7bb4a9ac23043dad4b03a4fe")),
x: FieldElement::from_be_hex("aa87ca22be8b05378eb1c71ef320ad746e1d3b628ba79b9859f741e082542a385502f25dbf55296c3a545e3872760ab7"),
y: FieldElement::from_be_hex("3617de4a96262c6f5d9e98bf9292dc29f8f41dbd289a147ce9da3113b5f0b8c00a60b1ce1d7e819d7a431d7c90ea0e5f"),
infinity: 0,
};

Expand Down
16 changes: 4 additions & 12 deletions p384/src/arithmetic/field.rs
View file
Expand Up @@ -40,10 +40,11 @@ pub(crate) const MODULUS: U384 = U384::from_be_hex("ffffffffffffffffffffffffffff
#[derive(Clone, Copy, Debug)]
pub struct FieldElement(pub(super) U384);

impl_sec1_field_element!(
impl_field_element!(
FieldElement,
U384,
FieldBytes,
U384,
MODULUS,
fiat_p384_montgomery_domain_field_element,
fiat_p384_from_montgomery,
fiat_p384_to_montgomery,
Expand All @@ -55,8 +56,6 @@ impl_sec1_field_element!(
fiat_p384_divstep_precomp,
fiat_p384_divstep,
fiat_p384_msat,
MODULUS,
"000000000000000000000000000000000000000000000000000000000000000100000000ffffffffffffffff00000001"
);

impl FieldElement {
Expand Down Expand Up @@ -111,14 +110,7 @@ impl FieldElement {

#[cfg(test)]
mod tests {
use super::{fiat_p384_to_montgomery, FieldElement, U384};

/// Test that the precomputed `FieldElement::ONE` constant is correct.
#[test]
fn one() {
let one_mont = fiat_p384_to_montgomery(U384::ONE.as_ref());
assert_eq!(FieldElement(one_mont.into()), FieldElement::ONE);
}
use super::FieldElement;

/// Basic tests that field inversion works.
#[test]
Expand Down
48 changes: 33 additions & 15 deletions p384/src/arithmetic/macros.rs
View file
Expand Up @@ -45,11 +45,12 @@
/// - `Mul`
/// - `MulAssign`
/// - `Neg`
macro_rules! impl_sec1_field_element {
macro_rules! impl_field_element {
(
$fe:tt,
$uint:ty,
$bytes:ty,
$uint:ty,
$modulus:expr,
$arr:ty,
$from_mont:ident,
$to_mont:ident,
Expand All @@ -61,15 +62,13 @@ macro_rules! impl_sec1_field_element {
$divstep_precomp:ident,
$divstep:ident,
$msat:ident,
$mod:expr,
$one:expr
) => {
impl $fe {
/// Zero element.
pub const ZERO: Self = Self(<$uint>::ZERO);

/// Multiplicative identity.
pub const ONE: Self = Self(<$uint>::from_be_hex($one));
pub const ONE: Self = Self::from_uint_unchecked(<$uint>::ONE);

/// Create a [`
#[doc = stringify!($fe)]
Expand Down Expand Up @@ -116,17 +115,41 @@ macro_rules! impl_sec1_field_element {
/// w * R^2 * R^-1 mod p = wR mod p
/// ```
pub fn from_uint(uint: $uint) -> ::elliptic_curve::subtle::CtOption<Self> {
let is_some = uint.ct_lt(&$mod);
let is_some = uint.ct_lt(&$modulus);
::elliptic_curve::subtle::CtOption::new(Self::from_uint_unchecked(uint), is_some)
}

/// Parse a [`
#[doc = stringify!($fe)]
/// `] from big endian hex-encoded bytes.
///
/// Does *not* perform a check that the field element does not overflow the order.
///
/// This method is primarily intended for defining internal constants.
#[allow(dead_code)]
pub(crate) const fn from_be_hex(hex: &str) -> Self {
Self::from_uint_unchecked(<$uint>::from_be_hex(hex))
}

/// Parse a [`
#[doc = stringify!($fe)]
/// `] from little endian hex-encoded bytes.
///
/// Does *not* perform a check that the field element does not overflow the order.
///
/// This method is primarily intended for defining internal constants.
#[allow(dead_code)]
pub(crate) const fn from_le_hex(hex: &str) -> Self {
Self::from_uint_unchecked(<$uint>::from_le_hex(hex))
}

/// Decode [`
#[doc = stringify!($fe)]
/// `] from [`
#[doc = stringify!($uint)]
/// `] converting it into Montgomery form.
///
/// Does not perform a check that the field element does not overflow the order.
/// Does *not* perform a check that the field element does not overflow the order.
///
/// Used incorrectly this can lead to invalid results!
const fn from_uint_unchecked(w: $uint) -> Self {
Expand Down Expand Up @@ -225,18 +248,13 @@ macro_rules! impl_sec1_field_element {

const LIMBS: usize = ::elliptic_curve::bigint::nlimbs!(<$uint>::BIT_SIZE);
const ITERATIONS: usize = (49 * <$uint>::BIT_SIZE + 57) / 17;
type XLimbs = [Word; LIMBS + 1];

let mut d: Word = 1;
let mut d = 1;
let mut f = $msat();

let mut g = XLimbs::default();
let mut g: [Word; LIMBS + 1] = Default::default();
g[..LIMBS].copy_from_slice(&$from_mont(self.as_ref()));

let mut r = <$arr>::from(Self::ONE.0);
let mut v = <$arr>::default();
let precomp = $divstep_precomp();

let mut i: usize = 0;

while i < ITERATIONS - ITERATIONS % 2 {
Expand Down Expand Up @@ -264,7 +282,7 @@ macro_rules! impl_sec1_field_element {
);

let v = <$uint>::conditional_select(&v, &v_opp, s);
let ret = $mul(v.as_ref(), &precomp);
let ret = $mul(v.as_ref(), &$divstep_precomp());
::elliptic_curve::subtle::CtOption::new(Self(ret.into()), !self.is_zero())
}

Expand Down
30 changes: 9 additions & 21 deletions p384/src/arithmetic/scalar.rs
View file
Expand Up @@ -17,7 +17,6 @@ use core::ops::{AddAssign, MulAssign, Neg, SubAssign};
use elliptic_curve::{
bigint::{ArrayEncoding, Encoding, Integer, Limb},
ff::PrimeField,
generic_array::arr,
ops::Reduce,
subtle::{
Choice, ConditionallySelectable, ConstantTimeEq, ConstantTimeGreater, ConstantTimeLess,
Expand All @@ -38,10 +37,11 @@ impl ScalarArithmetic for NistP384 {
#[cfg_attr(docsrs, doc(cfg(feature = "arithmetic")))]
pub struct Scalar(U384);

impl_sec1_field_element!(
impl_field_element!(
Scalar,
U384,
FieldBytes,
U384,
NistP384::ORDER,
fiat_p384_scalar_montgomery_domain_field_element,
fiat_p384_scalar_from_montgomery,
fiat_p384_scalar_to_montgomery,
Expand All @@ -53,11 +53,12 @@ impl_sec1_field_element!(
fiat_p384_scalar_divstep_precomp,
fiat_p384_scalar_divstep,
fiat_p384_scalar_msat,
NistP384::ORDER,
"000000000000000000000000000000000000000000000000389cb27e0bc8d220a7e5f24db74f58851313e695333ad68d"
);

impl Scalar {
/// `2^s` root of unity.
pub const ROOT_OF_UNITY: Self = Self::from_be_hex("ffffffffffffffffffffffffffffffffffffffffffffffffc7634d81f4372ddf581a0db248b0a77aecec196accc52972");

/// Compute modular square root.
pub fn sqrt(&self) -> CtOption<Self> {
// p mod 4 = 3 -> compute sqrt(x) using x^((p+1)/4) =
Expand Down Expand Up @@ -139,21 +140,15 @@ impl PrimeField for Scalar {
}

fn is_odd(&self) -> Choice {
self.to_canonical().is_odd()
self.is_odd()
}

fn multiplicative_generator() -> Self {
2u64.into()
}

fn root_of_unity() -> Self {
Scalar::from_repr(arr![u8;
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xc7, 0x63, 0x4d, 0x81, 0xf4, 0x37, 0x2d, 0xdf, 0x58, 0x1a, 0x0d, 0xb2,
0x48, 0xb0, 0xa7, 0x7a, 0xec, 0xec, 0x19, 0x6a, 0xcc, 0xc5, 0x29, 0x72
])
.unwrap()
Self::ROOT_OF_UNITY
}
}

Expand Down Expand Up @@ -249,17 +244,10 @@ impl TryFrom<U384> for Scalar {

#[cfg(test)]
mod tests {
use super::{fiat_p384_scalar_to_montgomery, Scalar, U384};
use super::Scalar;
use crate::FieldBytes;
use elliptic_curve::ff::{Field, PrimeField};

/// Test that the precomputed `Scalar::ONE` constant is correct.
#[test]
fn one() {
let one_mont = fiat_p384_scalar_to_montgomery(U384::ONE.as_ref());
assert_eq!(Scalar(one_mont.into()), Scalar::ONE);
}

#[test]
fn from_to_bytes_roundtrip() {
let k: u64 = 42;
Expand Down

0 comments on commit 0dbf939

Please sign in to comment.