From 939e9da774380b51391a95f214dcfaaa98a1074b Mon Sep 17 00:00:00 2001 From: Kyle Santiago Date: Thu, 22 Oct 2026 12:35:57 -0400 Subject: [PATCH 1/2] Refuse an enclave PSBT whose fee the policy does not bound --- keep-enclave/enclave/src/policy.rs | 29 ++++++++++++++++ keep-enclave/enclave/src/vsock_server.rs | 43 ++++++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/keep-enclave/enclave/src/policy.rs b/keep-enclave/enclave/src/policy.rs index a9d8be19..3e282849 100644 --- a/keep-enclave/enclave/src/policy.rs +++ b/keep-enclave/enclave/src/policy.rs @@ -23,6 +23,7 @@ pub enum PolicyRule { MaxPerMinute(u32), MaxPerHour(u32), MaxPerDay(u32), + MaxFeeSats(u64), AddressAllowlist(Vec), AddressBlocklist(Vec), AllowedHours { start: u8, end: u8 }, @@ -49,6 +50,7 @@ pub enum PolicyDecision { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum PolicyDenyReason { AmountExceeded, + FeeExceeded, RateLimitExceeded, AddressNotAllowed, AddressBlocked, @@ -62,6 +64,7 @@ impl std::fmt::Display for PolicyDenyReason { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::AmountExceeded => write!(f, "amount exceeds limit"), + Self::FeeExceeded => write!(f, "fee exceeds limit"), Self::RateLimitExceeded => write!(f, "rate limit exceeded"), Self::AddressNotAllowed => write!(f, "address not in allowlist"), Self::AddressBlocked => write!(f, "address is blocked"), @@ -76,6 +79,7 @@ impl std::fmt::Display for PolicyDenyReason { pub struct SigningContext<'a> { pub key_id: &'a str, pub amount_sats: Option, + pub fee_sats: Option, pub destination: Option<&'a str>, pub event_kind: Option, pub timestamp: u64, @@ -101,6 +105,12 @@ impl PolicyEngine { self.rate_limiter.clear(); } + /// A fee is bounded only by a rule the policy cannot skip. An `Or` bounds + /// it when every branch does, because one open branch would authorize any fee. + pub fn bounds_fee(&self) -> bool { + self.policies.iter().any(|policy| policy.rules.iter().any(rule_bounds_fee)) + } + pub fn evaluate(&mut self, ctx: &SigningContext<'_>) -> PolicyDecision { if self.policies.is_empty() { return PolicyDecision::Deny(PolicyDenyReason::ExplicitDeny); @@ -147,6 +157,11 @@ impl PolicyEngine { ctx.amount_sats.map(|a| a <= *max).unwrap_or(true) } + PolicyRule::MaxFeeSats(max) => match ctx.fee_sats { + Some(fee) if fee > *max => return Err(PolicyDenyReason::FeeExceeded), + _ => true, + }, + PolicyRule::MaxPerMinute(max) => { self.rate_limiter.check_minute(ctx.key_id, *max) } @@ -220,6 +235,15 @@ impl PolicyEngine { } } +fn rule_bounds_fee(rule: &PolicyRule) -> bool { + match rule { + PolicyRule::MaxFeeSats(_) | PolicyRule::MaxAmountSats(_) => true, + PolicyRule::And(rules) => rules.iter().any(rule_bounds_fee), + PolicyRule::Or(rules) => !rules.is_empty() && rules.iter().all(rule_bounds_fee), + _ => false, + } +} + impl Default for PolicyEngine { fn default() -> Self { Self::new() @@ -240,6 +264,7 @@ mod tests { let ctx = SigningContext { key_id: "test", amount_sats: Some(1), + fee_sats: None, destination: None, event_kind: None, timestamp: now(), @@ -264,6 +289,7 @@ mod tests { let ctx_ok = SigningContext { key_id: "test", amount_sats: Some(500), + fee_sats: None, destination: None, event_kind: None, timestamp: now(), @@ -273,6 +299,7 @@ mod tests { let ctx_fail = SigningContext { key_id: "test", amount_sats: Some(2000), + fee_sats: None, destination: None, event_kind: None, timestamp: now(), @@ -299,6 +326,7 @@ mod tests { let ctx_ok = SigningContext { key_id: "test", amount_sats: None, + fee_sats: None, destination: Some("bc1allowed"), event_kind: None, timestamp: now(), @@ -308,6 +336,7 @@ mod tests { let ctx_fail = SigningContext { key_id: "test", amount_sats: None, + fee_sats: None, destination: Some("bc1blocked"), event_kind: None, timestamp: now(), diff --git a/keep-enclave/enclave/src/vsock_server.rs b/keep-enclave/enclave/src/vsock_server.rs index a2f85151..24c96f55 100644 --- a/keep-enclave/enclave/src/vsock_server.rs +++ b/keep-enclave/enclave/src/vsock_server.rs @@ -427,6 +427,7 @@ impl VsockServer { let ctx = SigningContext { key_id: &req.key_id, amount_sats: req.amount_sats, + fee_sats: None, destination: req.destination.as_deref(), event_kind: req.event_kind, timestamp, @@ -529,6 +530,12 @@ impl VsockServer { total_spend: u64, timestamp: u64, ) -> Option { + if analysis.fee_sats > 0 && !self.policy_engine.bounds_fee() { + return Some(EnclaveResponse::Error { + code: ErrorCode::PolicyDenied, + message: "fee is not bounded".into(), + }); + } let named: Vec = analysis .destinations .iter() @@ -547,6 +554,7 @@ impl VsockServer { let ctx = SigningContext { key_id, amount_sats: Some(total_spend), + fee_sats: Some(analysis.fee_sats), destination: addr, event_kind: None, timestamp, @@ -786,6 +794,41 @@ mod tests { assert!(denied(&sign(&mut server, &psbt))); } + #[test] + fn an_allowlist_alone_does_not_authorize_a_fee() { + let to = p2tr(other_key()); + let mut server = server(vec![PolicyRule::AddressAllowlist(vec![address(&to)])]); + let psbt = psbt(&[(p2tr(x_only(&SECRET)), 50_000)], &[(to, 40_000)]); + assert!(denied(&sign(&mut server, &psbt))); + } + + #[test] + fn a_fee_cap_allows_a_small_fee_and_refuses_a_large_one() { + let to = p2tr(other_key()); + let mut server = server(vec![ + PolicyRule::AddressAllowlist(vec![address(&to)]), + PolicyRule::MaxFeeSats(1_000), + ]); + let small = psbt(&[(p2tr(x_only(&SECRET)), 50_000)], &[(to.clone(), 49_000)]); + match sign(&mut server, &small) { + EnclaveResponse::SignedPsbt { signed_inputs, .. } => assert_eq!(signed_inputs, 1), + other => panic!("expected a signed PSBT, got {other:?}"), + } + let large = psbt(&[(p2tr(x_only(&SECRET)), 50_000)], &[(to, 40_000)]); + assert!(denied(&sign(&mut server, &large))); + } + + #[test] + fn an_or_with_an_allowlist_does_not_bound_the_fee() { + let to = p2tr(other_key()); + let mut server = server(vec![PolicyRule::Or(vec![ + PolicyRule::AddressAllowlist(vec![address(&to)]), + PolicyRule::MaxAmountSats(1_000_000), + ])]); + let psbt = psbt(&[(p2tr(x_only(&SECRET)), 50_000)], &[(to, 40_000)]); + assert!(denied(&sign(&mut server, &psbt))); + } + #[test] fn the_policy_applies_when_only_the_fee_leaves() { let (change, fp, path) = child(&SECRET, Network::Bitcoin, "m/86'/0'/0'/1/0"); From 922f2ee6fe074f3e65d80d50bace170fd91ab0ac Mon Sep 17 00:00:00 2001 From: Kyle Santiago Date: Thu, 22 Oct 2026 13:23:03 -0400 Subject: [PATCH 2/2] Count a fee bound only when the policy cannot skip it --- keep-enclave/enclave/src/policy.rs | 135 +++++++++++++++++++++-- keep-enclave/enclave/src/vsock_server.rs | 50 +++++++++ keep-enclave/host/src/protocol.rs | 14 +++ 3 files changed, 190 insertions(+), 9 deletions(-) diff --git a/keep-enclave/enclave/src/policy.rs b/keep-enclave/enclave/src/policy.rs index 3e282849..481914a9 100644 --- a/keep-enclave/enclave/src/policy.rs +++ b/keep-enclave/enclave/src/policy.rs @@ -23,7 +23,6 @@ pub enum PolicyRule { MaxPerMinute(u32), MaxPerHour(u32), MaxPerDay(u32), - MaxFeeSats(u64), AddressAllowlist(Vec), AddressBlocklist(Vec), AllowedHours { start: u8, end: u8 }, @@ -31,6 +30,7 @@ pub enum PolicyRule { AllowedEventKinds(Vec), And(Vec), Or(Vec), + MaxFeeSats(u64), } #[derive(Debug, Clone, Copy, Serialize, Deserialize)] @@ -105,10 +105,11 @@ impl PolicyEngine { self.rate_limiter.clear(); } - /// A fee is bounded only by a rule the policy cannot skip. An `Or` bounds - /// it when every branch does, because one open branch would authorize any fee. + /// A fee is bounded when no rule sequence can allow it. An amount cap counts + /// only under Deny or RequireApproval. Under Allow, a failed rule is skipped + /// and a later policy can still sign. pub fn bounds_fee(&self) -> bool { - self.policies.iter().any(|policy| policy.rules.iter().any(rule_bounds_fee)) + self.policies.iter().any(|policy| !policy_allows_unbounded_fee(policy)) } pub fn evaluate(&mut self, ctx: &SigningContext<'_>) -> PolicyDecision { @@ -235,13 +236,77 @@ impl PolicyEngine { } } -fn rule_bounds_fee(rule: &PolicyRule) -> bool { +struct FeeEffect { + pass: bool, + fail: bool, +} + +fn policy_allows_unbounded_fee(policy: &Policy) -> bool { + for rule in &policy.rules { + let effect = rule_fee_effect(rule); + if effect.fail && matches!(policy.action, PolicyAction::Allow) { + return true; + } + if !effect.pass { + return false; + } + } + true +} + +fn rule_fee_effect(rule: &PolicyRule) -> FeeEffect { match rule { - PolicyRule::MaxFeeSats(_) | PolicyRule::MaxAmountSats(_) => true, - PolicyRule::And(rules) => rules.iter().any(rule_bounds_fee), - PolicyRule::Or(rules) => !rules.is_empty() && rules.iter().all(rule_bounds_fee), - _ => false, + PolicyRule::MaxFeeSats(_) => FeeEffect { + pass: false, + fail: false, + }, + PolicyRule::MaxAmountSats(_) => FeeEffect { + pass: false, + fail: true, + }, + PolicyRule::And(rules) => and_fee_effect(rules), + PolicyRule::Or(rules) => or_fee_effect(rules), + _ => FeeEffect { + pass: true, + fail: true, + }, + } +} + +fn and_fee_effect(rules: &[PolicyRule]) -> FeeEffect { + let mut effect = FeeEffect { + pass: true, + fail: false, + }; + for rule in rules { + if !effect.pass { + break; + } + let child = rule_fee_effect(rule); + effect.fail |= child.fail; + effect.pass = child.pass; + } + effect +} + +fn or_fee_effect(rules: &[PolicyRule]) -> FeeEffect { + let mut effect = FeeEffect { + pass: false, + fail: false, + }; + let mut open = true; + for rule in rules { + if !open { + break; + } + let child = rule_fee_effect(rule); + effect.pass |= child.pass; + open = child.fail; } + if open { + effect.fail = true; + } + effect } impl Default for PolicyEngine { @@ -346,4 +411,56 @@ mod tests { PolicyDecision::Deny(_) )); } + + fn policy(action: PolicyAction, rules: Vec) -> Policy { + Policy { + name: "p".into(), + rules, + action, + } + } + + fn engine(policies: Vec) -> PolicyEngine { + let mut engine = PolicyEngine::new(); + engine.set_policies(PolicyConfig { policies }); + engine + } + + #[test] + fn an_amount_cap_bounds_the_fee_only_when_breaking_it_stops_the_spend() { + let cap = PolicyRule::MaxAmountSats(1_000); + assert!(!engine(vec![policy(PolicyAction::Allow, vec![cap.clone()])]).bounds_fee()); + assert!(engine(vec![policy(PolicyAction::Deny, vec![cap.clone()])]).bounds_fee()); + assert!(engine(vec![policy(PolicyAction::RequireApproval, vec![cap])]).bounds_fee()); + } + + #[test] + fn a_fee_cap_bounds_the_fee_only_when_evaluation_reaches_it() { + let fee = PolicyRule::MaxFeeSats(1_000); + let listed = PolicyRule::AddressAllowlist(vec!["bc1".into()]); + assert!(engine(vec![policy(PolicyAction::Allow, vec![fee.clone()])]).bounds_fee()); + assert!(!engine(vec![policy( + PolicyAction::Allow, + vec![listed.clone(), fee.clone()], + )]) + .bounds_fee()); + assert!(engine(vec![policy(PolicyAction::Allow, vec![fee, listed.clone()])]).bounds_fee()); + assert!(!engine(vec![ + policy(PolicyAction::Allow, vec![PolicyRule::MaxAmountSats(1_000)]), + policy(PolicyAction::Deny, vec![listed]), + ]) + .bounds_fee()); + } + + #[test] + fn max_fee_keeps_the_previous_rule_indexes() { + let allow = postcard::to_allocvec(&PolicyRule::AddressAllowlist(vec!["a".into()])).unwrap(); + assert_eq!(allow[0], 4); + match postcard::from_bytes::(&[4, 1, 1, b'a']).unwrap() { + PolicyRule::AddressAllowlist(list) => assert_eq!(list, ["a"]), + other => panic!("expected an allowlist, got {other:?}"), + } + let fee = postcard::to_allocvec(&PolicyRule::MaxFeeSats(9)).unwrap(); + assert_eq!(fee[0], 11); + } } diff --git a/keep-enclave/enclave/src/vsock_server.rs b/keep-enclave/enclave/src/vsock_server.rs index 24c96f55..676a2f6a 100644 --- a/keep-enclave/enclave/src/vsock_server.rs +++ b/keep-enclave/enclave/src/vsock_server.rs @@ -818,6 +818,56 @@ mod tests { assert!(denied(&sign(&mut server, &large))); } + #[test] + fn an_allow_amount_cap_does_not_bound_the_fee() { + let to = p2tr(other_key()); + let mut server = bare(); + server.handle_set_policy(PolicyConfig { + policies: vec![ + Policy { + name: "cap".into(), + rules: vec![PolicyRule::MaxAmountSats(1_000)], + action: PolicyAction::Allow, + }, + Policy { + name: "dest".into(), + rules: vec![PolicyRule::AddressAllowlist(vec![address(&to)])], + action: PolicyAction::Deny, + }, + ], + }); + server.signer.import_key("k", &SECRET).unwrap(); + let psbt = psbt(&[(p2tr(x_only(&SECRET)), 50_000)], &[(to, 1)]); + assert!(denied(&sign(&mut server, &psbt))); + } + + #[test] + fn a_fee_cap_after_a_skippable_rule_does_not_bound_the_fee() { + let to = p2tr(other_key()); + let other = p2tr(x_only(&[9u8; 32])); + let mut server = bare(); + server.handle_set_policy(PolicyConfig { + policies: vec![ + Policy { + name: "cap".into(), + rules: vec![ + PolicyRule::AddressAllowlist(vec![address(&other)]), + PolicyRule::MaxFeeSats(1_000), + ], + action: PolicyAction::Allow, + }, + Policy { + name: "dest".into(), + rules: vec![PolicyRule::AddressAllowlist(vec![address(&to)])], + action: PolicyAction::Deny, + }, + ], + }); + server.signer.import_key("k", &SECRET).unwrap(); + let psbt = psbt(&[(p2tr(x_only(&SECRET)), 50_000)], &[(to, 1)]); + assert!(denied(&sign(&mut server, &psbt))); + } + #[test] fn an_or_with_an_allowlist_does_not_bound_the_fee() { let to = p2tr(other_key()); diff --git a/keep-enclave/host/src/protocol.rs b/keep-enclave/host/src/protocol.rs index 911a5534..b17e7757 100644 --- a/keep-enclave/host/src/protocol.rs +++ b/keep-enclave/host/src/protocol.rs @@ -141,6 +141,7 @@ pub enum PolicyRule { AllowedEventKinds(Vec), And(Vec), Or(Vec), + MaxFeeSats(u64), } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -159,3 +160,16 @@ pub enum PolicyDecision { pub const VSOCK_PORT: u32 = 5000; pub const ENCLAVE_CID: u32 = 16; + +#[cfg(test)] +mod tests { + use super::PolicyRule; + + #[test] + fn max_fee_keeps_the_previous_rule_indexes() { + let allow = postcard::to_allocvec(&PolicyRule::AddressAllowlist(vec!["a".into()])).unwrap(); + assert_eq!(allow[0], 4); + let fee = postcard::to_allocvec(&PolicyRule::MaxFeeSats(9)).unwrap(); + assert_eq!(fee[0], 11); + } +}