diff --git a/src/group/p256.rs b/src/group/p256.rs index 9a26bc6..8d03b30 100644 --- a/src/group/p256.rs +++ b/src/group/p256.rs @@ -3,6 +3,9 @@ // This source code is licensed under the MIT license found in the // LICENSE file in the root directory of this source tree. +// Note: This group implementation of p256 is experimental for now, +// until hash-to-curve or crypto-bigint are fully supported. + #![allow( clippy::borrow_interior_mutable_const, clippy::declare_interior_mutable_const @@ -11,7 +14,7 @@ use super::Group; use crate::errors::{InternalError, ProtocolError}; use crate::hash::Hash; -use core::ops::{Add, Div, Mul, Neg, Sub}; +use core::ops::{Add, Div, Mul, Neg}; use core::str::FromStr; use generic_array::typenum::{U32, U33}; use generic_array::{ArrayLength, GenericArray}; @@ -26,6 +29,7 @@ use p256_::elliptic_curve::subtle::ConstantTimeEq; use p256_::elliptic_curve::Field; use p256_::{AffinePoint, EncodedPoint, ProjectivePoint}; use rand::{CryptoRng, RngCore}; +use subtle::{Choice, ConditionallySelectable}; // `L: 48` pub const L: usize = 48; @@ -160,6 +164,10 @@ impl Group for ProjectivePoint { /// Corresponds to the map_to_curve_simple_swu() function defined in /// +// `cmov`, `mod_floor` and `modpow` needs to be made constant-time, which +// will be supported after crypto-bigint is no longer experimental. See +// https://github.com/novifinancial/opaque-ke/issues/239 for more context. + #[allow(clippy::many_single_char_names)] fn map_to_curve_simple_swu>( u: &[u8], @@ -186,11 +194,6 @@ fn map_to_curve_simple_swu>( fn one(&'a self) -> FieldElement<'a> { self.element(&BigInt::one()) } - - /// See - fn inv0(&'a self, number: &FieldElement<'a>) -> FieldElement<'a> { - number.pow_internal(&(self.0 - 2)) - } } /// Finite field arithmetic @@ -216,14 +219,6 @@ fn map_to_curve_simple_swu>( } } - impl<'a> Sub for &FieldElement<'a> { - type Output = FieldElement<'a>; - - fn sub(self, rhs: Self) -> Self::Output { - self.f.element(&(&self.number - &rhs.number)) - } - } - impl<'a> Neg for FieldElement<'a> { type Output = FieldElement<'a>; @@ -277,7 +272,7 @@ fn map_to_curve_simple_swu>( #[allow(clippy::suspicious_arithmetic_impl)] fn div(self, rhs: &Self) -> Self::Output { - self * rhs.f.inv0(rhs) + self * rhs.inv0() } } @@ -309,6 +304,11 @@ fn map_to_curve_simple_swu>( (&self.number % 2_usize).to_i32().unwrap() } + /// See + fn inv0(&self) -> Self { + self.pow_internal(&(self.f.0 - 2)) + } + fn is_zero(&self) -> bool { self.number.is_zero() } @@ -332,10 +332,27 @@ fn map_to_curve_simple_swu>( } fn cmov<'a>(x: &FieldElement<'a>, y: &FieldElement<'a>, b: bool) -> FieldElement<'a> { - if b { - y.clone() - } else { - x.clone() + let f = x.f; + + let x_bytes = x.number.to_bytes_le().1; + let mut x = [0; 32]; + x[..x_bytes.len()].copy_from_slice(&x_bytes); + + let y_bytes = y.number.to_bytes_le().1; + let mut y = [0; 32]; + y[..y_bytes.len()].copy_from_slice(&y_bytes); + + let mut bytes = [0; 32]; + + let choice = Choice::from(u8::from(b)); + + for ((byte, x), y) in bytes.iter_mut().zip(&x).zip(&y) { + *byte = u8::conditional_select(x, y, choice); + } + + FieldElement { + f, + number: BigInt::from_bytes_le(Sign::Plus, &bytes), } } @@ -359,7 +376,7 @@ fn map_to_curve_simple_swu>( // 3. x1 = tv1 + tv2 let mut x1 = &tv1 + &tv2; // 4. x1 = inv0(x1) - x1 = f.inv0(&x1); + x1 = x1.inv0(); // 5. e1 = x1 == 0 let e1 = x1.is_zero(); // 6. x1 = x1 + 1 diff --git a/src/lib.rs b/src/lib.rs index 28c55ef..e926621 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -819,7 +819,8 @@ //! [curve25519-dalek](https://doc.dalek.rs/curve25519_dalek/index.html#backends-and-features) and allow for selecting //! the corresponding backend for the curve arithmetic used. The `u64_backend` feature is included as the default. //! -//! - The `p256` feature enables the use of `p256::ProjectivePoint` as a `Group` for `CipherSuite`. +//! - The `p256` feature enables the use of `p256::ProjectivePoint` as a `Group` for `CipherSuite`. Note that this +//! is currently an experimental feature ⚠️, and is not yet ready for production use. //! //! - The `bench` feature is used only for running performance benchmarks for this implementation. //!