diff --git a/fastcrypto/src/groups/multiplier/integer_utils.rs b/fastcrypto/src/groups/multiplier/integer_utils.rs index 34efd35317..71504d93f0 100644 --- a/fastcrypto/src/groups/multiplier/integer_utils.rs +++ b/fastcrypto/src/groups/multiplier/integer_utils.rs @@ -27,15 +27,26 @@ pub fn compute_base_2w_expansion( } /// Get the integer represented by a given range of bits of a byte from start to end (exclusive). +/// Both the start and end parameter may be greater than 8, in which case the remaining bits of the +/// byte will be assumed to be zero. #[inline] fn get_lendian_from_substring(byte: &u8, start: usize, end: usize) -> u8 { assert!(start <= end); + if start > 7 { + return 0; + } else if end > 8 { + return get_lendian_from_substring(byte, start, 8); + } byte >> start & ((1 << (end - start)) - 1) as u8 } /// Compute ceil(numerator / denominator). pub(crate) fn div_ceil(numerator: usize, denominator: usize) -> usize { - (numerator + denominator - 1) / denominator + assert!(denominator > 0); + if numerator == 0 { + return 0; + } + 1 + ((numerator - 1) / denominator) } /// Get the integer represented by a given range of bits of a an integer represented by a little-endian @@ -91,6 +102,14 @@ pub const fn log2(x: usize) -> usize { (usize::BITS - x.leading_zeros() - 1) as usize } +/// Return true iff the given number is a power of 2. +pub fn is_power_of_2(x: usize) -> bool { + if x == 0 { + return false; + } + x & (x - 1) == 0 +} + #[cfg(test)] mod tests { use super::*; @@ -118,6 +137,9 @@ mod tests { assert_eq!(129, get_lendian_from_substring(&byte, 0, 8)); assert_eq!(64, get_lendian_from_substring(&byte, 1, 8)); assert_eq!(16, get_lendian_from_substring(&byte, 3, 8)); + assert_eq!(129, get_lendian_from_substring(&byte, 0, 100)); + assert_eq!(1, get_lendian_from_substring(&byte, 7, 8)); + assert_eq!(0, get_lendian_from_substring(&byte, 8, 8)); } #[test] @@ -153,4 +175,17 @@ mod tests { assert_eq!(0, get_bits_from_bytes(&bytes, 17, 23)); assert_eq!(1, get_bits_from_bytes(&bytes, 23, 100)); } + + #[test] + fn test_is_power_of_two() { + assert!(!is_power_of_2(0)); + assert!(is_power_of_2(1)); + assert!(is_power_of_2(2)); + assert!(!is_power_of_2(3)); + assert!(is_power_of_2(4)); + assert!(!is_power_of_2(511)); + assert!(is_power_of_2(512)); + assert!(!is_power_of_2(513)); + assert!(is_power_of_2(4096)); + } } diff --git a/fastcrypto/src/groups/multiplier/windowed.rs b/fastcrypto/src/groups/multiplier/windowed.rs index 20e1df4b68..006f577b71 100644 --- a/fastcrypto/src/groups/multiplier/windowed.rs +++ b/fastcrypto/src/groups/multiplier/windowed.rs @@ -4,7 +4,7 @@ use std::collections::HashMap; use std::iter::successors; -use crate::groups::multiplier::integer_utils::{get_bits_from_bytes, test_bit}; +use crate::groups::multiplier::integer_utils::{get_bits_from_bytes, is_power_of_2, test_bit}; use crate::groups::multiplier::{integer_utils, ScalarMultiplier}; use crate::groups::GroupElement; use crate::serde_helpers::ToFromByteArray; @@ -14,7 +14,7 @@ use crate::serde_helpers::ToFromByteArray; /// `double_mul`, is NOT constant time. However, the single multiplication method `mul` is constant /// time if the group operations for `G` are constant time. /// -/// The `CACHE_SIZE` should be a power of two. The `SCALAR_SIZE` is the number of bytes in the byte +/// The `CACHE_SIZE` should be a power of two > 1. The `SCALAR_SIZE` is the number of bytes in the byte /// representation of the scalar type `S`, and we assume that the `S::to_byte_array` method returns /// the scalar in little-endian format. /// @@ -54,6 +54,9 @@ impl< for WindowedScalarMultiplier { fn new(base_element: G) -> Self { + if !is_power_of_2(CACHE_SIZE) || CACHE_SIZE <= 1 { + panic!("CACHE_SIZE must be a power of two greater than 1"); + } let mut cache = [G::zero(); CACHE_SIZE]; cache[1] = base_element; for i in 2..CACHE_SIZE { @@ -212,8 +215,8 @@ pub fn multi_scalar_mul< /// Compute multiples 2w-1 base_element, (2w-1 + 1) base_element, ..., (2w - 1) base_element. fn compute_multiples(base_element: &G, window_size: usize) -> Vec { assert!(window_size > 0, "Window size must be strictly positive."); - let mut smallest_multiple = base_element.double(); - for _ in 2..window_size { + let mut smallest_multiple = *base_element; + for _ in 1..window_size { smallest_multiple = smallest_multiple.double(); } successors(Some(smallest_multiple), |g| Some(*g + base_element)) @@ -278,7 +281,7 @@ mod tests { for scalar in scalars { let expected = ProjectivePoint::generator() * scalar; - let multiplier = WindowedScalarMultiplier::::new( + let multiplier = WindowedScalarMultiplier::::new( ProjectivePoint::generator(), ); let actual = multiplier.mul(&scalar); @@ -290,12 +293,6 @@ mod tests { let actual = multiplier.mul(&scalar); assert_eq!(expected, actual); - let multiplier = WindowedScalarMultiplier::::new( - ProjectivePoint::generator(), - ); - let actual = multiplier.mul(&scalar); - assert_eq!(expected, actual); - let multiplier = WindowedScalarMultiplier::::new( ProjectivePoint::generator(), ); diff --git a/fastcrypto/src/groups/secp256r1.rs b/fastcrypto/src/groups/secp256r1.rs index d9bb463ddc..64043d8142 100644 --- a/fastcrypto/src/groups/secp256r1.rs +++ b/fastcrypto/src/groups/secp256r1.rs @@ -9,9 +9,9 @@ use crate::groups::{GroupElement, Scalar as ScalarTrait}; use crate::serde_helpers::ToFromByteArray; use crate::traits::AllowedRng; use ark_ec::Group; -use ark_ff::{Field, One, PrimeField, Zero}; +use ark_ff::{Field, One, UniformRand, Zero}; use ark_secp256r1::{Fr, Projective}; -use ark_serialize::CanonicalSerialize; +use ark_serialize::{CanonicalDeserialize, CanonicalSerialize}; use derive_more::{Add, From, Neg, Sub}; use fastcrypto_derive::GroupOpsExtend; use std::ops::{Div, Mul}; @@ -96,9 +96,7 @@ impl From for Scalar { impl ScalarTrait for Scalar { fn rand(rng: &mut R) -> Self { - let mut bytes = [0u8; SCALAR_SIZE_IN_BYTES]; - rng.fill_bytes(&mut bytes); - Scalar(Fr::from_be_bytes_mod_order(&bytes)) + Scalar(Fr::rand(rng)) } fn inverse(&self) -> FastCryptoResult { @@ -110,7 +108,10 @@ impl ScalarTrait for Scalar { impl ToFromByteArray for Scalar { fn from_byte_array(bytes: &[u8; SCALAR_SIZE_IN_BYTES]) -> Result { - Ok(Scalar(Fr::from_le_bytes_mod_order(bytes))) + Ok(Scalar( + Fr::deserialize_uncompressed(bytes.as_slice()) + .map_err(|_| FastCryptoError::InvalidInput)?, + )) } fn to_byte_array(&self) -> [u8; SCALAR_SIZE_IN_BYTES] { diff --git a/fastcrypto/src/secp256r1/conversion.rs b/fastcrypto/src/secp256r1/conversion.rs index dfd7856888..f88fb97c9d 100644 --- a/fastcrypto/src/secp256r1/conversion.rs +++ b/fastcrypto/src/secp256r1/conversion.rs @@ -29,23 +29,26 @@ pub(crate) fn fr_arkworks_to_p256(scalar: &ark_secp256r1::Fr) -> p256::Scalar { } /// Convert an arkworks field element to a p256 field element. -pub(crate) fn fq_arkworks_to_p256(scalar: &ark_secp256r1::Fq) -> p256::Scalar { +pub(crate) fn fq_arkworks_to_p256(scalar: &ark_secp256r1::Fq) -> FieldBytes { // This implementation is taken from bls_fq_to_blst_fp in fastcrypto-zkp. let mut bytes = [0u8; 32]; scalar.serialize_uncompressed(&mut bytes[..]).unwrap(); - p256::Scalar::from_uint_unchecked(U256::from_le_byte_array(FieldBytes::from(bytes))) + bytes.reverse(); + p256::FieldBytes::from(bytes) } /// Convert an p256 affine point to an arkworks affine point. -pub(crate) fn affine_pt_p256_to_arkworks(point: &p256::AffinePoint) -> ark_secp256r1::Affine { +pub(crate) fn affine_pt_p256_to_projective_arkworks( + point: &p256::AffinePoint, +) -> ark_secp256r1::Projective { if point.is_identity().into() { - return ark_secp256r1::Affine::zero(); + return ark_secp256r1::Projective::zero(); } let encoded_point = point.to_encoded_point(false); - ark_secp256r1::Affine::new_unchecked( + ark_secp256r1::Projective::from(ark_secp256r1::Affine::new_unchecked( ark_secp256r1::Fq::from_be_bytes_mod_order(encoded_point.x().unwrap()), ark_secp256r1::Fq::from_be_bytes_mod_order(encoded_point.y().unwrap()), - ) + )) } /// Reduce a big-endian integer representation modulo the subgroup order in arkworks representation. @@ -53,11 +56,13 @@ pub(crate) fn reduce_bytes(bytes: &[u8; 32]) -> ark_secp256r1::Fr { ark_secp256r1::Fr::from_be_bytes_mod_order(bytes) } -/// Reduce an arkworks field element (modulo field size) to a scalar (modulo subgroup order) -pub(crate) fn arkworks_fq_to_fr(scalar: &ark_secp256r1::Fq) -> ark_secp256r1::Fr { +/// Reduce an arkworks field element (modulo field size) to a scalar (modulo subgroup order). This also +/// returns a boolean indicating whether a modular reduction was performed. +pub(crate) fn arkworks_fq_to_fr(scalar: &ark_secp256r1::Fq) -> (ark_secp256r1::Fr, bool) { let mut bytes = [0u8; 32]; scalar.serialize_uncompressed(&mut bytes[..]).unwrap(); - ark_secp256r1::Fr::from_le_bytes_mod_order(&bytes) + let output = ark_secp256r1::Fr::from_le_bytes_mod_order(&bytes); + (output, output.into_bigint() != scalar.into_bigint()) } /// Converts an arkworks affine point to a p256 affine point. @@ -66,8 +71,8 @@ pub(crate) fn affine_pt_arkworks_to_p256(point: &ark_secp256r1::Affine) -> p256: return p256::AffinePoint::IDENTITY; } let encoded_point = p256::EncodedPoint::from_affine_coordinates( - &fq_arkworks_to_p256(point.x().expect("The point should not be zero")).to_bytes(), - &fq_arkworks_to_p256(point.y().expect("The point should not be zero")).to_bytes(), + &fq_arkworks_to_p256(point.x().expect("The point should not be zero")), + &fq_arkworks_to_p256(point.y().expect("The point should not be zero")), false, ); p256::AffinePoint::from_encoded_point(&encoded_point).unwrap() @@ -147,46 +152,48 @@ mod tests { fn test_pt_p256_to_arkworks() { // 0 assert_eq!( - ark_secp256r1::Affine::zero(), - affine_pt_p256_to_arkworks(&p256::AffinePoint::IDENTITY) + ark_secp256r1::Projective::zero(), + affine_pt_p256_to_projective_arkworks(&p256::AffinePoint::IDENTITY) ); // G assert_eq!( - ark_secp256r1::Affine::generator() * ark_secp256r1::Fr::from(7u32), - affine_pt_p256_to_arkworks( + ark_secp256r1::Projective::generator() * ark_secp256r1::Fr::from(7u32), + affine_pt_p256_to_projective_arkworks( &(p256::AffinePoint::generator() * p256::Scalar::from(7u32)).to_affine() ) ); // 7G assert_eq!( - ark_secp256r1::Affine::generator(), - affine_pt_p256_to_arkworks(&p256::AffinePoint::generator()) + ark_secp256r1::Projective::generator(), + affine_pt_p256_to_projective_arkworks(&p256::AffinePoint::generator()) ); // sG, random s let random_s = p256::Scalar::random(&mut rand::thread_rng()); assert_eq!( - (ark_secp256r1::Affine::generator() * fr_p256_to_arkworks(&random_s)).into_affine(), - affine_pt_p256_to_arkworks(&(p256::AffinePoint::generator() * random_s).to_affine()) + (ark_secp256r1::Projective::generator() * fr_p256_to_arkworks(&random_s)).into_affine(), + affine_pt_p256_to_projective_arkworks( + &(p256::AffinePoint::generator() * random_s).to_affine() + ) ); } #[test] fn test_arkworks_fq_to_fr() { let s = ark_secp256r1::Fq::rand(&mut rand::thread_rng()); - let s_fr = arkworks_fq_to_fr(&s); + let s_fr = arkworks_fq_to_fr(&s).0; let p256_s = fq_arkworks_to_p256(&s); - let reduced_s = p256::Scalar::reduce_bytes(&p256_s.to_bytes()); + let reduced_s = p256::Scalar::reduce_bytes(&p256_s); assert_eq!(fr_arkworks_to_p256(&s_fr), reduced_s); - assert_eq!(reduce_bytes(&p256_s.to_bytes().try_into().unwrap()), s_fr); + assert_eq!(reduce_bytes(&p256_s.try_into().unwrap()), s_fr); } #[test] fn test_fq_arkworks_to_p256() { let arkworks_seven = ark_secp256r1::Fq::from(7u32); - let p256_seven = p256::Scalar::from(7u32); + let p256_seven = p256::FieldBytes::from(p256::Scalar::from(7u32)); let actual_p256_seven = fq_arkworks_to_p256(&arkworks_seven); assert_eq!(actual_p256_seven, p256_seven); diff --git a/fastcrypto/src/secp256r1/mod.rs b/fastcrypto/src/secp256r1/mod.rs index 759aeb76dd..94b3be5ba7 100644 --- a/fastcrypto/src/secp256r1/mod.rs +++ b/fastcrypto/src/secp256r1/mod.rs @@ -27,9 +27,8 @@ use crate::{ serialize_deserialize_with_to_from_bytes, }; use ark_ec::{AffineRepr, CurveGroup}; -use ark_ff::Field; -use ark_secp256r1::Projective; -use elliptic_curve::{Curve, FieldBytesEncoding, PrimeField}; +use ark_ff::{BigInteger, Field, PrimeField}; +use elliptic_curve::{Curve, FieldBytesEncoding, PrimeField as OtherPrimeField}; use lazy_static::lazy_static; use once_cell::sync::OnceCell; use p256::ecdsa::{ @@ -38,7 +37,7 @@ use p256::ecdsa::{ }; use p256::elliptic_curve::group::GroupEncoding; use p256::elliptic_curve::scalar::IsHigh; -use p256::{FieldBytes, NistP256, Scalar}; +use p256::{NistP256, Scalar}; use std::fmt::{self, Debug}; use std::str::FromStr; use zeroize::Zeroize; @@ -51,8 +50,8 @@ use crate::groups::secp256r1; use crate::groups::secp256r1::{ProjectivePoint, SCALAR_SIZE_IN_BYTES}; use crate::hash::{HashFunction, Sha256}; use crate::secp256r1::conversion::{ - affine_pt_p256_to_arkworks, arkworks_fq_to_fr, fr_arkworks_to_p256, fr_p256_to_arkworks, - get_affine_x_coordinate, reduce_bytes, + affine_pt_p256_to_projective_arkworks, arkworks_fq_to_fr, fr_arkworks_to_p256, + fr_p256_to_arkworks, get_affine_x_coordinate, reduce_bytes, }; use crate::secp256r1::recoverable::Secp256r1RecoverableSignature; use crate::traits::Signer; @@ -119,13 +118,13 @@ impl std::hash::Hash for Secp256r1PublicKey { impl PartialOrd for Secp256r1PublicKey { fn partial_cmp(&self, other: &Self) -> Option { - self.as_ref().partial_cmp(other.as_ref()) + self.pubkey.partial_cmp(&other.pubkey) } } impl Ord for Secp256r1PublicKey { fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.as_ref().cmp(other.as_ref()) + self.pubkey.cmp(&other.pubkey) } } @@ -150,13 +149,13 @@ impl VerifyingKey for Secp256r1PublicKey { lazy_static! { static ref MULTIPLIER: WindowedScalarMultiplier< ProjectivePoint, - crate::groups::secp256r1::Scalar, + ::ScalarType, PRECOMPUTED_POINTS, SCALAR_SIZE_IN_BYTES, SLIDING_WINDOW_WIDTH, > = WindowedScalarMultiplier::< ProjectivePoint, - crate::groups::secp256r1::Scalar, + ::ScalarType, PRECOMPUTED_POINTS, SCALAR_SIZE_IN_BYTES, SLIDING_WINDOW_WIDTH, @@ -193,7 +192,7 @@ impl Secp256r1PublicKey { // Convert scalars to arkworks representation let r = fr_p256_to_arkworks(&r); let s = fr_p256_to_arkworks(&s); - let q = affine_pt_p256_to_arkworks(self.pubkey.as_affine()); + let q = affine_pt_p256_to_projective_arkworks(self.pubkey.as_affine()); // Compute inverse of s. This fails if s is zero which is checked in deserialization and in // split_scalars above, but we avoid an unwrap here to be safe. @@ -207,16 +206,17 @@ impl Secp256r1PublicKey { let p = MULTIPLIER .two_scalar_mul( &secp256r1::Scalar(u1), - &ProjectivePoint(Projective::from(q)), + &ProjectivePoint(q), &secp256r1::Scalar(u2), ) .0; - let x = get_affine_x_coordinate(&p); - // Note that x is none if and only if p is zero, in which case the signature is invalid. See // step 5 in section 4.1.4 in "SEC 1: Elliptic Curve Cryptography". - if x.is_some() && arkworks_fq_to_fr(&x.unwrap()) == r { + let x = get_affine_x_coordinate(&p).ok_or(FastCryptoError::InvalidSignature)?; + + let (x_reduced, _) = arkworks_fq_to_fr(&x); + if x_reduced == r { return Ok(()); } Err(FastCryptoError::InvalidSignature) @@ -299,14 +299,16 @@ impl AsRef<[u8]> for Secp256r1PrivateKey { impl zeroize::Zeroize for Secp256r1PrivateKey { fn zeroize(&mut self) { + // Unwrap is safe here because we are using a constant and it has been tested + // (see fastcrypto/src/tests/secp256r1_tests::test_sk_zeroization_on_drop) + self.privkey = ExternalSecretKey::from_bytes(&Scalar::ONE.to_bytes()).unwrap(); self.bytes.take().zeroize(); - // SigningKey from the p256 crate implements zeroize on drop, so we do not need to zeroize it here. } } impl Drop for Secp256r1PrivateKey { fn drop(&mut self) { - self.bytes.take().zeroize(); + self.zeroize(); } } @@ -384,9 +386,9 @@ pub struct Secp256r1KeyPair { impl Secp256r1KeyPair { /// Sign a message using the given hash function and return the signature and the elliptic curve /// point R = kG where k is the ephemeral nonce generated according to RFC6979. - fn sign_common>(&self, msg: &[u8]) -> (Signature, ark_secp256r1::Affine) { + fn sign_common>(&self, msg: &[u8]) -> (Signature, bool, bool) { // Hash message - let z = H::digest(msg).digest; + let z = reduce_bytes(&H::digest(msg).digest); // Private key as scalar let x = self.secret.privkey.as_nonzero_scalar(); @@ -397,7 +399,7 @@ impl Secp256r1KeyPair { &Scalar::from_repr(rfc6979::generate_k::( &x.to_bytes(), &NistP256::ORDER.encode_field_bytes(), - &FieldBytes::from(z), + &fr_arkworks_to_p256(&z).to_bytes(), &[], )) .unwrap(), @@ -405,7 +407,6 @@ impl Secp256r1KeyPair { // Convert secret key and message to arkworks scalars. let x = fr_p256_to_arkworks(x); - let z = reduce_bytes(&z); // Compute scalar inversion of k let k_inv = k.inverse().expect("k should not be zero"); @@ -414,10 +415,10 @@ impl Secp256r1KeyPair { let big_r = MULTIPLIER.mul(&secp256r1::Scalar(k)).0.into_affine(); // Lift x-coordinate of R and reduce it into an element of the scalar field - let r = arkworks_fq_to_fr(big_r.x().expect("R should not be zero")); + let (r, is_x_reduced) = arkworks_fq_to_fr(big_r.x().expect("R should not be zero")); // Compute s as a signature over r and z. - let s = k_inv * (z + (r * x)); + let s = k_inv * (z + r * x); // Convert to p256 format let s = fr_arkworks_to_p256(&s).to_bytes(); @@ -426,18 +427,23 @@ impl Secp256r1KeyPair { // This can only fail if either 𝒓 or 𝒔 are zero (see ecdsa-0.15.0/src/lib.rs) which is negligible. let signature = Signature::from_scalars(r, s).expect("r or s is zero"); - (signature, big_r) + // The parity of the y coordinate is needed for computing the recovery id. + let is_r_odd = big_r.y().expect("R is zero").into_bigint().is_odd(); + let is_s_high: bool = signature.s().is_high().into(); + let is_y_odd = is_r_odd ^ is_s_high; + + // Normalize signature + let normalized_signature = signature.normalize_s().unwrap_or(signature); + + (normalized_signature, is_y_odd, is_x_reduced) } /// Create a new signature using the given hash function to hash the message. pub fn sign_with_hash>(&self, msg: &[u8]) -> Secp256r1Signature { - let (signature, _) = self.sign_common::(msg); - - // Normalize signature - let normalized_signature = signature.normalize_s().unwrap_or(signature); + let (signature, _, _) = self.sign_common::(msg); Secp256r1Signature { - sig: normalized_signature, + sig: signature, bytes: OnceCell::new(), } } diff --git a/fastcrypto/src/secp256r1/recoverable.rs b/fastcrypto/src/secp256r1/recoverable.rs index 40a6be0ed6..8b0c642b23 100644 --- a/fastcrypto/src/secp256r1/recoverable.rs +++ b/fastcrypto/src/secp256r1/recoverable.rs @@ -23,8 +23,8 @@ use crate::groups::secp256r1; use crate::groups::secp256r1::ProjectivePoint; use crate::hash::HashFunction; use crate::secp256r1::conversion::{ - affine_pt_arkworks_to_p256, affine_pt_p256_to_arkworks, fq_arkworks_to_p256, - fr_p256_to_arkworks, reduce_bytes, + affine_pt_arkworks_to_p256, affine_pt_p256_to_projective_arkworks, fr_p256_to_arkworks, + reduce_bytes, }; use crate::secp256r1::{ DefaultHash, Secp256r1KeyPair, Secp256r1PublicKey, Secp256r1Signature, MULTIPLIER, @@ -37,10 +37,8 @@ use crate::{ traits::{EncodeDecodeBase64, ToFromBytes}, }; use crate::{impl_base64_display_fmt, serialize_deserialize_with_to_from_bytes}; -use ark_ec::{AffineRepr, CurveGroup}; +use ark_ec::CurveGroup; use ark_ff::Field; -use ark_secp256r1::Projective; -use ecdsa::elliptic_curve::scalar::IsHigh; use ecdsa::elliptic_curve::subtle::Choice; use ecdsa::RecoveryId; use once_cell::sync::OnceCell; @@ -75,11 +73,16 @@ impl ToFromBytes for Secp256r1RecoverableSignature { )); } + let recovery_id = bytes[SECP256R1_RECOVERABLE_SIGNATURE_LENGTH - 1]; + if recovery_id > 3 { + return Err(FastCryptoError::InvalidInput); + } + // This fails if either r or s are zero: https://docs.rs/ecdsa/0.16.6/src/ecdsa/lib.rs.html#209-219. ExternalSignature::try_from(&bytes[..SECP256R1_RECOVERABLE_SIGNATURE_LENGTH - 1]) .map(|sig| Secp256r1RecoverableSignature { sig, - recovery_id: bytes[SECP256R1_RECOVERABLE_SIGNATURE_LENGTH - 1], + recovery_id, bytes: OnceCell::new(), }) .map_err(|_| FastCryptoError::InvalidInput) @@ -164,18 +167,13 @@ impl RecoverableSigner for Secp256r1KeyPair { &self, msg: &[u8], ) -> Secp256r1RecoverableSignature { - let (signature, big_r) = self.sign_common::(msg); + let (signature, is_y_odd, is_x_reduced) = self.sign_common::(msg); - // Compute recovery id and normalize signature - let y = fq_arkworks_to_p256(big_r.y().expect("R is zero")); - let is_r_odd = y.is_odd(); - let is_s_high = signature.s().is_high(); - let is_y_odd = is_r_odd ^ is_s_high; - let normalized_signature = signature.normalize_s().unwrap_or(signature); - let recovery_id = RecoveryId::new(is_y_odd.into(), false); + // Compute recovery id + let recovery_id = RecoveryId::new(is_y_odd, is_x_reduced); Secp256r1RecoverableSignature { - sig: normalized_signature, + sig: signature, bytes: OnceCell::new(), recovery_id: recovery_id.to_byte(), } @@ -217,7 +215,7 @@ impl RecoverableSignature for Secp256r1RecoverableSignature { let r = fr_p256_to_arkworks(&r); let s = fr_p256_to_arkworks(&s); let z = reduce_bytes(&H::digest(msg).digest); - let big_r = affine_pt_p256_to_arkworks(&big_r.unwrap()); + let big_r = affine_pt_p256_to_projective_arkworks(&big_r.unwrap()); // Compute inverse of r. This fails if r is zero which is checked in deserialization and in // split_scalars called above, but we avoid an unwrap here to be safe. @@ -230,7 +228,7 @@ impl RecoverableSignature for Secp256r1RecoverableSignature { let pk = MULTIPLIER .two_scalar_mul( &secp256r1::Scalar(u1), - &ProjectivePoint(Projective::from(big_r)), + &ProjectivePoint(big_r), &secp256r1::Scalar(u2), ) .0;