vault: spec-conformance fixes from review (task: vault/review-vault-sync)

- EncryptionKey: remove Clone (move-only per spec), add custom redacting
  Debug impl, make new() private (cfg(test)), add pub(crate) key_bytes()
  accessor, make encrypt/decrypt pub(crate) module-internal helpers
- CachedKey: refactor to wrap DerivedKey (per service.md) with cached_at
  and last_accessed fields; add key_type()/private_key()/public_key()
  accessors
- Mnemonic: store validated Bip39Mnemonic to eliminate unwrap() in
  to_seed(); enable bip39 zeroize feature so inner is zeroized on drop
- Fix clippy: remove unused import in drop_tracker tests, use struct
  init syntax instead of field reassignment with Default
- Move low-level EncryptionKey round-trip/wrong-key tests from
  integration tests to unit tests (encrypt/decrypt now pub(crate))
This commit is contained in:
2026-06-23 14:07:24 +00:00
parent 968e3a09ee
commit 41f0fc7843
7 changed files with 172 additions and 103 deletions

1
Cargo.lock generated
View File

@@ -289,6 +289,7 @@ dependencies = [
"rand_core 0.6.4",
"serde",
"unicode-normalization",
"zeroize",
]
[[package]]

View File

@@ -14,7 +14,7 @@ default = []
secp256k1 = ["dep:secp256k1"]
[dependencies]
bip39 = { version = "2", features = ["rand"] }
bip39 = { version = "2", features = ["rand", "zeroize"] }
ed25519-bip32 = "0.4"
aes-gcm = "0.10"
sha2 = "0.10"

View File

