fix(tbtc/signer): quarantine the full Round2 subset; reject phantom included IDs

Two findings:

P1 (quarantine bypass) - the Round2 gate recheck only quarantine-checked
this node's own member_identifier. The chosen signing subset (the
package's participants) is known at Round2, so this node could release a
share into a package that includes a quarantined co-signer, bypassing
the all-signing-participants quarantine the coarse path enforces. Round2
now computes the package's u16 subset (after verify confirms it is a
threshold-sized subset of the included set) and quarantine-checks ALL of
it before consuming the nonce. The Open gate keeps checking only this
member (the responsive subset is not chosen until Round2); the gate
helper now takes the identifier set to check so the two sites stay
aligned.

P2 (phantom participants) - Open validated the attempt context's
internal consistency but never checked that the included participants
are real DKG members. A caller could pad the included set with phantom
ids to bias the RFC-21 coordinator/attempt derivation, with Round2 then
releasing a share under an attempt context that is not a genuine DKG
subset. Open now rejects any included participant absent from the
session's dkg_key_packages.

Tests: a co-signer quarantined after round 1 blocks the Round2 share
without consuming the attempt (clearing it lets the attempt complete);
a phantom included id is rejected at Open even with a valid local
member. Full suite 266 passed / 1 ignored, clippy -D warnings clean,
chaos green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: mswilkison

pkg/tbtc/signer/src/engine/interactive.rs

@@ -101,10 +101,11 @@ pub fn interactive_session_open(
                 request.threshold, dkg.threshold
             )));
         }
-        let key_package = session
+        let dkg_key_packages = session
             .dkg_key_packages
             .as_ref()
