diff --git a/pkg/tbtc/signer/README.md b/pkg/tbtc/signer/README.md index 46c59ef126..63ed74c845 100644 --- a/pkg/tbtc/signer/README.md +++ b/pkg/tbtc/signer/README.md @@ -142,8 +142,11 @@ Semantics: request is rejected. - The init validates enforcement-gated policy combinations (admission, signing-policy firewall, auto-quarantine) plus the provenance gate, so a - misconfigured signer fails at startup rather than at first signing. Since - production forces the provenance gate, production configs must carry a + misconfigured signer fails at startup rather than at first signing. Production + forces both the provenance gate and the signing-policy firewall; the firewall + resolves to conservative built-in defaults (standard tBTC script classes, + permissive numeric caps) so it needs no extra config to boot, while the + provenance gate requires production configs to carry a complete attestation set (`provenance_attestation_status`/`_payload`/ `_signature_hex`, `provenance_trust_root`, `min_approved_version`); the init-time pass does not exempt runtime re-checks — attestation TTL aging @@ -385,10 +388,15 @@ Scenario coverage and pass criteria: - `TBTC_SIGNER_ADMISSION_ALLOWLIST_IDENTIFIERS` (comma-separated; unset to disable, empty string is invalid) - Signing policy firewall config: - `TBTC_SIGNER_ENFORCE_SIGNING_POLICY_FIREWALL` - - `TBTC_SIGNER_POLICY_ALLOWED_SCRIPT_CLASSES` (comma-separated, e.g. `p2tr,p2wpkh`) - - `TBTC_SIGNER_POLICY_MAX_OUTPUT_COUNT` (required when firewall is enabled) - - `TBTC_SIGNER_POLICY_MAX_OUTPUT_VALUE_SATS` (required when firewall is enabled) - - `TBTC_SIGNER_POLICY_MAX_TOTAL_OUTPUT_VALUE_SATS` (required when firewall is enabled) + - `TBTC_SIGNER_POLICY_ALLOWED_SCRIPT_CLASSES` (comma-separated, e.g. + `p2tr,p2wpkh`; defaults to the standard tBTC output forms + `p2pkh,p2sh,p2wpkh,p2wsh,p2tr` when unset, failing closed on other forms) + - `TBTC_SIGNER_POLICY_MAX_OUTPUT_COUNT` (defaults to a conservative built-in + bound when unset) + - `TBTC_SIGNER_POLICY_MAX_OUTPUT_VALUE_SATS` (defaults to permissive/unbounded + when unset; operators should tighten per wallet sizing) + - `TBTC_SIGNER_POLICY_MAX_TOTAL_OUTPUT_VALUE_SATS` (defaults to + permissive/unbounded when unset) - `TBTC_SIGNER_POLICY_ALLOWED_UTC_START_HOUR` / `TBTC_SIGNER_POLICY_ALLOWED_UTC_END_HOUR` - Note: setting `ALLOWED_UTC_START_HOUR == ALLOWED_UTC_END_HOUR` opens a 24-hour window (all hours permitted). @@ -431,8 +439,10 @@ Scenario coverage and pass criteria: - `TBTC_SIGNER_CANARY_MAX_FINALIZE_SIGN_ROUND_P95_MS` - `TBTC_SIGNER_CANARY_MAX_POLICY_REJECT_RATE_BPS` - Known limitations (P0 scope): - - Policy gates default to disabled: provenance/admission/signing enforcement - gates require explicit `=true` env vars. + - Policy gates default to disabled in non-production profiles + (provenance/admission/signing enforcement gates require explicit `=true` + env vars). In a production profile the provenance gate and the + signing-policy firewall are force-enabled regardless. - `StartSignRound.attempt_transition_evidence.exclusion_evidence` schema: - `reason`: `coordinator_timeout` or `invalid_share_proof` - `excluded_member_identifiers`: members excluded from the next attempt diff --git a/pkg/tbtc/signer/src/engine/config.rs b/pkg/tbtc/signer/src/engine/config.rs index 79737296d9..be549ed425 100644 --- a/pkg/tbtc/signer/src/engine/config.rs +++ b/pkg/tbtc/signer/src/engine/config.rs @@ -289,28 +289,6 @@ pub(crate) fn parse_u64_from_env_with_default( Ok(parsed) } -pub(crate) fn parse_usize_from_env_required(env_name: &str) -> Result { - let raw_value = signer_env_var(env_name) - .ok_or_else(|| EngineError::Internal(format!("missing required env [{}]", env_name)))?; - raw_value.trim().parse::().map_err(|_| { - EngineError::Internal(format!( - "failed to parse usize env [{}] value [{}]", - env_name, raw_value - )) - }) -} - -pub(crate) fn parse_u64_from_env_required(env_name: &str) -> Result { - let raw_value = signer_env_var(env_name) - .ok_or_else(|| EngineError::Internal(format!("missing required env [{}]", env_name)))?; - raw_value.trim().parse::().map_err(|_| { - EngineError::Internal(format!( - "failed to parse u64 env [{}] value [{}]", - env_name, raw_value - )) - }) -} - pub(crate) fn parse_u8_from_env_optional(env_name: &str) -> Result, EngineError> { let Some(raw_value) = signer_env_var(env_name) else { return Ok(None); @@ -331,6 +309,22 @@ pub(crate) fn parse_u8_from_env_optional(env_name: &str) -> Result, E Ok(Some(parsed)) } +/// Like `parse_script_class_set_required`, but falls back to `default_classes` +/// when the env var is absent. An explicitly-set value is parsed normally (an +/// explicit empty value is still an error). +pub(crate) fn parse_script_class_set_with_default( + env_name: &str, + default_classes: &[&str], +) -> Result, EngineError> { + if signer_env_var(env_name).is_some() { + return parse_script_class_set_required(env_name); + } + Ok(default_classes + .iter() + .map(|class| class.to_ascii_lowercase()) + .collect()) +} + pub(crate) fn parse_script_class_set_required( env_name: &str, ) -> Result, EngineError> { diff --git a/pkg/tbtc/signer/src/engine/policy.rs b/pkg/tbtc/signer/src/engine/policy.rs index c13d2ec081..aea3f11218 100644 --- a/pkg/tbtc/signer/src/engine/policy.rs +++ b/pkg/tbtc/signer/src/engine/policy.rs @@ -4,6 +4,20 @@ use super::*; pub(crate) const BITCOIN_MAX_MONEY_SATS: u64 = 2_100_000_000_000_000; +/// Conservative built-in signing-policy-firewall defaults, applied when the +/// firewall is enforced (always in production, see `signing_policy_firewall_enforced`) +/// but the operator has not set explicit policy env. The script-class allowlist is +/// the meaningful default: it fails closed on any non-standard output form +/// (`classify_script_pubkey` returns "other"), which is the on-signer guard against +/// an authorized coordinator getting an unusual/unauthorized script signed. The +/// numeric caps default to permissive bounds (operators tighten them per wallet +/// sizing) -- a too-tight static cap would false-reject legitimate large +/// redemptions/sweeps, and `enforce_signing_message_binding_to_policy_checked_build_tx` +/// remains the primary control that the signed digest matches a policy-checked tx. +pub(crate) const DEFAULT_ALLOWED_SCRIPT_CLASSES: &[&str] = + &["p2pkh", "p2sh", "p2wpkh", "p2wsh", "p2tr"]; +pub(crate) const DEFAULT_MAX_OUTPUT_COUNT: usize = 10_000; + pub(crate) static POLICY_GATE_WARNING_EMITTED: OnceLock<()> = OnceLock::new(); pub(crate) static BUILD_TX_RATE_LIMITER: OnceLock> = OnceLock::new(); @@ -67,6 +81,15 @@ pub(crate) fn admission_policy_enforced() -> bool { } pub(crate) fn signing_policy_firewall_enforced() -> bool { + // Mirror provenance_gate_enforced(): the signing-policy firewall is always + // enforced in production. It resolves to conservative built-in policy + // defaults (see load_signing_policy_firewall_config) so production does not + // depend on every operator shipping explicit policy config -- closing the + // fail-open default without making firewall config mandatory to boot. + if signer_profile_is_production() { + return true; + } + signer_env_var(TBTC_SIGNER_ENFORCE_SIGNING_POLICY_FIREWALL_ENV) .map(|raw_value| truthy_env_flag(&raw_value)) .unwrap_or(false) @@ -256,13 +279,27 @@ pub(crate) fn load_signing_policy_firewall_config( return Ok(None); } - let allowed_script_classes = - parse_script_class_set_required(TBTC_SIGNER_POLICY_ALLOWED_SCRIPT_CLASSES_ENV)?; - let max_output_count = parse_usize_from_env_required(TBTC_SIGNER_POLICY_MAX_OUTPUT_COUNT_ENV)?; - let max_output_value_sats = - parse_u64_from_env_required(TBTC_SIGNER_POLICY_MAX_OUTPUT_VALUE_SATS_ENV)?; - let max_total_output_value_sats = - parse_u64_from_env_required(TBTC_SIGNER_POLICY_MAX_TOTAL_OUTPUT_VALUE_SATS_ENV)?; + // Resolve to conservative built-in defaults when explicit policy env is not + // set, so an enforced firewall (always on in production) does not require + // every operator to ship full policy config to boot. The script-class + // allowlist fails closed on non-standard forms; the numeric caps default + // permissive and are operator-tunable. + let allowed_script_classes = parse_script_class_set_with_default( + TBTC_SIGNER_POLICY_ALLOWED_SCRIPT_CLASSES_ENV, + DEFAULT_ALLOWED_SCRIPT_CLASSES, + )?; + let max_output_count = parse_usize_from_env_with_default( + TBTC_SIGNER_POLICY_MAX_OUTPUT_COUNT_ENV, + DEFAULT_MAX_OUTPUT_COUNT, + )?; + let max_output_value_sats = parse_u64_from_env_with_default( + TBTC_SIGNER_POLICY_MAX_OUTPUT_VALUE_SATS_ENV, + BITCOIN_MAX_MONEY_SATS, + )?; + let max_total_output_value_sats = parse_u64_from_env_with_default( + TBTC_SIGNER_POLICY_MAX_TOTAL_OUTPUT_VALUE_SATS_ENV, + BITCOIN_MAX_MONEY_SATS, + )?; let allowed_utc_start_hour = parse_u8_from_env_optional(TBTC_SIGNER_POLICY_ALLOWED_UTC_START_HOUR_ENV)?; let allowed_utc_end_hour = diff --git a/pkg/tbtc/signer/src/engine/tests.rs b/pkg/tbtc/signer/src/engine/tests.rs index 40d4b13780..5feef4944a 100644 --- a/pkg/tbtc/signer/src/engine/tests.rs +++ b/pkg/tbtc/signer/src/engine/tests.rs @@ -1632,6 +1632,63 @@ fn policy_bound_message_hex_from_tx_result(tx_result: &TransactionResult) -> Str hash_hex(&tx_bytes) } +#[test] +fn signing_policy_firewall_is_enforced_in_production_by_default() { + let _guard = lock_test_state(); + reset_for_tests(); + + // Development without the opt-in flag: not enforced (unchanged default). + std::env::remove_var(TBTC_SIGNER_ENFORCE_SIGNING_POLICY_FIREWALL_ENV); + assert!( + !signing_policy_firewall_enforced(), + "firewall must stay opt-in outside production" + ); + + // Production: always enforced, no flag required (mirrors the provenance gate). + std::env::set_var(TBTC_SIGNER_PROFILE_ENV, TBTC_SIGNER_PROFILE_PRODUCTION); + assert!( + signing_policy_firewall_enforced(), + "firewall must be force-enabled in production" + ); + + reset_for_tests(); +} + +#[test] +fn signing_policy_firewall_config_uses_builtin_defaults_in_production() { + let _guard = lock_test_state(); + reset_for_tests(); + std::env::set_var(TBTC_SIGNER_PROFILE_ENV, TBTC_SIGNER_PROFILE_PRODUCTION); + + // No explicit firewall policy env -> conservative built-in defaults, so a + // production signer boots without shipping full policy config. + for env in [ + TBTC_SIGNER_POLICY_ALLOWED_SCRIPT_CLASSES_ENV, + TBTC_SIGNER_POLICY_MAX_OUTPUT_COUNT_ENV, + TBTC_SIGNER_POLICY_MAX_OUTPUT_VALUE_SATS_ENV, + TBTC_SIGNER_POLICY_MAX_TOTAL_OUTPUT_VALUE_SATS_ENV, + ] { + std::env::remove_var(env); + } + + let config = load_signing_policy_firewall_config() + .expect("firewall config loads with built-in defaults") + .expect("firewall is enforced in production"); + + let expected_classes: std::collections::HashSet = DEFAULT_ALLOWED_SCRIPT_CLASSES + .iter() + .map(|class| class.to_string()) + .collect(); + assert_eq!(config.allowed_script_classes, expected_classes); + // "other" (non-standard) is not in the default allowlist -> fails closed. + assert!(!config.allowed_script_classes.contains("other")); + assert_eq!(config.max_output_count, DEFAULT_MAX_OUTPUT_COUNT); + assert_eq!(config.max_output_value_sats, BITCOIN_MAX_MONEY_SATS); + assert_eq!(config.max_total_output_value_sats, BITCOIN_MAX_MONEY_SATS); + + reset_for_tests(); +} + #[test] fn build_taproot_tx_signing_policy_firewall_rejects_disallowed_script_class() { let _guard = lock_test_state(); @@ -10846,19 +10903,19 @@ fn init_signer_config_rolls_back_install_when_policy_validation_fails() { let _clear = InstalledConfigClearGuard; clear_state_storage_policy_overrides(); - // Firewall enforcement on, but the required allowed-script-classes knob - // is absent from the same config (and, wholesale semantics, the - // environment cannot supply it) -> the loader rejects and the install - // must roll back. (Admission knobs would NOT trip this: that loader - // falls back to defaults for absent values.) + // Firewall enforcement on with an INVALID policy (a UTC start hour without a + // matching end hour) -> the loader rejects and the install must roll back. + // Absent firewall knobs no longer trip this: the loader now falls back to + // conservative built-in defaults, so only an explicitly-invalid value fails. let error = init_signer_config(InitSignerConfigRequest { profile: Some("development".to_string()), enforce_signing_policy_firewall: Some(true), + policy_allowed_utc_start_hour: Some(8), ..InitSignerConfigRequest::default() }) - .expect_err("incomplete firewall policy must fail the init"); + .expect_err("invalid firewall policy must fail the init"); assert!( - error.to_string().contains("missing required env"), + error.to_string().contains("must be configured together"), "unexpected error: {error}" );