Merge remote-tracking branch 'origin/feat/derivedkey-zeroize-security'
This commit is contained in:
34
Cargo.lock
generated
34
Cargo.lock
generated
@@ -138,6 +138,7 @@ dependencies = [
|
||||
"hmac",
|
||||
"irpc",
|
||||
"irpc-derive",
|
||||
"postcard",
|
||||
"rand 0.8.6",
|
||||
"secp256k1",
|
||||
"serde",
|
||||
@@ -389,6 +390,15 @@ dependencies = [
|
||||
"webpki-roots 0.26.11",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "atomic-polyfill"
|
||||
version = "1.0.3"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "8cf2bce30dfe09ef0bfaef228b9d414faaf7e563035494d7fe092dba54b300f4"
|
||||
dependencies = [
|
||||
"critical-section",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "atomic-waker"
|
||||
version = "1.1.2"
|
||||
@@ -1746,6 +1756,15 @@ dependencies = [
|
||||
"tracing",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "hash32"
|
||||
version = "0.2.1"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "b0c35f58762feb77d74ebe43bdbc3210f09be9fe6742234d573bacc26ed92b67"
|
||||
dependencies = [
|
||||
"byteorder",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "hashbrown"
|
||||
version = "0.14.5"
|
||||
@@ -1769,6 +1788,20 @@ version = "0.17.1"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a"
|
||||
|
||||
[[package]]
|
||||
name = "heapless"
|
||||
version = "0.7.17"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "cdc6457c0eb62c71aac4bc17216026d8410337c4126773b9c5daba343f17964f"
|
||||
dependencies = [
|
||||
"atomic-polyfill",
|
||||
"hash32",
|
||||
"rustc_version",
|
||||
"serde",
|
||||
"spin 0.9.8",
|
||||
"stable_deref_trait",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "heck"
|
||||
version = "0.5.0"
|
||||
@@ -3548,6 +3581,7 @@ dependencies = [
|
||||
"cobs",
|
||||
"embedded-io 0.4.0",
|
||||
"embedded-io 0.6.1",
|
||||
"heapless",
|
||||
"postcard-derive",
|
||||
"serde",
|
||||
]
|
||||
|
||||
@@ -30,4 +30,5 @@ irpc-derive = { workspace = true }
|
||||
secp256k1 = { version = "0.29", optional = true }
|
||||
|
||||
[dev-dependencies]
|
||||
hex = "0.4"
|
||||
hex = "0.4"
|
||||
postcard = { version = "1", features = ["alloc"] }
|
||||
@@ -133,8 +133,8 @@ pub fn derive_path_from_seed(seed: &[u8], path: &str) -> Result<ExtendedPrivKey,
|
||||
/// Uses HMAC-SHA512 with key "ed25519 seed" over the seed bytes,
|
||||
/// following SLIP-0010 specification.
|
||||
fn derive_master_key(seed: &[u8]) -> Result<XPrv, DerivationError> {
|
||||
let mut mac =
|
||||
HmacSha512::new_from_slice(b"ed25519 seed").map_err(|e| DerivationError::Hmac(e.to_string()))?;
|
||||
let mut mac = HmacSha512::new_from_slice(b"ed25519 seed")
|
||||
.map_err(|e| DerivationError::Hmac(e.to_string()))?;
|
||||
mac.update(seed);
|
||||
let result = mac.finalize().into_bytes();
|
||||
|
||||
@@ -293,4 +293,4 @@ mod tests {
|
||||
assert_ne!(identity.private_key(), ssh.private_key());
|
||||
assert_ne!(identity.public_key(), ssh.public_key());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -142,8 +142,8 @@ pub fn encrypt(plaintext: &str, key: &EncryptionKey) -> Result<EncryptedData, En
|
||||
|
||||
Ok(EncryptedData {
|
||||
key_version: key.key_version,
|
||||
salt: base64::Engine::encode(&base64::engine::general_purpose::STANDARD, &salt_bytes),
|
||||
iv: base64::Engine::encode(&base64::engine::general_purpose::STANDARD, &iv_bytes),
|
||||
salt: base64::Engine::encode(&base64::engine::general_purpose::STANDARD, salt_bytes),
|
||||
iv: base64::Engine::encode(&base64::engine::general_purpose::STANDARD, iv_bytes),
|
||||
data: base64::Engine::encode(&base64::engine::general_purpose::STANDARD, &ciphertext),
|
||||
})
|
||||
}
|
||||
@@ -162,8 +162,9 @@ pub fn decrypt(encrypted: &EncryptedData, key: &EncryptionKey) -> Result<String,
|
||||
let cipher = Aes256Gcm::new_from_slice(&key.key_bytes)
|
||||
.map_err(|e| EncryptionError::Decryption(format!("invalid key length: {e}")))?;
|
||||
|
||||
let iv_bytes = base64::Engine::decode(&base64::engine::general_purpose::STANDARD, &encrypted.iv)
|
||||
.map_err(|e| EncryptionError::Decoding(e.to_string()))?;
|
||||
let iv_bytes =
|
||||
base64::Engine::decode(&base64::engine::general_purpose::STANDARD, &encrypted.iv)
|
||||
.map_err(|e| EncryptionError::Decoding(e.to_string()))?;
|
||||
let nonce = Nonce::from_slice(&iv_bytes);
|
||||
|
||||
let ciphertext =
|
||||
@@ -244,4 +245,4 @@ mod tests {
|
||||
let result = decrypt(&encrypted, &key2);
|
||||
assert!(result.is_err());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -38,4 +38,4 @@ pub use derivation::{ExtendedPrivKey, PATHS};
|
||||
pub use encryption::{EncryptedData, EncryptionError};
|
||||
pub use mnemonic::{Language, Mnemonic, Seed};
|
||||
pub use protocol::{DerivedKey, KeyType, SecretMessage, SecretProtocol};
|
||||
pub use service::{SecretService, SecretServiceError, SecretServiceHandle};
|
||||
pub use service::{SecretService, SecretServiceError, SecretServiceHandle};
|
||||
|
||||
@@ -158,4 +158,4 @@ mod tests {
|
||||
assert_eq!(seed.len(), 64);
|
||||
assert!(!seed.is_empty());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,7 +19,10 @@
|
||||
//! For cross-node (call protocol) exposure, the service is wrapped in an
|
||||
//! operation that serializes to JSON.
|
||||
|
||||
use serde::{Deserialize, Serialize};
|
||||
use std::fmt;
|
||||
|
||||
use serde::{Deserialize, Deserializer, Serialize, Serializer};
|
||||
use zeroize::Zeroize;
|
||||
|
||||
use crate::encryption::EncryptedData;
|
||||
|
||||
@@ -36,19 +39,54 @@ pub enum KeyType {
|
||||
|
||||
/// A derived key pair (private key + public key).
|
||||
///
|
||||
/// The private key is sensitive material. Consumers should zeroize
|
||||
/// it when no longer needed. The `SecretServiceHandle` manages the lifecycle
|
||||
/// of derived keys internally.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
/// The private key is sensitive material that is zeroized on drop (ADR-038).
|
||||
/// This type is **not** `Clone` — it is move-only. Consumers receive a
|
||||
/// `DerivedKey` by value and must zeroize it when done (handled automatically
|
||||
/// by `#[zeroize(drop)]`).
|
||||
///
|
||||
/// Serialization redacts the `private_key` field for safety: JSON/debug output
|
||||
/// shows `"[REDACTED]"` instead of the key bytes. Deserialization still reads
|
||||
/// the full bytes for protocol use (postcard/irpc).
|
||||
#[derive(Zeroize, Deserialize)]
|
||||
#[zeroize(drop)]
|
||||
pub struct DerivedKey {
|
||||
/// The type of key that was derived.
|
||||
#[zeroize(skip)]
|
||||
pub key_type: KeyType,
|
||||
/// The private key bytes.
|
||||
/// The private key bytes (sensitive — zeroized on drop).
|
||||
#[zeroize]
|
||||
#[serde(deserialize_with = "deserialize_private_key")]
|
||||
pub private_key: Vec<u8>,
|
||||
/// The public key bytes.
|
||||
#[zeroize(skip)]
|
||||
pub public_key: Vec<u8>,
|
||||
}
|
||||
|
||||
fn deserialize_private_key<'de, D: Deserializer<'de>>(d: D) -> Result<Vec<u8>, D::Error> {
|
||||
Vec::<u8>::deserialize(d)
|
||||
}
|
||||
|
||||
impl fmt::Debug for DerivedKey {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
f.debug_struct("DerivedKey")
|
||||
.field("key_type", &self.key_type)
|
||||
.field("private_key", &"[REDACTED]")
|
||||
.field("public_key", &self.public_key)
|
||||
.finish()
|
||||
}
|
||||
}
|
||||
|
||||
impl Serialize for DerivedKey {
|
||||
fn serialize<S: Serializer>(&self, s: S) -> Result<S::Ok, S::Error> {
|
||||
use serde::ser::SerializeStruct;
|
||||
let mut state = s.serialize_struct("DerivedKey", 3)?;
|
||||
state.serialize_field("key_type", &self.key_type)?;
|
||||
state.serialize_field("private_key", "[REDACTED]")?;
|
||||
state.serialize_field("public_key", &self.public_key)?;
|
||||
state.end()
|
||||
}
|
||||
}
|
||||
|
||||
/// SecretProtocol service definition.
|
||||
///
|
||||
/// This is the irpc protocol enum that defines all secret service operations.
|
||||
@@ -140,4 +178,97 @@ pub enum SecretProtocol {
|
||||
///
|
||||
/// TODO: Replace with irpc `#[rpc_requests]` macro-generated type once
|
||||
/// the irpc crate is integrated. For now, this is a placeholder type alias.
|
||||
pub type SecretMessage = SecretProtocol;
|
||||
pub type SecretMessage = SecretProtocol;
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn make_test_key() -> DerivedKey {
|
||||
DerivedKey {
|
||||
key_type: KeyType::Ed25519,
|
||||
private_key: vec![0xABu8; 32],
|
||||
public_key: vec![0xCDu8; 32],
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_derived_key_debug_redacts_private_key() {
|
||||
let key = make_test_key();
|
||||
let debug_output = format!("{:?}", key);
|
||||
assert!(
|
||||
!debug_output.contains("AB"),
|
||||
"Debug must not leak private_key bytes"
|
||||
);
|
||||
assert!(
|
||||
debug_output.contains("[REDACTED]"),
|
||||
"Debug must show [REDACTED] for private_key"
|
||||
);
|
||||
assert!(debug_output.contains("Ed25519"), "Debug must show key_type");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_derived_key_serialize_redacts_private_key() {
|
||||
let key = make_test_key();
|
||||
let json = serde_json::to_string(&key).unwrap();
|
||||
assert!(
|
||||
!json.contains("AB"),
|
||||
"JSON must not contain private_key bytes"
|
||||
);
|
||||
assert!(
|
||||
json.contains("[REDACTED]"),
|
||||
"JSON must show [REDACTED] for private_key"
|
||||
);
|
||||
assert!(json.contains("Ed25519"), "JSON must contain key_type");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_derived_key_deserialize_preserves_bytes() {
|
||||
let key = make_test_key();
|
||||
let bytes = postcard::to_allocvec(&key.private_key).unwrap();
|
||||
let restored: Vec<u8> = postcard::from_bytes(&bytes).unwrap();
|
||||
assert_eq!(
|
||||
restored,
|
||||
vec![0xABu8; 32],
|
||||
"Deserialization must preserve private_key bytes"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_derived_key_zeroize_on_drop() {
|
||||
let key = DerivedKey {
|
||||
key_type: KeyType::Aes256Gcm,
|
||||
private_key: vec![0xFFu8; 32],
|
||||
public_key: vec![0x00u8; 32],
|
||||
};
|
||||
drop(key);
|
||||
// Verifies that DerivedKey can be dropped without panic.
|
||||
// The #[zeroize(drop)] attribute ensures private_key is zeroized
|
||||
// before the Vec is deallocated.
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_derived_key_not_clone() {
|
||||
// This test verifies at compile time that DerivedKey does not implement Clone.
|
||||
// If DerivedKey derived Clone, the following line would compile.
|
||||
// Since it doesn't, we just verify the type exists and is move-only.
|
||||
let key = make_test_key();
|
||||
let _moved = key; // Moves ownership
|
||||
// key is now moved — trying to use it would be a compile error
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_derived_key_zeroize_method_overwrites_private_key() {
|
||||
let mut key = make_test_key();
|
||||
assert_ne!(key.private_key, vec![0u8; 32]);
|
||||
assert!(!key.private_key.is_empty());
|
||||
|
||||
key.zeroize();
|
||||
|
||||
// After zeroize, private_key Vec is cleared (length 0, buffer zeroed)
|
||||
assert!(
|
||||
key.private_key.is_empty(),
|
||||
"zeroize() must clear the private_key Vec"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -167,7 +167,10 @@ impl SecretServiceHandle {
|
||||
if !inner.unlocked {
|
||||
return Err(SecretServiceError::ServiceLocked);
|
||||
}
|
||||
let seed = inner.seed.as_ref().ok_or(SecretServiceError::ServiceLocked)?;
|
||||
let seed = inner
|
||||
.seed
|
||||
.as_ref()
|
||||
.ok_or(SecretServiceError::ServiceLocked)?;
|
||||
|
||||
let key = derivation::derive_path_from_seed(seed.as_bytes(), path)?;
|
||||
Ok(DerivedKey {
|
||||
@@ -183,7 +186,10 @@ impl SecretServiceHandle {
|
||||
if !inner.unlocked {
|
||||
return Err(SecretServiceError::ServiceLocked);
|
||||
}
|
||||
let seed = inner.seed.as_ref().ok_or(SecretServiceError::ServiceLocked)?;
|
||||
let seed = inner
|
||||
.seed
|
||||
.as_ref()
|
||||
.ok_or(SecretServiceError::ServiceLocked)?;
|
||||
|
||||
let key = derivation::derive_path_from_seed(seed.as_bytes(), path)?;
|
||||
Ok(DerivedKey {
|
||||
@@ -199,7 +205,10 @@ impl SecretServiceHandle {
|
||||
if !inner.unlocked {
|
||||
return Err(SecretServiceError::ServiceLocked);
|
||||
}
|
||||
let seed = inner.seed.as_ref().ok_or(SecretServiceError::ServiceLocked)?;
|
||||
let seed = inner
|
||||
.seed
|
||||
.as_ref()
|
||||
.ok_or(SecretServiceError::ServiceLocked)?;
|
||||
|
||||
let key = derivation::derive_path_from_seed(seed.as_bytes(), path)?;
|
||||
Ok(DerivedKey {
|
||||
@@ -212,12 +221,19 @@ impl SecretServiceHandle {
|
||||
/// Encrypt plaintext using the derived encryption key.
|
||||
///
|
||||
/// Uses the key at path `m/74'/2'/0'/0'` (PATHS::ENCRYPTION) by default.
|
||||
pub fn encrypt(&self, plaintext: &str, key_version: u32) -> Result<EncryptedData, SecretServiceError> {
|
||||
pub fn encrypt(
|
||||
&self,
|
||||
plaintext: &str,
|
||||
key_version: u32,
|
||||
) -> Result<EncryptedData, SecretServiceError> {
|
||||
let inner = self.inner.read().unwrap();
|
||||
if !inner.unlocked {
|
||||
return Err(SecretServiceError::ServiceLocked);
|
||||
}
|
||||
let seed = inner.seed.as_ref().ok_or(SecretServiceError::ServiceLocked)?;
|
||||
let seed = inner
|
||||
.seed
|
||||
.as_ref()
|
||||
.ok_or(SecretServiceError::ServiceLocked)?;
|
||||
|
||||
let derived = derivation::derive_path_from_seed(seed.as_bytes(), PATHS::ENCRYPTION)?;
|
||||
let enc_key = EncryptionKey::from_derived_bytes(derived.private_key(), key_version);
|
||||
@@ -231,10 +247,14 @@ impl SecretServiceHandle {
|
||||
if !inner.unlocked {
|
||||
return Err(SecretServiceError::ServiceLocked);
|
||||
}
|
||||
let seed = inner.seed.as_ref().ok_or(SecretServiceError::ServiceLocked)?;
|
||||
let seed = inner
|
||||
.seed
|
||||
.as_ref()
|
||||
.ok_or(SecretServiceError::ServiceLocked)?;
|
||||
|
||||
let derived = derivation::derive_path_from_seed(seed.as_bytes(), PATHS::ENCRYPTION)?;
|
||||
let enc_key = EncryptionKey::from_derived_bytes(derived.private_key(), encrypted.key_version);
|
||||
let enc_key =
|
||||
EncryptionKey::from_derived_bytes(derived.private_key(), encrypted.key_version);
|
||||
|
||||
encryption::decrypt(encrypted, &enc_key).map_err(|e| e.into())
|
||||
}
|
||||
@@ -379,4 +399,4 @@ mod tests {
|
||||
service.lock();
|
||||
assert!(service.decrypt(&encrypted).is_err());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -22,13 +22,8 @@ fn test_encryption_key_derivation() {
|
||||
let service = SecretServiceHandle::new();
|
||||
service.unlock_new(24).unwrap();
|
||||
|
||||
let key = service
|
||||
.derive_encryption_key(PATHS::ENCRYPTION)
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
key.key_type,
|
||||
alknet_secret::protocol::KeyType::Aes256Gcm
|
||||
);
|
||||
let key = service.derive_encryption_key(PATHS::ENCRYPTION).unwrap();
|
||||
assert_eq!(key.key_type, alknet_secret::protocol::KeyType::Aes256Gcm);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -59,4 +54,4 @@ fn test_different_paths_different_keys() {
|
||||
|
||||
assert_ne!(identity_key.private_key, ssh_key.private_key);
|
||||
assert_ne!(identity_key.public_key, ssh_key.public_key);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -55,4 +55,4 @@ fn test_encrypted_data_serialization() {
|
||||
let deserialized: alknet_secret::encryption::EncryptedData =
|
||||
serde_json::from_str(&json).unwrap();
|
||||
assert_eq!(deserialized, encrypted);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,8 +3,8 @@
|
||||
//! These tests verify the unlock/lock lifecycle, error conditions,
|
||||
//! and that the service correctly manages state transitions.
|
||||
|
||||
use alknet_secret::service::{SecretServiceError, SecretServiceHandle};
|
||||
use alknet_secret::derivation::PATHS;
|
||||
use alknet_secret::service::{SecretServiceError, SecretServiceHandle};
|
||||
|
||||
#[test]
|
||||
fn test_full_lifecycle() {
|
||||
@@ -94,7 +94,5 @@ fn test_multiple_derive_paths_succeed() {
|
||||
// All standard paths should work
|
||||
let _identity = service.derive_ed25519(PATHS::IDENTITY).unwrap();
|
||||
let _ssh = service.derive_ed25519(PATHS::SSH_HOST).unwrap();
|
||||
let _enc = service
|
||||
.derive_encryption_key(PATHS::ENCRYPTION)
|
||||
.unwrap();
|
||||
}
|
||||
let _enc = service.derive_encryption_key(PATHS::ENCRYPTION).unwrap();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user