Rework scalar de-serialization

Author: daxpedda

src/error.rs

@@ -34,8 +34,8 @@ pub enum Error {
     ProofVerificationError,
     /// Encountered insufficient bytes when attempting to deserialize
     SizeError,
-    /// Encountered a zero scalar
-    ZeroScalarError,
+    /// Encountered an invalid scalar
+    ScalarError,
 }
 
 #[cfg(feature = "std")]

src/group/mod.rs

@@ -78,23 +78,9 @@ pub trait Group {
     where
         <D as Add<U1>>::Output: ArrayLength<u8>;
 
-    /// Return a scalar from its fixed-length bytes representation, without
-    /// checking if the scalar is zero.
-    fn from_scalar_slice_unchecked(
-        scalar_bits: &GenericArray<u8, Self::ScalarLen>,
-    ) -> Result<Self::Scalar>;
-
     /// Return a scalar from its fixed-length bytes representation. If the
-    /// scalar is zero, then return an error.
-    fn from_scalar_slice<'a>(
-        scalar_bits: impl Into<&'a GenericArray<u8, Self::ScalarLen>>,
-    ) -> Result<Self::Scalar> {
-        let scalar = Self::from_scalar_slice_unchecked(scalar_bits.into())?;
-        if scalar.ct_eq(&Self::scalar_zero()).into() {
-            return Err(Error::ZeroScalarError);
-        }
-        Ok(scalar)
-    }
+    /// scalar is zero or invalid, then return an error.
+    fn deserialize_scalar(scalar_bits: &GenericArray<u8, Self::ScalarLen>) -> Result<Self::Scalar>;
 
     /// picks a scalar at random
     fn random_nonzero_scalar<R: RngCore + CryptoRng>(rng: &mut R) -> Self::Scalar;

src/group/p256.rs

@@ -29,7 +29,7 @@ use p256_::elliptic_curve::group::GroupEncoding;
 use p256_::elliptic_curve::ops::Reduce;
 use p256_::elliptic_curve::sec1::{FromEncodedPoint, ToEncodedPoint};
 use p256_::elliptic_curve::Field;
-use p256_::{AffinePoint, EncodedPoint, NistP256, ProjectivePoint, Scalar};
+use p256_::{AffinePoint, EncodedPoint, NistP256, ProjectivePoint, Scalar, SecretKey};
 use rand_core::{CryptoRng, RngCore};
 use subtle::{Choice, ConditionallySelectable};
 
@@ -144,10 +144,10 @@ impl Group for NistP256 {
         Ok(Scalar::from_be_bytes_reduced(result))
     }
 
-    fn from_scalar_slice_unchecked(
-        scalar_bits: &GenericArray<u8, Self::ScalarLen>,
-    ) -> Result<Self::Scalar> {
-        Ok(Scalar::from_be_bytes_reduced(*scalar_bits))
+    fn deserialize_scalar(scalar_bits: &GenericArray<u8, Self::ScalarLen>) -> Result<Self::Scalar> {
+        SecretKey::from_be_bytes(scalar_bits)
+            .map(|secret_key| *secret_key.to_nonzero_scalar())
+            .map_err(|_| Error::ScalarError)
     }
 
     fn random_nonzero_scalar<R: RngCore + CryptoRng>(rng: &mut R) -> Self::Scalar {

src/group/ristretto.rs

@@ -78,10 +78,10 @@ impl Group for Ristretto255 {
         ))
     }
 
