From d50d95a7012017e8c3819634cd52fe5cd384e1f5 Mon Sep 17 00:00:00 2001 From: Kyle Santiago Date: Wed, 28 Oct 2026 13:56:36 -0400 Subject: [PATCH] Judge enclave raw messages and FROST shares only on what the enclave can read The enclave signs a raw Sign message as given, so it cannot tell what that message spends or which event it is. It judged the amount, address and event kind rules on fields the host sent with the request, and passed them when the host sent none; FROST shares carried none at all. SigningRequest no longer carries amount, destination or event kind, on either side of the wire. The policy engine now sees what the enclave itself read: an analyzed PSBT, or an unread request. An unread request fails MaxAmountSats and the address rules, with the policy's action deciding the outcome as for any failed rule. A MaxFeeSats anywhere in the loaded policy refuses it outright, since it may pay any fee. AllowedEventKinds fails every request, a PSBT included, so an Or holding it can no longer let a spend past the caps beside it. The rate, hour and weekend rules apply as before. The wire digest is updated on both sides, docs/ENCLAVE.md says which rules can judge which requests, and the stale enclave sign-psbt CLI example is removed. --- docs/ENCLAVE.md | 5 +- keep-cli/src/commands/enclave.rs | 6 - keep-enclave/enclave/src/policy.rs | 360 ++++++++++++++----- keep-enclave/enclave/src/vsock_server.rs | 195 ++++++---- keep-enclave/host/src/backend.rs | 3 - keep-enclave/host/src/mock.rs | 3 - keep-enclave/host/src/protocol.rs | 22 +- keep-enclave/host/tests/integration_tests.rs | 6 - 8 files changed, 399 insertions(+), 201 deletions(-) diff --git a/docs/ENCLAVE.md b/docs/ENCLAVE.md index 484c89dd..0f22f062 100644 --- a/docs/ENCLAVE.md +++ b/docs/ENCLAVE.md @@ -204,7 +204,6 @@ keep enclave verify --cid 16 # Verify attesta keep enclave generate-key --name mykey --cid 16 # Generate key in enclave keep enclave import-key --name enclavekey --from-vault vaultkey --cid 16 keep enclave sign --key mykey --message --cid 16 -keep enclave sign-psbt --key mykey --psbt tx.psbt --network testnet --cid 16 ``` Policy example (enforced inside enclave): @@ -214,7 +213,9 @@ PolicyRule::MaxAmountSats(1_000_000) // 0.01 BTC limit PolicyRule::MaxPerHour(10) // Rate limit ``` -A FROST share is imported sealed with its policy, the same way as a wallet key: seal the serialized key package with `EnvelopeEncryption::encrypt_for_enclave` and import it with `EnclaveClient::import_encrypted_frost_key`, which sends the public key package alongside in the clear. The first sealed key imported, wallet or FROST, sets the enclave's policy, and a key sealed to any other policy is refused. Both FROST rounds are held to that policy, and round 2 judges it again before making the share, so sessions opened together cannot all be signed past a rate limit; a session the policy refuses is closed. A FROST message is opaque to the enclave and carries no amount, destination or event kind, so only the rate, `AllowedHours` and `BlockWeekends` rules can stop a FROST share; amount, fee, address and event kind rules do not limit it. Plaintext `ImportFrostKey`, like `ImportKey`, is refused unless the enclave is built with the test-only `allow-plaintext-import` feature. +The amount, fee and address rules judge what the enclave reads from a PSBT it analyzes, never what the host says about a request. A raw `Sign` message and a FROST message are opaque to the enclave, so they fail `MaxAmountSats`, `AddressAllowlist` and `AddressBlocklist`, and a failed rule acts as it would for a PSBT that broke it: under a `Deny` policy the request is denied, under `RequireApproval` it is refused with "Requires approval", and under `Allow` evaluation moves on to the next policy. A `MaxFeeSats` anywhere in the policy, whatever its action and however it is nested, refuses every raw message and FROST message, since they may pay any fee. The rate, `AllowedHours` and `BlockWeekends` rules judge them as they judge a PSBT. To spend under amount, fee or address rules, sign a PSBT with a `SignPsbt` request (`EnclaveClient::sign_psbt`). No request carries an event kind the enclave can read, so `AllowedEventKinds` fails every request, a PSBT included. + +A FROST share is imported sealed with its policy, the same way as a wallet key: seal the serialized key package with `EnvelopeEncryption::encrypt_for_enclave` and import it with `EnclaveClient::import_encrypted_frost_key`, which sends the public key package alongside in the clear. The first sealed key imported, wallet or FROST, sets the enclave's policy, and a key sealed to any other policy is refused. Both FROST rounds are held to that policy, and round 2 judges it again before making the share, so sessions opened together cannot all be signed past a rate limit; a session the policy refuses is closed. A FROST message is judged like a raw `Sign` message, as described above. Plaintext `ImportFrostKey`, like `ImportKey`, is refused unless the enclave is built with the test-only `allow-plaintext-import` feature. Rate limits count each Sign or SignPsbt request a key signs (a PSBT counts once, however many inputs it signs) and each FROST signature share it makes, against the key's public key (a FROST share's group key) rather than the name it was imported under. The enclave keeps no count across restarts, so it cannot know what a key signed before it last started; it treats every rate limit as used up when a policy is loaded, which every launch needs before it can sign, and refills from there. A restart therefore buys nothing beyond the normal refill, at the cost of a wait after every launch: `MaxPerHour(10)` allows its first signature six minutes after the policy is loaded and its full ten an hour after. Each running enclave keeps its own count, so the limits bound one enclave, not every enclave that can unseal the key. diff --git a/keep-cli/src/commands/enclave.rs b/keep-cli/src/commands/enclave.rs index 7d2a5de1..befdd20e 100644 --- a/keep-cli/src/commands/enclave.rs +++ b/keep-cli/src/commands/enclave.rs @@ -294,9 +294,6 @@ pub fn cmd_enclave_sign( keep_enclave_host::EnclaveRequest::Sign(keep_enclave_host::SigningRequest { key_id: key.to_string(), message: message_bytes, - event_kind: None, - amount_sats: None, - destination: None, nonce: None, timestamp: None, }); @@ -324,9 +321,6 @@ pub fn cmd_enclave_sign( let request = keep_enclave_host::SigningRequest { key_id: key.to_string(), message: message_bytes, - event_kind: None, - amount_sats: None, - destination: None, nonce: None, timestamp: None, }; diff --git a/keep-enclave/enclave/src/policy.rs b/keep-enclave/enclave/src/policy.rs index fe4b7483..02151825 100644 --- a/keep-enclave/enclave/src/policy.rs +++ b/keep-enclave/enclave/src/policy.rs @@ -20,6 +20,11 @@ pub struct Policy { #[derive(Debug, Clone, Serialize, Deserialize)] pub enum PolicyRule { + /// What a PSBT takes out of the wallet, fee included, at most this. A raw + /// `Sign` message or a FROST share fails it, since the enclave cannot read + /// what they spend, and the address rules fail them the same way. A + /// `MaxFeeSats` anywhere in the policy refuses them outright, and + /// `AllowedEventKinds` fails every request. MaxAmountSats(u64), /// Sign or SignPsbt requests signed and FROST shares made by the key, this /// many a minute on average with bursts of up to this many, a PSBT counting @@ -67,6 +72,7 @@ pub enum PolicyDenyReason { EventKindNotAllowed, ExplicitDeny, PolicyDepthExceeded, + FeeUnknown, } impl std::fmt::Display for PolicyDenyReason { @@ -81,6 +87,7 @@ impl std::fmt::Display for PolicyDenyReason { Self::EventKindNotAllowed => write!(f, "event kind not allowed"), Self::ExplicitDeny => write!(f, "policy denies request"), Self::PolicyDepthExceeded => write!(f, "policy nesting depth exceeded"), + Self::FeeUnknown => write!(f, "fee cannot be read from the request"), } } } @@ -89,15 +96,31 @@ pub struct SigningContext<'a> { /// The signing key's public key, which rate limits count against, so a /// secret imported under several names shares its limits. pub pubkey: [u8; 32], - pub amount_sats: Option, - pub fee_sats: Option, - pub destination: Option<&'a str>, - pub event_kind: Option, + pub contents: Contents<'a>, pub timestamp: u64, } +/// What the enclave itself read from the request being judged. Nothing the +/// host says about a request counts, since the host is not trusted. +#[derive(Debug, Clone, Copy)] +pub enum Contents<'a> { + /// A raw `Sign` message or a FROST share. The enclave cannot tell what it + /// spends or which event it signs, so a rule on either fails it, and the + /// policy's action decides what that means. + Unread, + /// A PSBT the enclave analyzed: what leaves the wallet, the fee, and one + /// output it pays, or none when only the fee leaves. + Spend { + amount_sats: u64, + fee_sats: u64, + destination: Option<&'a str>, + }, +} + pub struct PolicyEngine { policies: Vec, + /// Whether any policy holds a `MaxFeeSats`, nested or not. + caps_fee: bool, rate_limiter: RateLimiter, } @@ -105,11 +128,13 @@ impl PolicyEngine { pub fn new() -> Self { Self { policies: Vec::new(), + caps_fee: false, rate_limiter: RateLimiter::new(), } } pub fn set_policies(&mut self, config: PolicyConfig) { + self.caps_fee = holds_fee_cap(&config.policies); self.policies = config.policies; // What was signed under a looser policy the host loaded first is not // carried over, so the new rules start with nothing left, as at launch. @@ -127,6 +152,11 @@ impl PolicyEngine { if self.policies.is_empty() { return PolicyDecision::Deny(PolicyDenyReason::ExplicitDeny); } + // A fee cap holds under any action and wherever it sits, and a message + // the enclave cannot read may pay any fee, so any cap refuses it. + if self.caps_fee && matches!(ctx.contents, Contents::Unread) { + return PolicyDecision::Deny(PolicyDenyReason::FeeUnknown); + } for i in 0..self.policies.len() { let policy = self.policies[i].clone(); let decision = self.check_policy(&policy, ctx); @@ -165,13 +195,19 @@ impl PolicyEngine { } let result = match rule { - PolicyRule::MaxAmountSats(max) => { - ctx.amount_sats.map(|a| a <= *max).unwrap_or(true) - } + PolicyRule::MaxAmountSats(max) => match ctx.contents { + Contents::Spend { amount_sats, .. } => amount_sats <= *max, + Contents::Unread => false, + }, - PolicyRule::MaxFeeSats(max) => match ctx.fee_sats { - Some(fee) if fee > *max => return Err(PolicyDenyReason::FeeExceeded), - _ => true, + PolicyRule::MaxFeeSats(max) => match ctx.contents { + Contents::Spend { fee_sats, .. } if fee_sats > *max => { + return Err(PolicyDenyReason::FeeExceeded) + } + Contents::Spend { .. } => true, + // `evaluate` refuses an unread request before any rule when a + // fee cap is loaded, so this only backs that up. + Contents::Unread => return Err(PolicyDenyReason::FeeUnknown), }, PolicyRule::MaxPerMinute(max) => { @@ -186,17 +222,27 @@ impl PolicyEngine { self.rate_limiter.allows(Window::Day, &ctx.pubkey, *max) } - PolicyRule::AddressAllowlist(allowed) => { - ctx.destination - .map(|d| allowed.iter().any(|a| a == d)) - .unwrap_or(true) - } + PolicyRule::AddressAllowlist(allowed) => match ctx.contents { + Contents::Spend { + destination: Some(d), + .. + } => allowed.iter().any(|a| a == d), + Contents::Spend { + destination: None, .. + } => true, + Contents::Unread => false, + }, - PolicyRule::AddressBlocklist(blocked) => { - ctx.destination - .map(|d| !blocked.iter().any(|b| b == d)) - .unwrap_or(true) - } + PolicyRule::AddressBlocklist(blocked) => match ctx.contents { + Contents::Spend { + destination: Some(d), + .. + } => !blocked.iter().any(|b| b == d), + Contents::Spend { + destination: None, .. + } => true, + Contents::Unread => false, + }, PolicyRule::AllowedHours { start, end } => { if *start > 23 || *end > 23 { @@ -215,11 +261,10 @@ impl PolicyEngine { day != 0 && day != 6 } - PolicyRule::AllowedEventKinds(kinds) => { - ctx.event_kind - .map(|k| kinds.contains(&k)) - .unwrap_or(true) - } + // The enclave reads no event kind from any request, so nothing can + // pass this rule, a PSBT included: passing would let an `Or` sign a + // spend past the caps beside it. + PolicyRule::AllowedEventKinds(_) => false, PolicyRule::And(rules) => { for r in rules { @@ -260,6 +305,20 @@ impl PolicyEngine { } } +/// Whether any rule in `policies` is a `MaxFeeSats`, at any depth. It walks +/// the rules without recursion, so no nesting can overflow the stack. +fn holds_fee_cap(policies: &[Policy]) -> bool { + let mut rules: Vec<&PolicyRule> = policies.iter().flat_map(|p| &p.rules).collect(); + while let Some(rule) = rules.pop() { + match rule { + PolicyRule::MaxFeeSats(_) => return true, + PolicyRule::And(nested) | PolicyRule::Or(nested) => rules.extend(nested), + _ => {} + } + } + false +} + fn collect_rate_limits(rule: &PolicyRule, depth: usize, limits: &mut BTreeSet<(Window, u32)>) { if depth == 0 { return; @@ -313,7 +372,7 @@ fn rule_fee_effect(rule: &PolicyRule) -> FeeEffect { pass: false, fail: false, }, - PolicyRule::MaxAmountSats(_) => FeeEffect { + PolicyRule::MaxAmountSats(_) | PolicyRule::AllowedEventKinds(_) => FeeEffect { pass: false, fail: true, }, @@ -376,93 +435,188 @@ mod tests { 1703001600 } + fn spend(amount_sats: u64, destination: Option<&str>) -> SigningContext<'_> { + SigningContext { + pubkey: [7u8; 32], + contents: Contents::Spend { + amount_sats, + fee_sats: 100, + destination, + }, + timestamp: now(), + } + } + + fn unread() -> SigningContext<'static> { + SigningContext { + pubkey: [7u8; 32], + contents: Contents::Unread, + timestamp: now(), + } + } + + fn denies(engine: &mut PolicyEngine, ctx: &SigningContext<'_>) -> bool { + matches!(engine.evaluate(ctx), PolicyDecision::Deny(_)) + } + #[test] fn an_empty_policy_denies() { let mut engine = PolicyEngine::new(); - let ctx = SigningContext { - pubkey: [7u8; 32], - amount_sats: Some(1), - fee_sats: None, - destination: None, - event_kind: None, - timestamp: now(), - }; assert_eq!( - engine.evaluate(&ctx), + engine.evaluate(&spend(1, None)), PolicyDecision::Deny(PolicyDenyReason::ExplicitDeny) ); } #[test] fn test_max_amount() { - let mut engine = PolicyEngine::new(); - engine.set_policies(PolicyConfig { - policies: vec![Policy { - name: "limit".into(), - rules: vec![PolicyRule::MaxAmountSats(1000)], - action: PolicyAction::Deny, - }], - }); - - let ctx_ok = SigningContext { - pubkey: [7u8; 32], - amount_sats: Some(500), - fee_sats: None, - destination: None, - event_kind: None, - timestamp: now(), - }; - assert_eq!(engine.evaluate(&ctx_ok), PolicyDecision::Allow); - - let ctx_fail = SigningContext { - pubkey: [7u8; 32], - amount_sats: Some(2000), - fee_sats: None, - destination: None, - event_kind: None, - timestamp: now(), - }; - assert!(matches!( - engine.evaluate(&ctx_fail), - PolicyDecision::Deny(_) - )); + let mut engine = engine(vec![policy( + PolicyAction::Deny, + vec![PolicyRule::MaxAmountSats(1000)], + )]); + assert_eq!(engine.evaluate(&spend(500, None)), PolicyDecision::Allow); + assert!(denies(&mut engine, &spend(2000, None))); } #[test] fn test_address_allowlist() { - let mut engine = PolicyEngine::new(); - engine.set_policies(PolicyConfig { - policies: vec![Policy { - name: "allowlist".into(), - rules: vec![PolicyRule::AddressAllowlist(vec![ - "bc1allowed".into(), - ])], - action: PolicyAction::Deny, - }], - }); + let mut engine = engine(vec![policy( + PolicyAction::Deny, + vec![PolicyRule::AddressAllowlist(vec!["bc1allowed".into()])], + )]); + assert_eq!( + engine.evaluate(&spend(1, Some("bc1allowed"))), + PolicyDecision::Allow + ); + assert!(denies(&mut engine, &spend(1, Some("bc1blocked")))); + } - let ctx_ok = SigningContext { - pubkey: [7u8; 32], - amount_sats: None, - fee_sats: None, - destination: Some("bc1allowed"), - event_kind: None, - timestamp: now(), - }; - assert_eq!(engine.evaluate(&ctx_ok), PolicyDecision::Allow); + #[test] + fn an_unread_request_fails_every_rule_on_what_it_spends_or_signs() { + for rule in [ + PolicyRule::MaxAmountSats(u64::MAX), + PolicyRule::AddressAllowlist(vec!["bc1allowed".into()]), + PolicyRule::AddressBlocklist(vec!["bc1blocked".into()]), + PolicyRule::AllowedEventKinds(vec![1]), + ] { + let mut deny = engine(vec![policy(PolicyAction::Deny, vec![rule.clone()])]); + assert_eq!( + deny.evaluate(&unread()), + PolicyDecision::Deny(PolicyDenyReason::ExplicitDeny), + "{rule:?}" + ); + let mut approve = engine(vec![policy( + PolicyAction::RequireApproval, + vec![rule.clone()], + )]); + assert_eq!(approve.evaluate(&unread()), PolicyDecision::RequireApproval); + let mut skip = engine(vec![ + policy(PolicyAction::Allow, vec![rule.clone()]), + policy(PolicyAction::Deny, vec![PolicyRule::BlockWeekends]), + ]); + assert_eq!(skip.evaluate(&unread()), PolicyDecision::Allow); + let mut next = engine(vec![ + policy(PolicyAction::Allow, vec![rule]), + policy(PolicyAction::Deny, vec![PolicyRule::MaxPerMinute(0)]), + ]); + assert!(denies(&mut next, &unread())); + } + } - let ctx_fail = SigningContext { - pubkey: [7u8; 32], - amount_sats: None, - fee_sats: None, - destination: Some("bc1blocked"), - event_kind: None, - timestamp: now(), - }; - assert!(matches!( - engine.evaluate(&ctx_fail), - PolicyDecision::Deny(_) - )); + #[test] + fn an_unread_request_is_refused_by_a_fee_cap_under_any_action() { + for action in [ + PolicyAction::Allow, + PolicyAction::Deny, + PolicyAction::RequireApproval, + ] { + let mut engine = engine(vec![policy(action, vec![PolicyRule::MaxFeeSats(u64::MAX)])]); + assert_eq!( + engine.evaluate(&unread()), + PolicyDecision::Deny(PolicyDenyReason::FeeUnknown) + ); + } + } + + #[test] + fn an_unread_request_passes_a_branch_that_needs_nothing_read() { + let mut engine = engine(vec![policy( + PolicyAction::Deny, + vec![PolicyRule::Or(vec![ + PolicyRule::MaxAmountSats(u64::MAX), + PolicyRule::BlockWeekends, + ])], + )]); + // now() is a Tuesday. + assert_eq!(engine.evaluate(&unread()), PolicyDecision::Allow); + } + + #[test] + fn a_spend_paying_nothing_but_the_fee_passes_the_address_rules() { + let mut engine = engine(vec![policy( + PolicyAction::Deny, + vec![ + PolicyRule::AddressAllowlist(vec!["bc1allowed".into()]), + PolicyRule::AddressBlocklist(vec!["bc1blocked".into()]), + ], + )]); + assert_eq!(engine.evaluate(&spend(1, None)), PolicyDecision::Allow); + assert!(denies(&mut engine, &spend(1, Some("bc1blocked")))); + } + + #[test] + fn a_fee_cap_refuses_an_unread_request_wherever_it_sits() { + let unreached = [ + policy( + PolicyAction::Allow, + vec![ + PolicyRule::MaxAmountSats(1), + PolicyRule::MaxFeeSats(u64::MAX), + ], + ), + policy( + PolicyAction::Deny, + vec![PolicyRule::Or(vec![ + PolicyRule::BlockWeekends, + PolicyRule::MaxFeeSats(u64::MAX), + ])], + ), + policy( + PolicyAction::Deny, + vec![(0..MAX_POLICY_DEPTH * 2) + .fold(PolicyRule::MaxFeeSats(u64::MAX), |rule, _| { + PolicyRule::Or(vec![PolicyRule::BlockWeekends, rule]) + })], + ), + ]; + for p in unreached { + let mut engine = engine(vec![p]); + assert_eq!( + engine.evaluate(&unread()), + PolicyDecision::Deny(PolicyDenyReason::FeeUnknown) + ); + } + } + + #[test] + fn the_event_kind_rule_fails_a_spend_too() { + let mut alone = engine(vec![policy( + PolicyAction::Deny, + vec![PolicyRule::AllowedEventKinds(vec![1])], + )]); + assert!(denies(&mut alone, &spend(1, Some("bc1")))); + let mut either = engine(vec![policy( + PolicyAction::Deny, + vec![PolicyRule::Or(vec![ + PolicyRule::AllowedEventKinds(vec![1]), + PolicyRule::MaxAmountSats(100), + ])], + )]); + assert_eq!( + either.evaluate(&spend(50, Some("bc1"))), + PolicyDecision::Allow + ); + assert!(denies(&mut either, &spend(1_000, Some("bc1")))); } fn policy(action: PolicyAction, rules: Vec) -> Policy { @@ -505,6 +659,18 @@ mod tests { .bounds_fee()); } + #[test] + fn a_fee_cap_beside_the_event_kind_rule_bounds_the_fee() { + let kinds = PolicyRule::AllowedEventKinds(vec![1]); + let fee = PolicyRule::MaxFeeSats(1_000); + assert!(engine(vec![policy( + PolicyAction::Deny, + vec![PolicyRule::Or(vec![kinds.clone(), fee])], + )]) + .bounds_fee()); + assert!(!engine(vec![policy(PolicyAction::Allow, vec![kinds])]).bounds_fee()); + } + #[test] fn max_fee_keeps_the_previous_rule_indexes() { let allow = postcard::to_allocvec(&PolicyRule::AddressAllowlist(vec!["a".into()])).unwrap(); diff --git a/keep-enclave/enclave/src/vsock_server.rs b/keep-enclave/enclave/src/vsock_server.rs index caf1494b..09d5bb11 100644 --- a/keep-enclave/enclave/src/vsock_server.rs +++ b/keep-enclave/enclave/src/vsock_server.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: MIT use crate::error::{EnclaveError, Result}; use crate::kms::{EncryptedWallet, EnclaveKms}; -use crate::policy::{PolicyConfig, PolicyDecision, PolicyEngine, SigningContext}; +use crate::policy::{Contents, PolicyConfig, PolicyDecision, PolicyEngine, SigningContext}; use crate::psbt::PsbtAnalysis; use crate::signer::EnclaveSigner; use bitcoin::Network; @@ -37,13 +37,13 @@ pub enum EnclaveRequest { }, } +/// A message to sign as it is. The enclave cannot read what it spends or which +/// event it is, so it fails every policy rule on either, and a `MaxFeeSats` +/// anywhere in the policy refuses it outright. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct SigningRequest { pub key_id: String, pub message: Vec, - pub event_kind: Option, - pub amount_sats: Option, - pub destination: Option, pub nonce: Option<[u8; 32]>, pub timestamp: Option, } @@ -499,10 +499,7 @@ impl VsockServer { let ctx = SigningContext { pubkey, - amount_sats: req.amount_sats, - fee_sats: None, - destination: req.destination.as_deref(), - event_kind: req.event_kind, + contents: Contents::Unread, timestamp, }; @@ -631,10 +628,11 @@ impl VsockServer { for addr in destinations { let ctx = SigningContext { pubkey, - amount_sats: Some(total_spend), - fee_sats: Some(analysis.fee_sats), - destination: addr, - event_kind: None, + contents: Contents::Spend { + amount_sats: total_spend, + fee_sats: analysis.fee_sats, + destination: addr, + }, timestamp, }; @@ -735,8 +733,8 @@ impl VsockServer { } /// Hold a FROST request for the share's group key to the policy. A FROST - /// message is opaque to the enclave, so it is judged with no amount, - /// destination or event kind, like a `Sign` request that names none. + /// message is opaque to the enclave, so it is judged like a `Sign` request, + /// failing every rule on what it spends or which event it signs. fn frost_policy(&mut self, pubkey: [u8; 32]) -> Option { let timestamp = match SystemTime::now().duration_since(UNIX_EPOCH) { Ok(d) => d.as_secs(), @@ -749,10 +747,7 @@ impl VsockServer { }; let ctx = SigningContext { pubkey, - amount_sats: None, - fee_sats: None, - destination: None, - event_kind: None, + contents: Contents::Unread, timestamp, }; match self.policy_engine.evaluate(&ctx) { @@ -881,7 +876,7 @@ mod tests { use bitcoin::{Address, ScriptBuf}; use std::time::Duration; - const WIRE_DIGEST: &str = "e07732b1b0bb25bccb7c26b722dcd96cfd47044aa74e6a3565007ec3f42d78b5"; + const WIRE_DIGEST: &str = "d136a7183840ba3dc5d39f640534e85a74e17d786691da1516f8ef6104e0b9b5"; /// One of every request, response and policy rule, encoded as they go /// over vsock. The host and the enclave each define these types, and @@ -941,9 +936,6 @@ mod tests { EnclaveRequest::Sign(SigningRequest { key_id: "s".into(), message: vec![15], - event_kind: Some(u32::MAX), - amount_sats: Some(u64::MAX), - destination: Some("d".into()), nonce: Some([18u8; 32]), timestamp: Some(u64::MAX - 19), }), @@ -1251,26 +1243,31 @@ mod tests { .unwrap() } - fn sign_amount(server: &mut VsockServer, amount: u64) -> EnclaveResponse { - server.handle_sign(SigningRequest { - key_id: "k".into(), - message: vec![1u8; 32], - event_kind: None, - amount_sats: Some(amount), - destination: None, - nonce: None, - timestamp: None, - }) + fn sign_raw(server: &mut VsockServer) -> EnclaveResponse { + sign_as(server, "k") + } + + /// Sign a PSBT paying `sats` away and 100 in fees. + fn pay(server: &mut VsockServer, sats: u64) -> EnclaveResponse { + let psbt = psbt( + &[(p2tr(x_only(&SECRET)), sats + 100)], + &[(p2tr(other_key()), sats)], + ); + sign(server, &psbt) + } + + fn paid(response: &EnclaveResponse) -> bool { + matches!(response, EnclaveResponse::SignedPsbt { .. }) } #[test] fn signing_is_refused_until_a_policy_is_loaded() { let mut server = bare(); - assert!(denied(&sign_amount(&mut server, 1))); + assert!(denied(&sign_raw(&mut server))); assert!(denied(&server.handle_frost_round1("k", &[1u8; 32], None, None))); assert!(denied(&server.handle_frost_round2([0u8; 32], None, None))); assert!(denied(&server.handle_set_policy(PolicyConfig { policies: vec![] }))); - assert!(denied(&sign_amount(&mut server, 1))); + assert!(denied(&sign_raw(&mut server))); } #[test] @@ -1288,11 +1285,8 @@ mod tests { EnclaveResponse::PublicKey { .. } => {} other => panic!("expected the key to import, got {other:?}"), } - assert!(matches!( - sign_amount(&mut server, 500), - EnclaveResponse::Signature { .. } - )); - assert!(denied(&sign_amount(&mut server, 2_000))); + assert!(paid(&pay(&mut server, 400))); + assert!(denied(&pay(&mut server, 1_900))); assert!(denied(&server.handle_set_policy(PolicyConfig { policies: vec![Policy { name: "looser".into(), @@ -1314,13 +1308,13 @@ mod tests { }); server.signer.import_key("k", &SECRET).unwrap(); server.policy_engine.advance_clock(Duration::from_secs(60)); - assert!(signed(&sign_amount(&mut server, 1))); + assert!(signed(&sign_raw(&mut server))); let blob = crate::kms::seal_for_test(&[42u8; 32], &SECRET, &rate_bytes(1)); imported(server.handle_import_encrypted_key("k", &blob)); - assert!(denied(&sign_amount(&mut server, 1))); + assert!(denied(&sign_raw(&mut server))); server.policy_engine.advance_clock(Duration::from_secs(60)); - assert!(signed(&sign_amount(&mut server, 1))); - assert!(denied(&sign_amount(&mut server, 1))); + assert!(signed(&sign_raw(&mut server))); + assert!(denied(&sign_raw(&mut server))); } fn signed(response: &EnclaveResponse) -> bool { @@ -1334,13 +1328,10 @@ mod tests { } } - fn sign_as(server: &mut VsockServer, key_id: &str, amount: u64) -> EnclaveResponse { + fn sign_as(server: &mut VsockServer, key_id: &str) -> EnclaveResponse { server.handle_sign(SigningRequest { key_id: key_id.into(), message: vec![1u8; 32], - event_kind: None, - amount_sats: Some(amount), - destination: None, nonce: None, timestamp: None, }) @@ -1364,10 +1355,10 @@ mod tests { #[test] fn a_restarted_enclave_starts_its_rate_limits_empty() { let mut server = sealed_rules(vec![PolicyRule::MaxPerMinute(2)]); - assert!(denied(&sign_amount(&mut server, 1))); + assert!(denied(&sign_raw(&mut server))); server.policy_engine.advance_clock(Duration::from_secs(30)); - assert!(signed(&sign_amount(&mut server, 1))); - assert!(denied(&sign_amount(&mut server, 1))); + assert!(signed(&sign_raw(&mut server))); + assert!(denied(&sign_raw(&mut server))); } #[test] @@ -1377,8 +1368,8 @@ mod tests { imported(server.handle_import_encrypted_key("k", &blob)); imported(server.handle_import_encrypted_key("again", &blob)); server.policy_engine.advance_clock(Duration::from_secs(60)); - assert!(signed(&sign_as(&mut server, "k", 1))); - assert!(denied(&sign_as(&mut server, "again", 1))); + assert!(signed(&sign_as(&mut server, "k"))); + assert!(denied(&sign_as(&mut server, "again"))); } #[test] @@ -1388,21 +1379,24 @@ mod tests { PolicyRule::MaxPerMinute(2), ]); server.policy_engine.advance_clock(Duration::from_secs(60)); - assert!(signed(&sign_amount(&mut server, 1))); - assert!(signed(&sign_amount(&mut server, 1))); - assert!(denied(&sign_amount(&mut server, 1))); + assert!(signed(&sign_raw(&mut server))); + assert!(signed(&sign_raw(&mut server))); + assert!(denied(&sign_raw(&mut server))); } #[test] fn a_signature_counts_against_a_rate_rule_its_branch_skipped() { - let mut server = sealed_rules(vec![PolicyRule::Or(vec![ - PolicyRule::MaxAmountSats(10), - PolicyRule::MaxPerMinute(2), - ])]); + let mut server = server(vec![ + PolicyRule::MaxFeeSats(1_000), + PolicyRule::Or(vec![ + PolicyRule::MaxAmountSats(1_000), + PolicyRule::MaxPerMinute(2), + ]), + ]); server.policy_engine.advance_clock(Duration::from_secs(60)); - assert!(signed(&sign_amount(&mut server, 5))); - assert!(signed(&sign_amount(&mut server, 5))); - assert!(denied(&sign_amount(&mut server, 100))); + assert!(paid(&pay(&mut server, 500))); + assert!(paid(&pay(&mut server, 500))); + assert!(denied(&pay(&mut server, 5_000))); } #[test] @@ -1430,14 +1424,14 @@ mod tests { #[test] fn a_refused_request_spends_no_rate_limit() { - let mut server = sealed_rules(vec![ + let mut server = server(vec![ PolicyRule::MaxPerMinute(1), - PolicyRule::MaxAmountSats(10), + PolicyRule::MaxAmountSats(1_000), ]); server.policy_engine.advance_clock(Duration::from_secs(60)); - assert!(denied(&sign_amount(&mut server, 100))); - assert!(signed(&sign_amount(&mut server, 5))); - assert!(denied(&sign_amount(&mut server, 5))); + assert!(denied(&pay(&mut server, 5_000))); + assert!(paid(&pay(&mut server, 500))); + assert!(denied(&pay(&mut server, 500))); } #[test] @@ -1468,13 +1462,43 @@ mod tests { let mut server = sealed_rules(vec![PolicyRule::MaxPerMinute(1)]); server.policy_engine.advance_clock(Duration::from_secs(60)); assert!(matches!( - sign_as(&mut server, "missing", 1), + sign_as(&mut server, "missing"), EnclaveResponse::Error { code: ErrorCode::KeyNotFound, .. } )); - assert!(signed(&sign_amount(&mut server, 1))); + assert!(signed(&sign_raw(&mut server))); + } + + /// A rule on what a request spends or which event it signs, for each kind. + fn reading_rules() -> Vec { + vec![ + PolicyRule::MaxAmountSats(u64::MAX), + PolicyRule::MaxFeeSats(u64::MAX), + PolicyRule::AddressAllowlist(vec![address(&p2tr(other_key()))]), + PolicyRule::AddressBlocklist(vec![address(&p2tr(other_key()))]), + PolicyRule::AllowedEventKinds(vec![1]), + ] + } + + #[test] + fn a_raw_message_fails_the_rules_on_what_it_spends_or_signs() { + for rule in reading_rules() { + let mut server = sealed_rules(vec![rule.clone()]); + assert!(denied(&sign_raw(&mut server)), "{rule:?}"); + } + } + + #[test] + fn a_raw_message_is_signed_under_the_rules_it_can_be_held_to() { + let mut server = sealed_rules(vec![PolicyRule::Or(vec![ + PolicyRule::MaxAmountSats(u64::MAX), + PolicyRule::MaxPerMinute(1), + ])]); + server.policy_engine.advance_clock(Duration::from_secs(60)); + assert!(signed(&sign_raw(&mut server))); + assert!(denied(&sign_raw(&mut server))); } #[test] @@ -1485,11 +1509,8 @@ mod tests { EnclaveResponse::PublicKey { .. } => {} other => panic!("expected the key to import, got {other:?}"), } - assert!(matches!( - sign_amount(&mut restarted, 500), - EnclaveResponse::Signature { .. } - )); - assert!(denied(&sign_amount(&mut restarted, 2_000))); + assert!(paid(&pay(&mut restarted, 400))); + assert!(denied(&pay(&mut restarted, 1_900))); } #[test] @@ -1523,7 +1544,8 @@ mod tests { )); let second = crate::kms::seal_for_test(&[42u8; 32], &SECRET, &limit_bytes(1_000_000)); assert!(denied(&server.handle_import_encrypted_key("other", &second))); - assert!(denied(&sign_amount(&mut server, 2_000))); + assert!(paid(&pay(&mut server, 400))); + assert!(denied(&pay(&mut server, 1_900))); } #[test] @@ -1562,7 +1584,7 @@ mod tests { .. } )); - assert!(denied(&sign_amount(&mut server, 1))); + assert!(denied(&sign_raw(&mut server))); } type FrostKeys = ( @@ -1715,6 +1737,27 @@ mod tests { assert!(denied(&open(&mut server, b"msg").unwrap_err())); } + #[test] + fn a_frost_round_fails_the_rules_on_what_it_spends_or_signs() { + for rule in reading_rules() { + let (mut server, _) = sealed_frost(vec![rule.clone()]); + assert!(denied(&open(&mut server, b"msg").unwrap_err()), "{rule:?}"); + } + } + + #[test] + fn a_frost_share_is_refused_in_round_two_by_the_rules_on_what_it_spends() { + let (mut server, theirs) = sealed_frost(vec![PolicyRule::Or(vec![ + PolicyRule::MaxPerMinute(1), + PolicyRule::MaxAmountSats(u64::MAX), + ])]); + server.policy_engine.advance_clock(Duration::from_secs(60)); + let first = open(&mut server, b"first").unwrap(); + let second = open(&mut server, b"second").unwrap(); + assert!(shared(&finish(&mut server, first, &theirs))); + assert!(denied(&finish(&mut server, second, &theirs))); + } + #[test] fn a_frost_share_counts_against_the_rate_limit() { let (mut server, theirs) = sealed_frost(vec![PolicyRule::MaxPerMinute(1)]); diff --git a/keep-enclave/host/src/backend.rs b/keep-enclave/host/src/backend.rs index 4ea28ebc..38410b1d 100644 --- a/keep-enclave/host/src/backend.rs +++ b/keep-enclave/host/src/backend.rs @@ -169,9 +169,6 @@ mod tests { let request = SigningRequest { key_id: "test".to_string(), message: b"hello".to_vec(), - event_kind: None, - amount_sats: None, - destination: None, nonce: None, timestamp: None, }; diff --git a/keep-enclave/host/src/mock.rs b/keep-enclave/host/src/mock.rs index be7b67cc..da15d8a2 100644 --- a/keep-enclave/host/src/mock.rs +++ b/keep-enclave/host/src/mock.rs @@ -575,9 +575,6 @@ mod tests { let sign_req = SigningRequest { key_id: "testkey".to_string(), message: message.to_vec(), - event_kind: None, - amount_sats: None, - destination: None, nonce: None, timestamp: None, }; diff --git a/keep-enclave/host/src/protocol.rs b/keep-enclave/host/src/protocol.rs index 5fd615f3..5b71c42a 100644 --- a/keep-enclave/host/src/protocol.rs +++ b/keep-enclave/host/src/protocol.rs @@ -54,13 +54,14 @@ pub enum EnclaveRequest { }, } +/// A message to sign as it is. The enclave cannot read what it spends or which +/// event it is, so it fails every policy rule on either, and a `MaxFeeSats` +/// anywhere in the policy refuses it outright; sign a spend with `SignPsbt` +/// instead. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct SigningRequest { pub key_id: String, pub message: Vec, - pub event_kind: Option, - pub amount_sats: Option, - pub destination: Option, pub nonce: Option<[u8; 32]>, pub timestamp: Option, } @@ -136,13 +137,21 @@ pub struct Policy { #[derive(Debug, Clone, Serialize, Deserialize)] pub enum PolicyRule { + /// What a PSBT takes out of the wallet, fee included, at most this. A raw + /// `Sign` message or a FROST share fails it, since the enclave cannot read + /// what they spend, and the address rules fail them the same way. A + /// `MaxFeeSats` anywhere in the policy refuses them outright, and + /// `AllowedEventKinds` fails every request. MaxAmountSats(u64), MaxPerMinute(u32), MaxPerHour(u32), MaxPerDay(u32), AddressAllowlist(Vec), AddressBlocklist(Vec), - AllowedHours { start: u8, end: u8 }, + AllowedHours { + start: u8, + end: u8, + }, BlockWeekends, AllowedEventKinds(Vec), And(Vec), @@ -172,7 +181,7 @@ mod tests { use super::*; use sha2::Digest; - const WIRE_DIGEST: &str = "e07732b1b0bb25bccb7c26b722dcd96cfd47044aa74e6a3565007ec3f42d78b5"; + const WIRE_DIGEST: &str = "d136a7183840ba3dc5d39f640534e85a74e17d786691da1516f8ef6104e0b9b5"; /// One of every request, response and policy rule, encoded as they go /// over vsock. The host and the enclave each define these types, and @@ -232,9 +241,6 @@ mod tests { EnclaveRequest::Sign(SigningRequest { key_id: "s".into(), message: vec![15], - event_kind: Some(u32::MAX), - amount_sats: Some(u64::MAX), - destination: Some("d".into()), nonce: Some([18u8; 32]), timestamp: Some(u64::MAX - 19), }), diff --git a/keep-enclave/host/tests/integration_tests.rs b/keep-enclave/host/tests/integration_tests.rs index 0d38f7e5..6b2b696e 100644 --- a/keep-enclave/host/tests/integration_tests.rs +++ b/keep-enclave/host/tests/integration_tests.rs @@ -43,9 +43,6 @@ fn test_backend_sign() { let request = SigningRequest { key_id: "signer".to_string(), message: b"test message".to_vec(), - event_kind: Some(1), - amount_sats: None, - destination: None, nonce: None, timestamp: None, }; @@ -90,9 +87,6 @@ fn test_backend_sign_nonexistent_key() { let request = SigningRequest { key_id: "nonexistent".to_string(), message: b"test".to_vec(), - event_kind: None, - amount_sats: None, - destination: None, nonce: None, timestamp: None, };