From 41f849255da1a41105971aa02885697dfbe829e3 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Tue, 27 Oct 2026 16:50:26 +1100 Subject: [PATCH 01/25] perf(aead-frame): refuse a repeated key as the storage writer writes, so no blob is read back before it is returned The storage writer read every blob back before returning it, to find a repeated key as a reader would. That pass rebuilt a HashSet for every object and hashed every field name, and was 55% of a 1,000-record write. An object now compares each field name with the ones it has written, held on the stack and spilling to the heap past 16, since the names are the type's own and never data. A map records where each key lies in the blob and checks them for a repeat when it ends, as before, without parsing the rest. A debug build still reads the blob back, so the tests keep checking that the writer and reader agree. On an M4 on mains power this made a 1,000-record write 47% faster, one record 41% and a map of 1,000 keys 29%, and left reads unchanged. --- packages/aead-frame/README.md | 2 +- packages/aead-frame/benches/storage.rs | 7 +- packages/aead-frame/src/storage.rs | 147 ++++++++++++++++++------- 3 files changed, 113 insertions(+), 43 deletions(-) diff --git a/packages/aead-frame/README.md b/packages/aead-frame/README.md index 07521d27..624f1b87 100644 --- a/packages/aead-frame/README.md +++ b/packages/aead-frame/README.md @@ -176,7 +176,7 @@ What the numbers show: - Reading text or bytes costs little more than copying them out: 64 KiB of bytes reads at about 32 GiB/s, as fast as MessagePack. - On values with many small parts, the storage profile is the slowest of the five. It writes 1,000 records 6.7 times as slowly as MessagePack and 31 times as slowly as Borsh, and reads them 3.2 and 6.4 times as slowly. MessagePack carries the same field names, so the names are not the cost; each element or field costs the storage writer about 40 to 90 ns more. - The storage profile is also the largest: 1,000 records take 57% more bytes than in MessagePack, and the map 89% more. Every length and count is a fixed four bytes and every value has a kind byte, where MessagePack packs small lengths and integers into one byte. Borsh is the smallest for records, but writes an integer at its full width, so the map of `u64` values is nearly as large as in the storage profile. -- A storage write costs more than a transport write on values with many parts, 26% more for 1,000 records and 43% more for the map, because it reads the blob back to find a repeated key before it returns it. Storage and transport reads are within about 12% of each other. +- The tables predate a change to the storage writer. It used to read the whole blob back before returning it, to find a repeated key, and that pass was 55% of a 1,000-record write. It now refuses a repeated field name as it writes, comparing the names an object has written on the stack, and checks a map's keys where they lie in the blob when the map ends. Measured against the tables' code on the same machine, that made a 1,000-record write 47% faster, a single record 41% faster and the map 28% faster, and left reads unchanged. The tables will be re-measured before this is released. ## Cargo features diff --git a/packages/aead-frame/benches/storage.rs b/packages/aead-frame/benches/storage.rs index 12873322..a02907fd 100644 --- a/packages/aead-frame/benches/storage.rs +++ b/packages/aead-frame/benches/storage.rs @@ -10,10 +10,9 @@ //! of the bytes each format writes, so the rates compare work per byte and //! the times compare work per value. //! -//! A storage write reads the blob back before it returns it, to find a -//! repeated key as a reader would. A write and both `to_vec`s wipe their -//! buffers as they grow and when they are dropped. Both costs are part of -//! what is measured. MessagePack, Borsh and JSON wipe nothing. +//! A write and both `to_vec`s wipe their buffers as they grow and when +//! they are dropped. That cost is part of what is measured. MessagePack, +//! Borsh and JSON wipe nothing. //! //! Run with `cargo bench -p vitaminc-aead-frame --features serde`. diff --git a/packages/aead-frame/src/storage.rs b/packages/aead-frame/src/storage.rs index b58c4de3..5738b92c 100644 --- a/packages/aead-frame/src/storage.rs +++ b/packages/aead-frame/src/storage.rs @@ -37,6 +37,7 @@ use std::collections::HashSet; use std::fmt; +use std::ops::Range; use ::serde::ser::{self, Serialize}; use ::serde::Deserialize; @@ -357,7 +358,7 @@ fn node(c: &mut Cursor<'_>, depth: usize) -> Result<(), Invalid> { keys.push(c.map_key()?); node(c, depth + 1)?; } - distinct(keys)?; + distinct(keys.into_iter())?; } SOME => { c.pos += 1; @@ -404,7 +405,7 @@ impl<'a> MapKey<'a> { /// sort. An integer's identity is its value, a copy of the key, so those /// are kept in a buffer sized up front, which never regrows and is wiped /// when dropped. Any other key is compared where it lies in the blob. -fn distinct(keys: Vec>) -> Result<(), Invalid> { +fn distinct<'a>(keys: impl ExactSizeIterator>) -> Result<(), Invalid> { let mut integers: Zeroizing> = Zeroizing::new(Vec::with_capacity(keys.len())); let mut nodes = Vec::new(); for key in keys { @@ -436,9 +437,9 @@ fn distinct(keys: Vec>) -> Result<(), Invalid> { /// /// The blob is the value's plaintext, so it is returned [`Protected`], and /// wiped when it is dropped. The buffer grows without leaving copies of what -/// it held behind, and is wiped if writing fails part way. What is written -/// is validated as a reader would before it is returned, so a blob this -/// returns always reads. +/// it held behind, and is wiped if writing fails part way. The writer +/// checks each rule a reader checks as it writes, so a blob this returns +/// always reads; a debug build reads it back to make sure. /// /// # Errors /// @@ -458,26 +459,17 @@ pub fn to_vec(value: &T) -> Result>, Er depth: 0, }) .map_err(unwritable)?; - // The writer checks depth, lengths and counts as it goes. A repeated - // key is found here, as a reader would find it, so the writer keeps no - // copy of the keys it wrote. - validate(&out[HEADER.len()..]).map_err(refused_own)?; + // The writer checks depth, lengths, counts and repeated keys as it + // goes, so reading the blob back finds nothing. A debug build does so + // anyway, since a blob that disagrees with the reader is a bug. + debug_assert_eq!( + validate(&out[HEADER.len()..]), + Ok(()), + "the writer wrote a blob the reader refuses" + ); Ok(Protected::new(core::mem::take(&mut *out))) } -/// What [`to_vec`] says of a blob it wrote but would not read. -fn refused_own(invalid: Invalid) -> Error { - match invalid { - Invalid::DuplicateKey => unwritable(written(FrameWriteError::DuplicateKey)), - // The writer checks everything else as it writes, so this is a bug - // in it, reported rather than returned as a blob no reader reads. - Invalid::Malformed => Error::of( - ErrorKind::Unwritable, - "the writer wrote a malformed storage blob", - ), - } -} - fn write_len(out: &mut Vec, len: usize) -> Result<(), transport::Error> { let len = u32::try_from(len).map_err(|_| written(FrameWriteError::TooLong))?; out.extend_from_slice(&len.to_le_bytes()); @@ -567,6 +559,7 @@ impl<'o> Writer<'o> { n: 0, expected, variant, + fields: FieldNames::default(), }) } @@ -592,6 +585,42 @@ impl<'o> Writer<'o> { } } +/// How many of an object's field names are kept on the stack before the +/// rest go to the heap. Few types have more fields than this. +const INLINE_FIELDS: usize = 16; + +/// The field names an object has written, to refuse a repeat. A derived +/// `Serialize` never repeats a name, but a hand-written one can. The names +/// are the type's own, never data, so keeping them copies nothing secret. +/// They are compared one by one: an object has too few for a hash set to +/// pay for its allocation and hashing. +#[derive(Default)] +struct FieldNames { + inline: [&'static str; INLINE_FIELDS], + len: usize, + spilled: Vec<&'static str>, +} + +impl FieldNames { + /// Record `name`; whether it was new. + fn insert(&mut self, name: &'static str) -> bool { + let seen = self.inline[..self.len.min(INLINE_FIELDS)] + .iter() + .chain(&self.spilled) + .any(|&known| known == name); + if seen { + return false; + } + if self.len < INLINE_FIELDS { + self.inline[self.len] = name; + } else { + self.spilled.push(name); + } + self.len += 1; + true + } +} + /// An array, an object or a map being written. struct Container<'o> { out: &'o mut Vec, @@ -601,6 +630,8 @@ struct Container<'o> { n: usize, expected: Option, variant: Option<&'static str>, + /// An object's field names so far; an array writes none. + fields: FieldNames, } impl Container<'_> { @@ -630,12 +661,15 @@ impl Container<'_> { } /// An object's entry: a field or variant name the type declares, then - /// its value. + /// its value. A name the object has already written is refused. fn field( &mut self, key: &'static str, value: &T, ) -> Result<(), transport::Error> { + if !self.fields.insert(key) { + return Err(written(FrameWriteError::DuplicateKey)); + } write_key(self.out, key)?; self.child(value, || PathSegment::Key(key.to_owned())) } @@ -651,11 +685,14 @@ impl Container<'_> { } /// A map being written. Each key is written as a node the moment it is -/// given, so no copy of it is kept. +/// given, so no copy of it is kept: only where it lies in the blob, so +/// that a repeat is found among the keys when the map ends. struct MapWriter<'o> { c: Container<'o>, /// Whether a key has been written whose value is next. pending: bool, + /// Where each key's node lies in the blob. + keys: Vec>, } impl<'o> ser::Serializer for Writer<'o> { @@ -788,6 +825,7 @@ impl<'o> ser::Serializer for Writer<'o> { Ok(MapWriter { c: self.container(MAP, len, None)?, pending: false, + keys: Vec::with_capacity(len.unwrap_or(0)), }) } fn serialize_struct( @@ -914,8 +952,10 @@ impl ser::SerializeMap for MapWriter<'_> { )); } let index = self.c.n; + let start = self.c.out.len(); key.serialize(KeyWriter(&mut *self.c.out)) .map_err(|e| e.at(PathSegment::Entry(index)))?; + self.keys.push(start..self.c.out.len()); self.pending = true; Ok(()) } @@ -938,6 +978,16 @@ impl ser::SerializeMap for MapWriter<'_> { "a map key was written without its value", )); } + // The blob only grows while the map is written, so every range + // still holds its key. + let out: &[u8] = self.c.out; + match distinct(self.keys.iter().map(|range| MapKey(&out[range.clone()]))) { + Ok(()) => {} + Err(Invalid::DuplicateKey) => return Err(written(FrameWriteError::DuplicateKey)), + // Every key was written by `KeyWriter`, which writes only the + // kinds a key may be. + Err(Invalid::Malformed) => unreachable!("a map key the writer wrote reads"), + } self.c.end() } } @@ -1134,18 +1184,34 @@ mod tests { assert!(full.capacity() >= 7); } - /// The two refusals no blob can reach: they mean the reader and the - /// validator, or the writer and the validator, disagree. + /// The refusal no blob can reach: it means the reader and the + /// validator disagree. #[test] fn a_bug_is_reported_as_one() { let error = mismatch(transport::Error::new(transport::MALFORMED)); assert_eq!(error, malformed()); - let error = refused_own(Invalid::Malformed); - assert_eq!(error.kind(), ErrorKind::Unwritable); - assert_eq!(error.message(), "the writer wrote a malformed storage blob"); - let error = refused_own(Invalid::DuplicateKey); - assert_eq!(error.kind(), ErrorKind::Unwritable); - assert_eq!(error.message(), "duplicate key"); + } + + /// Field names are kept on the stack until an object has more than + /// fit there, and a repeat is found on either side of that line. + #[test] + fn field_names_spill_past_the_inline_ones() { + const NAMES: [&str; INLINE_FIELDS + 2] = [ + "a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", + "r", + ]; + let mut fields = FieldNames::default(); + for name in NAMES { + assert!(fields.insert(name), "{name} is new"); + } + assert_eq!( + fields.spilled, + ["q", "r"], + "the names past the inline ones spill" + ); + assert!(!fields.insert("a"), "an inline name repeats"); + assert!(!fields.insert("r"), "a spilled name repeats"); + assert!(fields.insert("s"), "a new name is still new"); } /// Map keys are compared by value across integer widths, and by node @@ -1156,13 +1222,18 @@ mod tests { assert!(key(&[tags::UINT8, 1]) == key(&[tags::INT64, 1, 0, 0, 0, 0, 0, 0, 0])); assert!(key(&[tags::INT8, 0xFF]) != key(&[tags::UINT8, 0xFF])); assert!(key(&[tags::STRING, 1, 0, 0, 0, b'a']) != key(&[tags::BYTES, 1, 0, 0, 0, b'a'])); - assert!(distinct(vec![MapKey(&[tags::UINT8, 2]), MapKey(&[tags::UINT8, 1])]).is_ok()); + assert!( + distinct([MapKey(&[tags::UINT8, 2]), MapKey(&[tags::UINT8, 1])].into_iter()).is_ok() + ); assert_eq!( - distinct(vec![ - MapKey(&[tags::UINT8, 1]), - MapKey(&[tags::UINT8, 2]), - MapKey(&[tags::UINT16, 1, 0]) - ]), + distinct( + [ + MapKey(&[tags::UINT8, 1]), + MapKey(&[tags::UINT8, 2]), + MapKey(&[tags::UINT16, 1, 0]) + ] + .into_iter() + ), Err(Invalid::DuplicateKey) ); } From 0ad15fa9eeb2f545bb458c28443453bde00c93c1 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Tue, 27 Oct 2026 18:02:07 +1100 Subject: [PATCH 02/25] perf(protected): wipe sixteen bytes a store, so a growing frame or blob is not wiped a byte at a time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit zeroize stores one byte per volatile write, so wiping a 64 KiB buffer took 15 µs, and a frame writer that regrows its buffer paid that for every old allocation it wiped. wipe_raw, which the locked regions use, now stores a u128 at a time between a byte-wise head and tail, and falls back to bytes when the pointer cannot be aligned. The stores stay volatile, so the guarantee is unchanged: only the width is. A safe wipe_vec wraps it for a Vec's whole allocation, and both frame writers wipe their outgrown buffers with it. The Kani harness is widened to cover the word run, and two tests pin the bounds from every alignment; both pass under Miri's default and strict models. On an M4 on mains this took 17% off a 1,000-record storage write and 10% off the transport write. The drop of the returned buffer still goes through zeroize, which the next change addresses. --- packages/aead-frame/src/serde.rs | 4 +- packages/aead/src/frame.rs | 3 +- packages/protected/src/lib.rs | 2 +- packages/protected/src/locked/mod.rs | 104 ++++++++++++++++++++++-- packages/protected/src/locked/proofs.rs | 10 ++- 5 files changed, 109 insertions(+), 14 deletions(-) diff --git a/packages/aead-frame/src/serde.rs b/packages/aead-frame/src/serde.rs index fa00505a..cf09bf35 100644 --- a/packages/aead-frame/src/serde.rs +++ b/packages/aead-frame/src/serde.rs @@ -131,7 +131,7 @@ use vitaminc_aead::leaf::{self, Form}; use vitaminc_aead::plaintext::{Float, Int, Logical, MapSink, Physical, SeqSink, Sink}; use vitaminc_aead::{tags, Encrypt, Frame, FrameWriter}; use vitaminc_protected::Protected; -use zeroize::{Zeroize, Zeroizing}; +use zeroize::Zeroizing; // ============================================================================= // Errors @@ -1114,7 +1114,7 @@ pub(crate) fn reserve(out: &mut Vec, additional: usize) { .max(64); let mut grown = Vec::with_capacity(capacity); grown.extend_from_slice(out); - out.zeroize(); + vitaminc_protected::wipe_vec(out); *out = grown; } diff --git a/packages/aead/src/frame.rs b/packages/aead/src/frame.rs index 1bd02711..df1e11b9 100644 --- a/packages/aead/src/frame.rs +++ b/packages/aead/src/frame.rs @@ -20,7 +20,6 @@ use std::collections::HashSet; use std::ops::Range; use vitaminc_protected::{Controlled, OpaqueDebug, Protected, TimingSafeEq}; -use zeroize::Zeroize; use crate::leaf::{self, Form, Item, ScalarError}; use crate::plaintext::{ @@ -421,7 +420,7 @@ pub(crate) fn reserve(out: &mut Vec, additional: usize) { .max(64); let mut grown = Vec::with_capacity(capacity); grown.extend_from_slice(out); - out.zeroize(); + vitaminc_protected::wipe_vec(out); *out = grown; } diff --git a/packages/protected/src/lib.rs b/packages/protected/src/lib.rs index 602a838c..78d7fb76 100644 --- a/packages/protected/src/lib.rs +++ b/packages/protected/src/lib.rs @@ -55,7 +55,7 @@ pub use digest::ProtectedDigest; pub use ::digest::InvalidLength; pub use equatable::{ConstantTimeEq, Equatable}; pub use exportable::{Exportable, SafeDeserialize, SafeSerialize}; -pub use locked::{LockError, LockPolicy, LockPolicyError, Locked}; +pub use locked::{wipe_vec, LockError, LockPolicy, LockPolicyError, Locked}; /// The name `MaybeEmpty` had before it was renamed: it read as a marker /// ("values of this type are empty") when it is a capability ("this value /// can be asked whether it is empty"). Bounds and impls written against diff --git a/packages/protected/src/locked/mod.rs b/packages/protected/src/locked/mod.rs index c5e886b5..bc0befdc 100644 --- a/packages/protected/src/locked/mod.rs +++ b/packages/protected/src/locked/mod.rs @@ -651,22 +651,71 @@ impl Locked { } } +/// The width of one store in [`wipe_raw`]'s middle run: a `u128`, which +/// every target stores in one or two instructions. +const WIPE_WORD: usize = mem::size_of::(); + /// Overwrite `len` bytes at `ptr` with volatile zero stores and a compiler /// fence, so the wipe cannot be elided as a dead store. The bytes are -/// written as `MaybeUninit`, never read, so storage that holds padding -/// or a dropped value is fine. +/// written as `MaybeUninit`, never read, so storage that holds padding or +/// a dropped value is fine. +/// +/// Bytes up to the first [`WIPE_WORD`] boundary and after the last are +/// stored one at a time; those between are stored a word at a time, so a +/// large buffer is wiped at close to memory speed rather than a byte a +/// store. If the pointer cannot be brought to a word boundary, which +/// `align_offset` reports as `usize::MAX`, every byte is stored on its own. /// /// # Safety /// /// `ptr` must be valid for writes of `len` bytes. pub(super) unsafe fn wipe_raw(ptr: *mut u8, len: usize) { - let bytes = core::slice::from_raw_parts_mut(ptr.cast::>(), len); - for b in bytes { - ptr::write_volatile(b, MaybeUninit::new(0)); + let head = ptr.align_offset(WIPE_WORD).min(len); + let words = (len - head) / WIPE_WORD; + let tail = len - head - words * WIPE_WORD; + // SAFETY: each store below lies within the `len` bytes the caller + // vouches for: `head` bytes from `ptr`, then `words` whole words from + // `ptr + head`, which `align_offset` made word-aligned when `head` is + // less than `len`, then `tail` bytes after those. + unsafe { + for i in 0..head { + ptr::write_volatile(ptr.add(i).cast::>(), MaybeUninit::new(0)); + } + let body = ptr.add(head).cast::>(); + for i in 0..words { + ptr::write_volatile(body.add(i), MaybeUninit::new(0)); + } + let rest = ptr.add(head + words * WIPE_WORD); + for i in 0..tail { + ptr::write_volatile(rest.add(i).cast::>(), MaybeUninit::new(0)); + } } core::sync::atomic::compiler_fence(Ordering::SeqCst); } +/// Wipe every byte `out` holds or has room for, and leave it empty. +/// +/// This does what `out.zeroize()` does, but with sixteen bytes in each +/// volatile store rather than one, so a buffer of secret bytes that is +/// grown or dropped often (a frame, a blob) is wiped at close to memory +/// speed. The allocation is kept; only its contents go. +/// +/// # Examples +/// +/// ``` +/// let mut buffer = Vec::with_capacity(64); +/// buffer.extend_from_slice(b"a secret that must not outlive its use"); +/// vitaminc_protected::wipe_vec(&mut buffer); +/// assert!(buffer.is_empty()); +/// assert!(buffer.capacity() >= 64); +/// ``` +pub fn wipe_vec(out: &mut Vec) { + out.clear(); + // SAFETY: a `Vec`'s pointer is valid for writes of `capacity()` bytes, + // and after `clear` none of them is a live element. + unsafe { wipe_raw(out.as_mut_ptr(), out.capacity()) }; +} + /// [`wipe_raw`] over the bytes of a `T`, without any semantic effect on /// `T`. For a moved-from slot only. /// @@ -745,6 +794,51 @@ impl<'a, T: Zeroize> AsProtectedRef<'a, T> for Locked { mod tests { use super::*; + /// Every byte of the allocation is wiped, held or spare, whatever the + /// buffer's length: a byte-wise head and tail around whole words, or + /// no words at all. + #[test] + fn wipe_vec_wipes_the_whole_allocation_and_empties_it() { + for len in [0usize, 1, 15, 16, 17, 31, 33, 64, 100] { + let mut out: Vec = vec![0xFF; len]; + out.reserve(37); + let capacity = out.capacity(); + let ptr = out.as_ptr(); + // Fill the spare room too, so a wipe that stops at `len` shows. + out.resize(capacity, 0xEE); + out.truncate(len); + wipe_vec(&mut out); + assert!(out.is_empty(), "wiped buffer of {len} is empty"); + assert_eq!(out.capacity(), capacity, "the allocation is kept"); + assert!(std::ptr::eq(out.as_ptr(), ptr), "the allocation is kept"); + let spare = out.spare_capacity_mut(); + // SAFETY: `wipe_vec` stored zero to every one of these bytes. + let bytes = + unsafe { std::slice::from_raw_parts(spare.as_ptr().cast::(), spare.len()) }; + assert!( + bytes.iter().all(|&b| b == 0), + "every byte of the {capacity}-byte allocation is zero after wiping {len}" + ); + } + } + + /// A range that starts off a word boundary is wiped exactly, and the + /// bytes either side of it are left alone. + #[test] + fn wipe_raw_stops_at_both_ends_from_any_alignment() { + for start in 0..17usize { + for len in [0usize, 1, 7, 16, 17, 40] { + let mut buf = [0xA5u8; 80]; + // SAFETY: `start + len <= 57 < 80`, inside `buf`. + unsafe { wipe_raw(buf.as_mut_ptr().add(start), len) }; + for (i, &b) in buf.iter().enumerate() { + let inside = (start..start + len).contains(&i); + assert_eq!(b == 0, inside, "byte {i} for start {start}, len {len}"); + } + } + } + } + #[test] fn a_value_moved_in_reads_back() { let key = Locked::new([0xA5u8; 32]).unwrap(); diff --git a/packages/protected/src/locked/proofs.rs b/packages/protected/src/locked/proofs.rs index caada449..f29f436b 100644 --- a/packages/protected/src/locked/proofs.rs +++ b/packages/protected/src/locked/proofs.rs @@ -64,14 +64,16 @@ fn the_value_is_inside_the_interior_aligned_and_against_the_end() { #[kani::proof] #[kani::unwind(33)] fn wipe_raw_zeroes_exactly_the_bytes_it_is_given() { - let mut buf: [u8; 32] = kani::any(); + // Wide enough to hold a byte-wise head, whole words and a tail, + // whatever the buffer's alignment. + let mut buf: [u8; 64] = kani::any(); let len: usize = kani::any(); - kani::assume(len <= 24); + kani::assume(len <= 56); let before = buf; - // SAFETY: `len <= 24 < 32`, so the range is inside `buf`. + // SAFETY: `len <= 56 < 64`, so the range is inside `buf`. unsafe { wipe_raw(buf.as_mut_ptr(), len) }; let mut i = 0; - while i < 32 { + while i < 64 { if i < len { assert!(buf[i] == 0); } else { From 6c6880abcb1a0cd0d4aac95e30943d66e60042c5 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Tue, 27 Oct 2026 19:13:29 +1100 Subject: [PATCH 03/25] perf(protected)!: return frames and blobs in a Buffer, so the plaintext they hold is wiped sixteen bytes a store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A frame or blob came back as Protected>, and the drop of a Vec goes through zeroize, which stores one byte per volatile write: 15 µs for a 64 KiB value, and a quarter of a 1,000-record write. That wipe cannot be changed from here without specialisation, so the type is. Buffer, in vitaminc-protected, is a Vec that grows only through push, extend_from_slice and reserve, which copy into a fresh allocation and wipe the old one, and whose zeroize and drop wipe it with wipe_vec. It derefs to a slice for reading and patching, has no Clone, PartialEq or Debug since its bytes are secret, and risky_into_vec is the one way out. to_frame, decrypt_to_frame and both to_vecs return Protected and their writers hold &mut Buffer, so a writer cannot grow its buffer unwiped; the two private reserve copies are gone. leaf::write takes either buffer through an Out trait, so the sealed-leaf path is unchanged, and a passthrough frame converts back to a Vec at its boundary since it crosses in the clear. BREAKING: the four writers return Protected, not Protected>; FrameWriter and leaf::write take a Buffer; a caller that borrowed a slice is unchanged, one that took the Vec calls risky_into_vec. Two findings shaped the hot path. Buffer's methods are #[inline], as Vec's are, or structured writes lose up to 24% to a call per byte; but leaf::write stays out of line, since inlined into the storage writer it cost 10 to 15%. And distinct's scratch for integer keys, presized to the key count, is a Buffer too: as a Zeroizing> it wiped 17 KB of untouched spare capacity a byte at a time for a map of 1,000 text keys, 8 µs of a 38 µs write. On an M4 on mains, against the previous commit: a 1,000-record storage write 205 to 153 µs, a map of 1,000 keys 38.1 to 31.9 µs, one record 0.263 to 0.208 µs, and a 64 KiB value 17 to 1.9 µs. Transport writes gained 10 to 17%. Reads are unchanged. --- packages/aead-frame/README.md | 2 +- packages/aead-frame/src/serde.rs | 62 ++--- packages/aead-frame/src/storage.rs | 81 +++---- packages/aead-frame/tests/conformance.rs | 2 +- packages/aead-frame/tests/legacy_fixture.rs | 2 +- packages/aead-frame/tests/serde.rs | 20 +- packages/aead-frame/tests/storage.rs | 2 +- packages/aead-frame/tests/storage_wipe.rs | 21 +- packages/aead-napi/src/ciphertext.rs | 4 +- packages/aead-napi/src/js_ref.rs | 4 +- packages/aead-napi/tests/addon/src/lib.rs | 4 +- packages/aead/README.md | 6 +- packages/aead/src/frame.rs | 80 ++----- packages/aead/src/leaf.rs | 40 +++- packages/aead/src/shape.rs | 16 +- packages/aead/src/timestamp.rs | 2 +- packages/aead/src/walk.rs | 15 +- packages/aead/tests/typed_null.rs | 7 +- packages/protected/src/buffer.rs | 249 ++++++++++++++++++++ packages/protected/src/lib.rs | 2 + 20 files changed, 427 insertions(+), 194 deletions(-) create mode 100644 packages/protected/src/buffer.rs diff --git a/packages/aead-frame/README.md b/packages/aead-frame/README.md index 624f1b87..a91841f5 100644 --- a/packages/aead-frame/README.md +++ b/packages/aead-frame/README.md @@ -111,7 +111,7 @@ use vitaminc_aead_frame::storage; use vitaminc_protected::Controlled; let blob = storage::to_vec(&Some(Option::::None)).expect("writes"); -assert_eq!(blob.risky_ref(), &[0x56, 0x43, 0x53, 0x01, 0xF5, 0x00]); +assert_eq!(blob.risky_ref().as_slice(), &[0x56, 0x43, 0x53, 0x01, 0xF5, 0x00]); let back: Option> = storage::from_slice(blob.risky_ref()).expect("reads"); assert_eq!(back, Some(None)); # } diff --git a/packages/aead-frame/src/serde.rs b/packages/aead-frame/src/serde.rs index cf09bf35..a7519f1a 100644 --- a/packages/aead-frame/src/serde.rs +++ b/packages/aead-frame/src/serde.rs @@ -130,7 +130,7 @@ use vitaminc_aead::frame::{FrameWriteError, ARRAY, MAX_DEPTH, OBJECT, PASSTHROUG use vitaminc_aead::leaf::{self, Form}; use vitaminc_aead::plaintext::{Float, Int, Logical, MapSink, Physical, SeqSink, Sink}; use vitaminc_aead::{tags, Encrypt, Frame, FrameWriter}; -use vitaminc_protected::Protected; +use vitaminc_protected::{Buffer, Protected}; use zeroize::Zeroizing; // ============================================================================= @@ -1058,11 +1058,11 @@ impl<'de> VariantAccess<'de> for BareVariant<'de> { /// /// What a host and Rust exchange this way is not plaintext to seal, but may /// still be secret (a credential in a configuration, say). So the frame is -/// returned [`Protected`], and wiped when it is dropped. The buffer grows -/// without leaving copies of what it held behind, as every frame vitaminc -/// writes does, and is wiped if writing fails part way. A value written -/// through `Display` (`collect_str`) is formatted into a buffer that is -/// wiped too. +/// returned as a [`Buffer`] under [`Protected`], and wiped when it is +/// dropped. The buffer grows without leaving copies of what it held +/// behind, as every frame vitaminc writes does, and is wiped if writing +/// fails part way. A value written through `Display` (`collect_str`) is +/// formatted into a buffer that is wiped too. /// /// # Errors /// @@ -1077,17 +1077,17 @@ impl<'de> VariantAccess<'de> for BareVariant<'de> { /// refused ("container length does not match its children"). `serde_json` /// ignores a wrong length, so a hand-written impl that works there can fail /// here. Pass `None` when the count is not known up front. -pub fn to_vec(value: &T) -> Result>, Error> { - let mut out = Zeroizing::new(Vec::new()); +pub fn to_vec(value: &T) -> Result, Error> { + let mut out = Buffer::new(); value.serialize(Serializer(FrameWriter::new(&mut out)))?; - Ok(Protected::new(core::mem::take(&mut *out))) + Ok(Protected::new(out)) } /// Write `value` as a frame of its own, for a node `depth` levels below /// the top of a document. The buffer is wiped when it is dropped, whether /// or not it was copied into the document. -fn to_part(value: &T, depth: usize) -> Result>, Error> { - let mut out = Zeroizing::new(Vec::new()); +fn to_part(value: &T, depth: usize) -> Result { + let mut out = Buffer::new(); value.serialize(Serializer(FrameWriter::at_depth(&mut out, depth)))?; Ok(out) } @@ -1097,43 +1097,21 @@ pub(crate) fn written(error: FrameWriteError) -> Error { } /// Writes one node into a [`FrameWriter`]. -/// Make room for `additional` more bytes. A `Vec` that regrows frees its -/// old allocation without wiping it, so grow into a fresh allocation and -/// wipe the old one instead. -/// -/// The same as `reserve` in `vitaminc_aead::frame`, which the frame writer -/// uses; that one is private to its crate. A fix to either belongs in both. -pub(crate) fn reserve(out: &mut Vec, additional: usize) { - if out.capacity() - out.len() >= additional { - return; - } - let capacity = out - .len() - .saturating_add(additional) - .max(out.len().saturating_mul(2)) - .max(64); - let mut grown = Vec::with_capacity(capacity); - grown.extend_from_slice(out); - vitaminc_protected::wipe_vec(out); - *out = grown; -} - -/// Appends formatted text to a buffer, growing it as [`reserve`] does. -pub(crate) struct Appender<'o>(pub(crate) &'o mut Vec); +/// Appends formatted text to a buffer. +pub(crate) struct Appender<'o>(pub(crate) &'o mut Buffer); impl fmt::Write for Appender<'_> { fn write_str(&mut self, s: &str) -> fmt::Result { - reserve(self.0, s.len()); self.0.extend_from_slice(s.as_bytes()); Ok(()) } } -/// `value`'s `Display`, in a buffer that grows as [`reserve`] does and is -/// wiped when it is dropped. serde's default `collect_str` formats into a -/// `String` of its own and frees it unwiped. -fn displayed(value: &T) -> Result>, Error> { - let mut out = Zeroizing::new(Vec::new()); +/// `value`'s `Display`, in a [`Buffer`], so it is wiped when it is dropped. +/// serde's default `collect_str` formats into a `String` of its own and +/// frees it unwiped. +fn displayed(value: &T) -> Result { + let mut out = Buffer::new(); fmt::write(&mut Appender(&mut out), format_args!("{value}")) .map_err(|_| ::custom(""))?; Ok(out) @@ -1443,7 +1421,7 @@ struct SerVariant<'o> { variant: &'static str, /// Each part's key (a struct variant's field; `None` in a tuple /// variant) and frame. - parts: Vec<(Option<&'static str>, Zeroizing>)>, + parts: Vec<(Option<&'static str>, Buffer)>, } impl<'o> SerVariant<'o> { @@ -1488,7 +1466,7 @@ impl<'o> SerVariant<'o> { fn write_variant( writer: FrameWriter<'_>, variant: &str, - parts: &[(Option<&'static str>, Zeroizing>)], + parts: &[(Option<&'static str>, Buffer)], is_struct: bool, ) -> Result<(), Error> { let mut outer = writer.map(Some(1)).map_err(written)?; diff --git a/packages/aead-frame/src/storage.rs b/packages/aead-frame/src/storage.rs index 5738b92c..54214fa5 100644 --- a/packages/aead-frame/src/storage.rs +++ b/packages/aead-frame/src/storage.rs @@ -45,12 +45,9 @@ use vitaminc_aead::frame::{FrameWriteError, ARRAY, MAX_DEPTH, OBJECT}; use vitaminc_aead::leaf::{self, Form, ScalarError}; use vitaminc_aead::plaintext::{Float, Int, Physical}; use vitaminc_aead::tags; -use vitaminc_protected::Protected; -use zeroize::Zeroizing; +use vitaminc_protected::{Buffer, Protected}; -use crate::serde::{ - self as transport, from_validated, reserve, written, Appender, PathSegment, Profile, -}; +use crate::serde::{self as transport, from_validated, written, Appender, PathSegment, Profile}; /// The first three bytes of every blob: `"VCS"`. pub const MAGIC: [u8; 3] = *b"VCS"; @@ -399,26 +396,30 @@ impl<'a> MapKey<'a> { } } +/// The bytes an integer key's identity takes in [`distinct`]'s scratch: +/// its sign, then its magnitude as a little-endian `u128`. +const INT_IDENTITY: usize = 1 + 16; + /// Refuse a map whose keys repeat one. /// /// Each key's identity is worked out once, not at every comparison of a /// sort. An integer's identity is its value, a copy of the key, so those -/// are kept in a buffer sized up front, which never regrows and is wiped -/// when dropped. Any other key is compared where it lies in the blob. +/// are kept in a [`Buffer`] sized up front, which never regrows and is +/// wiped when dropped. Any other key is compared where it lies in the blob. fn distinct<'a>(keys: impl ExactSizeIterator>) -> Result<(), Invalid> { - let mut integers: Zeroizing> = Zeroizing::new(Vec::with_capacity(keys.len())); + let mut integers = Buffer::with_capacity(keys.len().saturating_mul(INT_IDENTITY)); let mut nodes = Vec::new(); for key in keys { match key.identity() { Identity::Int(negative, word) => { - let mut value = [0; 17]; - value[0] = u8::from(negative); - value[1..].copy_from_slice(&word.to_le_bytes()); - integers.push(value); + integers.push(u8::from(negative)); + integers.extend_from_slice(&word.to_le_bytes()); } Identity::Node(node) => nodes.push(node), } } + let (integers, rest) = integers.as_chunks_mut::(); + debug_assert!(rest.is_empty(), "every identity is whole"); integers.sort_unstable(); nodes.sort_unstable(); if integers.windows(2).any(|pair| pair[0] == pair[1]) @@ -435,9 +436,10 @@ fn distinct<'a>(keys: impl ExactSizeIterator>) -> Result<(), I /// Write `value` as a blob: [`HEADER`], then its node. /// -/// The blob is the value's plaintext, so it is returned [`Protected`], and -/// wiped when it is dropped. The buffer grows without leaving copies of what -/// it held behind, and is wiped if writing fails part way. The writer +/// The blob is the value's plaintext, so it is returned as a [`Buffer`] +/// under [`Protected`], and wiped when it is dropped. It grows without +/// leaving copies of what it held behind, and is wiped if writing fails +/// part way. The writer /// checks each rule a reader checks as it writes, so a blob this returns /// always reads; a debug build reads it back to make sure. /// @@ -449,9 +451,8 @@ fn distinct<'a>(keys: impl ExactSizeIterator>) -> Result<(), I /// length other than the number of children it wrote, a map key written /// without its value (or a value without its key), or a value its own /// `Serialize` refused. -pub fn to_vec(value: &T) -> Result>, Error> { - let mut out = Zeroizing::new(Vec::new()); - reserve(&mut out, HEADER.len()); +pub fn to_vec(value: &T) -> Result, Error> { + let mut out = Buffer::new(); out.extend_from_slice(&HEADER); value .serialize(Writer { @@ -467,25 +468,25 @@ pub fn to_vec(value: &T) -> Result>, Er Ok(()), "the writer wrote a blob the reader refuses" ); - Ok(Protected::new(core::mem::take(&mut *out))) + Ok(Protected::new(out)) } -fn write_len(out: &mut Vec, len: usize) -> Result<(), transport::Error> { +fn write_len(out: &mut Buffer, len: usize) -> Result<(), transport::Error> { let len = u32::try_from(len).map_err(|_| written(FrameWriteError::TooLong))?; out.extend_from_slice(&len.to_le_bytes()); Ok(()) } /// An object's key: a length, then UTF-8. -fn write_key(out: &mut Vec, key: &str) -> Result<(), transport::Error> { - reserve(out, key.len().saturating_add(4)); +fn write_key(out: &mut Buffer, key: &str) -> Result<(), transport::Error> { + out.reserve(key.len().saturating_add(4)); write_len(out, key.len())?; out.extend_from_slice(key.as_bytes()); Ok(()) } -fn write_scalar(out: &mut Vec, value: Physical<'_>) -> Result<(), transport::Error> { - reserve(out, leaf::max_len(&value)); +fn write_scalar(out: &mut Buffer, value: Physical<'_>) -> Result<(), transport::Error> { + out.reserve(leaf::max_len(&value)); leaf::write(None, value, Form::Frame, out).map_err(|e| { written(match e { ScalarError::Invalid => FrameWriteError::Invalid, @@ -500,10 +501,10 @@ fn write_scalar(out: &mut Vec, value: Physical<'_>) -> Result<(), transport: /// into the blob, as serde's default would format it into a `String` of its /// own and free it unwiped. fn write_display( - out: &mut Vec, + out: &mut Buffer, value: &T, ) -> Result<(), transport::Error> { - reserve(out, 5); + out.reserve(5); out.push(tags::STRING); let at = out.len(); out.extend_from_slice(&[0; 4]); @@ -517,7 +518,7 @@ fn write_display( /// Writes one node onto the end of a blob, `depth` levels below its root. struct Writer<'o> { - out: &'o mut Vec, + out: &'o mut Buffer, depth: usize, } @@ -548,7 +549,7 @@ impl<'o> Writer<'o> { variant: Option<&'static str>, ) -> Result, transport::Error> { self.check_depth()?; - reserve(self.out, 5); + self.out.reserve(5); self.out.push(tag); let at = self.out.len(); self.out.extend_from_slice(&[0; 4]); @@ -573,7 +574,7 @@ impl<'o> Writer<'o> { len: usize, ) -> Result, transport::Error> { self.check_depth()?; - reserve(self.out, 5); + self.out.reserve(5); self.out.push(OBJECT); self.out.extend_from_slice(&1u32.to_le_bytes()); write_key(self.out, variant)?; @@ -623,7 +624,7 @@ impl FieldNames { /// An array, an object or a map being written. struct Container<'o> { - out: &'o mut Vec, + out: &'o mut Buffer, depth: usize, /// Where the count goes. at: usize, @@ -760,7 +761,6 @@ impl<'o> ser::Serializer for Writer<'o> { } fn serialize_some(self, value: &T) -> Result<(), transport::Error> { self.check_depth()?; - reserve(self.out, 1); self.out.push(SOME); value.serialize(Writer { out: self.out, @@ -998,7 +998,7 @@ fn key_refused() -> transport::Error { /// Writes a map's key node: an integer, text or bytes. A `char` and a unit /// variant's name are text, and a newtype struct is what it holds. -struct KeyWriter<'o>(&'o mut Vec); +struct KeyWriter<'o>(&'o mut Buffer); type NoKey = ser::Impossible<(), transport::Error>; @@ -1167,23 +1167,6 @@ mod tests { } } - #[test] - fn reserve_keeps_the_bytes_and_makes_room() { - let mut out = vec![1, 2, 3]; - out.shrink_to_fit(); - reserve(&mut out, 100); - assert_eq!(out, [1, 2, 3]); - assert!(out.capacity() >= 103); - let capacity = out.capacity(); - reserve(&mut out, 1); - assert_eq!(out.capacity(), capacity, "a buffer with room is left alone"); - // A full buffer grows for even a little more than its length. - let mut full = vec![1, 2, 3]; - full.shrink_to_fit(); - reserve(&mut full, 4); - assert!(full.capacity() >= 7); - } - /// The refusal no blob can reach: it means the reader and the /// validator disagree. #[test] diff --git a/packages/aead-frame/tests/conformance.rs b/packages/aead-frame/tests/conformance.rs index 8eb3fec8..b319eb1c 100644 --- a/packages/aead-frame/tests/conformance.rs +++ b/packages/aead-frame/tests/conformance.rs @@ -32,7 +32,7 @@ fn unhex(text: &str) -> Vec { } fn framed(p: P) -> Vec { - to_frame(p).expect("frames").risky_ref().clone() + to_frame(p).expect("frames").risky_ref().to_vec() } fn scalar(ty: Option, v: Physical<'_>) -> Vec { diff --git a/packages/aead-frame/tests/legacy_fixture.rs b/packages/aead-frame/tests/legacy_fixture.rs index 7ec2a61a..92ffc6c1 100644 --- a/packages/aead-frame/tests/legacy_fixture.rs +++ b/packages/aead-frame/tests/legacy_fixture.rs @@ -164,7 +164,7 @@ fn open(fixture: &Fixture, vector: &Vector, aad: &[u8]) -> Opened { node(vector), Context::from_encoded(aad), ) - .map(|frame| frame.risky_ref().clone()) + .map(|frame| frame.risky_ref().to_vec()) } fn opens_into_its_frame(fixture: &Fixture) { diff --git a/packages/aead-frame/tests/serde.rs b/packages/aead-frame/tests/serde.rs index 3f89989f..42313e02 100644 --- a/packages/aead-frame/tests/serde.rs +++ b/packages/aead-frame/tests/serde.rs @@ -10,11 +10,11 @@ use serde::{Deserialize, Serialize}; use vitaminc_aead::plaintext::{Int, Logical, MapSink, Physical, Raw, Sink}; use vitaminc_aead::{Encrypt, FrameWriter}; use vitaminc_aead_frame::serde::{self as transport, from_slice, Error, PathSegment}; -use vitaminc_protected::Controlled; +use vitaminc_protected::{Buffer, Controlled}; /// Write `value`, and take the frame out of its `Protected`. fn to_vec(value: &T) -> Result, Error> { - transport::to_vec(value).map(Controlled::risky_unwrap) + transport::to_vec(value).map(|frame| frame.risky_unwrap().risky_into_vec()) } fn unhex(text: &str) -> Vec { @@ -122,7 +122,7 @@ fn enums_are_externally_tagged() { } // An object of one entry naming a unit variant, with a null payload, // reads as the unit variant too. - let mut bytes = Vec::new(); + let mut bytes = Buffer::new(); { let mut map = FrameWriter::new(&mut bytes).map(Some(1)).expect("map"); map.entry("eq") @@ -404,7 +404,7 @@ fn a_logical_scalar_and_a_passthrough_are_refused() { let error = from_slice::(date.risky_ref_for_test()).expect_err("refused"); assert!(error.message().contains("logical type"), "{error}"); - let mut bytes = Vec::new(); + let mut bytes = Buffer::new(); { let mut map = FrameWriter::new(&mut bytes).map(Some(1)).expect("map"); map.entry("p") @@ -439,7 +439,7 @@ fn reads_a_frame_a_sink_wrote() { name: String, age: u32, } - let mut bytes = Vec::new(); + let mut bytes = Buffer::new(); { let mut map = FrameWriter::new(&mut bytes).map(Some(2)).expect("map"); "alice" @@ -462,7 +462,7 @@ trait RiskyRefForTest { fn risky_ref_for_test(&self) -> &[u8]; } -impl RiskyRefForTest for vitaminc_protected::Protected> { +impl RiskyRefForTest for vitaminc_protected::Protected { fn risky_ref_for_test(&self) -> &[u8] { use vitaminc_protected::Controlled; self.risky_ref() @@ -697,7 +697,7 @@ fn a_unit_variant_reads_its_null_payload() { Eq, Ore, } - let mut bytes = Vec::new(); + let mut bytes = Buffer::new(); { let mut out = FrameWriter::new(&mut bytes).seq(Some(2)).expect("seq"); { @@ -1157,7 +1157,7 @@ fn a_key_only_a_host_chose_is_placed_by_position() { // A passthrough under a flattened key fails while serde buffers the // entry, through this format: the entry is placed by position. - let mut bytes = Vec::new(); + let mut bytes = Buffer::new(); { let mut map = FrameWriter::new(&mut bytes).map(Some(2)).expect("map"); map.entry("known") @@ -1220,7 +1220,7 @@ fn undefined_reads_as_null() { context: Option, unit: (), } - let mut bytes = Vec::new(); + let mut bytes = Buffer::new(); { let mut map = FrameWriter::new(&mut bytes).map(Some(2)).expect("map"); map.entry("context") @@ -1437,7 +1437,7 @@ fn a_string_reads_only_text_and_bytes_only_bytes() { assert!(error.message().contains("logical type"), "{error}"); let error = from_slice::>(date.risky_ref_for_test()).expect_err("refused"); assert!(error.message().contains("logical type"), "{error}"); - let mut passthrough = Vec::new(); + let mut passthrough = Buffer::new(); { let mut map = FrameWriter::new(&mut passthrough) .map(Some(1)) diff --git a/packages/aead-frame/tests/storage.rs b/packages/aead-frame/tests/storage.rs index e96a1e1e..5a04bc18 100644 --- a/packages/aead-frame/tests/storage.rs +++ b/packages/aead-frame/tests/storage.rs @@ -24,7 +24,7 @@ use support::{blob, erase, model, unhex, Model}; /// Write `value`, and take the blob out of its `Protected`. fn to_vec(value: &T) -> Result, Error> { - storage::to_vec(value).map(Controlled::risky_unwrap) + storage::to_vec(value).map(|blob| blob.risky_unwrap().risky_into_vec()) } /// The class the reference gives a refusal, as the crate names it. diff --git a/packages/aead-frame/tests/storage_wipe.rs b/packages/aead-frame/tests/storage_wipe.rs index aae2ee55..eaf90fc6 100644 --- a/packages/aead-frame/tests/storage_wipe.rs +++ b/packages/aead-frame/tests/storage_wipe.rs @@ -65,7 +65,12 @@ fn writing_frees_no_block_with_the_value_in_it() { // buffer grows many times over. let map: BTreeMap> = (0..64).map(|i| (marked(i), Some(marked(i + 64)))).collect(); - let (left, blob) = left_behind(|| to_vec(&map).expect("writes").risky_unwrap()); + let (left, blob) = left_behind(|| { + to_vec(&map) + .expect("writes") + .risky_unwrap() + .risky_into_vec() + }); assert!(blob.windows(MARKER.len()).any(|w| w == MARKER)); assert_eq!(left, 0, "a freed block still held the value"); @@ -94,7 +99,12 @@ fn writing_frees_no_block_with_the_value_in_it() { .map(|i| (Shown(marked(i)), Shown(marked(i + 64)))) .collect(), ); - let (left, blob) = left_behind(|| to_vec(&shown).expect("writes").risky_unwrap()); + let (left, blob) = left_behind(|| { + to_vec(&shown) + .expect("writes") + .risky_unwrap() + .risky_into_vec() + }); assert!(blob.windows(MARKER.len()).any(|w| w == MARKER)); assert_eq!(left, 0, "a displayed value was left in a freed block"); @@ -103,7 +113,12 @@ fn writing_frees_no_block_with_the_value_in_it() { // bytes are the marker show that copy is wiped. let base = u128::from_le_bytes(*MARKER); let integers: BTreeMap = (0..64).map(|i| (base + (i << 120), 1)).collect(); - let (left, blob) = left_behind(|| to_vec(&integers).expect("writes").risky_unwrap()); + let (left, blob) = left_behind(|| { + to_vec(&integers) + .expect("writes") + .risky_unwrap() + .risky_into_vec() + }); assert_eq!(left, 0, "an integer key's copy was left in a freed block"); // What is read is the caller's, and kept past the armed read. let mut read = BTreeMap::::new(); diff --git a/packages/aead-napi/src/ciphertext.rs b/packages/aead-napi/src/ciphertext.rs index f187a3ab..45ee9f82 100644 --- a/packages/aead-napi/src/ciphertext.rs +++ b/packages/aead-napi/src/ciphertext.rs @@ -122,7 +122,7 @@ impl NapiPassthrough for PassthroughFrame { fn from_js(value: Unknown<'_>, depth: usize) -> Result { let frame = vitaminc_aead::to_frame(JsRef::at_depth(value, depth)).map_err(frame_error)?; - Ok(PassthroughFrame(frame.risky_ref().clone())) + Ok(PassthroughFrame(frame.risky_ref().to_vec())) } /// A frame parsed `depth` levels down is bounded there; a payload that @@ -434,7 +434,7 @@ mod tests { } } let frame = to_frame(Arrays(levels)).expect("within the frame's own bound"); - CipherText::Passthrough(PassthroughFrame(frame.risky_ref().clone())) + CipherText::Passthrough(PassthroughFrame(frame.risky_ref().to_vec())) } #[test] diff --git a/packages/aead-napi/src/js_ref.rs b/packages/aead-napi/src/js_ref.rs index 4b0baf54..93036835 100644 --- a/packages/aead-napi/src/js_ref.rs +++ b/packages/aead-napi/src/js_ref.rs @@ -36,7 +36,7 @@ use vitaminc_aead::plaintext::{Error as _, SourceError}; use vitaminc_aead::{ to_frame, Encrypt, Float, Logical, MapSink, Physical, SeqSink, Sink, TimeUnit, }; -use vitaminc_protected::Protected; +use vitaminc_protected::{Buffer, Protected}; use crate::error::JsFault; use crate::js::{ @@ -67,7 +67,7 @@ impl<'e> JsRef<'e> { /// Snapshot the value as a frame, on the JavaScript thread, so it can be /// encrypted on a worker with no JavaScript access. The frame is /// plaintext and comes back wiped on drop. - pub fn to_frame(self) -> napi::Result>> { + pub fn to_frame(self) -> napi::Result> { to_frame(self).map_err(frame_error) } } diff --git a/packages/aead-napi/tests/addon/src/lib.rs b/packages/aead-napi/tests/addon/src/lib.rs index a1a62c14..153f9e72 100644 --- a/packages/aead-napi/tests/addon/src/lib.rs +++ b/packages/aead-napi/tests/addon/src/lib.rs @@ -126,7 +126,7 @@ pub fn decrypts_as_none(#[napi(ts_arg_type = "unknown")] ct: Ct, aad: String) -> /// here, on the JavaScript thread, and the worker seals the frame with no /// JavaScript access at all. pub struct EncryptFrame { - frame: Option>>, + frame: Option>, aad: String, } @@ -246,7 +246,7 @@ pub fn ciphertext_with_payload(ct_levels: u32, value_levels: u32) -> Result wrap: "array".into(), }); let payload = match payload { - Ok(frame) => frame.risky_ref().clone(), + Ok(frame) => frame.risky_ref().to_vec(), // Past the frame writer's bound: hand-write the frame bytes. Err(_) => { let mut bytes = Vec::new(); diff --git a/packages/aead/README.md b/packages/aead/README.md index 787fc4da..bececcd0 100644 --- a/packages/aead/README.md +++ b/packages/aead/README.md @@ -174,9 +174,11 @@ let ciphertext = encrypt(vec![Some(1u32), None], &cipher, "ctx")?; let back: Vec> = decrypt(&cipher, ciphertext, "ctx")?; assert_eq!(back, [Some(1), None]); -// The same ciphertext, written as a frame instead of a Rust type. +// The same ciphertext, written as a frame instead of a Rust type. The +// frame is plaintext, so it goes into a `Buffer`, which is wiped when it +// is dropped and leaves no copy behind as it grows. let ciphertext = encrypt(vec![Some(1u32), None], &cipher, "ctx")?; -let mut frame = Vec::new(); +let mut frame = vitaminc_protected::Buffer::new(); decrypt_into(&cipher, ciphertext, "ctx", FrameWriter::new(&mut frame))?; assert!(Frame::parse(&frame).is_ok()); diff --git a/packages/aead/src/frame.rs b/packages/aead/src/frame.rs index df1e11b9..c9f067a3 100644 --- a/packages/aead/src/frame.rs +++ b/packages/aead/src/frame.rs @@ -19,7 +19,7 @@ use std::collections::HashSet; use std::ops::Range; -use vitaminc_protected::{Controlled, OpaqueDebug, Protected, TimingSafeEq}; +use vitaminc_protected::{Buffer, Controlled, OpaqueDebug, Protected, TimingSafeEq}; use crate::leaf::{self, Form, Item, ScalarError}; use crate::plaintext::{ @@ -321,13 +321,14 @@ impl Error for FrameWriteError { } } -/// Write any source as a frame. The buffer is `Protected`, since a frame -/// of plaintext is plaintext, and it grows without leaving copies behind. +/// Write any source as a frame. The result is a [`Buffer`] under +/// `Protected`, since a frame of plaintext is plaintext: it grows without +/// leaving copies behind, and is wiped when it is dropped. /// /// The result is checked with [`Frame::parse`] before it is returned, so a /// source that opens a container and never ends it is refused here, not by /// every later reader of the frame. -pub fn to_frame(p: P) -> Result>, FrameWriteError> { +pub fn to_frame(p: P) -> Result, FrameWriteError> { to_frame_at(p, 0) } @@ -337,8 +338,8 @@ pub fn to_frame(p: P) -> Result>, FrameWriteError> pub(crate) fn to_frame_at( p: P, depth: usize, -) -> Result>, FrameWriteError> { - let mut out = Protected::new(Vec::new()); +) -> Result, FrameWriteError> { + let mut out = Protected::new(Buffer::new()); p.visit(FrameWriter { out: out.risky_inner_mut(), depth, @@ -355,21 +356,22 @@ pub(crate) fn to_frame_at( /// nor wipes that suffix. Prefer [`to_frame`] for an owned protected result /// returned only after successful writing and validation. pub struct FrameWriter<'o> { - out: &'o mut Vec, + out: &'o mut Buffer, depth: usize, } impl<'o> FrameWriter<'o> { /// Append a root-level node to `out`, preserving any existing prefix. - /// The caller owns the buffer and its cleanup, including after errors. - pub fn new(out: &'o mut Vec) -> Self { + /// The caller owns the buffer, which wipes its bytes when dropped, + /// including after an error. + pub fn new(out: &'o mut Buffer) -> Self { Self { out, depth: 0 } } /// A writer for a node that sits `depth` levels below the top of a /// document, so [`MAX_DEPTH`] counts from the top: for a subtree /// written to a buffer of its own and copied into the document later. - pub fn at_depth(out: &'o mut Vec, depth: usize) -> Self { + pub fn at_depth(out: &'o mut Buffer, depth: usize) -> Self { Self { out, depth } } @@ -379,18 +381,17 @@ impl<'o> FrameWriter<'o> { self.depth } - fn node(self, tag: u8) -> Result<&'o mut Vec, FrameWriteError> { + fn node(self, tag: u8) -> Result<&'o mut Buffer, FrameWriteError> { if self.depth > MAX_DEPTH { return Err(FrameWriteError::TooDeep); } - reserve(self.out, 1); self.out.push(tag); Ok(self.out) } fn container(self, tag: u8) -> Result, FrameWriteError> { let depth = self.depth; - reserve(self.out, 5); + self.out.reserve(5); let out = self.node(tag)?; let at = out.len(); out.extend_from_slice(&[0; 4]); @@ -404,27 +405,7 @@ impl<'o> FrameWriter<'o> { } } -/// Make room for `additional` more bytes. A `Vec` that regrows frees its -/// old allocation without wiping it, and a frame is plaintext, so grow into -/// a fresh allocation and wipe the old one instead. -pub(crate) fn reserve(out: &mut Vec, additional: usize) { - if out.capacity() - out.len() >= additional { - return; - } - // Doubling the length, not the capacity, still amortises a full - // buffer's growth, and never balloons a buffer that had room. - let capacity = out - .len() - .saturating_add(additional) - .max(out.len().saturating_mul(2)) - .max(64); - let mut grown = Vec::with_capacity(capacity); - grown.extend_from_slice(out); - vitaminc_protected::wipe_vec(out); - *out = grown; -} - -fn write_len(out: &mut Vec, len: usize) -> Result<(), FrameWriteError> { +fn write_len(out: &mut Buffer, len: usize) -> Result<(), FrameWriteError> { let len = u32::try_from(len).map_err(|_| FrameWriteError::TooLong)?; out.extend_from_slice(&len.to_le_bytes()); Ok(()) @@ -432,7 +413,7 @@ fn write_len(out: &mut Vec, len: usize) -> Result<(), FrameWriteError> { /// A container whose count is written once the children are known. struct Count<'o> { - out: &'o mut Vec, + out: &'o mut Buffer, depth: usize, at: usize, n: usize, @@ -480,7 +461,7 @@ impl<'o> Sink for FrameWriter<'o> { if self.depth > MAX_DEPTH { return Err(FrameWriteError::TooDeep); } - reserve(self.out, leaf::max_len(&v)); + self.out.reserve(leaf::max_len(&v)); leaf::write(ty, v, Form::Frame, self.out).map_err(|e| match e { ScalarError::Invalid => FrameWriteError::Invalid, ScalarError::TooLong | ScalarError::Malformed => FrameWriteError::TooLong, @@ -571,7 +552,7 @@ impl MapSink for FrameMap<'_> { fn entry(&mut self, key: &str) -> Result, FrameWriteError> { self.count.settle()?; - reserve(self.count.out, key.len().saturating_add(4)); + self.count.out.reserve(key.len().saturating_add(4)); write_len(self.count.out, key.len())?; let start = self.count.out.len(); self.count.out.extend_from_slice(key.as_bytes()); @@ -604,7 +585,7 @@ mod tests { use std::collections::BTreeMap; fn frame_of(p: P) -> Vec { - to_frame(p).expect("frames").risky_ref().clone() + to_frame(p).expect("frames").risky_ref().to_vec() } #[test] @@ -683,25 +664,6 @@ mod tests { )); } - #[test] - fn reserve_moves_to_a_bigger_buffer_and_keeps_the_bytes() { - let mut out = vec![1u8, 2, 3]; - out.shrink_to_fit(); - reserve(&mut out, 10); - assert_eq!(out, [1, 2, 3]); - assert!(out.capacity() >= 13); - let before = out.as_ptr(); - reserve(&mut out, 10); - assert_eq!(out.as_ptr(), before, "enough room already: no move"); - - // A full buffer grows, even by one byte. - let mut full = vec![7u8; 100]; - full.shrink_to_fit(); - reserve(&mut full, 1); - assert!(full.capacity() >= 101); - assert_eq!(full, [7u8; 100]); - } - #[test] fn a_parsed_frame_is_its_bytes() { let bytes = frame_of(vec![1u8, 2]); @@ -947,14 +909,14 @@ mod tests { #[test] fn a_writer_at_depth_counts_from_the_top() { - let mut out = Vec::new(); + let mut out = Buffer::new(); let writer = FrameWriter::at_depth(&mut out, 7); assert_eq!(writer.depth(), 7); let mut seq = writer.seq(Some(1)).expect("seq"); assert_eq!(seq.element().expect("element").depth(), 8); // A scalar fits at the bound, and not below it. - let mut out = Vec::new(); + let mut out = Buffer::new(); assert!(FrameWriter::at_depth(&mut out, MAX_DEPTH) .scalar(None, Physical::Bool(true)) .is_ok()); diff --git a/packages/aead/src/leaf.rs b/packages/aead/src/leaf.rs index 163b1da2..64476b05 100644 --- a/packages/aead/src/leaf.rs +++ b/packages/aead/src/leaf.rs @@ -41,6 +41,7 @@ use crate::plaintext::kinds::{ FixedType, Float, Int, Logical, Physical, TimeUnit, Width, MAX_DECIMAL_SCALE, }; use crate::tags; +use vitaminc_protected::Buffer; /// Which wire form a scalar is in. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -84,13 +85,44 @@ const DECIMAL_SCALE_MASK: u32 = 0xFF << DECIMAL_SCALE_SHIFT; /// pins it to the two masks. const DECIMAL_RESERVED: u32 = 0x7F00_FFFF; +/// Where [`write()`] puts a scalar's bytes: a `Vec` for a sealed leaf's +/// plaintext, sized up front with [`max_len`] so it never grows, or a +/// [`Buffer`] for a frame, which grows without leaving copies behind. +pub trait Out { + fn push(&mut self, byte: u8); + fn extend_from_slice(&mut self, bytes: &[u8]); +} + +impl Out for Vec { + fn push(&mut self, byte: u8) { + Vec::push(self, byte); + } + fn extend_from_slice(&mut self, bytes: &[u8]) { + Vec::extend_from_slice(self, bytes); + } +} + +impl Out for Buffer { + fn push(&mut self, byte: u8) { + Buffer::push(self, byte); + } + fn extend_from_slice(&mut self, bytes: &[u8]) { + Buffer::extend_from_slice(self, bytes); + } +} + /// Write one scalar in `form` onto `out`. Fails if the value is not valid /// for its logical type, or (in a frame) too long. -pub fn write( +/// +/// Compiled once for each `O` and called, never inlined: the frame writers +/// that call it for every scalar measured 10% to 15% slower with its body +/// inlined into them than with a call. +#[inline(never)] +pub fn write( ty: Option, v: Physical<'_>, form: Form, - out: &mut Vec, + out: &mut O, ) -> Result<(), ScalarError> { // Check everything that can fail before writing anything, so an error // never leaves a dangling tag in `out`. @@ -291,7 +323,7 @@ fn length_prefix(form: Form, len: usize) -> Result, ScalarError> { } } -fn write_physical(v: Physical<'_>, form: Form, out: &mut Vec) -> Result<(), ScalarError> { +fn write_physical(v: Physical<'_>, form: Form, out: &mut O) -> Result<(), ScalarError> { let mut variable = |tag, bytes: &[u8]| { out.push(tag); if let Some(len) = length_prefix(form, bytes.len())? { @@ -319,7 +351,7 @@ fn write_physical(v: Physical<'_>, form: Form, out: &mut Vec) -> Result<(), Ok(()) } -fn write_int(i: Int, out: &mut Vec) { +fn write_int(i: Int, out: &mut O) { macro_rules! le { ($tag:ident, $v:expr) => {{ out.push(tags::$tag); diff --git a/packages/aead/src/shape.rs b/packages/aead/src/shape.rs index 467924e5..1524f662 100644 --- a/packages/aead/src/shape.rs +++ b/packages/aead/src/shape.rs @@ -24,7 +24,7 @@ use std::collections::HashMap; -use vitaminc_protected::{OpaqueDebug, Protected}; +use vitaminc_protected::{Buffer, Controlled, OpaqueDebug, Protected}; use crate::frame::{to_frame_at, FrameWriteError, MAX_DEPTH}; use crate::plaintext::{Encrypt, Error, Logical, Path, Physical, Step}; @@ -190,11 +190,15 @@ where if let Err(refusal) = self::depth(depth, at) { return Err(refused(refusal)); } - to_frame_at(value, depth + 1).map_err(|e| match e { - FrameWriteError::Source(e) => E::source(e), - FrameWriteError::TooDeep => refused(refuse(at, Unencryptable::TooDeep)), - _ => refused(refuse(at, Unencryptable::UnframablePassthrough)), - }) + // A passthrough crosses in the clear, as its `PassthroughFrame` holds + // it, so the frame leaves the buffer's rules here. + to_frame_at(value, depth + 1) + .map(|frame| frame.map(Buffer::risky_into_vec)) + .map_err(|e| match e { + FrameWriteError::Source(e) => E::source(e), + FrameWriteError::TooDeep => refused(refuse(at, Unencryptable::TooDeep)), + _ => refused(refuse(at, Unencryptable::UnframablePassthrough)), + }) } /// How a container's children have gone: how many were handed a sink, how diff --git a/packages/aead/src/timestamp.rs b/packages/aead/src/timestamp.rs index d310b0af..a9eb3c62 100644 --- a/packages/aead/src/timestamp.rs +++ b/packages/aead/src/timestamp.rs @@ -147,7 +147,7 @@ mod tests { use vitaminc_protected::Controlled; fn frame(value: P) -> Vec { - to_frame(value).expect("frames").risky_ref().clone() + to_frame(value).expect("frames").risky_ref().to_vec() } fn leap() -> DateTime { diff --git a/packages/aead/src/walk.rs b/packages/aead/src/walk.rs index dd671023..64b4273c 100644 --- a/packages/aead/src/walk.rs +++ b/packages/aead/src/walk.rs @@ -17,7 +17,7 @@ use core::convert::Infallible; use std::collections::HashSet; -use vitaminc_protected::{Controlled, Protected}; +use vitaminc_protected::{Buffer, Controlled, Protected}; use crate::leaf::{self, Item}; use crate::plaintext::{Error, MapSink, SeqSink, Sink, SourceError}; @@ -221,18 +221,18 @@ fn walk_map( /// Decrypt into a frame: the Go guest's whole `decrypt`. /// -/// The frame is plaintext, so it is built and returned inside `Protected` +/// The frame is plaintext, so it is built in a [`Buffer`] under `Protected` /// and wiped on every error path. pub fn decrypt_to_frame<'a, C, A>( cipher: &C, node: Node, aad: A, -) -> Result>, DecryptError> +) -> Result, DecryptError> where C: Cipher, A: IntoAad<'a>, { - let mut out = Protected::new(Vec::new()); + let mut out = Protected::new(Buffer::new()); decrypt_into( cipher, node, @@ -407,8 +407,11 @@ mod tests { let sealed = encrypt(sample(), &Mock, "ctx").expect("encrypts"); let frame = decrypt_to_frame(&Mock, sealed, "ctx").expect("decrypts"); assert_eq!( - frame.risky_ref(), - crate::to_frame(sample()).expect("frames").risky_ref() + frame.risky_ref().as_slice(), + crate::to_frame(sample()) + .expect("frames") + .risky_ref() + .as_slice() ); } diff --git a/packages/aead/tests/typed_null.rs b/packages/aead/tests/typed_null.rs index cc3ca0c1..ab013618 100644 --- a/packages/aead/tests/typed_null.rs +++ b/packages/aead/tests/typed_null.rs @@ -214,12 +214,15 @@ fn a_frame_reads_a_typed_null_and_writes_the_plain_null() { assert_eq!(sealed(parsed), from_host); assert_eq!( - to_frame(None::).expect("frames").risky_ref(), + to_frame(None::) + .expect("frames") + .risky_ref() + .as_slice(), &[tags::NULL] ); let node = encrypt(None::, &Mock, "ctx").expect("encrypts"); let back = decrypt_to_frame(&Mock, node, "ctx").expect("decrypts"); - assert_eq!(back.risky_ref(), &[tags::NULL]); + assert_eq!(back.risky_ref().as_slice(), &[tags::NULL]); } #[derive(Encrypt, Decrypt, Debug, PartialEq, Clone)] diff --git a/packages/protected/src/buffer.rs b/packages/protected/src/buffer.rs new file mode 100644 index 00000000..acbcbf60 --- /dev/null +++ b/packages/protected/src/buffer.rs @@ -0,0 +1,249 @@ +use core::ops::{Deref, DerefMut}; + +use zeroize::{Zeroize, ZeroizeOnDrop}; + +use crate::locked::wipe_vec; + +/// Secret bytes on the heap, written a few at a time: a frame, a blob, a +/// serialisation of a value. +/// +/// A `Vec` that outgrows its allocation frees the old one with the +/// bytes still in it, and `zeroize` wipes a `Vec` one byte a store. A +/// `Buffer` does neither. It grows into a fresh allocation and wipes the +/// old one before it is freed; and it is wiped sixteen bytes a store, when +/// it is dropped, when it is [`zeroize`]d, and when it is [`clear`]ed. So +/// [`Protected`](crate::Protected) is what a writer of secret bytes +/// returns, and `&mut Buffer` is what it writes into. +/// +/// It is a slice for reading and for patching bytes in place, through +/// `Deref`. Every way to add bytes goes through [`reserve`], so there is no +/// way to grow it that leaves a copy behind. It is not `Clone` and has no +/// `PartialEq`: its bytes are secret, so a copy or a comparison is a +/// decision to make at the use, through the slice. +/// +/// [`zeroize`]: Zeroize::zeroize +/// [`clear`]: Buffer::clear +/// [`reserve`]: Buffer::reserve +/// +/// # Examples +/// +/// ``` +/// use vitaminc_protected::{Buffer, Controlled, Protected}; +/// +/// let mut out = Buffer::new(); +/// out.push(0x01); +/// out.extend_from_slice(b"secret"); +/// assert_eq!(&out[1..], b"secret"); +/// +/// // Returned under `Protected`, so it is wiped when the caller drops it. +/// let kept = Protected::new(out); +/// assert_eq!(kept.risky_ref().len(), 7); +/// ``` +#[derive(Default)] +pub struct Buffer(Vec); + +/// The smallest allocation a growing `Buffer` makes. +const LEAST: usize = 64; + +impl Buffer { + /// An empty buffer that allocates nothing until a byte is written. + pub const fn new() -> Self { + Self(Vec::new()) + } + + /// An empty buffer with room for `capacity` bytes before it grows. + pub fn with_capacity(capacity: usize) -> Self { + Self(Vec::with_capacity(capacity)) + } + + /// The bytes written so far. + #[inline] + pub fn as_slice(&self) -> &[u8] { + &self.0 + } + + /// How many bytes fit before the buffer grows. + #[inline] + pub fn capacity(&self) -> usize { + self.0.capacity() + } + + /// Make room for `additional` more bytes. + /// + /// A buffer with the room already is left alone. Otherwise its bytes + /// are copied into a fresh allocation, and the old one is wiped before + /// it is freed. The new allocation doubles the length, so a buffer + /// that grows byte by byte still grows in a bounded number of steps, + /// and never balloons a buffer that had room. + /// + /// The check is inlined into the writer, as `Vec`'s own is, so a byte + /// costs a compare; only the growth is a call. + #[inline] + pub fn reserve(&mut self, additional: usize) { + if self.0.capacity() - self.0.len() < additional { + self.grow(additional); + } + } + + #[cold] + #[inline(never)] + fn grow(&mut self, additional: usize) { + let capacity = self + .0 + .len() + .saturating_add(additional) + .max(self.0.len().saturating_mul(2)) + .max(LEAST); + let mut grown = Vec::with_capacity(capacity); + grown.extend_from_slice(&self.0); + wipe_vec(&mut self.0); + self.0 = grown; + } + + /// Append one byte. + #[inline] + pub fn push(&mut self, byte: u8) { + self.reserve(1); + self.0.push(byte); + } + + /// Append `bytes`. + #[inline] + pub fn extend_from_slice(&mut self, bytes: &[u8]) { + self.reserve(bytes.len()); + self.0.extend_from_slice(bytes); + } + + /// Wipe every byte and keep the allocation, for writing into again. + pub fn clear(&mut self) { + wipe_vec(&mut self.0); + } + + /// Take the bytes as a `Vec`, and with them the duty to wipe it: + /// the `Vec` is freed unwiped unless its new owner wipes it, and if it + /// grows it leaves its old bytes behind. For handing bytes to an API + /// that takes a `Vec`, right before they are used. + pub fn risky_into_vec(mut self) -> Vec { + core::mem::take(&mut self.0) + } +} + +impl From> for Buffer { + /// Take over `bytes`. Any allocation the `Vec` outgrew before this is + /// already gone, unwiped; from here on the buffer's rules apply. + fn from(bytes: Vec) -> Self { + Self(bytes) + } +} + +impl Deref for Buffer { + type Target = [u8]; + #[inline] + fn deref(&self) -> &[u8] { + &self.0 + } +} + +impl DerefMut for Buffer { + #[inline] + fn deref_mut(&mut self) -> &mut [u8] { + &mut self.0 + } +} + +impl AsRef<[u8]> for Buffer { + #[inline] + fn as_ref(&self) -> &[u8] { + &self.0 + } +} + +impl Zeroize for Buffer { + /// Wipe every byte, held or spare, and release the allocation, so the + /// drop that follows has nothing left to wipe. + fn zeroize(&mut self) { + wipe_vec(&mut self.0); + self.0 = Vec::new(); + } +} + +impl Drop for Buffer { + fn drop(&mut self) { + self.zeroize(); + } +} + +impl ZeroizeOnDrop for Buffer {} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn bytes_read_back_through_the_slice() { + let mut out = Buffer::new(); + out.push(1); + out.extend_from_slice(&[2, 3]); + assert_eq!(&*out, &[1, 2, 3]); + assert_eq!(out.as_slice(), &[1, 2, 3]); + assert_eq!(out.len(), 3); + out[0] = 9; + assert_eq!(&*out, &[9, 2, 3], "bytes are patched in place"); + } + + #[test] + fn growth_moves_to_a_bigger_allocation_and_keeps_the_bytes() { + let mut out = Buffer::with_capacity(4); + out.extend_from_slice(&[1, 2, 3, 4]); + let before = out.as_slice().as_ptr(); + out.push(5); + assert!( + !core::ptr::eq(out.as_slice().as_ptr(), before), + "a full buffer grows into a fresh allocation" + ); + assert_eq!(&*out, &[1, 2, 3, 4, 5]); + assert!( + out.capacity() >= LEAST, + "the first growth is at least {LEAST}" + ); + + let mut big = Buffer::with_capacity(100); + big.extend_from_slice(&[7; 100]); + big.reserve(1); + assert!(big.capacity() >= 200, "a buffer doubles its length"); + + let before = big.as_slice().as_ptr(); + big.reserve(10); + assert!( + core::ptr::eq(big.as_slice().as_ptr(), before), + "a buffer with room does not move" + ); + } + + #[test] + fn clear_wipes_and_keeps_the_allocation() { + let mut out = Buffer::with_capacity(16); + out.extend_from_slice(b"secret"); + let before = out.as_slice().as_ptr(); + out.clear(); + assert!(out.is_empty()); + assert!(core::ptr::eq(out.as_slice().as_ptr(), before)); + assert_eq!(out.capacity(), 16); + } + + #[test] + fn zeroize_wipes_and_releases_the_allocation() { + let mut out = Buffer::with_capacity(16); + out.extend_from_slice(b"secret"); + out.zeroize(); + assert!(out.is_empty()); + assert_eq!(out.capacity(), 0, "nothing is left for the drop to wipe"); + } + + #[test] + fn a_vec_moves_in_and_out() { + let out = Buffer::from(vec![1u8, 2]); + assert_eq!(&*out, &[1, 2]); + assert_eq!(out.risky_into_vec(), vec![1, 2]); + } +} diff --git a/packages/protected/src/lib.rs b/packages/protected/src/lib.rs index 78d7fb76..90b22b95 100644 --- a/packages/protected/src/lib.rs +++ b/packages/protected/src/lib.rs @@ -15,6 +15,7 @@ extern crate self as vitaminc_protected; mod as_protected_ref; +mod buffer; mod controlled; mod conversions; mod debug; @@ -45,6 +46,7 @@ pub mod bitvec; pub mod slice_index; pub use as_protected_ref::{AsProtectedRef, ProtectedRef}; +pub use buffer::Buffer; pub use zeroed::Zeroed; // Exports From 13c28927c56e98668f24e7aa584f77bc28e8a64d Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Tue, 27 Oct 2026 22:18:31 +1100 Subject: [PATCH 04/25] refactor(protected)!: make Buffer a Controlled type, so its bytes are read through risky_ref and not a plain slice A Buffer dereferenced to a slice, so a frame written into one could be read without a risky word anywhere, out of step with Protected and Locked in the same crate. Buffer now implements Controlled with a Vec inside: reads are risky_ref, taking the Vec is risky_unwrap, and a length is filled in through overwrite(at, bytes), which hands out no slice. Its Debug is redacted. The frame and blob writers return a bare Buffer. It already wipes on drop, so the Protected around it only added a second risky layer. BREAKING: to_frame, decrypt_to_frame, serde::to_vec and storage::to_vec return Buffer, not Protected. Buffer no longer implements Deref, DerefMut or AsRef<[u8]>; risky_into_vec is risky_unwrap. Measured on mains, three alternating rounds against the previous commit: every write within 3%, which is the run-to-run spread. --- packages/aead-frame/README.md | 2 +- packages/aead-frame/src/serde.rs | 24 +-- packages/aead-frame/src/storage.rs | 23 ++- packages/aead-frame/tests/serde.rs | 28 ++-- packages/aead-frame/tests/storage.rs | 2 +- packages/aead-frame/tests/storage_wipe.rs | 21 +-- packages/aead-napi/src/js_ref.rs | 4 +- packages/aead-napi/tests/addon/src/lib.rs | 2 +- packages/aead/README.md | 2 +- packages/aead/src/frame.rs | 20 +-- packages/aead/src/shape.rs | 4 +- packages/aead/src/walk.rs | 15 +- packages/protected/src/buffer.rs | 138 ++++++++++-------- .../controlled_is_sealed.stderr | 135 +++++------------ 14 files changed, 176 insertions(+), 244 deletions(-) diff --git a/packages/aead-frame/README.md b/packages/aead-frame/README.md index a91841f5..c1c1bc64 100644 --- a/packages/aead-frame/README.md +++ b/packages/aead-frame/README.md @@ -102,7 +102,7 @@ are nodes, so an integer, text or bytes key keeps its kind. Only the plain scalars are admitted: no logical types, no `undefined`, no passthrough. And every refusal has a stable `ErrorKind`, so a caller can tell a reader that needs upgrading from a corrupt blob. The blob is plaintext, so it -comes back `Protected`, and is wiped when dropped. +comes back in a `Buffer`: read through `risky_ref`, wiped when dropped. ```rust # #[cfg(feature = "serde")] diff --git a/packages/aead-frame/src/serde.rs b/packages/aead-frame/src/serde.rs index a7519f1a..f6a341c8 100644 --- a/packages/aead-frame/src/serde.rs +++ b/packages/aead-frame/src/serde.rs @@ -113,10 +113,10 @@ //! into a fresh allocation and wipes the old one. A `Display` value is //! formatted into a buffer that is wiped. A map's keys are checked for a //! repeat where they lie in the frame, rather than copied. The frame is -//! returned [`Protected`]. `tests/transport_wipe.rs` checks this. Copies -//! on the stack and in registers are not wiped, as for the storage -//! profile (STORAGE.md, *Memory and timing*). The reader borrows text and -//! bytes from its input. +//! returned in a [`Buffer`], whose bytes are read through `risky_ref`. +//! `tests/transport_wipe.rs` checks this. Copies on the stack and in +//! registers are not wiped, as for the storage profile (STORAGE.md, +//! *Memory and timing*). The reader borrows text and bytes from its input. use std::fmt; @@ -130,7 +130,7 @@ use vitaminc_aead::frame::{FrameWriteError, ARRAY, MAX_DEPTH, OBJECT, PASSTHROUG use vitaminc_aead::leaf::{self, Form}; use vitaminc_aead::plaintext::{Float, Int, Logical, MapSink, Physical, SeqSink, Sink}; use vitaminc_aead::{tags, Encrypt, Frame, FrameWriter}; -use vitaminc_protected::{Buffer, Protected}; +use vitaminc_protected::{Buffer, Controlled}; use zeroize::Zeroizing; // ============================================================================= @@ -1058,7 +1058,7 @@ impl<'de> VariantAccess<'de> for BareVariant<'de> { /// /// What a host and Rust exchange this way is not plaintext to seal, but may /// still be secret (a credential in a configuration, say). So the frame is -/// returned as a [`Buffer`] under [`Protected`], and wiped when it is +/// returned as a [`Buffer`], and wiped when it is /// dropped. The buffer grows without leaving copies of what it held /// behind, as every frame vitaminc writes does, and is wiped if writing /// fails part way. A value written through `Display` (`collect_str`) is @@ -1077,10 +1077,10 @@ impl<'de> VariantAccess<'de> for BareVariant<'de> { /// refused ("container length does not match its children"). `serde_json` /// ignores a wrong length, so a hand-written impl that works there can fail /// here. Pass `None` when the count is not known up front. -pub fn to_vec(value: &T) -> Result, Error> { +pub fn to_vec(value: &T) -> Result { let mut out = Buffer::new(); value.serialize(Serializer(FrameWriter::new(&mut out)))?; - Ok(Protected::new(out)) + Ok(out) } /// Write `value` as a frame of its own, for a node `depth` levels below @@ -1202,7 +1202,7 @@ impl<'o> ser::Serializer for Serializer<'o> { } fn collect_str(self, value: &T) -> Result<(), Error> { let shown = displayed(value)?; - self.serialize_str(text(&shown)?) + self.serialize_str(text(shown.risky_ref())?) } fn serialize_bytes(self, v: &[u8]) -> Result<(), Error> { self.scalar(Physical::Bytes(v)) @@ -1474,7 +1474,7 @@ fn write_variant( if is_struct { let mut map = entry.map(Some(parts.len())).map_err(written)?; for (key, frame) in parts { - let frame = Frame::parse(frame).map_err(|_| malformed())?; + let frame = Frame::parse(frame.risky_ref()).map_err(|_| malformed())?; frame .visit(map.entry(key.unwrap_or_default()).map_err(written)?) .map_err(written)?; @@ -1483,7 +1483,7 @@ fn write_variant( } else { let mut seq = entry.seq(Some(parts.len())).map_err(written)?; for (_, frame) in parts { - let frame = Frame::parse(frame).map_err(|_| malformed())?; + let frame = Frame::parse(frame.risky_ref()).map_err(|_| malformed())?; frame .visit(seq.element().map_err(written)?) .map_err(written)?; @@ -1544,7 +1544,7 @@ impl ser::Serializer for KeySerializer { // The key itself is kept `Zeroizing` by the map; only the text it // is copied from is this function's to wipe. let shown = displayed(value)?; - self.serialize_str(text(&shown)?) + self.serialize_str(text(shown.risky_ref())?) } fn serialize_char(self, v: char) -> Result { Ok(v.to_string()) diff --git a/packages/aead-frame/src/storage.rs b/packages/aead-frame/src/storage.rs index 54214fa5..d4c812cd 100644 --- a/packages/aead-frame/src/storage.rs +++ b/packages/aead-frame/src/storage.rs @@ -19,8 +19,8 @@ //! } //! //! let note = Note { title: "hi".into(), pinned: Some(()) }; -//! // The blob is plaintext, so it comes back `Protected`, and is wiped -//! // when it is dropped. +//! // The blob is plaintext, so it comes back in a `Buffer`: read through +//! // `risky_ref`, and wiped when it is dropped. //! let blob = storage::to_vec(¬e)?; //! assert_eq!(blob.risky_ref()[..4], HEADER); //! let back: Note = storage::from_slice(blob.risky_ref())?; @@ -45,7 +45,7 @@ use vitaminc_aead::frame::{FrameWriteError, ARRAY, MAX_DEPTH, OBJECT}; use vitaminc_aead::leaf::{self, Form, ScalarError}; use vitaminc_aead::plaintext::{Float, Int, Physical}; use vitaminc_aead::tags; -use vitaminc_protected::{Buffer, Protected}; +use vitaminc_protected::{Buffer, Controlled}; use crate::serde::{self as transport, from_validated, written, Appender, PathSegment, Profile}; @@ -418,7 +418,7 @@ fn distinct<'a>(keys: impl ExactSizeIterator>) -> Result<(), I Identity::Node(node) => nodes.push(node), } } - let (integers, rest) = integers.as_chunks_mut::(); + let (integers, rest) = integers.risky_inner_mut().as_chunks_mut::(); debug_assert!(rest.is_empty(), "every identity is whole"); integers.sort_unstable(); nodes.sort_unstable(); @@ -436,8 +436,7 @@ fn distinct<'a>(keys: impl ExactSizeIterator>) -> Result<(), I /// Write `value` as a blob: [`HEADER`], then its node. /// -/// The blob is the value's plaintext, so it is returned as a [`Buffer`] -/// under [`Protected`], and wiped when it is dropped. It grows without +/// The blob is the value's plaintext, so it is returned as a [`Buffer`], and wiped when it is dropped. It grows without /// leaving copies of what it held behind, and is wiped if writing fails /// part way. The writer /// checks each rule a reader checks as it writes, so a blob this returns @@ -451,7 +450,7 @@ fn distinct<'a>(keys: impl ExactSizeIterator>) -> Result<(), I /// length other than the number of children it wrote, a map key written /// without its value (or a value without its key), or a value its own /// `Serialize` refused. -pub fn to_vec(value: &T) -> Result, Error> { +pub fn to_vec(value: &T) -> Result { let mut out = Buffer::new(); out.extend_from_slice(&HEADER); value @@ -464,11 +463,11 @@ pub fn to_vec(value: &T) -> Result, Err // goes, so reading the blob back finds nothing. A debug build does so // anyway, since a blob that disagrees with the reader is a bug. debug_assert_eq!( - validate(&out[HEADER.len()..]), + validate(&out.risky_ref()[HEADER.len()..]), Ok(()), "the writer wrote a blob the reader refuses" ); - Ok(Protected::new(out)) + Ok(out) } fn write_len(out: &mut Buffer, len: usize) -> Result<(), transport::Error> { @@ -512,7 +511,7 @@ fn write_display( .map_err(|_| ::custom(""))?; let len = out.len() - at - 4; let len = u32::try_from(len).map_err(|_| written(FrameWriteError::TooLong))?; - out[at..at + 4].copy_from_slice(&len.to_le_bytes()); + out.overwrite(at, &len.to_le_bytes()); Ok(()) } @@ -680,7 +679,7 @@ impl Container<'_> { return Err(written(FrameWriteError::CountMismatch)); } let n = u32::try_from(self.n).map_err(|_| written(FrameWriteError::TooLong))?; - self.out[self.at..self.at + 4].copy_from_slice(&n.to_le_bytes()); + self.out.overwrite(self.at, &n.to_le_bytes()); Ok(()) } } @@ -980,7 +979,7 @@ impl ser::SerializeMap for MapWriter<'_> { } // The blob only grows while the map is written, so every range // still holds its key. - let out: &[u8] = self.c.out; + let out: &[u8] = self.c.out.risky_ref(); match distinct(self.keys.iter().map(|range| MapKey(&out[range.clone()]))) { Ok(()) => {} Err(Invalid::DuplicateKey) => return Err(written(FrameWriteError::DuplicateKey)), diff --git a/packages/aead-frame/tests/serde.rs b/packages/aead-frame/tests/serde.rs index 42313e02..65a4c98f 100644 --- a/packages/aead-frame/tests/serde.rs +++ b/packages/aead-frame/tests/serde.rs @@ -14,7 +14,7 @@ use vitaminc_protected::{Buffer, Controlled}; /// Write `value`, and take the frame out of its `Protected`. fn to_vec(value: &T) -> Result, Error> { - transport::to_vec(value).map(|frame| frame.risky_unwrap().risky_into_vec()) + transport::to_vec(value).map(Controlled::risky_unwrap) } fn unhex(text: &str) -> Vec { @@ -131,7 +131,10 @@ fn enums_are_externally_tagged() { .expect("null"); map.end().expect("end"); } - assert_eq!(from_slice::(&bytes).expect("reads"), Index::Eq); + assert_eq!( + from_slice::(bytes.risky_ref()).expect("reads"), + Index::Eq + ); } /// Every scalar kind serde has round trips at its own width. @@ -413,7 +416,7 @@ fn a_logical_scalar_and_a_passthrough_are_refused() { .expect("passthrough"); map.end().expect("end"); } - let error = from_slice::>(&bytes).expect_err("refused"); + let error = from_slice::>(bytes.risky_ref()).expect_err("refused"); assert!(error.message().contains("passthrough"), "{error}"); // A map's key is data, so the entry is placed by position only. assert_eq!(error.path(), [PathSegment::Entry(0)]); @@ -421,7 +424,7 @@ fn a_logical_scalar_and_a_passthrough_are_refused() { // Even a field nobody reads: an ignored key is skipped, not refused. #[derive(Deserialize)] struct Nothing {} - from_slice::(&bytes).expect("an ignored passthrough is skipped"); + from_slice::(bytes.risky_ref()).expect("an ignored passthrough is skipped"); } #[test] @@ -449,7 +452,7 @@ fn reads_a_frame_a_sink_wrote() { map.end().expect("end"); } assert_eq!( - from_slice::(&bytes).expect("reads"), + from_slice::(bytes.risky_ref()).expect("reads"), User { name: "alice".into(), age: 34 @@ -462,7 +465,7 @@ trait RiskyRefForTest { fn risky_ref_for_test(&self) -> &[u8]; } -impl RiskyRefForTest for vitaminc_protected::Protected { +impl RiskyRefForTest for Buffer { fn risky_ref_for_test(&self) -> &[u8] { use vitaminc_protected::Controlled; self.risky_ref() @@ -717,7 +720,7 @@ fn a_unit_variant_reads_its_null_payload() { vitaminc_aead::plaintext::SeqSink::end(out).expect("end"); } assert_eq!( - from_slice::>(&bytes).expect("reads"), + from_slice::>(bytes.risky_ref()).expect("reads"), [Index::Eq, Index::Ore] ); @@ -1170,7 +1173,7 @@ fn a_key_only_a_host_chose_is_placed_by_position() { .expect("passthrough"); map.end().expect("end"); } - let error = from_slice::(&bytes).expect_err("refused"); + let error = from_slice::(bytes.risky_ref()).expect_err("refused"); assert_eq!(error.path(), [PathSegment::Entry(1)]); assert!(!format!("{error} {error:?}").contains("secret_looking_key")); @@ -1234,7 +1237,7 @@ fn undefined_reads_as_null() { map.end().expect("end"); } assert_eq!( - from_slice::(&bytes).expect("reads"), + from_slice::(bytes.risky_ref()).expect("reads"), Options { context: None, unit: () @@ -1448,10 +1451,11 @@ fn a_string_reads_only_text_and_bytes_only_bytes() { .expect("passthrough"); map.end().expect("end"); } - let error = from_slice::>(&passthrough).expect_err("refused"); - assert!(error.message().contains("passthrough"), "{error}"); let error = - from_slice::>(&passthrough).expect_err("refused"); + from_slice::>(passthrough.risky_ref()).expect_err("refused"); + assert!(error.message().contains("passthrough"), "{error}"); + let error = from_slice::>(passthrough.risky_ref()) + .expect_err("refused"); assert!(error.message().contains("passthrough"), "{error}"); } diff --git a/packages/aead-frame/tests/storage.rs b/packages/aead-frame/tests/storage.rs index 5a04bc18..e96a1e1e 100644 --- a/packages/aead-frame/tests/storage.rs +++ b/packages/aead-frame/tests/storage.rs @@ -24,7 +24,7 @@ use support::{blob, erase, model, unhex, Model}; /// Write `value`, and take the blob out of its `Protected`. fn to_vec(value: &T) -> Result, Error> { - storage::to_vec(value).map(|blob| blob.risky_unwrap().risky_into_vec()) + storage::to_vec(value).map(Controlled::risky_unwrap) } /// The class the reference gives a refusal, as the crate names it. diff --git a/packages/aead-frame/tests/storage_wipe.rs b/packages/aead-frame/tests/storage_wipe.rs index eaf90fc6..aae2ee55 100644 --- a/packages/aead-frame/tests/storage_wipe.rs +++ b/packages/aead-frame/tests/storage_wipe.rs @@ -65,12 +65,7 @@ fn writing_frees_no_block_with_the_value_in_it() { // buffer grows many times over. let map: BTreeMap> = (0..64).map(|i| (marked(i), Some(marked(i + 64)))).collect(); - let (left, blob) = left_behind(|| { - to_vec(&map) - .expect("writes") - .risky_unwrap() - .risky_into_vec() - }); + let (left, blob) = left_behind(|| to_vec(&map).expect("writes").risky_unwrap()); assert!(blob.windows(MARKER.len()).any(|w| w == MARKER)); assert_eq!(left, 0, "a freed block still held the value"); @@ -99,12 +94,7 @@ fn writing_frees_no_block_with_the_value_in_it() { .map(|i| (Shown(marked(i)), Shown(marked(i + 64)))) .collect(), ); - let (left, blob) = left_behind(|| { - to_vec(&shown) - .expect("writes") - .risky_unwrap() - .risky_into_vec() - }); + let (left, blob) = left_behind(|| to_vec(&shown).expect("writes").risky_unwrap()); assert!(blob.windows(MARKER.len()).any(|w| w == MARKER)); assert_eq!(left, 0, "a displayed value was left in a freed block"); @@ -113,12 +103,7 @@ fn writing_frees_no_block_with_the_value_in_it() { // bytes are the marker show that copy is wiped. let base = u128::from_le_bytes(*MARKER); let integers: BTreeMap = (0..64).map(|i| (base + (i << 120), 1)).collect(); - let (left, blob) = left_behind(|| { - to_vec(&integers) - .expect("writes") - .risky_unwrap() - .risky_into_vec() - }); + let (left, blob) = left_behind(|| to_vec(&integers).expect("writes").risky_unwrap()); assert_eq!(left, 0, "an integer key's copy was left in a freed block"); // What is read is the caller's, and kept past the armed read. let mut read = BTreeMap::::new(); diff --git a/packages/aead-napi/src/js_ref.rs b/packages/aead-napi/src/js_ref.rs index 93036835..aedf9ef4 100644 --- a/packages/aead-napi/src/js_ref.rs +++ b/packages/aead-napi/src/js_ref.rs @@ -36,7 +36,7 @@ use vitaminc_aead::plaintext::{Error as _, SourceError}; use vitaminc_aead::{ to_frame, Encrypt, Float, Logical, MapSink, Physical, SeqSink, Sink, TimeUnit, }; -use vitaminc_protected::{Buffer, Protected}; +use vitaminc_protected::Buffer; use crate::error::JsFault; use crate::js::{ @@ -67,7 +67,7 @@ impl<'e> JsRef<'e> { /// Snapshot the value as a frame, on the JavaScript thread, so it can be /// encrypted on a worker with no JavaScript access. The frame is /// plaintext and comes back wiped on drop. - pub fn to_frame(self) -> napi::Result> { + pub fn to_frame(self) -> napi::Result { to_frame(self).map_err(frame_error) } } diff --git a/packages/aead-napi/tests/addon/src/lib.rs b/packages/aead-napi/tests/addon/src/lib.rs index 153f9e72..4373aa0e 100644 --- a/packages/aead-napi/tests/addon/src/lib.rs +++ b/packages/aead-napi/tests/addon/src/lib.rs @@ -126,7 +126,7 @@ pub fn decrypts_as_none(#[napi(ts_arg_type = "unknown")] ct: Ct, aad: String) -> /// here, on the JavaScript thread, and the worker seals the frame with no /// JavaScript access at all. pub struct EncryptFrame { - frame: Option>, + frame: Option, aad: String, } diff --git a/packages/aead/README.md b/packages/aead/README.md index bececcd0..3d6a4a81 100644 --- a/packages/aead/README.md +++ b/packages/aead/README.md @@ -180,7 +180,7 @@ assert_eq!(back, [Some(1), None]); let ciphertext = encrypt(vec![Some(1u32), None], &cipher, "ctx")?; let mut frame = vitaminc_protected::Buffer::new(); decrypt_into(&cipher, ciphertext, "ctx", FrameWriter::new(&mut frame))?; -assert!(Frame::parse(&frame).is_ok()); +assert!(Frame::parse(vitaminc_protected::Controlled::risky_ref(&frame)).is_ok()); // A target refuses a shape it cannot build, before anything is trusted. let ciphertext = encrypt("text", &cipher, "ctx")?; diff --git a/packages/aead/src/frame.rs b/packages/aead/src/frame.rs index c9f067a3..7e19a1e1 100644 --- a/packages/aead/src/frame.rs +++ b/packages/aead/src/frame.rs @@ -19,7 +19,7 @@ use std::collections::HashSet; use std::ops::Range; -use vitaminc_protected::{Buffer, Controlled, OpaqueDebug, Protected, TimingSafeEq}; +use vitaminc_protected::{Buffer, Controlled, OpaqueDebug, TimingSafeEq}; use crate::leaf::{self, Form, Item, ScalarError}; use crate::plaintext::{ @@ -321,27 +321,23 @@ impl Error for FrameWriteError { } } -/// Write any source as a frame. The result is a [`Buffer`] under -/// `Protected`, since a frame of plaintext is plaintext: it grows without +/// Write any source as a frame. The result is a [`Buffer`], since a frame of plaintext is plaintext: it grows without /// leaving copies behind, and is wiped when it is dropped. /// /// The result is checked with [`Frame::parse`] before it is returned, so a /// source that opens a container and never ends it is refused here, not by /// every later reader of the frame. -pub fn to_frame(p: P) -> Result, FrameWriteError> { +pub fn to_frame(p: P) -> Result { to_frame_at(p, 0) } /// [`to_frame`] for a subtree whose root sits `depth` levels below the top /// of a document, so [`MAX_DEPTH`] counts from the top, as the transport /// decoder counts it. -pub(crate) fn to_frame_at( - p: P, - depth: usize, -) -> Result, FrameWriteError> { - let mut out = Protected::new(Buffer::new()); +pub(crate) fn to_frame_at(p: P, depth: usize) -> Result { + let mut out = Buffer::new(); p.visit(FrameWriter { - out: out.risky_inner_mut(), + out: &mut out, depth, })?; Frame::parse(out.risky_ref()).map_err(|_| FrameWriteError::Unfinished)?; @@ -446,7 +442,7 @@ impl Count<'_> { return Err(FrameWriteError::CountMismatch); } let n = u32::try_from(self.n).map_err(|_| FrameWriteError::TooLong)?; - self.out[self.at..self.at + 4].copy_from_slice(&n.to_le_bytes()); + self.out.overwrite(self.at, &n.to_le_bytes()); Ok(()) } } @@ -563,7 +559,7 @@ impl MapSink for FrameMap<'_> { fn end(mut self) -> Result<(), FrameWriteError> { // The frame only grows while the map is written, so every range // still holds its key. - let out: &[u8] = self.count.out; + let out: &[u8] = self.count.out.risky_ref(); self.keys .sort_unstable_by(|a, b| out[a.clone()].cmp(&out[b.clone()])); if self diff --git a/packages/aead/src/shape.rs b/packages/aead/src/shape.rs index 1524f662..444d4a9b 100644 --- a/packages/aead/src/shape.rs +++ b/packages/aead/src/shape.rs @@ -24,7 +24,7 @@ use std::collections::HashMap; -use vitaminc_protected::{Buffer, Controlled, OpaqueDebug, Protected}; +use vitaminc_protected::{Controlled, OpaqueDebug, Protected}; use crate::frame::{to_frame_at, FrameWriteError, MAX_DEPTH}; use crate::plaintext::{Encrypt, Error, Logical, Path, Physical, Step}; @@ -193,7 +193,7 @@ where // A passthrough crosses in the clear, as its `PassthroughFrame` holds // it, so the frame leaves the buffer's rules here. to_frame_at(value, depth + 1) - .map(|frame| frame.map(Buffer::risky_into_vec)) + .map(|frame| Protected::new(frame.risky_unwrap())) .map_err(|e| match e { FrameWriteError::Source(e) => E::source(e), FrameWriteError::TooDeep => refused(refuse(at, Unencryptable::TooDeep)), diff --git a/packages/aead/src/walk.rs b/packages/aead/src/walk.rs index 64b4273c..1bdcc568 100644 --- a/packages/aead/src/walk.rs +++ b/packages/aead/src/walk.rs @@ -17,7 +17,7 @@ use core::convert::Infallible; use std::collections::HashSet; -use vitaminc_protected::{Buffer, Controlled, Protected}; +use vitaminc_protected::{Buffer, Controlled}; use crate::leaf::{self, Item}; use crate::plaintext::{Error, MapSink, SeqSink, Sink, SourceError}; @@ -221,24 +221,19 @@ fn walk_map( /// Decrypt into a frame: the Go guest's whole `decrypt`. /// -/// The frame is plaintext, so it is built in a [`Buffer`] under `Protected` +/// The frame is plaintext, so it is built in a [`Buffer`] /// and wiped on every error path. pub fn decrypt_to_frame<'a, C, A>( cipher: &C, node: Node, aad: A, -) -> Result, DecryptError> +) -> Result> where C: Cipher, A: IntoAad<'a>, { - let mut out = Protected::new(Buffer::new()); - decrypt_into( - cipher, - node, - aad, - crate::frame::FrameWriter::new(out.risky_inner_mut()), - )?; + let mut out = Buffer::new(); + decrypt_into(cipher, node, aad, crate::frame::FrameWriter::new(&mut out))?; Ok(out) } diff --git a/packages/protected/src/buffer.rs b/packages/protected/src/buffer.rs index acbcbf60..1ea71add 100644 --- a/packages/protected/src/buffer.rs +++ b/packages/protected/src/buffer.rs @@ -1,8 +1,6 @@ -use core::ops::{Deref, DerefMut}; - use zeroize::{Zeroize, ZeroizeOnDrop}; -use crate::locked::wipe_vec; +use crate::{locked::wipe_vec, private::ControlledPrivate, Controlled, OpaqueDebug}; /// Secret bytes on the heap, written a few at a time: a frame, a blob, a /// serialisation of a value. @@ -12,34 +10,39 @@ use crate::locked::wipe_vec; /// `Buffer` does neither. It grows into a fresh allocation and wipes the /// old one before it is freed; and it is wiped sixteen bytes a store, when /// it is dropped, when it is [`zeroize`]d, and when it is [`clear`]ed. So -/// [`Protected`](crate::Protected) is what a writer of secret bytes -/// returns, and `&mut Buffer` is what it writes into. +/// a `Buffer` is what a writer of secret bytes returns, and `&mut Buffer` +/// is what it writes into. /// -/// It is a slice for reading and for patching bytes in place, through -/// `Deref`. Every way to add bytes goes through [`reserve`], so there is no -/// way to grow it that leaves a copy behind. It is not `Clone` and has no -/// `PartialEq`: its bytes are secret, so a copy or a comparison is a -/// decision to make at the use, through the slice. +/// It is a [`Controlled`] type, like [`Protected`](crate::Protected): its +/// bytes are read through [`risky_ref`](Controlled::risky_ref) and taken +/// through [`risky_unwrap`](Controlled::risky_unwrap), so every read of +/// what it holds is named at the use. Writing needs no such name: every +/// way to add bytes goes through [`reserve`], so there is no way to grow +/// it that leaves a copy behind, and [`overwrite`] patches bytes already +/// written without handing out a slice. It is not `Clone` and has no +/// `PartialEq`: a copy or a comparison of secret bytes is a decision to +/// make at the use, through `risky_ref`. Its `Debug` shows nothing. /// /// [`zeroize`]: Zeroize::zeroize /// [`clear`]: Buffer::clear /// [`reserve`]: Buffer::reserve +/// [`overwrite`]: Buffer::overwrite /// /// # Examples /// /// ``` -/// use vitaminc_protected::{Buffer, Controlled, Protected}; +/// use vitaminc_protected::{Buffer, Controlled}; /// /// let mut out = Buffer::new(); -/// out.push(0x01); +/// out.push(0x00); /// out.extend_from_slice(b"secret"); -/// assert_eq!(&out[1..], b"secret"); +/// out.overwrite(0, &[0x01]); +/// assert_eq!(out.len(), 7); /// -/// // Returned under `Protected`, so it is wiped when the caller drops it. -/// let kept = Protected::new(out); -/// assert_eq!(kept.risky_ref().len(), 7); +/// // Reading the bytes is named: anything copied out is yours to wipe. +/// assert_eq!(&out.risky_ref()[1..], b"secret"); /// ``` -#[derive(Default)] +#[derive(Default, OpaqueDebug)] pub struct Buffer(Vec); /// The smallest allocation a growing `Buffer` makes. @@ -56,10 +59,16 @@ impl Buffer { Self(Vec::with_capacity(capacity)) } - /// The bytes written so far. + /// How many bytes have been written. #[inline] - pub fn as_slice(&self) -> &[u8] { - &self.0 + pub fn len(&self) -> usize { + self.0.len() + } + + /// Whether no byte has been written. + #[inline] + pub fn is_empty(&self) -> bool { + self.0.is_empty() } /// How many bytes fit before the buffer grows. @@ -114,46 +123,47 @@ impl Buffer { self.0.extend_from_slice(bytes); } + /// Replace the bytes from `at` with `bytes`, which must already have + /// been written: for a length filled in once its container is done. + /// + /// # Panics + /// + /// If `at + bytes.len()` is past the end of what has been written. + #[inline] + pub fn overwrite(&mut self, at: usize, bytes: &[u8]) { + self.0[at..][..bytes.len()].copy_from_slice(bytes); + } + /// Wipe every byte and keep the allocation, for writing into again. pub fn clear(&mut self) { wipe_vec(&mut self.0); } - - /// Take the bytes as a `Vec`, and with them the duty to wipe it: - /// the `Vec` is freed unwiped unless its new owner wipes it, and if it - /// grows it leaves its old bytes behind. For handing bytes to an API - /// that takes a `Vec`, right before they are used. - pub fn risky_into_vec(mut self) -> Vec { - core::mem::take(&mut self.0) - } } -impl From> for Buffer { +impl ControlledPrivate for Buffer {} + +impl Controlled for Buffer { + type Inner = Vec; + /// Take over `bytes`. Any allocation the `Vec` outgrew before this is /// already gone, unwiped; from here on the buffer's rules apply. - fn from(bytes: Vec) -> Self { + fn init_from_inner(bytes: Vec) -> Self { Self(bytes) } -} -impl Deref for Buffer { - type Target = [u8]; - #[inline] - fn deref(&self) -> &[u8] { - &self.0 + fn inner_mut(&mut self) -> &mut Vec { + &mut self.0 } -} -impl DerefMut for Buffer { - #[inline] - fn deref_mut(&mut self) -> &mut [u8] { - &mut self.0 + /// Take the bytes as a `Vec`, and with them the duty to wipe it: + /// the `Vec` is freed unwiped unless its new owner wipes it, and if it + /// grows it leaves its old bytes behind. For handing bytes to an API + /// that takes a `Vec`, right before they are used. + fn risky_unwrap(mut self) -> Vec { + core::mem::take(&mut self.0) } -} -impl AsRef<[u8]> for Buffer { - #[inline] - fn as_ref(&self) -> &[u8] { + fn risky_ref(&self) -> &Vec { &self.0 } } @@ -180,28 +190,36 @@ mod tests { use super::*; #[test] - fn bytes_read_back_through_the_slice() { + fn bytes_read_back_through_risky_ref() { let mut out = Buffer::new(); out.push(1); out.extend_from_slice(&[2, 3]); - assert_eq!(&*out, &[1, 2, 3]); - assert_eq!(out.as_slice(), &[1, 2, 3]); + assert_eq!(out.risky_ref(), &[1, 2, 3]); assert_eq!(out.len(), 3); - out[0] = 9; - assert_eq!(&*out, &[9, 2, 3], "bytes are patched in place"); + assert!(!out.is_empty()); + out.overwrite(0, &[9]); + assert_eq!(out.risky_ref(), &[9, 2, 3], "bytes are patched in place"); + } + + #[test] + #[should_panic(expected = "out of range")] + fn overwrite_refuses_to_write_past_the_end() { + let mut out = Buffer::new(); + out.extend_from_slice(&[1, 2, 3]); + out.overwrite(2, &[8, 9]); } #[test] fn growth_moves_to_a_bigger_allocation_and_keeps_the_bytes() { let mut out = Buffer::with_capacity(4); out.extend_from_slice(&[1, 2, 3, 4]); - let before = out.as_slice().as_ptr(); + let before = out.risky_ref().as_ptr(); out.push(5); assert!( - !core::ptr::eq(out.as_slice().as_ptr(), before), + !core::ptr::eq(out.risky_ref().as_ptr(), before), "a full buffer grows into a fresh allocation" ); - assert_eq!(&*out, &[1, 2, 3, 4, 5]); + assert_eq!(out.risky_ref(), &[1, 2, 3, 4, 5]); assert!( out.capacity() >= LEAST, "the first growth is at least {LEAST}" @@ -212,10 +230,10 @@ mod tests { big.reserve(1); assert!(big.capacity() >= 200, "a buffer doubles its length"); - let before = big.as_slice().as_ptr(); + let before = big.risky_ref().as_ptr(); big.reserve(10); assert!( - core::ptr::eq(big.as_slice().as_ptr(), before), + core::ptr::eq(big.risky_ref().as_ptr(), before), "a buffer with room does not move" ); } @@ -224,10 +242,10 @@ mod tests { fn clear_wipes_and_keeps_the_allocation() { let mut out = Buffer::with_capacity(16); out.extend_from_slice(b"secret"); - let before = out.as_slice().as_ptr(); + let before = out.risky_ref().as_ptr(); out.clear(); assert!(out.is_empty()); - assert!(core::ptr::eq(out.as_slice().as_ptr(), before)); + assert!(core::ptr::eq(out.risky_ref().as_ptr(), before)); assert_eq!(out.capacity(), 16); } @@ -242,8 +260,8 @@ mod tests { #[test] fn a_vec_moves_in_and_out() { - let out = Buffer::from(vec![1u8, 2]); - assert_eq!(&*out, &[1, 2]); - assert_eq!(out.risky_into_vec(), vec![1, 2]); + let out: Buffer = Controlled::new(vec![1u8, 2]); + assert_eq!(out.risky_ref(), &[1, 2]); + assert_eq!(out.risky_unwrap(), vec![1, 2]); } } diff --git a/packages/protected/tests/ui/negative_space/controlled_is_sealed.stderr b/packages/protected/tests/ui/negative_space/controlled_is_sealed.stderr index cbed48db..6a0a3490 100644 --- a/packages/protected/tests/ui/negative_space/controlled_is_sealed.stderr +++ b/packages/protected/tests/ui/negative_space/controlled_is_sealed.stderr @@ -9,26 +9,12 @@ help: the trait `vitaminc_protected::private::ControlledPrivate` is not implemen | 4 | struct Untrusted(Vec); | ^^^^^^^^^^^^^^^^ -help: the following other types implement trait `vitaminc_protected::private::ControlledPrivate` - --> src/equatable/mod.rs - | - | impl ControlledPrivate for Equatable {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Equatable` - | - ::: src/exportable/mod.rs - | - | impl ControlledPrivate for Exportable {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Exportable` - | - ::: src/protected/mod.rs - | - | impl ControlledPrivate for Protected {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Protected` - | - ::: src/usage/mod.rs - | - | impl ControlledPrivate for Usage {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Usage` + = help: the following other types implement trait `vitaminc_protected::private::ControlledPrivate`: + Buffer + Equatable + Exportable + Protected + Usage note: required by a bound in `Controlled` --> src/controlled.rs | @@ -36,6 +22,7 @@ note: required by a bound in `Controlled` | ^^^^^^^^^^^^^^^^^ required by this bound in `Controlled` = note: `Controlled` is a "sealed trait", because to implement it you also need to implement `vitaminc_protected::private::ControlledPrivate`, which is not accessible; this is usually done to force you to use one of the provided types that already implement it = help: the following types implement the trait: + vitaminc_protected::Buffer vitaminc_protected::Equatable vitaminc_protected::Exportable vitaminc_protected::Protected @@ -52,26 +39,12 @@ help: the trait `vitaminc_protected::private::ControlledPrivate` is not implemen | 4 | struct Untrusted(Vec); | ^^^^^^^^^^^^^^^^ -help: the following other types implement trait `vitaminc_protected::private::ControlledPrivate` - --> src/equatable/mod.rs - | - | impl ControlledPrivate for Equatable {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Equatable` - | - ::: src/exportable/mod.rs - | - | impl ControlledPrivate for Exportable {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Exportable` - | - ::: src/protected/mod.rs - | - | impl ControlledPrivate for Protected {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Protected` - | - ::: src/usage/mod.rs - | - | impl ControlledPrivate for Usage {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Usage` + = help: the following other types implement trait `vitaminc_protected::private::ControlledPrivate`: + Buffer + Equatable + Exportable + Protected + Usage note: required by a bound in `Controlled` --> src/controlled.rs | @@ -79,6 +52,7 @@ note: required by a bound in `Controlled` | ^^^^^^^^^^^^^^^^^ required by this bound in `Controlled` = note: `Controlled` is a "sealed trait", because to implement it you also need to implement `vitaminc_protected::private::ControlledPrivate`, which is not accessible; this is usually done to force you to use one of the provided types that already implement it = help: the following types implement the trait: + vitaminc_protected::Buffer vitaminc_protected::Equatable vitaminc_protected::Exportable vitaminc_protected::Protected @@ -95,26 +69,12 @@ help: the trait `vitaminc_protected::private::ControlledPrivate` is not implemen | 4 | struct Untrusted(Vec); | ^^^^^^^^^^^^^^^^ -help: the following other types implement trait `vitaminc_protected::private::ControlledPrivate` - --> src/equatable/mod.rs - | - | impl ControlledPrivate for Equatable {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Equatable` - | - ::: src/exportable/mod.rs - | - | impl ControlledPrivate for Exportable {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Exportable` - | - ::: src/protected/mod.rs - | - | impl ControlledPrivate for Protected {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Protected` - | - ::: src/usage/mod.rs - | - | impl ControlledPrivate for Usage {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Usage` + = help: the following other types implement trait `vitaminc_protected::private::ControlledPrivate`: + Buffer + Equatable + Exportable + Protected + Usage note: required by a bound in `Controlled` --> src/controlled.rs | @@ -122,6 +82,7 @@ note: required by a bound in `Controlled` | ^^^^^^^^^^^^^^^^^ required by this bound in `Controlled` = note: `Controlled` is a "sealed trait", because to implement it you also need to implement `vitaminc_protected::private::ControlledPrivate`, which is not accessible; this is usually done to force you to use one of the provided types that already implement it = help: the following types implement the trait: + vitaminc_protected::Buffer vitaminc_protected::Equatable vitaminc_protected::Exportable vitaminc_protected::Protected @@ -138,26 +99,12 @@ help: the trait `vitaminc_protected::private::ControlledPrivate` is not implemen | 4 | struct Untrusted(Vec); | ^^^^^^^^^^^^^^^^ -help: the following other types implement trait `vitaminc_protected::private::ControlledPrivate` - --> src/equatable/mod.rs - | - | impl ControlledPrivate for Equatable {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Equatable` - | - ::: src/exportable/mod.rs - | - | impl ControlledPrivate for Exportable {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Exportable` - | - ::: src/protected/mod.rs - | - | impl ControlledPrivate for Protected {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Protected` - | - ::: src/usage/mod.rs - | - | impl ControlledPrivate for Usage {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Usage` + = help: the following other types implement trait `vitaminc_protected::private::ControlledPrivate`: + Buffer + Equatable + Exportable + Protected + Usage note: required by a bound in `Controlled` --> src/controlled.rs | @@ -165,6 +112,7 @@ note: required by a bound in `Controlled` | ^^^^^^^^^^^^^^^^^ required by this bound in `Controlled` = note: `Controlled` is a "sealed trait", because to implement it you also need to implement `vitaminc_protected::private::ControlledPrivate`, which is not accessible; this is usually done to force you to use one of the provided types that already implement it = help: the following types implement the trait: + vitaminc_protected::Buffer vitaminc_protected::Equatable vitaminc_protected::Exportable vitaminc_protected::Protected @@ -181,26 +129,12 @@ help: the trait `vitaminc_protected::private::ControlledPrivate` is not implemen | 4 | struct Untrusted(Vec); | ^^^^^^^^^^^^^^^^ -help: the following other types implement trait `vitaminc_protected::private::ControlledPrivate` - --> src/equatable/mod.rs - | - | impl ControlledPrivate for Equatable {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Equatable` - | - ::: src/exportable/mod.rs - | - | impl ControlledPrivate for Exportable {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Exportable` - | - ::: src/protected/mod.rs - | - | impl ControlledPrivate for Protected {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Protected` - | - ::: src/usage/mod.rs - | - | impl ControlledPrivate for Usage {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Usage` + = help: the following other types implement trait `vitaminc_protected::private::ControlledPrivate`: + Buffer + Equatable + Exportable + Protected + Usage note: required by a bound in `Controlled` --> src/controlled.rs | @@ -208,6 +142,7 @@ note: required by a bound in `Controlled` | ^^^^^^^^^^^^^^^^^ required by this bound in `Controlled` = note: `Controlled` is a "sealed trait", because to implement it you also need to implement `vitaminc_protected::private::ControlledPrivate`, which is not accessible; this is usually done to force you to use one of the provided types that already implement it = help: the following types implement the trait: + vitaminc_protected::Buffer vitaminc_protected::Equatable vitaminc_protected::Exportable vitaminc_protected::Protected From 1cabc0aa221cf37b1356a8e596b8a269489ff25e Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 28 Oct 2026 00:24:15 +1100 Subject: [PATCH 05/25] perf(aead-frame): build an entry's error path only when an error is placed at it, so reading or writing a field allocates nothing The map and enum access held each entry's path segment ready while its value was read, and the transport writer did the same for each struct field and variant part. That segment owns a String copied from the type's own static name, so a struct of seven fields cost seven allocations and frees per read and per write that nothing used unless an error came back. The entry's place is now a Copy value holding the static name, and becomes a path segment only when an error is placed at it. Measured on mains, three alternating rounds against the previous commit: records read 9% to 10% faster in both profiles, a single record 16% to 18%, and the transport write of records 37% faster. --- packages/aead-frame/src/serde.rs | 68 ++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 26 deletions(-) diff --git a/packages/aead-frame/src/serde.rs b/packages/aead-frame/src/serde.rs index f6a341c8..ab05d8c7 100644 --- a/packages/aead-frame/src/serde.rs +++ b/packages/aead-frame/src/serde.rs @@ -192,12 +192,33 @@ impl Error { }) } - pub(crate) fn at(mut self, segment: PathSegment) -> Self { - self.path.insert(0, segment); + pub(crate) fn at(mut self, segment: impl Into) -> Self { + self.path.insert(0, segment.into()); self } } +/// Where an entry sits on the path, held while its value is read or +/// written. It names a key by the type's own `&'static str`, and becomes a +/// [`PathSegment`] only when an error is placed at it, so an entry read or +/// written without one allocates nothing for its path. +#[derive(Clone, Copy)] +enum At { + Key(&'static str), + Index(usize), + Entry(usize), +} + +impl From for PathSegment { + fn from(at: At) -> Self { + match at { + At::Key(key) => PathSegment::Key(key.to_owned()), + At::Index(index) => PathSegment::Index(index), + At::Entry(index) => PathSegment::Entry(index), + } + } +} + impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if self.path.is_empty() { @@ -617,10 +638,10 @@ fn is_plain(ty: Option) -> bool { /// The path segment for an object's entry: its key when that is one of the /// names the type declares, and otherwise only its position, so a key a /// host chose is never printed. -fn entry_at(names: &'static [&'static str], key: &str, index: usize) -> PathSegment { +fn entry_at(names: &'static [&'static str], key: &str, index: usize) -> At { match names.iter().find(|name| **name == key) { - Some(name) => PathSegment::Key((*name).to_owned()), - None => PathSegment::Entry(index), + Some(name) => At::Key(name), + None => At::Entry(index), } } @@ -833,7 +854,7 @@ struct Map<'a, 'de> { /// The position of the next entry. index: usize, /// Where the entry whose value is next sits on the path. - at: Option, + at: Option, } impl<'de> MapAccess<'de> for Map<'_, 'de> { @@ -870,7 +891,7 @@ impl<'de> MapAccess<'de> for Map<'_, 'de> { if self.keys == Keys::Names { return Err(de::Error::invalid_type(self.de.found()?, &"a field name")); } - self.at = Some(PathSegment::Entry(index)); + self.at = Some(At::Entry(index)); let start = self.de.pos; let key = seed.deserialize(&mut *self.de)?; if self.de.pos == start { @@ -886,9 +907,7 @@ impl<'de> MapAccess<'de> for Map<'_, 'de> { .take() .ok_or_else(|| Error::new("a map value was read before its key"))?; let start = self.de.pos; - let value = seed - .deserialize(&mut *self.de) - .map_err(|e| e.at(at.clone()))?; + let value = seed.deserialize(&mut *self.de).map_err(|e| e.at(at))?; if self.de.pos == start { return Err(unread().at(at)); } @@ -995,7 +1014,7 @@ impl Enum<'_, '_> { /// once the variant is accepted, and names it only if it is one the /// type declares: a variant read through `#[serde(other)]` accepts a /// name the type does not know, which is never printed. - fn at(&self) -> PathSegment { + fn at(&self) -> At { entry_at(self.variants, self.variant, 0) } } @@ -1134,11 +1153,15 @@ impl<'o> Serializer<'o> { } /// An object of one entry: an enum variant with a payload. - fn variant(self, variant: &str, payload: &T) -> Result<(), Error> { + fn variant( + self, + variant: &'static str, + payload: &T, + ) -> Result<(), Error> { let mut map = self.0.map(Some(1)).map_err(written)?; payload .serialize(Serializer(map.entry(variant).map_err(written)?)) - .map_err(|e| e.at(PathSegment::Key(variant.to_owned())))?; + .map_err(|e| e.at(At::Key(variant)))?; map.end().map_err(written) } } @@ -1349,12 +1372,7 @@ impl SerMap<'_> { /// Write one entry. `at` is where an error inside it is placed: a /// struct's field by name, a map's entry by position only, as in /// reading. - fn entry( - &mut self, - key: &str, - at: PathSegment, - value: &T, - ) -> Result<(), Error> { + fn entry(&mut self, key: &str, at: At, value: &T) -> Result<(), Error> { self.index += 1; value .serialize(Serializer(self.map.entry(key).map_err(written)?)) @@ -1380,8 +1398,7 @@ impl ser::SerializeMap for SerMap<'_> { .key .take() .ok_or_else(|| Error::new("a map value was written before its key"))?; - let at = PathSegment::Entry(self.index); - self.entry(&key, at, value) + self.entry(&key, At::Entry(self.index), value) } fn end(self) -> Result<(), Error> { if self.key.is_some() { @@ -1399,7 +1416,7 @@ impl ser::SerializeStruct for SerMap<'_> { key: &'static str, value: &T, ) -> Result<(), Error> { - self.entry(key, PathSegment::Key(key.to_owned()), value) + self.entry(key, At::Key(key), value) } fn end(self) -> Result<(), Error> { self.map.end().map_err(written) @@ -1446,14 +1463,13 @@ impl<'o> SerVariant<'o> { value: &T, ) -> Result<(), Error> { let at = match key { - Some(key) => PathSegment::Key(key.to_owned()), - None => PathSegment::Index(self.parts.len()), + Some(key) => At::Key(key), + None => At::Index(self.parts.len()), }; // A part is an element of the payload container, two levels below // the outer object. let depth = self.writer.depth() + 2; - let frame = to_part(value, depth) - .map_err(|e| e.at(at).at(PathSegment::Key(self.variant.to_owned())))?; + let frame = to_part(value, depth).map_err(|e| e.at(at).at(At::Key(self.variant)))?; self.parts.push((key, frame)); Ok(()) } From b9499f78eb2c146d77f6df1370c313cc96deb0e8 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 28 Oct 2026 00:36:00 +1100 Subject: [PATCH 06/25] perf(aead-frame): tell a node's kind from its tag, so a text or bytes value is decoded once on read and not twice A string, bytes or map read, and an Option read, first asked what kind of node came next. That answer was found by decoding the whole scalar, slicing its payload and checking its UTF-8, and then thrown away before the same scalar was decoded again for the visitor. The kind is in the tag byte, so it is read from there, and a tag the validator would not have passed is reported as malformed, as the decode reported it. Measured on mains, three alternating rounds against the previous commit: records read 14% faster in both profiles, a 64 KiB text 26%. --- packages/aead-frame/src/serde.rs | 47 ++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/packages/aead-frame/src/serde.rs b/packages/aead-frame/src/serde.rs index ab05d8c7..c4e29c22 100644 --- a/packages/aead-frame/src/serde.rs +++ b/packages/aead-frame/src/serde.rs @@ -489,14 +489,12 @@ impl<'de> Deserializer<'de> { Ok(()) } - /// Whether the next node is a null scalar. + /// Whether the next node is a null scalar: plain null, or `undefined`, + /// which reads as null ([`is_plain`]). Told from the tag alone, so a + /// node that is not null is not decoded here and then again by the + /// read that follows. fn next_is_null(&self) -> Result { - if matches!(self.peek()?, ARRAY | OBJECT | MAP | PASSTHROUGH | SOME) { - return Ok(false); - } - let rest = self.buf.get(self.pos..).ok_or_else(malformed)?; - let ((ty, value), _) = leaf::read(rest, Form::Frame).map_err(|_| malformed())?; - Ok(is_plain(ty) && matches!(value, Physical::Null)) + Ok(matches!(self.peek()?, tags::NULL | tags::UNDEFINED)) } /// A storage map's next key, consumed, if it is text; otherwise @@ -514,26 +512,35 @@ impl<'de> Deserializer<'de> { } /// The kind of the next node, for a refusal. It carries no value, so a - /// message built from it cannot repeat one. + /// message built from it cannot repeat one. The kind is told from the + /// tag alone: a scalar is not decoded here and then again by the read + /// that follows, and a tag the validator would not have passed is + /// reported as malformed, as that read would report it. fn found(&self) -> Result, Error> { Ok(match self.peek()? { ARRAY => Unexpected::Seq, OBJECT | MAP => Unexpected::Map, PASSTHROUGH => Unexpected::Other("a passthrough"), SOME => Unexpected::Option, - _ => { - let rest = self.buf.get(self.pos..).ok_or_else(malformed)?; - let ((ty, value), _) = leaf::read(rest, Form::Frame).map_err(|_| malformed())?; - match (ty, value) { - (Some(_), _) => Unexpected::Other("a value with a logical type"), - (None, Physical::Null) => Unexpected::Unit, - (None, Physical::Bool(_)) => Unexpected::Bool(false), - (None, Physical::Int(_)) => Unexpected::Unsigned(0), - (None, Physical::Float(_)) => Unexpected::Float(0.0), - (None, Physical::Text(_)) => Unexpected::Str(""), - (None, Physical::Bytes(_)) => Unexpected::Bytes(&[]), - } + tags::NULL => Unexpected::Unit, + tags::BOOL_FALSE | tags::BOOL_TRUE => Unexpected::Bool(false), + tags::INT8 + | tags::INT16 + | tags::INT32 + | tags::INT64 + | tags::INT128 + | tags::UINT8 + | tags::UINT16 + | tags::UINT32 + | tags::UINT64 + | tags::UINT128 => Unexpected::Unsigned(0), + tags::FLOAT32 | tags::FLOAT64 => Unexpected::Float(0.0), + tags::STRING => Unexpected::Str(""), + tags::BYTES => Unexpected::Bytes(&[]), + tags::UNDEFINED | tags::DATE | tags::TIMESTAMP | tags::DECIMAL | tags::LOGICAL => { + Unexpected::Other("a value with a logical type") } + _ => return Err(malformed()), }) } From 7d4e6505ccf105ff0a1177d090f732a17b25c643 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 28 Oct 2026 00:57:09 +1100 Subject: [PATCH 07/25] perf(aead): compare an object's keys one by one while there are few, so validating a record allocates no hash set Both validators, the frame's and the storage profile's, built a HashSet per object to refuse a repeated key: an allocation and a SipHash per field for objects that mostly have a handful. SeenKeys keeps the first sixteen keys inline and compares them directly, and hashes only once an object has more, so a host object with thousands of keys is still checked in linear time. The keys are borrowed from the frame, so nothing is copied. Measured on mains, three alternating rounds against the previous commit: records read 18% faster in the storage profile and 23% in the transport; writes unchanged. --- packages/aead-frame/src/storage.rs | 5 +-- packages/aead/src/frame.rs | 60 +++++++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/packages/aead-frame/src/storage.rs b/packages/aead-frame/src/storage.rs index d4c812cd..ce03d066 100644 --- a/packages/aead-frame/src/storage.rs +++ b/packages/aead-frame/src/storage.rs @@ -35,13 +35,12 @@ //! #![doc = include_str!("../STORAGE.md")] -use std::collections::HashSet; use std::fmt; use std::ops::Range; use ::serde::ser::{self, Serialize}; use ::serde::Deserialize; -use vitaminc_aead::frame::{FrameWriteError, ARRAY, MAX_DEPTH, OBJECT}; +use vitaminc_aead::frame::{FrameWriteError, SeenKeys, ARRAY, MAX_DEPTH, OBJECT}; use vitaminc_aead::leaf::{self, Form, ScalarError}; use vitaminc_aead::plaintext::{Float, Int, Physical}; use vitaminc_aead::tags; @@ -339,7 +338,7 @@ fn node(c: &mut Cursor<'_>, depth: usize) -> Result<(), Invalid> { OBJECT => { c.pos += 1; let count = c.count()?; - let mut seen = HashSet::new(); + let mut seen = SeenKeys::default(); for _ in 0..count { if !seen.insert(c.key()?) { return Err(Invalid::DuplicateKey); diff --git a/packages/aead/src/frame.rs b/packages/aead/src/frame.rs index 7e19a1e1..80b65b91 100644 --- a/packages/aead/src/frame.rs +++ b/packages/aead/src/frame.rs @@ -160,6 +160,44 @@ impl<'a> Cursor<'a> { } } +/// How many keys an object may have before [`SeenKeys`] hashes them. +const FEW_KEYS: usize = 16; + +/// The keys an object has given so far, to refuse a repeat. +/// +/// The keys are compared one by one while there are few: an object +/// rarely has more fields than a hash set's allocation and hashing would +/// pay for. Past sixteen they are hashed, so an object a host sends +/// with thousands of keys is still checked in linear time. The keys are +/// borrowed from the frame, so nothing is copied. +#[derive(Default)] +pub struct SeenKeys<'a> { + few: [&'a str; FEW_KEYS], + len: usize, + many: Option>, +} + +impl<'a> SeenKeys<'a> { + /// Record `key`; whether it was new. + pub fn insert(&mut self, key: &'a str) -> bool { + if let Some(many) = &mut self.many { + return many.insert(key); + } + if self.few[..self.len].contains(&key) { + return false; + } + if self.len < FEW_KEYS { + self.few[self.len] = key; + self.len += 1; + } else { + let mut many: HashSet<&'a str> = self.few.iter().copied().collect(); + many.insert(key); + self.many = Some(many); + } + true + } +} + /// Step over one value node, validating it. fn skip(c: &mut Cursor<'_>, depth: usize) -> Result<(), FrameError> { if depth > MAX_DEPTH { @@ -175,7 +213,7 @@ fn skip(c: &mut Cursor<'_>, depth: usize) -> Result<(), FrameError> { OBJECT => { c.byte()?; let count = c.count()?; - let mut seen = HashSet::new(); + let mut seen = SeenKeys::default(); for _ in 0..count { let key = c.key()?; if !seen.insert(key) { @@ -584,6 +622,26 @@ mod tests { to_frame(p).expect("frames").risky_ref().to_vec() } + #[test] + fn seen_keys_find_a_repeat_before_and_after_they_spill_to_a_set() { + let names: Vec = (0..40).map(|i| format!("k{i}")).collect(); + let mut seen = SeenKeys::default(); + for (i, name) in names.iter().enumerate() { + assert!(seen.insert(name), "key {i} is new"); + } + assert_eq!(seen.len, FEW_KEYS, "the first keys stay inline"); + assert!(seen.many.is_some(), "the rest spilled to a set"); + assert!(!seen.insert(&names[0]), "an inline key repeats"); + assert!(!seen.insert(&names[39]), "a spilled key repeats"); + assert!(seen.insert("new"), "a fresh key is still new"); + + let mut few = SeenKeys::default(); + assert!(few.insert("a")); + assert!(few.insert("b")); + assert!(!few.insert("a"), "a repeat is found without a set"); + assert!(few.many.is_none()); + } + #[test] fn a_frame_round_trips_through_write_and_walk() { let mut value = BTreeMap::new(); From 615a035252fd597786f8f00570c150a3330ff9f6 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 28 Oct 2026 11:29:36 +1100 Subject: [PATCH 08/25] fix(protected): unwind the wipe_raw proof 65 times, so Kani can finish its 64-byte check loop --- packages/protected/src/locked/proofs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/protected/src/locked/proofs.rs b/packages/protected/src/locked/proofs.rs index f29f436b..59a91248 100644 --- a/packages/protected/src/locked/proofs.rs +++ b/packages/protected/src/locked/proofs.rs @@ -62,7 +62,7 @@ fn the_value_is_inside_the_interior_aligned_and_against_the_end() { } #[kani::proof] -#[kani::unwind(33)] +#[kani::unwind(65)] fn wipe_raw_zeroes_exactly_the_bytes_it_is_given() { // Wide enough to hold a byte-wise head, whole words and a tail, // whatever the buffer's alignment. From e9f9fc8f6e2020414def67e2df80d376899083cc Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 28 Oct 2026 11:29:37 +1100 Subject: [PATCH 09/25] test(protected): check that a Buffer's drop wipes it and that risky_inner_mut reaches its bytes, so neither can be removed unnoticed --- packages/protected/src/buffer.rs | 13 +++++ packages/protected/tests/buffer_wipe.rs | 63 +++++++++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 packages/protected/tests/buffer_wipe.rs diff --git a/packages/protected/src/buffer.rs b/packages/protected/src/buffer.rs index 1ea71add..dd908f5e 100644 --- a/packages/protected/src/buffer.rs +++ b/packages/protected/src/buffer.rs @@ -258,6 +258,19 @@ mod tests { assert_eq!(out.capacity(), 0, "nothing is left for the drop to wipe"); } + #[test] + fn risky_inner_mut_changes_the_bytes_the_buffer_holds() { + let mut out = Buffer::new(); + out.extend_from_slice(&[1, 2]); + out.risky_inner_mut().push(3); + out.risky_inner_mut()[0] = 7; + assert_eq!( + out.risky_ref(), + &[7, 2, 3], + "a change through risky_inner_mut is a change to the buffer" + ); + } + #[test] fn a_vec_moves_in_and_out() { let out: Buffer = Controlled::new(vec![1u8, 2]); diff --git a/packages/protected/tests/buffer_wipe.rs b/packages/protected/tests/buffer_wipe.rs new file mode 100644 index 00000000..94da0d04 --- /dev/null +++ b/packages/protected/tests/buffer_wipe.rs @@ -0,0 +1,63 @@ +//! Dropping a `Buffer` must wipe its allocation before it is freed. +//! +//! This test installs a global allocator that watches one allocation and +//! records whether every byte of it was zero when it was freed. It lives +//! in its own test binary so that allocator sees nothing but this test. + +use std::alloc::{GlobalAlloc, Layout, System}; +use std::sync::atomic::{AtomicBool, AtomicPtr, Ordering}; + +use vitaminc_protected::{Buffer, Controlled}; + +/// The allocation under watch, whether it was freed, and whether every +/// byte was zero when it was. +static WATCHED: AtomicPtr = AtomicPtr::new(std::ptr::null_mut()); +static FREED: AtomicBool = AtomicBool::new(false); +static FREED_ZEROED: AtomicBool = AtomicBool::new(false); + +struct Watching; + +// SAFETY: every call is forwarded to `System` unchanged. `dealloc` only +// reads the block it is about to free, which the caller still owns. +unsafe impl GlobalAlloc for Watching { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + unsafe { System.alloc(layout) } + } + + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + // Only the first free of the watched address is the buffer's own; + // the address can be handed out again afterwards. + let watched = WATCHED.compare_exchange( + ptr, + std::ptr::null_mut(), + Ordering::SeqCst, + Ordering::SeqCst, + ); + if watched.is_ok() { + // SAFETY: `ptr` is a live block of `layout.size()` bytes. + let block = unsafe { core::slice::from_raw_parts(ptr, layout.size()) }; + FREED_ZEROED.store(block.iter().all(|&b| b == 0), Ordering::SeqCst); + FREED.store(true, Ordering::SeqCst); + } + unsafe { System.dealloc(ptr, layout) } + } +} + +#[global_allocator] +static ALLOCATOR: Watching = Watching; + +#[test] +fn dropping_a_buffer_frees_its_allocation_wiped() { + let mut out = Buffer::with_capacity(64); + out.extend_from_slice(b"a secret that must not outlive its buffer"); + WATCHED.store(out.risky_ref().as_ptr().cast_mut(), Ordering::SeqCst); + drop(out); + assert!( + FREED.load(Ordering::SeqCst), + "the buffer's allocation was never freed" + ); + assert!( + FREED_ZEROED.load(Ordering::SeqCst), + "the buffer's allocation was freed with its bytes still in it" + ); +} From 094bbe78fdb2d0d37ef4df345e4b09ecfe3e8170 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 28 Oct 2026 11:29:37 +1100 Subject: [PATCH 10/25] test(aead-frame): check a null, a boolean and a float read as a string, bytes or a struct are refused by their kind --- packages/aead-frame/tests/serde.rs | 38 ++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/packages/aead-frame/tests/serde.rs b/packages/aead-frame/tests/serde.rs index 65a4c98f..4facea21 100644 --- a/packages/aead-frame/tests/serde.rs +++ b/packages/aead-frame/tests/serde.rs @@ -1403,6 +1403,44 @@ fn a_struct_or_map_reads_only_an_object() { assert_eq!((read.x, read.y), (1, 2)); } +/// A null, a boolean or a float where a string, bytes or an object belongs +/// is refused, and the error names the kind that was there. +#[test] +fn a_scalar_of_the_wrong_kind_is_refused_by_its_kind() { + #[derive(Deserialize, Debug)] + #[allow(dead_code)] + struct Point { + x: u8, + } + let cases = [ + (to_vec(&()).expect("writes"), "null"), + (to_vec(&false).expect("writes"), "a boolean"), + (to_vec(&true).expect("writes"), "a boolean"), + (to_vec(&1.5f32).expect("writes"), "a float"), + (to_vec(&1.5f64).expect("writes"), "a float"), + ]; + for (frame, kind) in cases { + let error = from_slice::(&frame).expect_err("refused"); + assert_eq!( + error.message(), + format!("expected a string, found {kind}"), + "a string reads only text" + ); + let error = from_slice::(&frame).expect_err("refused"); + assert_eq!( + error.message(), + format!("expected byte array, found {kind}"), + "bytes read only bytes" + ); + let error = from_slice::(&frame).expect_err("refused"); + assert_eq!( + error.message(), + format!("expected struct Point, found {kind}"), + "a struct reads only an object" + ); + } +} + /// Text and bytes are kept apart: a string reads only text, and bytes read /// only bytes, though serde's own visitors would take either. #[test] From 40dd628295d4d7c3c32c761e7630364a804e3753 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 28 Oct 2026 11:33:35 +1100 Subject: [PATCH 11/25] docs(aead-frame): say the storage blob comes back in a Buffer, since to_vec no longer returns it Protected --- ...torage-profile-for-values-that-are-kept.md | 6 +++++ packages/aead-frame/STORAGE.md | 23 ++++++++++--------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/docs/adr/0003-a-versioned-storage-profile-for-values-that-are-kept.md b/docs/adr/0003-a-versioned-storage-profile-for-values-that-are-kept.md index 9c72db9c..debe4305 100644 --- a/docs/adr/0003-a-versioned-storage-profile-for-values-that-are-kept.md +++ b/docs/adr/0003-a-versioned-storage-profile-for-values-that-are-kept.md @@ -46,6 +46,12 @@ properties that are fine in flight and wrong at rest: deserializer, and has its own validator, writer and error type. `to_vec` returns the blob `Protected`, since it is the value's plaintext. + *Amended 2026-10-28 (#559):* `to_vec` now returns the blob in a + `vitaminc_protected::Buffer` rather than a `Protected>`. It is + still the value's plaintext and still read through `risky_ref` and + wiped when dropped; a `Buffer` also grows without leaving copies of its + bytes behind. The blob's bytes are unchanged. + 2. **A header.** `"VCS"`, then a version byte, `0x01`. No header byte is a node tag, so a transport frame never reads as a blob, nor a blob as a frame. A version the reader does not read gets its own `ErrorKind`, diff --git a/packages/aead-frame/STORAGE.md b/packages/aead-frame/STORAGE.md index 38c18327..ac53709f 100644 --- a/packages/aead-frame/STORAGE.md +++ b/packages/aead-frame/STORAGE.md @@ -14,10 +14,11 @@ The words MUST, MUST NOT and MAY are used as in RFC 2119. A blob is the plaintext of a ciphertext that is kept. That asks more of a format than a compact encoding, and the profile is built for it: -- **The plaintext is handled as a secret.** `to_vec` returns the blob - `Protected`. It grows its buffer into a fresh allocation and wipes the - old one, writes a `Display` value straight into the blob, keeps no copy - of a key, and wipes what it wrote if writing fails. The reader borrows +- **The plaintext is handled as a secret.** `to_vec` returns the blob in + a `Buffer`, which is read through `risky_ref` and wiped when it is + dropped. The writer grows its buffer into a fresh allocation and wipes + the old one, writes a `Display` value straight into the blob, keeps no + copy of a key, and wipes what it wrote if writing fails. The reader borrows text and bytes from the blob rather than copying them. See [Memory and timing](#memory-and-timing). - **A refusal never repeats the plaintext.** Error messages are fixed text. @@ -371,7 +372,7 @@ that, and never changes because the type did. ## The blob inside an outer envelope -A blob is plaintext, and `to_vec` returns it `Protected`. The caller seals +A blob is plaintext, and `to_vec` returns it in a `Buffer`. The caller seals it whole, header included, under an envelope of its own (Stack Encrypt's ciphertext). The header is therefore encrypted with the value: nothing about the blob, its version included, can be seen before decryption, and @@ -392,12 +393,12 @@ choice, and needs nothing from this profile. The writer grows its buffer into a fresh allocation and wipes the old one, formats a `Display` value straight into the blob, keeps no copy of a key, -and wipes everything if writing fails. The blob it returns is `Protected`, -and wiped when dropped. The reader borrows text and bytes from the blob -rather than copying them, and finds a repeated map key by comparing keys -where they lie in the blob. The writer frees no heap block that still -holds the value, and a zeroize audit checked in LLVM IR and assembly that -every wipe survives optimization. +and wipes everything if writing fails. The blob it returns is a `Buffer`: +its bytes are read through `risky_ref`, and it is wiped when dropped. The +reader borrows text and bytes from the blob rather than copying them, and +finds a repeated map key by comparing keys where they lie in the blob. The +writer frees no heap block that still holds the value, and a zeroize audit +checked in LLVM IR and assembly that every wipe survives optimization. Copies on the stack and in registers are not wiped: From 04b3d1210ded7b152f0b9049f266dd0a7ab0f307 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 28 Oct 2026 11:33:35 +1100 Subject: [PATCH 12/25] docs(aead-frame): rewrap the Buffer return docs and name Buffer in the test helper, so the text reads as the rest of the file --- packages/aead-frame/src/serde.rs | 10 +++++----- packages/aead-frame/src/storage.rs | 6 +++--- packages/aead-frame/tests/serde.rs | 2 +- packages/aead/src/frame.rs | 5 +++-- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/aead-frame/src/serde.rs b/packages/aead-frame/src/serde.rs index c4e29c22..911382eb 100644 --- a/packages/aead-frame/src/serde.rs +++ b/packages/aead-frame/src/serde.rs @@ -1084,11 +1084,11 @@ impl<'de> VariantAccess<'de> for BareVariant<'de> { /// /// What a host and Rust exchange this way is not plaintext to seal, but may /// still be secret (a credential in a configuration, say). So the frame is -/// returned as a [`Buffer`], and wiped when it is -/// dropped. The buffer grows without leaving copies of what it held -/// behind, as every frame vitaminc writes does, and is wiped if writing -/// fails part way. A value written through `Display` (`collect_str`) is -/// formatted into a buffer that is wiped too. +/// returned as a [`Buffer`], and wiped when it is dropped. The buffer +/// grows without leaving copies of what it held behind, as every frame +/// vitaminc writes does, and is wiped if writing fails part way. A value +/// written through `Display` (`collect_str`) is formatted into a buffer +/// that is wiped too. /// /// # Errors /// diff --git a/packages/aead-frame/src/storage.rs b/packages/aead-frame/src/storage.rs index ce03d066..b09913d6 100644 --- a/packages/aead-frame/src/storage.rs +++ b/packages/aead-frame/src/storage.rs @@ -435,9 +435,9 @@ fn distinct<'a>(keys: impl ExactSizeIterator>) -> Result<(), I /// Write `value` as a blob: [`HEADER`], then its node. /// -/// The blob is the value's plaintext, so it is returned as a [`Buffer`], and wiped when it is dropped. It grows without -/// leaving copies of what it held behind, and is wiped if writing fails -/// part way. The writer +/// The blob is the value's plaintext, so it is returned as a [`Buffer`], +/// and wiped when it is dropped. It grows without leaving copies of what +/// it held behind, and is wiped if writing fails part way. The writer /// checks each rule a reader checks as it writes, so a blob this returns /// always reads; a debug build reads it back to make sure. /// diff --git a/packages/aead-frame/tests/serde.rs b/packages/aead-frame/tests/serde.rs index 4facea21..5e03ac73 100644 --- a/packages/aead-frame/tests/serde.rs +++ b/packages/aead-frame/tests/serde.rs @@ -12,7 +12,7 @@ use vitaminc_aead::{Encrypt, FrameWriter}; use vitaminc_aead_frame::serde::{self as transport, from_slice, Error, PathSegment}; use vitaminc_protected::{Buffer, Controlled}; -/// Write `value`, and take the frame out of its `Protected`. +/// Write `value`, and take the frame out of its `Buffer`. fn to_vec(value: &T) -> Result, Error> { transport::to_vec(value).map(Controlled::risky_unwrap) } diff --git a/packages/aead/src/frame.rs b/packages/aead/src/frame.rs index 80b65b91..8c4f28d0 100644 --- a/packages/aead/src/frame.rs +++ b/packages/aead/src/frame.rs @@ -359,8 +359,9 @@ impl Error for FrameWriteError { } } -/// Write any source as a frame. The result is a [`Buffer`], since a frame of plaintext is plaintext: it grows without -/// leaving copies behind, and is wiped when it is dropped. +/// Write any source as a frame. The result is a [`Buffer`], since a frame +/// of plaintext is plaintext: it grows without leaving copies behind, and +/// is wiped when it is dropped. /// /// The result is checked with [`Frame::parse`] before it is returned, so a /// source that opens a container and never ends it is refused here, not by From c8b9a9e23bba817b872c874f33dfc8b3735ebf6a Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 28 Oct 2026 11:33:36 +1100 Subject: [PATCH 13/25] docs(aead-frame): state that the performance tables predate the Buffer and reader changes, rather than promise a re-measure --- packages/aead-frame/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aead-frame/README.md b/packages/aead-frame/README.md index c1c1bc64..18675b4a 100644 --- a/packages/aead-frame/README.md +++ b/packages/aead-frame/README.md @@ -176,7 +176,7 @@ What the numbers show: - Reading text or bytes costs little more than copying them out: 64 KiB of bytes reads at about 32 GiB/s, as fast as MessagePack. - On values with many small parts, the storage profile is the slowest of the five. It writes 1,000 records 6.7 times as slowly as MessagePack and 31 times as slowly as Borsh, and reads them 3.2 and 6.4 times as slowly. MessagePack carries the same field names, so the names are not the cost; each element or field costs the storage writer about 40 to 90 ns more. - The storage profile is also the largest: 1,000 records take 57% more bytes than in MessagePack, and the map 89% more. Every length and count is a fixed four bytes and every value has a kind byte, where MessagePack packs small lengths and integers into one byte. Borsh is the smallest for records, but writes an integer at its full width, so the map of `u64` values is nearly as large as in the storage profile. -- The tables predate a change to the storage writer. It used to read the whole blob back before returning it, to find a repeated key, and that pass was 55% of a 1,000-record write. It now refuses a repeated field name as it writes, comparing the names an object has written on the stack, and checks a map's keys where they lie in the blob when the map ends. Measured against the tables' code on the same machine, that made a 1,000-record write 47% faster, a single record 41% faster and the map 28% faster, and left reads unchanged. The tables will be re-measured before this is released. +- The tables predate a change to the storage writer. It used to read the whole blob back before returning it, to find a repeated key, and that pass was 55% of a 1,000-record write. It now refuses a repeated field name as it writes, comparing the names an object has written on the stack, and checks a map's keys where they lie in the blob when the map ends. Measured against the tables' code on the same machine, that made a 1,000-record write 47% faster, a single record 41% faster and the map 28% faster, and left reads unchanged. The tables also predate later changes to the writer and the reader: the blob is now written into a `Buffer` that is wiped sixteen bytes at a time, and the reader tells a value's kind from its tag. They have not been measured again since. ## Cargo features From e1bc3970da856a036092733d3b0106a20bc87f06 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 28 Oct 2026 11:36:32 +1100 Subject: [PATCH 14/25] refactor(protected): keep wipe_vec inside the crate, since only Buffer calls it and a public Vec wipe invites unwrapped secrets --- packages/protected/src/lib.rs | 2 +- packages/protected/src/locked/mod.rs | 15 ++++----------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/packages/protected/src/lib.rs b/packages/protected/src/lib.rs index 90b22b95..86404ac3 100644 --- a/packages/protected/src/lib.rs +++ b/packages/protected/src/lib.rs @@ -57,7 +57,7 @@ pub use digest::ProtectedDigest; pub use ::digest::InvalidLength; pub use equatable::{ConstantTimeEq, Equatable}; pub use exportable::{Exportable, SafeDeserialize, SafeSerialize}; -pub use locked::{wipe_vec, LockError, LockPolicy, LockPolicyError, Locked}; +pub use locked::{LockError, LockPolicy, LockPolicyError, Locked}; /// The name `MaybeEmpty` had before it was renamed: it read as a marker /// ("values of this type are empty") when it is a capability ("this value /// can be asked whether it is empty"). Bounds and impls written against diff --git a/packages/protected/src/locked/mod.rs b/packages/protected/src/locked/mod.rs index bc0befdc..d69f2fae 100644 --- a/packages/protected/src/locked/mod.rs +++ b/packages/protected/src/locked/mod.rs @@ -698,18 +698,11 @@ pub(super) unsafe fn wipe_raw(ptr: *mut u8, len: usize) { /// This does what `out.zeroize()` does, but with sixteen bytes in each /// volatile store rather than one, so a buffer of secret bytes that is /// grown or dropped often (a frame, a blob) is wiped at close to memory -/// speed. The allocation is kept; only its contents go. +/// speed. The allocation is kept; only its contents go. [`Buffer`] wipes +/// itself with this. /// -/// # Examples -/// -/// ``` -/// let mut buffer = Vec::with_capacity(64); -/// buffer.extend_from_slice(b"a secret that must not outlive its use"); -/// vitaminc_protected::wipe_vec(&mut buffer); -/// assert!(buffer.is_empty()); -/// assert!(buffer.capacity() >= 64); -/// ``` -pub fn wipe_vec(out: &mut Vec) { +/// [`Buffer`]: crate::Buffer +pub(crate) fn wipe_vec(out: &mut Vec) { out.clear(); // SAFETY: a `Vec`'s pointer is valid for writes of `capacity()` bytes, // and after `clear` none of them is a live element. From 55745a30add6384d24755ca9a2fa0cf4ebe863c1 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 28 Oct 2026 11:39:28 +1100 Subject: [PATCH 15/25] refactor(protected): drop Buffer::clear and Buffer::capacity, since no writer uses them and each is more surface to review --- packages/protected/src/buffer.rs | 42 ++++++++++---------------------- 1 file changed, 13 insertions(+), 29 deletions(-) diff --git a/packages/protected/src/buffer.rs b/packages/protected/src/buffer.rs index dd908f5e..1db76e88 100644 --- a/packages/protected/src/buffer.rs +++ b/packages/protected/src/buffer.rs @@ -9,9 +9,9 @@ use crate::{locked::wipe_vec, private::ControlledPrivate, Controlled, OpaqueDebu /// bytes still in it, and `zeroize` wipes a `Vec` one byte a store. A /// `Buffer` does neither. It grows into a fresh allocation and wipes the /// old one before it is freed; and it is wiped sixteen bytes a store, when -/// it is dropped, when it is [`zeroize`]d, and when it is [`clear`]ed. So -/// a `Buffer` is what a writer of secret bytes returns, and `&mut Buffer` -/// is what it writes into. +/// it is dropped and when it is [`zeroize`]d. So a `Buffer` is what a +/// writer of secret bytes returns, and `&mut Buffer` is what it writes +/// into. /// /// It is a [`Controlled`] type, like [`Protected`](crate::Protected): its /// bytes are read through [`risky_ref`](Controlled::risky_ref) and taken @@ -24,7 +24,6 @@ use crate::{locked::wipe_vec, private::ControlledPrivate, Controlled, OpaqueDebu /// make at the use, through `risky_ref`. Its `Debug` shows nothing. /// /// [`zeroize`]: Zeroize::zeroize -/// [`clear`]: Buffer::clear /// [`reserve`]: Buffer::reserve /// [`overwrite`]: Buffer::overwrite /// @@ -71,12 +70,6 @@ impl Buffer { self.0.is_empty() } - /// How many bytes fit before the buffer grows. - #[inline] - pub fn capacity(&self) -> usize { - self.0.capacity() - } - /// Make room for `additional` more bytes. /// /// A buffer with the room already is left alone. Otherwise its bytes @@ -133,11 +126,6 @@ impl Buffer { pub fn overwrite(&mut self, at: usize, bytes: &[u8]) { self.0[at..][..bytes.len()].copy_from_slice(bytes); } - - /// Wipe every byte and keep the allocation, for writing into again. - pub fn clear(&mut self) { - wipe_vec(&mut self.0); - } } impl ControlledPrivate for Buffer {} @@ -221,14 +209,17 @@ mod tests { ); assert_eq!(out.risky_ref(), &[1, 2, 3, 4, 5]); assert!( - out.capacity() >= LEAST, + out.risky_ref().capacity() >= LEAST, "the first growth is at least {LEAST}" ); let mut big = Buffer::with_capacity(100); big.extend_from_slice(&[7; 100]); big.reserve(1); - assert!(big.capacity() >= 200, "a buffer doubles its length"); + assert!( + big.risky_ref().capacity() >= 200, + "a buffer doubles its length" + ); let before = big.risky_ref().as_ptr(); big.reserve(10); @@ -238,24 +229,17 @@ mod tests { ); } - #[test] - fn clear_wipes_and_keeps_the_allocation() { - let mut out = Buffer::with_capacity(16); - out.extend_from_slice(b"secret"); - let before = out.risky_ref().as_ptr(); - out.clear(); - assert!(out.is_empty()); - assert!(core::ptr::eq(out.risky_ref().as_ptr(), before)); - assert_eq!(out.capacity(), 16); - } - #[test] fn zeroize_wipes_and_releases_the_allocation() { let mut out = Buffer::with_capacity(16); out.extend_from_slice(b"secret"); out.zeroize(); assert!(out.is_empty()); - assert_eq!(out.capacity(), 0, "nothing is left for the drop to wipe"); + assert_eq!( + out.risky_ref().capacity(), + 0, + "nothing is left for the drop to wipe" + ); } #[test] From fccc2fe931a6423e0e1baceb680e635f1682dc90 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 28 Oct 2026 11:41:48 +1100 Subject: [PATCH 16/25] refactor(aead): seal the scalar writer's output trait and name it ByteSink, so it is not an open extension point named Out --- packages/aead/src/leaf.rs | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/packages/aead/src/leaf.rs b/packages/aead/src/leaf.rs index 64476b05..08615bd9 100644 --- a/packages/aead/src/leaf.rs +++ b/packages/aead/src/leaf.rs @@ -88,12 +88,28 @@ const DECIMAL_RESERVED: u32 = 0x7F00_FFFF; /// Where [`write()`] puts a scalar's bytes: a `Vec` for a sealed leaf's /// plaintext, sized up front with [`max_len`] so it never grows, or a /// [`Buffer`] for a frame, which grows without leaving copies behind. -pub trait Out { +/// +/// It is sealed: a scalar is written only into these two, so the trait is +/// not a way to add a third, and can change without breaking anyone. +pub trait ByteSink: sealed::Sealed { + /// Append one byte. fn push(&mut self, byte: u8); + /// Append `bytes`. fn extend_from_slice(&mut self, bytes: &[u8]); } -impl Out for Vec { +mod sealed { + use vitaminc_protected::Buffer; + + /// Implemented only here, so [`ByteSink`](super::ByteSink) cannot be + /// implemented outside this crate. + pub trait Sealed {} + + impl Sealed for Vec {} + impl Sealed for Buffer {} +} + +impl ByteSink for Vec { fn push(&mut self, byte: u8) { Vec::push(self, byte); } @@ -102,7 +118,7 @@ impl Out for Vec { } } -impl Out for Buffer { +impl ByteSink for Buffer { fn push(&mut self, byte: u8) { Buffer::push(self, byte); } @@ -118,7 +134,7 @@ impl Out for Buffer { /// that call it for every scalar measured 10% to 15% slower with its body /// inlined into them than with a call. #[inline(never)] -pub fn write( +pub fn write( ty: Option, v: Physical<'_>, form: Form, @@ -323,7 +339,11 @@ fn length_prefix(form: Form, len: usize) -> Result, ScalarError> { } } -fn write_physical(v: Physical<'_>, form: Form, out: &mut O) -> Result<(), ScalarError> { +fn write_physical( + v: Physical<'_>, + form: Form, + out: &mut O, +) -> Result<(), ScalarError> { let mut variable = |tag, bytes: &[u8]| { out.push(tag); if let Some(len) = length_prefix(form, bytes.len())? { @@ -351,7 +371,7 @@ fn write_physical(v: Physical<'_>, form: Form, out: &mut O) -> Result<() Ok(()) } -fn write_int(i: Int, out: &mut O) { +fn write_int(i: Int, out: &mut O) { macro_rules! le { ($tag:ident, $v:expr) => {{ out.push(tags::$tag); From eec32dc2f44b567e6be18cad3e4ad5276b9a9a8b Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 28 Oct 2026 13:14:56 +1100 Subject: [PATCH 17/25] docs(protected): limit Buffer's no-copy promise to its own methods, since the Controlled methods that lend the Vec can grow it unwiped --- packages/protected/src/buffer.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/protected/src/buffer.rs b/packages/protected/src/buffer.rs index 1db76e88..4cf935ca 100644 --- a/packages/protected/src/buffer.rs +++ b/packages/protected/src/buffer.rs @@ -16,10 +16,20 @@ use crate::{locked::wipe_vec, private::ControlledPrivate, Controlled, OpaqueDebu /// It is a [`Controlled`] type, like [`Protected`](crate::Protected): its /// bytes are read through [`risky_ref`](Controlled::risky_ref) and taken /// through [`risky_unwrap`](Controlled::risky_unwrap), so every read of -/// what it holds is named at the use. Writing needs no such name: every -/// way to add bytes goes through [`reserve`], so there is no way to grow -/// it that leaves a copy behind, and [`overwrite`] patches bytes already -/// written without handing out a slice. It is not `Clone` and has no +/// what it holds is named at the use. Writing through `Buffer`'s own +/// methods needs no such name: each of them that adds bytes goes through +/// [`reserve`], so none of them grows it in a way that leaves a copy +/// behind, and [`overwrite`] patches bytes already written without +/// handing out a slice. +/// +/// The [`Controlled`] trait also has methods that lend out the `Vec` +/// itself, such as [`inner_mut`](Controlled::inner_mut) and +/// [`risky_inner_mut`](Controlled::risky_inner_mut). Those are escape +/// hatches: a `Vec` grown past its capacity through one of them frees its +/// old allocation unwiped, and that is the caller's risk, as it is with +/// [`risky_unwrap`](Controlled::risky_unwrap). +/// +/// It is not `Clone` and has no /// `PartialEq`: a copy or a comparison of secret bytes is a decision to /// make at the use, through `risky_ref`. Its `Debug` shows nothing. /// From e26d9075aaacc7be08d88a836cc19f8469a476e5 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 28 Oct 2026 13:20:51 +1100 Subject: [PATCH 18/25] test(protected): check that Buffer wipes the old block when it grows, so the guarantee is tested at the type that owns it --- packages/protected/tests/buffer_wipe.rs | 86 +++++++++++++++++++++---- 1 file changed, 75 insertions(+), 11 deletions(-) diff --git a/packages/protected/tests/buffer_wipe.rs b/packages/protected/tests/buffer_wipe.rs index 94da0d04..85542b8b 100644 --- a/packages/protected/tests/buffer_wipe.rs +++ b/packages/protected/tests/buffer_wipe.rs @@ -1,11 +1,14 @@ -//! Dropping a `Buffer` must wipe its allocation before it is freed. +//! A `Buffer` must wipe an allocation before it is freed: when it is +//! dropped, and when it grows out of one into a bigger one. //! -//! This test installs a global allocator that watches one allocation and -//! records whether every byte of it was zero when it was freed. It lives -//! in its own test binary so that allocator sees nothing but this test. +//! These tests install a global allocator that watches one allocation and +//! records whether every byte of it was zero when it was freed. They live +//! in their own test binary so that allocator sees nothing but them, and +//! they take turns, since there is one allocation under watch at a time. use std::alloc::{GlobalAlloc, Layout, System}; use std::sync::atomic::{AtomicBool, AtomicPtr, Ordering}; +use std::sync::{Mutex, MutexGuard}; use vitaminc_protected::{Buffer, Controlled}; @@ -15,6 +18,9 @@ static WATCHED: AtomicPtr = AtomicPtr::new(std::ptr::null_mut()); static FREED: AtomicBool = AtomicBool::new(false); static FREED_ZEROED: AtomicBool = AtomicBool::new(false); +/// Held by a test for as long as it watches an allocation. +static TURN: Mutex<()> = Mutex::new(()); + struct Watching; // SAFETY: every call is forwarded to `System` unchanged. `dealloc` only @@ -46,18 +52,76 @@ unsafe impl GlobalAlloc for Watching { #[global_allocator] static ALLOCATOR: Watching = Watching; -#[test] -fn dropping_a_buffer_frees_its_allocation_wiped() { - let mut out = Buffer::with_capacity(64); - out.extend_from_slice(b"a secret that must not outlive its buffer"); +/// Wait for this test's turn to watch an allocation. Taken before the +/// buffer is made, so the buffer is dropped before the turn passes on. +fn take_turn() -> MutexGuard<'static, ()> { + TURN.lock().unwrap_or_else(|poisoned| poisoned.into_inner()) +} + +/// Start watching the allocation `out` holds now. +fn watch(out: &Buffer) { + FREED.store(false, Ordering::SeqCst); + FREED_ZEROED.store(false, Ordering::SeqCst); WATCHED.store(out.risky_ref().as_ptr().cast_mut(), Ordering::SeqCst); - drop(out); +} + +/// Fail unless the watched allocation was freed, and freed all zeros. +fn assert_freed_wiped(when: &str) { assert!( FREED.load(Ordering::SeqCst), - "the buffer's allocation was never freed" + "the buffer's allocation was never freed {when}" ); assert!( FREED_ZEROED.load(Ordering::SeqCst), - "the buffer's allocation was freed with its bytes still in it" + "the buffer's allocation was freed with its bytes still in it {when}" + ); +} + +/// A buffer holding secret bytes up to its capacity, so the next byte +/// written makes it grow. +fn full_buffer() -> Buffer { + let mut out = Buffer::with_capacity(64); + let capacity = out.risky_ref().capacity(); + for i in 0..capacity { + out.push(0xA5 ^ i as u8); + } + assert_eq!( + out.risky_ref().capacity(), + capacity, + "filling the buffer to its capacity does not grow it" + ); + out +} + +#[test] +fn dropping_a_buffer_frees_its_allocation_wiped() { + let _turn = take_turn(); + let mut out = Buffer::with_capacity(64); + out.extend_from_slice(b"a secret that must not outlive its buffer"); + watch(&out); + drop(out); + assert_freed_wiped("when it was dropped"); +} + +#[test] +fn growing_by_push_frees_the_old_allocation_wiped() { + let _turn = take_turn(); + let mut out = full_buffer(); + watch(&out); + out.push(0x5A); + assert_freed_wiped("when a push made it grow"); + assert_eq!(out.risky_ref().last(), Some(&0x5A), "the byte was written"); +} + +#[test] +fn growing_by_extend_from_slice_frees_the_old_allocation_wiped() { + let _turn = take_turn(); + let mut out = full_buffer(); + watch(&out); + out.extend_from_slice(b"more secret"); + assert_freed_wiped("when extend_from_slice made it grow"); + assert!( + out.risky_ref().ends_with(b"more secret"), + "the bytes were written" ); } From 0fb174064c880ea36f479dfcfd2cae6595492f31 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 28 Oct 2026 13:47:05 +1100 Subject: [PATCH 19/25] fix(aead): check in debug builds that a Vec leaf sink was sized with max_len, and show presized wiping sinks in the examples, since a growing Vec frees plaintext unwiped --- packages/aead/src/leaf.rs | 72 +++++++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 21 deletions(-) diff --git a/packages/aead/src/leaf.rs b/packages/aead/src/leaf.rs index 08615bd9..6371b3e3 100644 --- a/packages/aead/src/leaf.rs +++ b/packages/aead/src/leaf.rs @@ -109,11 +109,26 @@ mod sealed { impl Sealed for Buffer {} } +/// A `Vec` is a sink for one sealed leaf's plaintext. Size it with +/// [`max_len`] before writing, so it never grows, and hold it in a wrapper +/// that wipes it, such as +/// [`Protected`](vitaminc_protected::Protected). A `Vec` that grows frees +/// its old allocation without wiping it, and a `Vec` on its own is not +/// wiped when it is dropped. A debug build checks that every write fits in +/// the room the `Vec` has left. impl ByteSink for Vec { fn push(&mut self, byte: u8) { + debug_assert!( + self.capacity() > self.len(), + "a Vec sink must be sized with max_len before it is written" + ); Vec::push(self, byte); } fn extend_from_slice(&mut self, bytes: &[u8]) { + debug_assert!( + self.capacity() - self.len() >= bytes.len(), + "a Vec sink must be sized with max_len before it is written" + ); Vec::extend_from_slice(self, bytes); } } @@ -205,15 +220,20 @@ pub fn write( /// A date, 11,017 days after 1970-01-01, under today's `DATE` tag. The /// payload is chrono's day count from the common era, little-endian. /// +/// The plaintext goes into a `Vec` sized with [`max_len`], so it never +/// grows and leaves a copy behind, held in a `Protected` so it is wiped +/// when dropped. +/// /// ``` -/// use vitaminc_aead::leaf::{read_leaf, write_leaf, ScalarError}; +/// use vitaminc_aead::leaf::{max_len, read_leaf, write_leaf, ScalarError}; /// use vitaminc_aead::plaintext::{Int, Logical, Physical}; +/// use vitaminc_protected::{Controlled, Protected}; /// /// let date = (Some(Logical::Date), Physical::Int(Int::I32(11_017))); -/// let mut out = Vec::new(); -/// write_leaf(date.0, date.1, &mut out)?; -/// assert_eq!(out, [0x12, 0x44, 0x24, 0x0B, 0x00]); -/// assert_eq!(read_leaf(&out)?, date); +/// let mut out = Protected::new(Vec::with_capacity(max_len(&date.1))); +/// write_leaf(date.0, date.1, out.risky_inner_mut())?; +/// assert_eq!(out.risky_ref(), &[0x12, 0x44, 0x24, 0x0B, 0x00]); +/// assert_eq!(read_leaf(out.risky_ref())?, date); /// # Ok::<(), ScalarError>(()) /// ``` pub fn write_leaf( @@ -234,15 +254,18 @@ pub fn write_leaf( /// A null date is as long as a date. /// /// ``` -/// use vitaminc_aead::leaf::{read_leaf_item, write_typed_null, Item, ScalarError}; +/// use vitaminc_aead::leaf::{ +/// read_leaf_item, typed_null_len, write_typed_null, Item, ScalarError, +/// }; /// use vitaminc_aead::plaintext::{FixedType, Logical, Width}; /// use vitaminc_aead::tags; +/// use vitaminc_protected::{Controlled, Protected}; /// /// let date = FixedType::new(Some(Logical::Date), Width::I32).expect("fixed"); -/// let mut out = Vec::new(); -/// write_typed_null(date, &mut out); -/// assert_eq!(out, [tags::TYPED_NULL, tags::DATE, 0, 0, 0]); -/// assert_eq!(read_leaf_item(&out)?, Item::TypedNull(date)); +/// let mut out = Protected::new(Vec::with_capacity(typed_null_len(date))); +/// write_typed_null(date, out.risky_inner_mut()); +/// assert_eq!(out.risky_ref(), &[tags::TYPED_NULL, tags::DATE, 0, 0, 0]); +/// assert_eq!(read_leaf_item(out.risky_ref())?, Item::TypedNull(date)); /// # Ok::<(), ScalarError>(()) /// ``` pub fn write_typed_null(ty: FixedType, out: &mut Vec) { @@ -418,12 +441,13 @@ pub enum Item<'a> { /// use vitaminc_aead::leaf::{read, write, Form, ScalarError}; /// use vitaminc_aead::plaintext::Physical; /// use vitaminc_aead::tags; +/// use vitaminc_protected::{Buffer, Controlled}; /// -/// let mut frame = Vec::new(); +/// let mut frame = Buffer::new(); /// write(None, Physical::Text("hi"), Form::Frame, &mut frame)?; /// let written = frame.len(); /// frame.push(tags::NULL); // the next node -/// let ((ty, value), used) = read(&frame, Form::Frame)?; +/// let ((ty, value), used) = read(frame.risky_ref(), Form::Frame)?; /// assert_eq!((ty, value, used), (None, Physical::Text("hi"), written)); /// # Ok::<(), ScalarError>(()) /// ``` @@ -689,7 +713,9 @@ mod tests { Physical::Float(Float::F64(1.5)), ] { for form in [Form::Leaf, Form::Frame] { - let mut out = Vec::new(); + // A debug build checks each write against the room, so + // this also checks that no write passes `max_len`. + let mut out = Vec::with_capacity(max_len(&v)); write(ty, v, form, &mut out).expect("writes"); assert!(out.len() <= max_len(&v), "{v:?}"); } @@ -697,13 +723,13 @@ mod tests { } fn leaf(ty: Option, v: Physical<'_>) -> Vec { - let mut out = Vec::new(); + let mut out = Vec::with_capacity(max_len(&v)); write_leaf(ty, v, &mut out).expect("writes"); out } fn frame(ty: Option, v: Physical<'_>) -> Vec { - let mut out = Vec::new(); + let mut out = Vec::with_capacity(max_len(&v)); write(ty, v, Form::Frame, &mut out).expect("writes"); out } @@ -967,7 +993,7 @@ mod tests { // A leaf has no prefix, so no limit. assert_eq!(length_prefix(Form::Leaf, u32::MAX as usize + 1), Ok(None)); } - let mut out = Vec::new(); + let mut out = Vec::with_capacity(max_len(&Physical::Text(""))); assert!(write(None, Physical::Text(""), Form::Frame, &mut out).is_ok()); assert_eq!(out, [0x0A, 0, 0, 0, 0]); } @@ -1017,7 +1043,7 @@ mod tests { } let ((ty, value), used) = read(&bytes, Form::Frame).expect(name); assert_eq!(used, bytes.len(), "{name}"); - let mut out = Vec::new(); + let mut out = Vec::with_capacity(max_len(&value)); write(ty, value, Form::Frame, &mut out).expect(name); assert_eq!(out, bytes, "{name}"); checked += 1; @@ -1036,8 +1062,12 @@ mod tests { assert_eq!(used, bytes.len(), "{name}"); let mut out = Vec::new(); match item { - Item::Scalar(ty, value) => write(ty, value, Form::Frame, &mut out).expect(name), + Item::Scalar(ty, value) => { + out.reserve(max_len(&value)); + write(ty, value, Form::Frame, &mut out).expect(name); + } Item::TypedNull(ty) => { + out.reserve(max_len(&Physical::Null)); write(None, Physical::Null, Form::Frame, &mut out).expect(name); let mut typed = Vec::new(); write_typed_null(ty, &mut typed); @@ -1140,7 +1170,7 @@ mod tests { .all(|form| match read(bytes, form) { Err(_) => true, Ok(((ty, v), used)) => { - let mut out = Vec::new(); + let mut out = Vec::with_capacity(max_len(&v)); write(ty, v, form, &mut out).is_ok() && (out == bytes[..used] || is_negative_zero_decimal(&bytes[..used])) } @@ -1184,7 +1214,7 @@ mod tests { ]; cases.into_iter().all(|(ty, int)| { [Form::Frame, Form::Leaf].into_iter().all(|form| { - let mut bytes = Vec::new(); + let mut bytes = Vec::with_capacity(max_len(&Physical::Int(int))); write(Some(ty), Physical::Int(int), form, &mut bytes).expect("writes"); let read_back = read(&bytes, form).is_ok_and(|((t, v), used)| { t == Some(ty) @@ -1467,7 +1497,7 @@ mod tests { let Some(fixed) = FixedType::new(ty, width) else { continue; }; - let mut out = Vec::new(); + let mut out = Vec::with_capacity(max_len(&width.zero())); assert!( write_leaf(fixed.logical(), width.zero(), &mut out).is_ok(), "{fixed:?}" From 0ce43462451d8c53197d72cc93cb9b43bb28b920 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 28 Oct 2026 13:48:38 +1100 Subject: [PATCH 20/25] docs(aead-frame): say that the writer's wipes run on drop, so a panic under panic=abort leaves a partly written blob unwiped --- packages/aead-frame/STORAGE.md | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/aead-frame/STORAGE.md b/packages/aead-frame/STORAGE.md index ac53709f..593e399c 100644 --- a/packages/aead-frame/STORAGE.md +++ b/packages/aead-frame/STORAGE.md @@ -393,12 +393,15 @@ choice, and needs nothing from this profile. The writer grows its buffer into a fresh allocation and wipes the old one, formats a `Display` value straight into the blob, keeps no copy of a key, -and wipes everything if writing fails. The blob it returns is a `Buffer`: -its bytes are read through `risky_ref`, and it is wiped when dropped. The -reader borrows text and bytes from the blob rather than copying them, and -finds a repeated map key by comparing keys where they lie in the blob. The -writer frees no heap block that still holds the value, and a zeroize audit -checked in LLVM IR and assembly that every wipe survives optimization. +and wipes everything if writing fails. The wipes run when a `Buffer` is +dropped, so under `panic = "unwind"` a panic still wipes a partly written +blob, and under `panic = "abort"` nothing is wiped when the process aborts. +The blob it returns is a `Buffer`: its bytes are read through `risky_ref`, +and it is wiped when dropped. The reader borrows text and bytes from the +blob rather than copying them, and finds a repeated map key by comparing +keys where they lie in the blob. The writer frees no heap block that still +holds the value, and a zeroize audit checked in LLVM IR and assembly that +every wipe survives optimization. Copies on the stack and in registers are not wiped: From 6f983f9d0da18cd645f9d409f685c1518b6a8abc Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 28 Oct 2026 22:26:05 +1100 Subject: [PATCH 21/25] perf(protected): tell the compiler reserve made room, so Vec's own reallocating growth is compiled out of Buffer's push paths After grow, which the compiler does not look into, Vec's own growth code stayed compiled into push and extend_from_slice in every writer. It could never run, because grow always makes room, but if it ever did it would realloc and free the old allocation unwiped. An assert_unchecked on the spare capacity after reserve lets the compiler drop it: every function that called grow also called RawVec::grow_one or do_reserve_and_handle before (8 of 10 in vitaminc-aead, 6 of 21 in vitaminc-aead-frame) and none does now, with or without fat LTO. A debug_assert of the same condition checks it in debug and Miri builds. Measured: the storage writer is unchanged; the transport map_1000 write is about 6% slower, accepted for the hardening. --- packages/protected/src/buffer.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/protected/src/buffer.rs b/packages/protected/src/buffer.rs index 4cf935ca..31242623 100644 --- a/packages/protected/src/buffer.rs +++ b/packages/protected/src/buffer.rs @@ -95,6 +95,17 @@ impl Buffer { if self.0.capacity() - self.0.len() < additional { self.grow(additional); } + // `grow` is a call the compiler does not look into, so without + // this it cannot tell that there is room now, and it keeps `Vec`'s + // own growth in `push` and `extend_from_slice`. That growth would + // free the old allocation without wiping it, if it ever ran. + // Telling the compiler there is room lets it drop that code. + debug_assert!(self.0.len() + additional <= self.0.capacity()); + // SAFETY: either the check above found the room, or `grow` made a + // new allocation of at least `len + additional` bytes (its + // capacity is the largest of that, twice the length and `LEAST`, + // and `Vec::with_capacity` panics rather than allocate less). + unsafe { core::hint::assert_unchecked(self.0.len() + additional <= self.0.capacity()) } } #[cold] From 5e67c17fac5b1458aa5d9912b69a75ec1ec03609 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Mon, 2 Nov 2026 12:56:39 +1100 Subject: [PATCH 22/25] test(aead): pin the debug checks on a Vec leaf sink and document their panic, so removing one fails a test and a caller with an unsized Vec knows why it panics --- packages/aead/src/leaf.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/packages/aead/src/leaf.rs b/packages/aead/src/leaf.rs index 6371b3e3..7c780c0d 100644 --- a/packages/aead/src/leaf.rs +++ b/packages/aead/src/leaf.rs @@ -148,6 +148,13 @@ impl ByteSink for Buffer { /// Compiled once for each `O` and called, never inlined: the frame writers /// that call it for every scalar measured 10% to 15% slower with its body /// inlined into them than with a call. +/// +/// # Panics +/// +/// In a debug build, panics if `out` is a `Vec` without room for every +/// byte the scalar takes. Size it with [`max_len`] first. A release build +/// does not check, and lets the `Vec` grow, which frees its old allocation +/// without wiping it. A [`Buffer`] needs no sizing: it wipes as it grows. #[inline(never)] pub fn write( ty: Option, @@ -236,6 +243,13 @@ pub fn write( /// assert_eq!(read_leaf(out.risky_ref())?, date); /// # Ok::<(), ScalarError>(()) /// ``` +/// +/// # Panics +/// +/// In a debug build, panics if `out` has no room for every byte the +/// plaintext takes. Size it with [`max_len`] first. A release build does +/// not check, and lets the `Vec` grow, which frees its old allocation +/// without wiping it. pub fn write_leaf( ty: Option, v: Physical<'_>, @@ -722,6 +736,25 @@ mod tests { } } + // A `Vec` that grows frees its old allocation unwiped, so a debug + // build refuses to write past the room it was sized with. These pin + // both checks, so neither can be removed unnoticed. + #[test] + #[cfg(debug_assertions)] + #[should_panic(expected = "a Vec sink must be sized with max_len")] + fn a_vec_sink_with_no_room_for_a_byte_panics_in_a_debug_build() { + let _ = write(None, Physical::Null, Form::Frame, &mut Vec::new()); + } + + #[test] + #[cfg(debug_assertions)] + #[should_panic(expected = "a Vec sink must be sized with max_len")] + fn a_vec_sink_with_no_room_for_a_slice_panics_in_a_debug_build() { + // Room for the tag, which `push` writes, but not for the text. + let mut out = Vec::with_capacity(1); + let _ = write_leaf(None, Physical::Text("text"), &mut out); + } + fn leaf(ty: Option, v: Physical<'_>) -> Vec { let mut out = Vec::with_capacity(max_len(&v)); write_leaf(ty, v, &mut out).expect("writes"); From 0b1a5fc3426812a1a50d936cfd57792a60bbd90f Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Mon, 2 Nov 2026 12:57:21 +1100 Subject: [PATCH 23/25] test(protected): check that Buffer::reserve panics when it cannot allocate the room, since the room it then promises the compiler rests on that panic --- packages/protected/src/buffer.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/protected/src/buffer.rs b/packages/protected/src/buffer.rs index 31242623..e255e328 100644 --- a/packages/protected/src/buffer.rs +++ b/packages/protected/src/buffer.rs @@ -250,6 +250,17 @@ mod tests { ); } + // `reserve` tells the compiler there is room once `grow` returns, which + // is sound only because `grow` panics when it cannot make that room. If + // it ever returned instead, `push` would write past the allocation. + #[test] + #[should_panic(expected = "capacity overflow")] + fn reserve_panics_when_the_room_cannot_be_allocated() { + let mut out = Buffer::new(); + out.push(1); + out.reserve(usize::MAX); + } + #[test] fn zeroize_wipes_and_releases_the_allocation() { let mut out = Buffer::with_capacity(16); From efac5e48a43cf62519f5c2638e70e83fbe0a6139 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Mon, 2 Nov 2026 12:57:34 +1100 Subject: [PATCH 24/25] refactor(aead): hide SeenKeys from the docs, since only vitaminc-aead-frame uses it and it should not read as an API to keep stable --- packages/aead/src/frame.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/aead/src/frame.rs b/packages/aead/src/frame.rs index 8c4f28d0..bb738203 100644 --- a/packages/aead/src/frame.rs +++ b/packages/aead/src/frame.rs @@ -170,6 +170,10 @@ const FEW_KEYS: usize = 16; /// pay for. Past sixteen they are hashed, so an object a host sends /// with thousands of keys is still checked in linear time. The keys are /// borrowed from the frame, so nothing is copied. +/// +/// Not an API: it is public only so `vitaminc-aead-frame`'s storage reader +/// can share this one copy, and it may change in any release. +#[doc(hidden)] #[derive(Default)] pub struct SeenKeys<'a> { few: [&'a str; FEW_KEYS], From bd45c10ab768ca61d9d4c34338007610eaaf1849 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Mon, 2 Nov 2026 14:54:12 +1100 Subject: [PATCH 25/25] refactor(aead): move SeenKeys into frame::__private, so its path says it is shared with vitaminc-aead-frame and is not an API --- packages/aead-frame/src/storage.rs | 3 +- packages/aead/src/frame.rs | 124 ++++++++++++++++------------- 2 files changed, 70 insertions(+), 57 deletions(-) diff --git a/packages/aead-frame/src/storage.rs b/packages/aead-frame/src/storage.rs index b09913d6..703e4742 100644 --- a/packages/aead-frame/src/storage.rs +++ b/packages/aead-frame/src/storage.rs @@ -40,7 +40,8 @@ use std::ops::Range; use ::serde::ser::{self, Serialize}; use ::serde::Deserialize; -use vitaminc_aead::frame::{FrameWriteError, SeenKeys, ARRAY, MAX_DEPTH, OBJECT}; +use vitaminc_aead::frame::__private::SeenKeys; +use vitaminc_aead::frame::{FrameWriteError, ARRAY, MAX_DEPTH, OBJECT}; use vitaminc_aead::leaf::{self, Form, ScalarError}; use vitaminc_aead::plaintext::{Float, Int, Physical}; use vitaminc_aead::tags; diff --git a/packages/aead/src/frame.rs b/packages/aead/src/frame.rs index bb738203..0cf163c8 100644 --- a/packages/aead/src/frame.rs +++ b/packages/aead/src/frame.rs @@ -16,11 +16,12 @@ //! [`PassthroughFrame`] is the owned frame a ciphertext carries for a //! passthrough subtree. -use std::collections::HashSet; use std::ops::Range; use vitaminc_protected::{Buffer, Controlled, OpaqueDebug, TimingSafeEq}; +use __private::SeenKeys; + use crate::leaf::{self, Form, Item, ScalarError}; use crate::plaintext::{ Encrypt, Error, FixedType, Logical, MapSink, Physical, SeqSink, Sink, SourceError, @@ -160,45 +161,76 @@ impl<'a> Cursor<'a> { } } -/// How many keys an object may have before [`SeenKeys`] hashes them. -const FEW_KEYS: usize = 16; - -/// The keys an object has given so far, to refuse a repeat. -/// -/// The keys are compared one by one while there are few: an object -/// rarely has more fields than a hash set's allocation and hashing would -/// pay for. Past sixteen they are hashed, so an object a host sends -/// with thousands of keys is still checked in linear time. The keys are -/// borrowed from the frame, so nothing is copied. -/// -/// Not an API: it is public only so `vitaminc-aead-frame`'s storage reader -/// can share this one copy, and it may change in any release. +/// What `vitaminc-aead-frame` shares with this crate by path. Not an API: +/// anything here may change in any release. #[doc(hidden)] -#[derive(Default)] -pub struct SeenKeys<'a> { - few: [&'a str; FEW_KEYS], - len: usize, - many: Option>, -} +pub mod __private { + use std::collections::HashSet; -impl<'a> SeenKeys<'a> { - /// Record `key`; whether it was new. - pub fn insert(&mut self, key: &'a str) -> bool { - if let Some(many) = &mut self.many { - return many.insert(key); - } - if self.few[..self.len].contains(&key) { - return false; + /// How many keys an object may have before `SeenKeys` hashes them. + const FEW_KEYS: usize = 16; + + /// The keys an object has given so far, to refuse a repeat. + /// + /// The keys are compared one by one while there are few: an object + /// rarely has more fields than a hash set's allocation and hashing would + /// pay for. Past sixteen they are hashed, so an object a host sends + /// with thousands of keys is still checked in linear time. The keys are + /// borrowed from the frame, so nothing is copied. + /// + /// Not an API: it is reachable only so `vitaminc-aead-frame`'s storage + /// reader can share this one copy, and it may change in any release. + #[derive(Default)] + pub struct SeenKeys<'a> { + few: [&'a str; FEW_KEYS], + len: usize, + many: Option>, + } + + impl<'a> SeenKeys<'a> { + /// Record `key`; whether it was new. + pub fn insert(&mut self, key: &'a str) -> bool { + if let Some(many) = &mut self.many { + return many.insert(key); + } + if self.few[..self.len].contains(&key) { + return false; + } + if self.len < FEW_KEYS { + self.few[self.len] = key; + self.len += 1; + } else { + let mut many: HashSet<&'a str> = self.few.iter().copied().collect(); + many.insert(key); + self.many = Some(many); + } + true } - if self.len < FEW_KEYS { - self.few[self.len] = key; - self.len += 1; - } else { - let mut many: HashSet<&'a str> = self.few.iter().copied().collect(); - many.insert(key); - self.many = Some(many); + } + + #[cfg(test)] + mod tests { + use super::*; + + #[test] + fn seen_keys_find_a_repeat_before_and_after_they_spill_to_a_set() { + let names: Vec = (0..40).map(|i| format!("k{i}")).collect(); + let mut seen = SeenKeys::default(); + for (i, name) in names.iter().enumerate() { + assert!(seen.insert(name), "key {i} is new"); + } + assert_eq!(seen.len, FEW_KEYS, "the first keys stay inline"); + assert!(seen.many.is_some(), "the rest spilled to a set"); + assert!(!seen.insert(&names[0]), "an inline key repeats"); + assert!(!seen.insert(&names[39]), "a spilled key repeats"); + assert!(seen.insert("new"), "a fresh key is still new"); + + let mut few = SeenKeys::default(); + assert!(few.insert("a")); + assert!(few.insert("b")); + assert!(!few.insert("a"), "a repeat is found without a set"); + assert!(few.many.is_none()); } - true } } @@ -627,26 +659,6 @@ mod tests { to_frame(p).expect("frames").risky_ref().to_vec() } - #[test] - fn seen_keys_find_a_repeat_before_and_after_they_spill_to_a_set() { - let names: Vec = (0..40).map(|i| format!("k{i}")).collect(); - let mut seen = SeenKeys::default(); - for (i, name) in names.iter().enumerate() { - assert!(seen.insert(name), "key {i} is new"); - } - assert_eq!(seen.len, FEW_KEYS, "the first keys stay inline"); - assert!(seen.many.is_some(), "the rest spilled to a set"); - assert!(!seen.insert(&names[0]), "an inline key repeats"); - assert!(!seen.insert(&names[39]), "a spilled key repeats"); - assert!(seen.insert("new"), "a fresh key is still new"); - - let mut few = SeenKeys::default(); - assert!(few.insert("a")); - assert!(few.insert("b")); - assert!(!few.insert("a"), "a repeat is found without a set"); - assert!(few.many.is_none()); - } - #[test] fn a_frame_round_trips_through_write_and_walk() { let mut value = BTreeMap::new();