From f34a431f86dfab38857341869e56d9fdf1704162 Mon Sep 17 00:00:00 2001 From: Keenan Breik Date: Mon, 27 Jul 2026 09:01:12 -0700 Subject: [PATCH 1/2] Add two-group signing with group-tagged binding factors Generalizes nested signing to a second participant group that is itself a t-of-n FROST group with its own identifier space. Collections are keyed by (group, identifier) and the binding-factor preimage is domain-separated by a one-byte group tag (0x00/0x01), in both the per-signer preimage and each commitment-list entry, so identifiers that collide numerically across groups never alias. Group commitment, challenge, share computation/verification, and even-Y handling reuse the Ciphersuite hooks unchanged; existing entry points are untouched. Aggregation carries verifying shares for the primary group only: share-level blame is assigned for primary signers, while a failure that lies with the secondary group (whose verifying shares are not available) is reported as InvalidSignature without blame. Co-Authored-By: Claude Fable 5 --- frost-core/src/lib.rs | 1 + frost-core/src/two_group.rs | 407 ++++++++++++++++++ frost-secp256k1-tr/tests/two_group_tests.rs | 439 ++++++++++++++++++++ 3 files changed, 847 insertions(+) create mode 100644 frost-core/src/two_group.rs create mode 100644 frost-secp256k1-tr/tests/two_group_tests.rs diff --git a/frost-core/src/lib.rs b/frost-core/src/lib.rs index 7be4bc8..020092f 100644 --- a/frost-core/src/lib.rs +++ b/frost-core/src/lib.rs @@ -38,6 +38,7 @@ pub mod keys; pub mod round1; pub mod round2; mod scalar_mul; +pub mod two_group; // We'd like to make this conditionally pub but the attribute below does // not work yet (https://github.com/rust-lang/rust/issues/54727) // #[cfg_attr(feature = "internals", visibility::make(pub))] diff --git a/frost-core/src/two_group.rs b/frost-core/src/two_group.rs new file mode 100644 index 0000000..b97e294 --- /dev/null +++ b/frost-core/src/two_group.rs @@ -0,0 +1,407 @@ +//! Two-group FROST signing with group-tagged binding factors. +//! +//! Generalizes the nested-signing scheme (one flat commitment set with +//! per-group Lagrange interpolation, [`SigningPackage::new_with_participants_groups`]) +//! to the case where the second participant group is itself a t-of-n FROST +//! group with its own identifier space. The two identifier spaces are +//! independent and may collide numerically, so every collection in this +//! module is keyed by `(group, identifier)` and the binding-factor preimage +//! is domain-separated by a one-byte group tag: +//! +//! ```text +//! rho_i = H1(tag(group(i)) || vk || H4(msg) || H5(commitment_list) || id(i)) +//! commitment_list = concatenation, over all signers of both groups sorted +//! by (tag, identifier), of +//! tag || identifier || hiding || binding +//! ``` +//! +//! The tag appears both in each signer's own preimage and in every +//! commitment-list entry, so signers whose identifiers collide across groups +//! alias in neither the per-signer factor nor the commitment-set hash. +//! Everything else — the group commitment `R`, the challenge, per-signer +//! share computation and verification — reuses the [`Ciphersuite`] hooks +//! unchanged, so ciphersuite-specific behavior (e.g. BIP-340 even-Y nonce +//! negation, which is decided by the parity of the single `R` computed over +//! both groups' commitments) is identical to single-group signing. Existing +//! single-group entry points are untouched: callers select this scheme +//! explicitly by calling into this module. +//! +//! Like the participant-groups scheme, each signer's Lagrange coefficient is +//! computed within its own group over that group's participating identifiers, +//! and is folded into the signer's share; [`aggregate`] only ever sums shares. +//! +//! Limitations, by design: no adaptor points, and no ciphersuites whose +//! pre-processing hooks depend on the commitment set or transform signature +//! shares (the hooks are invoked with a synthetic single-group +//! [`SigningPackage`] carrying only the relevant group's commitments; every +//! ciphersuite in this workspace satisfies this, including +//! secp256k1-tr, whose hooks only normalize key material to even-Y). + +use alloc::collections::{BTreeMap, BTreeSet}; +use alloc::vec::Vec; + +use crate::{ + compute_lagrange_coefficient, keys, round1, round2, BindingFactor, Ciphersuite, Error, Field, + Group, GroupCommitment, Identifier, Signature, SigningPackage, VerifyingKey, +}; + +/// Which of the two signing groups a signer belongs to. +/// +/// The variant order defines the domain-separation tag byte and the +/// commitment-list ordering; it is part of the wire-level scheme and MUST NOT +/// change. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub enum SignerGroup { + /// The first group (tag byte `0x00`). + Primary, + /// The second group (tag byte `0x01`). + Secondary, +} + +impl SignerGroup { + /// The domain-separation tag byte for this group. + pub fn tag(self) -> u8 { + match self { + SignerGroup::Primary => 0x00, + SignerGroup::Secondary => 0x01, + } + } +} + +/// The group-tagged H1 preimages of every signer's binding factor, in +/// `(tag, identifier)` order. +pub type BindingFactorPreimages = Vec<((SignerGroup, Identifier), Vec)>; + +type BindingFactors = BTreeMap<(SignerGroup, Identifier), BindingFactor>; + +/// The message and both groups' round-one commitments for one two-group +/// signing run. +/// +/// The commitments of the two groups are kept in separate maps because their +/// identifier spaces are independent: the same identifier scalar may appear +/// in both groups and refers to two different signers. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct TwoGroupSigningPackage { + primary_commitments: BTreeMap, round1::SigningCommitments>, + secondary_commitments: BTreeMap, round1::SigningCommitments>, + message: Vec, +} + +impl TwoGroupSigningPackage { + /// Creates a package from both groups' commitments and the message. + /// Both groups must have at least one participating signer. + pub fn new( + primary_commitments: BTreeMap, round1::SigningCommitments>, + secondary_commitments: BTreeMap, round1::SigningCommitments>, + message: &[u8], + ) -> Result> { + if primary_commitments.is_empty() || secondary_commitments.is_empty() { + return Err(Error::IncorrectNumberOfCommitments); + } + Ok(Self { + primary_commitments, + secondary_commitments, + message: message.to_vec(), + }) + } + + /// The message to be signed. + pub fn message(&self) -> &[u8] { + &self.message + } + + /// The given group's commitments. + pub fn signing_commitments( + &self, + group: SignerGroup, + ) -> &BTreeMap, round1::SigningCommitments> { + match group { + SignerGroup::Primary => &self.primary_commitments, + SignerGroup::Secondary => &self.secondary_commitments, + } + } + + /// Iterates over all signers of both groups in `(tag, identifier)` order — + /// the canonical order of the commitment-list encoding. + fn iter_all( + &self, + ) -> impl Iterator, &round1::SigningCommitments)> { + self.primary_commitments + .iter() + .map(|(id, c)| (SignerGroup::Primary, id, c)) + .chain( + self.secondary_commitments + .iter() + .map(|(id, c)| (SignerGroup::Secondary, id, c)), + ) + } + + /// Encodes both groups' commitments in `(tag, identifier)` order, each + /// entry as `tag || identifier || hiding || binding`. + fn encode_tagged_commitments(&self) -> Result, Error> { + let mut bytes = Vec::new(); + for (group, identifier, commitment) in self.iter_all() { + bytes.push(group.tag()); + bytes.extend_from_slice(identifier.serialize().as_ref()); + bytes.extend_from_slice(::serialize(&commitment.hiding.value())?.as_ref()); + bytes.extend_from_slice(::serialize(&commitment.binding.value())?.as_ref()); + } + Ok(bytes) + } + + /// Computes the group-tagged H1 preimages of every signer's binding + /// factor, in `(tag, identifier)` order. + pub fn binding_factor_preimages( + &self, + verifying_key: &VerifyingKey, + ) -> Result, Error> { + let mut common_suffix = Vec::new(); + common_suffix.extend_from_slice(verifying_key.serialize()?.as_ref()); + common_suffix.extend_from_slice(C::H4(&self.message).as_ref()); + common_suffix.extend_from_slice(C::H5(&self.encode_tagged_commitments()?).as_ref()); + + Ok(self + .iter_all() + .map(|(group, identifier, _)| { + let mut preimage = Vec::new(); + preimage.push(group.tag()); + preimage.extend_from_slice(&common_suffix); + preimage.extend_from_slice(identifier.serialize().as_ref()); + ((group, *identifier), preimage) + }) + .collect()) + } + + fn compute_binding_factors( + &self, + verifying_key: &VerifyingKey, + ) -> Result, Error> { + Ok(self + .binding_factor_preimages(verifying_key)? + .into_iter() + .map(|(key, preimage)| (key, BindingFactor(C::H1(&preimage)))) + .collect()) + } + + /// Computes the single group commitment `R` over both groups' + /// commitments. Mirrors [`crate::compute_group_commitment`], with the + /// binding factors keyed by `(group, identifier)`. + fn compute_group_commitment( + &self, + binding_factors: &BindingFactors, + ) -> Result, Error> { + let identity = ::identity(); + let mut group_commitment = identity; + + let mut binding_scalars = Vec::new(); + let mut binding_elements = Vec::new(); + + for (group, identifier, commitment) in self.iter_all() { + // The following check prevents a party from accidentally revealing their share. + if identity == commitment.binding.value() || identity == commitment.hiding.value() { + return Err(Error::IdentityCommitment); + } + + let binding_factor = binding_factors + .get(&(group, *identifier)) + .ok_or(Error::UnknownIdentifier)?; + + binding_elements.push(commitment.binding.value()); + binding_scalars.push(binding_factor.0); + + group_commitment = group_commitment + commitment.hiding.value(); + } + + let accumulated_binding_commitment = + crate::scalar_mul::VartimeMultiscalarMul::::vartime_multiscalar_mul( + binding_scalars, + binding_elements, + ); + + Ok(GroupCommitment( + group_commitment + accumulated_binding_commitment, + )) + } +} + +/// Produces one signer's signature share in a two-group signing run. +/// +/// `group` states which group the signer belongs to; it is an explicit input +/// because the two identifier spaces may collide, so membership cannot be +/// inferred from `key_package.identifier`. The signer's Lagrange coefficient +/// is computed within its own group's participating set and folded into the +/// returned share. +pub fn sign( + signing_package: &TwoGroupSigningPackage, + signer_nonces: &round1::SigningNonces, + key_package: &keys::KeyPackage, + group: SignerGroup, +) -> Result, Error> { + let own_commitments = signing_package.signing_commitments(group); + + if own_commitments.len() < key_package.min_signers as usize { + return Err(Error::IncorrectNumberOfCommitments); + } + + let commitment = own_commitments + .get(&key_package.identifier) + .ok_or(Error::MissingCommitment)?; + if &signer_nonces.commitments != commitment { + return Err(Error::IncorrectCommitment); + } + + // Run the ciphersuite sign pre-processing (e.g. secp256k1-tr even-Y key + // normalization) with a synthetic single-group package; see the module + // docs for why this is sound. + let synthetic = SigningPackage::new(own_commitments.clone(), signing_package.message()); + let (_, signer_nonces, key_package) = ::pre_sign(&synthetic, signer_nonces, key_package)?; + + let binding_factors = signing_package.compute_binding_factors(&key_package.verifying_key)?; + let binding_factor = binding_factors + .get(&(group, key_package.identifier)) + .ok_or(Error::UnknownIdentifier)? + .clone(); + + let group_commitment = signing_package.compute_group_commitment(&binding_factors)?; + + let own_participants: BTreeSet> = own_commitments.keys().copied().collect(); + let lambda_i = compute_lagrange_coefficient(&own_participants, None, key_package.identifier)?; + + let challenge = ::challenge( + &group_commitment.0, + &key_package.verifying_key, + signing_package.message(), + )?; + + Ok(::compute_signature_share( + &group_commitment, + &signer_nonces, + binding_factor, + lambda_i, + &key_package, + challenge, + )) +} + +/// Aggregates both groups' signature shares into the final signature and +/// verifies it against `pubkeys.verifying_key` (the combined key both groups +/// signed under). +/// +/// `pubkeys` carries verifying shares for the **primary** group only. If the +/// aggregate does not verify, every primary share is checked individually and +/// the culprits reported via [`Error::InvalidSignatureShare`]; if all primary +/// shares verify, the failure lies with the secondary group (or with +/// inconsistent inputs), for which no per-signer verification material +/// exists, and [`Error::InvalidSignature`] is returned without blame. +pub fn aggregate( + signing_package: &TwoGroupSigningPackage, + primary_shares: &BTreeMap, round2::SignatureShare>, + secondary_shares: &BTreeMap, round2::SignatureShare>, + pubkeys: &keys::PublicKeyPackage, +) -> Result, Error> { + for (group, shares) in [ + (SignerGroup::Primary, primary_shares), + (SignerGroup::Secondary, secondary_shares), + ] { + let commitments = signing_package.signing_commitments(group); + if commitments.len() != shares.len() + || !commitments.keys().all(|id| shares.contains_key(id)) + { + return Err(Error::UnknownIdentifier); + } + } + if !signing_package + .signing_commitments(SignerGroup::Primary) + .keys() + .all(|id| pubkeys.verifying_shares.contains_key(id)) + { + return Err(Error::UnknownIdentifier); + } + + if let Some(min) = pubkeys.min_signers() { + if primary_shares.len() < min as usize { + return Err(Error::IncorrectNumberOfShares); + } + } + + // Run the ciphersuite aggregate pre-processing (e.g. secp256k1-tr even-Y + // public-key normalization) with a synthetic single-group package; see + // the module docs for why this is sound. + let synthetic = SigningPackage::new( + signing_package + .signing_commitments(SignerGroup::Primary) + .clone(), + signing_package.message(), + ); + let (_, primary_shares, pubkeys) = ::pre_aggregate(&synthetic, primary_shares, pubkeys)?; + + let binding_factors = signing_package.compute_binding_factors(&pubkeys.verifying_key)?; + let group_commitment = signing_package.compute_group_commitment(&binding_factors)?; + + let mut z = <::Field as Field>::zero(); + for share in primary_shares.values().chain(secondary_shares.values()) { + z = z + share.to_scalar(); + } + + let signature = Signature { + R: group_commitment.0, + z, + }; + + if pubkeys + .verifying_key + .verify(signing_package.message(), &signature) + .is_ok() + { + return Ok(signature); + } + + // The aggregate did not verify: check each primary share to assign blame. + let challenge = ::challenge( + &group_commitment.0, + &pubkeys.verifying_key, + signing_package.message(), + )?; + let primary_participants: BTreeSet> = signing_package + .signing_commitments(SignerGroup::Primary) + .keys() + .copied() + .collect(); + + let mut culprits = Vec::new(); + for (identifier, share) in primary_shares.iter() { + let commitment = signing_package + .signing_commitments(SignerGroup::Primary) + .get(identifier) + .ok_or(Error::UnknownIdentifier)?; + let binding_factor = binding_factors + .get(&(SignerGroup::Primary, *identifier)) + .ok_or(Error::UnknownIdentifier)?; + let verifying_share = pubkeys + .verifying_shares + .get(identifier) + .ok_or(Error::UnknownIdentifier)?; + let lambda_i = compute_lagrange_coefficient(&primary_participants, None, *identifier)?; + let commitment_share = commitment.to_group_commitment_share(binding_factor); + + if ::verify_share( + &group_commitment, + share, + *identifier, + &commitment_share, + verifying_share, + lambda_i, + &challenge, + ) + .is_err() + { + culprits.push(*identifier); + } + } + + if culprits.is_empty() { + Err(Error::InvalidSignature) + } else { + Err(Error::InvalidSignatureShare { culprits }) + } +} diff --git a/frost-secp256k1-tr/tests/two_group_tests.rs b/frost-secp256k1-tr/tests/two_group_tests.rs new file mode 100644 index 0000000..c7e6f41 --- /dev/null +++ b/frost-secp256k1-tr/tests/two_group_tests.rs @@ -0,0 +1,439 @@ +//! Tests for two-group signing with group-tagged binding factors +//! (`frost_core::two_group`) over the secp256k1-tr ciphersuite. + +use std::collections::BTreeMap; +use std::error::Error; + +use frost_core::two_group::{self, SignerGroup, TwoGroupSigningPackage}; +use frost_secp256k1_tr as frost; + +use frost::keys::{IdentifierList, KeyPackage, PublicKeyPackage, Tweak}; +use frost::round1::{SigningCommitments, SigningNonces}; +use frost::round2::SignatureShare; +use frost::{Identifier, VerifyingKey}; + +struct Groups { + primary_kps: BTreeMap, + secondary_kps: BTreeMap, + /// Primary verifying shares under the combined key, for aggregation. + pubkeys: PublicKeyPackage, + combined_vk: VerifyingKey, +} + +/// Generates two independent dealer groups whose secrets add up to the key +/// behind `combined_vk`, with every key package rewritten to carry the +/// combined verifying key (the key both groups jointly sign under). +fn make_groups( + primary: (u16, u16), + secondary: (u16, u16), + primary_ids: Option<&[Identifier]>, + secondary_ids: Option<&[Identifier]>, +) -> Result> { + let rng = rand::rngs::OsRng; + let id_list = |ids: Option<&[Identifier]>| match ids { + Some(ids) => IdentifierList::Custom(ids.to_vec().leak()), + None => IdentifierList::Default, + }; + let (p_shares, p_pub) = + frost::keys::generate_with_dealer(primary.0, primary.1, id_list(primary_ids), rng)?; + let (s_shares, s_pub) = + frost::keys::generate_with_dealer(secondary.0, secondary.1, id_list(secondary_ids), rng)?; + + let combined_vk = + VerifyingKey::new(p_pub.verifying_key().to_element() + s_pub.verifying_key().to_element()); + + let rebuild = |shares: BTreeMap, + min: u16| + -> Result, Box> { + shares + .into_iter() + .map(|(id, share)| { + let kp = KeyPackage::try_from(share)?; + Ok(( + id, + KeyPackage::new( + id, + *kp.signing_share(), + *kp.verifying_share(), + combined_vk, + min, + ), + )) + }) + .collect() + }; + + Ok(Groups { + primary_kps: rebuild(p_shares, primary.1)?, + secondary_kps: rebuild(s_shares, secondary.1)?, + pubkeys: PublicKeyPackage::new( + p_pub.verifying_shares().clone(), + combined_vk, + Some(primary.1), + ), + combined_vk, + }) +} + +type Round1 = ( + BTreeMap, + BTreeMap, +); + +fn commit_round(kps: &BTreeMap, signers: &[Identifier]) -> Round1 { + let mut rng = rand::rngs::OsRng; + let mut nonces_map = BTreeMap::new(); + let mut commitments_map = BTreeMap::new(); + for id in signers { + let (nonces, commitments) = frost::round1::commit(kps[id].signing_share(), &mut rng); + nonces_map.insert(*id, nonces); + commitments_map.insert(*id, commitments); + } + (nonces_map, commitments_map) +} + +fn sign_group( + package: &TwoGroupSigningPackage, + kps: &BTreeMap, + nonces: &BTreeMap, + group: SignerGroup, +) -> Result, Box> { + nonces + .iter() + .map(|(id, n)| Ok((*id, two_group::sign(package, n, &kps[id], group)?))) + .collect() +} + +fn ids(range: std::ops::RangeInclusive) -> Vec { + range + .map(|i| i.try_into().expect("nonzero identifier")) + .collect() +} + +#[test] +fn sign_and_aggregate_verifies() -> Result<(), Box> { + // Default identifier lists give BOTH groups identifiers 1..=n, so every + // run of this test also exercises cross-group identifier collisions. + // Iterate so both parities of the aggregate nonce R occur. + for _ in 0..8 { + let groups = make_groups((5, 3), (3, 2), None, None)?; + let message = b"two-group message"; + + let (p_nonces, p_commitments) = commit_round(&groups.primary_kps, &ids(1..=3)); + let (s_nonces, s_commitments) = commit_round(&groups.secondary_kps, &ids(1..=2)); + let package = TwoGroupSigningPackage::new(p_commitments, s_commitments, message)?; + + let p_shares = sign_group( + &package, + &groups.primary_kps, + &p_nonces, + SignerGroup::Primary, + )?; + let s_shares = sign_group( + &package, + &groups.secondary_kps, + &s_nonces, + SignerGroup::Secondary, + )?; + + let signature = two_group::aggregate(&package, &p_shares, &s_shares, &groups.pubkeys)?; + groups.combined_vk.verify(message, &signature)?; + } + Ok(()) +} + +#[test] +fn colliding_identifiers_get_distinct_tagged_binding_factors() -> Result<(), Box> { + let groups = make_groups((3, 2), (3, 2), None, None)?; + let message = b"collision message"; + + let (_, p_commitments) = commit_round(&groups.primary_kps, &ids(1..=2)); + let (_, s_commitments) = commit_round(&groups.secondary_kps, &ids(1..=2)); + let package = TwoGroupSigningPackage::new(p_commitments, s_commitments, message)?; + + let preimages: BTreeMap<_, _> = package + .binding_factor_preimages(&groups.combined_vk)? + .into_iter() + .collect(); + let id1: Identifier = 1u16.try_into()?; + let primary = &preimages[&(SignerGroup::Primary, id1)]; + let secondary = &preimages[&(SignerGroup::Secondary, id1)]; + + assert_eq!(primary.first(), Some(&0x00)); + assert_eq!(secondary.first(), Some(&0x01)); + // Same identifier, same commitment set — only the tag distinguishes them. + assert_eq!(primary[1..], secondary[1..]); + assert_ne!(primary, secondary); + Ok(()) +} + +#[test] +fn tampered_primary_shares_are_blamed() -> Result<(), Box> { + let groups = make_groups((5, 3), (3, 2), None, None)?; + let message = b"blame message"; + + let (p_nonces, p_commitments) = commit_round(&groups.primary_kps, &ids(1..=3)); + let (s_nonces, s_commitments) = commit_round(&groups.secondary_kps, &ids(1..=2)); + let package = TwoGroupSigningPackage::new(p_commitments, s_commitments, message)?; + + let mut p_shares = sign_group( + &package, + &groups.primary_kps, + &p_nonces, + SignerGroup::Primary, + )?; + let s_shares = sign_group( + &package, + &groups.secondary_kps, + &s_nonces, + SignerGroup::Secondary, + )?; + + let id1: Identifier = 1u16.try_into()?; + p_shares.insert(id1, corrupt(&p_shares[&id1])?); + + let err = two_group::aggregate(&package, &p_shares, &s_shares, &groups.pubkeys) + .expect_err("tampered primary share must not aggregate"); + match err { + frost::Error::InvalidSignatureShare { culprits } => { + assert_eq!(culprits, vec![id1]); + } + other => panic!("expected InvalidSignatureShare, got {other:?}"), + } + Ok(()) +} + +/// Returns a share whose scalar differs from the input's (flips the low byte). +fn corrupt(share: &SignatureShare) -> Result> { + let mut bytes = share.serialize(); + let last = bytes.last_mut().expect("share serialization is nonempty"); + *last = last.wrapping_add(1); + Ok(SignatureShare::deserialize(&bytes)?) +} + +#[test] +fn bad_secondary_share_fails_without_blame() -> Result<(), Box> { + let groups = make_groups((5, 3), (3, 2), None, None)?; + let message = b"secondary failure message"; + + let (p_nonces, p_commitments) = commit_round(&groups.primary_kps, &ids(1..=3)); + let (s_nonces, s_commitments) = commit_round(&groups.secondary_kps, &ids(1..=2)); + let package = TwoGroupSigningPackage::new(p_commitments, s_commitments, message)?; + + let p_shares = sign_group( + &package, + &groups.primary_kps, + &p_nonces, + SignerGroup::Primary, + )?; + let mut s_shares = sign_group( + &package, + &groups.secondary_kps, + &s_nonces, + SignerGroup::Secondary, + )?; + + let id1: Identifier = 1u16.try_into()?; + s_shares.insert(id1, corrupt(&s_shares[&id1])?); + + // No per-signer verification material exists for the secondary group, so + // the failure is reported without blame. + let err = two_group::aggregate(&package, &p_shares, &s_shares, &groups.pubkeys) + .expect_err("tampered secondary shares must not aggregate"); + assert!(matches!(err, frost::Error::InvalidSignature)); + Ok(()) +} + +#[test] +fn divergent_primary_commitment_view_fails_loudly() -> Result<(), Box> { + let groups = make_groups((5, 3), (3, 2), None, None)?; + let message = b"divergent view message"; + + let (p_nonces, p_commitments) = commit_round(&groups.primary_kps, &ids(1..=3)); + let (s_nonces, s_commitments) = commit_round(&groups.secondary_kps, &ids(1..=2)); + let package = + TwoGroupSigningPackage::new(p_commitments.clone(), s_commitments.clone(), message)?; + + let p_shares = sign_group( + &package, + &groups.primary_kps, + &p_nonces, + SignerGroup::Primary, + )?; + + // One secondary signer binds a different primary commitment set (its own + // freshly resampled commitment for primary signer 1). + let id1: Identifier = 1u16.try_into()?; + let id2: Identifier = 2u16.try_into()?; + let mut divergent_p_commitments = p_commitments; + let (_, resampled) = frost::round1::commit( + groups.primary_kps[&id1].signing_share(), + &mut rand::rngs::OsRng, + ); + divergent_p_commitments.insert(id1, resampled); + let divergent_package = + TwoGroupSigningPackage::new(divergent_p_commitments, s_commitments, message)?; + + let mut s_shares = BTreeMap::new(); + s_shares.insert( + id1, + two_group::sign( + &divergent_package, + &s_nonces[&id1], + &groups.secondary_kps[&id1], + SignerGroup::Secondary, + )?, + ); + s_shares.insert( + id2, + two_group::sign( + &package, + &s_nonces[&id2], + &groups.secondary_kps[&id2], + SignerGroup::Secondary, + )?, + ); + + // All primary shares verify against the true package, so the divergence + // is detected by the aggregate check and reported without blame. + let err = two_group::aggregate(&package, &p_shares, &s_shares, &groups.pubkeys) + .expect_err("divergent commitment views must not aggregate"); + assert!(matches!(err, frost::Error::InvalidSignature)); + Ok(()) +} + +#[test] +fn tweaked_two_group_sign_and_aggregate_verifies() -> Result<(), Box> { + // Mirrors the deployed taproot convention: the taptweak scalar enters the + // key sum exactly once, on the secondary (user) side, which calls + // `tweak()` at signing time on a key package carrying the untweaked + // combined key. Primary (operator) key packages are pre-normalized to the + // untweaked combined key's parity and carry the tweaked combined key, and + // sign without further tweaking. The aggregation public-key package holds + // the tweaked combined key and the primary verifying shares exactly as + // the primary signers signed (pre-normalized, no tweak). + use frost::keys::{EvenY, VerifyingShare}; + + let merkle_root: Vec = vec![]; + for _ in 0..4 { + let groups = make_groups((5, 3), (3, 2), None, None)?; + let message = b"tweaked two-group message"; + + let untweaked_vk = groups.combined_vk; + let tweaked_vk = *PublicKeyPackage::new(BTreeMap::new(), untweaked_vk, None) + .tweak(Some(&merkle_root)) + .verifying_key(); + + let mut primary_kps = BTreeMap::new(); + let mut primary_verifying_shares: BTreeMap = BTreeMap::new(); + for (id, kp) in &groups.primary_kps { + let normalized = kp.clone().into_even_y(Some(untweaked_vk.has_even_y())); + primary_verifying_shares.insert(*id, *normalized.verifying_share()); + primary_kps.insert( + *id, + KeyPackage::new( + *id, + *normalized.signing_share(), + *normalized.verifying_share(), + tweaked_vk, + *kp.min_signers(), + ), + ); + } + + let (p_nonces, p_commitments) = commit_round(&primary_kps, &ids(1..=3)); + let (s_nonces, s_commitments) = commit_round(&groups.secondary_kps, &ids(1..=2)); + let package = TwoGroupSigningPackage::new(p_commitments, s_commitments, message)?; + + let p_shares = sign_group(&package, &primary_kps, &p_nonces, SignerGroup::Primary)?; + let tweaked_secondary_kps = groups + .secondary_kps + .iter() + .map(|(id, kp)| (*id, kp.clone().tweak(Some(&merkle_root)))) + .collect::>(); + let s_shares = sign_group( + &package, + &tweaked_secondary_kps, + &s_nonces, + SignerGroup::Secondary, + )?; + + let pubkeys = PublicKeyPackage::new(primary_verifying_shares, tweaked_vk, Some(3)); + let signature = two_group::aggregate(&package, &p_shares, &s_shares, &pubkeys)?; + tweaked_vk.verify(message, &signature)?; + + untweaked_vk + .verify(message, &signature) + .expect_err("signature must not verify under the untweaked key"); + } + Ok(()) +} + +#[test] +fn tagged_scheme_differs_from_flat_participant_groups() -> Result<(), Box> { + // With collision-free identifiers the flat participant-groups scheme (the + // deployed nested-signing path) can sign the same message with the same + // nonces; the group tag must still change the binding factors, so the two + // schemes must produce different signatures (both valid). + let secondary_ids = ids(101..=103); + let groups = make_groups((3, 2), (3, 2), None, Some(&secondary_ids))?; + let message = b"scheme divergence message"; + + let p_signers = ids(1..=2); + let s_signers = ids(101..=102); + let (p_nonces, p_commitments) = commit_round(&groups.primary_kps, &p_signers); + let (s_nonces, s_commitments) = commit_round(&groups.secondary_kps, &s_signers); + + // Tagged two-group signature. + let package = + TwoGroupSigningPackage::new(p_commitments.clone(), s_commitments.clone(), message)?; + let p_shares = sign_group( + &package, + &groups.primary_kps, + &p_nonces, + SignerGroup::Primary, + )?; + let s_shares = sign_group( + &package, + &groups.secondary_kps, + &s_nonces, + SignerGroup::Secondary, + )?; + let tagged_signature = two_group::aggregate(&package, &p_shares, &s_shares, &groups.pubkeys)?; + + // Flat participant-groups signature over the same commitments and nonces. + let mut flat_commitments = p_commitments; + flat_commitments.extend(s_commitments); + let flat_package = frost::SigningPackage::new_with_participants_groups( + flat_commitments, + Some(vec![ + p_signers.iter().copied().collect(), + s_signers.iter().copied().collect(), + ]), + message, + ); + let mut flat_shares = BTreeMap::new(); + for (id, nonces) in p_nonces.iter().chain(s_nonces.iter()) { + let kps = if p_nonces.contains_key(id) { + &groups.primary_kps + } else { + &groups.secondary_kps + }; + flat_shares.insert(*id, frost::round2::sign(&flat_package, nonces, &kps[id])?); + } + let mut all_verifying_shares = groups.pubkeys.verifying_shares().clone(); + for (id, kp) in &groups.secondary_kps { + all_verifying_shares.insert(*id, *kp.verifying_share()); + } + let flat_pubkeys = PublicKeyPackage::new(all_verifying_shares, groups.combined_vk, None); + let flat_signature = frost::aggregate(&flat_package, &flat_shares, &flat_pubkeys)?; + + assert_ne!( + tagged_signature.serialize()?, + flat_signature.serialize()?, + "the group tag must change the binding factors and hence the signature" + ); + groups.combined_vk.verify(message, &tagged_signature)?; + groups.combined_vk.verify(message, &flat_signature)?; + Ok(()) +} From 038f116c3dc4606f45f9f3fec9f89ff5dbfd4bd5 Mon Sep 17 00:00:00 2001 From: Keenan Breik Date: Thu, 6 Aug 2026 09:59:04 -0700 Subject: [PATCH 2/2] Cover even-Y parity branches by construction in two-group tests The two sign+aggregate tests looped a fixed number of rounds and relied on random nonces to hit both parities of the aggregate nonce R, without checking that they had, so they could pass while claiming coverage they did not get. Loop until every parity case has actually occurred instead: a parity-specific regression now fails deterministically rather than at 1 - 2^(1-n), and the tests finish sooner because they stop as soon as they are covered. The tweaked test waits on a four-case grid, since the untweaked combined key's parity decides the primary key-package normalization and flips per round too. Co-Authored-By: Claude Opus 5 (1M context) --- frost-secp256k1-tr/tests/two_group_tests.rs | 39 +++++++++++++++------ 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/frost-secp256k1-tr/tests/two_group_tests.rs b/frost-secp256k1-tr/tests/two_group_tests.rs index c7e6f41..db58d64 100644 --- a/frost-secp256k1-tr/tests/two_group_tests.rs +++ b/frost-secp256k1-tr/tests/two_group_tests.rs @@ -1,13 +1,13 @@ //! Tests for two-group signing with group-tagged binding factors //! (`frost_core::two_group`) over the secp256k1-tr ciphersuite. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::error::Error; use frost_core::two_group::{self, SignerGroup, TwoGroupSigningPackage}; use frost_secp256k1_tr as frost; -use frost::keys::{IdentifierList, KeyPackage, PublicKeyPackage, Tweak}; +use frost::keys::{EvenY, IdentifierList, KeyPackage, PublicKeyPackage, Tweak}; use frost::round1::{SigningCommitments, SigningNonces}; use frost::round2::SignatureShare; use frost::{Identifier, VerifyingKey}; @@ -110,12 +110,30 @@ fn ids(range: std::ops::RangeInclusive) -> Vec { .collect() } +/// Each round's parities are random, so coverage of the even-Y branches has to +/// be waited for rather than assumed. +fn sign_until_parity_cases_seen( + cases: usize, + mut round: impl FnMut() -> Result>, +) -> Result<(), Box> { + let mut seen = BTreeSet::new(); + for _ in 0..256 { + seen.insert(round()?); + if seen.len() == cases { + return Ok(()); + } + } + panic!( + "only {} of {cases} parity cases occurred in 256 rounds", + seen.len() + ); +} + #[test] fn sign_and_aggregate_verifies() -> Result<(), Box> { // Default identifier lists give BOTH groups identifiers 1..=n, so every // run of this test also exercises cross-group identifier collisions. - // Iterate so both parities of the aggregate nonce R occur. - for _ in 0..8 { + sign_until_parity_cases_seen(2, || { let groups = make_groups((5, 3), (3, 2), None, None)?; let message = b"two-group message"; @@ -138,8 +156,8 @@ fn sign_and_aggregate_verifies() -> Result<(), Box> { let signature = two_group::aggregate(&package, &p_shares, &s_shares, &groups.pubkeys)?; groups.combined_vk.verify(message, &signature)?; - } - Ok(()) + Ok(signature.has_even_y()) + }) } #[test] @@ -312,10 +330,10 @@ fn tweaked_two_group_sign_and_aggregate_verifies() -> Result<(), Box> // sign without further tweaking. The aggregation public-key package holds // the tweaked combined key and the primary verifying shares exactly as // the primary signers signed (pre-normalized, no tweak). - use frost::keys::{EvenY, VerifyingShare}; + use frost::keys::VerifyingShare; let merkle_root: Vec = vec![]; - for _ in 0..4 { + sign_until_parity_cases_seen(4, || { let groups = make_groups((5, 3), (3, 2), None, None)?; let message = b"tweaked two-group message"; @@ -365,8 +383,9 @@ fn tweaked_two_group_sign_and_aggregate_verifies() -> Result<(), Box> untweaked_vk .verify(message, &signature) .expect_err("signature must not verify under the untweaked key"); - } - Ok(()) + + Ok((untweaked_vk.has_even_y(), signature.has_even_y())) + }) } #[test]