-            .ok_or_else(|| EngineError::Internal("missing DKG key package cache".to_string()))?
+            .ok_or_else(|| EngineError::Internal("missing DKG key package cache".to_string()))?;
+        let key_package = dkg_key_packages
             .get(&request.member_identifier)
             .ok_or_else(|| {
                 EngineError::Validation(
@@ -118,10 +119,12 @@ pub fn interactive_session_open(
         // runs again at Round2 (the share-release moment) so a policy
         // change recorded after Open - emergency rekey, finalization,
         // quarantine, or a re-bound policy-checked tx - cannot let a
-        // share escape.
+        // share escape. At Open only this node's own member is known to
+        // sign; Round2 re-checks quarantine over the actual chosen
+        // subset.
         enforce_interactive_signing_gates(
             &request.session_id,
-            request.member_identifier,
+            &[request.member_identifier],
             &request.message_hex,
             session.emergency_rekey_event.as_ref(),
             session.finalize_request_fingerprint.is_some(),
@@ -153,6 +156,19 @@ pub fn interactive_session_open(
                     .to_string(),
             ));
         }
+        // Every included participant must be a real DKG member of this
+        // session. Otherwise a caller could pad the included set with
+        // phantom identifiers to bias the RFC-21 coordinator/attempt
+        // derivation, and Round2 could release a share under an attempt
+        // context that is not a genuine DKG subset.
+        for participant in &canonical_included_participants {
+            if !dkg_key_packages.contains_key(participant) {
+                return Err(EngineError::Validation(format!(
+                    "attempt_context.included_participants contains [{participant}], \
+                     which is not a DKG participant for this session"
+                )));
+            }
+        }
         (key_package, canonical_included_participants)
     };
 
@@ -418,9 +434,13 @@ pub fn interactive_round2(
             && interactive.member_identifier == request.member_identifier
     }) {
         let bound_message_hex = hex::encode(interactive.message_bytes.as_slice());
+        // Fast-path lifecycle/firewall and this node's own quarantine.
+        // The full chosen signing subset is quarantine-checked after the
+        // package is verified (below), once it is known to be a real
+        // subset of the included set.
         enforce_interactive_signing_gates(
             &request.session_id,
-            request.member_identifier,
+            &[request.member_identifier],
             &bound_message_hex,
             session.emergency_rekey_event.as_ref(),
             session.finalize_request_fingerprint.is_some(),
@@ -450,6 +470,20 @@ pub fn interactive_round2(
     // the consumption marker is written before the share is released.
     verify_round2_signing_package(interactive, &signing_package)?;
 
+    // The package is now confirmed to be a threshold-sized subset of the
+    // attempt's included set, so the chosen signing subset is known.
+    // Quarantine-check ALL of it before releasing a share: this node
+    // must not contribute to a signature whose subset includes a
+    // locally quarantined co-signer, matching the coarse path's
+    // all-signing-participants quarantine enforcement.
+    let signing_subset = round2_signing_subset(interactive, &signing_package)?;
+    enforce_not_quarantined_identifiers(
+        &request.session_id,
+        &signing_subset,
+        &quarantined_operator_identifiers,
+        auto_quarantine_config.as_ref(),
+    )?;
+
     // Consumption-before-release: the durable marker is persisted
     // BEFORE the share is computed and returned. If persistence fails,
     // the marker is rolled back and the nonces remain live - no share
@@ -690,13 +724,20 @@ fn verify_round2_signing_package(
 // The signing gates the interactive path enforces at BOTH Open and
 // the Round2 share-release moment, mirroring the coarse
 // start_sign_round: emergency-rekey and finalized lifecycle, quarantine
-// of this node's own member, and the signing-policy firewall binding of
-// the message to a policy-checked build_taproot_tx. Centralized in one
-// function so the two call sites cannot drift apart.
+// of the signing participants, and the signing-policy firewall binding
+// of the message to a policy-checked build_taproot_tx. Centralized in
+// one function so the two call sites cannot drift apart.
+//
+// quarantine_identifiers is the set to quarantine-check: at Open only
+// this node's own member is known to sign; at Round2 it is the full
+// chosen signing subset (the package's participants), so this node
+// refuses to contribute a share to a package that includes any
+// quarantined co-signer - the same all-participants check the coarse
+// path applies.
 #[allow(clippy::too_many_arguments)]
 fn enforce_interactive_signing_gates(
     session_id: &str,
-    member_identifier: u16,
+    quarantine_identifiers: &[u16],
     message_hex: &str,
     emergency_rekey_event: Option<&EmergencyRekeyEvent>,
     session_finalized: bool,
@@ -721,7 +762,7 @@ fn enforce_interactive_signing_gates(
     }
     enforce_not_quarantined_identifiers(
         session_id,
-        &[member_identifier],
+        quarantine_identifiers,
         quarantined_operator_identifiers,
         auto_quarantine_config,
     )?;
@@ -737,6 +778,30 @@ fn canonical_attempt_id(attempt_id: &str) -> String {
     attempt_id.to_ascii_lowercase()
 }
 
+// The chosen signing subset as Go u16 identifiers: the included
+// participants whose commitment appears in the signing package. The
+// caller MUST have run verify_round2_signing_package first (which
+// confirms the package is a threshold-sized subset of the included
+// set), so every package participant maps back to an included member.
+fn round2_signing_subset(
+    interactive: &InteractiveSigningState,
+    signing_package: &frost::SigningPackage,
+) -> Result<Vec<u16>, EngineError> {
+    let package_identifiers = signing_package
+        .signing_commitments()
+        .keys()
+        .copied()
+        .collect::<BTreeSet<_>>();
+    let mut subset = Vec::with_capacity(package_identifiers.len());
+    for participant in &interactive.canonical_included_participants {
+        let frost_identifier = participant_identifier_to_frost_identifier(*participant)?;
+        if package_identifiers.contains(&frost_identifier) {
+            subset.push(*participant);
+        }
+    }
+    Ok(subset)
+}
+
 pub(crate) fn zeroize_interactive_round1(interactive: &mut InteractiveSigningState) {
     if let Some(mut round1) = interactive.round1.take() {
         round1.nonces.zeroize();

pkg/tbtc/signer/src/engine/tests.rs

@@ -12612,3 +12612,125 @@ fn interactive_open_requires_an_existing_dkg_session() {
         "unexpected error: {absent:?}"
     );
 }
+
+#[test]
+fn interactive_round2_rejects_quarantined_co_signer_in_package() {
+    let _guard = lock_test_state();
+    reset_for_tests();
+
+    let session_id = "interactive-round2-quarantined-cosigner";
+    let key_group = "interactive-test-key-group";
+    let message = [0x1cu8; 32];
+    let included = [1u16, 2];
+    let key_packages = ensure_interactive_dkg_session(session_id, key_group);
+
+    std::env::set_var(TBTC_SIGNER_ENABLE_AUTO_QUARANTINE_ENV, "true");
+    std::env::set_var(TBTC_SIGNER_AUTO_QUARANTINE_FAULT_THRESHOLD_ENV, "2");
+    std::env::set_var(TBTC_SIGNER_AUTO_QUARANTINE_TIMEOUT_PENALTY_ENV, "1");
+    std::env::set_var(TBTC_SIGNER_AUTO_QUARANTINE_INVALID_SHARE_PENALTY_ENV, "2");
+
+    let outcome = (|| -> Result<(), EngineError> {
+        // This member (1) opens and runs round 1 while no one is
+        // quarantined; the co-signer (2) is quarantined afterward.
+        let opened =
+            open_interactive_for_test(session_id, key_group, &message, &included, 1, 1, 2)?;
+        let round1 = interactive_round1(InteractiveRound1Request {
+            session_id: session_id.to_string(),
+            attempt_id: opened.attempt_id.clone(),
+            member_identifier: 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(),
+        })?;
+        let signing_package_hex = interactive_package_for_test(
+            &message,
+            vec![
+                NativeFrostCommitment {
+                    identifier: key_packages[&1].identifier.clone(),
+                    data_hex: round1.commitments_hex,
+                },
+                member2.commitment,
+            ],
+        );
+
+        // Quarantine the co-signer (member 2) after round 1.
+        {
+            let mut guard = state().expect("state").lock().expect("lock");
+            guard.quarantined_operator_identifiers.insert(2);
+        }
+
+        // Round2 must refuse: this node will not contribute a share to a
+        // package whose subset includes a quarantined co-signer, even
+        // though this node (member 1) is not itself quarantined.
+        let blocked = interactive_round2(InteractiveRound2Request {
+            session_id: session_id.to_string(),
+            attempt_id: opened.attempt_id.clone(),
+            member_identifier: 1,
+            signing_package_hex,
+        })
+        .expect_err("a quarantined co-signer in the package must block the share");
+        assert!(
+            matches!(blocked, EngineError::QuarantinePolicyRejected { ref reason_code, .. }
+                if reason_code == "operator_auto_quarantined"),
+            "unexpected error: {blocked:?}"
+        );
+
+        // Fail-closed without consuming: clearing the quarantine lets the
+        // same attempt complete (the rejection preceded consumption).
+        {
+            let mut guard = state().expect("state").lock().expect("lock");
+            assert!(
+                !guard
+                    .sessions
+                    .get(session_id)
+                    .expect("session")
+                    .consumed_interactive_attempt_markers
+                    .contains(&opened.attempt_id),
+                "a quarantine rejection must not consume the attempt"
+            );
+            guard.quarantined_operator_identifiers.remove(&2);
+        }
+        Ok(())
+    })();
+
+    std::env::remove_var(TBTC_SIGNER_ENABLE_AUTO_QUARANTINE_ENV);
+    std::env::remove_var(TBTC_SIGNER_AUTO_QUARANTINE_FAULT_THRESHOLD_ENV);
+    std::env::remove_var(TBTC_SIGNER_AUTO_QUARANTINE_TIMEOUT_PENALTY_ENV);
+    std::env::remove_var(TBTC_SIGNER_AUTO_QUARANTINE_INVALID_SHARE_PENALTY_ENV);
+    outcome.expect("round2 co-signer quarantine lifecycle");
+}
+
+#[test]
+fn interactive_open_rejects_phantom_included_participant() {
+    let _guard = lock_test_state();
+    reset_for_tests();
+
+    // The session's DKG group is members 1..3. An attempt context whose
+    // included set names a phantom id (99) must be rejected even though
+    // the local member (1) is a real participant - otherwise a caller
+    // could bias the RFC-21 coordinator/attempt derivation with
+    // non-participants.
+    let session_id = "interactive-phantom-included";
+    let key_group = "interactive-test-key-group";
+    let message = [0x1du8; 32];
+    ensure_interactive_dkg_session(session_id, key_group);
+
+    let attempt_context =
+        interactive_test_attempt_context(session_id, key_group, &message, &[1u16, 99], 1);
+    let err = interactive_session_open(InteractiveSessionOpenRequest {
+        session_id: session_id.to_string(),
+        member_identifier: 1,
+        message_hex: hex::encode(message),
+        key_group: key_group.to_string(),
+        threshold: 2,
+        taproot_merkle_root_hex: None,
+        attempt_context,
+    })
+    .expect_err("a phantom included participant must be rejected");
+    assert!(
+        matches!(err, EngineError::Validation(ref m)
+            if m.contains("not a DKG participant for this session")),
+        "unexpected error: {err:?}"
+    );
+}