diff --git a/crates/akroasis/src/vault/mod.rs b/crates/akroasis/src/vault/mod.rs index 7c41307..498098a 100644 --- a/crates/akroasis/src/vault/mod.rs +++ b/crates/akroasis/src/vault/mod.rs @@ -579,7 +579,7 @@ mod tests { .unwrap(); let entry = vault.get("test-key").unwrap(); - assert_eq!(entry.secret, b"secret-123"); + assert_eq!(entry.secret.as_slice(), b"secret-123".as_slice()); assert_eq!(entry.credential_type, CredentialType::ApiKey); } @@ -640,7 +640,7 @@ mod tests { vault.rotate("rotate-key", b"new-secret").unwrap(); let entry = vault.get("rotate-key").unwrap(); - assert_eq!(entry.secret, b"new-secret"); + assert_eq!(entry.secret.as_slice(), b"new-secret".as_slice()); } #[test] @@ -658,7 +658,8 @@ mod tests { let entry = vault.get("binary-key").unwrap(); assert_eq!( - entry.secret, non_utf8_secret, + entry.secret.as_slice(), + non_utf8_secret, "secret bytes must round-trip exactly, with no UTF-8 lossy substitution" ); } diff --git a/crates/kryphos/src/config.rs b/crates/kryphos/src/config.rs index b9c03aa..dc91693 100644 --- a/crates/kryphos/src/config.rs +++ b/crates/kryphos/src/config.rs @@ -2,6 +2,7 @@ use figment::value::{Dict, Map, Value}; use figment::{Error, Metadata, Profile, Provider}; +use zeroize::Zeroizing; use crate::storage::Vault; @@ -82,13 +83,25 @@ impl

VaultProvider

