diff --git a/protobufs/shared/evm.proto b/protobufs/shared/evm.proto index afac94d..5ddd6f2 100644 --- a/protobufs/shared/evm.proto +++ b/protobufs/shared/evm.proto @@ -36,6 +36,10 @@ message GasLimitExceededViolation { } message EvalViolation { + message ChainIdMismatch { + uint64 expected = 1; + uint64 actual = 2; + } oneof kind { bytes invalid_target = 1; // 20-byte Ethereum address GasLimitExceededViolation gas_limit_exceeded = 2; @@ -43,6 +47,8 @@ message EvalViolation { google.protobuf.Empty volumetric_limit_exceeded = 4; google.protobuf.Empty invalid_time = 5; google.protobuf.Empty invalid_transaction_type = 6; + + ChainIdMismatch chain_id_mismatch = 7; } } diff --git a/server/Cargo.lock b/server/Cargo.lock index efdb0a8..e94a9b8 100644 --- a/server/Cargo.lock +++ b/server/Cargo.lock @@ -752,6 +752,7 @@ dependencies = [ "rcgen", "restructed", "rsa", + "rstest", "rustls", "secrecy", "serde", diff --git a/server/crates/arbiter-server/Cargo.toml b/server/crates/arbiter-server/Cargo.toml index e9da3fa..dff79e9 100644 --- a/server/crates/arbiter-server/Cargo.toml +++ b/server/crates/arbiter-server/Cargo.toml @@ -65,4 +65,5 @@ mutants.workspace = true [dev-dependencies] insta = "1.46.3" +rstest.workspace = true test-log = { version = "0.2", default-features = false, features = ["trace"] } diff --git a/server/crates/arbiter-server/src/actors/evm/mod.rs b/server/crates/arbiter-server/src/actors/evm/mod.rs index a615e19..84326eb 100644 --- a/server/crates/arbiter-server/src/actors/evm/mod.rs +++ b/server/crates/arbiter-server/src/actors/evm/mod.rs @@ -15,10 +15,11 @@ use crate::{ schema, }, evm::{ - self, ListError, RunKind, policies::{ + self, ListError, RunKind, + policies::{ CombinedSettings, Grant, SharedGrantSettings, SpecificGrant, SpecificMeaning, ether_transfer::EtherTransfer, token_transfers::TokenTransfer, - } + }, }, safe_cell::{SafeCell, SafeCellHandle as _}, }; diff --git a/server/crates/arbiter-server/src/actors/user_agent/auth/state.rs b/server/crates/arbiter-server/src/actors/user_agent/auth/state.rs index 5ce6374..ccc4b31 100644 --- a/server/crates/arbiter-server/src/actors/user_agent/auth/state.rs +++ b/server/crates/arbiter-server/src/actors/user_agent/auth/state.rs @@ -103,7 +103,6 @@ async fn verify_integrity( })?; Ok(()) - } async fn create_nonce( diff --git a/server/crates/arbiter-server/src/actors/user_agent/mod.rs b/server/crates/arbiter-server/src/actors/user_agent/mod.rs index efe3adf..dd65fc5 100644 --- a/server/crates/arbiter-server/src/actors/user_agent/mod.rs +++ b/server/crates/arbiter-server/src/actors/user_agent/mod.rs @@ -1,5 +1,7 @@ use crate::{ - actors::{GlobalActors, client::ClientProfile}, crypto::integrity::Integrable, db::{self, models::KeyType} + actors::{GlobalActors, client::ClientProfile}, + crypto::integrity::Integrable, + db::{self, models::KeyType}, }; fn serialize_ecdsa(key: &k256::ecdsa::VerifyingKey, serializer: S) -> Result @@ -44,7 +46,10 @@ where pub enum AuthPublicKey { Ed25519(ed25519_dalek::VerifyingKey), /// Compressed SEC1 public key; signature bytes are raw 64-byte (r||s). - #[serde(serialize_with = "serialize_ecdsa", deserialize_with = "deserialize_ecdsa")] + #[serde( + serialize_with = "serialize_ecdsa", + deserialize_with = "deserialize_ecdsa" + )] EcdsaSecp256k1(k256::ecdsa::VerifyingKey), /// RSA-2048+ public key (Windows Hello / KeyCredentialManager); signature bytes are PSS+SHA-256. Rsa(rsa::RsaPublicKey), @@ -53,7 +58,7 @@ pub enum AuthPublicKey { #[derive(Debug, Serialize)] pub struct UserAgentCredentials { pub pubkey: AuthPublicKey, - pub nonce: i32 + pub nonce: i32, } impl Integrable for UserAgentCredentials { diff --git a/server/crates/arbiter-server/src/crypto/encryption/mod.rs b/server/crates/arbiter-server/src/crypto/encryption/mod.rs index 22fb807..44c6ed4 100644 --- a/server/crates/arbiter-server/src/crypto/encryption/mod.rs +++ b/server/crates/arbiter-server/src/crypto/encryption/mod.rs @@ -1,3 +1,3 @@ pub mod v1; -pub use v1::*; \ No newline at end of file +pub use v1::*; diff --git a/server/crates/arbiter-server/src/crypto/integrity/mod.rs b/server/crates/arbiter-server/src/crypto/integrity/mod.rs index 22fb807..44c6ed4 100644 --- a/server/crates/arbiter-server/src/crypto/integrity/mod.rs +++ b/server/crates/arbiter-server/src/crypto/integrity/mod.rs @@ -1,3 +1,3 @@ pub mod v1; -pub use v1::*; \ No newline at end of file +pub use v1::*; diff --git a/server/crates/arbiter-server/src/crypto/integrity/v1.rs b/server/crates/arbiter-server/src/crypto/integrity/v1.rs index 3fa7d17..fb0fd7e 100644 --- a/server/crates/arbiter-server/src/crypto/integrity/v1.rs +++ b/server/crates/arbiter-server/src/crypto/integrity/v1.rs @@ -1,4 +1,4 @@ -use crate::{actors::keyholder, crypto::KeyCell,safe_cell::SafeCellHandle as _}; +use crate::{actors::keyholder, crypto::KeyCell, safe_cell::SafeCellHandle as _}; use chacha20poly1305::Key; use hmac::{Hmac, Mac as _}; use serde::Serialize; @@ -128,7 +128,7 @@ pub async fn sign_entity( .values(NewIntegrityEnvelope { entity_kind: E::KIND.to_owned(), entity_id: entity_id, - payload_version: E::VERSION , + payload_version: E::VERSION, key_version, mac: mac.to_vec(), }) @@ -162,7 +162,9 @@ pub async fn verify_entity( .first(conn) .await .map_err(|err| match err { - diesel::result::Error::NotFound => Error::MissingEnvelope { entity_kind: E::KIND }, + diesel::result::Error::NotFound => Error::MissingEnvelope { + entity_kind: E::KIND, + }, other => Error::Database(db::DatabaseError::from(other)), })?; @@ -176,12 +178,7 @@ pub async fn verify_entity( let payload = postcard::to_stdvec(entity)?; let payload_hash = payload_hash(&payload); - let mac_input = build_mac_input( - E::KIND, - &entity_id, - envelope.payload_version, - &payload_hash, - ); + let mac_input = build_mac_input(E::KIND, &entity_id, envelope.payload_version, &payload_hash); let result = keyholder .ask(VerifyIntegrity { @@ -189,13 +186,16 @@ pub async fn verify_entity( expected_mac: envelope.mac, key_version: envelope.key_version, }) - .await - ; + .await; match result { Ok(true) => Ok(AttestationStatus::Attested), - Ok(false) => Err(Error::MacMismatch { entity_kind: E::KIND }), - Err(SendError::HandlerError(keyholder::Error::NotBootstrapped)) => Ok(AttestationStatus::Unavailable), + Ok(false) => Err(Error::MacMismatch { + entity_kind: E::KIND, + }), + Err(SendError::HandlerError(keyholder::Error::NotBootstrapped)) => { + Ok(AttestationStatus::Unavailable) + } Err(_) => Err(Error::KeyholderSend), } } @@ -248,7 +248,9 @@ mod tests { payload: b"payload-v1".to_vec(), }; - sign_entity(&mut conn, &keyholder, &entity, ENTITY_ID).await.unwrap(); + sign_entity(&mut conn, &keyholder, &entity, ENTITY_ID) + .await + .unwrap(); let count: i64 = schema::integrity_envelope::table .filter(schema::integrity_envelope::entity_kind.eq("dummy_entity")) @@ -259,7 +261,9 @@ mod tests { .unwrap(); assert_eq!(count, 1, "envelope row must be created exactly once"); - verify_entity(&mut conn, &keyholder, &entity, ENTITY_ID).await.unwrap(); + verify_entity(&mut conn, &keyholder, &entity, ENTITY_ID) + .await + .unwrap(); } #[tokio::test] @@ -275,7 +279,9 @@ mod tests { payload: b"payload-v1".to_vec(), }; - sign_entity(&mut conn, &keyholder, &entity, ENTITY_ID).await.unwrap(); + sign_entity(&mut conn, &keyholder, &entity, ENTITY_ID) + .await + .unwrap(); diesel::update(schema::integrity_envelope::table) .filter(schema::integrity_envelope::entity_kind.eq("dummy_entity")) @@ -304,7 +310,9 @@ mod tests { payload: b"payload-v1".to_vec(), }; - sign_entity(&mut conn, &keyholder, &entity, ENTITY_ID).await.unwrap(); + sign_entity(&mut conn, &keyholder, &entity, ENTITY_ID) + .await + .unwrap(); let tampered = DummyEntity { payload: b"payload-v1-but-tampered".to_vec(), diff --git a/server/crates/arbiter-server/src/crypto/mod.rs b/server/crates/arbiter-server/src/crypto/mod.rs index 21889bc..d26c41f 100644 --- a/server/crates/arbiter-server/src/crypto/mod.rs +++ b/server/crates/arbiter-server/src/crypto/mod.rs @@ -105,16 +105,16 @@ impl KeyCell { /// Derive a fixed-length key from the password using Argon2id, which is designed for password hashing and key derivation. pub fn derive_key(mut password: SafeCell>, salt: &Salt) -> KeyCell { let params = { - #[cfg(debug_assertions)] - { - argon2::Params::new(8, 1, 1, None).unwrap() - } + #[cfg(debug_assertions)] + { + argon2::Params::new(8, 1, 1, None).unwrap() + } - #[cfg(not(debug_assertions))] - { - argon2::Params::new(262_144, 3, 4, None).unwrap() - } -}; + #[cfg(not(debug_assertions))] + { + argon2::Params::new(262_144, 3, 4, None).unwrap() + } + }; #[allow(clippy::unwrap_used)] let hasher = Argon2::new(Algorithm::Argon2id, argon2::Version::V0x13, params); diff --git a/server/crates/arbiter-server/src/evm/mod.rs b/server/crates/arbiter-server/src/evm/mod.rs index 6727650..15ac999 100644 --- a/server/crates/arbiter-server/src/evm/mod.rs +++ b/server/crates/arbiter-server/src/evm/mod.rs @@ -21,8 +21,8 @@ use crate::{ schema::{self, evm_transaction_log}, }, evm::policies::{ - DatabaseID, EvalContext, EvalViolation, Grant, Policy, CombinedSettings, SharedGrantSettings, - SpecificGrant, SpecificMeaning, ether_transfer::EtherTransfer, + CombinedSettings, DatabaseID, EvalContext, EvalViolation, Grant, Policy, + SharedGrantSettings, SpecificGrant, SpecificMeaning, ether_transfer::EtherTransfer, token_transfers::TokenTransfer, }, }; @@ -90,6 +90,14 @@ async fn check_shared_constraints( let mut violations = Vec::new(); let now = Utc::now(); + if shared.chain != context.chain { + violations.push(EvalViolation::MismatchingChainId { + expected: shared.chain, + actual: context.chain, + }); + return Ok(violations); + } + // Validity window if shared.valid_from.is_some_and(|t| now < t) || shared.valid_until.is_some_and(|t| now > t) { violations.push(EvalViolation::InvalidTime); @@ -250,14 +258,9 @@ impl Engine { P::create_grant(&basic_grant, &full_grant.specific, conn).await?; - integrity::sign_entity( - conn, - &keyholder, - &full_grant, - basic_grant.id, - ) - .await - .map_err(|_| diesel::result::Error::RollbackTransaction)?; + integrity::sign_entity(conn, &keyholder, &full_grant, basic_grant.id) + .await + .map_err(|_| diesel::result::Error::RollbackTransaction)?; QueryResult::Ok(basic_grant.id) }) @@ -342,3 +345,255 @@ impl Engine { Err(VetError::UnsupportedTransactionType) } } + +#[cfg(test)] +mod tests { + use alloy::primitives::{Address, Bytes, U256, address}; + use chrono::{Duration, Utc}; + use diesel::{SelectableHelper, insert_into}; + use diesel_async::RunQueryDsl; + use rstest::rstest; + + use crate::db::{ + self, DatabaseConnection, + models::{ + EvmBasicGrant, EvmWalletAccess, NewEvmBasicGrant, NewEvmTransactionLog, SqliteTimestamp, + }, + schema::{evm_basic_grant, evm_transaction_log}, + }; + use crate::evm::policies::{ + EvalContext, EvalViolation, SharedGrantSettings, TransactionRateLimit, + }; + + use super::check_shared_constraints; + + const WALLET_ACCESS_ID: i32 = 1; + const CHAIN_ID: u64 = 1; + const RECIPIENT: Address = address!("1111111111111111111111111111111111111111"); + + fn context() -> EvalContext { + EvalContext { + target: EvmWalletAccess { + id: WALLET_ACCESS_ID, + wallet_id: 10, + client_id: 20, + created_at: SqliteTimestamp(Utc::now()), + }, + chain: CHAIN_ID, + to: RECIPIENT, + value: U256::ZERO, + calldata: Bytes::new(), + max_fee_per_gas: 100, + max_priority_fee_per_gas: 10, + } + } + + fn shared_settings() -> SharedGrantSettings { + SharedGrantSettings { + wallet_access_id: WALLET_ACCESS_ID, + chain: CHAIN_ID, + valid_from: None, + valid_until: None, + max_gas_fee_per_gas: None, + max_priority_fee_per_gas: None, + rate_limit: None, + } + } + + async fn insert_basic_grant( + conn: &mut DatabaseConnection, + shared: &SharedGrantSettings, + ) -> EvmBasicGrant { + insert_into(evm_basic_grant::table) + .values(NewEvmBasicGrant { + wallet_access_id: shared.wallet_access_id, + chain_id: shared.chain as i32, + valid_from: shared.valid_from.map(SqliteTimestamp), + valid_until: shared.valid_until.map(SqliteTimestamp), + max_gas_fee_per_gas: shared + .max_gas_fee_per_gas + .map(|fee| super::utils::u256_to_bytes(fee).to_vec()), + max_priority_fee_per_gas: shared + .max_priority_fee_per_gas + .map(|fee| super::utils::u256_to_bytes(fee).to_vec()), + rate_limit_count: shared.rate_limit.as_ref().map(|limit| limit.count as i32), + rate_limit_window_secs: shared + .rate_limit + .as_ref() + .map(|limit| limit.window.num_seconds() as i32), + revoked_at: None, + }) + .returning(EvmBasicGrant::as_select()) + .get_result(conn) + .await + .unwrap() + } + + #[rstest] + #[case::matching_chain(CHAIN_ID, false)] + #[case::mismatching_chain(CHAIN_ID + 1, true)] + #[tokio::test] + async fn check_shared_constraints_enforces_chain_id( + #[case] context_chain: u64, + #[case] expect_mismatch: bool, + ) { + let db = db::create_test_pool().await; + let mut conn = db.get().await.unwrap(); + + let context = EvalContext { + chain: context_chain, + ..context() + }; + + let violations = check_shared_constraints(&context, &shared_settings(), 999, &mut *conn) + .await + .unwrap(); + + assert_eq!( + violations + .iter() + .any(|violation| matches!(violation, EvalViolation::MismatchingChainId { .. })), + expect_mismatch + ); + + if expect_mismatch { + assert_eq!(violations.len(), 1); + } else { + assert!(violations.is_empty()); + } + } + + #[rstest] + #[case::valid_from_in_bounds(Some(Utc::now() - Duration::hours(1)), None, false)] + #[case::valid_from_out_of_bounds(Some(Utc::now() + Duration::hours(1)), None, true)] + #[case::valid_until_in_bounds(None, Some(Utc::now() + Duration::hours(1)), false)] + #[case::valid_until_out_of_bounds(None, Some(Utc::now() - Duration::hours(1)), true)] + #[tokio::test] + async fn check_shared_constraints_enforces_validity_window( + #[case] valid_from: Option>, + #[case] valid_until: Option>, + #[case] expect_invalid_time: bool, + ) { + let db = db::create_test_pool().await; + let mut conn = db.get().await.unwrap(); + + let shared = SharedGrantSettings { + valid_from, + valid_until, + ..shared_settings() + }; + + let violations = check_shared_constraints(&context(), &shared, 999, &mut *conn) + .await + .unwrap(); + + assert_eq!( + violations + .iter() + .any(|violation| matches!(violation, EvalViolation::InvalidTime)), + expect_invalid_time + ); + + if expect_invalid_time { + assert_eq!(violations.len(), 1); + } else { + assert!(violations.is_empty()); + } + } + + #[rstest] + #[case::max_fee_within_limit(Some(U256::from(100u64)), None, 100, 10, false)] + #[case::max_fee_exceeded(Some(U256::from(99u64)), None, 100, 10, true)] + #[case::priority_fee_within_limit(None, Some(U256::from(10u64)), 100, 10, false)] + #[case::priority_fee_exceeded(None, Some(U256::from(9u64)), 100, 10, true)] + #[tokio::test] + async fn check_shared_constraints_enforces_gas_fee_caps( + #[case] max_gas_fee_per_gas: Option, + #[case] max_priority_fee_per_gas: Option, + #[case] actual_max_fee_per_gas: u128, + #[case] actual_max_priority_fee_per_gas: u128, + #[case] expect_gas_limit_violation: bool, + ) { + let db = db::create_test_pool().await; + let mut conn = db.get().await.unwrap(); + + let context = EvalContext { + max_fee_per_gas: actual_max_fee_per_gas, + max_priority_fee_per_gas: actual_max_priority_fee_per_gas, + ..context() + }; + + let shared = SharedGrantSettings { + max_gas_fee_per_gas, + max_priority_fee_per_gas, + ..shared_settings() + }; + let violations = check_shared_constraints(&context, &shared, 999, &mut *conn) + .await + .unwrap(); + + assert_eq!( + violations + .iter() + .any(|violation| matches!(violation, EvalViolation::GasLimitExceeded { .. })), + expect_gas_limit_violation + ); + + if expect_gas_limit_violation { + assert_eq!(violations.len(), 1); + } else { + assert!(violations.is_empty()); + } + } + + #[rstest] + #[case::under_rate_limit(2, false)] + #[case::at_rate_limit(1, true)] + #[tokio::test] + async fn check_shared_constraints_enforces_rate_limit( + #[case] rate_limit_count: u32, + #[case] expect_rate_limit_violation: bool, + ) { + let db = db::create_test_pool().await; + let mut conn = db.get().await.unwrap(); + + let shared = SharedGrantSettings { + rate_limit: Some(TransactionRateLimit { + count: rate_limit_count, + window: Duration::hours(1), + }), + ..shared_settings() + }; + + let basic_grant = insert_basic_grant(&mut conn, &shared).await; + + insert_into(evm_transaction_log::table) + .values(NewEvmTransactionLog { + grant_id: basic_grant.id, + wallet_access_id: WALLET_ACCESS_ID, + chain_id: CHAIN_ID as i32, + eth_value: super::utils::u256_to_bytes(U256::ZERO).to_vec(), + signed_at: SqliteTimestamp(Utc::now()), + }) + .execute(&mut *conn) + .await + .unwrap(); + + let violations = check_shared_constraints(&context(), &shared, basic_grant.id, &mut *conn) + .await + .unwrap(); + + assert_eq!( + violations + .iter() + .any(|violation| matches!(violation, EvalViolation::RateLimitExceeded)), + expect_rate_limit_violation + ); + + if expect_rate_limit_violation { + assert_eq!(violations.len(), 1); + } else { + assert!(violations.is_empty()); + } + } +} diff --git a/server/crates/arbiter-server/src/evm/policies.rs b/server/crates/arbiter-server/src/evm/policies.rs index d69af22..7d73d75 100644 --- a/server/crates/arbiter-server/src/evm/policies.rs +++ b/server/crates/arbiter-server/src/evm/policies.rs @@ -11,7 +11,9 @@ use serde::Serialize; use thiserror::Error; use crate::{ - crypto::integrity::v1::Integrable, db::models::{self, EvmBasicGrant, EvmWalletAccess}, evm::utils + crypto::integrity::v1::Integrable, + db::models::{self, EvmBasicGrant, EvmWalletAccess}, + evm::utils, }; pub mod ether_transfer; @@ -55,6 +57,9 @@ pub enum EvalViolation { #[error("Transaction type is not allowed by this grant")] InvalidTransactionType, + + #[error("Mismatching chain ID")] + MismatchingChainId { expected: ChainId, actual: ChainId }, } pub type DatabaseID = i32; @@ -215,4 +220,3 @@ impl Integrable for CombinedSettings

{ const KIND: &'static str = P::KIND; const VERSION: i32 = P::VERSION; } - diff --git a/server/crates/arbiter-server/src/grpc/common/outbound.rs b/server/crates/arbiter-server/src/grpc/common/outbound.rs index 1b500c9..c2a2045 100644 --- a/server/crates/arbiter-server/src/grpc/common/outbound.rs +++ b/server/crates/arbiter-server/src/grpc/common/outbound.rs @@ -8,7 +8,7 @@ use arbiter_proto::proto::{ EvalViolation as ProtoEvalViolation, GasLimitExceededViolation, NoMatchingGrantError, PolicyViolationsError, SpecificMeaning as ProtoSpecificMeaning, TokenInfo as ProtoTokenInfo, TransactionEvalError as ProtoTransactionEvalError, - eval_violation::Kind as ProtoEvalViolationKind, + eval_violation as proto_eval_violation, eval_violation::Kind as ProtoEvalViolationKind, specific_meaning::Meaning as ProtoSpecificMeaningKind, transaction_eval_error::Kind as ProtoTransactionEvalErrorKind, }, @@ -79,6 +79,12 @@ impl Convert for EvalViolation { EvalViolation::InvalidTransactionType => { ProtoEvalViolationKind::InvalidTransactionType(()) } + EvalViolation::MismatchingChainId { expected, actual } => { + ProtoEvalViolationKind::ChainIdMismatch(proto_eval_violation::ChainIdMismatch { + expected, + actual, + }) + } }; ProtoEvalViolation { kind: Some(kind) } @@ -108,7 +114,7 @@ impl Convert for VetError { violations: violations.into_iter().map(Convert::convert).collect(), }) } - PolicyError::Database(_)| PolicyError::Integrity(_) => { + PolicyError::Database(_) | PolicyError::Integrity(_) => { return EvmSignTransactionResult::Error(ProtoEvmError::Internal.into()); } }, diff --git a/server/crates/arbiter-server/tests/user_agent/unseal.rs b/server/crates/arbiter-server/tests/user_agent/unseal.rs index 57cd1ee..232b2e9 100644 --- a/server/crates/arbiter-server/tests/user_agent/unseal.rs +++ b/server/crates/arbiter-server/tests/user_agent/unseal.rs @@ -151,4 +151,4 @@ pub async fn test_unseal_retry_after_invalid_key() { let response = user_agent.ask(encrypted_key).await; assert!(matches!(response, Ok(()))); } -} \ No newline at end of file +}