ExpandMsg improvements (#874)

Follow-up for #400

Prevent overflows and add zero check.

.github/workflows/elliptic-curve.yml

@@ -43,14 +43,15 @@ jobs:
       - run: cargo build --target ${{ matrix.target }} --release --no-default-features --features digest
       - run: cargo build --target ${{ matrix.target }} --release --no-default-features --features ecdh
       - run: cargo build --target ${{ matrix.target }} --release --no-default-features --features hazmat
+      - run: cargo build --target ${{ matrix.target }} --release --no-default-features --features hash2curve
       - run: cargo build --target ${{ matrix.target }} --release --no-default-features --features jwk
       - run: cargo build --target ${{ matrix.target }} --release --no-default-features --features pem
       - run: cargo build --target ${{ matrix.target }} --release --no-default-features --features pkcs8
       - run: cargo build --target ${{ matrix.target }} --release --no-default-features --features sec1
       - run: cargo build --target ${{ matrix.target }} --release --no-default-features --features serde
       - run: cargo build --target ${{ matrix.target }} --release --no-default-features --features pkcs8,sec1
       - run: cargo build --target ${{ matrix.target }} --release --no-default-features --features pem,pkcs8,sec1
-      - run: cargo build --target ${{ matrix.target }} --release --no-default-features --features alloc,digest,ecdh,hazmat,jwk,pem,pkcs8,sec1,serde
+      - run: cargo build --target ${{ matrix.target }} --release --no-default-features --features alloc,digest,ecdh,hazmat,hash2curve,jwk,pem,pkcs8,sec1,serde
 
   test:
     runs-on: ubuntu-latest

elliptic-curve/Cargo.toml

@@ -40,6 +40,8 @@ serde_json = { version = "1", optional = true, default-features = false, feature
 
 [dev-dependencies]
 hex-literal = "0.3"
+sha2 = "0.9"
+sha3 = "0.9"
 
 [features]
 default = ["arithmetic"]

elliptic-curve/src/hash2curve/group_digest.rs

@@ -8,10 +8,9 @@ use group::cofactor::CofactorGroup;
 /// Adds hashing arbitrary byte sequences to a valid group element
 pub trait GroupDigest {
     /// The field element representation for a group value with multiple elements
-    type FieldElement: FromOkm + Default + Copy;
+    type FieldElement: FromOkm + MapToCurve<Output = Self::Output> + Default + Copy;
     /// The resulting group element
-    type Output: CofactorGroup<Subgroup = Self::Output>
-        + MapToCurve<FieldElement = Self::FieldElement, Output = Self::Output>;
+    type Output: CofactorGroup<Subgroup = Self::Output>;
 
     /// Computes the hash to curve routine according to
     /// <https://www.ietf.org/archive/id/draft-irtf-cfrg-hash-to-curve-13.html>
@@ -40,8 +39,8 @@ pub trait GroupDigest {
     fn hash_from_bytes<X: ExpandMsg>(msg: &[u8], dst: &'static [u8]) -> Result<Self::Output> {
         let mut u = [Self::FieldElement::default(), Self::FieldElement::default()];
         hash_to_field::<X, _>(msg, dst, &mut u)?;
-        let q0 = Self::Output::map_to_curve(u[0]);
-        let q1 = Self::Output::map_to_curve(u[1]);
+        let q0 = u[0].map_to_curve();
+        let q1 = u[1].map_to_curve();
         // Ideally we could add and then clear cofactor once
         // thus saving a call but the field elements may not
         // add properly due to the underlying implementation
@@ -66,7 +65,7 @@ pub trait GroupDigest {
     fn encode_from_bytes<X: ExpandMsg>(msg: &[u8], dst: &'static [u8]) -> Result<Self::Output> {
         let mut u = [Self::FieldElement::default()];
         hash_to_field::<X, _>(msg, dst, &mut u)?;
-        let q0 = Self::Output::map_to_curve(u[0]);
+        let q0 = u[0].map_to_curve();
         Ok(q0.clear_cofactor())
     }
 }

elliptic-curve/src/hash2curve/map2curve.rs

@@ -2,11 +2,9 @@
 /// via a mapping method like Simplified Shallue-van de Woestijne-Ulas
 /// or Elligator
 pub trait MapToCurve {
-    /// The input values representing x and y
-    type FieldElement;
     /// The output point
     type Output;
 
     /// Map a field element into a point
-    fn map_to_curve(u: Self::FieldElement) -> Self::Output;
+    fn map_to_curve(&self) -> Self::Output;
 }

elliptic-curve/src/hash2field/expand_msg.rs

@@ -1,5 +1,6 @@
 use crate::Result;
-use digest::{Digest, ExtendableOutputDirty, Update, XofReader};
+use digest::{Digest, ExtendableOutput, Update, XofReader};
+use generic_array::typenum::{IsLess, U256};
 use generic_array::{ArrayLength, GenericArray};
 
 /// Salt when the DST is too long
@@ -23,24 +24,30 @@ pub trait ExpandMsg: Sized {
 /// Implements [section 5.4.3 of `draft-irtf-cfrg-hash-to-curve-13`][dst].
 ///
 /// [dst]: https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-hash-to-curve-13#section-5.4.3
-pub(crate) enum Domain<L: ArrayLength<u8>> {
+pub(crate) enum Domain<L>
+where
+    L: ArrayLength<u8> + IsLess<U256>,
+{
     /// > 255
     Hashed(GenericArray<u8, L>),
     /// <= 255
     Array(&'static [u8]),
 }
 
-impl<L: ArrayLength<u8>> Domain<L> {
+impl<L> Domain<L>
+where
+    L: ArrayLength<u8> + IsLess<U256>,
+{
     pub fn xof<X>(dst: &'static [u8]) -> Self
     where
-        X: Default + ExtendableOutputDirty + Update,
+        X: Default + ExtendableOutput + Update,
     {
         if dst.len() > MAX_DST_LEN {
             let mut data = GenericArray::<u8, L>::default();
             X::default()
                 .chain(OVERSIZE_DST_SALT)
                 .chain(dst)
-                .finalize_xof_dirty()
+                .finalize_xof()
                 .read(&mut data);
             Self::Hashed(data)
         } else {
@@ -66,10 +73,18 @@ impl<L: ArrayLength<u8>> Domain<L> {
         }
     }
 
-    pub fn len(&self) -> usize {
+    pub fn len(&self) -> u8 {
         match self {
-            Self::Hashed(_) => L::to_usize(),
-            Self::Array(d) => d.len(),
+            // Can't overflow because it's enforced on a type level.
+            Self::Hashed(_) => L::to_u8(),
+            // Can't overflow because it's checked on creation.
+            Self::Array(d) => u8::try_from(d.len()).expect("length overflow"),
         }
     }
+
+    #[cfg(test)]
+    pub fn assert(&self, bytes: &[u8]) {
+        assert_eq!(self.data(), &bytes[..bytes.len() - 1]);
+        assert_eq!(self.len(), bytes[bytes.len() - 1]);
+    }
 }