Browse Source

fix: key the basepoints map on the group type

`StaticTypeMap::call_once::<Type, _>` looks the entry up by
`TypeId::of::<Type>()`, and this passed `Box<dyn CMZbp>` -- the same type
for every group. So the map held one slot shared by all groups rather
than one per group, which is the whole point of the design described at
length above it: a second `cmz_group_init` for a different group was
ignored, and the following `cmz_basepoints` read back the first group's
basepoints and panicked in the downcast.

Key on `G` instead. This makes previously-panicking programs work, so it
is a behaviour change and kept apart from the refactor that follows it.

tests/two_groups.rs covers it, and needs a second curve: cmz had only
curve25519-dalek available, so p256 joins the dev-dependencies.
Michele Orrù 1 month ago
parent
commit
25e60c1529
4 changed files with 33 additions and 4 deletions
  1. 1 0
      Cargo.lock
  2. 2 0
      Cargo.toml
  3. 2 4
      src/lib.rs
  4. 28 0
      tests/two_groups.rs

+ 1 - 0
Cargo.lock

@@ -359,6 +359,7 @@ dependencies = [
  "group",
  "hex",
  "lazy_static",
+ "p256",
  "postcard",
  "rand 0.8.5",
  "serde",

+ 2 - 0
Cargo.toml

@@ -24,6 +24,8 @@ thiserror = "2"
 [dev-dependencies]
 chrono = "0.4"
 curve25519-dalek = { version = "4", features = [ "group", "rand_core", "digest" ] }
+# Used for testing co-habitating with a second PrimeGroup.
+p256 = { version = "0.13", features = [ "arithmetic" ] }
 sha2 = "0.10"
 
 [patch.crates-io]

+ 2 - 4
src/lib.rs

@@ -246,10 +246,8 @@ lazy_static! {
 /// `&'static CMZBasepoints<G>`.
 fn load_bp<G: Group>(bp: Option<CMZBasepoints<G>>) -> &'static CMZBasepoints<G> {
     match bp {
-        Some(b) => basepoints_map.call_once::<Box<dyn CMZbp>, _>(|| Box::new(b.clone())),
-        None => {
-            basepoints_map.call_once::<Box<dyn CMZbp>, _>(|| panic!("basepoints uninitialized"))
-        }
+        Some(b) => basepoints_map.call_once::<G, _>(|| Box::new(b.clone())),
+        None => basepoints_map.call_once::<G, _>(|| panic!("basepoints uninitialized")),
     }
     .as_any()
     .downcast_ref::<CMZBasepoints<G>>()

+ 28 - 0
tests/two_groups.rs

@@ -0,0 +1,28 @@
+//! Basepoint derivation is keyed by group type, so two groups can coexist.
+
+use cmz::{cmz_basepoints, cmz_group_init};
+use curve25519_dalek::ristretto::RistrettoPoint;
+use group::Group;
+use sha2::Sha512;
+
+#[test]
+fn two_groups_coexist() {
+    let ristretto_a = RistrettoPoint::hash_from_bytes::<Sha512>(b"CMZ Generator A");
+    let p256_a = p256::ProjectivePoint::generator() * p256::Scalar::from(42u64);
+
+    cmz_group_init::<RistrettoPoint>(ristretto_a);
+    cmz_group_init::<p256::ProjectivePoint>(p256_a);
+
+    // Each group must read back its own basepoints, not the other's.
+    assert_eq!(cmz_basepoints::<RistrettoPoint>().A(), ristretto_a);
+    assert_eq!(cmz_basepoints::<RistrettoPoint>().B(), RistrettoPoint::generator());
+    assert_eq!(cmz_basepoints::<p256::ProjectivePoint>().A(), p256_a);
+    assert_eq!(
+        cmz_basepoints::<p256::ProjectivePoint>().B(),
+        p256::ProjectivePoint::generator()
+    );
+
+    // Re-initializing keeps the first value, per cmz_group_init's contract.
+    cmz_group_init::<RistrettoPoint>(RistrettoPoint::generator());
+    assert_eq!(cmz_basepoints::<RistrettoPoint>().A(), ristretto_a);
+}