Group trait overhaul (#52)

* Decouple element from `Group`

* Change `SUITE_ID` to `u16` and rework `get_context_string()`

* Rework scalar de-serialization

* Rename `Group` methods

- `random_nonzero_scalar` -> `random_scalar`
- `scalar_as_bytes` -> `serialize_scalar`
- `scalar_invert` -> `invert_scalar`

* Rework element de-serialization

* Rename and remove `Group` methods

`to_arr` -> `serialize_elem`
`base_point` -> `base_elem`
`is_identity` -> removed
`identity` -> `identity_elem`
`zero_scalar` -> hidden behind `cfg(test)`

* Sort `Group` methods

* Rework `expand_message_xmd` and remove utility

* Improve P256 `hash_to_scalar`

src/group/ristretto.rs

@@ -6,7 +6,6 @@
 // of this source tree.
 
 use core::convert::TryInto;
-use core::ops::Add;
 
 use curve25519_dalek::constants::RISTRETTO_BASEPOINT_POINT;
 use curve25519_dalek::ristretto::{CompressedRistretto, RistrettoPoint};
@@ -14,53 +13,55 @@ use curve25519_dalek::scalar::Scalar;
 use curve25519_dalek::traits::Identity;
 use digest::core_api::BlockSizeUser;
 use digest::{Digest, FixedOutputReset};
-use generic_array::typenum::{U1, U32, U64};
-use generic_array::{ArrayLength, GenericArray};
+use generic_array::sequence::Concat;
+use generic_array::typenum::{U32, U64};
+use generic_array::GenericArray;
 use rand_core::{CryptoRng, RngCore};
 
-use super::Group;
+use super::{expand, Group, STR_HASH_TO_GROUP, STR_HASH_TO_SCALAR};
+use crate::voprf::{self, Mode};
 use crate::{Error, Result};
 
+/// [`Group`] implementation for Ristretto255.
+pub struct Ristretto255;
+
 // `cfg` here is only needed because of a bug in Rust's crate feature documentation. See: https://github.com/rust-lang/rust/issues/83428
 #[cfg(feature = "ristretto255")]
-/// The implementation of such a subgroup for Ristretto
-impl Group for RistrettoPoint {
-    const SUITE_ID: usize = 0x0001;
+impl Group for Ristretto255 {
+    const SUITE_ID: u16 = 0x0001;
+
+    type Elem = RistrettoPoint;
+
+    type ElemLen = U32;
+
+    type Scalar = Scalar;
+
+    type ScalarLen = U32;
 
     // Implements the `hash_to_ristretto255()` function from
     // https://www.ietf.org/archive/id/draft-irtf-cfrg-hash-to-curve-10.txt
-    fn hash_to_curve<H: BlockSizeUser + Digest + FixedOutputReset, D: ArrayLength<u8> + Add<U1>>(
-        msg: &[u8],
-        dst: GenericArray<u8, D>,
-    ) -> Result<Self>
-    where
-        <D as Add<U1>>::Output: ArrayLength<u8>,
-    {
-        let uniform_bytes = super::expand::expand_message_xmd::<H, U64, _, _>(Some(msg), dst)?;
+    fn hash_to_curve<H: BlockSizeUser + Digest + FixedOutputReset>(
+        msg: &[&[u8]],
+        mode: Mode,
+    ) -> Result<Self::Elem> {
+        let dst =
+            GenericArray::from(STR_HASH_TO_GROUP).concat(voprf::get_context_string::<Self>(mode));
 
-        Ok(RistrettoPoint::from_uniform_bytes(
-            uniform_bytes
-                .as_slice()
-                .try_into()
-                .map_err(|_| Error::HashToCurveError)?,
-        ))
+        let uniform_bytes = expand::expand_message_xmd::<H, U64>(msg, &dst)?;
+
+        Ok(RistrettoPoint::from_uniform_bytes(&uniform_bytes.into()))
     }
 
     // Implements the `HashToScalar()` function from
     // https://www.ietf.org/archive/id/draft-irtf-cfrg-voprf-07.html#section-4.1
-    fn hash_to_scalar<
-        'a,
-        H: BlockSizeUser + Digest + FixedOutputReset,
-        D: ArrayLength<u8> + Add<U1>,
-        I: IntoIterator<Item = &'a [u8]>,
-    >(
-        input: I,
-        dst: GenericArray<u8, D>,
-    ) -> Result<Self::Scalar>
-    where
-        <D as Add<U1>>::Output: ArrayLength<u8>,
-    {
-        let uniform_bytes = super::expand::expand_message_xmd::<H, U64, _, _>(input, dst)?;
+    fn hash_to_scalar<'a, H: BlockSizeUser + Digest + FixedOutputReset>(
+        input: &[&[u8]],
+        mode: Mode,
+    ) -> Result<Self::Scalar> {
+        let dst =
+            GenericArray::from(STR_HASH_TO_SCALAR).concat(voprf::get_context_string::<Self>(mode));
+
+        let uniform_bytes = expand::expand_message_xmd::<H, U64>(input, &dst)?;
 
         Ok(Scalar::from_bytes_mod_order_wide(
             uniform_bytes
@@ -70,15 +71,27 @@ impl Group for RistrettoPoint {
         ))
     }
 
-    type Scalar = Scalar;
-    type ScalarLen = U32;
-    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 base_elem() -> Self::Elem {
+        RISTRETTO_BASEPOINT_POINT
     }
 
-    fn random_nonzero_scalar<R: RngCore + CryptoRng>(rng: &mut R) -> Self::Scalar {
+    fn identity_elem() -> Self::Elem {
+        RistrettoPoint::identity()
+    }
+
+    // serialization of a group element
+    fn serialize_elem(elem: Self::Elem) -> GenericArray<u8, Self::ElemLen> {
+        elem.compress().to_bytes().into()
+    }
+
+    fn deserialize_elem(element_bits: &GenericArray<u8, Self::ElemLen>) -> Result<Self::Elem> {
+        CompressedRistretto::from_slice(element_bits)
+            .decompress()
+            .filter(|point| point != &RistrettoPoint::identity())
+            .ok_or(Error::PointError)
+    }
+
+    fn random_scalar<R: RngCore + CryptoRng>(rng: &mut R) -> Self::Scalar {
         loop {
             let scalar = {
                 let mut scalar_bytes = [0u8; 64];
@@ -92,37 +105,22 @@ impl Group for RistrettoPoint {
         }
     }
 
-    fn scalar_as_bytes(scalar: Self::Scalar) -> GenericArray<u8, Self::ScalarLen> {
-        scalar.to_bytes().into()
-    }
-
-    fn scalar_invert(scalar: &Self::Scalar) -> Self::Scalar {
+    fn invert_scalar(scalar: Self::Scalar) -> Self::Scalar {
         scalar.invert()
     }
 
-    // The byte length necessary to represent group elements
-    type ElemLen = U32;
-    fn from_element_slice_unchecked(
-        element_bits: &GenericArray<u8, Self::ElemLen>,
-    ) -> Result<Self> {
-        CompressedRistretto::from_slice(element_bits)
-            .decompress()
-            .ok_or(Error::PointError)
-    }
-    // serialization of a group element
-    fn to_arr(&self) -> GenericArray<u8, Self::ElemLen> {
-        self.compress().to_bytes().into()
+    #[cfg(test)]
+    fn zero_scalar() -> Self::Scalar {
+        Scalar::zero()
     }
 
-    fn base_point() -> Self {
-        RISTRETTO_BASEPOINT_POINT
+    fn serialize_scalar(scalar: Self::Scalar) -> GenericArray<u8, Self::ScalarLen> {
+        scalar.to_bytes().into()
     }
 
-    fn identity() -> Self {
-        <Self as Identity>::identity()
-    }
-
-    fn scalar_zero() -> Self::Scalar {
-        Self::Scalar::zero()
+    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)
     }
 }