@@ -10,7 +10,7 @@ use std::time::{Duration, Instant};
use zeroize::Zeroize;
use crate::protocol::KeyType;
use crate::protocol::{DerivedKey, KeyType};
/// Default TTL for cached keys (1 hour).
pub const DEFAULT_TTL: Duration = Duration::from_secs(3600);
@@ -18,47 +18,53 @@ pub const DEFAULT_TTL: Duration = Duration::from_secs(3600);
/// Default maximum number of cache entries.
pub const DEFAULT_MAX_ENTRIES: usize = 64;
/// A cached derived key with metadata for TTL and LRU tracking.
/// A cached derived key. Wraps a `DerivedKey` with cache metadata.
///
/// The `private_key` field is zeroized on drop via `#[zeroize(drop)]`.
/// This is a separate internal type from `DerivedKey` — it holds the same
/// data but is managed within the cache lifecycle.
/// Derives `Zeroize` and `ZeroizeOnDrop` — the private key is zeroized
/// when the entry is evicted (LRU/TTL) or the cache is cleared.
#[derive(Zeroize)]
#[zeroize(drop)]
pub struct CachedKey {
/// When this key was derived (for TTL checking).
/// The derived key (zeroized on drop).
#[zeroize(skip)]
pub derived_at: Instant,
/// The type of key that was derived.
pub key: DerivedKey,
/// When the entry was inserted (for TTL).
#[zeroize(skip)]
pub key_type: KeyType,
/// The private key bytes (sensitive — zeroized on drop).
#[zeroize]
pub private_key: Vec<u8>,
/// The public key bytes.
#[zeroize(skip)]
pub public_key: Vec<u8>,
pub cached_at: Instant,
/// Last access time for LRU ordering.
#[zeroize(skip)]
last_accessed: Instant,
}
impl CachedKey {
/// Create a new `CachedKey` from derived key material.
pub fn new(key_type: KeyType, private_key: Vec<u8>, public_key: Vec<u8>) -> Self {
/// Create a new `CachedKey` from a `DerivedKey`.
pub fn new(key: DerivedKey) -> Self {
let now = Instant::now();
Self {
derived_at: now,
key_type,
private_key,
public_key,
key,
cached_at: now,
last_accessed: now,
}
}
/// The key type of the cached derived key.
pub fn key_type(&self) -> &KeyType {
&self.key.key_type
}
/// The private key bytes of the cached derived key.
pub fn private_key(&self) -> &[u8] {
&self.key.private_key
}
/// The public key bytes of the cached derived key.
pub fn public_key(&self) -> &[u8] {
&self.key.public_key
}
/// Check whether this cached entry has expired.
pub fn is_expired(&self, ttl: Duration) -> bool {
Instant::now().duration_since(self.derived_at) > ttl
Instant::now().duration_since(self.cached_at) > ttl
}
/// Touch the entry to update its last-accessed time (for LRU).
@@ -212,8 +218,6 @@ mod drop_tracker {
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::Arc;
use super::*;
struct DropTrackedKey {
flag: Arc<AtomicBool>,
bytes: Vec<u8>,
@@ -289,7 +293,11 @@ mod tests {
use super::*;
fn make_cached_key(key_type: KeyType) -> CachedKey {
CachedKey::new(key_type, vec![0xABu8; 32], vec![0xCDu8; 32])
CachedKey::new(DerivedKey {
key_type,
private_key: vec![0xABu8; 32],
public_key: vec![0xCDu8; 32],
})
}
#[test]
@@ -298,7 +306,7 @@ mod tests {
cache.insert("m/74'/0'/0'/0'", make_cached_key(KeyType::Ed25519));
let entry = cache.get("m/74'/0'/0'/0'").unwrap();
assert_eq!(entry.key_type, KeyType::Ed25519);
assert_eq!(*entry.key_type(), KeyType::Ed25519);
}
#[test]
@@ -410,23 +418,33 @@ mod tests {
let mut cache = KeyCache::with_defaults();
cache.insert(
"path1",
CachedKey::new(KeyType::Ed25519, vec![1u8; 32], vec![2u8; 32]),
CachedKey::new(DerivedKey {
key_type: KeyType::Ed25519,
private_key: vec![1u8; 32],
public_key: vec![2u8; 32],
}),
);
cache.insert(
"path1",
CachedKey::new(KeyType::Aes256Gcm, vec![3u8; 32], vec![4u8; 32]),
CachedKey::new(DerivedKey {
key_type: KeyType::Aes256Gcm,
private_key: vec![3u8; 32],
public_key: vec![4u8; 32],
}),
);
let entry = cache.get("path1").unwrap();
assert_eq!(entry.key_type, KeyType::Aes256Gcm);
assert_eq!(entry.private_key, vec![3u8; 32]);
assert_eq!(*entry.key_type(), KeyType::Aes256Gcm);
assert_eq!(entry.private_key(), vec![3u8; 32]);
assert_eq!(cache.len(), 1);
}
#[test]
fn test_lru_eviction_drops_evicted_cached_key() {
let mut config = CacheConfig::default();
config.max_entries = 2;
let config = CacheConfig {
max_entries: 2,
..Default::default()
};
let mut cache = KeyCache::new(config);
@@ -444,8 +462,10 @@ mod tests {
#[test]
fn test_ttl_expiry_evicts_entry_on_access() {
let mut config = CacheConfig::default();
config.ttl = Duration::from_millis(1);
let config = CacheConfig {
ttl: Duration::from_millis(1),
..Default::default()
};
let mut cache = KeyCache::new(config);
cache.insert("path1", make_cached_key(KeyType::Ed25519));

View File

@@ -42,6 +42,7 @@ use aes_gcm::{
};
use rand::{rngs::OsRng, RngCore};
use serde::{Deserialize, Serialize};
use std::fmt;
use zeroize::Zeroize;
/// Current default key version for encryption.
@@ -83,8 +84,9 @@ pub struct EncryptedData {
/// Encryption key material derived from the seed.
///
/// Holds the 32-byte AES-256-GCM key and its derivation metadata.
/// Zeroized on drop per ADR-038.
#[derive(Clone, Zeroize)]
/// Zeroized on drop per ADR-038. Not `Clone` — move-only, like `DerivedKey`.
/// Implements a custom redacting `Debug` (never prints key bytes).
#[derive(Zeroize)]
#[zeroize(drop)]
pub struct EncryptionKey {
key_bytes: [u8; 32],
@@ -92,18 +94,21 @@ pub struct EncryptionKey {
}
impl EncryptionKey {
/// Create a new encryption key from raw bytes and a version number.
pub fn new(key_bytes: [u8; 32], key_version: u32) -> Self {
/// Construct from raw 32 bytes. Private — for internal use (tests).
#[cfg(test)]
fn new(key_bytes: [u8; 32], key_version: u32) -> Self {
Self {
key_bytes,
key_version,
}
}
/// Create a new encryption key from the first 32 bytes of derived key material.
///
/// The input is typically the private key bytes from derivation at path
/// `m/74'/2'/0'/0'`.
/// Take the first 32 bytes of derived key material (the private key
/// bytes from SLIP-0010 derivation) and construct an `EncryptionKey`.
/// This is the bridge from `DerivedKey` (SLIP-0010 output) to
/// `EncryptionKey` (AES-256-GCM input). `VaultServiceHandle::encrypt`
/// and `decrypt` call this on the cached `DerivedKey` to obtain the
/// `EncryptionKey` for the crypto layer.
pub fn from_derived_bytes(bytes: &[u8], key_version: u32) -> Self {
let mut key = [0u8; 32];
key.copy_from_slice(&bytes[..32]);
@@ -113,10 +118,24 @@ impl EncryptionKey {
}
}
/// Returns the key version.
/// Return the key version (for rotation tracking).
pub fn version(&self) -> u32 {
self.key_version
}
/// Return the key bytes (crate-internal — for `encrypt`/`decrypt`).
pub(crate) fn key_bytes(&self) -> &[u8; 32] {
&self.key_bytes
}
}
impl fmt::Debug for EncryptionKey {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("EncryptionKey")
.field("key_version", &self.key_version)
.field("key_bytes", &"[REDACTED]")
.finish()
}
}
/// Encrypt plaintext using an AES-256-GCM key.
@@ -133,8 +152,11 @@ impl EncryptionKey {
/// # Returns
///
/// An `EncryptedData` struct suitable for storage in the metagraph.
pub fn encrypt(plaintext: &str, key: &EncryptionKey) -> Result<EncryptedData, EncryptionError> {
let cipher = Aes256Gcm::new_from_slice(&key.key_bytes)
pub(crate) fn encrypt(
plaintext: &str,
key: &EncryptionKey,
) -> Result<EncryptedData, EncryptionError> {
let cipher = Aes256Gcm::new_from_slice(key.key_bytes())
.map_err(|e| EncryptionError::Encryption(format!("invalid key length: {e}")))?;
// Generate random IV (12 bytes for AES-GCM) using OsRng CSPRNG
@@ -168,8 +190,11 @@ pub fn encrypt(plaintext: &str, key: &EncryptionKey) -> Result<EncryptedData, En
/// # Returns
///
/// The decrypted plaintext string.
pub fn decrypt(encrypted: &EncryptedData, key: &EncryptionKey) -> Result<String, EncryptionError> {
let cipher = Aes256Gcm::new_from_slice(&key.key_bytes)
pub(crate) fn decrypt(
encrypted: &EncryptedData,
key: &EncryptionKey,
) -> Result<String, EncryptionError> {
let cipher = Aes256Gcm::new_from_slice(key.key_bytes())
.map_err(|e| EncryptionError::Decryption(format!("invalid key length: {e}")))?;
let iv_bytes =
@@ -255,4 +280,42 @@ mod tests {
let result = decrypt(&encrypted, &key2);
assert!(result.is_err());
}
#[test]
fn test_encryption_key_debug_redacts_key_bytes() {
let key = EncryptionKey::new([0xABu8; 32], 2);
let debug_output = format!("{:?}", key);
assert!(
debug_output.contains("[REDACTED]"),
"Debug must redact key_bytes, got: {debug_output}"
);
assert!(
!debug_output.contains("AB"),
"Debug must not leak key bytes, got: {debug_output}"
);
assert!(
debug_output.contains("key_version"),
"Debug must show key_version, got: {debug_output}"
);
}
#[test]
fn test_encryption_key_version_accessor() {
let key = EncryptionKey::new([0u8; 32], 7);
assert_eq!(key.version(), 7);
}
#[test]
fn test_encryption_key_key_bytes_accessor() {
let key = EncryptionKey::new([0x42u8; 32], 2);
assert_eq!(key.key_bytes(), &[0x42u8; 32]);
}
#[test]
fn test_encryption_key_from_derived_bytes_takes_first_32() {
let derived = [0xAAu8; 64];
let key = EncryptionKey::from_derived_bytes(&derived, 3);
assert_eq!(key.key_bytes(), &[0xAAu8; 32]);
assert_eq!(key.version(), 3);
}
}

View File

@@ -34,6 +34,7 @@ impl From<Language> for bip39::Language {
/// The internal phrase is zeroized on drop.
#[derive(Debug)]
pub struct Mnemonic {
inner: Bip39Mnemonic,
phrase: String,
}
@@ -44,9 +45,7 @@ impl Mnemonic {
pub fn generate(word_count: usize) -> Result<Self, MnemonicError> {
let mnemonic: Bip39Mnemonic = Bip39Mnemonic::generate(word_count)
.map_err(|e: bip39::Error| MnemonicError::Generation(e.to_string()))?;
Ok(Self {
phrase: mnemonic.to_string(),
})
Ok(Self::from_bip39(mnemonic))
}
/// Create a mnemonic from an existing phrase string.
@@ -55,9 +54,15 @@ impl Mnemonic {
pub fn from_phrase(phrase: &str, _language: Language) -> Result<Self, MnemonicError> {
let mnemonic: Bip39Mnemonic = Bip39Mnemonic::parse_normalized(phrase)
.map_err(|e: bip39::Error| MnemonicError::InvalidPhrase(e.to_string()))?;
Ok(Self {
phrase: mnemonic.to_string(),
})
Ok(Self::from_bip39(mnemonic))
}
fn from_bip39(mnemonic: Bip39Mnemonic) -> Self {
let phrase = mnemonic.to_string();
Self {
inner: mnemonic,
phrase,
}
}
/// Derive the master seed from this mnemonic.
@@ -65,9 +70,8 @@ impl Mnemonic {
/// The optional passphrase is used as the BIP39 password for PBKDF2
/// key derivation (BIP39 standard). An empty string means no passphrase.
pub fn to_seed(&self, passphrase: Option<&str>) -> Seed {
let mnemonic = Bip39Mnemonic::parse_normalized(&self.phrase).unwrap();
let normalized_passphrase = passphrase.unwrap_or("");
let seed_bytes = mnemonic.to_seed_normalized(normalized_passphrase);
let seed_bytes = self.inner.to_seed_normalized(normalized_passphrase);
Seed {
bytes: seed_bytes.to_vec(),
}
@@ -84,6 +88,7 @@ impl Mnemonic {
impl Zeroize for Mnemonic {
fn zeroize(&mut self) {
self.phrase.zeroize();
self.inner.zeroize();
}
}

View File

@@ -194,9 +194,9 @@ impl VaultServiceHandle {
if let Some(cached) = inner.cache.get(path) {
return Ok(DerivedKey {
key_type: cached.key_type.clone(),
private_key: cached.private_key.clone(),
public_key: cached.public_key.clone(),
key_type: cached.key_type().clone(),
private_key: cached.private_key().to_vec(),
public_key: cached.public_key().to_vec(),
});
}
@@ -204,8 +204,12 @@ impl VaultServiceHandle {
let key = derivation::derive_path_from_seed(seed.as_bytes(), path)?;
let private_key = key.private_key().to_vec();
let public_key = key.public_key().to_vec();
let cached = CachedKey::new(KeyType::Ed25519, private_key.clone(), public_key.clone());
inner.cache.insert(path, cached);
let derived = DerivedKey {
key_type: KeyType::Ed25519,
private_key: private_key.clone(),
public_key: public_key.clone(),
};
inner.cache.insert(path, CachedKey::new(derived));
Ok(DerivedKey {
key_type: KeyType::Ed25519,
private_key,
@@ -222,9 +226,9 @@ impl VaultServiceHandle {
if let Some(cached) = inner.cache.get(path) {
return Ok(DerivedKey {
key_type: cached.key_type.clone(),
private_key: cached.private_key.clone(),
public_key: cached.public_key.clone(),
key_type: cached.key_type().clone(),
private_key: cached.private_key().to_vec(),
public_key: cached.public_key().to_vec(),
});
}
@@ -232,8 +236,12 @@ impl VaultServiceHandle {
let key = derivation::derive_path_from_seed(seed.as_bytes(), path)?;
let private_key = key.private_key().to_vec();
let public_key = key.public_key().to_vec();
let cached = CachedKey::new(KeyType::Aes256Gcm, private_key.clone(), public_key.clone());
inner.cache.insert(path, cached);
let derived = DerivedKey {
key_type: KeyType::Aes256Gcm,
private_key: private_key.clone(),
public_key: public_key.clone(),
};
inner.cache.insert(path, CachedKey::new(derived));
Ok(DerivedKey {
key_type: KeyType::Aes256Gcm,
private_key,
@@ -273,9 +281,9 @@ impl VaultServiceHandle {
if let Some(cached) = inner.cache.get(path) {
return Ok(DerivedKey {
key_type: cached.key_type.clone(),
private_key: cached.private_key.clone(),
public_key: cached.public_key.clone(),
key_type: cached.key_type().clone(),
private_key: cached.private_key().to_vec(),
public_key: cached.public_key().to_vec(),
});
}
@@ -284,9 +292,12 @@ impl VaultServiceHandle {
let key = crate::ethereum::derive_secp256k1_path(seed.as_bytes(), path)?;
let private_key = key.private_key().to_vec();
let public_key = key.public_key().to_vec();
let cached =
CachedKey::new(KeyType::Secp256k1, private_key.clone(), public_key.clone());
inner.cache.insert(path, cached);
let derived = DerivedKey {
key_type: KeyType::Secp256k1,
private_key: private_key.clone(),
public_key: public_key.clone(),
};
inner.cache.insert(path, CachedKey::new(derived));
Ok(DerivedKey {
key_type: KeyType::Secp256k1,
private_key,

View File

@@ -18,7 +18,6 @@
//! representation handles clamping differently.
use alknet_vault::derivation::{derive_path_from_seed, PATHS};
use alknet_vault::encryption::{decrypt, encrypt, EncryptionKey, CURRENT_KEY_VERSION};
use alknet_vault::mnemonic::{Language, Mnemonic};
use alknet_vault::protocol::KeyType;
@@ -305,36 +304,6 @@ fn test_aes256gcm_known_key_encrypt_decrypt() {
);
}
/// AES-256-GCM: encrypt/decrypt round-trip through our EncryptionKey API.
#[test]
fn test_aes256gcm_encryption_key_round_trip() {
let key_bytes: [u8; 32] = [0x42u8; 32];
let key = EncryptionKey::new(key_bytes, CURRENT_KEY_VERSION);
let plaintext = "known-plaintext-for-aes-256-gcm-test";
let encrypted = encrypt(plaintext, &key).unwrap();
let decrypted = decrypt(&encrypted, &key).unwrap();
assert_eq!(
decrypted, plaintext,
"Round-trip through our API must preserve plaintext"
);
}
/// AES-256-GCM: wrong key produces decryption failure.
#[test]
fn test_aes256gcm_wrong_key_fails() {
let key1 = EncryptionKey::new([0x01u8; 32], CURRENT_KEY_VERSION);
let key2 = EncryptionKey::new([0x02u8; 32], CURRENT_KEY_VERSION);
let plaintext = "test-data-for-wrong-key";
let encrypted = encrypt(plaintext, &key1).unwrap();
let result = decrypt(&encrypted, &key2);
assert!(result.is_err(), "Decryption with wrong key must fail");
}
// ---------------------------------------------------------------------------
// Alknet-specific regression tests
// ---------------------------------------------------------------------------