From 686a5ee36425e119b1e762c88e70811c4755eaec Mon Sep 17 00:00:00 2001
From: Luke Parker <lukeparker5132@gmail.com>
Date: Thu, 23 Feb 2023 01:03:53 -0500
Subject: [PATCH] 3.1.4 Further document hash_to_F which may collide

---
 crypto/ciphersuite/src/dalek.rs | 18 ++++++++++++++++--
 crypto/ciphersuite/src/ed448.rs |  7 ++++++-
 crypto/ciphersuite/src/kp256.rs | 14 ++++++++++++--
 3 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/crypto/ciphersuite/src/dalek.rs b/crypto/ciphersuite/src/dalek.rs
index ee49459f..3c40506d 100644
--- a/crypto/ciphersuite/src/dalek.rs
+++ b/crypto/ciphersuite/src/dalek.rs
@@ -17,8 +17,6 @@ macro_rules! dalek_curve {
   ) => {
     use dalek_ff_group::$Point;
 
-    #[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)]
-    pub struct $Ciphersuite;
     impl Ciphersuite for $Ciphersuite {
       type F = Scalar;
       type G = $Point;
@@ -37,6 +35,14 @@ macro_rules! dalek_curve {
   };
 }
 
+/// Ciphersuite for Ristretto.
+///
+/// hash_to_F is implemented with a naive concatenation of the dst and data, allowing transposition
+/// between the two. This means `dst: b"abc", data: b"def"`, will produce the same scalar as
+/// `dst: "abcdef", data: b""`. Please use carefully, not letting dsts be substrings of each other.
+#[cfg(any(test, feature = "ristretto"))]
+#[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)]
+pub struct Ristretto;
 #[cfg(any(test, feature = "ristretto"))]
 dalek_curve!("ristretto", Ristretto, RistrettoPoint, b"ristretto");
 #[cfg(any(test, feature = "ristretto"))]
@@ -60,6 +66,14 @@ fn test_ristretto() {
   );
 }
 
+/// Ciphersuite for Ed25519.
+///
+/// hash_to_F is implemented with a naive concatenation of the dst and data, allowing transposition
+/// between the two. This means `dst: b"abc", data: b"def"`, will produce the same scalar as
+/// `dst: "abcdef", data: b""`. Please use carefully, not letting dsts be substrings of each other.
+#[cfg(feature = "ed25519")]
+#[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)]
+pub struct Ed25519;
 #[cfg(feature = "ed25519")]
 dalek_curve!("ed25519", Ed25519, EdwardsPoint, b"edwards25519");
 #[cfg(feature = "ed25519")]
diff --git a/crypto/ciphersuite/src/ed448.rs b/crypto/ciphersuite/src/ed448.rs
index 30301ec9..e4f1d0a9 100644
--- a/crypto/ciphersuite/src/ed448.rs
+++ b/crypto/ciphersuite/src/ed448.rs
@@ -11,7 +11,7 @@ use minimal_ed448::{scalar::Scalar, point::Point};
 
 use crate::Ciphersuite;
 
-// Re-define Shake256 as a traditional Digest to meet API expectations
+/// Shake256, fixed to a 114-byte output, as used by Ed448.
 #[derive(Clone, Default)]
 pub struct Shake256_114(Shake256);
 impl BlockSizeUser for Shake256_114 {
@@ -48,6 +48,11 @@ impl FixedOutput for Shake256_114 {
 }
 impl HashMarker for Shake256_114 {}
 
+/// Ciphersuite for Ed448.
+///
+/// hash_to_F is implemented with a naive concatenation of the dst and data, allowing transposition
+/// between the two. This means `dst: b"abc", data: b"def"`, will produce the same scalar as
+/// `dst: "abcdef", data: b""`. Please use carefully, not letting dsts be substrings of each other.
 #[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)]
 pub struct Ed448;
 impl Ciphersuite for Ed448 {
diff --git a/crypto/ciphersuite/src/kp256.rs b/crypto/ciphersuite/src/kp256.rs
index aa84f8e5..c90b7214 100644
--- a/crypto/ciphersuite/src/kp256.rs
+++ b/crypto/ciphersuite/src/kp256.rs
@@ -20,8 +20,6 @@ macro_rules! kp_curve {
     $Ciphersuite: ident,
     $ID:          literal
   ) => {
-    #[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)]
-    pub struct $Ciphersuite;
     impl Ciphersuite for $Ciphersuite {
       type F = $lib::Scalar;
       type G = $lib::ProjectivePoint;
@@ -105,6 +103,12 @@ fn test_oversize_dst<C: Ciphersuite>() {
   assert_eq!(C::hash_to_F(&oversize_dst, &[]), C::hash_to_F(&actual_dst, &[]));
 }
 
+/// Ciphersuite for Secp256k1.
+///
+/// hash_to_F is implemented via the IETF draft for hash to curve's hash_to_field (v16).
+#[cfg(feature = "secp256k1")]
+#[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)]
+pub struct Secp256k1;
 #[cfg(feature = "secp256k1")]
 kp_curve!("secp256k1", k256, Secp256k1, b"secp256k1");
 #[cfg(feature = "secp256k1")]
@@ -137,6 +141,12 @@ fn test_secp256k1() {
   test_oversize_dst::<Secp256k1>();
 }
 
+/// Ciphersuite for P-256.
+///
+/// hash_to_F is implemented via the IETF draft for hash to curve's hash_to_field (v16).
+#[cfg(feature = "p256")]
+#[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)]
+pub struct P256;
 #[cfg(feature = "p256")]
 kp_curve!("p256", p256, P256, b"P-256");
 #[cfg(feature = "p256")]