From 2ff52de737530881f82da9d26e20d992e46a1a3e Mon Sep 17 00:00:00 2001 From: Andrew Briscoe Date: Tue, 15 Sep 2026 22:52:40 -0600 Subject: [PATCH 1/3] fix(crypto): redact key debug formatting --- src/crypto/keys.rs | 111 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 109 insertions(+), 2 deletions(-) diff --git a/src/crypto/keys.rs b/src/crypto/keys.rs index 559035d..299d402 100644 --- a/src/crypto/keys.rs +++ b/src/crypto/keys.rs @@ -1,8 +1,33 @@ //! Zeroizing key types. KEK, MK, and derived 256-bit symmetric keys all use //! a single shared inner representation; the wrapper type signals intent. +//! +//! # Formatting +//! +//! Every type here formats as a redacted placeholder and the byte array is +//! private, so there is no safe-looking way to spell a key into a log line. +//! +//! Both halves are load bearing. `Zeroizing` derives `Debug` and forwards to +//! the inner `T`, so a `pub(crate)` field was enough for +//! `format!("{key.0:?}")` to print all 32 bytes — a wrapper's own `Debug` does +//! not help if callers can reach past it. Keeping the field private removes +//! that spelling; the manual `Debug` implementations make the ordinary +//! `{key:?}` spelling inert rather than a compile error someone works around. + +use std::fmt; use zeroize::Zeroizing; +/// Formats a key as a redacted placeholder, never as bytes. +macro_rules! redacted_debug { + ($type:ty) => { + impl fmt::Debug for $type { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(concat!(stringify!($type), "()")) + } + } + }; +} + /// 256-bit key-encryption key supplied by the embedder. /// /// The bytes are zeroized on drop and intentionally never exposed outside this @@ -21,9 +46,11 @@ impl From<[u8; 32]> for SecretKey { } } +redacted_debug!(SecretKey); + /// 256-bit master key derived from the embedder-supplied KEK and the per-DB /// `kek_salt` / `mk_epoch`. Held in memory only; zeroized on drop. -pub struct MasterKey(pub(crate) Zeroizing<[u8; 32]>); +pub struct MasterKey(Zeroizing<[u8; 32]>); impl MasterKey { pub(crate) fn from_bytes(bytes: [u8; 32]) -> Self { @@ -40,9 +67,11 @@ impl Clone for MasterKey { } } +redacted_debug!(MasterKey); + /// 256-bit derived key: realm DEK (AEAD modes), Integrity Key (plaintext+MAC), /// or Header Key. Zeroized on drop. -pub struct DerivedKey(pub(crate) Zeroizing<[u8; 32]>); +pub struct DerivedKey(Zeroizing<[u8; 32]>); impl DerivedKey { pub(crate) fn from_bytes(bytes: [u8; 32]) -> Self { @@ -58,3 +87,81 @@ impl Clone for DerivedKey { Self(Zeroizing::new(*self.0)) } } + +redacted_debug!(DerivedKey); + +#[cfg(test)] +mod tests { + use super::*; + + /// Every byte is distinct, so a leak in any position is caught rather than + /// only a leak of the first byte. + fn distinctive_bytes() -> [u8; 32] { + #[allow(clippy::cast_possible_truncation)] + std::array::from_fn(|index| (index as u8).wrapping_mul(7).wrapping_add(3)) // index < 32 + } + + /// A key must not spell its bytes when formatted. + /// + /// Check every byte in both the decimal form an array's `Debug` uses and + /// the hexadecimal form a hand-written formatter might use. + fn assert_redacted(rendered: &str, label: &str) { + let bytes = distinctive_bytes(); + assert!( + rendered.contains(""), + "{label} must render a redaction marker, got {rendered:?}" + ); + for (index, byte) in bytes.iter().enumerate() { + let decimal = byte.to_string(); + let hex = format!("{byte:02x}"); + assert!( + !rendered.contains(&decimal), + "{label} leaked byte {index} ({byte}) in decimal: {rendered:?}" + ); + assert!( + !rendered.contains(&hex), + "{label} leaked byte {index} ({byte}) in hex: {rendered:?}" + ); + } + } + + #[test] + fn a_secret_key_does_not_format_its_bytes() { + let key = SecretKey::from(distinctive_bytes()); + assert_redacted(&format!("{key:?}"), "SecretKey"); + } + + #[test] + fn a_master_key_does_not_format_its_bytes() { + let key = MasterKey::from_bytes(distinctive_bytes()); + assert_redacted(&format!("{key:?}"), "MasterKey"); + } + + #[test] + fn a_derived_key_does_not_format_its_bytes() { + let key = DerivedKey::from_bytes(distinctive_bytes()); + assert_redacted(&format!("{key:?}"), "DerivedKey"); + } + + #[test] + fn alternate_debug_is_redacted() { + let key = MasterKey::from_bytes(distinctive_bytes()); + assert_redacted(&format!("{key:#?}"), "MasterKey alternate"); + } + + #[test] + fn nested_debug_is_redacted() { + #[derive(Debug)] + #[allow(dead_code)] + struct Envelope { + epoch: u64, + key: DerivedKey, + } + + let envelope = Envelope { + epoch: 7, + key: DerivedKey::from_bytes(distinctive_bytes()), + }; + assert_redacted(&format!("{envelope:?}"), "nested DerivedKey"); + } +} From d6c5169d24d4e5eaf58a691257af17bf04597583 Mon Sep 17 00:00:00 2001 From: Andrew Briscoe Date: Thu, 17 Sep 2026 07:23:26 -0600 Subject: [PATCH 2/3] docs(crypto): bound key formatting hardening claims --- src/crypto/keys.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/crypto/keys.rs b/src/crypto/keys.rs index 299d402..0afd201 100644 --- a/src/crypto/keys.rs +++ b/src/crypto/keys.rs @@ -3,8 +3,10 @@ //! //! # Formatting //! -//! Every type here formats as a redacted placeholder and the byte array is -//! private, so there is no safe-looking way to spell a key into a log line. +//! Every wrapper formats as a redacted placeholder and its tuple field is +//! private, preventing accidental wrapper or direct-field debug disclosure. +//! Internal cryptographic code can still explicitly access the bytes through +//! `as_bytes`; that material must not be logged. //! //! Both halves are load bearing. `Zeroizing` derives `Debug` and forwards to //! the inner `T`, so a `pub(crate)` field was enough for From e3d7209a34ec152de6f74b2eca7a636c1949576c Mon Sep 17 00:00:00 2001 From: Farhan Syah Date: Sun, 27 Sep 2026 19:14:05 +0800 Subject: [PATCH 3/3] docs(crypto): fix invalid format! example syntax The doc comment used field access inside a format string (format!("{key.0:?}")), which is not valid Rust. Use the positional-argument form instead. --- src/crypto/keys.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crypto/keys.rs b/src/crypto/keys.rs index 0afd201..13b20bc 100644 --- a/src/crypto/keys.rs +++ b/src/crypto/keys.rs @@ -10,7 +10,7 @@ //! //! Both halves are load bearing. `Zeroizing` derives `Debug` and forwards to //! the inner `T`, so a `pub(crate)` field was enough for -//! `format!("{key.0:?}")` to print all 32 bytes — a wrapper's own `Debug` does +//! `format!("{:?}", key.0)` to print all 32 bytes — a wrapper's own `Debug` does //! not help if callers can reach past it. Keeping the field private removes //! that spelling; the manual `Debug` implementations make the ordinary //! `{key:?}` spelling inert rather than a compile error someone works around.