From 871b54e314e3df7bbf3aa076e0623d5be0ce5c1a Mon Sep 17 00:00:00 2001 From: aniampio Date: Thu, 17 Oct 2024 12:40:32 +0100 Subject: [PATCH 1/4] Fix computation of h for the SI-86 --- common/nymcoconut/src/scheme/aggregation.rs | 2 +- common/nymcoconut/src/scheme/issuance.rs | 25 ++++--- common/nymcoconut/src/scheme/mod.rs | 4 +- common/nymcoconut/src/scheme/verification.rs | 72 ++++++++++++++++++++ 4 files changed, 92 insertions(+), 11 deletions(-) diff --git a/common/nymcoconut/src/scheme/aggregation.rs b/common/nymcoconut/src/scheme/aggregation.rs index 68dbb037ef..e954e5b8b0 100644 --- a/common/nymcoconut/src/scheme/aggregation.rs +++ b/common/nymcoconut/src/scheme/aggregation.rs @@ -253,7 +253,7 @@ mod tests { let sigs = sks .iter() - .map(|sk| sign(¶ms, sk, &attributes).unwrap()) + .map(|sk| sign(sk, &attributes).unwrap()) .collect::>(); // aggregating (any) threshold works diff --git a/common/nymcoconut/src/scheme/issuance.rs b/common/nymcoconut/src/scheme/issuance.rs index fb3a09a7dd..a119a9585b 100644 --- a/common/nymcoconut/src/scheme/issuance.rs +++ b/common/nymcoconut/src/scheme/issuance.rs @@ -453,7 +453,6 @@ pub fn verify_partial_blind_signature( /// Creates a Coconut Signature under a given secret key on a set of public attributes only. pub fn sign( - params: &Parameters, secret_key: &SecretKey, public_attributes: &[&Attribute], ) -> Result { @@ -464,13 +463,23 @@ pub fn sign( }); } - // TODO: why in the python implementation this hash onto the curve is present - // while it's not used in the paper? the paper uses random exponent instead. - // (the python implementation hashes string representation of all attributes onto the curve, - // but I think the same can be achieved by just summing the attributes thus avoiding the unnecessary - // transformation. If I'm wrong, please correct me.) - let attributes_sum = public_attributes.iter().copied().sum::(); - let h = hash_g1((params.gen1() * attributes_sum).to_bytes()); + //Serialize the array structure of the public attributes into a byte array + let mut serialized_attributes = Vec::new(); + //Prepend the length of the entire array (in bytes) + let array_len = public_attributes.len() as u64; + serialized_attributes.extend_from_slice(&array_len.to_le_bytes()); + //Serialize each attribute with its length + for &attribute in public_attributes.iter() { + let attr_bytes = attribute.to_bytes(); + let attr_len = attr_bytes.len() as u64; + + // Prefix the attribute with its length + serialized_attributes.extend_from_slice(&attr_len.to_le_bytes()); + serialized_attributes.extend_from_slice(&attr_bytes); + } + + //Hash the resulting byte array to derive the point H + let h = hash_g1(serialized_attributes); // x + m0 * y0 + m1 * y1 + ... mn * yn let exponent = secret_key.x diff --git a/common/nymcoconut/src/scheme/mod.rs b/common/nymcoconut/src/scheme/mod.rs index 6ac5feb6a1..b811b6e250 100644 --- a/common/nymcoconut/src/scheme/mod.rs +++ b/common/nymcoconut/src/scheme/mod.rs @@ -436,8 +436,8 @@ mod tests { let keypair1 = keygen(¶ms); let keypair2 = keygen(¶ms); - let sig1 = sign(¶ms, keypair1.secret_key(), &attributes).unwrap(); - let sig2 = sign(¶ms, keypair2.secret_key(), &attributes).unwrap(); + let sig1 = sign(keypair1.secret_key(), &attributes).unwrap(); + let sig2 = sign(keypair2.secret_key(), &attributes).unwrap(); assert!(verify( ¶ms, diff --git a/common/nymcoconut/src/scheme/verification.rs b/common/nymcoconut/src/scheme/verification.rs index a1968ced47..1cda14730e 100644 --- a/common/nymcoconut/src/scheme/verification.rs +++ b/common/nymcoconut/src/scheme/verification.rs @@ -312,6 +312,7 @@ pub fn verify( #[cfg(test)] mod tests { + use crate::scheme::issuance::sign; use crate::scheme::keygen::keygen; use crate::scheme::setup::setup; @@ -355,4 +356,75 @@ mod tests { theta ); } + + #[test] + fn reject_forged_signature_via_linear_combination() { + // This test checks if the protocol correctly rejects forged signatures created + // by linear combinations of valid signatures. The verification for forged + // signatures should fail. + let params = Parameters::new(4).unwrap(); + + let scalar_2 = Scalar::one() + Scalar::one(); + let scalar_2_inv = Scalar::invert(&scalar_2).unwrap(); + + //#1 + let a = params.random_scalar(); + let zero = Scalar::zero(); + let a_zero = vec![&a, &zero]; + let zero_a = vec![&zero, &a]; + + let validator_keypair = keygen(¶ms); + + //#2 + let sig_a_zero = sign(validator_keypair.secret_key(), &a_zero).unwrap(); + let sig_zero_a = sign(validator_keypair.secret_key(), &zero_a).unwrap(); + + assert!(verify( + ¶ms, + validator_keypair.verification_key(), + &a_zero, + &sig_a_zero + )); + assert!(verify( + ¶ms, + validator_keypair.verification_key(), + &zero_a, + &sig_zero_a + )); + + //#3 + let h0 = sig_a_zero.0; + let h1 = &scalar_2_inv * &sig_a_zero.1 + &scalar_2_inv * &sig_zero_a.1; + let forged_signature = Signature(h0, h1); + let a_half = a * scalar_2_inv; + let new_plaintext = vec![&a_half, &a_half]; + + // The forged signature should not pass verification + assert!(!verify( + ¶ms, + validator_keypair.verification_key(), + &new_plaintext, + &forged_signature + )); + + //#4 + let scalar_3 = Scalar::one() + Scalar::one() + Scalar::one(); + let scalar_4 = Scalar::one() + Scalar::one() + Scalar::one() + Scalar::one(); + let scalar_4_inv = Scalar::invert(&scalar_4).unwrap(); + let scalar_3_over_4 = scalar_3 * scalar_4_inv; + + let h1_2 = &scalar_4_inv * &sig_a_zero.1 + &scalar_3_over_4 * &sig_zero_a.1; + let forged_signature_2 = Signature(h0, h1_2); + let a_quarter = a * scalar_4_inv; + let a_3_over_4 = a * scalar_3_over_4; + let new_plaintext_2 = vec![&a_quarter, &a_3_over_4]; + + // The second forged signature should also not pass verification + assert!(!verify( + ¶ms, + validator_keypair.verification_key(), + &new_plaintext_2, + &forged_signature_2 + )); + } } From c5b74353f323bbd09505c11732e92737091c47ea Mon Sep 17 00:00:00 2001 From: aniampio Date: Thu, 17 Oct 2024 13:17:34 +0100 Subject: [PATCH 2/4] Add infinity checks to fix SI-87 --- Cargo.lock | 1 + Cargo.toml | 1 + common/nym_offline_compact_ecash/Cargo.toml | 1 + .../nym_offline_compact_ecash/src/common_types.rs | 6 ++++++ .../src/scheme/aggregation.rs | 14 +++++++++++++- nym-wallet/Cargo.lock | 1 + 6 files changed, 23 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 8de83de65b..1fe90814a8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4868,6 +4868,7 @@ dependencies = [ "rayon", "serde", "sha2 0.9.9", + "subtle 2.5.0", "thiserror", "zeroize", ] diff --git a/Cargo.toml b/Cargo.toml index f7fdcd1716..d72cb74007 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -350,6 +350,7 @@ prometheus = { version = "0.13.0" } bls12_381 = { git = "https://github.com/jstuczyn/bls12_381", default-features = false, branch = "temp/experimental-serdect" } group = { version = "0.13.0", default-features = false } ff = { version = "0.13.0", default-features = false } +subtle = "2.5.0" # cosmwasm-related cosmwasm-schema = "=1.4.3" diff --git a/common/nym_offline_compact_ecash/Cargo.toml b/common/nym_offline_compact_ecash/Cargo.toml index e9d2acbda5..8e3e774ba0 100644 --- a/common/nym_offline_compact_ecash/Cargo.toml +++ b/common/nym_offline_compact_ecash/Cargo.toml @@ -25,6 +25,7 @@ rayon = { workspace = true, optional = true } zeroize = { workspace = true, features = ["zeroize_derive"] } ff = { workspace = true } group = { workspace = true } +subtle = { workspace = true } nym-pemstore = { path = "../pemstore" } nym-network-defaults = { path = "../network-defaults", default-features = false } diff --git a/common/nym_offline_compact_ecash/src/common_types.rs b/common/nym_offline_compact_ecash/src/common_types.rs index 6aaf81f673..14f9b1cb66 100644 --- a/common/nym_offline_compact_ecash/src/common_types.rs +++ b/common/nym_offline_compact_ecash/src/common_types.rs @@ -6,6 +6,7 @@ use crate::error::Result; use crate::helpers::{g1_tuple_to_bytes, recover_g1_tuple}; use bls12_381::{G1Projective, Scalar}; use serde::{Deserialize, Serialize}; +use subtle::Choice; pub type SignerIndex = u64; @@ -58,6 +59,11 @@ impl Signature { let (h, s) = recover_g1_tuple::(bytes)?; Ok(Signature { h, s }) } + + /// Checks whether any of the group elements of the signature is an identity element located at infinity + pub fn is_at_infinity(&self) -> Choice { + self.s.is_identity() | self.h.is_identity() + } } #[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)] diff --git a/common/nym_offline_compact_ecash/src/scheme/aggregation.rs b/common/nym_offline_compact_ecash/src/scheme/aggregation.rs index 994e64c2e2..9018cde7ea 100644 --- a/common/nym_offline_compact_ecash/src/scheme/aggregation.rs +++ b/common/nym_offline_compact_ecash/src/scheme/aggregation.rs @@ -50,6 +50,17 @@ where impl Aggregatable for PartialSignature { fn aggregate(sigs: &[PartialSignature], indices: Option<&[u64]>) -> Result { + // Ensure that we have valid signatures + if sigs.is_empty() { + return Err(CompactEcashError::AggregationEmptySet); + } + + // Check each individual signature for point at infinity + for sig in sigs { + if bool::from(sig.is_at_infinity()) { + return Err(CompactEcashError::IdentitySignature); + } + } let h = sigs .first() .ok_or(CompactEcashError::AggregationEmptySet)? @@ -109,7 +120,8 @@ pub fn aggregate_signatures( Err(err) => return Err(err), }; - if bool::from(signature.h.is_identity()) { + // Ensure the aggregated signature is not an infinity point + if bool::from(signature.is_at_infinity()) { return Err(CompactEcashError::IdentitySignature); } diff --git a/nym-wallet/Cargo.lock b/nym-wallet/Cargo.lock index 445c5f297a..6f20f00931 100644 --- a/nym-wallet/Cargo.lock +++ b/nym-wallet/Cargo.lock @@ -3156,6 +3156,7 @@ dependencies = [ "rand 0.8.5", "serde", "sha2 0.9.9", + "subtle 2.5.0", "thiserror", "zeroize", ] From f37eb9db235df3e146e232270bfe81d70d8546fb Mon Sep 17 00:00:00 2001 From: aniampio Date: Thu, 17 Oct 2024 13:34:45 +0100 Subject: [PATCH 3/4] Fixes following cargo clippy --- common/nymcoconut/src/scheme/verification.rs | 7 +++++-- wasm/zknym-lib/src/generic_scheme/coconut/mod.rs | 5 +---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/common/nymcoconut/src/scheme/verification.rs b/common/nymcoconut/src/scheme/verification.rs index 1cda14730e..067eb32185 100644 --- a/common/nymcoconut/src/scheme/verification.rs +++ b/common/nymcoconut/src/scheme/verification.rs @@ -394,7 +394,8 @@ mod tests { //#3 let h0 = sig_a_zero.0; - let h1 = &scalar_2_inv * &sig_a_zero.1 + &scalar_2_inv * &sig_zero_a.1; + // Removed unnecessary references + let h1 = scalar_2_inv * sig_a_zero.1 + scalar_2_inv * sig_zero_a.1; let forged_signature = Signature(h0, h1); let a_half = a * scalar_2_inv; let new_plaintext = vec![&a_half, &a_half]; @@ -413,7 +414,8 @@ mod tests { let scalar_4_inv = Scalar::invert(&scalar_4).unwrap(); let scalar_3_over_4 = scalar_3 * scalar_4_inv; - let h1_2 = &scalar_4_inv * &sig_a_zero.1 + &scalar_3_over_4 * &sig_zero_a.1; + // Removed unnecessary references + let h1_2 = scalar_4_inv * sig_a_zero.1 + scalar_3_over_4 * sig_zero_a.1; let forged_signature_2 = Signature(h0, h1_2); let a_quarter = a * scalar_4_inv; let a_3_over_4 = a * scalar_3_over_4; @@ -427,4 +429,5 @@ mod tests { &forged_signature_2 )); } + } diff --git a/wasm/zknym-lib/src/generic_scheme/coconut/mod.rs b/wasm/zknym-lib/src/generic_scheme/coconut/mod.rs index 178f53106c..5ac1b00dd7 100644 --- a/wasm/zknym-lib/src/generic_scheme/coconut/mod.rs +++ b/wasm/zknym-lib/src/generic_scheme/coconut/mod.rs @@ -96,17 +96,14 @@ pub fn ttp_keygen( pub fn sign_simple( attributes: Vec, keys: &KeyPairWrapper, - parameters: Option, ) -> Result { - let params = get_params(¶meters); - let public_attributes = attributes .into_iter() .map(hash_to_scalar) .collect::>(); let attributes_ref = public_attributes.iter().collect::>(); - nym_coconut::sign(params, keys.secret_key(), &attributes_ref) + nym_coconut::sign(keys.secret_key(), &attributes_ref) .map(Into::into) .map_err(Into::into) } From 25326e5f9b18e9a0ff61c5ef2b8525dc23d436e5 Mon Sep 17 00:00:00 2001 From: aniampio Date: Thu, 17 Oct 2024 13:35:24 +0100 Subject: [PATCH 4/4] Fixes following cargo fmt --- common/nymcoconut/src/scheme/issuance.rs | 5 +---- common/nymcoconut/src/scheme/verification.rs | 1 - 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/common/nymcoconut/src/scheme/issuance.rs b/common/nymcoconut/src/scheme/issuance.rs index a119a9585b..24a4c85bbf 100644 --- a/common/nymcoconut/src/scheme/issuance.rs +++ b/common/nymcoconut/src/scheme/issuance.rs @@ -452,10 +452,7 @@ pub fn verify_partial_blind_signature( } /// Creates a Coconut Signature under a given secret key on a set of public attributes only. -pub fn sign( - secret_key: &SecretKey, - public_attributes: &[&Attribute], -) -> Result { +pub fn sign(secret_key: &SecretKey, public_attributes: &[&Attribute]) -> Result { if public_attributes.len() > secret_key.ys.len() { return Err(CoconutError::IssuanceMaxAttributes { max: secret_key.ys.len(), diff --git a/common/nymcoconut/src/scheme/verification.rs b/common/nymcoconut/src/scheme/verification.rs index 067eb32185..149c60d3e3 100644 --- a/common/nymcoconut/src/scheme/verification.rs +++ b/common/nymcoconut/src/scheme/verification.rs @@ -429,5 +429,4 @@ mod tests { &forged_signature_2 )); } - }