From af1cc10ac353b394c59d2036a9cea87f08be8d2a Mon Sep 17 00:00:00 2001 From: maclane Date: Tue, 30 Jun 2026 10:38:49 -0500 Subject: [PATCH 01/12] fix(tbtc/signer): gate plaintext state, zeroize FFI buffers, allow periodic reshare Three findings from the #4005 review (workflow review + Codex): 1. [HIGH] persistence: the legacy unencrypted plaintext state fallback was accepted unconditionally, bypassing the AEAD envelope -- anyone who could write the state file could forge it (cleared replay markers / attacker key material) without the state-encryption key. Per the secret-material hardening plan the plaintext path is now emergency-rollback-only: compile-time disabled in release builds, rejected in production profiles, and gated behind an explicit TBTC_SIGNER_PERMIT_PLAINTEXT_STATE_ROLLBACK opt-in. The two tests that load plaintext now opt in; a negative assertion pins the fail-closed default. 2. [Codex P1] ffi: free_buffer now zeroizes the buffer before deallocation, so secret material returned by nonce/DKG/key-package endpoints is wiped rather than left in allocator memory when a caller forgets to clear it. 3. [Codex P2] lifecycle: a subsequent same-session refresh with updated shares produced a different fingerprint and was rejected as SessionConflict, so a long-lived session could never advance refresh_history past the first refresh. A different fingerprint is now treated as a new periodic refresh; idempotent replay of the same fingerprint is unchanged. Deferred (flagged, not in this change): - Signing-policy firewall production force-on: force-enabling it makes the entire firewall policy config mandatory in production (a deployment-contract change); the verifier flagged it as needing product confirmation, so left as-is. - State-at-rest anti-rollback freshness counter and quarantine-reset marker loss: design-level / inherent to the opt-in policy. Verified: cargo test (297 passed, 0 failed) and cargo clippy -D warnings clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/tbtc/signer/src/engine/config.rs | 3 +++ pkg/tbtc/signer/src/engine/lifecycle.rs | 9 ++++--- pkg/tbtc/signer/src/engine/persistence.rs | 31 +++++++++++++++++++++++ pkg/tbtc/signer/src/engine/tests.rs | 15 +++++++++++ pkg/tbtc/signer/src/ffi.rs | 10 +++++++- 5 files changed, 64 insertions(+), 4 deletions(-) diff --git a/pkg/tbtc/signer/src/engine/config.rs b/pkg/tbtc/signer/src/engine/config.rs index 79737296d9..65e6ac87ba 100644 --- a/pkg/tbtc/signer/src/engine/config.rs +++ b/pkg/tbtc/signer/src/engine/config.rs @@ -123,6 +123,9 @@ pub(crate) const TBTC_SIGNER_ADMISSION_ALLOWLIST_IDENTIFIERS_ENV: &str = pub(crate) const TBTC_SIGNER_ENFORCE_SIGNING_POLICY_FIREWALL_ENV: &str = "TBTC_SIGNER_ENFORCE_SIGNING_POLICY_FIREWALL"; +pub(crate) const TBTC_SIGNER_PERMIT_PLAINTEXT_STATE_ROLLBACK_ENV: &str = + "TBTC_SIGNER_PERMIT_PLAINTEXT_STATE_ROLLBACK"; + pub(crate) const TBTC_SIGNER_POLICY_ALLOWED_SCRIPT_CLASSES_ENV: &str = "TBTC_SIGNER_POLICY_ALLOWED_SCRIPT_CLASSES"; diff --git a/pkg/tbtc/signer/src/engine/lifecycle.rs b/pkg/tbtc/signer/src/engine/lifecycle.rs index 85083fba6d..e2917e3cb3 100644 --- a/pkg/tbtc/signer/src/engine/lifecycle.rs +++ b/pkg/tbtc/signer/src/engine/lifecycle.rs @@ -393,15 +393,18 @@ pub fn refresh_shares(request: RefreshSharesRequest) -> Result bool { + cfg!(debug_assertions) + && !signer_profile_is_production() + && signer_env_var(TBTC_SIGNER_PERMIT_PLAINTEXT_STATE_ROLLBACK_ENV) + .map(|raw_value| truthy_env_flag(&raw_value)) + .unwrap_or(false) +} + pub(crate) fn decode_persisted_state_storage_format( bytes: &[u8], ) -> Result { @@ -895,6 +911,21 @@ pub(crate) fn decode_persisted_state_storage_format( }); } + // The bytes are not an encrypted envelope. Only fall back to the legacy + // UNAUTHENTICATED plaintext format on the gated emergency-rollback path; + // otherwise refuse, so an attacker who can write the state file cannot + // bypass the AEAD envelope (forged replay markers / key material) without + // the state-encryption key. + if !legacy_plaintext_state_permitted() { + return Err(EngineError::Internal( + "refusing to load unauthenticated plaintext signer state; an \ + encrypted state envelope is required (legacy plaintext is an \ + emergency-rollback-only path, disabled in production and release \ + builds)" + .to_string(), + )); + } + let persisted = serde_json::from_slice::(bytes).map_err(|e| { EngineError::Internal(format!("failed to decode signer state file payload: {e}")) })?; diff --git a/pkg/tbtc/signer/src/engine/tests.rs b/pkg/tbtc/signer/src/engine/tests.rs index 40d4b13780..65337bf864 100644 --- a/pkg/tbtc/signer/src/engine/tests.rs +++ b/pkg/tbtc/signer/src/engine/tests.rs @@ -10121,10 +10121,14 @@ fn schema_mismatch_state_file_fails_closed_by_default() { let persisted_bytes = serde_json::to_vec(&persisted).expect("encode mismatched schema"); std::fs::write(&state_path, &persisted_bytes).expect("write mismatched schema state file"); + // Schema validation runs only after the plaintext gate, so opt into the + // legacy plaintext rollback path (development profile + flag) to reach it. + std::env::set_var(TBTC_SIGNER_PERMIT_PLAINTEXT_STATE_ROLLBACK_ENV, "true"); let err = match load_engine_state_from_storage() { Ok(_) => panic!("expected schema mismatch failure"), Err(err) => err, }; + std::env::remove_var(TBTC_SIGNER_PERMIT_PLAINTEXT_STATE_ROLLBACK_ENV); assert!(matches!(err, EngineError::Internal(_))); let err_message = err.to_string(); @@ -10282,7 +10286,18 @@ fn legacy_plaintext_state_migrates_to_encrypted_envelope_on_load() { let plaintext_bytes = serde_json::to_vec(&plaintext_state).expect("encode plaintext state"); std::fs::write(&state_path, &plaintext_bytes).expect("write plaintext state file"); + // Without the opt-in rollback flag the unauthenticated plaintext is refused + // (fail-closed), even in a non-production profile. + assert!( + load_engine_state_from_storage().is_err(), + "plaintext signer state must be rejected without the rollback opt-in" + ); + + // Plaintext load is an opt-in emergency-rollback path: development profile + // (selected by reset_for_tests) + this flag. + std::env::set_var(TBTC_SIGNER_PERMIT_PLAINTEXT_STATE_ROLLBACK_ENV, "true"); let loaded = load_engine_state_from_storage().expect("load and migrate legacy plaintext"); + std::env::remove_var(TBTC_SIGNER_PERMIT_PLAINTEXT_STATE_ROLLBACK_ENV); assert_eq!(loaded.sessions.len(), 1); assert_eq!(loaded.refresh_epoch_counter, 7); diff --git a/pkg/tbtc/signer/src/ffi.rs b/pkg/tbtc/signer/src/ffi.rs index 4715f05dce..6693a71ce0 100644 --- a/pkg/tbtc/signer/src/ffi.rs +++ b/pkg/tbtc/signer/src/ffi.rs @@ -2,6 +2,8 @@ use std::panic::{catch_unwind, AssertUnwindSafe}; use serde::de::DeserializeOwned; +use zeroize::Zeroize; + use crate::api::ErrorResponse; use crate::errors::EngineError; @@ -88,7 +90,13 @@ pub fn free_buffer(ptr: *mut u8, len: usize) { } unsafe { - drop(Box::from_raw(std::ptr::slice_from_raw_parts_mut(ptr, len))); + let mut buffer = Box::from_raw(std::ptr::slice_from_raw_parts_mut(ptr, len)); + // Wipe any plaintext secret material (e.g. FROST nonces, DKG/key-package + // bytes) before deallocation rather than trusting every FFI caller to do + // it correctly. Leaking a nonce after a share is produced can expose the + // signing share. `zeroize` is a volatile wipe the optimizer cannot elide. + buffer.zeroize(); + drop(buffer); } } From 6d2a4dff5595a222467a2281eabc3172623a1e53 Mon Sep 17 00:00:00 2001 From: maclane Date: Tue, 30 Jun 2026 13:28:58 -0500 Subject: [PATCH 02/12] fix(tbtc/signer): address self-review findings on the review-fix PR Follow-up to the #4124 review (multi-agent + Codex), which found regressions in the periodic-refresh change and a gap in the FFI wipe: 1. Refresh stale-retry (CONFIRMED, Codex P2): removing SessionConflict made refresh re-execute on a replayed/older request -- re-deriving stale shares and bumping the epoch. Now a fingerprint already in refresh_history (accepted but no longer current) is rejected as a stale retry; a genuinely-new fingerprint still proceeds (repeatable periodic reshares). RefreshHistoryRecord gains an optional request_fingerprint (serde-default, backward compatible). 2. Unbounded refresh_history (CONFIRMED): the removed guard was the only bound. Cap per-session history at MAX_REFRESH_HISTORY (256), dropping oldest; strictly-increasing epochs preserved so continuity still holds. 3. FFI incomplete wipe (PLAUSIBLE): to_ffi_buffer's into_boxed_slice() shrinks and reallocates when capacity > len (serde_json over-allocates), freeing the original secret buffer un-wiped. Copy into an exact boxed slice and zeroize the source Vec so no un-wiped copy survives. 4. Quarantine schema-mismatch test exercised plaintext-refusal, not schema validation, after the plaintext gate landed: opt into the rollback flag like its sibling so it tests the intended path. 5. Plaintext-gate doc comment overclaimed "compile-time disabled in release"; clarify the load-bearing guard is the runtime non-production check. Verified: cargo test (298 passed) + clippy -D warnings + fmt clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/tbtc/signer/src/engine/lifecycle.rs | 37 ++++++++++--- pkg/tbtc/signer/src/engine/persistence.rs | 9 ++-- pkg/tbtc/signer/src/engine/state.rs | 5 ++ pkg/tbtc/signer/src/engine/tests.rs | 65 +++++++++++++++++++++++ pkg/tbtc/signer/src/ffi.rs | 10 +++- 5 files changed, 114 insertions(+), 12 deletions(-) diff --git a/pkg/tbtc/signer/src/engine/lifecycle.rs b/pkg/tbtc/signer/src/engine/lifecycle.rs index e2917e3cb3..b53170c5c0 100644 --- a/pkg/tbtc/signer/src/engine/lifecycle.rs +++ b/pkg/tbtc/signer/src/engine/lifecycle.rs @@ -2,6 +2,12 @@ use super::*; +/// Upper bound on per-session `refresh_history` length. Older records are +/// dropped once this is exceeded, bounding persisted-state size for a long-lived +/// / frequently-refreshed session. Also bounds the stale-fingerprint detection +/// window (retries older than this many refreshes are no longer recognized). +const MAX_REFRESH_HISTORY: usize = 256; + pub(crate) fn canary_max_start_sign_round_p95_ms() -> u64 { signer_env_var(TBTC_SIGNER_CANARY_MAX_START_SIGN_ROUND_P95_MS_ENV) .and_then(|value| value.trim().parse::().ok()) @@ -393,18 +399,27 @@ pub fn refresh_shares(request: RefreshSharesRequest) -> Result Result MAX_REFRESH_HISTORY { + let excess = session.refresh_history.len() - MAX_REFRESH_HISTORY; + session.refresh_history.drain(0..excess); + } persist_engine_state_to_storage(&guard)?; record_hardening_telemetry(|telemetry| { telemetry.refresh_shares_success_total = diff --git a/pkg/tbtc/signer/src/engine/persistence.rs b/pkg/tbtc/signer/src/engine/persistence.rs index 3c05d26db4..6b04afe239 100644 --- a/pkg/tbtc/signer/src/engine/persistence.rs +++ b/pkg/tbtc/signer/src/engine/persistence.rs @@ -887,9 +887,12 @@ pub(crate) fn decode_encrypted_state_envelope( /// Plaintext state is UNAUTHENTICATED, so accepting it would let anyone who can /// write the state file forge it (cleared replay markers, attacker key material) /// without holding the state-encryption key. Per the secret-material hardening -/// plan this is an emergency-rollback-only path: compile-time disabled in -/// release builds, never permitted in a production profile, and otherwise gated -/// behind an explicit opt-in env flag. +/// plan this is an emergency-rollback-only path. The load-bearing guard is the +/// runtime non-production check (a production profile NEVER accepts plaintext, +/// regardless of build); it is additionally gated off in optimized builds via +/// `debug_assertions` and behind an explicit opt-in env flag. (Note: a release +/// build compiled with `debug-assertions = on` would still require both the +/// non-production profile and the opt-in flag.) fn legacy_plaintext_state_permitted() -> bool { cfg!(debug_assertions) && !signer_profile_is_production() diff --git a/pkg/tbtc/signer/src/engine/state.rs b/pkg/tbtc/signer/src/engine/state.rs index f145920e4a..995c940dfc 100644 --- a/pkg/tbtc/signer/src/engine/state.rs +++ b/pkg/tbtc/signer/src/engine/state.rs @@ -141,6 +141,11 @@ pub(crate) struct RefreshHistoryRecord { pub(crate) share_count: u16, #[serde(default, skip_serializing_if = "Option::is_none")] pub(crate) key_group: Option, + /// Fingerprint of the refresh request that produced this record, used to + /// reject stale / out-of-order retries of an already-accepted refresh. + /// Optional for backward compatibility with state written before this field. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub(crate) request_fingerprint: Option, } #[derive(Clone, Debug, Deserialize, Serialize)] diff --git a/pkg/tbtc/signer/src/engine/tests.rs b/pkg/tbtc/signer/src/engine/tests.rs index 65337bf864..c36cc9b0d6 100644 --- a/pkg/tbtc/signer/src/engine/tests.rs +++ b/pkg/tbtc/signer/src/engine/tests.rs @@ -9436,6 +9436,66 @@ fn finalize_sign_round_rejects_materially_different_retry_after_canonicalization assert!(matches!(err, EngineError::SessionConflict { .. })); } +#[test] +fn refresh_shares_allows_new_fingerprint_but_rejects_stale_retry() { + let _guard = lock_test_state(); + let state_path = configure_test_state_path("refresh_stale_retry"); + reset_for_tests(); + + let session_id = "session-refresh-stale".to_string(); + let req_a = RefreshSharesRequest { + session_id: session_id.clone(), + current_shares: vec![ShareMaterial { + identifier: 1, + encrypted_share_hex: "aaaa".to_string(), + }], + }; + let req_b = RefreshSharesRequest { + session_id: session_id.clone(), + current_shares: vec![ShareMaterial { + identifier: 1, + encrypted_share_hex: "bbbb".to_string(), + }], + }; + + // First refresh A. + assert_eq!( + refresh_shares(req_a.clone()) + .expect("refresh A") + .refresh_epoch, + 1 + ); + // Idempotent replay of A (most recent) returns the cached result, no new epoch. + assert_eq!( + refresh_shares(req_a.clone()) + .expect("idempotent replay of A") + .refresh_epoch, + 1 + ); + // A genuinely new fingerprint B is a real subsequent periodic refresh. + assert_eq!( + refresh_shares(req_b.clone()) + .expect("refresh B") + .refresh_epoch, + 2 + ); + // Idempotent replay of B (now most recent) returns the cached result. + assert_eq!( + refresh_shares(req_b) + .expect("idempotent replay of B") + .refresh_epoch, + 2 + ); + // A stale retry of A (already accepted, no longer most recent) is rejected -- + // it must NOT re-derive old shares or bump the epoch forward. + let err = refresh_shares(req_a).expect_err("stale retry of A must be rejected"); + assert!(matches!(err, EngineError::SessionConflict { .. })); + + reset_for_tests(); + cleanup_test_state_artifacts(&state_path); + clear_state_storage_policy_overrides(); +} + #[test] fn refresh_epoch_counter_persists_across_storage_reload() { let _guard = lock_test_state(); @@ -10169,7 +10229,12 @@ fn schema_mismatch_state_file_quarantines_and_resets_when_enabled() { let persisted_bytes = serde_json::to_vec(&persisted).expect("encode mismatched schema"); std::fs::write(&state_path, &persisted_bytes).expect("write mismatched schema state file"); + // Reach schema validation (the test's intent) by opting into the plaintext + // rollback path; otherwise the plaintext gate refuses the bytes before the + // schema check and the quarantine-and-reset would fire for the wrong reason. + std::env::set_var(TBTC_SIGNER_PERMIT_PLAINTEXT_STATE_ROLLBACK_ENV, "true"); let loaded = load_engine_state_from_storage().expect("recover from schema mismatch state"); + std::env::remove_var(TBTC_SIGNER_PERMIT_PLAINTEXT_STATE_ROLLBACK_ENV); assert!(loaded.sessions.is_empty()); assert_eq!(loaded.refresh_epoch_counter, 0); assert!(!state_path.exists()); diff --git a/pkg/tbtc/signer/src/ffi.rs b/pkg/tbtc/signer/src/ffi.rs index 6693a71ce0..8321476c1e 100644 --- a/pkg/tbtc/signer/src/ffi.rs +++ b/pkg/tbtc/signer/src/ffi.rs @@ -146,7 +146,7 @@ fn request_bytes<'a>(ptr: *const u8, len: usize) -> Result<&'a [u8], EngineError unsafe { Ok(std::slice::from_raw_parts(ptr, len)) } } -fn to_ffi_buffer(bytes: Vec) -> TbtcBuffer { +fn to_ffi_buffer(mut bytes: Vec) -> TbtcBuffer { let len = bytes.len(); if len == 0 { return TbtcBuffer { @@ -155,7 +155,13 @@ fn to_ffi_buffer(bytes: Vec) -> TbtcBuffer { }; } - let boxed = bytes.into_boxed_slice(); + // Copy into an exact-capacity boxed slice, then wipe the source Vec. + // `bytes.into_boxed_slice()` shrink-to-fits and reallocates when capacity > len + // (serde_json::to_vec over-allocates secret-bearing JSON), which would free the + // original secret buffer WITHOUT zeroizing it. free_buffer wipes the boxed + // slice on free; we wipe the source here so no un-zeroized copy survives. + let boxed: Box<[u8]> = bytes.as_slice().into(); + bytes.zeroize(); let ptr = Box::into_raw(boxed) as *mut u8; TbtcBuffer { ptr, len } From 07238e63abca25b73b8ab2f2e32e2736f7a0e8d4 Mon Sep 17 00:00:00 2001 From: maclane Date: Tue, 30 Jun 2026 13:49:32 -0500 Subject: [PATCH 03/12] fix(tbtc/signer): gate plaintext rollback-path tests to debug builds Codex re-review: the rollback-path tests opt into the plaintext path, which legacy_plaintext_state_permitted() compiles out in release (cfg!(debug_assertions) is false), so under `cargo test --release` they fail at the post-acceptance assertions. cfg-gate the three plaintext-acceptance tests to debug_assertions; release always refuses plaintext before those assertions, so there is nothing for them to cover there. Verified: cargo test (298 passed, debug) + cargo test --release (295 passed; the 3 tests correctly compiled out) + clippy --release -D warnings clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/tbtc/signer/src/engine/tests.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/tbtc/signer/src/engine/tests.rs b/pkg/tbtc/signer/src/engine/tests.rs index c36cc9b0d6..77e8400de0 100644 --- a/pkg/tbtc/signer/src/engine/tests.rs +++ b/pkg/tbtc/signer/src/engine/tests.rs @@ -10159,6 +10159,10 @@ fn truncated_state_file_quarantines_and_resets_when_enabled() { clear_state_storage_policy_overrides(); } +// The plaintext-acceptance path is debug-only (legacy_plaintext_state_permitted +// gates on cfg!(debug_assertions)), so this rollback-path test is too; in a +// release build the bytes are always refused before schema validation is reached. +#[cfg(debug_assertions)] #[test] fn schema_mismatch_state_file_fails_closed_by_default() { let _guard = lock_test_state(); @@ -10202,6 +10206,7 @@ fn schema_mismatch_state_file_fails_closed_by_default() { clear_state_storage_policy_overrides(); } +#[cfg(debug_assertions)] // plaintext rollback path is debug-only; see legacy_plaintext_state_permitted #[test] fn schema_mismatch_state_file_quarantines_and_resets_when_enabled() { let _guard = lock_test_state(); @@ -10329,6 +10334,7 @@ fn persisted_state_is_encrypted_envelope() { clear_state_storage_policy_overrides(); } +#[cfg(debug_assertions)] // plaintext rollback path is debug-only; see legacy_plaintext_state_permitted #[test] fn legacy_plaintext_state_migrates_to_encrypted_envelope_on_load() { let _guard = lock_test_state(); From cba4cd16629a1bb35205c647a5cf1132fec2d243 Mon Sep 17 00:00:00 2001 From: maclane Date: Tue, 30 Jun 2026 16:50:32 -0500 Subject: [PATCH 04/12] fix(tbtc/signer): wire plaintext rollback opt-in through init config Codex re-review: a host that installs an init-time config has signer_env_var read the installed config, not the process environment, so the documented TBTC_SIGNER_PERMIT_PLAINTEXT_STATE_ROLLBACK env flag had no effect for it -- the emergency plaintext migration was impossible to enable via the configured-host path. Add permit_plaintext_state_rollback to InitSignerConfigRequest and map it in config_values_from_request, mirroring the other enforcement flags. Verified: cargo test (298 passed) + clippy -D warnings + fmt clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/tbtc/signer/src/api.rs | 2 ++ pkg/tbtc/signer/src/engine/init_config.rs | 8 ++++++++ pkg/tbtc/signer/src/engine/tests.rs | 9 +++++++++ 3 files changed, 19 insertions(+) diff --git a/pkg/tbtc/signer/src/api.rs b/pkg/tbtc/signer/src/api.rs index 7e59bf32ff..9334ec8760 100644 --- a/pkg/tbtc/signer/src/api.rs +++ b/pkg/tbtc/signer/src/api.rs @@ -799,6 +799,8 @@ pub struct InitSignerConfigRequest { #[serde(default, skip_serializing_if = "Option::is_none")] pub state_corrupt_backup_limit: Option, #[serde(default, skip_serializing_if = "Option::is_none")] + pub permit_plaintext_state_rollback: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] pub max_sessions: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub max_live_interactive_sessions: Option, diff --git a/pkg/tbtc/signer/src/engine/init_config.rs b/pkg/tbtc/signer/src/engine/init_config.rs index d20b6f42c0..36d193c2ef 100644 --- a/pkg/tbtc/signer/src/engine/init_config.rs +++ b/pkg/tbtc/signer/src/engine/init_config.rs @@ -241,6 +241,14 @@ pub(crate) fn config_values_from_request( TBTC_SIGNER_ENFORCE_SIGNING_POLICY_FIREWALL_ENV, request.enforce_signing_policy_firewall, ); + // Make the emergency plaintext-state rollback opt-in reachable for hosts that + // configure via init-time config (where signer_env_var reads the installed + // config, not the process environment), not just raw env. + insert_bool( + &mut values, + TBTC_SIGNER_PERMIT_PLAINTEXT_STATE_ROLLBACK_ENV, + request.permit_plaintext_state_rollback, + ); insert_bool( &mut values, TBTC_SIGNER_ENABLE_AUTO_QUARANTINE_ENV, diff --git a/pkg/tbtc/signer/src/engine/tests.rs b/pkg/tbtc/signer/src/engine/tests.rs index 77e8400de0..3efccccd37 100644 --- a/pkg/tbtc/signer/src/engine/tests.rs +++ b/pkg/tbtc/signer/src/engine/tests.rs @@ -11076,6 +11076,7 @@ fn init_signer_config_canonicalizes_list_and_bool_encodings() { enable_auto_quarantine: Some(false), auto_quarantine_dao_allowlist_identifiers: Some(vec![3, 1, 2, 2]), policy_allowed_script_classes: Some(vec!["P2TR".to_string(), "p2wpkh".to_string()]), + permit_plaintext_state_rollback: Some(true), ..InitSignerConfigRequest::default() }) .expect("convert request"); @@ -11100,6 +11101,14 @@ fn init_signer_config_canonicalizes_list_and_bool_encodings() { .map(String::as_str), Some("P2TR,p2wpkh") ); + // The plaintext rollback opt-in is reachable via init-time config, not only + // the process environment. + assert_eq!( + values + .get(TBTC_SIGNER_PERMIT_PLAINTEXT_STATE_ROLLBACK_ENV) + .map(String::as_str), + Some("true") + ); let empty_list = config_values_from_request(&InitSignerConfigRequest { admission_required_identifiers: Some(Vec::new()), From 39bed8be385b01c8b22f9851a110e1bf4372b493 Mon Sep 17 00:00:00 2001 From: maclane Date: Tue, 30 Jun 2026 17:08:44 -0500 Subject: [PATCH 05/12] fix(tbtc/signer): backfill legacy refresh fingerprint before overwriting Codex re-review: a session loaded from state written before RefreshHistoryRecord.request_fingerprint deserializes its history records with None, so the last accepted refresh fingerprint survives only in session.refresh_request_fingerprint. The first post-upgrade periodic refresh overwrote that value, so a delayed retry of the pre-upgrade refresh no longer matched the history scan and was accepted as a new epoch instead of SessionConflict. Before overwriting refresh_request_fingerprint, backfill the previously-accepted fingerprint onto the most-recent history record when it is not already tracked (the legacy None case). Adds a regression test that simulates pre-upgrade state (strip history fingerprints) and verifies a retry of the old refresh is rejected after a new one. Verified: cargo test (299 debug / 296 release) + clippy -D warnings + fmt clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/tbtc/signer/src/engine/lifecycle.rs | 18 +++++++++ pkg/tbtc/signer/src/engine/tests.rs | 52 +++++++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/pkg/tbtc/signer/src/engine/lifecycle.rs b/pkg/tbtc/signer/src/engine/lifecycle.rs index b53170c5c0..06bb6d1eb0 100644 --- a/pkg/tbtc/signer/src/engine/lifecycle.rs +++ b/pkg/tbtc/signer/src/engine/lifecycle.rs @@ -464,6 +464,24 @@ pub fn refresh_shares(request: RefreshSharesRequest) -> Result Date: Tue, 30 Jun 2026 17:28:51 -0500 Subject: [PATCH 06/12] fix(tbtc/signer): preserve total refresh count across history pruning Codex re-review: refresh_cadence_status reported refresh_count as session.refresh_history.len(), which the MAX_REFRESH_HISTORY cap bounds -- so a long-lived session that ran more than 256 refreshes showed a stuck count of 256 even as later refreshes succeeded. Add a monotonic SessionState.refresh_count (and its PersistedSessionState mirror, serde-default for back-compat, plus both conversions) incremented on each accepted refresh and backfilled from the retained history length for legacy state; report it from refresh_cadence_status instead of the pruned history length. Adds a regression test that prunes history and asserts the reported count is the true total. Verified: cargo test (300 passed) + clippy -D warnings + fmt clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/tbtc/signer/src/engine/lifecycle.rs | 9 ++++- pkg/tbtc/signer/src/engine/persistence.rs | 4 +++ pkg/tbtc/signer/src/engine/state.rs | 4 +++ pkg/tbtc/signer/src/engine/tests.rs | 40 +++++++++++++++++++++++ 4 files changed, 56 insertions(+), 1 deletion(-) diff --git a/pkg/tbtc/signer/src/engine/lifecycle.rs b/pkg/tbtc/signer/src/engine/lifecycle.rs index 06bb6d1eb0..37a95a1c2a 100644 --- a/pkg/tbtc/signer/src/engine/lifecycle.rs +++ b/pkg/tbtc/signer/src/engine/lifecycle.rs @@ -109,7 +109,7 @@ pub fn refresh_cadence_status( Ok(RefreshCadenceStatusResult { session_id: request.session_id, - refresh_count: session.refresh_history.len() as u64, + refresh_count: session.refresh_count, last_refresh_epoch: last_refresh_record .map(|record| record.refresh_epoch) .unwrap_or(0), @@ -482,6 +482,13 @@ pub fn refresh_shares(request: RefreshSharesRequest) -> Result, #[serde(default, skip_serializing_if = "Vec::is_empty")] pub(crate) refresh_history: Vec, + #[serde(default)] + pub(crate) refresh_count: u64, #[serde(default, skip_serializing_if = "Option::is_none")] pub(crate) emergency_rekey_event: Option, // Phase 7.1 interactive consumption markers - the ONLY durable @@ -1477,6 +1479,7 @@ impl TryFrom for SessionState { refresh_request_fingerprint: persisted.refresh_request_fingerprint, refresh_result: persisted.refresh_result, refresh_history: persisted.refresh_history, + refresh_count: persisted.refresh_count, emergency_rekey_event: persisted.emergency_rekey_event, // Live interactive state never restores: nonces are gone by // construction after a restart, so the attempt fails safe and @@ -1622,6 +1625,7 @@ impl TryFrom<&SessionState> for PersistedSessionState { refresh_request_fingerprint: session_state.refresh_request_fingerprint.clone(), refresh_result: session_state.refresh_result.clone(), refresh_history: session_state.refresh_history.clone(), + refresh_count: session_state.refresh_count, emergency_rekey_event: session_state.emergency_rekey_event.clone(), consumed_interactive_attempt_markers, aggregated_interactive_attempt_markers, diff --git a/pkg/tbtc/signer/src/engine/state.rs b/pkg/tbtc/signer/src/engine/state.rs index 995c940dfc..efe487e978 100644 --- a/pkg/tbtc/signer/src/engine/state.rs +++ b/pkg/tbtc/signer/src/engine/state.rs @@ -108,6 +108,10 @@ pub(crate) struct SessionState { pub(crate) refresh_request_fingerprint: Option, pub(crate) refresh_result: Option, pub(crate) refresh_history: Vec, + /// Monotonic count of accepted refreshes, independent of refresh_history + /// pruning (refresh_history is capped, so its length undercounts long-lived + /// sessions). Backfilled from history length when first incremented. + pub(crate) refresh_count: u64, pub(crate) emergency_rekey_event: Option, // Multi-seat: a process-global engine may hold several LOCAL members (seats) // signing the same session concurrently, each on its own attempt timeline. diff --git a/pkg/tbtc/signer/src/engine/tests.rs b/pkg/tbtc/signer/src/engine/tests.rs index aad69f654c..f1fe610591 100644 --- a/pkg/tbtc/signer/src/engine/tests.rs +++ b/pkg/tbtc/signer/src/engine/tests.rs @@ -700,6 +700,7 @@ fn persisted_session_state_fixture() -> PersistedSessionState { refresh_request_fingerprint: None, refresh_result: None, refresh_history: vec![], + refresh_count: 0, emergency_rekey_event: None, consumed_interactive_attempt_markers: vec![], aggregated_interactive_attempt_markers: vec![], @@ -9548,6 +9549,45 @@ fn refresh_shares_rejects_legacy_pre_upgrade_fingerprint_after_new_refresh() { clear_state_storage_policy_overrides(); } +#[test] +fn refresh_cadence_status_count_survives_history_pruning() { + let _guard = lock_test_state(); + let state_path = configure_test_state_path("refresh_count_pruning"); + reset_for_tests(); + + let session_id = "session-refresh-count".to_string(); + for hex in ["aaaa", "bbbb", "cccc"] { + refresh_shares(RefreshSharesRequest { + session_id: session_id.clone(), + current_shares: vec![ShareMaterial { + identifier: 1, + encrypted_share_hex: hex.to_string(), + }], + }) + .expect("refresh"); + } + + // Simulate the MAX_REFRESH_HISTORY prune (drop older records) without touching + // the monotonic refresh_count. + { + let mut guard = state().expect("engine state").lock().expect("engine lock"); + let session = guard.sessions.get_mut(&session_id).expect("session state"); + assert_eq!(session.refresh_count, 3); + session.refresh_history.drain(0..2); + } + + let status = refresh_cadence_status(RefreshCadenceStatusRequest { + session_id: session_id.clone(), + }) + .expect("cadence status"); + // Reports the true total (3), not the pruned history window (1). + assert_eq!(status.refresh_count, 3); + + reset_for_tests(); + cleanup_test_state_artifacts(&state_path); + clear_state_storage_policy_overrides(); +} + #[test] fn refresh_epoch_counter_persists_across_storage_reload() { let _guard = lock_test_state(); From 3efa362df033381d30c95084a86f7a47d71f9b86 Mon Sep 17 00:00:00 2001 From: maclane Date: Tue, 30 Jun 2026 17:49:06 -0500 Subject: [PATCH 07/12] fix(tbtc/signer): backfill refresh_count from history on legacy state load Codex re-review: refresh_count is #[serde(default)], so state written before the field existed deserializes it to 0 even when refresh_history holds accepted refreshes -- and since refresh_cadence_status now reports refresh_count, an upgraded node reported 0 until the next refresh. Backfill it from refresh_history.len() in the Persisted->Session conversion (evaluated before refresh_history is moved), with a regression test. Verified: cargo test + clippy -D warnings + fmt clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/tbtc/signer/src/engine/persistence.rs | 8 ++++- pkg/tbtc/signer/src/engine/tests.rs | 43 +++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/pkg/tbtc/signer/src/engine/persistence.rs b/pkg/tbtc/signer/src/engine/persistence.rs index 11f974e14d..bfc352591b 100644 --- a/pkg/tbtc/signer/src/engine/persistence.rs +++ b/pkg/tbtc/signer/src/engine/persistence.rs @@ -1478,8 +1478,14 @@ impl TryFrom for SessionState { tx_result: persisted.tx_result, refresh_request_fingerprint: persisted.refresh_request_fingerprint, refresh_result: persisted.refresh_result, + // Backfill from history length for state written before refresh_count + // existed (serde defaults it to 0), so refresh_cadence_status reports + // the true total immediately after upgrade rather than 0 until the next + // refresh. Evaluated before refresh_history is moved below. + refresh_count: persisted + .refresh_count + .max(persisted.refresh_history.len() as u64), refresh_history: persisted.refresh_history, - refresh_count: persisted.refresh_count, emergency_rekey_event: persisted.emergency_rekey_event, // Live interactive state never restores: nonces are gone by // construction after a restart, so the attempt fails safe and diff --git a/pkg/tbtc/signer/src/engine/tests.rs b/pkg/tbtc/signer/src/engine/tests.rs index f1fe610591..a5815df6de 100644 --- a/pkg/tbtc/signer/src/engine/tests.rs +++ b/pkg/tbtc/signer/src/engine/tests.rs @@ -9549,6 +9549,49 @@ fn refresh_shares_rejects_legacy_pre_upgrade_fingerprint_after_new_refresh() { clear_state_storage_policy_overrides(); } +#[test] +fn refresh_count_backfills_from_history_on_legacy_load() { + let _guard = lock_test_state(); + let state_path = configure_test_state_path("refresh_count_legacy_load"); + reset_for_tests(); + + let session_id = "session-refresh-count-legacy".to_string(); + for hex in ["aaaa", "bbbb"] { + refresh_shares(RefreshSharesRequest { + session_id: session_id.clone(), + current_shares: vec![ShareMaterial { + identifier: 1, + encrypted_share_hex: hex.to_string(), + }], + }) + .expect("refresh"); + } + + // Simulate state written before refresh_count existed: the field deserializes + // to 0 even though refresh_history already holds accepted refreshes. + { + let mut guard = state().expect("engine state").lock().expect("engine lock"); + let session = guard.sessions.get_mut(&session_id).expect("session state"); + assert_eq!(session.refresh_count, 2); + assert_eq!(session.refresh_history.len(), 2); + session.refresh_count = 0; + persist_engine_state_to_storage(&guard).expect("persist legacy-shaped state"); + } + reload_state_from_storage_for_tests(); + + // On load, refresh_count is backfilled from history length, so cadence status + // reports the true total immediately -- not 0 until the next refresh. + let status = refresh_cadence_status(RefreshCadenceStatusRequest { + session_id: session_id.clone(), + }) + .expect("cadence status"); + assert_eq!(status.refresh_count, 2); + + reset_for_tests(); + cleanup_test_state_artifacts(&state_path); + clear_state_storage_policy_overrides(); +} + #[test] fn refresh_cadence_status_count_survives_history_pruning() { let _guard = lock_test_state(); From b32f5151c402511385dc85a08a3cdace68d76210 Mon Sep 17 00:00:00 2001 From: maclane Date: Tue, 30 Jun 2026 18:24:32 -0500 Subject: [PATCH 08/12] fix(tbtc/signer): harden signer FFI (secret fields, panic hook, quarantine recheck) Codex re-review of the signer FFI surface: 1. [P1] Secret request fields not zeroized (api.rs): SignShareRequest.nonces_hex and key_package_hex deserialized into plain Strings dropped without zeroization, retaining the one-time nonce / private key package in freed heap. Wrap both in Zeroizing (wiped on drop) + a redacting Debug so the secrets no longer leak via {:?} either. 2. [P1] Panic hook defeats payload redaction (ffi.rs): catch_unwind does not suppress Rust's default panic hook, so a panic payload (path/config/secret) prints to stderr before being converted to the redacted ErrorResponse. Install a panic hook at the FFI boundary that redacts the payload outside development. 3. [P2] Quarantine not rechecked on cached-round reuse (signing.rs): the matched- fingerprint reuse branch returned a fresh share without the new-round path's enforce_not_quarantined_identifiers check, so a participant quarantined after the round opened could still obtain a share. Apply the check on reuse too. Verified: cargo test (301 passed) + clippy -D warnings + fmt clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/tbtc/signer/src/api.rs | 22 +++++++++-- pkg/tbtc/signer/src/engine/signing.rs | 12 ++++++ pkg/tbtc/signer/src/engine/tests.rs | 53 ++++++++++++++------------- pkg/tbtc/signer/src/ffi.rs | 32 ++++++++++++++++ pkg/tbtc/signer/src/lib.rs | 4 +- 5 files changed, 92 insertions(+), 31 deletions(-) diff --git a/pkg/tbtc/signer/src/api.rs b/pkg/tbtc/signer/src/api.rs index 9334ec8760..0336247e70 100644 --- a/pkg/tbtc/signer/src/api.rs +++ b/pkg/tbtc/signer/src/api.rs @@ -1,4 +1,5 @@ use serde::{Deserialize, Serialize}; +use zeroize::Zeroizing; #[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)] pub struct DkgParticipant { @@ -129,16 +130,31 @@ pub struct NewSigningPackageResult { pub signing_package_hex: String, } -#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)] +#[derive(Clone, Deserialize, PartialEq, Eq, Serialize)] pub struct SignShareRequest { pub signing_package_hex: String, /// Secret one-time nonces returned by `GenerateNoncesAndCommitmentsResult`. /// /// This stateless endpoint cannot remember consumed nonces across FFI /// calls. The caller is cryptographically responsible for single use. - pub nonces_hex: String, + /// Wrapped in `Zeroizing` so the deserialized secret is wiped from the heap + /// on drop rather than lingering in freed memory after the share is produced. + pub nonces_hex: Zeroizing, pub key_package_identifier: String, - pub key_package_hex: String, + /// Secret private key-package material; `Zeroizing` so it is wiped on drop. + pub key_package_hex: Zeroizing, +} + +// Custom Debug redacts the secret fields (the derive would print them verbatim). +impl std::fmt::Debug for SignShareRequest { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("SignShareRequest") + .field("signing_package_hex", &self.signing_package_hex) + .field("nonces_hex", &"") + .field("key_package_identifier", &self.key_package_identifier) + .field("key_package_hex", &"") + .finish() + } } #[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)] diff --git a/pkg/tbtc/signer/src/engine/signing.rs b/pkg/tbtc/signer/src/engine/signing.rs index 631fac28c1..dfe68857e7 100644 --- a/pkg/tbtc/signer/src/engine/signing.rs +++ b/pkg/tbtc/signer/src/engine/signing.rs @@ -198,6 +198,18 @@ pub fn start_sign_round(mut request: StartSignRoundRequest) -> Result(response: &T) -> Result, .map_err(|e| EngineError::Internal(format!("failed to encode response: {e}"))) } +// Install a panic hook that redacts the panic payload outside the development +// profile. `catch_unwind` does not suppress Rust's default hook, so a panic +// carrying a path / config value / secret would otherwise print verbatim to +// stderr before it is converted to a redacted ErrorResponse. Installed once. +fn install_redacting_panic_hook() { + static INSTALLED: std::sync::Once = std::sync::Once::new(); + INSTALLED.call_once(|| { + let default_hook = std::panic::take_hook(); + std::panic::set_hook(Box::new(move |info| { + let development_profile = + crate::engine::signer_env_var(crate::engine::TBTC_SIGNER_PROFILE_ENV) + .map(|raw| { + raw.trim() + .eq_ignore_ascii_case(crate::engine::TBTC_SIGNER_PROFILE_DEVELOPMENT) + }) + .unwrap_or(false); + if development_profile { + default_hook(info); + } else if let Some(location) = info.location() { + eprintln!( + "panic at {}:{} (payload redacted)", + location.file(), + location.line() + ); + } else { + eprintln!("panic (payload redacted)"); + } + })); + }); +} + pub fn ffi_entry(f: F) -> TbtcSignerResult where F: FnOnce() -> Result, EngineError>, { + install_redacting_panic_hook(); match catch_unwind(AssertUnwindSafe(f)) { Ok(Ok(bytes)) => success_from_serialized(bytes), Ok(Err(err)) => error_result(err), diff --git a/pkg/tbtc/signer/src/lib.rs b/pkg/tbtc/signer/src/lib.rs index 170f948085..4bae4def89 100644 --- a/pkg/tbtc/signer/src/lib.rs +++ b/pkg/tbtc/signer/src/lib.rs @@ -1063,9 +1063,9 @@ mod tests { for id in signing_participants { let request = SignShareRequest { signing_package_hex: signing_package.signing_package_hex.clone(), - nonces_hex: nonces_by_participant[&id].clone(), + nonces_hex: nonces_by_participant[&id].clone().into(), key_package_identifier: part3_results[&id].key_package.identifier.clone(), - key_package_hex: part3_results[&id].key_package.data_hex.clone(), + key_package_hex: part3_results[&id].key_package.data_hex.clone().into(), }; let (status, payload) = call_ffi(&request, frost_tbtc_sign_share); assert_eq!(status, 0); From 6f92479246b773b5634e31edd9cc34984c160e1b Mon Sep 17 00:00:00 2001 From: maclane Date: Tue, 30 Jun 2026 18:40:22 -0500 Subject: [PATCH 09/12] fix(tbtc/signer): synthesize refresh fingerprint record for legacy empty history Codex re-review: legacy state written before refresh_history existed can carry refresh_request_fingerprint/refresh_result with an EMPTY refresh_history. The fingerprint backfill used refresh_history.last_mut(), which is None for empty history, so the previous fingerprint was not recorded and a delayed retry of that pre-upgrade refresh was re-executed as a new refresh. When there is no record to backfill onto, synthesize one from the cached refresh_result (prior epoch, keeping history strictly increasing) carrying the previous fingerprint, so stale retries stay rejected. Adds a regression test for the empty-history legacy shape. Verified: cargo test (302 passed) + clippy -D warnings + fmt clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/tbtc/signer/src/engine/lifecycle.rs | 14 ++++++++ pkg/tbtc/signer/src/engine/tests.rs | 47 +++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/pkg/tbtc/signer/src/engine/lifecycle.rs b/pkg/tbtc/signer/src/engine/lifecycle.rs index 37a95a1c2a..71508d1fcb 100644 --- a/pkg/tbtc/signer/src/engine/lifecycle.rs +++ b/pkg/tbtc/signer/src/engine/lifecycle.rs @@ -479,6 +479,20 @@ pub fn refresh_shares(request: RefreshSharesRequest) -> Result Date: Tue, 30 Jun 2026 19:08:55 -0500 Subject: [PATCH 10/12] fix(tbtc/signer): canonicalize StartSignRound message hex + seed bench env Two Codex-flagged issues in the non-interactive signing path: 1. start_sign_round fingerprinted and derived the round id from the raw request.message_hex string. hex::decode accepts mixed casing, so two calls carrying identical message bytes but different hex casing derived different fingerprints/round ids and a semantically identical retry was rejected as a SessionConflict. Lowercase message_hex right after it validates as hex, mirroring the interactive path (interactive.rs), so the fingerprint and derive_round_id are casing-independent. 2. The README-documented `cargo bench --features bench-restart-hook --bench phase5_roast` failed in a clean shell: ensure_benchmark_environment never set TBTC_SIGNER_PROFILE (missing -> production) or TBTC_SIGNER_STATE_ENCRYPTION_KEY_HEX (required by the default `env` state-key provider), so the first RunDkg persist aborted. Seed profile=development, provider=env, and a fixed dev key. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/tbtc/signer/benches/phase5_roast.rs | 12 ++++++++++++ pkg/tbtc/signer/src/engine/signing.rs | 7 +++++++ 2 files changed, 19 insertions(+) diff --git a/pkg/tbtc/signer/benches/phase5_roast.rs b/pkg/tbtc/signer/benches/phase5_roast.rs index 68255da742..af6e93e390 100644 --- a/pkg/tbtc/signer/benches/phase5_roast.rs +++ b/pkg/tbtc/signer/benches/phase5_roast.rs @@ -282,6 +282,18 @@ fn ensure_benchmark_environment() { std::env::set_var("TBTC_SIGNER_MAX_SESSIONS", "200000"); std::env::set_var("TBTC_SIGNER_ALLOW_BOOTSTRAP", "true"); std::env::set_var("TBTC_SIGNER_ALLOW_BENCH_RESTART_HOOK", "true"); + // The signer treats a missing profile as production, and the default + // `env` state-key provider requires an encryption key for persistence. + // Seed both (and pin the provider) so the README-documented + // `cargo bench --features bench-restart-hook --bench phase5_roast` + // runs in a clean shell without any pre-set TBTC_SIGNER_* variables; + // otherwise the first RunDkg persist fails. + std::env::set_var("TBTC_SIGNER_PROFILE", "development"); + std::env::set_var("TBTC_SIGNER_STATE_KEY_PROVIDER", "env"); + std::env::set_var( + "TBTC_SIGNER_STATE_ENCRYPTION_KEY_HEX", + "0c9258935f0a30c065befcd746cb1564e9f3c91936c0f0f1c78853fa2d6713dc", + ); }); } diff --git a/pkg/tbtc/signer/src/engine/signing.rs b/pkg/tbtc/signer/src/engine/signing.rs index dfe68857e7..f412876a95 100644 --- a/pkg/tbtc/signer/src/engine/signing.rs +++ b/pkg/tbtc/signer/src/engine/signing.rs @@ -23,6 +23,13 @@ pub fn start_sign_round(mut request: StartSignRoundRequest) -> Result Date: Tue, 30 Jun 2026 19:44:13 -0500 Subject: [PATCH 11/12] fix(tbtc/signer): preserve legacy mixed-case message fingerprints The prior commit lowercases message_hex before computing every canonical and legacy fingerprint. That regressed the upgrade path: when a node upgrades with an active sign round that a previous build started using mixed/uppercase message_hex, the persisted sign_request_fingerprint was computed over that original casing. The same retry then matched none of the fingerprints and fell through to SessionConflict instead of reusing the cached round. Capture the pre-canonicalization casing and, only when it differs from the lowercased form, compute the four legacy fingerprint shapes over the original casing and accept them in the cached-round match. A match migrates the stored fingerprint to the canonical lowercase form, so the compatibility cost is paid once per in-flight round. Adds a regression test that persists a mixed-case fingerprint and asserts the retry reuses the round and migrates it. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/tbtc/signer/src/engine/signing.rs | 39 +++++++++++- pkg/tbtc/signer/src/engine/tests.rs | 90 +++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 1 deletion(-) diff --git a/pkg/tbtc/signer/src/engine/signing.rs b/pkg/tbtc/signer/src/engine/signing.rs index f412876a95..67d053497a 100644 --- a/pkg/tbtc/signer/src/engine/signing.rs +++ b/pkg/tbtc/signer/src/engine/signing.rs @@ -29,6 +29,11 @@ pub fn start_sign_round(mut request: StartSignRoundRequest) -> Result Result = + if original_message_hex != request.message_hex { + let mut mixed_case_request = request.clone(); + mixed_case_request.message_hex = original_message_hex; + vec![ + start_sign_round_request_fingerprint(&mixed_case_request, 0)?, + start_sign_round_request_fingerprint( + &mixed_case_request, + mixed_case_request.member_identifier, + )?, + start_sign_round_request_fingerprint_including_transition_evidence( + &mixed_case_request, + 0, + )?, + start_sign_round_request_fingerprint_including_transition_evidence( + &mixed_case_request, + mixed_case_request.member_identifier, + )?, + ] + } else { + Vec::new() + }; let mut guard = state()? .lock() .map_err(|_| EngineError::Internal("engine lock poisoned".to_string()))?; @@ -182,7 +216,10 @@ pub fn start_sign_round(mut request: StartSignRoundRequest) -> Result Date: Tue, 30 Jun 2026 20:27:52 -0500 Subject: [PATCH 12/12] fix(tbtc/signer): preserve legacy refresh fingerprint without a cached result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The empty-history synthesize branch was guarded by `else if let Some(previous_result)`, so a legacy/degraded state that kept refresh_request_fingerprint but whose refresh_result deserialized to None (and whose refresh_history is empty) fell through: neither the last_mut() backfill nor the synthesize branch ran. The next refresh then overwrote the only copy of the old fingerprint, so a delayed retry of the pre-upgrade request was treated as fresh and advanced the epoch — a replay. Make the branch unconditional: synthesize a history record carrying the previous fingerprint whether or not a cached result is present. Prefer the result's epoch (when non-zero and below the new refresh) and share count; otherwise fall back to refresh_epoch-1 (min 1) and a zero share count. refresh_epoch_counter is persisted, so a prior accepted refresh implies refresh_epoch >= 2 and the fallback stays non-zero and strictly below the new record, keeping history monotonic. Adds a regression test (empty history + refresh_result=None) verified to fail against the pre-fix code (the stale retry was re-executed instead of rejected). Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/tbtc/signer/src/engine/lifecycle.rs | 35 ++++++++++++----- pkg/tbtc/signer/src/engine/tests.rs | 51 +++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 9 deletions(-) diff --git a/pkg/tbtc/signer/src/engine/lifecycle.rs b/pkg/tbtc/signer/src/engine/lifecycle.rs index 71508d1fcb..6be554fea0 100644 --- a/pkg/tbtc/signer/src/engine/lifecycle.rs +++ b/pkg/tbtc/signer/src/engine/lifecycle.rs @@ -479,17 +479,34 @@ pub fn refresh_shares(request: RefreshSharesRequest) -> Result= 2 + // and the fallback stays non-zero. + let previous_result = session.refresh_result.clone(); + let synthesized_epoch = previous_result + .as_ref() + .map(|previous| previous.refresh_epoch) + .filter(|&epoch| epoch != 0 && epoch < refresh_epoch) + .unwrap_or_else(|| refresh_epoch.saturating_sub(1).max(1)); + let synthesized_share_count = previous_result + .as_ref() + .map(|previous| previous.new_shares.len().min(u16::MAX as usize) as u16) + .unwrap_or(0); session.refresh_history.push(RefreshHistoryRecord { - refresh_epoch: previous_result.refresh_epoch, + refresh_epoch: synthesized_epoch, refreshed_at_unix: now_unix(), - share_count: previous_result.new_shares.len().min(u16::MAX as usize) as u16, + share_count: synthesized_share_count, key_group: session.dkg_result.as_ref().map(|dkg| dkg.key_group.clone()), request_fingerprint: Some(previous_fingerprint), }); diff --git a/pkg/tbtc/signer/src/engine/tests.rs b/pkg/tbtc/signer/src/engine/tests.rs index 8ab718118b..2f0522ee47 100644 --- a/pkg/tbtc/signer/src/engine/tests.rs +++ b/pkg/tbtc/signer/src/engine/tests.rs @@ -9635,6 +9635,57 @@ fn refresh_shares_rejects_legacy_fingerprint_with_empty_history() { clear_state_storage_policy_overrides(); } +#[test] +fn refresh_shares_rejects_legacy_fingerprint_with_empty_history_and_no_result() { + let _guard = lock_test_state(); + let state_path = configure_test_state_path("refresh_legacy_empty_history_no_result"); + reset_for_tests(); + + let session_id = "session-refresh-legacy-empty-no-result".to_string(); + let req_a = RefreshSharesRequest { + session_id: session_id.clone(), + current_shares: vec![ShareMaterial { + identifier: 1, + encrypted_share_hex: "aaaa".to_string(), + }], + }; + let req_b = RefreshSharesRequest { + session_id: session_id.clone(), + current_shares: vec![ShareMaterial { + identifier: 1, + encrypted_share_hex: "bbbb".to_string(), + }], + }; + + refresh_shares(req_a.clone()).expect("refresh A"); + + // Simulate a truncated/legacy blob that kept A's fingerprint but whose + // refresh_result deserialized to None and whose refresh_history is empty. + // The synthesize branch must still preserve A's fingerprint even without a + // cached result to derive the epoch/share_count from. + { + let mut guard = state().expect("engine state").lock().expect("engine lock"); + let session = guard.sessions.get_mut(&session_id).expect("session state"); + session.refresh_history.clear(); + session.refresh_result = None; + assert!(session.refresh_request_fingerprint.is_some()); + persist_engine_state_to_storage(&guard).expect("persist legacy-shaped state"); + } + reload_state_from_storage_for_tests(); + + // Refresh B synthesizes a history record carrying A's fingerprint (falling + // back to a below-B epoch since there is no cached result) before + // overwriting the fingerprint, so a delayed retry of A is still rejected as + // stale instead of being re-executed as a new refresh. + assert_eq!(refresh_shares(req_b).expect("refresh B").refresh_epoch, 2); + let err = refresh_shares(req_a).expect_err("stale retry of A must be rejected"); + assert!(matches!(err, EngineError::SessionConflict { .. })); + + reset_for_tests(); + cleanup_test_state_artifacts(&state_path); + clear_state_storage_policy_overrides(); +} + #[test] fn refresh_shares_rejects_legacy_pre_upgrade_fingerprint_after_new_refresh() { let _guard = lock_test_state();