Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions crates/akroasis/src/vault/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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]
Expand All @@ -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"
);
}
Expand Down
19 changes: 16 additions & 3 deletions crates/kryphos/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use figment::value::{Dict, Map, Value};
use figment::{Error, Metadata, Profile, Provider};
use zeroize::Zeroizing;

use crate::storage::Vault;

Expand Down Expand Up @@ -82,13 +83,25 @@ impl<P> VaultProvider<P> {
.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<Vec<u8>>` and has
// no public escape hatch to a bare `Vec<u8>`/`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))
}
Expand Down
47 changes: 41 additions & 6 deletions crates/kryphos/src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -44,11 +44,7 @@ impl SigningKey {
///
/// Returns [`KeyError::WrongKeyLength`] if `bytes` is not 32 bytes.
pub fn from_bytes(bytes: &[u8]) -> Result<Self, KeyError> {
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),
})
Expand Down Expand Up @@ -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<Zeroizing<[u8; SIGNING_KEY_LEN]>, 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])")
Expand Down Expand Up @@ -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: ZeroizeOnDrop>(_: &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();
Expand Down
Loading
Loading