fix-security #42
@@ -214,7 +214,6 @@ impl KeyHolder {
|
|||||||
let mut conn = self.db.get().await?;
|
let mut conn = self.db.get().await?;
|
||||||
schema::root_key_history::table
|
schema::root_key_history::table
|
||||||
.filter(schema::root_key_history::id.eq(*root_key_history_id))
|
.filter(schema::root_key_history::id.eq(*root_key_history_id))
|
||||||
.select(schema::root_key_history::data_encryption_nonce)
|
|
||||||
.select(RootKeyHistory::as_select())
|
.select(RootKeyHistory::as_select())
|
||||||
.first(&mut conn)
|
.first(&mut conn)
|
||||||
.await?
|
.await?
|
||||||
|
|||||||
@@ -210,13 +210,16 @@ where
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
if valid {
|
if !valid {
|
||||||
self.transport
|
error!("Invalid challenge solution signature");
|
||||||
.send(Ok(Outbound::AuthSuccess))
|
return Err(Error::InvalidChallengeSolution);
|
||||||
.await
|
|
||||||
.map_err(|_| Error::Transport)?;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
self.transport
|
||||||
|
.send(Ok(Outbound::AuthSuccess))
|
||||||
|
.await
|
||||||
|
.map_err(|_| Error::Transport)?;
|
||||||
|
|
||||||
Ok(key.clone())
|
Ok(key.clone())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -91,6 +91,7 @@ async fn query_relevant_past_transaction(
|
|||||||
|
|
||||||
async fn check_rate_limits(
|
async fn check_rate_limits(
|
||||||
grant: &Grant<Settings>,
|
grant: &Grant<Settings>,
|
||||||
|
current_transfer_value: U256,
|
||||||
db: &mut impl AsyncConnection<Backend = Sqlite>,
|
db: &mut impl AsyncConnection<Backend = Sqlite>,
|
||||||
) -> QueryResult<Vec<EvalViolation>> {
|
) -> QueryResult<Vec<EvalViolation>> {
|
||||||
let mut violations = Vec::new();
|
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 past_transaction = query_relevant_past_transaction(grant.id, window, db).await?;
|
||||||
|
|
||||||
let window_start = chrono::Utc::now() - grant.settings.limit.window;
|
let window_start = chrono::Utc::now() - grant.settings.limit.window;
|
||||||
let cumulative_volume: U256 = past_transaction
|
let prospective_cumulative_volume: U256 = past_transaction
|
||||||
.iter()
|
.iter()
|
||||||
.filter(|(_, timestamp)| timestamp >= &window_start)
|
.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);
|
violations.push(EvalViolation::VolumetricLimitExceeded);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -141,7 +142,7 @@ impl Policy for EtherTransfer {
|
|||||||
violations.push(EvalViolation::InvalidTarget { target: meaning.to });
|
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);
|
violations.extend(rate_violations);
|
||||||
|
|
||||||
Ok(violations)
|
Ok(violations)
|
||||||
|
|||||||
@@ -198,7 +198,7 @@ async fn evaluate_rejects_volume_over_limit() {
|
|||||||
grant_id,
|
grant_id,
|
||||||
wallet_access_id: WALLET_ACCESS_ID,
|
wallet_access_id: WALLET_ACCESS_ID,
|
||||||
chain_id: CHAIN_ID as i32,
|
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()),
|
signed_at: SqliteTimestamp(Utc::now()),
|
||||||
})
|
})
|
||||||
.execute(&mut *conn)
|
.execute(&mut *conn)
|
||||||
@@ -211,7 +211,7 @@ async fn evaluate_rejects_volume_over_limit() {
|
|||||||
shared: shared(),
|
shared: shared(),
|
||||||
settings,
|
settings,
|
||||||
};
|
};
|
||||||
let context = ctx(ALLOWED, U256::from(100u64));
|
let context = ctx(ALLOWED, U256::from(1u64));
|
||||||
let m = EtherTransfer::analyze(&context).unwrap();
|
let m = EtherTransfer::analyze(&context).unwrap();
|
||||||
let v = EtherTransfer::evaluate(&context, &m, &grant, &mut *conn)
|
let v = EtherTransfer::evaluate(&context, &m, &grant, &mut *conn)
|
||||||
.await
|
.await
|
||||||
@@ -233,13 +233,13 @@ async fn evaluate_passes_at_exactly_volume_limit() {
|
|||||||
.await
|
.await
|
||||||
.unwrap();
|
.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)
|
insert_into(evm_transaction_log::table)
|
||||||
.values(NewEvmTransactionLog {
|
.values(NewEvmTransactionLog {
|
||||||
grant_id,
|
grant_id,
|
||||||
wallet_access_id: WALLET_ACCESS_ID,
|
wallet_access_id: WALLET_ACCESS_ID,
|
||||||
chain_id: CHAIN_ID as i32,
|
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()),
|
signed_at: SqliteTimestamp(Utc::now()),
|
||||||
})
|
})
|
||||||
.execute(&mut *conn)
|
.execute(&mut *conn)
|
||||||
|
|||||||
@@ -101,6 +101,7 @@ async fn query_relevant_past_transfers(
|
|||||||
|
|
||||||
async fn check_volume_rate_limits(
|
async fn check_volume_rate_limits(
|
||||||
grant: &Grant<Settings>,
|
grant: &Grant<Settings>,
|
||||||
|
current_transfer_value: U256,
|
||||||
db: &mut impl AsyncConnection<Backend = Sqlite>,
|
db: &mut impl AsyncConnection<Backend = Sqlite>,
|
||||||
) -> QueryResult<Vec<EvalViolation>> {
|
) -> QueryResult<Vec<EvalViolation>> {
|
||||||
let mut violations = Vec::new();
|
let mut violations = Vec::new();
|
||||||
@@ -113,12 +114,12 @@ async fn check_volume_rate_limits(
|
|||||||
|
|
||||||
for limit in &grant.settings.volume_limits {
|
for limit in &grant.settings.volume_limits {
|
||||||
let window_start = chrono::Utc::now() - limit.window;
|
let window_start = chrono::Utc::now() - limit.window;
|
||||||
let cumulative_volume: U256 = past_transfers
|
let prospective_cumulative_volume: U256 = past_transfers
|
||||||
.iter()
|
.iter()
|
||||||
.filter(|(_, timestamp)| timestamp >= &window_start)
|
.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);
|
violations.push(EvalViolation::VolumetricLimitExceeded);
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
@@ -163,7 +164,7 @@ impl Policy for TokenTransfer {
|
|||||||
violations.push(EvalViolation::InvalidTarget { target: meaning.to });
|
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);
|
violations.extend(rate_violations);
|
||||||
|
|
||||||
Ok(violations)
|
Ok(violations)
|
||||||
|
|||||||
@@ -220,7 +220,7 @@ async fn evaluate_rejects_wrong_restricted_recipient() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[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 db = db::create_test_pool().await;
|
||||||
let mut conn = db.get().await.unwrap();
|
let mut conn = db.get().await.unwrap();
|
||||||
|
|
||||||
@@ -230,7 +230,7 @@ async fn evaluate_passes_volume_within_limit() {
|
|||||||
.await
|
.await
|
||||||
.unwrap();
|
.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};
|
use crate::db::{models::NewEvmTokenTransferLog, schema::evm_token_transfer_log};
|
||||||
insert_into(evm_token_transfer_log::table)
|
insert_into(evm_token_transfer_log::table)
|
||||||
.values(NewEvmTokenTransferLog {
|
.values(NewEvmTokenTransferLog {
|
||||||
@@ -239,7 +239,7 @@ async fn evaluate_passes_volume_within_limit() {
|
|||||||
chain_id: CHAIN_ID as i32,
|
chain_id: CHAIN_ID as i32,
|
||||||
token_contract: DAI.to_vec(),
|
token_contract: DAI.to_vec(),
|
||||||
recipient_address: RECIPIENT.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)
|
.execute(&mut *conn)
|
||||||
.await
|
.await
|
||||||
@@ -282,7 +282,7 @@ async fn evaluate_rejects_volume_over_limit() {
|
|||||||
chain_id: CHAIN_ID as i32,
|
chain_id: CHAIN_ID as i32,
|
||||||
token_contract: DAI.to_vec(),
|
token_contract: DAI.to_vec(),
|
||||||
recipient_address: RECIPIENT.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)
|
.execute(&mut *conn)
|
||||||
.await
|
.await
|
||||||
@@ -294,7 +294,7 @@ async fn evaluate_rejects_volume_over_limit() {
|
|||||||
shared: shared(),
|
shared: shared(),
|
||||||
settings,
|
settings,
|
||||||
};
|
};
|
||||||
let calldata = transfer_calldata(RECIPIENT, U256::from(100u64));
|
let calldata = transfer_calldata(RECIPIENT, U256::from(1u64));
|
||||||
let context = ctx(DAI, calldata);
|
let context = ctx(DAI, calldata);
|
||||||
let m = TokenTransfer::analyze(&context).unwrap();
|
let m = TokenTransfer::analyze(&context).unwrap();
|
||||||
let v = TokenTransfer::evaluate(&context, &m, &grant, &mut *conn)
|
let v = TokenTransfer::evaluate(&context, &m, &grant, &mut *conn)
|
||||||
|
|||||||
@@ -165,3 +165,69 @@ pub async fn test_challenge_auth() {
|
|||||||
|
|
||||||
task.await.unwrap().unwrap();
|
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