Rework element de-serialization

Author: daxpedda

src/group/mod.rs

@@ -26,7 +26,7 @@ pub use ristretto::Ristretto255;
 use subtle::ConstantTimeEq;
 use zeroize::Zeroize;
 
-use crate::{Error, Result};
+use crate::Result;
 
 /// A prime-order subgroup of a base field (EC, prime-order field ...). This
 /// subgroup is noted additively — as in the draft RFC — in this trait.
@@ -91,27 +91,9 @@ pub trait Group {
     /// The multiplicative inverse of this scalar
     fn invert_scalar(scalar: Self::Scalar) -> Self::Scalar;
 
-    /// Return an element from its fixed-length bytes representation. This is
-    /// the unchecked version, which does not check for deserializing the
-    /// identity element
-    fn from_element_slice_unchecked(
-        element_bits: &GenericArray<u8, Self::ElemLen>,
-    ) -> Result<Self::Elem>;
-
     /// Return an element from its fixed-length bytes representation. If the
     /// element is the identity element, return an error.
-    fn from_element_slice<'a>(
-        element_bits: impl Into<&'a GenericArray<u8, Self::ElemLen>>,
-    ) -> Result<Self::Elem> {
-        let elem = Self::from_element_slice_unchecked(element_bits.into())?;
-
-        if Self::Elem::ct_eq(&elem, &Self::identity()).into() {
-            // found the identity element
-            return Err(Error::PointError);
-        }
-
-        Ok(elem)
-    }
+    fn deserialize_element(element_bits: &GenericArray<u8, Self::ElemLen>) -> Result<Self::Elem>;
 
     /// Serializes the `self` group element
     fn to_arr(elem: Self::Elem) -> GenericArray<u8, Self::ElemLen>;

src/group/p256.rs

@@ -25,11 +25,10 @@ use num_integer::Integer;
 use num_traits::{One, ToPrimitive, Zero};
 use once_cell::unsync::Lazy;
 use p256_::elliptic_curve::group::prime::PrimeCurveAffine;
-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, SecretKey};
+use p256_::{AffinePoint, EncodedPoint, NistP256, ProjectivePoint, PublicKey, Scalar, SecretKey};
 use rand_core::{CryptoRng, RngCore};
 use subtle::{Choice, ConditionallySelectable};
 
@@ -162,10 +161,10 @@ impl Group for NistP256 {
         Option::from(scalar.invert()).unwrap()
     }
 
-    fn from_element_slice_unchecked(
-        element_bits: &GenericArray<u8, Self::ElemLen>,
-    ) -> Result<Self::Elem> {
-        Option::from(ProjectivePoint::from_bytes(element_bits)).ok_or(Error::PointError)
+    fn deserialize_element(element_bits: &GenericArray<u8, Self::ElemLen>) -> Result<Self::Elem> {
+        PublicKey::from_sec1_bytes(element_bits)
+            .map(|public_key| public_key.to_projective())
+            .map_err(|_| Error::PointError)
     }
 
     fn to_arr(elem: Self::Elem) -> GenericArray<u8, Self::ElemLen> {

src/group/ristretto.rs

@@ -108,11 +108,10 @@ impl Group for Ristretto255 {
         scalar.invert()
     }
 
-    fn from_element_slice_unchecked(
-        element_bits: &GenericArray<u8, Self::ElemLen>,
-    ) -> Result<Self::Elem> {
+    fn deserialize_element(element_bits: &GenericArray<u8, Self::ElemLen>) -> Result<Self::Elem> {
         CompressedRistretto::from_slice(element_bits)
             .decompress()
+            .filter(|point| point != &RistrettoPoint::identity())
             .ok_or(Error::PointError)
     }
 

src/group/tests.rs

@@ -36,7 +36,7 @@ fn test_group_properties() -> Result<()> {
 // Checks that the identity element cannot be deserialized
 fn test_identity_element_error<G: Group>() -> Result<()> {
     let identity = G::identity();
-    let result = G::from_element_slice(&G::to_arr(identity));
+    let result = G::deserialize_element(&G::to_arr(identity));
     assert!(matches!(result, Err(Error::PointError)));
 
     Ok(())

src/serialization.rs

@@ -61,7 +61,7 @@ impl<G: Group, H: BlockSizeUser + Digest + FixedOutputReset> VerifiableClient<G,
         let mut input = input.iter().copied();
 
         let blind = G::deserialize_scalar(&deserialize(&mut input)?)?;
-        let blinded_element = G::from_element_slice(&deserialize(&mut input)?)?;
+        let blinded_element = G::deserialize_element(&deserialize(&mut input)?)?;
 
         Ok(Self {
             blind,
@@ -105,7 +105,7 @@ impl<G: Group, H: BlockSizeUser + Digest + FixedOutputReset> VerifiableServer<G,
         let mut input = input.iter().copied();
 
         let sk = G::deserialize_scalar(&deserialize(&mut input)?)?;
-        let pk = G::from_element_slice(&deserialize(&mut input)?)?;
+        let pk = G::deserialize_element(&deserialize(&mut input)?)?;
 
         Ok(Self {
             sk,
@@ -150,7 +150,7 @@ impl<G: Group, H: BlockSizeUser + Digest + FixedOutputReset> BlindedElement<G, H
     pub fn deserialize(input: &[u8]) -> Result<Self> {
         let mut input = input.iter().copied();
 
-        let value = G::from_element_slice(&deserialize(&mut input)?)?;
+        let value = G::deserialize_element(&deserialize(&mut input)?)?;
 
         Ok(Self {
             value,
@@ -169,7 +169,7 @@ impl<G: Group, H: BlockSizeUser + Digest + FixedOutputReset> EvaluationElement<G
     pub fn deserialize(input: &[u8]) -> Result<Self> {
         let mut input = input.iter().copied();
 
-        let value = G::from_element_slice(&deserialize(&mut input)?)?;
+        let value = G::deserialize_element(&deserialize(&mut input)?)?;
 
         Ok(Self {
             value,

src/tests/voprf_test_vectors.rs

@@ -323,7 +323,7 @@ fn test_verifiable_finalize<G: Group, H: BlockSizeUser + Digest + FixedOutputRes
         for i in 0..parameters.input.len() {
             let client = VerifiableClient::<G, H>::from_blind_and_element(
                 G::deserialize_scalar(&GenericArray::clone_from_slice(&parameters.blind[i]))?,
-                G::from_element_slice(&GenericArray::clone_from_slice(
+                G::deserialize_element(&GenericArray::clone_from_slice(
                     &parameters.blinded_element[i],
                 ))?,
             );
@@ -341,7 +341,7 @@ fn test_verifiable_finalize<G: Group, H: BlockSizeUser + Digest + FixedOutputRes
             &clients,
             &messages,
             &Proof::deserialize(&parameters.proof)?,
-            G::from_element_slice(GenericArray::from_slice(&parameters.pksm))?,
+            G::deserialize_element(GenericArray::from_slice(&parameters.pksm))?,
             Some(&parameters.info),
         )?;