Skip to content

Commit 03fb6d6

Browse files
mswilkisonclaude
andcommitted
fix(tbtc/signer): apply session lifecycle and quarantine gates on interactive open
The interactive path accepted an open after only the signing-policy firewall check, so on a session start_sign_round would refuse it could still emit a share - bypassing the established lifecycle/quarantine gates (review finding). InteractiveSessionOpen now enforces, before installing any interactive state, the same gates the coarse path does: - emergency_rekey_event on an existing session -> LifecyclePolicyRejected (emergency_rekey_required), - a terminally finalized session -> SessionFinalized, - an auto-quarantined member_identifier (absent a DAO allowlist override) -> QuarantinePolicyRejected, reusing enforce_not_quarantined_identifiers so the allowlist override is honored identically. The quarantine check targets this node's own member_identifier - the member it is about to produce a share for - rather than the whole included set, since under t-of-included a quarantined included member simply will not be among the responsive subset. Tests: an emergency-rekey session and a finalized session both refuse interactive open; a quarantined member is rejected and a DAO allowlist override restores signing. Full suite 261 passed / 1 ignored, clippy -D warnings clean, chaos suite green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent fb6f33d commit 03fb6d6

2 files changed

Lines changed: 175 additions & 0 deletions

File tree

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,44 @@ pub fn interactive_session_open(
107107

108108
ensure_session_insert_capacity(&guard.sessions, &request.session_id)?;
109109

110+
// Session lifecycle gates (frozen spec section 5: Open "checks
111+
// policy gates"). The interactive path must refuse in exactly the
112+
// states the coarse start_sign_round refuses: a session under an
113+
// emergency rekey, or one already terminally finalized. Without
114+
// these, InteractiveRound1/Round2 could emit a share where the
115+
// established path would not.
116+
if let Some(existing_session) = guard.sessions.get(&request.session_id) {
117+
if let Some(emergency_rekey_event) = existing_session.emergency_rekey_event.as_ref() {
118+
return Err(EngineError::LifecyclePolicyRejected {
119+
session_id: request.session_id.clone(),
120+
reason_code: "emergency_rekey_required".to_string(),
121+
detail: format!(
122+
"emergency rekey required for session [{}] since [{}]: {}",
123+
request.session_id,
124+
emergency_rekey_event.triggered_at_unix,
125+
emergency_rekey_event.reason
126+
),
127+
});
128+
}
129+
if existing_session.finalize_request_fingerprint.is_some() {
130+
return Err(EngineError::SessionFinalized {
131+
session_id: request.session_id.clone(),
132+
});
133+
}
134+
}
135+
136+
// Quarantine gate: this node is about to produce a share for
137+
// member_identifier, so an auto-quarantined member (absent a DAO
138+
// allowlist override) must not be able to sign through the
139+
// interactive path either.
140+
let auto_quarantine_config = load_auto_quarantine_config()?;
141+
enforce_not_quarantined_identifiers(
142+
&request.session_id,
143+
&[request.member_identifier],
144+
&guard.quarantined_operator_identifiers,
145+
auto_quarantine_config.as_ref(),
146+
)?;
147+
110148
// Signing-policy firewall (frozen spec section 5: Open "checks
111149
// policy gates"). When the firewall is enabled, the message must be
112150
// bound to a prior policy-checked build_taproot_tx for this

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

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12454,3 +12454,140 @@ fn interactive_abort_sweeps_expired_sessions() {
1245412454
"an abort must sweep expired interactive state in other sessions"
1245512455
);
1245612456
}
12457+
12458+
#[test]
12459+
fn interactive_open_rejected_on_session_lifecycle_states() {
12460+
let _guard = lock_test_state();
12461+
reset_for_tests();
12462+
12463+
let key_packages = interactive_test_key_packages();
12464+
let key_group = "interactive-test-key-group";
12465+
let message = [0x17u8; 32];
12466+
let included = [1u16, 2];
12467+
12468+
// A session under an emergency rekey must refuse interactive opens,
12469+
// exactly as start_sign_round does.
12470+
{
12471+
let mut guard = state().expect("state").lock().expect("lock");
12472+
guard.sessions.insert(
12473+
"interactive-lifecycle-rekey".to_string(),
12474+
SessionState {
12475+
emergency_rekey_event: Some(EmergencyRekeyEvent {
12476+
reason: "test rekey".to_string(),
12477+
triggered_at_unix: now_unix(),
12478+
}),
12479+
..Default::default()
12480+
},
12481+
);
12482+
}
12483+
let rekey = open_interactive_for_test(
12484+
&key_packages,
12485+
"interactive-lifecycle-rekey",
12486+
key_group,
12487+
&message,
12488+
&included,
12489+
1,
12490+
1,
12491+
2,
12492+
)
12493+
.expect_err("an emergency-rekey session must refuse interactive open");
12494+
assert!(
12495+
matches!(rekey, EngineError::LifecyclePolicyRejected { ref reason_code, .. }
12496+
if reason_code == "emergency_rekey_required"),
12497+
"unexpected error: {rekey:?}"
12498+
);
12499+
12500+
// A terminally finalized session must refuse interactive opens.
12501+
{
12502+
let mut guard = state().expect("state").lock().expect("lock");
12503+
guard.sessions.insert(
12504+
"interactive-lifecycle-finalized".to_string(),
12505+
SessionState {
12506+
finalize_request_fingerprint: Some("already-finalized".to_string()),
12507+
..Default::default()
12508+
},
12509+
);
12510+
}
12511+
let finalized = open_interactive_for_test(
12512+
&key_packages,
12513+
"interactive-lifecycle-finalized",
12514+
key_group,
12515+
&message,
12516+
&included,
12517+
1,
12518+
1,
12519+
2,
12520+
)
12521+
.expect_err("a finalized session must refuse interactive open");
12522+
assert!(
12523+
matches!(finalized, EngineError::SessionFinalized { .. }),
12524+
"unexpected error: {finalized:?}"
12525+
);
12526+
}
12527+
12528+
#[test]
12529+
fn interactive_open_rejected_for_quarantined_member_honors_dao_allowlist() {
12530+
let _guard = lock_test_state();
12531+
reset_for_tests();
12532+
12533+
std::env::set_var(TBTC_SIGNER_ENABLE_AUTO_QUARANTINE_ENV, "true");
12534+
std::env::set_var(TBTC_SIGNER_AUTO_QUARANTINE_FAULT_THRESHOLD_ENV, "2");
12535+
std::env::set_var(TBTC_SIGNER_AUTO_QUARANTINE_TIMEOUT_PENALTY_ENV, "1");
12536+
std::env::set_var(TBTC_SIGNER_AUTO_QUARANTINE_INVALID_SHARE_PENALTY_ENV, "2");
12537+
12538+
// Member 1 is auto-quarantined.
12539+
{
12540+
let mut guard = state().expect("state").lock().expect("lock");
12541+
guard.quarantined_operator_identifiers.insert(1);
12542+
}
12543+
12544+
let key_packages = interactive_test_key_packages();
12545+
let key_group = "interactive-test-key-group";
12546+
let message = [0x18u8; 32];
12547+
let included = [1u16, 2];
12548+
12549+
let outcome = (|| -> Result<(), EngineError> {
12550+
let quarantined = open_interactive_for_test(
12551+
&key_packages,
12552+
"interactive-quarantine",
12553+
key_group,
12554+
&message,
12555+
&included,
12556+
1,
12557+
1,
12558+
2,
12559+
)
12560+
.expect_err("a quarantined member must not open an interactive session");
12561+
assert!(
12562+
matches!(quarantined, EngineError::QuarantinePolicyRejected { ref reason_code, .. }
12563+
if reason_code == "operator_auto_quarantined"),
12564+
"unexpected error: {quarantined:?}"
12565+
);
12566+
12567+
// A DAO allowlist override restores the member's ability to sign.
12568+
std::env::set_var(
12569+
TBTC_SIGNER_AUTO_QUARANTINE_DAO_ALLOWLIST_IDENTIFIERS_ENV,
12570+
"1",
12571+
);
12572+
let allowlisted = open_interactive_for_test(
12573+
&key_packages,
12574+
"interactive-quarantine-allowlisted",
12575+
key_group,
12576+
&message,
12577+
&included,
12578+
1,
12579+
1,
12580+
2,
12581+
)?;
12582+
assert!(!allowlisted.idempotent);
12583+
Ok(())
12584+
})();
12585+
12586+
std::env::remove_var(TBTC_SIGNER_ENABLE_AUTO_QUARANTINE_ENV);
12587+
std::env::remove_var(TBTC_SIGNER_AUTO_QUARANTINE_FAULT_THRESHOLD_ENV);
12588+
std::env::remove_var(TBTC_SIGNER_AUTO_QUARANTINE_TIMEOUT_PENALTY_ENV);
12589+
std::env::remove_var(TBTC_SIGNER_AUTO_QUARANTINE_INVALID_SHARE_PENALTY_ENV);
12590+
std::env::remove_var(TBTC_SIGNER_AUTO_QUARANTINE_DAO_ALLOWLIST_IDENTIFIERS_ENV);
12591+
12592+
outcome.expect("quarantine gate lifecycle");
12593+
}

0 commit comments

Comments
 (0)