Re-organize testing strategy and document Ciphersuite::hash_to_F.

crypto/ciphersuite/Cargo.toml

@@ -33,6 +33,9 @@ k256 = { version = "0.11", features = ["arithmetic", "bits", "hash2curve"], opti
 
 minimal-ed448 = { path = "../ed448", version = "^0.1.2", optional = true }
 
+[dev-dependencies]
+ff-group-tests = { version = "0.12", path = "../ff-group-tests" }
+
 [features]
 std = []
 

crypto/ciphersuite/README.md

@@ -1,3 +1,35 @@
 # Ciphersuite
 
 Ciphersuites for elliptic curves premised on ff/group.
+
+### Secp256k1/P-256
+
+Secp256k1 and P-256 are offered via [k256](https://crates.io/crates/k256) and
+[p256](https://crates.io/crates/p256), two libraries maintained by
+[RustCrypto](https://github.com/RustCrypto).
+
+Their `hash_to_F` is the
+[IETF's hash to curve](https://www.ietf.org/archive/id/draft-irtf-cfrg-hash-to-curve-16.html),
+yet applied to their scalar field.
+
+### Ed25519/Ristretto
+
+Ed25519/Ristretto are offered via
+[dalek-ff-group](https://crates.io/crates/dalek-ff-group), an ff/group wrapper
+around [curve25519-dalek](https://crates.io/crates/curve25519-dalek).
+
+Their `hash_to_F` is the wide reduction of SHA2-512, as used in
+[RFC 8032](https://www.rfc-editor.org/rfc/rfc8032). This is also compliant with
+the draft
+[RFC RISTRETTO](https://www.ietf.org/archive/id/draft-rtf-cfrg-ristretto255-decaf448-05.html).
+The domain-separation tag is naively prefixed to the message.
+
+### Ed448
+
+Ed448 is offered via [minimal-ed448](https://crates.io/crates/minimal-ed448), an
+explicitly not recommended Ed448 implementation, limited to its prime-order
+subgroup.
+
+Its `hash_to_F` is the wide reduction of SHAKE256, with a 114-byte output, as
+used in [RFC 8032](https://www.rfc-editor.org/rfc/rfc8032). The
+domain-separation tag is naively prefixed to the message.

crypto/ciphersuite/src/dalek.rs

@@ -39,6 +39,16 @@ macro_rules! dalek_curve {
 
 #[cfg(any(test, feature = "ristretto"))]
 dalek_curve!("ristretto", Ristretto, RistrettoPoint, b"ristretto");
+#[cfg(any(test, feature = "ristretto"))]
+#[test]
+fn test_ristretto() {
+  ff_group_tests::group::test_prime_group_bits::<RistrettoPoint>();
+}
 
 #[cfg(feature = "ed25519")]
 dalek_curve!("ed25519", Ed25519, EdwardsPoint, b"edwards25519");
+#[cfg(feature = "ed25519")]
+#[test]
+fn test_ed25519() {
+  ff_group_tests::group::test_prime_group_bits::<EdwardsPoint>();
+}

crypto/ciphersuite/src/ed448.rs

@@ -65,3 +65,9 @@ impl Ciphersuite for Ed448 {
     Scalar::wide_reduce(Self::H::digest([dst, data].concat()).as_ref().try_into().unwrap())
   }
 }
+
+#[test]
+fn test_ed448() {
+  // TODO: Enable once ed448 passes these tests
+  //ff_group_tests::group::test_prime_group_bits::<Point>();
+}

crypto/ciphersuite/src/kp256.rs

@@ -67,6 +67,16 @@ macro_rules! kp_curve {
 
 #[cfg(feature = "p256")]
 kp_curve!("p256", p256, P256, b"P-256");
+#[cfg(feature = "p256")]
+#[test]
+fn test_p256() {
+  ff_group_tests::group::test_prime_group_bits::<p256::ProjectivePoint>();
+}
 
 #[cfg(feature = "secp256k1")]
 kp_curve!("secp256k1", k256, Secp256k1, b"secp256k1");
+#[cfg(feature = "secp256k1")]
+#[test]
+fn test_secp256k1() {
+  ff_group_tests::group::test_prime_group_bits::<k256::ProjectivePoint>();
+}

crypto/ciphersuite/src/lib.rs

@@ -58,7 +58,14 @@ pub trait Ciphersuite: Clone + Copy + PartialEq + Eq + Debug + Zeroize {
   // While group does provide this in its API, privacy coins may want to use a custom basepoint
   fn generator() -> Self::G;
 
-  /// Hash the provided dst and message to a scalar.
+  /// Hash the provided domain-separation tag and message to a scalar. Ciphersuites MAY naively
+  /// prefix the tag to the message, enabling transpotion between the two. Accordingly, this
+  /// function should NOT be used in any scheme where one tag is a valid substring of another
+  /// UNLESS the specific Ciphersuite is verified to handle the DST securely.
+  ///
+  /// Verifying specific ciphersuites have secure tag handling is not recommended, due to it
+  /// breaking the intended modularity of ciphersuites. Instead, component-specific tags with
+  /// further purpose tags are recommended ("Schnorr-nonce", "Schnorr-chal").
   #[allow(non_snake_case)]
   fn hash_to_F(dst: &[u8], msg: &[u8]) -> Self::F;
 

crypto/dkg/Cargo.toml

@@ -32,5 +32,8 @@ ciphersuite = { path = "../ciphersuite", version = "0.1", features = ["std"] }
 schnorr = { package = "schnorr-signatures", path = "../schnorr", version = "0.2" }
 dleq = { path = "../dleq", version = "0.2", features = ["serialize"] }
 
+[dev-dependencies]
+ciphersuite = { path = "../ciphersuite", version = "0.1", features = ["std", "ristretto"] }
+
 [features]
 tests = []

crypto/dkg/src/tests/mod.rs

@@ -68,3 +68,8 @@ pub fn test_ciphersuite<R: RngCore + CryptoRng, C: Ciphersuite>(rng: &mut R) {
   key_gen::<_, C>(rng);
   test_generator_promotion::<_, C>(rng);
 }
+
+#[test]
+fn test_with_ristretto() {
+  test_ciphersuite::<_, ciphersuite::Ristretto>(&mut rand_core::OsRng);
+}

crypto/dleq/src/tests/mod.rs

@@ -52,6 +52,38 @@ fn test_dleq() {
       keys[k] = generators[k] * key.deref();
     }
     proof.verify(&mut transcript(), &generators[.. i], &keys[.. i]).unwrap();
+    // Different challenge
+    assert!(proof
+      .verify(
+        &mut RecommendedTranscript::new(b"different challenge"),
+        &generators[.. i],
+        &keys[.. i]
+      )
+      .is_err());
+
+    // We could edit these tests to always test with at least two generators
+    // Then we don't test proofs with zero/one generator(s)
+    // While those are stupid, and pointless, and potentially point to a failure in the caller,
+    // it could also be part of a dynamic system which deals with variable amounts of generators
+    // Not panicking in such use cases, even if they're inefficient, provides seamless behavior
+    if i >= 2 {
+      // Different generators
+      assert!(proof
+        .verify(
+          &mut transcript(),
+          generators[.. i].iter().cloned().rev().collect::<Vec<_>>().as_ref(),
+          &keys[.. i]
+        )
+        .is_err());
+      // Different keys
+      assert!(proof
+        .verify(
+          &mut transcript(),
+          &generators[.. i],
+          keys[.. i].iter().cloned().rev().collect::<Vec<_>>().as_ref()
+        )
+        .is_err());
+    }
 
     #[cfg(feature = "serialize")]
     {

crypto/ed448/README.md

@@ -3,7 +3,8 @@
 Inefficient, barebones implementation of Ed448 bound to the ff/group API,
 rejecting torsion to achieve a PrimeGroup definition. This likely should not be
 used and was only done so another library under Serai could confirm its
-completion. It is minimally tested, yet should be correct for what it has.
+completion. It is minimally tested, yet should be correct for what it has. The
-Multiple functions remain unimplemented.
+functions it doesn't have are marked `unimplemented!()`. This has not undergone
+auditing.
 
 constant time and no_std.

crypto/frost/src/tests/curve.rs

@@ -1,27 +0,0 @@
-use rand_core::{RngCore, CryptoRng};
-
-use group::Group;
-
-use crate::Curve;
-
-// Test successful multiexp, with enough pairs to trigger its variety of algorithms
-// Multiexp has its own tests, yet only against k256 and Ed25519 (which should be sufficient
-// as-is to prove multiexp), and this doesn't hurt
-pub fn test_multiexp<R: RngCore + CryptoRng, C: Curve>(rng: &mut R) {
-  let mut pairs = Vec::with_capacity(1000);
-  let mut sum = C::G::identity();
-  for _ in 0 .. 10 {
-    for _ in 0 .. 100 {
-      pairs.push((C::random_nonzero_F(&mut *rng), C::generator() * C::random_nonzero_F(&mut *rng)));
-      sum += pairs[pairs.len() - 1].1 * pairs[pairs.len() - 1].0;
-    }
-    assert_eq!(multiexp::multiexp(&pairs), sum);
-    assert_eq!(multiexp::multiexp_vartime(&pairs), sum);
-  }
-}
-
-pub fn test_curve<R: RngCore + CryptoRng, C: Curve>(rng: &mut R) {
-  // TODO: Test the Curve functions themselves
-
-  test_multiexp::<_, C>(rng);
-}

crypto/frost/src/tests/mod.rs

@@ -10,8 +10,6 @@ use crate::{
   sign::{Writable, PreprocessMachine, SignMachine, SignatureMachine, AlgorithmMachine},
 };
 
-/// Curve tests.
-pub mod curve;
 /// Vectorized test suite to ensure consistency.
 pub mod vectors;
 

crypto/frost/src/tests/vectors.rs

@@ -9,7 +9,7 @@ use rand_core::{RngCore, CryptoRng};
 
 use group::{ff::PrimeField, GroupEncoding};
 
-use dkg::tests::{key_gen, test_ciphersuite as test_dkg};
+use dkg::tests::key_gen;
 
 use crate::{
   curve::Curve,
@@ -19,7 +19,7 @@ use crate::{
     Nonce, GeneratorCommitments, NonceCommitments, Commitments, Writable, Preprocess, SignMachine,
     SignatureMachine, AlgorithmMachine,
   },
-  tests::{clone_without, recover_key, algorithm_machines, sign, curve::test_curve},
+  tests::{clone_without, recover_key, algorithm_machines, sign},
 };
 
 pub struct Vectors {
@@ -118,12 +118,6 @@ pub fn test_with_vectors<R: RngCore + CryptoRng, C: Curve, H: Hram<C>>(
   rng: &mut R,
   vectors: Vectors,
 ) {
-  // Do basic tests before trying the vectors
-  test_curve::<_, C>(&mut *rng);
-
-  // Test the DKG
-  test_dkg::<_, C>(&mut *rng);
-
   // Test a basic Schnorr signature
   {
     let keys = key_gen(&mut *rng);