fix!: protect evm_wallet integrity and bind key ciphertext to wallet address
Three independent failures allowed an offline attacker with DB write access to sign transactions using a different wallet's private key: 1. evm_wallet had no HMAC envelope — aead_encrypted_id could be swapped silently. 2. AEAD used a static tag as AAD — any valid ciphertext decrypted as any wallet key. 3. No post-decryption check that the derived address matched the requested wallet. Fix: sign_entity covers (address, aead_encrypted_id) in a single transaction; CreateNew/Decrypt take caller-provided AAD (wallet address bytes); after decryption signer.address() is verified against the requested wallet address.
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
use crate::{
|
||||
actors::vault::{CreateNew, Decrypt, Vault},
|
||||
crypto::integrity,
|
||||
crypto::integrity::{self, Integrable},
|
||||
db::{
|
||||
DatabaseError, DatabasePool,
|
||||
models::{self},
|
||||
@@ -25,14 +25,35 @@ use diesel::{
|
||||
use diesel_async::RunQueryDsl;
|
||||
use kameo::{Actor, actor::ActorRef, messages};
|
||||
use rand::{SeedableRng, rng, rngs::StdRng};
|
||||
use tracing::error;
|
||||
|
||||
pub use crate::evm::safe_signer;
|
||||
|
||||
/// Integrity guard that binds a wallet's encrypted key ID to its Ethereum address.
|
||||
/// Both fields are included in the HMAC — swapping `aead_encrypted_id` in the DB
|
||||
/// invalidates the envelope MAC, and the AEAD ciphertext is also bound to `address`
|
||||
/// as AAD, so decryption fails too.
|
||||
#[derive(arbiter_macros::Hashable)]
|
||||
struct EvmWalletIntegrity {
|
||||
aead_encrypted_id: i32,
|
||||
address: Address,
|
||||
}
|
||||
|
||||
impl Integrable for EvmWalletIntegrity {
|
||||
const KIND: &'static str = "evm_wallet";
|
||||
}
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
pub enum SignTransactionError {
|
||||
#[error("Wallet not found")]
|
||||
WalletNotFound,
|
||||
|
||||
#[error("Decrypted key does not match requested wallet address")]
|
||||
KeyAddressMismatch,
|
||||
|
||||
#[error("Internal signing error")]
|
||||
Internal,
|
||||
|
||||
#[error("Database error: {0}")]
|
||||
Database(#[from] DatabaseError),
|
||||
|
||||
@@ -64,6 +85,12 @@ pub enum Error {
|
||||
Integrity(#[from] integrity::Error),
|
||||
}
|
||||
|
||||
impl From<diesel::result::Error> for Error {
|
||||
fn from(e: diesel::result::Error) -> Self {
|
||||
Self::Database(DatabaseError::from(e))
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Actor)]
|
||||
pub struct EvmActor {
|
||||
pub vault: ActorRef<Vault>,
|
||||
@@ -97,20 +124,39 @@ impl EvmActor {
|
||||
|
||||
let aead_id: i32 = self
|
||||
.vault
|
||||
.ask(CreateNew { plaintext })
|
||||
.ask(CreateNew {
|
||||
plaintext,
|
||||
aad: address.as_slice().to_vec(),
|
||||
})
|
||||
.await
|
||||
.map_err(|_| Error::VaultSend)?;
|
||||
|
||||
let mut conn = self.db.get().await.map_err(DatabaseError::from)?;
|
||||
let wallet_id = insert_into(schema::evm_wallet::table)
|
||||
.values(&models::NewEvmWallet {
|
||||
address: address.as_slice().to_vec(),
|
||||
aead_encrypted_id: aead_id,
|
||||
let wallet_id = conn
|
||||
.exclusive_transaction(async |conn| {
|
||||
let wallet_id: i32 = insert_into(schema::evm_wallet::table)
|
||||
.values(&models::NewEvmWallet {
|
||||
address: address.as_slice().to_vec(),
|
||||
aead_encrypted_id: aead_id,
|
||||
})
|
||||
.returning(schema::evm_wallet::id)
|
||||
.get_result(conn)
|
||||
.await
|
||||
.map_err(DatabaseError::from)
|
||||
.map_err(Error::Database)?;
|
||||
|
||||
integrity::sign_entity(
|
||||
conn,
|
||||
&self.vault,
|
||||
&EvmWalletIntegrity { address, aead_encrypted_id: aead_id },
|
||||
wallet_id,
|
||||
)
|
||||
.await
|
||||
.map_err(Error::Integrity)?;
|
||||
|
||||
Ok::<i32, Error>(wallet_id)
|
||||
})
|
||||
.returning(schema::evm_wallet::id)
|
||||
.get_result(&mut conn)
|
||||
.await
|
||||
.map_err(DatabaseError::from)?;
|
||||
.await?;
|
||||
|
||||
Ok((wallet_id, address))
|
||||
}
|
||||
@@ -241,16 +287,51 @@ impl EvmActor {
|
||||
.ok_or(SignTransactionError::WalletNotFound)?;
|
||||
drop(conn);
|
||||
|
||||
let mut conn = self.db.get().await.map_err(DatabaseError::from)?;
|
||||
let attestation = integrity::verify_entity(
|
||||
&mut conn,
|
||||
&self.vault,
|
||||
&EvmWalletIntegrity {
|
||||
address: wallet_address,
|
||||
aead_encrypted_id: wallet.aead_encrypted_id,
|
||||
},
|
||||
wallet.id,
|
||||
)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
error!(?e, wallet_id = wallet.id, "EVM wallet integrity check failed");
|
||||
SignTransactionError::Internal
|
||||
})?;
|
||||
drop(conn);
|
||||
|
||||
if attestation != integrity::AttestationStatus::Attested {
|
||||
error!(
|
||||
wallet_id = wallet.id,
|
||||
"EVM wallet integrity unavailable; refusing to sign"
|
||||
);
|
||||
return Err(SignTransactionError::Internal);
|
||||
}
|
||||
|
||||
let raw_key: SafeCell<Vec<u8>> = self
|
||||
.vault
|
||||
.ask(Decrypt {
|
||||
aead_id: wallet.aead_encrypted_id,
|
||||
aad: wallet.address.clone(),
|
||||
})
|
||||
.await
|
||||
.map_err(|_| SignTransactionError::VaultSend)?;
|
||||
|
||||
let signer = safe_signer::SafeSigner::from_cell(raw_key)?;
|
||||
|
||||
if signer.address() != wallet_address {
|
||||
error!(
|
||||
expected = %wallet_address,
|
||||
actual = %signer.address(),
|
||||
"Decrypted private key address does not match requested wallet"
|
||||
);
|
||||
return Err(SignTransactionError::KeyAddressMismatch);
|
||||
}
|
||||
|
||||
self.engine
|
||||
.evaluate_transaction(wallet_access, transaction.clone(), RunKind::Execution)
|
||||
.await?;
|
||||
|
||||
@@ -298,8 +298,10 @@ impl Vault {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Decrypts an AEAD entry. The `aad` must match the value used at encryption time;
|
||||
/// a mismatch causes authentication failure, preventing cross-wallet key swaps.
|
||||
#[message]
|
||||
pub async fn decrypt(&mut self, aead_id: i32) -> Result<SafeCell<Vec<u8>>, Error> {
|
||||
pub async fn decrypt(&mut self, aead_id: i32, aad: Vec<u8>) -> Result<SafeCell<Vec<u8>>, Error> {
|
||||
let Unsealed { root_key, .. } = Self::expect_unsealed(&mut self.state)?;
|
||||
|
||||
let row: models::AeadEncrypted = {
|
||||
@@ -321,13 +323,15 @@ impl Vault {
|
||||
Error::BrokenDatabase
|
||||
})?;
|
||||
let mut output = SafeCell::new(row.ciphertext);
|
||||
root_key.decrypt_in_place(&nonce, v1::TAG, &mut output)?;
|
||||
root_key.decrypt_in_place(&nonce, &aad, &mut output)?;
|
||||
Ok(output)
|
||||
}
|
||||
|
||||
/// Creates a new `aead_encrypted` entry and returns its ID.
|
||||
/// The `aad` is bound into the ciphertext and must be reproduced exactly at decryption time.
|
||||
// Creates new `aead_encrypted` entry in the database and returns it's ID
|
||||
#[message]
|
||||
pub async fn create_new(&mut self, mut plaintext: SafeCell<Vec<u8>>) -> Result<i32, Error> {
|
||||
pub async fn create_new(&mut self, mut plaintext: SafeCell<Vec<u8>>, aad: Vec<u8>) -> Result<i32, Error> {
|
||||
let Unsealed {
|
||||
root_key,
|
||||
root_key_history_id,
|
||||
@@ -339,7 +343,7 @@ impl Vault {
|
||||
|
||||
let mut ciphertext_buffer = plaintext.write();
|
||||
let ciphertext_buffer: &mut Vec<u8> = ciphertext_buffer.as_mut();
|
||||
root_key.encrypt_in_place(&nonce, v1::TAG, &mut *ciphertext_buffer)?;
|
||||
root_key.encrypt_in_place(&nonce, &aad, &mut *ciphertext_buffer)?;
|
||||
|
||||
let ciphertext = std::mem::take(ciphertext_buffer);
|
||||
|
||||
@@ -474,7 +478,7 @@ mod tests {
|
||||
assert_eq!(root_row.data_encryption_nonce, n2.to_vec());
|
||||
|
||||
let id = actor
|
||||
.create_new(SafeCell::new(b"post-interleave".to_vec()))
|
||||
.create_new(SafeCell::new(b"post-interleave".to_vec()), b"test-aad".to_vec())
|
||||
.await
|
||||
.unwrap();
|
||||
let row: models::AeadEncrypted = schema::aead_encrypted::table
|
||||
|
||||
Reference in New Issue
Block a user