Group revamp (#261)

* Revamp `KeGroup` trait

* Update dependencies

* Fix `hash_to_scalar` using `OprfGroup` instead of `KeGroup`

* Relax constraints on associated types of `KeGroup`

* Improve `KeGroup` implementation on `Curve`

* Improve `KeyExchange` trait

* Fix new Clippy 1.59 warnings

src/key_exchange/group/elliptic_curve.rs

@@ -0,0 +1,85 @@
+// Copyright (c) Facebook, Inc. and its affiliates.
+//
+// This source code is licensed under both the MIT license found in the
+// LICENSE-MIT file in the root directory of this source tree and the Apache
+// License, Version 2.0 found in the LICENSE-APACHE file in the root directory
+// of this source tree.
+
+use digest::core_api::BlockSizeUser;
+use digest::Digest;
+use elliptic_curve::group::cofactor::CofactorGroup;
+use elliptic_curve::hash2curve::{ExpandMsgXmd, FromOkm, GroupDigest};
+use elliptic_curve::sec1::{FromEncodedPoint, ModulusSize, ToEncodedPoint};
+use elliptic_curve::{
+    AffinePoint, Curve, FieldSize, NonZeroScalar, ProjectiveArithmetic, ProjectivePoint, PublicKey,
+    Scalar, SecretKey,
+};
+use generic_array::typenum::{IsLess, IsLessOrEqual, U256};
+use generic_array::GenericArray;
+use rand::{CryptoRng, RngCore};
+
+use super::KeGroup;
+use crate::errors::InternalError;
+
+impl<G: Curve + GroupDigest + ProjectiveArithmetic> KeGroup for G
+where
+    FieldSize<Self>: ModulusSize,
+    AffinePoint<Self>: FromEncodedPoint<Self> + ToEncodedPoint<Self>,
+    ProjectivePoint<Self>: CofactorGroup + ToEncodedPoint<Self>,
+    Scalar<Self>: FromOkm,
+{
+    type Pk = PublicKey<Self>;
+
+    type PkLen = <FieldSize<Self> as ModulusSize>::CompressedPointSize;
+
+    type Sk = SecretKey<Self>;
+
+    type SkLen = FieldSize<Self>;
+
+    fn serialize_pk(pk: &Self::Pk) -> GenericArray<u8, Self::PkLen> {
+        GenericArray::clone_from_slice(pk.to_encoded_point(true).as_bytes())
+    }
+
+    fn deserialize_pk(bytes: &GenericArray<u8, Self::PkLen>) -> Result<Self::Pk, InternalError> {
+        PublicKey::from_sec1_bytes(bytes).map_err(|_| InternalError::PointError)
+    }
+
+    fn random_sk<R: RngCore + CryptoRng>(rng: &mut R) -> Self::Sk {
+        SecretKey::random(rng)
+    }
+
+    // Implements the `HashToScalar()` function
+    fn hash_to_scalar<H>(input: &[&[u8]], dst: &[u8]) -> Result<Self::Sk, InternalError>
+    where
+        H: Digest + BlockSizeUser,
+        H::OutputSize: IsLess<U256> + IsLessOrEqual<H::BlockSize>,
+    {
+        Self::hash_to_scalar::<ExpandMsgXmd<H>>(input, dst)
+            .ok()
+            .and_then(|scalar| Option::<NonZeroScalar<Self>>::from(NonZeroScalar::new(scalar)))
+            .map(SecretKey::from)
+            .ok_or(InternalError::HashToScalar)
+    }
+
+    fn public_key(sk: &Self::Sk) -> Self::Pk {
+        sk.public_key()
+    }
+
+    fn diffie_hellman(pk: &Self::Pk, sk: &Self::Sk) -> GenericArray<u8, Self::PkLen> {
+        GenericArray::clone_from_slice(
+            (pk.to_projective() * sk.to_nonzero_scalar().as_ref())
+                .to_encoded_point(true)
+                .as_bytes(),
+        )
+    }
+
+    fn zeroize_sk_on_drop(_sk: &mut Self::Sk) {}
+
+    fn serialize_sk(sk: &Self::Sk) -> GenericArray<u8, Self::SkLen> {
+        sk.to_be_bytes()
+    }
+
+    fn deserialize_sk(bytes: &GenericArray<u8, Self::SkLen>) -> Result<Self::Sk, InternalError> {
+        SecretKey::from_be_bytes(bytes).map_err(|_| InternalError::PointError)
+    }
+}

src/key_exchange/group/mod.rs

@@ -7,37 +7,62 @@
 
 //! Includes the KeGroup trait and definitions for the key exchange groups
 
+mod elliptic_curve;
+#[cfg(feature = "ristretto255")]
+pub mod ristretto255;
+#[cfg(feature = "x25519")]
+pub mod x25519;
+
+use digest::core_api::BlockSizeUser;
+use digest::Digest;
+use generic_array::typenum::{IsLess, IsLessOrEqual, U256};
 use generic_array::{ArrayLength, GenericArray};
 use rand::{CryptoRng, RngCore};
 
 use crate::errors::InternalError;
 
 /// A group representation for use in the key exchange
-pub trait KeGroup: Sized + Clone {
+pub trait KeGroup {
+    /// Public key
+    type Pk: Clone;
     /// Length of the public key
-    type PkLen: ArrayLength<u8> + 'static;
+    type PkLen: ArrayLength<u8>;
+    /// Secret key
+    type Sk: Clone;
     /// Length of the secret key
-    type SkLen: ArrayLength<u8> + 'static;
-
-    /// Return a public key from its fixed-length bytes representation
-    fn from_pk_slice(element_bits: &GenericArray<u8, Self::PkLen>) -> Result<Self, InternalError>;
-
-    /// Generate a random secret key
-    fn random_sk<R: RngCore + CryptoRng>(rng: &mut R) -> GenericArray<u8, Self::SkLen>;
-
-    /// Return a public key from its secret key
-    fn public_key(sk: &GenericArray<u8, Self::SkLen>) -> Self;
+    type SkLen: ArrayLength<u8>;
 
     /// Serializes `self`
-    fn to_arr(&self) -> GenericArray<u8, Self::PkLen>;
+    fn serialize_pk(pk: &Self::Pk) -> GenericArray<u8, Self::PkLen>;
+
+    /// Return a public key from its fixed-length bytes representation
+    fn deserialize_pk(bytes: &GenericArray<u8, Self::PkLen>) -> Result<Self::Pk, InternalError>;
+
+    /// Generate a random secret key
+    fn random_sk<R: RngCore + CryptoRng>(rng: &mut R) -> Self::Sk;
+
+    /// Hashes a slice of pseudo-random bytes to a scalar
+    ///
+    /// # Errors
+    /// [`InternalError::HashToScalar`] if the `input` is empty or longer then
+    /// [`u16::MAX`].
+    fn hash_to_scalar<H>(input: &[&[u8]], dst: &[u8]) -> Result<Self::Sk, InternalError>
+    where
+        H: Digest + BlockSizeUser,
+        H::OutputSize: IsLess<U256> + IsLessOrEqual<H::BlockSize>;
+
+    /// Return a public key from its secret key
+    fn public_key(sk: &Self::Sk) -> Self::Pk;
 
     /// Diffie-Hellman key exchange
-    fn diffie_hellman(&self, sk: &GenericArray<u8, Self::SkLen>) -> GenericArray<u8, Self::PkLen>;
-}
+    fn diffie_hellman(pk: &Self::Pk, sk: &Self::Sk) -> GenericArray<u8, Self::PkLen>;
 
-#[cfg(feature = "p256")]
-pub mod p256;
-#[cfg(feature = "ristretto255")]
-pub mod ristretto255;
-#[cfg(feature = "x25519")]
-pub mod x25519;
+    /// Zeroize secret key on drop.
+    fn zeroize_sk_on_drop(sk: &mut Self::Sk);
+
+    /// Serializes `self`
+    fn serialize_sk(sk: &Self::Sk) -> GenericArray<u8, Self::SkLen>;
+
+    /// Return a public key from its fixed-length bytes representation
+    fn deserialize_sk(bytes: &GenericArray<u8, Self::SkLen>) -> Result<Self::Sk, InternalError>;
+}

src/key_exchange/group/p256.rs

@@ -1,52 +0,0 @@
-// Copyright (c) Facebook, Inc. and its affiliates.
-//
-// This source code is licensed under both the MIT license found in the
-// LICENSE-MIT file in the root directory of this source tree and the Apache
-// License, Version 2.0 found in the LICENSE-APACHE file in the root directory
-// of this source tree.
-
-//! Key Exchange group implementation for p256
-
-use generic_array::typenum::{U32, U33};
-use generic_array::GenericArray;
-use p256_::elliptic_curve::group::GroupEncoding;
-use p256_::elliptic_curve::sec1::ToEncodedPoint;
-use p256_::elliptic_curve::{PublicKey, SecretKey};
-use p256_::NistP256;
-use rand::{CryptoRng, RngCore};
-
-use super::KeGroup;
-use crate::errors::InternalError;
-
-impl KeGroup for PublicKey<NistP256> {
-    type PkLen = U33;
-    type SkLen = U32;
-
-    fn from_pk_slice(element_bits: &GenericArray<u8, Self::PkLen>) -> Result<Self, InternalError> {
-        Self::from_sec1_bytes(element_bits).map_err(|_| InternalError::PointError)
-    }
-
-    fn random_sk<R: RngCore + CryptoRng>(rng: &mut R) -> GenericArray<u8, Self::SkLen> {
-        SecretKey::<NistP256>::random(rng).to_be_bytes()
-    }
-
-    fn public_key(sk: &GenericArray<u8, Self::SkLen>) -> Self {
-        SecretKey::<NistP256>::from_be_bytes(sk)
-            .unwrap()
-            .public_key()
-    }
-
-    fn to_arr(&self) -> GenericArray<u8, Self::PkLen> {
-        GenericArray::clone_from_slice(self.to_encoded_point(true).as_bytes())
-    }
-
-    fn diffie_hellman(&self, sk: &GenericArray<u8, Self::SkLen>) -> GenericArray<u8, Self::PkLen> {
-        (self.to_projective()
-            * SecretKey::<NistP256>::from_be_bytes(sk)
-                .unwrap()
-                .to_nonzero_scalar()
-                .as_ref())
-        .to_affine()
-        .to_bytes()
-    }
-}

src/key_exchange/group/ristretto255.rs

@@ -10,24 +10,40 @@
 use curve25519_dalek::constants::RISTRETTO_BASEPOINT_POINT;
 use curve25519_dalek::ristretto::{CompressedRistretto, RistrettoPoint};
 use curve25519_dalek::scalar::Scalar;
-use generic_array::typenum::U32;
+use digest::core_api::BlockSizeUser;
+use digest::{Digest, OutputSizeUser};
+use elliptic_curve::hash2curve::{ExpandMsg, ExpandMsgXmd, Expander};
+use generic_array::typenum::{IsLess, IsLessOrEqual, U256, U32, U64};
 use generic_array::GenericArray;
 use rand::{CryptoRng, RngCore};
+use voprf::Group;
+use zeroize::Zeroize;
 
 use super::KeGroup;
 use crate::errors::InternalError;
 
-impl KeGroup for RistrettoPoint {
+/// Implementation for Ristretto255.
+// This is necessary because Rust lacks specialization, otherwise we could
+// implement `KeGroup` for `voprf::Ristretto255`.
+pub struct Ristretto255;
+
+impl KeGroup for Ristretto255 {
+    type Pk = RistrettoPoint;
     type PkLen = U32;
+    type Sk = Scalar;
     type SkLen = U32;
 
-    fn from_pk_slice(element_bits: &GenericArray<u8, Self::PkLen>) -> Result<Self, InternalError> {
-        CompressedRistretto::from_slice(element_bits)
+    fn serialize_pk(pk: &Self::Pk) -> GenericArray<u8, Self::PkLen> {
+        pk.compress().to_bytes().into()
+    }
+
+    fn deserialize_pk(bytes: &GenericArray<u8, Self::PkLen>) -> Result<Self::Pk, InternalError> {
+        CompressedRistretto::from_slice(bytes)
             .decompress()
             .ok_or(InternalError::PointError)
     }
 
-    fn random_sk<R: RngCore + CryptoRng>(rng: &mut R) -> GenericArray<u8, Self::SkLen> {
+    fn random_sk<R: RngCore + CryptoRng>(rng: &mut R) -> Self::Sk {
         loop {
             let scalar = {
                 #[cfg(not(test))]
@@ -47,21 +63,121 @@ impl KeGroup for RistrettoPoint {
                 }
             };
 
-            if scalar != Scalar::zero() {
-                break scalar.to_bytes().into();
+            if scalar != Scalar::zero() && scalar.is_canonical() {
+                break scalar;
             }
         }
     }
 
-    fn public_key(sk: &GenericArray<u8, Self::SkLen>) -> Self {
-        RISTRETTO_BASEPOINT_POINT * Scalar::from_bits(*sk.as_ref())
+    // Implements the `HashToScalar()` function from
+    // https://www.ietf.org/archive/id/draft-irtf-cfrg-voprf-08.html#section-4.1
+    fn hash_to_scalar<'a, H>(input: &[&[u8]], dst: &[u8]) -> Result<Self::Sk, InternalError>
+    where
+        H: Digest + BlockSizeUser,
+        H::OutputSize: IsLess<U256> + IsLessOrEqual<H::BlockSize>,
+    {
+        let mut uniform_bytes = GenericArray::<_, U64>::default();
+        ExpandMsgXmd::<H>::expand_message(input, dst, 64)
+            .map_err(|_| InternalError::HashToScalar)?
+            .fill_bytes(&mut uniform_bytes);
+
+        Ok(Scalar::from_bytes_mod_order_wide(&uniform_bytes.into()))
     }
 
-    fn to_arr(&self) -> GenericArray<u8, Self::PkLen> {
-        self.compress().to_bytes().into()
+    fn public_key(sk: &Self::Sk) -> Self::Pk {
+        RISTRETTO_BASEPOINT_POINT * sk
     }
 
-    fn diffie_hellman(&self, sk: &GenericArray<u8, Self::SkLen>) -> GenericArray<u8, Self::SkLen> {
-        (self * Scalar::from_bits(*sk.as_ref())).to_arr()
+    fn diffie_hellman(pk: &Self::Pk, sk: &Self::Sk) -> GenericArray<u8, Self::PkLen> {
+        Self::serialize_pk(&(pk * sk))
+    }
+
+    fn zeroize_sk_on_drop(sk: &mut Self::Sk) {
+        sk.zeroize()
+    }
+
+    fn serialize_sk(sk: &Self::Sk) -> GenericArray<u8, Self::SkLen> {
+        sk.to_bytes().into()
+    }
+
+    fn deserialize_sk(bytes: &GenericArray<u8, Self::PkLen>) -> Result<Self::Sk, InternalError> {
+        Scalar::from_canonical_bytes((*bytes).into()).ok_or(InternalError::PointError)
+    }
+}
+
+#[cfg(feature = "ristretto255_voprf")]
+impl voprf::CipherSuite for Ristretto255 {
+    const ID: u16 = voprf::Ristretto255::ID;
+
+    type Group = <voprf::Ristretto255 as voprf::CipherSuite>::Group;
+
+    type Hash = <voprf::Ristretto255 as voprf::CipherSuite>::Hash;
+}
+
+impl Group for Ristretto255 {
+    type Elem = <voprf::Ristretto255 as Group>::Elem;
+
+    type ElemLen = <voprf::Ristretto255 as Group>::ElemLen;
+
+    type Scalar = <voprf::Ristretto255 as Group>::Scalar;
+
+    type ScalarLen = <voprf::Ristretto255 as Group>::ScalarLen;
+
+    fn hash_to_curve<CS: voprf::CipherSuite>(
+        input: &[&[u8]],
+        dst: &[u8],
+    ) -> voprf::Result<Self::Elem, voprf::InternalError>
+    where
+        <CS::Hash as OutputSizeUser>::OutputSize:
+            IsLess<U256> + IsLessOrEqual<<CS::Hash as BlockSizeUser>::BlockSize>,
+    {
+        <voprf::Ristretto255 as Group>::hash_to_curve::<CS>(input, dst)
+    }
+
+    fn hash_to_scalar<CS: voprf::CipherSuite>(
+        input: &[&[u8]],
+        dst: &[u8],
+    ) -> voprf::Result<Self::Scalar, voprf::InternalError>
+    where
+        <CS::Hash as OutputSizeUser>::OutputSize:
+            IsLess<U256> + IsLessOrEqual<<CS::Hash as BlockSizeUser>::BlockSize>,
+    {
+        <voprf::Ristretto255 as Group>::hash_to_scalar::<CS>(input, dst)
+    }
+
+    fn base_elem() -> Self::Elem {
+        <voprf::Ristretto255 as Group>::base_elem()
+    }
+
+    fn identity_elem() -> Self::Elem {
+        <voprf::Ristretto255 as Group>::identity_elem()
+    }
+
+    fn serialize_elem(elem: Self::Elem) -> GenericArray<u8, Self::ElemLen> {
+        <voprf::Ristretto255 as Group>::serialize_elem(elem)
+    }
+
+    fn deserialize_elem(element_bits: &[u8]) -> voprf::Result<Self::Elem> {
+        <voprf::Ristretto255 as Group>::deserialize_elem(element_bits)
+    }
+
+    fn random_scalar<R: RngCore + CryptoRng>(rng: &mut R) -> Self::Scalar {
+        <voprf::Ristretto255 as Group>::random_scalar(rng)
+    }
+
+    fn invert_scalar(scalar: Self::Scalar) -> Self::Scalar {
+        <voprf::Ristretto255 as Group>::invert_scalar(scalar)
+    }
+
+    fn is_zero_scalar(scalar: Self::Scalar) -> subtle::Choice {
+        <voprf::Ristretto255 as Group>::is_zero_scalar(scalar)
+    }
+
+    fn serialize_scalar(scalar: Self::Scalar) -> GenericArray<u8, Self::ScalarLen> {
+        <voprf::Ristretto255 as Group>::serialize_scalar(scalar)
+    }
+
+    fn deserialize_scalar(scalar_bits: &[u8]) -> voprf::Result<Self::Scalar> {
+        <voprf::Ristretto255 as Group>::deserialize_scalar(scalar_bits)
     }
 }

src/key_exchange/group/x25519.rs

@@ -7,47 +7,97 @@
 
 //! Key Exchange group implementation for X25519
 
-use generic_array::typenum::U32;
+use curve25519_dalek_3::scalar::Scalar;
+use digest::core_api::BlockSizeUser;
+use digest::Digest;
+use elliptic_curve::hash2curve::{ExpandMsg, ExpandMsgXmd, Expander};
+use generic_array::typenum::{IsLess, IsLessOrEqual, U256, U32, U64};
 use generic_array::GenericArray;
 use rand::{CryptoRng, RngCore};
 use x25519_dalek::{PublicKey, StaticSecret};
+use zeroize::Zeroize;
 
 use super::KeGroup;
 use crate::errors::InternalError;
 
+/// Implementation for X25519.
+pub struct X25519;
+
 /// The implementation of such a subgroup for Ristretto
-impl KeGroup for PublicKey {
+impl KeGroup for X25519 {
+    type Pk = PublicKey;
     type PkLen = U32;
+    type Sk = StaticSecret;
     type SkLen = U32;
 
-    fn from_pk_slice(element_bits: &GenericArray<u8, Self::PkLen>) -> Result<Self, InternalError> {
-        Ok(Self::from(<[u8; 32]>::from(*element_bits)))
+    fn serialize_pk(pk: &Self::Pk) -> GenericArray<u8, Self::PkLen> {
+        pk.to_bytes().into()
     }
 
-    fn random_sk<R: RngCore + CryptoRng>(rng: &mut R) -> GenericArray<u8, Self::SkLen> {
+    fn deserialize_pk(bytes: &GenericArray<u8, Self::PkLen>) -> Result<Self::Pk, InternalError> {
+        if **bytes == [0; 32] {
+            Err(InternalError::PointError)
+        } else {
+            Ok(PublicKey::from(<[_; 32]>::from(*bytes)))
+        }
+    }
+
+    fn random_sk<R: RngCore + CryptoRng>(rng: &mut R) -> Self::Sk {
         let mut scalar_bytes = [0u8; 32];
 
         loop {
             rng.fill_bytes(&mut scalar_bytes);
 
             if scalar_bytes != [0u8; 32] {
-                break StaticSecret::from(scalar_bytes).to_bytes().into();
+                break StaticSecret::from(scalar_bytes);
             }
         }
     }
 
-    fn public_key(sk: &GenericArray<u8, Self::SkLen>) -> Self {
-        Self::from(&StaticSecret::from(<[u8; 32]>::from(*sk)))
+    // Implements the `HashToScalar()` function from
+    // https://www.ietf.org/archive/id/draft-irtf-cfrg-voprf-08.html#section-4.1
+    fn hash_to_scalar<'a, H>(input: &[&[u8]], dst: &[u8]) -> Result<Self::Sk, InternalError>
+    where
+        H: Digest + BlockSizeUser,
+        H::OutputSize: IsLess<U256> + IsLessOrEqual<H::BlockSize>,
+    {
+        let mut uniform_bytes = GenericArray::<_, U64>::default();
+        ExpandMsgXmd::<H>::expand_message(input, dst, 64)
+            .map_err(|_| InternalError::HashToScalar)?
+            .fill_bytes(&mut uniform_bytes);
+
+        Ok(StaticSecret::from(
+            Scalar::from_bytes_mod_order_wide(&uniform_bytes.into()).to_bytes(),
+        ))
     }
 
-    fn to_arr(&self) -> GenericArray<u8, Self::PkLen> {
-        self.to_bytes().into()
+    fn public_key(sk: &Self::Sk) -> Self::Pk {
+        PublicKey::from(sk)
     }
 
-    fn diffie_hellman(&self, sk: &GenericArray<u8, Self::SkLen>) -> GenericArray<u8, Self::SkLen> {
-        StaticSecret::from(<[u8; 32]>::from(*sk))
-            .diffie_hellman(self)
-            .to_bytes()
-            .into()
+    fn diffie_hellman(pk: &Self::Pk, sk: &Self::Sk) -> GenericArray<u8, Self::PkLen> {
+        sk.diffie_hellman(pk).to_bytes().into()
+    }
+
+    fn zeroize_sk_on_drop(sk: &mut Self::Sk) {
+        sk.zeroize()
+    }
+
+    fn serialize_sk(sk: &Self::Sk) -> GenericArray<u8, Self::SkLen> {
+        sk.to_bytes().into()
+    }
+
+    fn deserialize_sk(bytes: &GenericArray<u8, Self::PkLen>) -> Result<Self::Sk, InternalError> {
+        if **bytes == [0; 32] {
+            Err(InternalError::PointError)
+        } else {
+            let sk = StaticSecret::from(<[u8; 32]>::from(*bytes));
+
+            if sk.to_bytes() == **bytes {
+                Ok(sk)
+            } else {
+                Err(InternalError::PointError)
+            }
+        }
     }
 }