From f3ab6b55b9bea6e8301b51c768a1175f7cdbaf92 Mon Sep 17 00:00:00 2001 From: maclane Date: Sat, 13 Jun 2026 02:32:36 -0400 Subject: [PATCH 1/4] feat(tbtc/signer): Phase 7.2a InteractiveAggregate with attributable blame Coordinator-side InteractiveAggregate per the frozen spec: collect the responsive subset's signature shares, verify each against its verifying share, and produce the BIP-340 signature - then self-verify it against the (taproot-tweaked) group key before release, matching the coarse finalize path. Verifying material is resolved from the session's own DKG public key package, never the request, consistent with the no-secret-on-the-FFI discipline (the session must exist with completed DKG). Aggregation operates on public material only, so no policy gate runs here: the secret-bearing step is each signer's Round2, where lifecycle / quarantine (full subset) / firewall were already enforced. frost::aggregate already verifies every share and reports the culprit identifiers on failure; instead of flattening that to a string, a bad share now surfaces as the structured EngineError::InvalidSignatureShare { culprits } (code invalid_signature_share, recoverable) naming the offending member(s), so the coordinator has attributable blame and can exclude them on the next attempt. FFI export frost_tbtc_interactive_aggregate + C header declaration; call/success counters and latency telemetry consistent with the other interactive ops. Tests: interactive member + stateless member shares aggregate to a verified BIP-340 signature through the engine; a corrupt co-signer share fails with attributable blame naming the culprit; FFI dispatch smoke. Full suite 268 passed / 1 ignored, clippy -D warnings clean, header parses, chaos green. Deferred to 7.2b (needs persistence plumbing, not security-load-bearing since aggregate is deterministic over public data): the "mark session complete" marker, plus the signed-body package envelopes and cross-language vectors. Co-Authored-By: Claude Fable 5 --- pkg/tbtc/signer/include/frost_tbtc.h | 8 + pkg/tbtc/signer/src/api.rs | 32 ++++ pkg/tbtc/signer/src/engine/interactive.rs | 131 ++++++++++++++ pkg/tbtc/signer/src/engine/mod.rs | 25 +-- pkg/tbtc/signer/src/engine/telemetry.rs | 22 ++- pkg/tbtc/signer/src/engine/tests.rs | 205 +++++++++++++++++++++- pkg/tbtc/signer/src/errors.rs | 15 ++ pkg/tbtc/signer/src/lib.rs | 43 ++++- 8 files changed, 460 insertions(+), 21 deletions(-) diff --git a/pkg/tbtc/signer/include/frost_tbtc.h b/pkg/tbtc/signer/include/frost_tbtc.h index 74bfc078ed..aed2583ac5 100644 --- a/pkg/tbtc/signer/include/frost_tbtc.h +++ b/pkg/tbtc/signer/include/frost_tbtc.h @@ -72,6 +72,14 @@ TbtcSignerResult frost_tbtc_interactive_session_open(const uint8_t* request_ptr, TbtcSignerResult frost_tbtc_interactive_round1(const uint8_t* request_ptr, size_t request_len); TbtcSignerResult frost_tbtc_interactive_round2(const uint8_t* request_ptr, size_t request_len); TbtcSignerResult frost_tbtc_interactive_session_abort(const uint8_t* request_ptr, size_t request_len); +/* + * Coordinator-side aggregation: verifies each collected signature share + * against its verifying share (resolved from the session's DKG state) and, + * on failure, reports the culprit member(s) as attributable blame + * (`invalid_signature_share`); otherwise returns the aggregated BIP-340 + * signature. Operates on public material only - no secret crosses here. + */ +TbtcSignerResult frost_tbtc_interactive_aggregate(const uint8_t* request_ptr, size_t request_len); #ifdef __cplusplus } diff --git a/pkg/tbtc/signer/src/api.rs b/pkg/tbtc/signer/src/api.rs index d02f2bcb89..a1fe1ca453 100644 --- a/pkg/tbtc/signer/src/api.rs +++ b/pkg/tbtc/signer/src/api.rs @@ -212,6 +212,30 @@ pub struct InteractiveRound2Result { pub signature_share_hex: String, } +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)] +pub struct InteractiveAggregateRequest { + pub session_id: String, + pub attempt_id: String, + /// The signing package the shares were produced over (carries the + /// message and the chosen subset's commitments). + pub signing_package_hex: String, + /// The collected signature shares from the responsive subset. Each + /// is verified against the member's verifying share (resolved from + /// the session's DKG public key package) before aggregation; an + /// invalid share yields attributable blame naming the culprit. + pub signature_shares: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub taproot_merkle_root_hex: Option, +} + +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)] +pub struct InteractiveAggregateResult { + pub session_id: String, + pub attempt_id: String, + /// The aggregated BIP-340 Schnorr signature, hex-encoded. + pub signature_hex: String, +} + #[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)] pub struct InteractiveSessionAbortRequest { pub session_id: String, @@ -619,6 +643,10 @@ pub struct SignerHardeningMetricsResult { #[serde(default)] pub interactive_session_abort_success_total: u64, #[serde(default)] + pub interactive_aggregate_calls_total: u64, + #[serde(default)] + pub interactive_aggregate_success_total: u64, + #[serde(default)] pub interactive_round1_latency_p95_ms: u64, #[serde(default)] pub interactive_round1_latency_samples: u64, @@ -626,6 +654,10 @@ pub struct SignerHardeningMetricsResult { pub interactive_round2_latency_p95_ms: u64, #[serde(default)] pub interactive_round2_latency_samples: u64, + #[serde(default)] + pub interactive_aggregate_latency_p95_ms: u64, + #[serde(default)] + pub interactive_aggregate_latency_samples: u64, pub last_updated_unix: u64, } diff --git a/pkg/tbtc/signer/src/engine/interactive.rs b/pkg/tbtc/signer/src/engine/interactive.rs index 690870452e..bc60a9eb6a 100644 --- a/pkg/tbtc/signer/src/engine/interactive.rs +++ b/pkg/tbtc/signer/src/engine/interactive.rs @@ -561,6 +561,137 @@ pub fn interactive_round2( }) } +pub fn interactive_aggregate( + request: InteractiveAggregateRequest, +) -> Result { + record_hardening_telemetry(|telemetry| { + telemetry.interactive_aggregate_calls_total = telemetry + .interactive_aggregate_calls_total + .saturating_add(1); + }); + let _latency_guard = + HardeningOperationLatencyGuard::new(HardeningOperation::InteractiveAggregate); + enforce_provenance_gate()?; + validate_session_id(&request.session_id)?; + let attempt_id = canonical_attempt_id(&request.attempt_id); + + let mut signing_package_bytes = decode_hex_field( + "InteractiveAggregate", + "signing_package_hex", + &request.signing_package_hex, + )?; + let signing_package_result = frost::SigningPackage::deserialize(&signing_package_bytes); + signing_package_bytes.zeroize(); + let signing_package = signing_package_result.map_err(|e| { + EngineError::Validation(format!( + "InteractiveAggregate: invalid signing package: {e}" + )) + })?; + let signature_shares = + decode_signature_share_map("InteractiveAggregate", &request.signature_shares)?; + let mut taproot_merkle_root_hex = request.taproot_merkle_root_hex.clone(); + let taproot_merkle_root = canonicalize_taproot_merkle_root_hex(&mut taproot_merkle_root_hex)?; + + let guard = state()? + .lock() + .map_err(|_| EngineError::Internal("engine lock poisoned".to_string()))?; + + // Resolve the group's public key package (the verifying shares used + // to check each contribution) from the session's own DKG state, not + // the request - consistent with the no-secret-on-the-FFI discipline + // and so a caller cannot substitute verifying material. The session + // must exist with completed DKG. + let public_key_package = { + let session = guard.sessions.get(&request.session_id).ok_or_else(|| { + EngineError::SessionNotFound { + session_id: request.session_id.clone(), + } + })?; + if session.dkg_result.is_none() { + return Err(EngineError::DkgNotReady { + session_id: request.session_id.clone(), + }); + } + session + .dkg_public_key_package + .as_ref() + .ok_or_else(|| { + EngineError::Internal("missing DKG public key package cache".to_string()) + })? + .clone() + }; + drop(guard); + + // Aggregation uses only public material (commitments, shares, + // verifying shares), so no policy gate runs here - the secret-bearing + // step is each signer's Round2, where lifecycle/quarantine/firewall + // were already enforced (including the full-subset quarantine check). + // frost verifies every share against its verifying share and reports + // the culprits on failure; surface them as attributable blame. + let verification_key_package = match taproot_merkle_root.as_ref() { + Some(root) => public_key_package.clone().tweak(Some(root.as_slice())), + None => public_key_package.clone(), + }; + + let aggregate_result = match taproot_merkle_root.as_ref() { + Some(root) => frost::aggregate_with_tweak( + &signing_package, + &signature_shares, + &public_key_package, + Some(root.as_slice()), + ), + None => frost::aggregate(&signing_package, &signature_shares, &public_key_package), + }; + let signature = aggregate_result + .map_err(|error| map_aggregate_error_to_blame(&request.session_id, error))?; + + // Self-verify the aggregate against the (tweaked) group verifying + // key before releasing it, matching the coarse finalize path. + verification_key_package + .verifying_key() + .verify(signing_package.message().as_slice(), &signature) + .map_err(|e| { + EngineError::Validation(format!( + "InteractiveAggregate: aggregate signature failed self-verification: {e}" + )) + })?; + + let signature_bytes = signature + .serialize() + .map_err(|e| EngineError::Internal(format!("failed to serialize aggregate: {e}")))?; + + record_hardening_telemetry(|telemetry| { + telemetry.interactive_aggregate_success_total = telemetry + .interactive_aggregate_success_total + .saturating_add(1); + }); + + Ok(InteractiveAggregateResult { + session_id: request.session_id, + attempt_id, + signature_hex: hex::encode(signature_bytes), + }) +} + +// Convert a frost aggregation error into an attributable blame error +// when it identifies culprit shares, so the coordinator can exclude the +// offending member(s) on the next attempt. Other failures map to a +// generic validation error. +fn map_aggregate_error_to_blame(session_id: &str, error: frost::Error) -> EngineError { + if let frost::Error::InvalidSignatureShare { culprits } = error { + return EngineError::InvalidSignatureShare { + session_id: session_id.to_string(), + culprits: culprits + .into_iter() + .map(frost_identifier_to_go_string) + .collect(), + }; + } + EngineError::Validation(format!( + "InteractiveAggregate: failed to aggregate: {error}" + )) +} + pub fn interactive_session_abort( request: InteractiveSessionAbortRequest, ) -> Result { diff --git a/pkg/tbtc/signer/src/engine/mod.rs b/pkg/tbtc/signer/src/engine/mod.rs index ff2854c950..ffa4d695f9 100644 --- a/pkg/tbtc/signer/src/engine/mod.rs +++ b/pkg/tbtc/signer/src/engine/mod.rs @@ -69,18 +69,19 @@ use crate::api::{ DkgPart2Request, DkgPart2Result, DkgPart3Request, DkgPart3Result, DkgResult, DkgRound1Package, DkgRound2Package, FinalizeSignRoundRequest, GenerateNoncesAndCommitmentsRequest, GenerateNoncesAndCommitmentsResult, InitSignerConfigRequest, InitSignerConfigResult, - InteractiveRound1Request, InteractiveRound1Result, InteractiveRound2Request, - InteractiveRound2Result, InteractiveSessionAbortRequest, InteractiveSessionAbortResult, - InteractiveSessionOpenRequest, InteractiveSessionOpenResult, NativeFrostCommitment, - NativeFrostKeyPackage, NativeFrostPublicKeyPackage, NativeFrostSignatureShare, - NewSigningPackageRequest, NewSigningPackageResult, PromoteCanaryRequest, PromoteCanaryResult, - QuarantineStatusRequest, QuarantineStatusResult, RefreshCadenceStatusRequest, - RefreshCadenceStatusResult, RefreshSharesRequest, RefreshSharesResult, - RoastLivenessPolicyResult, RollbackCanaryRequest, RollbackCanaryResult, RoundContribution, - RoundState, RunDkgRequest, ShareMaterial, SignShareRequest, SignShareResult, SignatureResult, - SignerHardeningMetricsResult, StartSignRoundRequest, TransactionResult, TranscriptAuditRecord, - TranscriptAuditRequest, TranscriptAuditResult, TriggerEmergencyRekeyRequest, - TriggerEmergencyRekeyResult, VerifyBlameProofRequest, + InteractiveAggregateRequest, InteractiveAggregateResult, InteractiveRound1Request, + InteractiveRound1Result, InteractiveRound2Request, InteractiveRound2Result, + InteractiveSessionAbortRequest, InteractiveSessionAbortResult, InteractiveSessionOpenRequest, + InteractiveSessionOpenResult, NativeFrostCommitment, NativeFrostKeyPackage, + NativeFrostPublicKeyPackage, NativeFrostSignatureShare, NewSigningPackageRequest, + NewSigningPackageResult, PromoteCanaryRequest, PromoteCanaryResult, QuarantineStatusRequest, + QuarantineStatusResult, RefreshCadenceStatusRequest, RefreshCadenceStatusResult, + RefreshSharesRequest, RefreshSharesResult, RoastLivenessPolicyResult, RollbackCanaryRequest, + RollbackCanaryResult, RoundContribution, RoundState, RunDkgRequest, ShareMaterial, + SignShareRequest, SignShareResult, SignatureResult, SignerHardeningMetricsResult, + StartSignRoundRequest, TransactionResult, TranscriptAuditRecord, TranscriptAuditRequest, + TranscriptAuditResult, TriggerEmergencyRekeyRequest, TriggerEmergencyRekeyResult, + VerifyBlameProofRequest, }; use crate::errors::EngineError; use crate::go_math_rand::select_coordinator_identifier; diff --git a/pkg/tbtc/signer/src/engine/telemetry.rs b/pkg/tbtc/signer/src/engine/telemetry.rs index 054d25dd05..749f28715d 100644 --- a/pkg/tbtc/signer/src/engine/telemetry.rs +++ b/pkg/tbtc/signer/src/engine/telemetry.rs @@ -69,6 +69,8 @@ pub(crate) struct HardeningTelemetryState { pub(crate) interactive_round2_success_total: u64, pub(crate) interactive_session_abort_calls_total: u64, pub(crate) interactive_session_abort_success_total: u64, + pub(crate) interactive_aggregate_calls_total: u64, + pub(crate) interactive_aggregate_success_total: u64, pub(crate) run_dkg_latency: HardeningLatencyTracker, pub(crate) start_sign_round_latency: HardeningLatencyTracker, pub(crate) build_taproot_tx_latency: HardeningLatencyTracker, @@ -76,6 +78,7 @@ pub(crate) struct HardeningTelemetryState { pub(crate) refresh_shares_latency: HardeningLatencyTracker, pub(crate) interactive_round1_latency: HardeningLatencyTracker, pub(crate) interactive_round2_latency: HardeningLatencyTracker, + pub(crate) interactive_aggregate_latency: HardeningLatencyTracker, pub(crate) last_updated_unix: u64, } @@ -87,10 +90,11 @@ pub(crate) enum HardeningOperation { FinalizeSignRound, RefreshShares, // Interactive Open/Abort are O(1) registry mutations and record - // call/success counters only; the two cryptographic rounds get - // latency tracking. + // call/success counters only; the cryptographic rounds and the + // aggregation get latency tracking. InteractiveRound1, InteractiveRound2, + InteractiveAggregate, } pub(crate) struct HardeningOperationLatencyGuard { @@ -155,6 +159,9 @@ pub(crate) fn record_hardening_operation_latency(operation: HardeningOperation, HardeningOperation::InteractiveRound2 => { telemetry.interactive_round2_latency.record(duration_ms) } + HardeningOperation::InteractiveAggregate => { + telemetry.interactive_aggregate_latency.record(duration_ms) + } }); } @@ -209,10 +216,14 @@ pub fn hardening_metrics() -> SignerHardeningMetricsResult { interactive_round2_success_total: 0, interactive_session_abort_calls_total: 0, interactive_session_abort_success_total: 0, + interactive_aggregate_calls_total: 0, + interactive_aggregate_success_total: 0, interactive_round1_latency_p95_ms: 0, interactive_round1_latency_samples: 0, interactive_round2_latency_p95_ms: 0, interactive_round2_latency_samples: 0, + interactive_aggregate_latency_p95_ms: 0, + interactive_aggregate_latency_samples: 0, last_updated_unix: 0, }; @@ -274,6 +285,9 @@ pub fn hardening_metrics() -> SignerHardeningMetricsResult { telemetry.interactive_session_abort_calls_total; result.interactive_session_abort_success_total = telemetry.interactive_session_abort_success_total; + result.interactive_aggregate_calls_total = telemetry.interactive_aggregate_calls_total; + result.interactive_aggregate_success_total = + telemetry.interactive_aggregate_success_total; result.interactive_round1_latency_p95_ms = telemetry.interactive_round1_latency.p95_ms(); result.interactive_round1_latency_samples = @@ -282,6 +296,10 @@ pub fn hardening_metrics() -> SignerHardeningMetricsResult { telemetry.interactive_round2_latency.p95_ms(); result.interactive_round2_latency_samples = telemetry.interactive_round2_latency.sample_count(); + result.interactive_aggregate_latency_p95_ms = + telemetry.interactive_aggregate_latency.p95_ms(); + result.interactive_aggregate_latency_samples = + telemetry.interactive_aggregate_latency.sample_count(); result.last_updated_unix = telemetry.last_updated_unix; } Err(error) => { diff --git a/pkg/tbtc/signer/src/engine/tests.rs b/pkg/tbtc/signer/src/engine/tests.rs index 4f852670ec..01a08af0c4 100644 --- a/pkg/tbtc/signer/src/engine/tests.rs +++ b/pkg/tbtc/signer/src/engine/tests.rs @@ -11178,7 +11178,18 @@ fn ensure_interactive_dkg_session( session_id: &str, key_group: &str, ) -> BTreeMap { - let native = interactive_test_key_packages(); + let fixture = deterministic_interactive_dkg_fixture(0); + let mut native = BTreeMap::new(); + let mut public_key_package_native = None; + for (id, request) in fixture.part3_requests { + let result = dkg_part3(request).expect("DKG part3 for fixture"); + if public_key_package_native.is_none() { + public_key_package_native = Some(result.public_key_package.clone()); + } + native.insert(id, result.key_package); + } + let public_key_package_native = + public_key_package_native.expect("fixture has at least one participant"); let mut guard = state().expect("engine state").lock().expect("engine lock"); let session = guard.sessions.entry(session_id.to_string()).or_default(); @@ -11191,6 +11202,9 @@ fn ensure_interactive_dkg_session( .expect("fixture key package deserializes"); frost_key_packages.insert(*id, deserialized); } + let public_key_package = + native_public_key_package_to_frost("interactive-dkg-seed", &public_key_package_native) + .expect("fixture public key package converts"); session.dkg_result = Some(DkgResult { session_id: session_id.to_string(), key_group: key_group.to_string(), @@ -11199,6 +11213,7 @@ fn ensure_interactive_dkg_session( created_at_unix: now_unix(), }); session.dkg_key_packages = Some(frost_key_packages); + session.dkg_public_key_package = Some(public_key_package); } native @@ -12734,3 +12749,191 @@ fn interactive_open_rejects_phantom_included_participant() { "unexpected error: {err:?}" ); } + +#[test] +fn interactive_aggregate_produces_and_self_verifies_bip340() { + let _guard = lock_test_state(); + reset_for_tests(); + + let session_id = "interactive-aggregate-e2e"; + let key_group = "interactive-test-key-group"; + let message = [0x4au8; 32]; + let included = [1u16, 2]; + let key_packages = ensure_interactive_dkg_session(session_id, key_group); + + // Member 1 signs through the hardened session API; member 2 through + // the stateless primitive. Both shares feed the coordinator's + // InteractiveAggregate, which resolves the verifying shares from the + // session's own DKG state. + let opened = open_interactive_for_test(session_id, key_group, &message, &included, 1, 1, 2) + .expect("opens"); + let round1 = interactive_round1(InteractiveRound1Request { + session_id: session_id.to_string(), + attempt_id: opened.attempt_id.clone(), + member_identifier: 1, + }) + .expect("round 1"); + let member2 = generate_nonces_and_commitments(GenerateNoncesAndCommitmentsRequest { + key_package_identifier: key_packages[&2].identifier.clone(), + key_package_hex: key_packages[&2].data_hex.clone(), + }) + .expect("member 2 nonces"); + let signing_package_hex = interactive_package_for_test( + &message, + vec![ + NativeFrostCommitment { + identifier: key_packages[&1].identifier.clone(), + data_hex: round1.commitments_hex, + }, + member2.commitment.clone(), + ], + ); + let round2 = interactive_round2(InteractiveRound2Request { + session_id: session_id.to_string(), + attempt_id: opened.attempt_id.clone(), + member_identifier: 1, + signing_package_hex: signing_package_hex.clone(), + }) + .expect("round 2 share"); + let member2_share = sign_share(SignShareRequest { + signing_package_hex: signing_package_hex.clone(), + nonces_hex: member2.nonces_hex, + key_package_identifier: key_packages[&2].identifier.clone(), + key_package_hex: key_packages[&2].data_hex.clone(), + }) + .expect("member 2 share"); + + let aggregate = interactive_aggregate(InteractiveAggregateRequest { + session_id: session_id.to_string(), + attempt_id: opened.attempt_id.clone(), + signing_package_hex, + signature_shares: vec![ + crate::api::NativeFrostSignatureShare { + identifier: key_packages[&1].identifier.clone(), + data_hex: round2.signature_share_hex, + }, + member2_share.signature_share, + ], + taproot_merkle_root_hex: None, + }) + .expect("interactive aggregate"); + assert_eq!(aggregate.attempt_id, opened.attempt_id); + + // The engine already self-verified; re-verify here against the DKG + // group key to pin the round trip. + let public_key_package = { + let guard = state().expect("state").lock().expect("lock"); + guard + .sessions + .get(session_id) + .expect("session") + .dkg_public_key_package + .clone() + .expect("public key package") + }; + let verifying_key_bytes = public_key_package.verifying_key().serialize().expect("vk"); + let signature_bytes = hex::decode(aggregate.signature_hex).expect("sig hex"); + let signature = SchnorrSignature::from_slice(&signature_bytes).expect("BIP340 signature"); + let public_key = XOnlyPublicKey::from_slice(&verifying_key_bytes[1..]).expect("x-only key"); + Secp256k1::verification_only() + .verify_schnorr(&signature, &SecpMessage::from_digest(message), &public_key) + .expect("interactive aggregate yields a valid BIP-340 signature"); +} + +#[test] +fn interactive_aggregate_blames_invalid_share_culprit() { + let _guard = lock_test_state(); + reset_for_tests(); + + let session_id = "interactive-aggregate-blame"; + let key_group = "interactive-test-key-group"; + let message = [0x4bu8; 32]; + let included = [1u16, 2]; + let key_packages = ensure_interactive_dkg_session(session_id, key_group); + + let opened = open_interactive_for_test(session_id, key_group, &message, &included, 1, 1, 2) + .expect("opens"); + let round1 = interactive_round1(InteractiveRound1Request { + session_id: session_id.to_string(), + attempt_id: opened.attempt_id.clone(), + member_identifier: 1, + }) + .expect("round 1"); + let member2 = generate_nonces_and_commitments(GenerateNoncesAndCommitmentsRequest { + key_package_identifier: key_packages[&2].identifier.clone(), + key_package_hex: key_packages[&2].data_hex.clone(), + }) + .expect("member 2 nonces"); + let signing_package_hex = interactive_package_for_test( + &message, + vec![ + NativeFrostCommitment { + identifier: key_packages[&1].identifier.clone(), + data_hex: round1.commitments_hex, + }, + member2.commitment, + ], + ); + let round2 = interactive_round2(InteractiveRound2Request { + session_id: session_id.to_string(), + attempt_id: opened.attempt_id.clone(), + member_identifier: 1, + signing_package_hex: signing_package_hex.clone(), + }) + .expect("round 2 share"); + + // Member 2 contributes a structurally valid but WRONG share (a fresh + // share over a different signing package). Aggregation must fail with + // attributable blame naming member 2, not an opaque error. + let bogus_member2 = generate_nonces_and_commitments(GenerateNoncesAndCommitmentsRequest { + key_package_identifier: key_packages[&2].identifier.clone(), + key_package_hex: key_packages[&2].data_hex.clone(), + }) + .expect("bogus member 2 nonces"); + let other_message = [0x4cu8; 32]; + let other_package = interactive_package_for_test( + &other_message, + vec![ + NativeFrostCommitment { + identifier: key_packages[&2].identifier.clone(), + data_hex: bogus_member2.commitment.data_hex.clone(), + }, + NativeFrostCommitment { + identifier: key_packages[&1].identifier.clone(), + data_hex: bogus_member2.commitment.data_hex, + }, + ], + ); + let bogus_share = sign_share(SignShareRequest { + signing_package_hex: other_package, + nonces_hex: bogus_member2.nonces_hex, + key_package_identifier: key_packages[&2].identifier.clone(), + key_package_hex: key_packages[&2].data_hex.clone(), + }) + .expect("bogus member 2 share"); + + let err = interactive_aggregate(InteractiveAggregateRequest { + session_id: session_id.to_string(), + attempt_id: opened.attempt_id.clone(), + signing_package_hex, + signature_shares: vec![ + crate::api::NativeFrostSignatureShare { + identifier: key_packages[&1].identifier.clone(), + data_hex: round2.signature_share_hex, + }, + bogus_share.signature_share, + ], + taproot_merkle_root_hex: None, + }) + .expect_err("an invalid share must fail aggregation with attributable blame"); + match err { + EngineError::InvalidSignatureShare { ref culprits, .. } => { + assert!( + culprits.contains(&key_packages[&2].identifier), + "culprit list must name member 2: {culprits:?}" + ); + } + other => panic!("unexpected error: {other:?}"), + } + assert_eq!(err.code(), "invalid_signature_share"); +} diff --git a/pkg/tbtc/signer/src/errors.rs b/pkg/tbtc/signer/src/errors.rs index 636d9ccf82..994e60353c 100644 --- a/pkg/tbtc/signer/src/errors.rs +++ b/pkg/tbtc/signer/src/errors.rs @@ -82,6 +82,17 @@ pub enum EngineError { session_id: String, attempt_id: String, }, + /// Returned by InteractiveAggregate when one or more collected signature + /// shares fail verification against their verifying share. The culprits + /// are named (as Go member identifiers) so the coordinator has + /// attributable blame evidence: it can exclude the offending member from + /// the next attempt rather than failing opaquely. Distinct structured + /// code so cross-language callers act on the culprit list, not a string. + #[error("invalid signature share(s) in session [{session_id}] from member(s): {culprits:?}")] + InvalidSignatureShare { + session_id: String, + culprits: Vec, + }, #[error("internal error: {0}")] Internal(String), } @@ -104,6 +115,7 @@ impl EngineError { Self::ConsumedAttemptReplay { .. } => "consumed_attempt_replay", Self::ConsumedRoundReplay { .. } => "consumed_round_replay", Self::ConsumedNonceReplay { .. } => "consumed_nonce_replay", + Self::InvalidSignatureShare { .. } => "invalid_signature_share", Self::Internal(_) => "internal_error", } } @@ -128,6 +140,9 @@ impl EngineError { Self::ConsumedAttemptReplay { .. } => "recoverable", Self::ConsumedRoundReplay { .. } => "recoverable", Self::ConsumedNonceReplay { .. } => "recoverable", + // Recoverable: the coordinator retries with a new attempt that + // excludes the blamed member(s). + Self::InvalidSignatureShare { .. } => "recoverable", Self::SessionFinalized { .. } => "terminal", Self::SessionNotFound { .. } => "terminal", Self::Internal(_) => "terminal", diff --git a/pkg/tbtc/signer/src/lib.rs b/pkg/tbtc/signer/src/lib.rs index 81c9499996..1f8e30172f 100644 --- a/pkg/tbtc/signer/src/lib.rs +++ b/pkg/tbtc/signer/src/lib.rs @@ -10,12 +10,12 @@ use std::sync::OnceLock; use api::{ AggregateRequest, BuildTaprootTxRequest, DifferentialFuzzRequest, DkgPart1Request, DkgPart2Request, DkgPart3Request, FinalizeSignRoundRequest, - GenerateNoncesAndCommitmentsRequest, InitSignerConfigRequest, InteractiveRound1Request, - InteractiveRound2Request, InteractiveSessionAbortRequest, InteractiveSessionOpenRequest, - NewSigningPackageRequest, PromoteCanaryRequest, QuarantineStatusRequest, - RefreshCadenceStatusRequest, RefreshSharesRequest, RollbackCanaryRequest, RunDkgRequest, - SignShareRequest, StartSignRoundRequest, TranscriptAuditRequest, TriggerEmergencyRekeyRequest, - VerifyBlameProofRequest, + GenerateNoncesAndCommitmentsRequest, InitSignerConfigRequest, InteractiveAggregateRequest, + InteractiveRound1Request, InteractiveRound2Request, InteractiveSessionAbortRequest, + InteractiveSessionOpenRequest, NewSigningPackageRequest, PromoteCanaryRequest, + QuarantineStatusRequest, RefreshCadenceStatusRequest, RefreshSharesRequest, + RollbackCanaryRequest, RunDkgRequest, SignShareRequest, StartSignRoundRequest, + TranscriptAuditRequest, TriggerEmergencyRekeyRequest, VerifyBlameProofRequest, }; use ffi::{ ffi_entry, free_buffer, parse_request, serialize_response, success_from_string, @@ -354,6 +354,18 @@ pub extern "C" fn frost_tbtc_interactive_session_abort( }) } +#[no_mangle] +pub extern "C" fn frost_tbtc_interactive_aggregate( + request_ptr: *const u8, + request_len: usize, +) -> TbtcSignerResult { + ffi_entry(|| { + let request: InteractiveAggregateRequest = parse_request(request_ptr, request_len)?; + let response = engine::interactive_aggregate(request)?; + serialize_response(&response) + }) +} + #[no_mangle] pub extern "C" fn frost_tbtc_start_sign_round( request_ptr: *const u8, @@ -820,6 +832,25 @@ mod tests { let result: crate::api::InteractiveSessionAbortResult = serde_json::from_slice(&payload).expect("abort result payload"); assert!(!result.aborted); + + // Aggregate fails closed: the malformed signing package is + // rejected at parse (before the session lookup), proving the + // symbol -> parse -> engine -> structured-error dispatch. + let aggregate = crate::api::InteractiveAggregateRequest { + session_id: "ffi-interactive-smoke-missing".to_string(), + attempt_id: "missing".to_string(), + signing_package_hex: "00".to_string(), + signature_shares: vec![crate::api::NativeFrostSignatureShare { + identifier: "00".to_string(), + data_hex: "00".to_string(), + }], + taproot_merkle_root_hex: None, + }; + let (status, payload) = call_ffi(&aggregate, super::frost_tbtc_interactive_aggregate); + assert_ne!(status, 0); + let error: ErrorResponse = + serde_json::from_slice(&payload).expect("aggregate error payload"); + assert_eq!(error.code, "validation_error"); } fn native_frost_identifier(member_index: u8) -> String { From 24f4eb293e8b8d81ef21e8e7fda6ce0433902408 Mon Sep 17 00:00:00 2001 From: maclane Date: Sat, 13 Jun 2026 09:01:00 -0400 Subject: [PATCH 2/4] fix(tbtc/signer): defer attributable aggregate blame until inputs are bound Review finding (Codex P1): emitting per-member InvalidSignatureShare blame from InteractiveAggregate is forgeable. The engine cannot yet bind the aggregate's public inputs (signing package, taproot root) to what each member actually signed at Round2, so a buggy or malicious coordinator aggregating against a different package/root makes honest shares fail verification and frames their members. Binding "to what the members signed" requires the per-member signed-package envelopes (frozen spec section 6), which are Phase 7.2b. So this drops the premature attributable blame: a share-verification failure is now a generic fail-closed Validation error (no signature, no culprit naming). The InvalidSignatureShare error variant and the culprit-mapping helper are removed and will be reintroduced in 7.2b together with the envelope binding that makes the attribution sound - and with the FFI structured-culprit payload (Codex P2), since the current ffi error_result carries only code/message/recovery_class and would drop a culprit vector anyway. The aggregate still verifies every share (frost) and self-verifies the result against the tweaked group key; only the blame OUTPUT is deferred. Test renamed to assert the fail-closed generic error. Full suite 268 passed / 1 ignored, clippy -D warnings clean, chaos green. Co-Authored-By: Claude Fable 5 --- pkg/tbtc/signer/src/engine/interactive.rs | 38 +++++++++-------------- pkg/tbtc/signer/src/engine/tests.rs | 23 +++++++------- pkg/tbtc/signer/src/errors.rs | 15 --------- 3 files changed, 26 insertions(+), 50 deletions(-) diff --git a/pkg/tbtc/signer/src/engine/interactive.rs b/pkg/tbtc/signer/src/engine/interactive.rs index bc60a9eb6a..065c66c2c1 100644 --- a/pkg/tbtc/signer/src/engine/interactive.rs +++ b/pkg/tbtc/signer/src/engine/interactive.rs @@ -626,8 +626,16 @@ pub fn interactive_aggregate( // verifying shares), so no policy gate runs here - the secret-bearing // step is each signer's Round2, where lifecycle/quarantine/firewall // were already enforced (including the full-subset quarantine check). - // frost verifies every share against its verifying share and reports - // the culprits on failure; surface them as attributable blame. + // + // frost verifies every share and can name which failed, but this path + // does NOT surface those as attributable member blame: the engine + // cannot yet bind these public inputs (signing package, taproot root) + // to what each member actually signed at Round2, so a coordinator + // aggregating against a different package/root would make honest + // shares fail and frame their members. Attributable blame waits for + // the signed-package envelopes (Phase 7.2b, frozen spec section 6), + // which prove what each member signed. Until then a verification + // failure is a generic fail-closed error: no signature, no blame. let verification_key_package = match taproot_merkle_root.as_ref() { Some(root) => public_key_package.clone().tweak(Some(root.as_slice())), None => public_key_package.clone(), @@ -642,8 +650,11 @@ pub fn interactive_aggregate( ), None => frost::aggregate(&signing_package, &signature_shares, &public_key_package), }; - let signature = aggregate_result - .map_err(|error| map_aggregate_error_to_blame(&request.session_id, error))?; + let signature = aggregate_result.map_err(|error| { + EngineError::Validation(format!( + "InteractiveAggregate: failed to aggregate: {error}" + )) + })?; // Self-verify the aggregate against the (tweaked) group verifying // key before releasing it, matching the coarse finalize path. @@ -673,25 +684,6 @@ pub fn interactive_aggregate( }) } -// Convert a frost aggregation error into an attributable blame error -// when it identifies culprit shares, so the coordinator can exclude the -// offending member(s) on the next attempt. Other failures map to a -// generic validation error. -fn map_aggregate_error_to_blame(session_id: &str, error: frost::Error) -> EngineError { - if let frost::Error::InvalidSignatureShare { culprits } = error { - return EngineError::InvalidSignatureShare { - session_id: session_id.to_string(), - culprits: culprits - .into_iter() - .map(frost_identifier_to_go_string) - .collect(), - }; - } - EngineError::Validation(format!( - "InteractiveAggregate: failed to aggregate: {error}" - )) -} - pub fn interactive_session_abort( request: InteractiveSessionAbortRequest, ) -> Result { diff --git a/pkg/tbtc/signer/src/engine/tests.rs b/pkg/tbtc/signer/src/engine/tests.rs index 01a08af0c4..abe6246e7b 100644 --- a/pkg/tbtc/signer/src/engine/tests.rs +++ b/pkg/tbtc/signer/src/engine/tests.rs @@ -12841,7 +12841,7 @@ fn interactive_aggregate_produces_and_self_verifies_bip340() { } #[test] -fn interactive_aggregate_blames_invalid_share_culprit() { +fn interactive_aggregate_rejects_invalid_share_fail_closed() { let _guard = lock_test_state(); reset_for_tests(); @@ -12925,15 +12925,14 @@ fn interactive_aggregate_blames_invalid_share_culprit() { ], taproot_merkle_root_hex: None, }) - .expect_err("an invalid share must fail aggregation with attributable blame"); - match err { - EngineError::InvalidSignatureShare { ref culprits, .. } => { - assert!( - culprits.contains(&key_packages[&2].identifier), - "culprit list must name member 2: {culprits:?}" - ); - } - other => panic!("unexpected error: {other:?}"), - } - assert_eq!(err.code(), "invalid_signature_share"); + .expect_err("an invalid share must fail aggregation closed"); + // 7.2a fails closed without attributable member blame: the engine + // cannot yet bind the aggregate inputs to what each member signed + // (that needs the Phase 7.2b signed-package envelopes), so a + // verification failure is a generic error - no signature, and no + // culprit naming that a wrong-package/root coordinator could forge. + assert!( + matches!(err, EngineError::Validation(ref m) if m.contains("failed to aggregate")), + "unexpected error: {err:?}" + ); } diff --git a/pkg/tbtc/signer/src/errors.rs b/pkg/tbtc/signer/src/errors.rs index 994e60353c..636d9ccf82 100644 --- a/pkg/tbtc/signer/src/errors.rs +++ b/pkg/tbtc/signer/src/errors.rs @@ -82,17 +82,6 @@ pub enum EngineError { session_id: String, attempt_id: String, }, - /// Returned by InteractiveAggregate when one or more collected signature - /// shares fail verification against their verifying share. The culprits - /// are named (as Go member identifiers) so the coordinator has - /// attributable blame evidence: it can exclude the offending member from - /// the next attempt rather than failing opaquely. Distinct structured - /// code so cross-language callers act on the culprit list, not a string. - #[error("invalid signature share(s) in session [{session_id}] from member(s): {culprits:?}")] - InvalidSignatureShare { - session_id: String, - culprits: Vec, - }, #[error("internal error: {0}")] Internal(String), } @@ -115,7 +104,6 @@ impl EngineError { Self::ConsumedAttemptReplay { .. } => "consumed_attempt_replay", Self::ConsumedRoundReplay { .. } => "consumed_round_replay", Self::ConsumedNonceReplay { .. } => "consumed_nonce_replay", - Self::InvalidSignatureShare { .. } => "invalid_signature_share", Self::Internal(_) => "internal_error", } } @@ -140,9 +128,6 @@ impl EngineError { Self::ConsumedAttemptReplay { .. } => "recoverable", Self::ConsumedRoundReplay { .. } => "recoverable", Self::ConsumedNonceReplay { .. } => "recoverable", - // Recoverable: the coordinator retries with a new attempt that - // excludes the blamed member(s). - Self::InvalidSignatureShare { .. } => "recoverable", Self::SessionFinalized { .. } => "terminal", Self::SessionNotFound { .. } => "terminal", Self::Internal(_) => "terminal", From 312e106a4108622b06fb2d578ddea2500d709aab Mon Sep 17 00:00:00 2001 From: maclane Date: Sat, 13 Jun 2026 09:12:16 -0400 Subject: [PATCH 3/4] docs(tbtc/signer): align aggregate FFI/API contract with fail-closed behavior Review finding (Codex P2): the previous commit removed the structured invalid_signature_share error but left the public C header and the InteractiveAggregateRequest doc still advertising attributable blame / the invalid_signature_share code. A coordinator built against that contract would wait for an error code that can no longer be returned. Both now state the actual 7.2a behavior: an invalid share (or a mismatched package/root) fails closed with the generic validation_error code and no signature, and per-member attributable blame is deferred to Phase 7.2b where the signed-package envelopes make the attribution unforgeable. The frozen spec's design statement is unchanged - it describes the completed feature, and the deferral is tracked. Doc-only: header parses, crate builds clean. Co-Authored-By: Claude Fable 5 --- pkg/tbtc/signer/include/frost_tbtc.h | 12 ++++++++---- pkg/tbtc/signer/src/api.rs | 5 ++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/pkg/tbtc/signer/include/frost_tbtc.h b/pkg/tbtc/signer/include/frost_tbtc.h index aed2583ac5..7f184fe408 100644 --- a/pkg/tbtc/signer/include/frost_tbtc.h +++ b/pkg/tbtc/signer/include/frost_tbtc.h @@ -74,10 +74,14 @@ TbtcSignerResult frost_tbtc_interactive_round2(const uint8_t* request_ptr, size_ TbtcSignerResult frost_tbtc_interactive_session_abort(const uint8_t* request_ptr, size_t request_len); /* * Coordinator-side aggregation: verifies each collected signature share - * against its verifying share (resolved from the session's DKG state) and, - * on failure, reports the culprit member(s) as attributable blame - * (`invalid_signature_share`); otherwise returns the aggregated BIP-340 - * signature. Operates on public material only - no secret crosses here. + * against its verifying share (resolved from the session's DKG state) and + * returns the aggregated BIP-340 signature. Operates on public material + * only - no secret crosses here. On any verification failure it fails + * closed with the generic `validation_error` code and returns no + * signature; per-member attributable blame (a structured culprit list) + * is intentionally NOT emitted yet - it requires the signed-package + * envelope binding added in Phase 7.2b, without which the attribution + * would be forgeable by a coordinator using a mismatched package/root. */ TbtcSignerResult frost_tbtc_interactive_aggregate(const uint8_t* request_ptr, size_t request_len); diff --git a/pkg/tbtc/signer/src/api.rs b/pkg/tbtc/signer/src/api.rs index a1fe1ca453..86fc85b344 100644 --- a/pkg/tbtc/signer/src/api.rs +++ b/pkg/tbtc/signer/src/api.rs @@ -222,7 +222,10 @@ pub struct InteractiveAggregateRequest { /// The collected signature shares from the responsive subset. Each /// is verified against the member's verifying share (resolved from /// the session's DKG public key package) before aggregation; an - /// invalid share yields attributable blame naming the culprit. + /// invalid share fails the call closed with `validation_error` and + /// no signature. Per-member attributable blame (a culprit list) is + /// deferred to Phase 7.2b, where the signed-package envelopes bind + /// what each member signed and make the attribution unforgeable. pub signature_shares: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] pub taproot_merkle_root_hex: Option, From f5a08a681625db0328f3748bdf937a8454ce3db2 Mon Sep 17 00:00:00 2001 From: maclane Date: Sat, 13 Jun 2026 09:24:39 -0400 Subject: [PATCH 4/4] fix(tbtc/signer): sweep expired interactive state in InteractiveAggregate Review finding (Codex P2): aggregate took the engine lock but, unlike open/round1/round2/abort, never called sweep_expired_interactive_state. A process receiving only InteractiveAggregate calls would let expired Round1 nonce handles linger in memory past the TTL until some other interactive endpoint happened to run, breaking the documented TTL guarantee for secret nonce handles. Aggregate now sweeps right after acquiring the lock, like every other interactive entry point. Test: an aged Round1 handle in one session is cleared by an aggregate call targeting a different (missing) session - proving the sweep runs regardless of which interactive endpoint takes the lock. Full suite 269 passed / 1 ignored, clippy -D warnings clean, chaos green. Co-Authored-By: Claude Fable 5 --- pkg/tbtc/signer/src/engine/interactive.rs | 7 +- pkg/tbtc/signer/src/engine/tests.rs | 103 ++++++++++++++++++++++ 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/pkg/tbtc/signer/src/engine/interactive.rs b/pkg/tbtc/signer/src/engine/interactive.rs index 065c66c2c1..211dda0404 100644 --- a/pkg/tbtc/signer/src/engine/interactive.rs +++ b/pkg/tbtc/signer/src/engine/interactive.rs @@ -592,9 +592,14 @@ pub fn interactive_aggregate( let mut taproot_merkle_root_hex = request.taproot_merkle_root_hex.clone(); let taproot_merkle_root = canonicalize_taproot_merkle_root_hex(&mut taproot_merkle_root_hex)?; - let guard = state()? + let mut guard = state()? .lock() .map_err(|_| EngineError::Internal("engine lock poisoned".to_string()))?; + // Aggregate takes the engine lock like every other interactive entry + // point, so it sweeps expired interactive state too: the TTL + // guarantee (a nonce handle gone within the TTL of inactivity) must + // hold even when the only post-expiry traffic is aggregate calls. + sweep_expired_interactive_state(&mut guard); // Resolve the group's public key package (the verifying shares used // to check each contribution) from the session's own DKG state, not diff --git a/pkg/tbtc/signer/src/engine/tests.rs b/pkg/tbtc/signer/src/engine/tests.rs index abe6246e7b..cc0df1d11a 100644 --- a/pkg/tbtc/signer/src/engine/tests.rs +++ b/pkg/tbtc/signer/src/engine/tests.rs @@ -12936,3 +12936,106 @@ fn interactive_aggregate_rejects_invalid_share_fail_closed() { "unexpected error: {err:?}" ); } + +#[test] +fn interactive_aggregate_sweeps_expired_sessions() { + let _guard = lock_test_state(); + reset_for_tests(); + + let key_group = "interactive-test-key-group"; + let message = [0x4du8; 32]; + let included = [1u16, 2]; + + // An interactive attempt is opened + round-1'd on session A, then + // aged past the TTL. + let key_packages = ensure_interactive_dkg_session("interactive-aggregate-sweep-a", key_group); + let opened = open_interactive_for_test( + "interactive-aggregate-sweep-a", + key_group, + &message, + &included, + 1, + 1, + 2, + ) + .expect("session A opens"); + interactive_round1(InteractiveRound1Request { + session_id: "interactive-aggregate-sweep-a".to_string(), + attempt_id: opened.attempt_id.clone(), + member_identifier: 1, + }) + .expect("round 1"); + { + let mut guard = state().expect("state").lock().expect("lock"); + let interactive = guard + .sessions + .get_mut("interactive-aggregate-sweep-a") + .expect("session A") + .interactive_signing + .as_mut() + .expect("live interactive state"); + interactive.opened_at_unix = interactive + .opened_at_unix + .saturating_sub(interactive_session_ttl_seconds() + 1); + } + + // A parseable threshold-sized package + share so the aggregate call + // reaches the lock and the sweep (it then fails on the missing + // target session). The package needs `threshold` (2) commitments for + // sign_share to produce a share. + let member1 = generate_nonces_and_commitments(GenerateNoncesAndCommitmentsRequest { + key_package_identifier: key_packages[&1].identifier.clone(), + key_package_hex: key_packages[&1].data_hex.clone(), + }) + .expect("member 1 nonces"); + let member2 = generate_nonces_and_commitments(GenerateNoncesAndCommitmentsRequest { + key_package_identifier: key_packages[&2].identifier.clone(), + key_package_hex: key_packages[&2].data_hex.clone(), + }) + .expect("member 2 nonces"); + let parseable_package = interactive_package_for_test( + &message, + vec![ + NativeFrostCommitment { + identifier: key_packages[&1].identifier.clone(), + data_hex: member1.commitment.data_hex.clone(), + }, + member2.commitment, + ], + ); + let parseable_share = sign_share(SignShareRequest { + signing_package_hex: parseable_package.clone(), + nonces_hex: member1.nonces_hex, + key_package_identifier: key_packages[&1].identifier.clone(), + key_package_hex: key_packages[&1].data_hex.clone(), + }) + .expect("member 1 share"); + + // Aggregate against a session that does not exist: the inputs parse, + // so the call reaches the lock and the sweep before failing with + // SessionNotFound. + let err = interactive_aggregate(InteractiveAggregateRequest { + session_id: "interactive-aggregate-sweep-missing".to_string(), + attempt_id: "missing".to_string(), + signing_package_hex: parseable_package, + signature_shares: vec![parseable_share.signature_share], + taproot_merkle_root_hex: None, + }) + .expect_err("aggregate against a missing session fails closed"); + assert!( + matches!(err, EngineError::SessionNotFound { .. }), + "unexpected error: {err:?}" + ); + + // The aggregate call's sweep must have cleared session A's expired + // nonce handle even though the call targeted a different session. + let guard = state().expect("state").lock().expect("lock"); + let session_a = guard + .sessions + .get("interactive-aggregate-sweep-a") + .expect("session A (DKG state) retained"); + assert!( + session_a.interactive_signing.is_none(), + "an aggregate call must sweep expired interactive state in other sessions" + ); +}