fix(tbtc/signer): bound interactive session registry and validate threshold

Two findings on the 7.1 path:

P2 (liveness/DoS) - an interactive open that later expired or was
aborted before Round2 left an otherwise-empty SessionState in the
registry. Since open inserts new session IDs and
ensure_session_insert_capacity counts every map entry, a caller could
churn unique sessions until TBTC_SIGNER_MAX_SESSIONS filled, after
which DKG / build_tx / new interactive sessions were rejected until
restart. The TTL sweep and abort now drop a session that holds nothing
durable once its live attempt is cleared, via a new
SessionState::is_disposable that checks EVERY field so a session still
carrying consumed markers or DKG material is never removed.

P2 (verify-before-consume) - Open accepted a threshold below the key
package's min_signers; Round2 would then accept a too-small signing
package, persist the consumed marker, and only then have
frost::round2::sign fail on the commitment count - burning the nonce
for a validation error. Open now rejects threshold != key package
min_signers before storing the session.

Tests: open-then-abort churn under a 2-session cap stays bounded (no
accumulation); the abort-sweep test now asserts the empty session is
dropped, not just cleared; threshold-below-min_signers is rejected and
the matching threshold opens. Full suite 264 passed / 1 ignored,
clippy -D warnings clean, chaos suite green.

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

Author: mswilkison

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

@@ -96,6 +96,19 @@ pub fn interactive_session_open(
             "key_package_identifier must match member_identifier".to_string(),
         ));
     }
+    // The signing threshold is fixed by the key material. Reject a
+    // mismatch at Open: otherwise Round2 would accept a signing package
+    // of the requested (wrong) size, persist the consumed marker, and
+    // only then have frost::round2::sign fail on the commitment count -
+    // burning the nonce handle for a validation error, against the
+    // verify-before-consume contract.
+    if *key_package.min_signers() != request.threshold {
+        return Err(EngineError::Validation(format!(
+            "threshold [{}] does not match the key package min_signers [{}]",
+            request.threshold,
+            *key_package.min_signers()
+        )));
+    }
 
     let request_fingerprint = interactive_open_request_fingerprint(&request)?;
     let attempt_id = request.attempt_context.attempt_id.clone();
@@ -524,6 +537,18 @@ pub fn interactive_session_abort(
         None => false,
     };
 
