fix(server): added chain_id check and covered check_shared_constraints with unit tests
Some checks failed
ci/woodpecker/pr/server-audit Pipeline was successful
ci/woodpecker/pr/server-lint Pipeline failed
ci/woodpecker/pr/server-vet Pipeline failed
ci/woodpecker/pr/server-test Pipeline was successful
ci/woodpecker/push/server-audit Pipeline was successful
ci/woodpecker/push/server-lint Pipeline failed
ci/woodpecker/push/server-vet Pipeline failed
ci/woodpecker/push/server-test Pipeline was successful

This commit was merged in pull request #76.
This commit is contained in:
hdbg
2026-04-06 12:05:55 +02:00
parent 2dd5a3f32f
commit f6b62ab884
14 changed files with 335 additions and 49 deletions

View File

@@ -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;
}
}

1
server/Cargo.lock generated
View File

@@ -752,6 +752,7 @@ dependencies = [
"rcgen",
"restructed",
"rsa",
"rstest",
"rustls",
"secrecy",
"serde",

View File

@@ -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"] }

View File

@@ -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 _},
};

View File

@@ -103,7 +103,6 @@ async fn verify_integrity(
})?;
Ok(())
}
async fn create_nonce(

View File

@@ -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<S>(key: &k256::ecdsa::VerifyingKey, serializer: S) -> Result<S::Ok, S::Error>
@@ -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 {

View File

@@ -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<E: Integrable>(
.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<E: Integrable>(
.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<E: Integrable>(
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<E: Integrable>(
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(),

View File

@@ -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<Vec<u8>>, 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);

View File

@@ -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<chrono::DateTime<Utc>>,
#[case] valid_until: Option<chrono::DateTime<Utc>>,
#[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<U256>,
#[case] max_priority_fee_per_gas: Option<U256>,
#[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());
}
}
}

View File

@@ -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<P: Integrable> Integrable for CombinedSettings<P> {
const KIND: &'static str = P::KIND;
const VERSION: i32 = P::VERSION;
}

View File

@@ -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());
}
},