-    fn from_scalar_slice_unchecked(
-        scalar_bits: &GenericArray<u8, Self::ScalarLen>,
-    ) -> Result<Self::Scalar> {
-        Ok(Scalar::from_bytes_mod_order(*scalar_bits.as_ref()))
+    fn deserialize_scalar(scalar_bits: &GenericArray<u8, Self::ScalarLen>) -> Result<Self::Scalar> {
+        Scalar::from_canonical_bytes((*scalar_bits).into())
+            .filter(|scalar| scalar != &Scalar::zero())
+            .ok_or(Error::ScalarError)
     }
 
     fn random_nonzero_scalar<R: RngCore + CryptoRng>(rng: &mut R) -> Self::Scalar {

src/group/tests.rs

@@ -45,8 +45,8 @@ fn test_identity_element_error<G: Group>() -> Result<()> {
 // Checks that the zero scalar cannot be deserialized
 fn test_zero_scalar_error<G: Group>() -> Result<()> {
     let zero_scalar = G::scalar_zero();
-    let result = G::from_scalar_slice(&G::scalar_as_bytes(zero_scalar));
-    assert!(matches!(result, Err(Error::ZeroScalarError)));
+    let result = G::deserialize_scalar(&G::scalar_as_bytes(zero_scalar));
+    assert!(matches!(result, Err(Error::ScalarError)));
 
     Ok(())
 }

src/serialization.rs

@@ -37,7 +37,7 @@ impl<G: Group, H: BlockSizeUser + Digest + FixedOutputReset> NonVerifiableClient
     pub fn deserialize(input: &[u8]) -> Result<Self> {
         let mut input = input.iter().copied();
 
-        let blind = G::from_scalar_slice(&deserialize(&mut input)?)?;
+        let blind = G::deserialize_scalar(&deserialize(&mut input)?)?;
 
         Ok(Self {
             blind,
@@ -60,7 +60,7 @@ impl<G: Group, H: BlockSizeUser + Digest + FixedOutputReset> VerifiableClient<G,
     pub fn deserialize(input: &[u8]) -> Result<Self> {
         let mut input = input.iter().copied();
 
-        let blind = G::from_scalar_slice(&deserialize(&mut input)?)?;
+        let blind = G::deserialize_scalar(&deserialize(&mut input)?)?;
         let blinded_element = G::from_element_slice(&deserialize(&mut input)?)?;
 
         Ok(Self {
@@ -81,7 +81,7 @@ impl<G: Group, H: BlockSizeUser + Digest + FixedOutputReset> NonVerifiableServer
     pub fn deserialize(input: &[u8]) -> Result<Self> {
         let mut input = input.iter().copied();
 
-        let sk = G::from_scalar_slice(&deserialize(&mut input)?)?;
+        let sk = G::deserialize_scalar(&deserialize(&mut input)?)?;
 
         Ok(Self {
             sk,
@@ -104,7 +104,7 @@ impl<G: Group, H: BlockSizeUser + Digest + FixedOutputReset> VerifiableServer<G,
     pub fn deserialize(input: &[u8]) -> Result<Self> {
         let mut input = input.iter().copied();
 
-        let sk = G::from_scalar_slice(&deserialize(&mut input)?)?;
+        let sk = G::deserialize_scalar(&deserialize(&mut input)?)?;
         let pk = G::from_element_slice(&deserialize(&mut input)?)?;
 
         Ok(Self {
@@ -129,8 +129,8 @@ impl<G: Group, H: BlockSizeUser + Digest + FixedOutputReset> Proof<G, H> {
     pub fn deserialize(input: &[u8]) -> Result<Self> {
         let mut input = input.iter().copied();
 
-        let c_scalar = G::from_scalar_slice(&deserialize(&mut input)?)?;
-        let s_scalar = G::from_scalar_slice(&deserialize(&mut input)?)?;
+        let c_scalar = G::deserialize_scalar(&deserialize(&mut input)?)?;
+        let s_scalar = G::deserialize_scalar(&deserialize(&mut input)?)?;
 
         Ok(Proof {
             c_scalar,

src/tests/voprf_test_vectors.rs

@@ -184,7 +184,7 @@ fn test_base_blind<G: Group, H: BlockSizeUser + Digest + FixedOutputReset>(
     for parameters in tvs {
         for i in 0..parameters.input.len() {
             let blind =
-                G::from_scalar_slice(&GenericArray::clone_from_slice(&parameters.blind[i]))?;
+                G::deserialize_scalar(&GenericArray::clone_from_slice(&parameters.blind[i]))?;
             let client_result = NonVerifiableClient::<G, H>::deterministic_blind_unchecked(
                 &parameters.input[i],
                 blind,
@@ -210,7 +210,7 @@ fn test_verifiable_blind<G: Group, H: BlockSizeUser + Digest + FixedOutputReset>
     for parameters in tvs {
         for i in 0..parameters.input.len() {
             let blind =
-                G::from_scalar_slice(&GenericArray::clone_from_slice(&parameters.blind[i]))?;
+                G::deserialize_scalar(&GenericArray::clone_from_slice(&parameters.blind[i]))?;
             let client_blind_result = VerifiableClient::<G, H>::deterministic_blind_unchecked(
                 &parameters.input[i],
                 blind,
@@ -299,7 +299,7 @@ fn test_base_finalize<G: Group, H: BlockSizeUser + Digest + FixedOutputReset>(
 ) -> Result<()> {
     for parameters in tvs {
         for i in 0..parameters.input.len() {
-            let client = NonVerifiableClient::<G, H>::from_blind(G::from_scalar_slice(
+            let client = NonVerifiableClient::<G, H>::from_blind(G::deserialize_scalar(
                 &GenericArray::clone_from_slice(&parameters.blind[i]),
             )?);
 
@@ -322,7 +322,7 @@ fn test_verifiable_finalize<G: Group, H: BlockSizeUser + Digest + FixedOutputRes
         let mut clients = vec![];
         for i in 0..parameters.input.len() {
             let client = VerifiableClient::<G, H>::from_blind_and_element(
-                G::from_scalar_slice(&GenericArray::clone_from_slice(&parameters.blind[i]))?,
+                G::deserialize_scalar(&GenericArray::clone_from_slice(&parameters.blind[i]))?,
                 G::from_element_slice(&GenericArray::clone_from_slice(
                     &parameters.blinded_element[i],
                 ))?,

src/voprf.rs

@@ -414,7 +414,7 @@ impl<G: Group, H: BlockSizeUser + Digest + FixedOutputReset> NonVerifiableServer
     /// Produces a new instance of a [NonVerifiableServer] using a supplied set
     /// of bytes to represent the server's private key
     pub fn new_with_key(private_key_bytes: &[u8]) -> Result<Self> {
-        let sk = G::from_scalar_slice(private_key_bytes)?;
+        let sk = G::deserialize_scalar(private_key_bytes.into())?;
         Ok(Self {
             sk,
             hash: PhantomData,
@@ -480,7 +480,7 @@ impl<G: Group, H: BlockSizeUser + Digest + FixedOutputReset> VerifiableServer<G,
     /// Produces a new instance of a [VerifiableServer] using a supplied set of
     /// bytes to represent the server's private key
     pub fn new_with_key(key: &[u8]) -> Result<Self> {
-        let sk = G::from_scalar_slice(key)?;
+        let sk = G::deserialize_scalar(key.into())?;
         let pk = G::base_point() * &sk;
         Ok(Self {
             sk,