+    // Drop the session if aborting left it with nothing durable, so an
+    // open-then-abort churn cannot accumulate empty entries against
+    // TBTC_SIGNER_MAX_SESSIONS.
+    if aborted
+        && guard
+            .sessions
+            .get(&request.session_id)
+            .is_some_and(SessionState::is_disposable)
+    {
+        guard.sessions.remove(&request.session_id);
+    }
+
     record_hardening_telemetry(|telemetry| {
         telemetry.interactive_session_abort_success_total = telemetry
             .interactive_session_abort_success_total
@@ -704,19 +729,28 @@ pub(crate) fn zeroize_interactive_round1(interactive: &mut InteractiveSigningSta
 pub(crate) fn sweep_expired_interactive_state(engine_state: &mut EngineState) {
     let ttl_seconds = interactive_session_ttl_seconds();
     let now = now_unix();
-    for session in engine_state.sessions.values_mut() {
+    engine_state.sessions.retain(|_session_id, session| {
         let expired = session
             .interactive_signing
             .as_ref()
             .is_some_and(|interactive| {
                 now.saturating_sub(interactive.opened_at_unix) > ttl_seconds
             });
-        if expired {
-            if let Some(mut removed) = session.interactive_signing.take() {
-                zeroize_interactive_round1(&mut removed);
-            }
+        if !expired {
+            // Untouched sessions are kept as-is; only sessions whose
+            // live attempt we just expired are candidates for removal.
+            return true;
         }
-    }
+        if let Some(mut removed) = session.interactive_signing.take() {
+            zeroize_interactive_round1(&mut removed);
+        }
+        // Having cleared the expired attempt, drop the session if it now
+        // holds nothing durable, so churned interactive opens cannot
+        // accumulate empty entries against TBTC_SIGNER_MAX_SESSIONS. A
+        // session that still carries consumed markers or DKG material is
+        // kept.
+        !session.is_disposable()
+    });
 }
 
 pub(crate) fn max_live_interactive_sessions_limit() -> usize {

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

@@ -113,6 +113,44 @@ pub(crate) struct SessionState {
     pub(crate) consumed_interactive_attempt_markers: HashSet<String>,
 }
 
+impl SessionState {
+    // True when the session holds no durable or live state worth a
+    // registry slot: removing it loses nothing. Used to drop a session
+    // that only ever held a now-cleared interactive attempt, so churned
+    // interactive opens (open -> expire/abort before Round2) cannot
+    // accumulate empty entries and exhaust TBTC_SIGNER_MAX_SESSIONS.
+    //
+    // EVERY field must be checked here: a field omitted from this
+    // conjunction risks dropping a session that still carries replay
+    // protection (consumed markers) or DKG material. When adding a
+    // field to SessionState, add it here too.
+    pub(crate) fn is_disposable(&self) -> bool {
+        self.dkg_request_fingerprint.is_none()
+            && self.dkg_key_packages.is_none()
+            && self.dkg_public_key_package.is_none()
+            && self.dkg_result.is_none()
+            && self.sign_request_fingerprint.is_none()
+            && self.sign_message_bytes.is_none()
+            && self.round_state.is_none()
+            && self.active_attempt_context.is_none()
+            && self.attempt_transition_records.is_empty()
+            && self.consumed_attempt_ids.is_empty()
+            && self.consumed_sign_round_ids.is_empty()
+            && self.finalize_request_fingerprint.is_none()
+            && self.signature_result.is_none()
+            && self.consumed_finalize_round_ids.is_empty()
+            && self.consumed_finalize_request_fingerprints.is_empty()
+            && self.build_tx_request_fingerprint.is_none()
+            && self.tx_result.is_none()
+            && self.refresh_request_fingerprint.is_none()
+            && self.refresh_result.is_none()
+            && self.refresh_history.is_empty()
+            && self.emergency_rekey_event.is_none()
+            && self.interactive_signing.is_none()
+            && self.consumed_interactive_attempt_markers.is_empty()
+    }
+}
+
 #[derive(Default)]
 pub(crate) struct EngineState {
     pub(crate) sessions: HashMap<String, SessionState>,

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

@@ -12444,14 +12444,13 @@ fn interactive_abort_sweeps_expired_sessions() {
     })
     .expect("abort for an unrelated session");
 
+    // Session A held only its (now-expired) interactive attempt, so the
+    // sweep must remove the whole entry, not just clear the live state -
+    // otherwise empty sessions accumulate against TBTC_SIGNER_MAX_SESSIONS.
     let guard = state().expect("state").lock().expect("lock");
-    let session = guard
-        .sessions
-        .get("interactive-abort-sweep-a")
-        .expect("session A still present");
     assert!(
-        session.interactive_signing.is_none(),
-        "an abort must sweep expired interactive state in other sessions"
+        !guard.sessions.contains_key("interactive-abort-sweep-a"),
+        "an abort must sweep AND drop an otherwise-empty expired session"
     );
 }
 
@@ -12687,3 +12686,92 @@ fn interactive_round2_rechecks_gates_at_share_release() {
     })
     .expect("the same attempt completes once the kill switch clears");
 }
+
+#[test]
+fn interactive_open_rejects_threshold_below_key_package_min_signers() {
+    let _guard = lock_test_state();
+    reset_for_tests();
+
+    // The fixture key packages are min_signers = 2. A request threshold
+    // of 3 must be rejected at Open: otherwise Round2 would accept a
+    // 3-commitment package, persist the marker, and only then have
+    // frost::round2::sign fail on the count - burning the nonce for a
+    // validation error.
+    let key_packages = interactive_test_key_packages();
+    let mismatch = open_interactive_for_test(
+        &key_packages,
+        "interactive-threshold-mismatch",
+        "interactive-test-key-group",
+        &[0x1au8; 32],
+        &[1u16, 2, 3],
+        1,
+        1,
+        3,
+    )
+    .expect_err("a threshold below the key package min_signers must be rejected");
+    assert!(
+        matches!(mismatch, EngineError::Validation(ref m)
+            if m.contains("does not match the key package min_signers")),
+        "unexpected error: {mismatch:?}"
+    );
+
+    // The matching threshold (2) opens.
+    open_interactive_for_test(
+        &key_packages,
+        "interactive-threshold-match",
+        "interactive-test-key-group",
+        &[0x1au8; 32],
+        &[1u16, 2],
+        1,
+        1,
+        2,
+    )
+    .expect("the key-package-matching threshold opens");
+}
+
+#[test]
+fn interactive_open_abort_churn_does_not_exhaust_session_registry() {
+    let _guard = lock_test_state();
+    reset_for_tests();
+
+    // A tiny global session cap: if open-then-abort left empty session
+    // entries behind, this churn would fill the registry and then reject
+    // a fresh open. The disposal on abort must keep the registry clear.
+    std::env::set_var(TBTC_SIGNER_MAX_SESSIONS_ENV, "2");
+
+    let key_packages = interactive_test_key_packages();
+    let key_group = "interactive-test-key-group";
+    let message = [0x1bu8; 32];
+    let included = [1u16, 2];
+
+    let outcome = (|| -> Result<(), EngineError> {
+        for cycle in 0..16 {
+            let session_id = format!("interactive-churn-{cycle}");
+            open_interactive_for_test(
+                &key_packages,
+                &session_id,
+                key_group,
+                &message,
+                &included,
+                1,
+                1,
+                2,
+            )?;
+            interactive_session_abort(InteractiveSessionAbortRequest {
+                session_id: session_id.clone(),
+                attempt_id: None,
+            })?;
+        }
+        // The registry is clear, so the global cap still has room.
+        let guard = state().expect("state").lock().expect("lock");
+        assert!(
+            guard.sessions.is_empty(),
+            "open-then-abort churn must not accumulate session entries: {} present",
+            guard.sessions.len()
+        );
+        Ok(())
+    })();
+
+    std::env::remove_var(TBTC_SIGNER_MAX_SESSIONS_ENV);
+    outcome.expect("session churn stays bounded");
+}