{ .get(entry_name) .map_err(|e| Error::from(e.to_string()))?; - let secret_str = String::from_utf8(decrypted.secret).map_err(|_| { + // WHY: `decrypted.secret` is `Zeroizing>` and has + // no public escape hatch to a bare `Vec`/`String` — + // `str::from_utf8` borrows instead of consuming, so the + // validated bytes never leave the zeroizing wrapper. + // `decrypted` (and its `secret` field) is scrubbed on + // drop at the end of this function. + let validated = std::str::from_utf8(&decrypted.secret).map_err(|_| { Error::from(format!( "vault entry '{entry_name}' contains non-UTF-8 data" )) })?; - - Ok(Value::String(tag, secret_str)) + let secret_str = Zeroizing::new(validated.to_owned()); + + // NOTE: `figment::Value::String` requires an owned, + // non-zeroizing `String` — this clone (deref past the + // `Zeroizing` wrapper first) is the one unavoidable copy + // that crosses into a type we don't control. `secret_str` + // itself still zeroizes on drop immediately after. + Ok(Value::String(tag, (*secret_str).clone())) } else { Ok(Value::String(tag, s)) } diff --git a/crates/kryphos/src/key.rs b/crates/kryphos/src/key.rs index 4dc1e85..c55106e 100644 --- a/crates/kryphos/src/key.rs +++ b/crates/kryphos/src/key.rs @@ -4,7 +4,7 @@ use std::fmt; use serde::{Deserialize, Serialize}; use subtle::ConstantTimeEq; -use zeroize::{Zeroize, ZeroizeOnDrop}; +use zeroize::{Zeroize, ZeroizeOnDrop, Zeroizing}; use ed25519_dalek::Signer; @@ -44,11 +44,7 @@ impl SigningKey { /// /// Returns [`KeyError::WrongKeyLength`] if `bytes` is not 32 bytes. pub fn from_bytes(bytes: &[u8]) -> Result { - let arr: [u8; SIGNING_KEY_LEN] = - bytes.try_into().map_err(|_| KeyError::WrongKeyLength { - expected: SIGNING_KEY_LEN, - actual: bytes.len(), - })?; + let arr = zeroizing_key_array(bytes)?; Ok(Self { inner: ed25519_dalek::SigningKey::from_bytes(&arr), }) @@ -83,6 +79,26 @@ impl SigningKey { } } +/// Copies `bytes` into a fixed-size array wrapped for zero-on-drop. +/// +/// WHY: `TryInto<[u8; N]>` makes an unavoidable copy crossing from the +/// caller's slice into a stack array; `SigningKey::from_bytes` is the +/// call-frame directly above `unseal_signing_key` (vault.rs), whose own +/// decrypt-output copy is `Zeroizing`-wrapped (RUST/#218) — this closes the +/// next frame so that coverage does not stop one call short. +/// +/// # Errors +/// +/// Returns [`KeyError::WrongKeyLength`] if `bytes` is not `SIGNING_KEY_LEN` +/// bytes. +fn zeroizing_key_array(bytes: &[u8]) -> Result, KeyError> { + let arr: [u8; SIGNING_KEY_LEN] = bytes.try_into().map_err(|_| KeyError::WrongKeyLength { + expected: SIGNING_KEY_LEN, + actual: bytes.len(), + })?; + Ok(Zeroizing::new(arr)) +} + impl fmt::Debug for SigningKey { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str("SigningKey([REDACTED])") @@ -319,6 +335,25 @@ mod tests { assert!(result.is_err()); } + /// Dispositive by construction, same mechanism as + /// `decrypted_secret_is_zeroized_on_drop_by_type` (kryphos storage + /// tests): `[u8; N]` alone does not implement `ZeroizeOnDrop` (only + /// `Zeroize`), so this specific bound fails to compile against a bare + /// array and passes only because `zeroizing_key_array` returns + /// `Zeroizing<[u8; N]>`. + #[test] + fn signing_key_from_bytes_intermediate_array_is_zeroize_on_drop_by_type() { + fn assert_zeroizes_on_drop(_: &T) {} + + let bytes = [0x11; SIGNING_KEY_LEN]; + let arr = zeroizing_key_array(&bytes).unwrap(); + assert_zeroizes_on_drop(&arr); + assert_eq!( + *arr, bytes, + "wrapped array must carry the same bytes through" + ); + } + #[test] fn signing_key_debug_is_redacted() { let sk = SigningKey::generate(); diff --git a/crates/kryphos/src/storage.rs b/crates/kryphos/src/storage.rs index 61fa201..a6498de 100644 --- a/crates/kryphos/src/storage.rs +++ b/crates/kryphos/src/storage.rs @@ -10,6 +10,7 @@ use jiff::Timestamp; use koinon::{ChainKey, LogEntryKind, TamperLog, VerificationResult}; use serde::{Deserialize, Serialize}; use snafu::ResultExt; +use zeroize::Zeroizing; use crate::crypto::{self, decrypt, encrypt}; use crate::error::{ @@ -45,6 +46,13 @@ const TAMPER_LOG_FILE: &str = "tamper.log"; /// vault key from a leaked chain key is infeasible. const CHAIN_KEY_DOMAIN: &[u8] = b"kryphos/tamper-log/chain-key/v1"; +/// Domain-separation tag for deriving the fjall lookup-key subkey from +/// the vault's [`VaultKey`] (see [`Vault::lookup_key`]). +/// +/// Reuses the vault's existing secret rather than requiring a second one +/// to manage, mirroring [`CHAIN_KEY_DOMAIN`]. +const LOOKUP_KEY_DOMAIN: &[u8] = b"kryphos/vault/lookup-key/v1"; + /// On-disk vault header stored as JSON. #[derive(Debug, Serialize, Deserialize)] struct StoredHeader { @@ -55,10 +63,25 @@ struct StoredHeader { } /// Entry as stored in fjall (JSON-serialized value). +/// +/// Both fields are independently-nonced ChaCha20-Poly1305 ciphertexts. +/// `encrypted_metadata` decrypts to an [`EntryMetadataRecord`] carrying +/// the name, type, metadata, status, and history — none of it readable +/// from the fjall data directory without the vault key. Keeping it +/// separate from `encrypted_secret` means listing entries (which needs +/// only the metadata) never touches secret ciphertext. #[derive(Debug, Serialize, Deserialize)] struct StoredEntry { - credential_type: CredentialType, encrypted_secret: Vec, + encrypted_metadata: Vec, +} + +/// Plaintext form of everything but the secret value; JSON-serialized +/// and encrypted as `StoredEntry::encrypted_metadata`. +#[derive(Debug, Serialize, Deserialize)] +struct EntryMetadataRecord { + name: CompactString, + credential_type: CredentialType, metadata: EntryMetadata, #[serde(default)] status: EntryStatus, @@ -74,7 +97,12 @@ pub struct DecryptedEntry { /// What kind of credential this is. pub credential_type: CredentialType, /// The decrypted secret bytes. - pub secret: Vec, + /// + /// Wrapped in [`Zeroizing`] at the point of allocation (the return + /// of [`decrypt`], moved straight in — no unwrapped copy exists in + /// between) so the plaintext is scrubbed on drop rather than left in + /// freed heap memory. + pub secret: Zeroizing>, /// Associated metadata. pub metadata: EntryMetadata, } @@ -280,7 +308,9 @@ impl Vault { credential_type: CredentialType, secret: &[u8], ) -> Result<(), VaultError> { - if self.keyspace.get(name).map_err(fjall_err)?.is_some() { + let key = self.lookup_key(name); + + if self.keyspace.get(key).map_err(fjall_err)?.is_some() { return Err(VaultError::DuplicateEntry { name: name.to_owned(), }); @@ -289,9 +319,9 @@ impl Vault { let encrypted_secret = encrypt(&self.key, secret).context(EntryCryptoSnafu)?; let now = Timestamp::now(); - let entry = StoredEntry { + let record = EntryMetadataRecord { + name: CompactString::from(name), credential_type, - encrypted_secret, metadata: EntryMetadata { created_at: now, rotated_at: None, @@ -305,9 +335,15 @@ impl Vault { kind: HistoryEventKind::Created, }], }; + let encrypted_metadata = self.encrypt_metadata(&record)?; + + let entry = StoredEntry { + encrypted_secret, + encrypted_metadata, + }; let value = serde_json::to_vec(&entry).context(SerializationSnafu)?; - self.keyspace.insert(name, value).map_err(fjall_err)?; + self.keyspace.insert(key, value).map_err(fjall_err)?; self.db .persist(fjall::PersistMode::SyncAll) .map_err(fjall_err)?; @@ -324,50 +360,57 @@ impl Vault { /// Returns [`VaultError::EntryRevoked`] if the entry has been revoked. /// Returns [`VaultError::EntryCrypto`] if decryption fails. pub fn get(&self, name: &str) -> Result { - let raw = self.keyspace.get(name).map_err(fjall_err)?.ok_or_else(|| { + let key = self.lookup_key(name); + let raw = self.keyspace.get(key).map_err(fjall_err)?.ok_or_else(|| { VaultError::EntryNotFound { name: name.to_owned(), } })?; let entry: StoredEntry = serde_json::from_slice(&raw).context(SerializationSnafu)?; + let record = self.decrypt_metadata(&entry)?; - if entry.status == EntryStatus::Revoked { + if record.status == EntryStatus::Revoked { return EntryRevokedSnafu { name }.fail(); } - let secret = decrypt(&self.key, &entry.encrypted_secret).context(EntryCryptoSnafu)?; + // WHY: wrap at the point of allocation — `decrypt`'s return is moved + // straight into `Zeroizing::new` with no intermediate unwrapped + // binding, so there is no plaintext copy that this fix leaves + // unscrubbed on drop. + let secret = + Zeroizing::new(decrypt(&self.key, &entry.encrypted_secret).context(EntryCryptoSnafu)?); Ok(DecryptedEntry { - name: CompactString::from(name), - credential_type: entry.credential_type, + name: record.name, + credential_type: record.credential_type, secret, - metadata: entry.metadata, + metadata: record.metadata, }) } /// Lists all entries in the vault (names and metadata only). /// - /// No secrets are decrypted or returned. + /// No secrets are decrypted or returned: only `encrypted_metadata` is + /// touched, never `encrypted_secret`. /// /// # Errors /// /// Returns [`VaultError::StorageBackend`] on iteration errors. + /// Returns [`VaultError::EntryCrypto`] if metadata decryption fails. pub fn list(&self) -> Result, VaultError> { let mut entries = Vec::new(); for guard in self.keyspace.iter() { - let (key, value) = guard.into_inner().map_err(fjall_err)?; - let name = std::str::from_utf8(&key).map_err(|e| VaultError::StorageBackend { - message: format!("invalid UTF-8 key: {e}"), - })?; + let (_key, value) = guard.into_inner().map_err(fjall_err)?; let entry: StoredEntry = serde_json::from_slice(&value).context(SerializationSnafu)?; + let record = self.decrypt_metadata(&entry)?; entries.push(EntryInfo { - name: CompactString::from(name), - credential_type: entry.credential_type, - status: entry.status, - metadata: entry.metadata, + name: record.name, + credential_type: record.credential_type, + status: record.status, + metadata: record.metadata, }); } @@ -383,19 +426,21 @@ impl Vault { /// Returns [`VaultError::EntryNotFound`] if no entry with this name exists. /// Returns [`VaultError::EntryNotDeletable`] if the entry is revoked. pub fn remove(&self, name: &str) -> Result<(), VaultError> { - let raw = self.keyspace.get(name).map_err(fjall_err)?.ok_or_else(|| { + let key = self.lookup_key(name); + let raw = self.keyspace.get(key).map_err(fjall_err)?.ok_or_else(|| { VaultError::EntryNotFound { name: name.to_owned(), } })?; let entry: StoredEntry = serde_json::from_slice(&raw).context(SerializationSnafu)?; + let record = self.decrypt_metadata(&entry)?; - if entry.status == EntryStatus::Revoked { + if record.status == EntryStatus::Revoked { return EntryNotDeletableSnafu { name }.fail(); } - self.keyspace.remove(name).map_err(fjall_err)?; + self.keyspace.remove(key).map_err(fjall_err)?; self.db .persist(fjall::PersistMode::SyncAll) .map_err(fjall_err)?; @@ -415,30 +460,38 @@ impl Vault { /// Returns [`VaultError::EntryRevoked`] if the entry has been revoked. /// Returns [`VaultError::EntryCrypto`] if encryption fails. pub fn rotate(&self, name: &str, new_secret: &[u8]) -> Result<(), VaultError> { - let raw = self.keyspace.get(name).map_err(fjall_err)?.ok_or_else(|| { + let key = self.lookup_key(name); + let raw = self.keyspace.get(key).map_err(fjall_err)?.ok_or_else(|| { VaultError::EntryNotFound { name: name.to_owned(), } })?; - let mut entry: StoredEntry = serde_json::from_slice(&raw).context(SerializationSnafu)?; + let entry: StoredEntry = serde_json::from_slice(&raw).context(SerializationSnafu)?; + let mut record = self.decrypt_metadata(&entry)?; - if entry.status == EntryStatus::Revoked { + if record.status == EntryStatus::Revoked { return EntryRevokedSnafu { name }.fail(); } - entry.encrypted_secret = encrypt(&self.key, new_secret).context(EntryCryptoSnafu)?; + let encrypted_secret = encrypt(&self.key, new_secret).context(EntryCryptoSnafu)?; let now = Timestamp::now(); - entry.metadata.rotated_at = Some(now); - entry.metadata.rotation_count += 1; - entry.history.push(HistoryEvent { + record.metadata.rotated_at = Some(now); + record.metadata.rotation_count += 1; + record.history.push(HistoryEvent { timestamp: now, kind: HistoryEventKind::Rotated, }); + let encrypted_metadata = self.encrypt_metadata(&record)?; + + let entry = StoredEntry { + encrypted_secret, + encrypted_metadata, + }; let value = serde_json::to_vec(&entry).context(SerializationSnafu)?; - self.keyspace.insert(name, value).map_err(fjall_err)?; + self.keyspace.insert(key, value).map_err(fjall_err)?; self.db .persist(fjall::PersistMode::SyncAll) .map_err(fjall_err)?; @@ -457,28 +510,36 @@ impl Vault { /// Returns [`VaultError::EntryNotFound`] if no entry with this name exists. /// Returns [`VaultError::EntryRevoked`] if the entry is already revoked. pub fn revoke(&self, name: &str) -> Result<(), VaultError> { - let raw = self.keyspace.get(name).map_err(fjall_err)?.ok_or_else(|| { + let key = self.lookup_key(name); + let raw = self.keyspace.get(key).map_err(fjall_err)?.ok_or_else(|| { VaultError::EntryNotFound { name: name.to_owned(), } })?; - let mut entry: StoredEntry = serde_json::from_slice(&raw).context(SerializationSnafu)?; + let entry: StoredEntry = serde_json::from_slice(&raw).context(SerializationSnafu)?; + let mut record = self.decrypt_metadata(&entry)?; - if entry.status == EntryStatus::Revoked { + if record.status == EntryStatus::Revoked { return EntryRevokedSnafu { name }.fail(); } let now = Timestamp::now(); - entry.status = EntryStatus::Revoked; - entry.metadata.revoked_at = Some(now); - entry.history.push(HistoryEvent { + record.status = EntryStatus::Revoked; + record.metadata.revoked_at = Some(now); + record.history.push(HistoryEvent { timestamp: now, kind: HistoryEventKind::Revoked, }); + let encrypted_metadata = self.encrypt_metadata(&record)?; + + let entry = StoredEntry { + encrypted_secret: entry.encrypted_secret, + encrypted_metadata, + }; let value = serde_json::to_vec(&entry).context(SerializationSnafu)?; - self.keyspace.insert(name, value).map_err(fjall_err)?; + self.keyspace.insert(key, value).map_err(fjall_err)?; self.db .persist(fjall::PersistMode::SyncAll) .map_err(fjall_err)?; @@ -496,19 +557,21 @@ impl Vault { /// /// Returns [`VaultError::EntryNotFound`] if no entry with this name exists. pub fn history(&self, name: &str) -> Result { - let raw = self.keyspace.get(name).map_err(fjall_err)?.ok_or_else(|| { + let key = self.lookup_key(name); + let raw = self.keyspace.get(key).map_err(fjall_err)?.ok_or_else(|| { VaultError::EntryNotFound { name: name.to_owned(), } })?; let entry: StoredEntry = serde_json::from_slice(&raw).context(SerializationSnafu)?; + let record = self.decrypt_metadata(&entry)?; Ok(EntryHistory { - name: CompactString::from(name), - status: entry.status, - metadata: entry.metadata, - events: entry.history, + name: record.name, + status: record.status, + metadata: record.metadata, + events: record.history, }) } @@ -543,6 +606,32 @@ impl Vault { ChainKey::from_bytes(blake3::keyed_hash(self.key.as_bytes(), CHAIN_KEY_DOMAIN).into()) } + /// Derives the fjall record key for `name` via a two-step keyed BLAKE3 + /// hash of the vault key, so credential names never appear as fjall + /// keys on disk. + /// + /// Deterministic (same name -> same key), so `get`/`add`/`remove` stay + /// O(1) keyspace lookups without ever storing the name itself. A fresh + /// derivation on every call, mirroring [`Self::chain_key`]. + fn lookup_key(&self, name: &str) -> [u8; 32] { + let subkey = blake3::keyed_hash(self.key.as_bytes(), LOOKUP_KEY_DOMAIN); + blake3::keyed_hash(subkey.as_bytes(), name.as_bytes()).into() + } + + /// Decrypts and parses an entry's `encrypted_metadata` field. + fn decrypt_metadata(&self, entry: &StoredEntry) -> Result { + let metadata_bytes = + decrypt(&self.key, &entry.encrypted_metadata).context(EntryCryptoSnafu)?; + serde_json::from_slice(&metadata_bytes).context(SerializationSnafu) + } + + /// Serializes and encrypts an [`EntryMetadataRecord`] for storage as + /// `StoredEntry::encrypted_metadata`. + fn encrypt_metadata(&self, record: &EntryMetadataRecord) -> Result, VaultError> { + let metadata_bytes = serde_json::to_vec(record).context(SerializationSnafu)?; + encrypt(&self.key, &metadata_bytes).context(EntryCryptoSnafu) + } + fn append_vault_audit(&self, name: &str, operation: &str) -> Result<(), VaultError> { let mut log = TamperLog::open(self.tamper_log_path(), self.chain_key()).context(TamperLogSnafu)?; diff --git a/crates/kryphos/src/storage_tests.rs b/crates/kryphos/src/storage_tests.rs index ccd7ebf..a986191 100644 --- a/crates/kryphos/src/storage_tests.rs +++ b/crates/kryphos/src/storage_tests.rs @@ -62,7 +62,7 @@ fn add_and_get_round_trip() { let entry = vault.get("openai-key").unwrap(); assert_eq!(entry.name, "openai-key"); assert_eq!(entry.credential_type, CredentialType::ApiKey); - assert_eq!(entry.secret, secret); + assert_eq!(entry.secret.as_slice(), secret.as_slice()); } #[test] @@ -157,7 +157,11 @@ fn entries_persist_across_open_close() { let vault = Vault::open(&vault_path, TEST_PASSPHRASE).unwrap(); let entry = vault.get("persistent").unwrap(); - assert_eq!(entry.secret, b"cert-pem", "secret must survive close/open"); + assert_eq!( + entry.secret.as_slice(), + b"cert-pem".as_slice(), + "secret must survive close/open" + ); } #[test] @@ -190,7 +194,8 @@ fn rotate_updates_secret_and_preserves_name() { let entry = vault.get("api-key").unwrap(); assert_eq!(entry.name, "api-key", "name must be preserved after rotate"); assert_eq!( - entry.secret, b"new-secret", + entry.secret.as_slice(), + b"new-secret".as_slice(), "secret must be updated after rotate" ); assert_eq!( @@ -617,3 +622,90 @@ fn open_reports_absence_rather_than_io_failure_for_a_bare_directory() { "a headerless directory must report typed absence, got: {result:?}" ); } + +// ----------------------------------------------------------------- +// Secret zeroization (akroasis#218) +// ----------------------------------------------------------------- + +#[test] +fn decrypted_secret_is_zeroized_on_drop_by_type() { + // Regression for forkwright/akroasis#218: `DecryptedEntry.secret` used + // to be a bare `Vec`, which does not implement `ZeroizeOnDrop` — + // this assertion would FAIL TO COMPILE against that field type. Wrapping + // it in `Zeroizing>` (at the point `decrypt` returns, with no + // intermediate unwrapped copy) makes the guarantee type-enforced rather + // than a claim: `Zeroizing: ZeroizeOnDrop` for any `Z: Zeroize`, so + // this now compiles and holds for every `DecryptedEntry` the shipped + // `Vault::get` returns. + fn assert_zeroizes_on_drop(_: &T) {} + + let dir = tempfile::tempdir().unwrap(); + let vault_path = dir.path().join("zeroize-vault"); + + let vault = Vault::create(&vault_path, TEST_PASSPHRASE).unwrap(); + vault + .add("zeroize-key", CredentialType::ApiKey, b"scrub-me-on-drop") + .unwrap(); + + let entry = vault.get("zeroize-key").unwrap(); + assert_zeroizes_on_drop(&entry.secret); +} + +// ----------------------------------------------------------------- +// At-rest metadata encryption (akroasis#215) +// ----------------------------------------------------------------- + +#[test] +fn on_disk_fjall_contents_do_not_reveal_credential_name() { + // Regression for forkwright/akroasis#215: `StoredEntry` used to keep + // credential_type/metadata/status/history as plaintext JSON, and the + // fjall record KEY was the credential name itself in cleartext — so the + // name appeared on disk twice over, once as the key and (implicitly, via + // being findable) as an index into the plaintext value. This reads + // every file fjall actually wrote under the vault's `data/` directory + // and asserts the name is not a byte-for-byte substring of any of them. + // + // Before the fix this failed: the name was the literal fjall key, so it + // appears verbatim in the LSM tree's persisted pages. After the fix the + // fjall key is a keyed BLAKE3 hash of the name and the value is two + // ChaCha20-Poly1305 ciphertexts, neither of which can contain the + // plaintext name as a substring without breaking either primitive. + const DISTINCTIVE_NAME: &str = "surveillance-counter-mesh-psk-zzyzx9182"; + + let dir = tempfile::tempdir().unwrap(); + let vault_path = dir.path().join("metadata-at-rest-vault"); + + let vault = Vault::create(&vault_path, TEST_PASSPHRASE).unwrap(); + vault + .add(DISTINCTIVE_NAME, CredentialType::Psk, b"mesh-secret-value") + .unwrap(); + drop(vault); + + let mut on_disk = Vec::new(); + collect_file_bytes(&vault_path.join(DATA_DIR), &mut on_disk); + + assert!( + !on_disk.is_empty(), + "validation check: fjall must have written SOMETHING to the data directory" + ); + assert!( + !on_disk + .windows(DISTINCTIVE_NAME.len()) + .any(|window| window == DISTINCTIVE_NAME.as_bytes()), + "credential name must not appear in plaintext anywhere under the fjall data directory" + ); +} + +/// Recursively reads every regular file under `dir`, appending its bytes to +/// `out`. Test-only: used to scan fjall's actual on-disk output for +/// plaintext leakage rather than trusting the in-process API surface. +fn collect_file_bytes(dir: &std::path::Path, out: &mut Vec) { + for entry in std::fs::read_dir(dir).unwrap() { + let path = entry.unwrap().path(); + if path.is_dir() { + collect_file_bytes(&path, out); + } else if let Ok(bytes) = std::fs::read(&path) { + out.extend_from_slice(&bytes); + } + } +} diff --git a/crates/kryphos/src/vault.rs b/crates/kryphos/src/vault.rs index 3e44920..6ec3262 100644 --- a/crates/kryphos/src/vault.rs +++ b/crates/kryphos/src/vault.rs @@ -6,9 +6,10 @@ use compact_str::CompactString; use jiff::Timestamp; use serde::{Deserialize, Serialize}; use snafu::ResultExt; +use zeroize::Zeroizing; use crate::error::{CryptoError, KeyParseSnafu}; -use crate::key::{InstallationIdentity, SigningKey, VaultKey}; +use crate::key::{InstallationIdentity, SIGNING_KEY_LEN, SigningKey, VaultKey}; /// Size of the Argon2id salt in bytes. pub const SALT_LEN: usize = 16; @@ -17,7 +18,15 @@ pub const SALT_LEN: usize = 16; pub const NONCE_LEN: usize = 12; /// Current vault format version. -pub const VAULT_VERSION: u32 = 1; +/// +/// WARNING: bumping this is a hard break, not a migration point — `open` +/// rejects any header whose `version` does not match exactly, so a vault +/// written under a prior version simply fails to open under a newer one. +/// v2 changed `StoredEntry`'s on-disk shape: names, types, tags, status, and +/// history moved from plaintext fields into `encrypted_metadata`, and the +/// fjall record key changed from the plaintext name to a keyed-hash lookup +/// key, so a v1 store has neither the fields nor the keys v2 code expects. +pub const VAULT_VERSION: u32 = 2; /// The kind of credential stored in a [`VaultEntry`]. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -218,7 +227,12 @@ pub fn seal_signing_key( ) -> Result, CryptoError> { let cipher = ChaCha20Poly1305::new(vault_key.as_bytes().into()); let nonce = Nonce::from_slice(nonce); - let plaintext = identity.signing_key().to_bytes(); + // WHY: `to_bytes()` returns the raw Ed25519 signing key in an ordinary + // array; wrap immediately so this ephemeral copy is scrubbed on drop + // rather than left on the stack after `encrypt` returns — the same + // coverage `unseal_signing_key` gives the decrypt direction below + // (RUST/#218). + let plaintext = zeroizing_signing_key_bytes(identity); cipher .encrypt(nonce, plaintext.as_ref()) @@ -227,6 +241,13 @@ pub fn seal_signing_key( }) } +/// Copies the signing key's raw bytes into a zero-on-drop buffer. +fn zeroizing_signing_key_bytes( + identity: &InstallationIdentity, +) -> Zeroizing<[u8; SIGNING_KEY_LEN]> { + Zeroizing::new(identity.signing_key().to_bytes()) +} + /// Decrypts a signing key from vault ciphertext, reconstructing the /// [`InstallationIdentity`]. /// @@ -244,9 +265,21 @@ pub fn unseal_signing_key( let cipher = ChaCha20Poly1305::new(vault_key.as_bytes().into()); let nonce = Nonce::from_slice(nonce); - let plaintext = cipher - .decrypt(nonce, ciphertext) - .map_err(|_| CryptoError::DecryptionFailed)?; + // WHY: wrap at the point of allocation — `decrypt`'s return (the raw + // Ed25519 signing key bytes) is moved straight into `Zeroizing::new` + // with no intermediate unwrapped binding, so this ephemeral copy is + // scrubbed on drop rather than left in freed heap memory. + // `SigningKey::from_bytes` (key.rs) makes one further copy crossing + // into its own fixed-size array before constructing the + // `ZeroizeOnDrop`-protected `ed25519_dalek::SigningKey`; that copy is + // wrapped the same way (`zeroizing_key_array`), so no unprotected frame + // remains between this decrypt output and the protected key it becomes + // (RUST/#218). + let plaintext = Zeroizing::new( + cipher + .decrypt(nonce, ciphertext) + .map_err(|_| CryptoError::DecryptionFailed)?, + ); let signing = SigningKey::from_bytes(&plaintext).context(KeyParseSnafu)?; @@ -440,6 +473,28 @@ mod tests { ); } + /// Dispositive by construction, same mechanism as + /// `decrypted_secret_is_zeroized_on_drop_by_type` (kryphos storage + /// tests): `[u8; N]` alone does not implement `ZeroizeOnDrop` (only + /// `Zeroize`), so this specific bound fails to compile against a bare + /// array and passes only because `zeroizing_signing_key_bytes` returns + /// `Zeroizing<[u8; N]>` — the encrypt-direction counterpart of the + /// `Zeroizing` wrap `unseal_signing_key` already applies on decrypt. + #[test] + fn seal_signing_key_plaintext_buffer_is_zeroize_on_drop_by_type() { + use zeroize::ZeroizeOnDrop; + fn assert_zeroizes_on_drop(_: &T) {} + + let identity = InstallationIdentity::generate(); + let plaintext = zeroizing_signing_key_bytes(&identity); + assert_zeroizes_on_drop(&plaintext); + assert_eq!( + *plaintext, + identity.signing_key().to_bytes(), + "wrapped buffer must carry the same bytes through" + ); + } + #[test] fn unseal_with_wrong_key_fails() { let identity = InstallationIdentity::generate(); diff --git a/docs/fjall-column-encryption.md b/docs/fjall-column-encryption.md index e7345ca..ea5603a 100644 --- a/docs/fjall-column-encryption.md +++ b/docs/fjall-column-encryption.md @@ -3,7 +3,8 @@ Issue #132 tracks a future declarative encryption layer for fjall-backed stores. Current main does not have a generic table/column store abstraction: the only fjall-backed runtime store is `kryphos::Vault`, and it already -encrypts credential secrets through a typed field before serializing the row. +encrypts every field of a credential record — secret, name, type, metadata, +status, and history — through two typed fields before serializing the row. This note defines the boundary to use when akroasis adds its first mixed plaintext/ciphertext fjall schema for signals, references, or other indexed @@ -12,11 +13,18 @@ runtime data. It is not an implementation of #132. ## Current State - `crates/kryphos/src/storage.rs` owns one fjall keyspace named `entries`. -- `StoredEntry` keeps `encrypted_secret` as the only encrypted field; metadata, - status, and history remain structured so vault listing and lifecycle logic can - run without decrypting secret material. -- `Vault::add`, `Vault::get`, and `Vault::rotate` call the existing - ChaCha20-Poly1305 helpers directly for that one field. +- `StoredEntry` holds two independently-nonced ChaCha20-Poly1305 ciphertexts: + `encrypted_secret` and `encrypted_metadata`. `encrypted_metadata` decrypts to + an `EntryMetadataRecord` carrying name, credential type, metadata, status, + and history (akroasis#215) — nothing about a credential is plaintext at + rest. The fjall record KEY is a keyed-BLAKE3 hash of the name, not the name + itself, so no credential name appears verbatim in the fjall data directory. +- `Vault::list` decrypts only `encrypted_metadata`, never `encrypted_secret` — + callers already hold the vault key (an unlocked `Vault`), so this is the + "explicit decrypted view" the Non-Goals below require, not a bypass of it. +- `Vault::add`, `Vault::get`, `Vault::rotate`, `Vault::revoke`, and + `Vault::history` call the existing ChaCha20-Poly1305 helpers directly for + both fields via `Vault::encrypt_metadata`/`Vault::decrypt_metadata`. - There is no fjall-backed signal store in current main. Mesh signals are produced in memory and forwarded through the collector/processor path. @@ -44,8 +52,10 @@ fields may remain plaintext for indexing, filtering, or redacted display. ## Non-Goals -- Do not retrofit `kryphos::Vault` only to satisfy the shape. Its existing typed - `encrypted_secret` model is clearer than a generic map until another store +- Do not retrofit `kryphos::Vault` onto the generic `ColumnCodec`/ + `ENCRYPTED_FIELDS` shape only for consistency with future stores. Its + existing typed-field model (now two fields: `encrypted_secret` and + `encrypted_metadata`) is clearer than a generic map until another store proves the abstraction. - Do not encrypt fields that are required for safe listing or lifecycle checks unless the caller has an explicit decrypted view.