Merge pull request 'fix-security' (#42) from fix-security into main
Reviewed-on: #42 Reviewed-by: Stas <business@jexter.tech>
This commit is contained in:
@@ -214,7 +214,6 @@ impl KeyHolder {
|
||||
let mut conn = self.db.get().await?;
|
||||
schema::root_key_history::table
|
||||
.filter(schema::root_key_history::id.eq(*root_key_history_id))
|
||||
.select(schema::root_key_history::data_encryption_nonce)
|
||||
.select(RootKeyHistory::as_select())
|
||||
.first(&mut conn)
|
||||
.await?
|
||||
|
||||
@@ -210,12 +210,15 @@ where
|
||||
}
|
||||
};
|
||||
|
||||
if valid {
|
||||
if !valid {
|
||||
error!("Invalid challenge solution signature");
|
||||
return Err(Error::InvalidChallengeSolution);
|
||||
}
|
||||
|
||||
self.transport
|
||||
.send(Ok(Outbound::AuthSuccess))
|
||||
.await
|
||||
.map_err(|_| Error::Transport)?;
|
||||
}
|
||||
|
||||
Ok(key.clone())
|
||||
}
|
||||
|
||||
@@ -91,6 +91,7 @@ async fn query_relevant_past_transaction(
|
||||
|
||||
async fn check_rate_limits(
|
||||
grant: &Grant<Settings>,
|
||||
current_transfer_value: U256,
|
||||
db: &mut impl AsyncConnection<Backend = Sqlite>,
|
||||
) -> QueryResult<Vec<EvalViolation>> {
|
||||
let mut violations = Vec::new();
|
||||
@@ -99,12 +100,12 @@ async fn check_rate_limits(
|
||||
let past_transaction = query_relevant_past_transaction(grant.id, window, db).await?;
|
||||
|
||||
let window_start = chrono::Utc::now() - grant.settings.limit.window;
|
||||
let cumulative_volume: U256 = past_transaction
|
||||
let prospective_cumulative_volume: U256 = past_transaction
|
||||
.iter()
|
||||
.filter(|(_, timestamp)| timestamp >= &window_start)
|
||||
.fold(U256::default(), |acc, (value, _)| acc + *value);
|
||||
.fold(current_transfer_value, |acc, (value, _)| acc + *value);
|
||||
|
||||
if cumulative_volume > grant.settings.limit.max_volume {
|
||||
if prospective_cumulative_volume > grant.settings.limit.max_volume {
|
||||
violations.push(EvalViolation::VolumetricLimitExceeded);
|
||||
}
|
||||
|
||||
@@ -141,7 +142,7 @@ impl Policy for EtherTransfer {
|
||||
violations.push(EvalViolation::InvalidTarget { target: meaning.to });
|
||||
}
|
||||
|
||||
let rate_violations = check_rate_limits(grant, db).await?;
|
||||
let rate_violations = check_rate_limits(grant, meaning.value, db).await?;
|
||||
violations.extend(rate_violations);
|
||||
|
||||
Ok(violations)
|
||||
|
||||
@@ -198,7 +198,7 @@ async fn evaluate_rejects_volume_over_limit() {
|
||||
grant_id,
|
||||
wallet_access_id: WALLET_ACCESS_ID,
|
||||
chain_id: CHAIN_ID as i32,
|
||||
eth_value: utils::u256_to_bytes(U256::from(1_001u64)).to_vec(),
|
||||
eth_value: utils::u256_to_bytes(U256::from(1_000u64)).to_vec(),
|
||||
signed_at: SqliteTimestamp(Utc::now()),
|
||||
})
|
||||
.execute(&mut *conn)
|
||||
@@ -211,7 +211,7 @@ async fn evaluate_rejects_volume_over_limit() {
|
||||
shared: shared(),
|
||||
settings,
|
||||
};
|
||||
let context = ctx(ALLOWED, U256::from(100u64));
|
||||
let context = ctx(ALLOWED, U256::from(1u64));
|
||||
let m = EtherTransfer::analyze(&context).unwrap();
|
||||
let v = EtherTransfer::evaluate(&context, &m, &grant, &mut *conn)
|
||||
.await
|
||||
@@ -233,13 +233,13 @@ async fn evaluate_passes_at_exactly_volume_limit() {
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// Exactly at the limit — the check is `>`, so this should not violate
|
||||
// Exactly at the limit including current transfer — check is `>`, so this should not violate
|
||||
insert_into(evm_transaction_log::table)
|
||||
.values(NewEvmTransactionLog {
|
||||
grant_id,
|
||||
wallet_access_id: WALLET_ACCESS_ID,
|
||||
chain_id: CHAIN_ID as i32,
|
||||
eth_value: utils::u256_to_bytes(U256::from(1_000u64)).to_vec(),
|
||||
eth_value: utils::u256_to_bytes(U256::from(900u64)).to_vec(),
|
||||
signed_at: SqliteTimestamp(Utc::now()),
|
||||
})
|
||||
.execute(&mut *conn)
|
||||
|
||||
@@ -101,6 +101,7 @@ async fn query_relevant_past_transfers(
|
||||
|
||||
async fn check_volume_rate_limits(
|
||||
grant: &Grant<Settings>,
|
||||
current_transfer_value: U256,
|
||||
db: &mut impl AsyncConnection<Backend = Sqlite>,
|
||||
) -> QueryResult<Vec<EvalViolation>> {
|
||||
let mut violations = Vec::new();
|
||||
@@ -113,12 +114,12 @@ async fn check_volume_rate_limits(
|
||||
|
||||
for limit in &grant.settings.volume_limits {
|
||||
let window_start = chrono::Utc::now() - limit.window;
|
||||
let cumulative_volume: U256 = past_transfers
|
||||
let prospective_cumulative_volume: U256 = past_transfers
|
||||
.iter()
|
||||
.filter(|(_, timestamp)| timestamp >= &window_start)
|
||||
.fold(U256::default(), |acc, (value, _)| acc + *value);
|
||||
.fold(current_transfer_value, |acc, (value, _)| acc + *value);
|
||||
|
||||
if cumulative_volume > limit.max_volume {
|
||||
if prospective_cumulative_volume > limit.max_volume {
|
||||
violations.push(EvalViolation::VolumetricLimitExceeded);
|
||||
break;
|
||||
}
|
||||
@@ -163,7 +164,7 @@ impl Policy for TokenTransfer {
|
||||
violations.push(EvalViolation::InvalidTarget { target: meaning.to });
|
||||
}
|
||||
|
||||
let rate_violations = check_volume_rate_limits(grant, db).await?;
|
||||
let rate_violations = check_volume_rate_limits(grant, meaning.value, db).await?;
|
||||
violations.extend(rate_violations);
|
||||
|
||||
Ok(violations)
|
||||
|
||||
@@ -220,7 +220,7 @@ async fn evaluate_rejects_wrong_restricted_recipient() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn evaluate_passes_volume_within_limit() {
|
||||
async fn evaluate_passes_volume_at_exact_limit() {
|
||||
let db = db::create_test_pool().await;
|
||||
let mut conn = db.get().await.unwrap();
|
||||
|
||||
@@ -230,7 +230,7 @@ async fn evaluate_passes_volume_within_limit() {
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// Record a past transfer of 500 (within 1000 limit)
|
||||
// Record a past transfer of 900, with current transfer 100 => exactly 1000 limit
|
||||
use crate::db::{models::NewEvmTokenTransferLog, schema::evm_token_transfer_log};
|
||||
insert_into(evm_token_transfer_log::table)
|
||||
.values(NewEvmTokenTransferLog {
|
||||
@@ -239,7 +239,7 @@ async fn evaluate_passes_volume_within_limit() {
|
||||
chain_id: CHAIN_ID as i32,
|
||||
token_contract: DAI.to_vec(),
|
||||
recipient_address: RECIPIENT.to_vec(),
|
||||
value: utils::u256_to_bytes(U256::from(500u64)).to_vec(),
|
||||
value: utils::u256_to_bytes(U256::from(900u64)).to_vec(),
|
||||
})
|
||||
.execute(&mut *conn)
|
||||
.await
|
||||
@@ -282,7 +282,7 @@ async fn evaluate_rejects_volume_over_limit() {
|
||||
chain_id: CHAIN_ID as i32,
|
||||
token_contract: DAI.to_vec(),
|
||||
recipient_address: RECIPIENT.to_vec(),
|
||||
value: utils::u256_to_bytes(U256::from(1_001u64)).to_vec(),
|
||||
value: utils::u256_to_bytes(U256::from(1_000u64)).to_vec(),
|
||||
})
|
||||
.execute(&mut *conn)
|
||||
.await
|
||||
@@ -294,7 +294,7 @@ async fn evaluate_rejects_volume_over_limit() {
|
||||
shared: shared(),
|
||||
settings,
|
||||
};
|
||||
let calldata = transfer_calldata(RECIPIENT, U256::from(100u64));
|
||||
let calldata = transfer_calldata(RECIPIENT, U256::from(1u64));
|
||||
let context = ctx(DAI, calldata);
|
||||
let m = TokenTransfer::analyze(&context).unwrap();
|
||||
let v = TokenTransfer::evaluate(&context, &m, &grant, &mut *conn)
|
||||
|
||||
@@ -165,3 +165,69 @@ pub async fn test_challenge_auth() {
|
||||
|
||||
task.await.unwrap().unwrap();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[test_log::test]
|
||||
pub async fn test_challenge_auth_rejects_invalid_signature() {
|
||||
let db = db::create_test_pool().await;
|
||||
let actors = GlobalActors::spawn(db.clone()).await.unwrap();
|
||||
|
||||
let new_key = ed25519_dalek::SigningKey::generate(&mut rand::rng());
|
||||
let pubkey_bytes = new_key.verifying_key().to_bytes().to_vec();
|
||||
|
||||
// Pre-register key with key_type
|
||||
{
|
||||
let mut conn = db.get().await.unwrap();
|
||||
insert_into(schema::useragent_client::table)
|
||||
.values((
|
||||
schema::useragent_client::public_key.eq(pubkey_bytes.clone()),
|
||||
schema::useragent_client::key_type.eq(1i32),
|
||||
))
|
||||
.execute(&mut conn)
|
||||
.await
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
let (server_transport, mut test_transport) = ChannelTransport::new();
|
||||
let db_for_task = db.clone();
|
||||
let task = tokio::spawn(async move {
|
||||
let mut props = UserAgentConnection::new(db_for_task, actors);
|
||||
auth::authenticate(&mut props, server_transport).await
|
||||
});
|
||||
|
||||
test_transport
|
||||
.send(auth::Inbound::AuthChallengeRequest {
|
||||
pubkey: AuthPublicKey::Ed25519(new_key.verifying_key()),
|
||||
bootstrap_token: None,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let response = test_transport
|
||||
.recv()
|
||||
.await
|
||||
.expect("should receive challenge");
|
||||
let challenge = match response {
|
||||
Ok(resp) => match resp {
|
||||
auth::Outbound::AuthChallenge { nonce } => nonce,
|
||||
other => panic!("Expected AuthChallenge, got {other:?}"),
|
||||
},
|
||||
Err(err) => panic!("Expected Ok response, got Err({err:?})"),
|
||||
};
|
||||
|
||||
// Sign a different challenge value so signature format is valid but verification must fail.
|
||||
let wrong_challenge = arbiter_proto::format_challenge(challenge + 1, &pubkey_bytes);
|
||||
let signature = new_key.sign(&wrong_challenge);
|
||||
|
||||
test_transport
|
||||
.send(auth::Inbound::AuthChallengeSolution {
|
||||
signature: signature.to_bytes().to_vec(),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert!(matches!(
|
||||
task.await.unwrap(),
|
||||
Err(auth::Error::InvalidChallengeSolution)
|
||||
));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user