4.2 KiB
id, name, status, depends_on, scope, risk, impact, level
| id | name | status | depends_on | scope | risk | impact | level | |
|---|---|---|---|---|---|---|---|---|
| derivedkey-zeroize-security | Make DerivedKey private_key Zeroize-derived and fix clone semantics for ADR-038 compliance | completed |
|
narrow | low | component | implementation |
Description
The DerivedKey struct in protocol.rs carries private_key: Vec<u8> which is sensitive key material, but it derives Clone and Serialize/Deserialize without any zeroize protection. Per ADR-038, all sensitive material must implement Zeroize and be zeroized on drop.
The current code:
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct DerivedKey {
pub key_type: KeyType,
pub private_key: Vec<u8>, // SENSITIVE — must zeroize
pub public_key: Vec<u8>, // Not sensitive (public)
}
Problems:
private_keydoesn't deriveZeroize— it stays in memory afterDerivedKeyis droppedClonecopiesprivate_keywithout zeroizing the source — if the clone is dropped, one copy may linger- The struct is
Serialize— serializingprivate_keyto JSON is a potential leak vector (logging, debug output)
Fix approach:
- Make
DerivedKeyimplementZeroizewith#[zeroize(drop)] - Replace
#[derive(Clone)]with a manualCloneimpl that zeroizes the source'sprivate_keyafter copying (move semantics through clone — the source key is consumed, not left in memory) - OR change the API to return
DerivedKeyby value only (no Clone) — consumers get one copy and must zeroize it when done. This is the more conventional crypto API pattern. - Add
#[serde(skip_serializing)]or a custom serializer that redactsprivate_keyfrom JSON output (or use a dedicated display format that shows only the public key) KeyTypeandpublic_keyare not sensitive and can remain as-is
Important: This change affects the SecretServiceHandle methods that return DerivedKey. If DerivedKey becomes non-Clone, those methods must return DerivedKey by value (which they already do — they construct a new DerivedKey each time). The key caching task (which adds a cache) will need to handle this carefully.
Acceptance Criteria
DerivedKeyderivesZeroizewith#[zeroize(drop)]on theprivate_keyfieldDerivedKeydoes NOT deriveClone— it's a move-only type (consumers must zeroize when done)DerivedKeyserialization redactsprivate_key— JSON output shows a placeholder (e.g.,"[REDACTED]") instead of key bytesDerivedKey::zeroize()overwritesprivate_keywith zerosDropforDerivedKeycallszeroize()on theprivate_keyfield- Existing
SecretServiceHandlemethods compile withoutClone(they already returnDerivedKeyby value) - Unit test:
DerivedKeyzeroesprivate_keyon drop - Unit test:
DerivedKeyserialization does NOT containprivate_keybytes - ADR-038 compliance: all types holding private key material derive
Zeroize
References
- docs/architecture/secret-service.md — DerivedKey specification
- docs/architecture/decisions/038-seed-lifecycle-memory-security.md — ADR-038
- crates/alknet-secret/src/protocol.rs — Current DerivedKey definition
- crates/alknet-secret/src/derivation.rs — ExtendedPrivKey (already Zeroize-derived)
Notes
The
ExtendedPrivKeyinderivation.rsalready correctly implementsZeroizewith#[zeroize(drop)]. This task brings the same security discipline toDerivedKey.
Making
DerivedKeynon-Clone is the safer choice. In crypto APIs, returning key material by value and requiring explicit zeroization is the standard pattern. The key cache (in the caching task) will hold derived keys in an internal cache type, not inDerivedKeydirectly.
For serialization redaction: consider a custom
Serializeimpl that serializesprivate_keyas"[REDACTED]"for JSON display butDeserializestill reads the full bytes for protocol use. Alternatively,private_keycan be skipped entirely in serialization (sinceDerivedKeyis intended for local use, not wire transfer — the irpc protocol sendsDerivedKeythrough postcard, not JSON). The key cache task will need to handle this.
Summary
To be filled on completion