From 23447e580a27cc4010bd2ae158e3b24452335b4d Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 31 Oct 2026 10:36:44 +0000 Subject: [PATCH 01/10] perf(byte-storage): decode the envelope store writes in one pass on retrieve retrieve and validate walked every envelope twice: the structural pre-scan, then rmp_serde, which reads the 8-byte checksum element by element through serde. At 40 B that was most of a read's cost. decode_envelope now first tries a one-pass decode of the exact shape rmp_serde writes: a 4-element fixarray holding a bin8/16/32 payload, a fixarray of 8 checksum bytes (positive fixint or uint8), original_size as a uint of at most 32 bits, and format as a fixstr or str8 of valid UTF-8. It checks each declared length against the bytes after it before reading them and allocates nothing until the whole envelope has been read, so it holds the same decode bounds the pre-scan enforces. Any other input, and every input it would reject, goes to the pre-scan and rmp_serde as before, so the read accepts and rejects exactly what it did, with the same errors, and still ignores bytes after the envelope. one_pass_decode_matches_full_decode checks that on the decode corpus, the decode-bounds vectors and every truncation and single-byte edit of the canonical envelopes at each width boundary. byte_storage_corrupted_envelope becomes a differential fuzz target over raw and corrupted stored envelopes, through a hook built only under cfg(fuzzing). perf_ir, Ir/op: retrieve/64 2,552 -> 1,083 (-57.6%), /1024 7,323 -> 5,705 (-22.1%), /65536 391,697 -> 390,158 (-0.4%); every other case unchanged. --- Cargo.toml | 5 +- README.md | 20 +- SECURITY.md | 16 +- benches/perf_ir.rs | 2 +- fuzz/README.md | 4 +- .../byte_storage_corrupted_envelope.rs | 78 +++-- src/byte_storage.rs | 312 +++++++++++++++++- 7 files changed, 390 insertions(+), 47 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 45f3980..794fd1e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -149,8 +149,9 @@ stubbing = false # Lint configuration [lints.rust] -# Suppress benign warnings about cfg(kani) which is set by Kani verifier -unexpected_cfgs = { level = "warn", check-cfg = ['cfg(kani)'] } +# Suppress benign warnings about cfg(kani), set by the Kani verifier, and +# cfg(fuzzing), set by cargo-fuzz +unexpected_cfgs = { level = "warn", check-cfg = ['cfg(kani)', 'cfg(fuzzing)'] } # Release profile for cachekit-core's own release builds. # diff --git a/README.md b/README.md index 7640479..770dce2 100644 --- a/README.md +++ b/README.md @@ -254,13 +254,17 @@ Malicious payloads claiming `original_size: 500GB` with 100 bytes of data are re
Envelope Decode Bounds -`retrieve()` and `validate()` run a header-only structural pre-scan over the -envelope bytes **before** MessagePack decoding: nesting deeper than 100 levels, -headers declaring more elements or bytes than the input can back, the reserved -marker `0xc1` and truncated input are all rejected before decoding, without -allocating in proportion to any declared length. A rejection is -`ByteStorageError::DeserializationFailed` with the message prefix -`decode pre-scan: `. The same walk is public as +`retrieve()` and `validate()` read an envelope in the shape `store()` writes in +one pass, which checks each declared length against the input before reading it +and allocates nothing until the whole envelope has been read. Every other input +gets a header-only structural pre-scan **before** MessagePack decoding: nesting +deeper than 100 levels, headers declaring more elements or bytes than the input +can back, the reserved marker `0xc1` and truncated input are all rejected +before decoding, without allocating in proportion to any declared length. A +rejection is `ByteStorageError::DeserializationFailed` with the message prefix +`decode pre-scan: `. The one-pass read accepts only envelopes the pre-scan and +the typed decode accept, into the same fields, so every input gets the result +and error it got before. The same walk is public as `check_msgpack_structure(bytes, max_depth)` for callers that decode untrusted MessagePack themselves; it returns a `MsgpackStructureError` whose `Display` is the bare reason, with no prefix. `msgpack_document_len(bytes, max_depth)` runs @@ -355,7 +359,7 @@ The Criterion suite in `benches/hot_path.rs` needs the `encryption` feature (`ma | Case | What one call does | |:-----|:-------------------| | `store/`, `retrieve/` | `ByteStorage::store` / `retrieve` on the realistic msgpack payload `hot_path` uses | -| `prescan/` | `check_msgpack_structure` on the envelope, the pre-scan `retrieve` runs first | +| `prescan/` | `check_msgpack_structure` on the envelope, the pre-scan `retrieve` runs on any envelope not in the shape `store` writes | | `encrypt/`, `decrypt/` | `ZeroKnowledgeEncryptor` AES-256-GCM | | `keyring_decrypt/` | `Keyring::decrypt_indexed`, which re-derives the tenant key (HKDF) on every call | | `tenant_keyring_decrypt/` | `TenantKeyring::decrypt_indexed`, key derived once at `for_tenant` | diff --git a/SECURITY.md b/SECURITY.md index 3805bbf..c1593ef 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -176,7 +176,21 @@ cover only those two proofs. come from a backend the caller may not control. Before `rmp_serde` materialises a `StorageEnvelope`, both run a header-only structural pre-scan over those bytes (protocol `spec/wire-format.md` → Retrieve Flow, step 2; the bounds themselves -are `spec/interop-mode.md` → Decode bounds). The pre-scan rejects: +are `spec/interop-mode.md` → Decode bounds). + +An envelope in the shape `store` writes skips the pre-scan and `rmp_serde`: a +one-pass decode reads its four fields in their exact encodings (a 4-element +fixarray; bin8/16/32; a fixarray of 8 checksum bytes; a uint of at most 32 +bits; a fixstr or str8 of valid UTF-8). It holds the same bounds. It reads two +levels deep, checks every declared length against the bytes after it before +reading them, and allocates nothing until it has read the whole envelope; a +complete envelope always declares fewer slots than its length. It hands any +other input, and every input it would reject, to the pre-scan and `rmp_serde`, +so it accepts only what they accept, into the same fields, and every rejection +still carries the error below. `one_pass_decode_matches_full_decode` and the +`byte_storage_corrupted_envelope` fuzz target compare the two decodes. + +The pre-scan rejects: | Rule | Bound | |:-----|:------| diff --git a/benches/perf_ir.rs b/benches/perf_ir.rs index 679b20e..72f8626 100644 --- a/benches/perf_ir.rs +++ b/benches/perf_ir.rs @@ -76,7 +76,7 @@ const TENANT: &str = "tenant-123"; /// Shaped like an SDK's AAD: version byte, then length-prefixed tenant id and /// cache key. GHASH cost depends on its length, not its bytes. const AAD: &[u8] = b"\x03\x00\x00\x00\x0atenant-123\x00\x00\x00\x10ns:bench:func:x"; -/// ByteStorage's envelope nesting bound (`decode_envelope`). +/// ByteStorage's envelope nesting bound (`decode_envelope_full`). const ENVELOPE_MAX_DEPTH: usize = 100; fn case_ids() -> Vec { diff --git a/fuzz/README.md b/fuzz/README.md index 239c5a6..ff54b96 100644 --- a/fuzz/README.md +++ b/fuzz/README.md @@ -46,8 +46,8 @@ For a coverage report, see [Coverage Reporting](#coverage-reporting). **byte_storage_corrupted_envelope.rs** - Attack: Malformed MessagePack `StorageEnvelope` deserialization -- Validates: Safe rejection of corrupted serialization without panics -- Tests: Invalid MessagePack structures, truncated data, type confusion +- Validates: Safe rejection of corrupted serialization without panics, and that the one-pass envelope decode `retrieve` tries first agrees with the pre-scan plus `rmp_serde` decode on accept, reject and every decoded field +- Tests: Invalid MessagePack structures, truncated data, type confusion, and envelopes `store` writes with bytes replaced, inserted or cut off **byte_storage_integer_overflow.rs** - Attack: Decompression bomb via oversized `original_size` (u32::MAX, boundary cases) diff --git a/fuzz/fuzz_targets/byte_storage_corrupted_envelope.rs b/fuzz/fuzz_targets/byte_storage_corrupted_envelope.rs index 01bc171..2016d0c 100644 --- a/fuzz/fuzz_targets/byte_storage_corrupted_envelope.rs +++ b/fuzz/fuzz_targets/byte_storage_corrupted_envelope.rs @@ -1,34 +1,62 @@ #![no_main] +use cachekit_core::byte_storage::{fuzz_envelope_decodes, StorageEnvelope}; +use cachekit_core::ByteStorage; use libfuzzer_sys::fuzz_target; -use cachekit_core::byte_storage::StorageEnvelope; + +// Attack: malformed and near-canonical envelopes through the envelope decode. +// Validates, besides fail-closed rejection without panics, that the one-pass +// decode `retrieve` tries first agrees with the pre-scan plus serde decode it +// falls back to: it accepts only what that decode accepts, into the same +// fields, and `retrieve` returns what that decode would give it, error text +// included. fuzz_target!(|data: &[u8]| { - // Attack: Attempt to deserialize arbitrary bytes as StorageEnvelope - // Validates: Safe rejection of corrupted MessagePack serialization without panics + // The input as envelope bytes. + check(data); - // Try to deserialize fuzz input as MessagePack StorageEnvelope - match rmp_serde::from_slice::(data) { - Ok(envelope) => { - // Successfully deserialized - now test extract() validation - // extract() should either succeed (valid envelope) or reject safely (invalid) - match envelope.extract() { - Ok(_decompressed) => { - // Valid envelope and decompression succeeded - this is acceptable - // (rare but possible if fuzzer generates valid structure) - } - Err(_) => { - // Envelope validation or decompression failed - expected for most malformed input - // This is the fail-closed behavior we want - } - } - } - Err(_) => { - // Deserialization failed - expected for invalid MessagePack - // This is also safe fail-closed behavior + // The input as an envelope `store` writes, then corrupted, which reaches + // the one-pass decode's boundaries far more often than raw bytes do. The + // first byte picks up to three 3-byte edits (a position and a byte) and + // what they do; the rest is the stored value. + let Some((&plan, rest)) = data.split_first() else { + return; + }; + let edit_bytes = usize::from(plan % 4) * 3; + let Some((edits, value)) = rest.split_at_checked(edit_bytes) else { + return; + }; + let mut envelope = ByteStorage::new(None).store(value, None).unwrap(); + for edit in edits.chunks_exact(3) { + // Up to one past the end, so an edit can also append. + let pos = usize::from(u16::from_be_bytes([edit[0], edit[1]])) % (envelope.len() + 1); + match plan >> 2 & 3 { + 0 if pos < envelope.len() => envelope[pos] = edit[2], + 1 => envelope.truncate(pos), + _ => envelope.insert(pos, edit[2]), } } - - // Success: No panics occurred, all paths handled safely - // Invariant: Malformed MessagePack must be rejected without crashes + check(&envelope); }); + +fn check(bytes: &[u8]) { + let (one_pass, full, decoded) = fuzz_envelope_decodes(bytes); + match (&one_pass, &full) { + (Some(one), Ok(full)) => assert_eq!(one, full, "one-pass and full decodes differ"), + (Some(_), Err(e)) => panic!("one-pass decode accepted what the full decode rejects: {e}"), + _ => {} + } + assert_eq!(decoded, full, "decode_envelope differs from the full decode"); + + // `retrieve` is the full decode's fields, extracted. + let expected = full.and_then(|(compressed_data, checksum, original_size, format)| { + let envelope = StorageEnvelope { + compressed_data, + checksum, + original_size, + format, + }; + envelope.extract().map(|data| (data, envelope.format)) + }); + assert_eq!(ByteStorage::new(None).retrieve(bytes), expected); +} diff --git a/src/byte_storage.rs b/src/byte_storage.rs index 80c8305..67ced1c 100644 --- a/src/byte_storage.rs +++ b/src/byte_storage.rs @@ -479,12 +479,14 @@ impl ByteStorage { /// /// # Errors /// - /// `envelope_bytes` is untrusted. Before it is decoded, a structural - /// pre-scan bounds its nesting depth and rejects any header that declares - /// more than the input can back (protocol Retrieve Flow, step 2). A - /// pre-scan rejection is `DeserializationFailed` whose message starts with - /// `decode pre-scan: `, the same variant as any other bytes that do not - /// decode as a `StorageEnvelope`. + /// `envelope_bytes` is untrusted. An envelope in the shape `store` writes + /// is decoded in one pass that checks each declared length against the + /// input before reading it. Any other input is decoded only after a + /// structural pre-scan bounds its nesting depth and rejects any header + /// that declares more than the input can back (protocol Retrieve Flow, + /// step 2). A pre-scan rejection is `DeserializationFailed` whose message + /// starts with `decode pre-scan: `, the same variant as any other bytes + /// that do not decode as a `StorageEnvelope`. #[cfg(all(feature = "compression", feature = "checksum", feature = "messagepack"))] pub fn retrieve(&self, envelope_bytes: &[u8]) -> Result<(Vec, String), ByteStorageError> { // Security: Check envelope size before deserializing @@ -552,11 +554,29 @@ impl Default for ByteStorage { } } +/// Decode untrusted envelope bytes: the one-pass decode of the shape `store` +/// writes, and for any input it declines, `decode_envelope_full`. +/// +/// The one-pass decode accepts only inputs the full decode accepts, into the +/// same fields, and declines every input the full decode rejects, so this +/// returns what `decode_envelope_full` returns, error text included +/// (`one_pass_decode_matches_full_decode`). It walks the envelope once instead +/// of twice, which at small sizes is most of the cost of a read. +#[cfg(all(feature = "compression", feature = "checksum", feature = "messagepack"))] +fn decode_envelope( + envelope_bytes: &[u8], +) -> Result, ByteStorageError> { + match borrowed::StorageEnvelope::decode_canonical(envelope_bytes) { + Some(envelope) => Ok(envelope), + None => decode_envelope_full(envelope_bytes), + } +} + /// Decode untrusted envelope bytes: structural pre-scan first, then the typed /// decode. Serde's derive skips an unknown map key with `IgnoredAny`, which /// recurses, so the depth bound has to hold before `rmp_serde` sees the bytes. #[cfg(all(feature = "compression", feature = "checksum", feature = "messagepack"))] -fn decode_envelope( +fn decode_envelope_full( envelope_bytes: &[u8], ) -> Result, ByteStorageError> { // Nesting bound for the envelope decode. The protocol requires 32..=1024; @@ -569,6 +589,42 @@ fn decode_envelope( .map_err(|e| ByteStorageError::DeserializationFailed(e.to_string())) } +/// A decoded envelope's `(compressed_data, checksum, original_size, format)`. +#[cfg(all( + any(test, fuzzing), + feature = "compression", + feature = "checksum", + feature = "messagepack" +))] +#[doc(hidden)] +pub type EnvelopeFields = (Vec, [u8; 8], u32, String); + +/// The same bytes through the one-pass decode, `decode_envelope_full` and +/// `decode_envelope`, for the `byte_storage_corrupted_envelope` fuzz target's +/// differential. Built only under `cfg(fuzzing)`, which cargo-fuzz sets. +#[cfg(all( + fuzzing, + feature = "compression", + feature = "checksum", + feature = "messagepack" +))] +#[doc(hidden)] +#[allow(clippy::type_complexity)] +pub fn fuzz_envelope_decodes( + envelope_bytes: &[u8], +) -> ( + Option, + Result, + Result, +) { + ( + borrowed::StorageEnvelope::decode_canonical(envelope_bytes) + .map(borrowed::StorageEnvelope::into_fields), + decode_envelope_full(envelope_bytes).map(borrowed::StorageEnvelope::into_fields), + decode_envelope(envelope_bytes).map(borrowed::StorageEnvelope::into_fields), + ) +} + /// What `retrieve` and `validate` decode: `StorageEnvelope` with /// `compressed_data` borrowed from the input instead of copied out of it, /// below `LARGE_PAYLOAD`. @@ -597,11 +653,109 @@ mod borrowed { pub(super) format: String, } - impl StorageEnvelope<'_> { + impl<'a> StorageEnvelope<'a> { /// `StorageEnvelope::extract`, on the borrowed payload. pub(super) fn extract(&self) -> Result, ByteStorageError> { extract_payload(&self.compressed_data, &self.checksum, self.original_size) } + + /// The fields, owned, for the differential tests and fuzz target. + #[cfg(any(test, fuzzing))] + pub(super) fn into_fields(self) -> super::EnvelopeFields { + let Self { + compressed_data, + checksum, + original_size, + format, + } = self; + ( + compressed_data.into_owned(), + checksum, + original_size, + format, + ) + } + + /// One pass over the shape `rmp_serde` writes for every envelope: a + /// fixarray of 4 holding `compressed_data` as bin8, bin16 or bin32; + /// `checksum` as a fixarray of 8, each byte a positive fixint or a + /// uint8; `original_size` as a positive fixint or a uint8, uint16 or + /// uint32; and `format` as a fixstr or str8 of valid UTF-8. Like the + /// full decode, it ignores bytes after the envelope. + /// + /// `None` for any other input, whether the full decode accepts it (a + /// legacy int-array payload, a bin8 checksum, a wider integer or + /// string, the map form) or rejects it: the caller's fallback decides + /// it, with the error it reports today. + /// + /// It holds the decode bounds the pre-scan enforces. The shape nests + /// two deep; every declared length is checked against the bytes after + /// it before they are read; and a complete envelope declares fewer + /// slots than it has bytes, since each slot is at least one byte and + /// the root header is not a slot. Nothing is allocated until the whole + /// envelope has been read. + pub(super) fn decode_canonical(bytes: &'a [u8]) -> Option { + let mut rest = bytes; + if take_byte(&mut rest)? != 0x94 { + return None; + } + let payload_len = match take_byte(&mut rest)? { + 0xc4 => take_uint(&mut rest, 1)?, + 0xc5 => take_uint(&mut rest, 2)?, + 0xc6 => take_uint(&mut rest, 4)?, + _ => return None, + }; + let payload = take(&mut rest, payload_len)?; + if take_byte(&mut rest)? != 0x98 { + return None; + } + let mut checksum = [0u8; 8]; + for byte in &mut checksum { + *byte = match take_byte(&mut rest)? { + fixint @ 0x00..=0x7f => fixint, + 0xcc => take_byte(&mut rest)?, + _ => return None, + }; + } + let original_size = match take_byte(&mut rest)? { + fixint @ 0x00..=0x7f => u32::from(fixint), + 0xcc => take_uint(&mut rest, 1)?, + 0xcd => take_uint(&mut rest, 2)?, + 0xce => take_uint(&mut rest, 4)?, + _ => return None, + }; + let format_len = match take_byte(&mut rest)? { + fixstr @ 0xa0..=0xbf => u32::from(fixstr & 0x1f), + 0xd9 => take_uint(&mut rest, 1)?, + _ => return None, + }; + let format = std::str::from_utf8(take(&mut rest, format_len)?).ok()?; + Some(StorageEnvelope { + compressed_data: borrow_below_large(payload), + checksum, + original_size, + format: format.to_owned(), + }) + } + } + + fn take_byte(rest: &mut &[u8]) -> Option { + let (&byte, tail) = rest.split_first()?; + *rest = tail; + Some(byte) + } + + /// The next `len` bytes, or `None` when fewer remain. + fn take<'a>(rest: &mut &'a [u8], len: u32) -> Option<&'a [u8]> { + let (head, tail) = rest.split_at_checked(usize::try_from(len).ok()?)?; + *rest = tail; + Some(head) + } + + /// A big-endian unsigned integer `width` bytes wide, at most 4. + fn take_uint(rest: &mut &[u8], width: u32) -> Option { + let bytes = take(rest, width)?; + Some(bytes.iter().fold(0, |acc, &b| (acc << 8) | u32::from(b))) } /// serde_bytes' `ByteBuf` decode, which `StorageEnvelope::compressed_data` @@ -1151,6 +1305,148 @@ mod tests { } } + /// What `store` writes, at each field's encoding width boundaries: bin8, + /// bin16 and bin32 payloads (the last two either side of LARGE_PAYLOAD), + /// fixint and uint8 checksum bytes, every width of `original_size`, and + /// fixstr and str8 formats. + fn canonical_envelopes() -> Vec> { + let mut out = Vec::new(); + for (payload_len, size, format_len) in [ + (0, 0, 0), + (1, 0x7f, 7), + (40, 0x80, 31), + (255, 0xff, 32), + (256, 0x100, 255), + (300, 0xffff, 1), + (0x1_0000, 0x1_0000, 7), + (0x1_0001, u32::MAX, 7), + ] { + for checksum in [[0; 8], [0x7f, 0x80, 0xff, 0, 1, 0x7e, 0x81, 0xfe]] { + let envelope = envelope_with(payload_len, checksum, size, format_len); + out.push(rmp_serde::to_vec(&envelope).unwrap()); + } + } + out + } + + #[test] + fn one_pass_decode_takes_every_canonical_envelope() { + // The read path's speed rests on this: an envelope `store` writes + // never falls back to the pre-scan and serde. + for bytes in canonical_envelopes() { + assert!( + borrowed::StorageEnvelope::decode_canonical(&bytes).is_some(), + "declined {:02x?}", + &bytes[..bytes.len().min(16)] + ); + } + let fixture: serde_json::Value = + serde_json::from_str(include_str!("../tests/vectors/wire-format.json")).unwrap(); + for v in fixture["vectors"].as_array().unwrap() { + let name = v["name"].as_str().unwrap(); + let bytes = hex::decode(v["envelope_hex"].as_str().unwrap()).unwrap(); + let one_pass = borrowed::StorageEnvelope::decode_canonical(&bytes); + // Legacy int-array payloads are the full decode's to read. + assert_eq!(one_pass.is_some(), name.ends_with("_bin"), "[{name}]"); + } + } + + #[test] + fn one_pass_decode_matches_full_decode() { + // `decode_envelope` returns exactly what `decode_envelope_full` does, + // fields or error text, and the one-pass decode accepts only what the + // full decode accepts, into the same fields. Inputs: the decode corpus, + // the decode-bounds vectors, and the canonical envelopes truncated at + // every length, with a byte inserted or replaced (by values that change + // its MessagePack meaning) at every header position, and with bytes + // appended. + let (mut one_pass_accepted, mut full_only_accepted) = (0, 0); + let mut check = |bytes: &[u8]| { + let shown = &bytes[..bytes.len().min(48)]; + let one_pass = borrowed::StorageEnvelope::decode_canonical(bytes) + .map(borrowed::StorageEnvelope::into_fields); + let full = decode_envelope_full(bytes).map(borrowed::StorageEnvelope::into_fields); + match (&one_pass, &full) { + (Some(one), Ok(full)) => { + assert_eq!( + one, + full, + "fields differ, len {}: {shown:02x?}", + bytes.len() + ); + one_pass_accepted += 1; + } + (Some(_), Err(e)) => panic!( + "one-pass accepted what the full decode rejects ({e}), len {}: {shown:02x?}", + bytes.len() + ), + (None, Ok(_)) => full_only_accepted += 1, + (None, Err(_)) => {} + } + let decoded = decode_envelope(bytes).map(borrowed::StorageEnvelope::into_fields); + assert_eq!(decoded, full, "len {}: {shown:02x?}", bytes.len()); + }; + + for (_, bytes) in decode_corpus() { + check(&bytes); + } + let bounds: serde_json::Value = + serde_json::from_str(include_str!("../tests/vectors/decode-bounds.json")).unwrap(); + for group in ["reject_vectors", "accept_vectors"] { + for v in bounds[group].as_array().unwrap() { + check(&hex::decode(v["input_hex"].as_str().unwrap()).unwrap()); + } + } + // A format of str16 and a bin8 checksum: both full-decode only. + check(&rmp_serde::to_vec(&envelope_with(3, [1; 8], 3, 256)).unwrap()); + let mut bin8_checksum = rmp_serde::to_vec(&envelope_with(3, [1; 8], 3, 1)).unwrap(); + bin8_checksum.splice(6..15, [0xc4, 8, 1, 1, 1, 1, 1, 1, 1, 1]); + check(&bin8_checksum); + + let markers = [ + 0x00, 0x01, 0x07, 0x7f, 0x80, 0x84, 0x90, 0x93, 0x94, 0x95, 0x97, 0x98, 0x99, 0xa0, + 0xa1, 0xbf, 0xc0, 0xc1, 0xc2, 0xc4, 0xc5, 0xc6, 0xca, 0xcc, 0xcd, 0xce, 0xcf, 0xd0, + 0xd9, 0xda, 0xdb, 0xdc, 0xdd, 0xde, 0xe0, 0xff, + ]; + for env in canonical_envelopes() { + // Every position of a small envelope. Of a large one, the headers + // and the fields after the payload (at most 30 bytes). + let positions = + (0..env.len()).filter(|&i| env.len() <= 1024 || i < 8 || i + 40 >= env.len()); + for i in positions { + check(&env[..i]); + for &m in markers.iter().chain(&[env[i] ^ 1, env[i].wrapping_add(1)]) { + let mut replaced = env.clone(); + replaced[i] = m; + check(&replaced); + let mut inserted = env.clone(); + inserted.insert(i, m); + check(&inserted); + } + } + for tail in [ + &[0x00][..], + &[0xc1], + &[0x91], + &[0xdd, 0xff, 0xff, 0xff, 0xff], + ] { + check(&[&env[..], tail].concat()); + } + check(&env); + } + + // Both sides of the fallback ran: accepted by the one-pass decode, and + // accepted only by the full decode (legacy, bin8 checksum, str16, map). + assert!( + one_pass_accepted > 1000, + "one-pass accepted {one_pass_accepted}" + ); + assert!( + full_only_accepted > 100, + "full-only accepted {full_only_accepted}" + ); + } + #[test] fn bin_payload_is_borrowed_and_legacy_payload_is_owned() { let fixture: serde_json::Value = From 74eb9fa354ba2db7f368e79d8846eeaaad9812c7 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 31 Oct 2026 10:51:20 +0000 Subject: [PATCH 02/10] perf(byte-storage): read every format width in one pass; share the differential check The one-pass decode declined a format of 256 bytes or more (str16, str32), so "an envelope store writes is read in one pass" did not hold for those. It now takes every str width, and the canonical-envelope tests cover str16 and str32; a uint64 original_size stands in as a full-decode-only input. decode_canonical is renamed decode_one_pass: it also takes encodings wider than the shortest. The unit test and the fuzz target now call one shared assert_one_pass_matches_full_decode (built for tests and cfg(fuzzing)), which also checks retrieve end to end; the field tuple, its conversion and the three-way hook are gone. The borrow test runs both decodes. Docs state the subset property on its own and in the present tense. --- README.md | 6 +- SECURITY.md | 19 +- fuzz/README.md | 2 +- .../byte_storage_corrupted_envelope.rs | 39 +-- src/byte_storage.rs | 232 ++++++++---------- 5 files changed, 124 insertions(+), 174 deletions(-) diff --git a/README.md b/README.md index 770dce2..3b67428 100644 --- a/README.md +++ b/README.md @@ -263,8 +263,8 @@ can back, the reserved marker `0xc1` and truncated input are all rejected before decoding, without allocating in proportion to any declared length. A rejection is `ByteStorageError::DeserializationFailed` with the message prefix `decode pre-scan: `. The one-pass read accepts only envelopes the pre-scan and -the typed decode accept, into the same fields, so every input gets the result -and error it got before. The same walk is public as +the typed decode accept, into the same fields, and leaves every rejection to +them, so every input gets their result and error. The same walk is public as `check_msgpack_structure(bytes, max_depth)` for callers that decode untrusted MessagePack themselves; it returns a `MsgpackStructureError` whose `Display` is the bare reason, with no prefix. `msgpack_document_len(bytes, max_depth)` runs @@ -283,7 +283,7 @@ cachekit-core/ ├── src/ │ ├── lib.rs # Public API exports │ ├── byte_storage.rs # LZ4 + xxHash3 storage envelope -│ ├── msgpack_bounds.rs # Structural pre-scan (public check_msgpack_structure, msgpack_document_len), run before the envelope decode +│ ├── msgpack_bounds.rs # Structural pre-scan (public check_msgpack_structure, msgpack_document_len), run before the fallback envelope decode │ ├── checksum.rs # Standalone xxHash3 checksum/verify primitive (feature = "checksum") │ │ │ ├── encryption/ # (feature = "encryption") diff --git a/SECURITY.md b/SECURITY.md index c1593ef..8c01e06 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -180,15 +180,16 @@ are `spec/interop-mode.md` → Decode bounds). An envelope in the shape `store` writes skips the pre-scan and `rmp_serde`: a one-pass decode reads its four fields in their exact encodings (a 4-element -fixarray; bin8/16/32; a fixarray of 8 checksum bytes; a uint of at most 32 -bits; a fixstr or str8 of valid UTF-8). It holds the same bounds. It reads two -levels deep, checks every declared length against the bytes after it before -reading them, and allocates nothing until it has read the whole envelope; a -complete envelope always declares fewer slots than its length. It hands any -other input, and every input it would reject, to the pre-scan and `rmp_serde`, -so it accepts only what they accept, into the same fields, and every rejection -still carries the error below. `one_pass_decode_matches_full_decode` and the -`byte_storage_corrupted_envelope` fuzz target compare the two decodes. +fixarray; bin8/16/32; a fixarray of 8 checksum bytes, each a positive fixint or +uint8; a uint of at most 32 bits; a str of valid UTF-8). It holds the same +bounds. It reads two levels deep, checks every declared length against the +bytes after it before reading them, and allocates nothing until it has read +the whole envelope; a complete envelope always declares fewer slots than its +length. It accepts only envelopes the pre-scan and `rmp_serde` accept, into the +same fields, which `one_pass_decode_matches_full_decode` and the +`byte_storage_corrupted_envelope` fuzz target check. It hands every other +input, including every input it would reject, to the pre-scan and `rmp_serde`, +so every rejection carries the error below. The pre-scan rejects: diff --git a/fuzz/README.md b/fuzz/README.md index ff54b96..38bffc9 100644 --- a/fuzz/README.md +++ b/fuzz/README.md @@ -46,7 +46,7 @@ For a coverage report, see [Coverage Reporting](#coverage-reporting). **byte_storage_corrupted_envelope.rs** - Attack: Malformed MessagePack `StorageEnvelope` deserialization -- Validates: Safe rejection of corrupted serialization without panics, and that the one-pass envelope decode `retrieve` tries first agrees with the pre-scan plus `rmp_serde` decode on accept, reject and every decoded field +- Validates: Safe rejection of corrupted serialization without panics; that the one-pass envelope decode `retrieve` tries first accepts only what the pre-scan plus `rmp_serde` decode accepts, into the same fields; and that `retrieve` returns that decode's result - Tests: Invalid MessagePack structures, truncated data, type confusion, and envelopes `store` writes with bytes replaced, inserted or cut off **byte_storage_integer_overflow.rs** diff --git a/fuzz/fuzz_targets/byte_storage_corrupted_envelope.rs b/fuzz/fuzz_targets/byte_storage_corrupted_envelope.rs index 2016d0c..77525c5 100644 --- a/fuzz/fuzz_targets/byte_storage_corrupted_envelope.rs +++ b/fuzz/fuzz_targets/byte_storage_corrupted_envelope.rs @@ -1,19 +1,15 @@ #![no_main] -use cachekit_core::byte_storage::{fuzz_envelope_decodes, StorageEnvelope}; +use cachekit_core::byte_storage::assert_one_pass_matches_full_decode; use cachekit_core::ByteStorage; use libfuzzer_sys::fuzz_target; -// Attack: malformed and near-canonical envelopes through the envelope decode. -// Validates, besides fail-closed rejection without panics, that the one-pass -// decode `retrieve` tries first agrees with the pre-scan plus serde decode it -// falls back to: it accepts only what that decode accepts, into the same -// fields, and `retrieve` returns what that decode would give it, error text -// included. +// Attack: malformed and corrupted envelopes through the envelope decode. +// Validates: fail-closed rejection without panics, and that the one-pass decode agrees with the full decode. fuzz_target!(|data: &[u8]| { // The input as envelope bytes. - check(data); + assert_one_pass_matches_full_decode(data); // The input as an envelope `store` writes, then corrupted, which reaches // the one-pass decode's boundaries far more often than raw bytes do. The @@ -22,8 +18,7 @@ fuzz_target!(|data: &[u8]| { let Some((&plan, rest)) = data.split_first() else { return; }; - let edit_bytes = usize::from(plan % 4) * 3; - let Some((edits, value)) = rest.split_at_checked(edit_bytes) else { + let Some((edits, value)) = rest.split_at_checked(usize::from(plan % 4) * 3) else { return; }; let mut envelope = ByteStorage::new(None).store(value, None).unwrap(); @@ -36,27 +31,5 @@ fuzz_target!(|data: &[u8]| { _ => envelope.insert(pos, edit[2]), } } - check(&envelope); + assert_one_pass_matches_full_decode(&envelope); }); - -fn check(bytes: &[u8]) { - let (one_pass, full, decoded) = fuzz_envelope_decodes(bytes); - match (&one_pass, &full) { - (Some(one), Ok(full)) => assert_eq!(one, full, "one-pass and full decodes differ"), - (Some(_), Err(e)) => panic!("one-pass decode accepted what the full decode rejects: {e}"), - _ => {} - } - assert_eq!(decoded, full, "decode_envelope differs from the full decode"); - - // `retrieve` is the full decode's fields, extracted. - let expected = full.and_then(|(compressed_data, checksum, original_size, format)| { - let envelope = StorageEnvelope { - compressed_data, - checksum, - original_size, - format, - }; - envelope.extract().map(|data| (data, envelope.format)) - }); - assert_eq!(ByteStorage::new(None).retrieve(bytes), expected); -} diff --git a/src/byte_storage.rs b/src/byte_storage.rs index 67ced1c..12933fa 100644 --- a/src/byte_storage.rs +++ b/src/byte_storage.rs @@ -189,7 +189,8 @@ fn compress_with_thread_table(data: &[u8]) -> Result, ByteStorageError> /// Storage envelope for raw byte storage /// Contains compressed data with integrity checking // `retrieve` and `validate` decode into its private borrowing twin, -// `borrowed::StorageEnvelope`: change the two together. +// `borrowed::StorageEnvelope`, whose `decode_one_pass` hard-codes this field +// order and these encodings: change the three together. #[derive(Serialize, Deserialize)] pub struct StorageEnvelope { /// Compressed payload data @@ -554,19 +555,15 @@ impl Default for ByteStorage { } } -/// Decode untrusted envelope bytes: the one-pass decode of the shape `store` -/// writes, and for any input it declines, `decode_envelope_full`. -/// -/// The one-pass decode accepts only inputs the full decode accepts, into the -/// same fields, and declines every input the full decode rejects, so this -/// returns what `decode_envelope_full` returns, error text included -/// (`one_pass_decode_matches_full_decode`). It walks the envelope once instead -/// of twice, which at small sizes is most of the cost of a read. +/// Decode untrusted envelope bytes: `decode_one_pass`, or `decode_envelope_full` +/// for any input it declines. Both give the same result +/// (`assert_one_pass_matches_full_decode`), and one walk instead of two is most +/// of a small read's cost. #[cfg(all(feature = "compression", feature = "checksum", feature = "messagepack"))] fn decode_envelope( envelope_bytes: &[u8], ) -> Result, ByteStorageError> { - match borrowed::StorageEnvelope::decode_canonical(envelope_bytes) { + match borrowed::StorageEnvelope::decode_one_pass(envelope_bytes) { Some(envelope) => Ok(envelope), None => decode_envelope_full(envelope_bytes), } @@ -589,7 +586,13 @@ fn decode_envelope_full( .map_err(|e| ByteStorageError::DeserializationFailed(e.to_string())) } -/// A decoded envelope's `(compressed_data, checksum, original_size, format)`. +/// Panics unless `decode_one_pass` accepts only what `decode_envelope_full` +/// accepts, into the same fields, and `retrieve` returns that decode's +/// envelope, extracted. Returns whether each decode accepted. +/// +/// The differential behind `one_pass_decode_matches_full_decode` and the +/// `byte_storage_corrupted_envelope` fuzz target. Built only for tests and +/// under `cfg(fuzzing)`, which cargo-fuzz sets. #[cfg(all( any(test, fuzzing), feature = "compression", @@ -597,32 +600,31 @@ fn decode_envelope_full( feature = "messagepack" ))] #[doc(hidden)] -pub type EnvelopeFields = (Vec, [u8; 8], u32, String); - -/// The same bytes through the one-pass decode, `decode_envelope_full` and -/// `decode_envelope`, for the `byte_storage_corrupted_envelope` fuzz target's -/// differential. Built only under `cfg(fuzzing)`, which cargo-fuzz sets. -#[cfg(all( - fuzzing, - feature = "compression", - feature = "checksum", - feature = "messagepack" -))] -#[doc(hidden)] -#[allow(clippy::type_complexity)] -pub fn fuzz_envelope_decodes( - envelope_bytes: &[u8], -) -> ( - Option, - Result, - Result, -) { - ( - borrowed::StorageEnvelope::decode_canonical(envelope_bytes) - .map(borrowed::StorageEnvelope::into_fields), - decode_envelope_full(envelope_bytes).map(borrowed::StorageEnvelope::into_fields), - decode_envelope(envelope_bytes).map(borrowed::StorageEnvelope::into_fields), - ) +pub fn assert_one_pass_matches_full_decode(envelope_bytes: &[u8]) -> (bool, bool) { + let input = format!( + "{} bytes, starting {:02x?}", + envelope_bytes.len(), + &envelope_bytes[..envelope_bytes.len().min(48)] + ); + let one_pass = borrowed::StorageEnvelope::decode_one_pass(envelope_bytes); + let full = decode_envelope_full(envelope_bytes); + match (&one_pass, &full) { + (Some(one), Ok(full)) => assert!(one == full, "the decodes read different fields: {input}"), + (Some(_), Err(e)) => { + panic!("one-pass accepted what the full decode rejects ({e}): {input}") + } + (None, _) => {} + } + let expected = full + .as_ref() + .map_err(Clone::clone) + .and_then(|envelope| Ok((envelope.extract()?, envelope.format.clone()))); + let retrieved = ByteStorage::new(None).retrieve(envelope_bytes); + assert!( + retrieved == expected, + "retrieve differs from the full decode: {input}" + ); + (one_pass.is_some(), full.is_ok()) } /// What `retrieve` and `validate` decode: `StorageEnvelope` with @@ -645,6 +647,7 @@ mod borrowed { use super::{extract_payload, ByteStorageError, LARGE_PAYLOAD}; #[derive(Deserialize)] + #[cfg_attr(any(test, fuzzing), derive(Debug, PartialEq))] pub(super) struct StorageEnvelope<'a> { #[serde(borrow, deserialize_with = "borrow_payload")] pub(super) compressed_data: Cow<'a, [u8]>, @@ -659,34 +662,18 @@ mod borrowed { extract_payload(&self.compressed_data, &self.checksum, self.original_size) } - /// The fields, owned, for the differential tests and fuzz target. - #[cfg(any(test, fuzzing))] - pub(super) fn into_fields(self) -> super::EnvelopeFields { - let Self { - compressed_data, - checksum, - original_size, - format, - } = self; - ( - compressed_data.into_owned(), - checksum, - original_size, - format, - ) - } - /// One pass over the shape `rmp_serde` writes for every envelope: a /// fixarray of 4 holding `compressed_data` as bin8, bin16 or bin32; /// `checksum` as a fixarray of 8, each byte a positive fixint or a /// uint8; `original_size` as a positive fixint or a uint8, uint16 or - /// uint32; and `format` as a fixstr or str8 of valid UTF-8. Like the - /// full decode, it ignores bytes after the envelope. + /// uint32; and `format` as a str of valid UTF-8 (fixstr, str8, str16 + /// or str32). Like the full decode, it ignores bytes after the + /// envelope. /// /// `None` for any other input, whether the full decode accepts it (a - /// legacy int-array payload, a bin8 checksum, a wider integer or - /// string, the map form) or rejects it: the caller's fallback decides - /// it, with the error it reports today. + /// legacy int-array payload, a bin8 checksum, a uint64 or signed + /// integer, the map form) or rejects it: the caller's fallback decides + /// it, and reports the full decode's error. /// /// It holds the decode bounds the pre-scan enforces. The shape nests /// two deep; every declared length is checked against the bytes after @@ -694,7 +681,7 @@ mod borrowed { /// slots than it has bytes, since each slot is at least one byte and /// the root header is not a slot. Nothing is allocated until the whole /// envelope has been read. - pub(super) fn decode_canonical(bytes: &'a [u8]) -> Option { + pub(super) fn decode_one_pass(bytes: &'a [u8]) -> Option { let mut rest = bytes; if take_byte(&mut rest)? != 0x94 { return None; @@ -727,6 +714,8 @@ mod borrowed { let format_len = match take_byte(&mut rest)? { fixstr @ 0xa0..=0xbf => u32::from(fixstr & 0x1f), 0xd9 => take_uint(&mut rest, 1)?, + 0xda => take_uint(&mut rest, 2)?, + 0xdb => take_uint(&mut rest, 4)?, _ => return None, }; let format = std::str::from_utf8(take(&mut rest, format_len)?).ok()?; @@ -1308,7 +1297,7 @@ mod tests { /// What `store` writes, at each field's encoding width boundaries: bin8, /// bin16 and bin32 payloads (the last two either side of LARGE_PAYLOAD), /// fixint and uint8 checksum bytes, every width of `original_size`, and - /// fixstr and str8 formats. + /// fixstr, str8, str16 and str32 formats. fn canonical_envelopes() -> Vec> { let mut out = Vec::new(); for (payload_len, size, format_len) in [ @@ -1317,7 +1306,8 @@ mod tests { (40, 0x80, 31), (255, 0xff, 32), (256, 0x100, 255), - (300, 0xffff, 1), + (300, 0xffff, 256), + (3, 3, 0x1_0000), (0x1_0000, 0x1_0000, 7), (0x1_0001, u32::MAX, 7), ] { @@ -1335,7 +1325,7 @@ mod tests { // never falls back to the pre-scan and serde. for bytes in canonical_envelopes() { assert!( - borrowed::StorageEnvelope::decode_canonical(&bytes).is_some(), + borrowed::StorageEnvelope::decode_one_pass(&bytes).is_some(), "declined {:02x?}", &bytes[..bytes.len().min(16)] ); @@ -1345,7 +1335,7 @@ mod tests { for v in fixture["vectors"].as_array().unwrap() { let name = v["name"].as_str().unwrap(); let bytes = hex::decode(v["envelope_hex"].as_str().unwrap()).unwrap(); - let one_pass = borrowed::StorageEnvelope::decode_canonical(&bytes); + let one_pass = borrowed::StorageEnvelope::decode_one_pass(&bytes); // Legacy int-array payloads are the full decode's to read. assert_eq!(one_pass.is_some(), name.ends_with("_bin"), "[{name}]"); } @@ -1353,38 +1343,15 @@ mod tests { #[test] fn one_pass_decode_matches_full_decode() { - // `decode_envelope` returns exactly what `decode_envelope_full` does, - // fields or error text, and the one-pass decode accepts only what the - // full decode accepts, into the same fields. Inputs: the decode corpus, - // the decode-bounds vectors, and the canonical envelopes truncated at - // every length, with a byte inserted or replaced (by values that change - // its MessagePack meaning) at every header position, and with bytes - // appended. + // `assert_one_pass_matches_full_decode` on the decode corpus, the + // decode-bounds vectors, and the canonical envelopes truncated at every + // length, with a byte inserted or replaced (by values that change its + // MessagePack meaning) at every header position, and with bytes appended. let (mut one_pass_accepted, mut full_only_accepted) = (0, 0); - let mut check = |bytes: &[u8]| { - let shown = &bytes[..bytes.len().min(48)]; - let one_pass = borrowed::StorageEnvelope::decode_canonical(bytes) - .map(borrowed::StorageEnvelope::into_fields); - let full = decode_envelope_full(bytes).map(borrowed::StorageEnvelope::into_fields); - match (&one_pass, &full) { - (Some(one), Ok(full)) => { - assert_eq!( - one, - full, - "fields differ, len {}: {shown:02x?}", - bytes.len() - ); - one_pass_accepted += 1; - } - (Some(_), Err(e)) => panic!( - "one-pass accepted what the full decode rejects ({e}), len {}: {shown:02x?}", - bytes.len() - ), - (None, Ok(_)) => full_only_accepted += 1, - (None, Err(_)) => {} - } - let decoded = decode_envelope(bytes).map(borrowed::StorageEnvelope::into_fields); - assert_eq!(decoded, full, "len {}: {shown:02x?}", bytes.len()); + let mut check = |bytes: &[u8]| match assert_one_pass_matches_full_decode(bytes) { + (true, _) => one_pass_accepted += 1, + (false, true) => full_only_accepted += 1, + (false, false) => {} }; for (_, bytes) in decode_corpus() { @@ -1397,8 +1364,10 @@ mod tests { check(&hex::decode(v["input_hex"].as_str().unwrap()).unwrap()); } } - // A format of str16 and a bin8 checksum: both full-decode only. - check(&rmp_serde::to_vec(&envelope_with(3, [1; 8], 3, 256)).unwrap()); + // A uint64 `original_size` and a bin8 checksum: both full-decode only. + let mut uint64_size = rmp_serde::to_vec(&envelope_with(3, [1; 8], 3, 1)).unwrap(); + uint64_size.splice(15..16, [0xcf, 0, 0, 0, 0, 0, 0, 0, 3]); + check(&uint64_size); let mut bin8_checksum = rmp_serde::to_vec(&envelope_with(3, [1; 8], 3, 1)).unwrap(); bin8_checksum.splice(6..15, [0xc4, 8, 1, 1, 1, 1, 1, 1, 1, 1]); check(&bin8_checksum); @@ -1409,8 +1378,8 @@ mod tests { 0xd9, 0xda, 0xdb, 0xdc, 0xdd, 0xde, 0xe0, 0xff, ]; for env in canonical_envelopes() { - // Every position of a small envelope. Of a large one, the headers - // and the fields after the payload (at most 30 bytes). + // Every position of a small envelope. Of a large one, the first 8 + // bytes and the last 40. let positions = (0..env.len()).filter(|&i| env.len() <= 1024 || i < 8 || i + 40 >= env.len()); for i in positions { @@ -1436,7 +1405,7 @@ mod tests { } // Both sides of the fallback ran: accepted by the one-pass decode, and - // accepted only by the full decode (legacy, bin8 checksum, str16, map). + // accepted only by the full decode (legacy, bin8 checksum, uint64, map). assert!( one_pass_accepted > 1000, "one-pass accepted {one_pass_accepted}" @@ -1452,23 +1421,28 @@ mod tests { let fixture: serde_json::Value = serde_json::from_str(include_str!("../tests/vectors/wire-format.json")).unwrap(); use std::borrow::Cow; + // Both decodes borrow alike: the one-pass decode, and serde's, which + // still reads every envelope the one-pass decode declines. + let decoders = [decode_envelope, decode_envelope_full]; let mut seen = (0, 0); - for v in fixture["vectors"].as_array().unwrap() { - let bytes = hex::decode(v["envelope_hex"].as_str().unwrap()).unwrap(); - let envelope = decode_envelope(&bytes).unwrap(); - let name = v["name"].as_str().unwrap(); - if name.ends_with("_bin") { - seen.0 += 1; - assert!( - matches!(envelope.compressed_data, Cow::Borrowed(_)), - "[{name}] bin payload copied" - ); - } else { - seen.1 += 1; - assert!( - matches!(envelope.compressed_data, Cow::Owned(_)), - "[{name}] legacy payload" - ); + for decode in decoders { + for v in fixture["vectors"].as_array().unwrap() { + let bytes = hex::decode(v["envelope_hex"].as_str().unwrap()).unwrap(); + let envelope = decode(&bytes).unwrap(); + let name = v["name"].as_str().unwrap(); + if name.ends_with("_bin") { + seen.0 += 1; + assert!( + matches!(envelope.compressed_data, Cow::Borrowed(_)), + "[{name}] bin payload copied" + ); + } else { + seen.1 += 1; + assert!( + matches!(envelope.compressed_data, Cow::Owned(_)), + "[{name}] legacy payload" + ); + } } } assert!( @@ -1481,18 +1455,20 @@ mod tests { for (len, borrowed) in [(LARGE_PAYLOAD - 512, true), (LARGE_PAYLOAD + 512, false)] { let data = xorshift_bytes(len); let stored = storage.store(&data, None).unwrap(); - let envelope = decode_envelope(&stored).unwrap(); - let payload = envelope.compressed_data.len(); - assert_eq!( - payload < LARGE_PAYLOAD, - borrowed, - "len {len}: payload {payload}" - ); - assert_eq!( - matches!(envelope.compressed_data, Cow::Borrowed(_)), - borrowed, - "len {len}: payload {payload}" - ); + for decode in decoders { + let envelope = decode(&stored).unwrap(); + let payload = envelope.compressed_data.len(); + assert_eq!( + payload < LARGE_PAYLOAD, + borrowed, + "len {len}: payload {payload}" + ); + assert_eq!( + matches!(envelope.compressed_data, Cow::Borrowed(_)), + borrowed, + "len {len}: payload {payload}" + ); + } assert_eq!(storage.retrieve(&stored).unwrap().0, data, "len {len}"); } } From 4f636a7c1dba644a03b0871d83ec2c589f8b8f70 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 31 Oct 2026 11:21:42 +0000 Subject: [PATCH 03/10] =?UTF-8?q?fix:=20address=20coderabbit=20review=20?= =?UTF-8?q?=E2=80=94=20one-pass=20scope=20wording,=20owned=20decode=20fuzz?= =?UTF-8?q?ing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The one-pass decode also takes non-minimal length and size encodings (str8 for a short format, bin16 for a short payload), so the docs now name the layout and say any width of its lengths and size qualifies, instead of "the shape store writes". The corrupted-envelope fuzz target again feeds raw input to the public owned StorageEnvelope decoder and extract, as it did before the differential. CodeRabbit-Resolved: byte_storage.rs:483:Correct the one-pass decode CodeRabbit-Resolved: byte_storage_corrupted_envelope.rs:11:Restore direct owned Storag --- README.md | 9 +++++---- SECURITY.md | 9 +++++---- fuzz/fuzz_targets/byte_storage_corrupted_envelope.rs | 10 +++++++--- src/byte_storage.rs | 9 ++++++--- 4 files changed, 23 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 3b67428..189f17b 100644 --- a/README.md +++ b/README.md @@ -254,9 +254,10 @@ Malicious payloads claiming `original_size: 500GB` with 100 bytes of data are re
Envelope Decode Bounds -`retrieve()` and `validate()` read an envelope in the shape `store()` writes in -one pass, which checks each declared length against the input before reading it -and allocates nothing until the whole envelope has been read. Every other input +`retrieve()` and `validate()` read an envelope laid out as `store()` writes one, +whatever width its lengths and size are encoded in, in one pass, which checks +each declared length against the input before reading it and allocates nothing +until the whole envelope has been read. Every other input gets a header-only structural pre-scan **before** MessagePack decoding: nesting deeper than 100 levels, headers declaring more elements or bytes than the input can back, the reserved marker `0xc1` and truncated input are all rejected @@ -359,7 +360,7 @@ The Criterion suite in `benches/hot_path.rs` needs the `encryption` feature (`ma | Case | What one call does | |:-----|:-------------------| | `store/`, `retrieve/` | `ByteStorage::store` / `retrieve` on the realistic msgpack payload `hot_path` uses | -| `prescan/` | `check_msgpack_structure` on the envelope, the pre-scan `retrieve` runs on any envelope not in the shape `store` writes | +| `prescan/` | `check_msgpack_structure` on the envelope, the pre-scan `retrieve` runs on any envelope not laid out as `store` writes one | | `encrypt/`, `decrypt/` | `ZeroKnowledgeEncryptor` AES-256-GCM | | `keyring_decrypt/` | `Keyring::decrypt_indexed`, which re-derives the tenant key (HKDF) on every call | | `tenant_keyring_decrypt/` | `TenantKeyring::decrypt_indexed`, key derived once at `for_tenant` | diff --git a/SECURITY.md b/SECURITY.md index 8c01e06..f5cab5e 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -178,10 +178,11 @@ a `StorageEnvelope`, both run a header-only structural pre-scan over those bytes (protocol `spec/wire-format.md` → Retrieve Flow, step 2; the bounds themselves are `spec/interop-mode.md` → Decode bounds). -An envelope in the shape `store` writes skips the pre-scan and `rmp_serde`: a -one-pass decode reads its four fields in their exact encodings (a 4-element -fixarray; bin8/16/32; a fixarray of 8 checksum bytes, each a positive fixint or -uint8; a uint of at most 32 bits; a str of valid UTF-8). It holds the same +An envelope laid out as `store` writes one skips the pre-scan and `rmp_serde`, +whatever width its lengths and size are encoded in: a one-pass decode reads +its four fields in these encodings only (a 4-element fixarray; bin8/16/32; a +fixarray of 8 checksum bytes, each a positive fixint or uint8; a uint of at +most 32 bits; a str of valid UTF-8, any width). It holds the same bounds. It reads two levels deep, checks every declared length against the bytes after it before reading them, and allocates nothing until it has read the whole envelope; a complete envelope always declares fewer slots than its diff --git a/fuzz/fuzz_targets/byte_storage_corrupted_envelope.rs b/fuzz/fuzz_targets/byte_storage_corrupted_envelope.rs index 77525c5..0ec0a6c 100644 --- a/fuzz/fuzz_targets/byte_storage_corrupted_envelope.rs +++ b/fuzz/fuzz_targets/byte_storage_corrupted_envelope.rs @@ -1,15 +1,19 @@ #![no_main] -use cachekit_core::byte_storage::assert_one_pass_matches_full_decode; +use cachekit_core::byte_storage::{assert_one_pass_matches_full_decode, StorageEnvelope}; use cachekit_core::ByteStorage; use libfuzzer_sys::fuzz_target; -// Attack: malformed and corrupted envelopes through the envelope decode. +// Attack: malformed and corrupted envelopes through the envelope decodes. // Validates: fail-closed rejection without panics, and that the one-pass decode agrees with the full decode. fuzz_target!(|data: &[u8]| { - // The input as envelope bytes. + // The input as envelope bytes, through `retrieve`'s decodes and through + // the public owned `StorageEnvelope`, which has no pre-scan of its own. assert_one_pass_matches_full_decode(data); + if let Ok(envelope) = rmp_serde::from_slice::(data) { + let _ = envelope.extract(); + } // The input as an envelope `store` writes, then corrupted, which reaches // the one-pass decode's boundaries far more often than raw bytes do. The diff --git a/src/byte_storage.rs b/src/byte_storage.rs index 12933fa..b85e817 100644 --- a/src/byte_storage.rs +++ b/src/byte_storage.rs @@ -480,9 +480,12 @@ impl ByteStorage { /// /// # Errors /// - /// `envelope_bytes` is untrusted. An envelope in the shape `store` writes - /// is decoded in one pass that checks each declared length against the - /// input before reading it. Any other input is decoded only after a + /// `envelope_bytes` is untrusted. An envelope laid out as `store` writes + /// one (a 4-element array of a bin payload, 8 uint8 checksum bytes, a + /// uint32 size and a str format), whatever width its lengths and size are + /// encoded in, is decoded in one pass that checks each declared length + /// against the input before reading it. Any other input is decoded only + /// after a /// structural pre-scan bounds its nesting depth and rejects any header /// that declares more than the input can back (protocol Retrieve Flow, /// step 2). A pre-scan rejection is `DeserializationFailed` whose message From 478bb8019c5d397527a1301ac480befd9e626219 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 31 Oct 2026 11:32:26 +0000 Subject: [PATCH 04/10] test(byte-storage): assert store's own output takes the one-pass decode either side of the literal envelope --- src/byte_storage.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/byte_storage.rs b/src/byte_storage.rs index 9297ba8..2233c3f 100644 --- a/src/byte_storage.rs +++ b/src/byte_storage.rs @@ -1471,6 +1471,15 @@ mod tests { &bytes[..bytes.len().min(16)] ); } + // `store` itself, either side of its hand-written literal envelope. + let storage = ByteStorage::new(None); + for len in [0, 1, 40, 63, 64, 1024] { + let stored = storage.store(&xorshift_bytes(len), None).unwrap(); + assert!( + borrowed::StorageEnvelope::decode_one_pass(&stored).is_some(), + "declined store output for {len} B" + ); + } let fixture: serde_json::Value = serde_json::from_str(include_str!("../tests/vectors/wire-format.json")).unwrap(); for v in fixture["vectors"].as_array().unwrap() { From a9639115a88583b16f2aefb2052159fdc8722d99 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 31 Oct 2026 12:15:26 +0000 Subject: [PATCH 05/10] docs(byte-storage): state the one-pass decode's encodings and the pre-scan's scope exactly The retrieve rustdoc listed uint8 checksum bytes and a uint32 size where the one-pass decode takes a positive fixint or uint8 and any uint up to 32 bits. The msgpack_bounds module doc, the README and SECURITY.md said or implied that the pre-scan runs on every envelope, or that any width of size qualifies; it runs on every envelope the one-pass decode declines. The decode-bounds test doc now states its real premise, and the test-only derive drops Debug, which nothing formats. --- README.md | 14 +++++++------- SECURITY.md | 10 +++++----- src/byte_storage.rs | 15 ++++++++------- src/msgpack_bounds.rs | 5 +++-- tests/decode_bounds_vectors.rs | 13 +++++++------ 5 files changed, 30 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 1a3c3e4..85571d9 100644 --- a/README.md +++ b/README.md @@ -254,18 +254,18 @@ Malicious payloads claiming `original_size: 500GB` with 100 bytes of data are re
Envelope Decode Bounds -`retrieve()` and `validate()` read an envelope laid out as `store()` writes one, -whatever width its lengths and size are encoded in, in one pass, which checks -each declared length against the input before reading it and allocates nothing -until the whole envelope has been read. Every other input -gets a header-only structural pre-scan **before** MessagePack decoding: nesting +`retrieve()` and `validate()` read an envelope in any encoding `store()` writes +in one pass, which checks each declared length against the input before reading +it and allocates nothing until the whole envelope has been read. Every input +that read declines gets a header-only structural pre-scan **before** MessagePack +decoding: nesting deeper than 100 levels, headers declaring more elements or bytes than the input can back, the reserved marker `0xc1` and truncated input are all rejected before decoding, without allocating in proportion to any declared length. A rejection is `ByteStorageError::DeserializationFailed` with the message prefix `decode pre-scan: `. The one-pass read accepts only envelopes the pre-scan and the typed decode accept, into the same fields, and leaves every rejection to -them, so every input gets their result and error. The same walk is public as +them, so every input gets their result and error. The pre-scan's walk is public as `check_msgpack_structure(bytes, max_depth)` for callers that decode untrusted MessagePack themselves; it returns a `MsgpackStructureError` whose `Display` is the bare reason, with no prefix. `msgpack_document_len(bytes, max_depth)` runs @@ -360,7 +360,7 @@ The Criterion suite in `benches/hot_path.rs` needs the `encryption` feature (`ma | Case | What one call does | |:-----|:-------------------| | `store/`, `retrieve/` | `ByteStorage::store` / `retrieve` on the realistic msgpack payload `hot_path` uses | -| `prescan/` | `check_msgpack_structure` on the envelope, the pre-scan `retrieve` runs on any envelope not laid out as `store` writes one | +| `prescan/` | `check_msgpack_structure` on the envelope, the pre-scan `retrieve` runs on every envelope its one-pass read declines | | `encrypt/`, `decrypt/` | `ZeroKnowledgeEncryptor` AES-256-GCM | | `keyring_decrypt/` | `Keyring::decrypt_indexed`, which re-derives the tenant key (HKDF) on every call | | `tenant_keyring_decrypt/` | `TenantKeyring::decrypt_indexed`, key derived once at `for_tenant` | diff --git a/SECURITY.md b/SECURITY.md index f5cab5e..c72f882 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -178,11 +178,11 @@ a `StorageEnvelope`, both run a header-only structural pre-scan over those bytes (protocol `spec/wire-format.md` → Retrieve Flow, step 2; the bounds themselves are `spec/interop-mode.md` → Decode bounds). -An envelope laid out as `store` writes one skips the pre-scan and `rmp_serde`, -whatever width its lengths and size are encoded in: a one-pass decode reads -its four fields in these encodings only (a 4-element fixarray; bin8/16/32; a -fixarray of 8 checksum bytes, each a positive fixint or uint8; a uint of at -most 32 bits; a str of valid UTF-8, any width). It holds the same +An envelope whose four fields use these encodings skips the pre-scan and +`rmp_serde`, and they cover every envelope `store` writes: a 4-element +fixarray; bin8/16/32; a fixarray of 8 checksum bytes, each a positive fixint or +uint8; a uint of at most 32 bits; a str of valid UTF-8, any width. A one-pass +decode reads them. It holds the same bounds. It reads two levels deep, checks every declared length against the bytes after it before reading them, and allocates nothing until it has read the whole envelope; a complete envelope always declares fewer slots than its diff --git a/src/byte_storage.rs b/src/byte_storage.rs index 2233c3f..32ad97a 100644 --- a/src/byte_storage.rs +++ b/src/byte_storage.rs @@ -618,12 +618,13 @@ impl ByteStorage { /// /// # Errors /// - /// `envelope_bytes` is untrusted. An envelope laid out as `store` writes - /// one (a 4-element array of a bin payload, 8 uint8 checksum bytes, a - /// uint32 size and a str format), whatever width its lengths and size are - /// encoded in, is decoded in one pass that checks each declared length - /// against the input before reading it. Any other input is decoded only - /// after a + /// `envelope_bytes` is untrusted. An envelope in the encodings + /// `decode_one_pass` takes (a fixarray of 4 holding a bin8, bin16 or bin32 + /// payload, a fixarray of 8 checksum bytes each a positive fixint or + /// uint8, a size as a uint of at most 32 bits, and a str format of valid + /// UTF-8), which cover every envelope `store` writes, is decoded in one + /// pass that checks each declared length against the input before reading + /// it. Every input that decode declines is decoded only after a /// structural pre-scan bounds its nesting depth and rejects any header /// that declares more than the input can back (protocol Retrieve Flow, /// step 2). A pre-scan rejection is `DeserializationFailed` whose message @@ -788,7 +789,7 @@ mod borrowed { use super::{extract_payload, ByteStorageError, LARGE_PAYLOAD}; #[derive(Deserialize)] - #[cfg_attr(any(test, fuzzing), derive(Debug, PartialEq))] + #[cfg_attr(any(test, fuzzing), derive(PartialEq))] pub(super) struct StorageEnvelope<'a> { #[serde(borrow, deserialize_with = "borrow_payload")] pub(super) compressed_data: Cow<'a, [u8]>, diff --git a/src/msgpack_bounds.rs b/src/msgpack_bounds.rs index bb62253..c67dbef 100644 --- a/src/msgpack_bounds.rs +++ b/src/msgpack_bounds.rs @@ -1,7 +1,8 @@ //! Structural pre-scan for untrusted MessagePack. //! -//! `ByteStorage::retrieve` runs it before the envelope decode; it is exported -//! for SDK bindings that decode untrusted MessagePack themselves. The bounds are +//! `ByteStorage::retrieve` runs it before the fallback envelope decode, on +//! every envelope its one-pass decode declines; it is exported for SDK +//! bindings that decode untrusted MessagePack themselves. The bounds are //! `spec/interop-mode.md` → Decode bounds, pinned by //! `tests/vectors/decode-bounds.json`. diff --git a/tests/decode_bounds_vectors.rs b/tests/decode_bounds_vectors.rs index 8ccbf5e..d7c122f 100644 --- a/tests/decode_bounds_vectors.rs +++ b/tests/decode_bounds_vectors.rs @@ -1,13 +1,14 @@ //! Decode-bounds vectors through `ByteStorage::retrieve`. //! //! `retrieve` is the untrusted envelope decode that cachekit-py and cachekit-ts -//! (NAPI and wasm) reach, so the protocol requires it to pre-scan the envelope -//! bytes before materialising a `StorageEnvelope` (`protocol/spec/wire-format.md` +//! (NAPI and wasm) reach, so the protocol requires it to hold the decode bounds +//! before materialising a `StorageEnvelope` (`protocol/spec/wire-format.md` //! → Retrieve Flow, step 2; the bounds are `spec/interop-mode.md` → Decode -//! bounds). Asserting only that a reject vector fails would not show that: a -//! bare decoder fails on every one of them too, after it has started -//! materialising. Each reject vector must therefore fail with the message -//! prefix that only the pre-scan produces. +//! bounds). Its one-pass decode hands every input it would reject to the +//! pre-scan, so each reject vector must fail with the message prefix that only +//! the pre-scan produces. Asserting only that a reject vector fails would not +//! show that: a bare decoder fails on every one of them too, after it has +//! started materialising. //! //! Fixture provenance: vendored from //! `test-vectors/decode-bounds.json` From 75a0f84191949b66d58ab1d555c4d668954612ac Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 31 Oct 2026 12:47:44 +0000 Subject: [PATCH 06/10] perf(byte-storage): keep extract_payload out of line and the full decode cold Inlined into retrieve beside the one-pass decode, the decompress and checksum loops compiled about 1% longer at 1 MiB under rustc 1.99.1 (perf_ir retrieve/1048576 +1.01%, /65536 +0.68% against the merge-base). One out-of-line extract_payload compiles them once, whatever decode feeds them. The full decode is marked cold: no envelope store writes reaches it. perf_ir against the merge-base, rustc 1.99.1: retrieve/40 -61.6%, /64 -57.4%, /1024 -21.9%, /65536 -0.17%, /1048576 +0.17%; rustc 1.98.1: -61.6%, -57.4%, -21.9%, -0.34%, +0.02%. Every other case is unchanged. --- src/byte_storage.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/byte_storage.rs b/src/byte_storage.rs index 32ad97a..8a467a9 100644 --- a/src/byte_storage.rs +++ b/src/byte_storage.rs @@ -498,7 +498,11 @@ const _: () = assert!(LITERAL_BLOCK_THRESHOLD <= 0x80); /// Decompress and verify one envelope's payload. `StorageEnvelope::extract` /// forwards to it, and `retrieve` and `validate` call it on the borrowed /// decode, so every read path runs the same checks in the same order. +// Out of line, so the decompress and checksum loops compile once, the same +// whichever decode feeds them. Inlined into `retrieve` beside the one-pass +// decode, rustc 1.99.1 built them about 1% longer at 1 MiB (`perf_ir`). #[cfg(all(feature = "compression", feature = "checksum"))] +#[inline(never)] fn extract_payload( compressed_data: &[u8], checksum: &[u8; 8], @@ -714,7 +718,11 @@ fn decode_envelope( /// Decode untrusted envelope bytes: structural pre-scan first, then the typed /// decode. Serde's derive skips an unknown map key with `IgnoredAny`, which /// recurses, so the depth bound has to hold before `rmp_serde` sees the bytes. +// Cold: no envelope `store` writes reaches it (the one-pass decode takes them +// all), and out of line it keeps the pre-scan and serde off `retrieve`'s path. #[cfg(all(feature = "compression", feature = "checksum", feature = "messagepack"))] +#[cold] +#[inline(never)] fn decode_envelope_full( envelope_bytes: &[u8], ) -> Result, ByteStorageError> { From 83450875647ed92ca3a2cbef20f3f4ab719a8625 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 31 Oct 2026 13:26:02 +0000 Subject: [PATCH 07/10] =?UTF-8?q?fix:=20address=20coderabbit=20review=20?= =?UTF-8?q?=E2=80=94=20pre-scan=20error=20scope,=20size=20cap=20in=20the?= =?UTF-8?q?=20differential=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SECURITY.md said every rejection carries the decode pre-scan prefix; an input that passes the pre-scan and fails the typed decode carries rmp_serde's message instead. README and SECURITY.md now say "a pre-scan rejection". assert_one_pass_matches_full_decode expects InputTooLarge above MAX_COMPRESSED_SIZE, as retrieve returns before decoding. CodeRabbit-Resolved: SECURITY.md:193:Limit the prefix claim to p CodeRabbit-Resolved: byte_storage.rs:775:Skip the full-decode compa --- README.md | 2 +- SECURITY.md | 7 ++++--- src/byte_storage.rs | 12 ++++++++---- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 85571d9..919e869 100644 --- a/README.md +++ b/README.md @@ -262,7 +262,7 @@ decoding: nesting deeper than 100 levels, headers declaring more elements or bytes than the input can back, the reserved marker `0xc1` and truncated input are all rejected before decoding, without allocating in proportion to any declared length. A -rejection is `ByteStorageError::DeserializationFailed` with the message prefix +pre-scan rejection is `ByteStorageError::DeserializationFailed` with the message prefix `decode pre-scan: `. The one-pass read accepts only envelopes the pre-scan and the typed decode accept, into the same fields, and leaves every rejection to them, so every input gets their result and error. The pre-scan's walk is public as diff --git a/SECURITY.md b/SECURITY.md index c72f882..71414dc 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -190,7 +190,8 @@ length. It accepts only envelopes the pre-scan and `rmp_serde` accept, into the same fields, which `one_pass_decode_matches_full_decode` and the `byte_storage_corrupted_envelope` fuzz target check. It hands every other input, including every input it would reject, to the pre-scan and `rmp_serde`, -so every rejection carries the error below. +so a rejection is theirs: a pre-scan rejection carries the error below, and an +`rmp_serde` rejection is `DeserializationFailed` with `rmp_serde`'s message. The pre-scan rejects: @@ -207,8 +208,8 @@ crate, would set how deep the decode recurses. All counts are `u64`, and the walk skips str/bin/ext payloads by offset: it allocates one `u64` per open collection and nothing proportional to a declared length. -A rejection is `ByteStorageError::DeserializationFailed` with a message that -starts with `decode pre-scan: `, the same variant as any other bytes that do +A pre-scan rejection is `ByteStorageError::DeserializationFailed` with a message +that starts with `decode pre-scan: `, the same variant as any other bytes that do not decode as an envelope, so bindings that map on the variant see no change. Trailing bytes after the envelope are still ignored, as before. diff --git a/src/byte_storage.rs b/src/byte_storage.rs index 8a467a9..ad02ab2 100644 --- a/src/byte_storage.rs +++ b/src/byte_storage.rs @@ -765,10 +765,14 @@ pub fn assert_one_pass_matches_full_decode(envelope_bytes: &[u8]) -> (bool, bool } (None, _) => {} } - let expected = full - .as_ref() - .map_err(Clone::clone) - .and_then(|envelope| Ok((envelope.extract()?, envelope.format.clone()))); + // `retrieve` refuses an input over the size cap before decoding it. + let expected = if envelope_bytes.len() > MAX_COMPRESSED_SIZE { + Err(ByteStorageError::InputTooLarge) + } else { + full.as_ref() + .map_err(Clone::clone) + .and_then(|envelope| Ok((envelope.extract()?, envelope.format.clone()))) + }; let retrieved = ByteStorage::new(None).retrieve(envelope_bytes); assert!( retrieved == expected, From 476c63729d874eaa06f022469737b44242405490 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 31 Oct 2026 16:06:27 +0000 Subject: [PATCH 08/10] docs(security): state that decode_envelope pre-scans only before rmp_serde After the merge, SECURITY.md said decode_envelope always runs the pre-scan. It runs it only on the fallback, before rmp_serde; the next paragraph covers the one-pass decode. --- SECURITY.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 501f6c9..208e3a8 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -173,9 +173,9 @@ cover only those two proofs. ### Envelope decode bounds `ByteStorage::retrieve`, `ByteStorage::retrieve_data` and `ByteStorage::validate` -decode envelope bytes that come from a backend the caller may not control. Before -`rmp_serde` materialises a `StorageEnvelope`, all three run one decode path, -`decode_envelope`, which runs a header-only structural pre-scan over those bytes +decode envelope bytes that come from a backend the caller may not control. All +three run one decode path, `decode_envelope`. Before `rmp_serde` materialises a +`StorageEnvelope`, it runs a header-only structural pre-scan over those bytes (protocol `spec/wire-format.md` → Retrieve Flow, step 2; the bounds themselves are `spec/interop-mode.md` → Decode bounds). From f40f8ee2d5b95a99a9f19f09710d3f33b6046464 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 31 Oct 2026 16:40:29 +0000 Subject: [PATCH 09/10] docs(byte-storage): scope the fallback and size-cap wording; pin the zero-allocation read The decode_envelope_full rustdoc now names it the fallback behind decode_envelope's size cap, and its cold comment and extract_payload's out-of-line comment name every caller. The README limits "every input gets their result and error" to input within the size cap. The size-cap and ratio allocation test now asserts the read requests nothing, and names the test that shows the probe counts inside a read. --- README.md | 2 +- src/byte_storage.rs | 18 +++++++++++------- src/read_allocation_probe.rs | 8 +++++--- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 518f0c2..ef8a705 100644 --- a/README.md +++ b/README.md @@ -269,7 +269,7 @@ before decoding, without allocating in proportion to any declared length. A pre-scan rejection is `ByteStorageError::DeserializationFailed` with the message prefix `decode pre-scan: `. The one-pass read accepts only envelopes the pre-scan and the typed decode accept, into the same fields, and leaves every rejection to -them, so every input gets their result and error. The pre-scan's walk is public as +them, so every input within the size cap gets their result and error. The pre-scan's walk is public as `check_msgpack_structure(bytes, max_depth)` for callers that decode untrusted MessagePack themselves; it returns a `MsgpackStructureError` whose `Display` is the bare reason, with no prefix. `msgpack_document_len(bytes, max_depth)` runs diff --git a/src/byte_storage.rs b/src/byte_storage.rs index cbe81b8..786e5ae 100644 --- a/src/byte_storage.rs +++ b/src/byte_storage.rs @@ -500,9 +500,11 @@ const _: () = assert!(LITERAL_BLOCK_THRESHOLD <= 0x80); /// forwards to it, and `retrieve` and `retrieve_data` (and through it /// `validate`) call it on the borrowed decode, so every read path runs the /// same checks in the same order. -// Out of line, so the decompress and checksum loops compile once, the same -// whichever decode feeds them. Inlined into `retrieve` beside the one-pass -// decode, rustc 1.99.1 built them about 1% longer at 1 MiB (`perf_ir`). +// Out of line, so the decompress and checksum loops compile once for all +// three callers (`retrieve`, `retrieve_data`, `StorageEnvelope::extract`), +// the same whichever decode feeds them. Inlined into `retrieve` beside the +// one-pass decode, rustc 1.99.1 built them about 1% longer at 1 MiB +// (`perf_ir`). #[cfg(all(feature = "compression", feature = "checksum"))] #[inline(never)] fn extract_payload( @@ -724,11 +726,13 @@ fn decode_envelope( } } -/// Decode untrusted envelope bytes: structural pre-scan first, then the typed -/// decode. Serde's derive skips an unknown map key with `IgnoredAny`, which -/// recurses, so the depth bound has to hold before `rmp_serde` sees the bytes. +/// The fallback decode behind `decode_envelope`'s size cap: structural +/// pre-scan first, then the typed decode. Serde's derive skips an unknown map +/// key with `IgnoredAny`, which recurses, so the depth bound has to hold +/// before `rmp_serde` sees the bytes. // Cold: no envelope `store` writes reaches it (the one-pass decode takes them -// all), and out of line it keeps the pre-scan and serde off `retrieve`'s path. +// all), and out of line it keeps the pre-scan and serde off the `retrieve` +// and `retrieve_data` path. #[cfg(all(feature = "compression", feature = "checksum", feature = "messagepack"))] #[cold] #[inline(never)] diff --git a/src/read_allocation_probe.rs b/src/read_allocation_probe.rs index a811a0b..ee91acd 100644 --- a/src/read_allocation_probe.rs +++ b/src/read_allocation_probe.rs @@ -136,11 +136,13 @@ mod tests { let (result, requested) = retrieve_counted(&envelope); assert_eq!(result.err(), Some(expected), "[{name}] wrong rejection"); // The read allocates nothing here: the one-pass decode borrows - // these vectors' bin payload and their format. So check on its - // own that the probe counts this thread's allocations; one that - // does not would pass the bound below vacuously. + // these vectors' bin payload and their format. A zero count + // cannot show the probe was live, so this checks that it counts + // this thread's allocations, and `probe_catches_a_reserve_first_reader` + // shows that it counts them inside a read. let ((), live) = bytes_requested(|| drop(std::hint::black_box(vec![0u8; 1]))); assert!(live > 0, "[{name}] probe saw no allocation at all"); + assert_eq!(requested, 0, "[{name}] the read allocated"); assert!( requested < original_size, "[{name}] the read requested {requested} B from the allocator, \ From 98035c3087e5998e79bea8f5f4f461cc71bd4439 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 31 Oct 2026 16:49:45 +0000 Subject: [PATCH 10/10] docs(bench): restate the retrieve_data LTO cost for the one-pass decode With the one-pass decode, a second caller costs retrieve 26 to 27 Ir/op at 40 B to 1 KiB under fat LTO (2.8% at 40 B, rustc 1.99.1), measured by building perf_ir with and without the retrieve_data case. The figure merged from main, 68 to 77, was measured on the pre-scan plus serde decode, which reproduces at 67 to 72 on rustc 1.99.1. --- README.md | 2 +- benches/perf_ir.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index ef8a705..90fce53 100644 --- a/README.md +++ b/README.md @@ -371,7 +371,7 @@ The Criterion suite in `benches/hot_path.rs` needs the `encryption` feature (`ma | `tenant_keyring_decrypt/` | `TenantKeyring::decrypt_indexed`, key derived once at `for_tenant` | | `hkdf` | `derive_domain_key` (takes no payload, so it has no size) | -Sizes are 64 B, 1 KiB and 64 KiB. `store`, `retrieve` and `retrieve_data` also run at 40 B, below the 64-byte literals-only block threshold, and at 1 MiB. `ByteStorage` switches to its copying allocation pattern once the envelope or the compressed payload reaches 64 KiB. The 64 KiB payload compresses to a 24,702-byte envelope and never gets there; the 1 MiB one compresses to 383,893 bytes. One case at a time: `make perf-ir ARGS="--case store/1024"`. The bench binary is built with fat LTO, so its cases shape each other's code. Because both `retrieve` and `retrieve_data` call the envelope decode, it stays out of line, and `retrieve` costs 68 to 77 Ir/op more at 40 B to 1 KiB (2.6% at 40 B) than in a binary that calls `retrieve` alone. A consumer that calls only one of the two gets the decode inlined. +Sizes are 64 B, 1 KiB and 64 KiB. `store`, `retrieve` and `retrieve_data` also run at 40 B, below the 64-byte literals-only block threshold, and at 1 MiB. `ByteStorage` switches to its copying allocation pattern once the envelope or the compressed payload reaches 64 KiB. The 64 KiB payload compresses to a 24,702-byte envelope and never gets there; the 1 MiB one compresses to 383,893 bytes. One case at a time: `make perf-ir ARGS="--case store/1024"`. The bench binary is built with fat LTO, so its cases shape each other's code. Because both `retrieve` and `retrieve_data` call the envelope decode, it stays out of line, and `retrieve` costs 26 to 27 Ir/op more at 40 B to 1 KiB (2.8% at 40 B, rustc 1.99.1) than in a binary that calls `retrieve` alone. A consumer that calls only one of the two gets the decode inlined. **Method** (`benches/perf_ir.rs`): each case runs in its own process under `valgrind --tool=cachegrind --cache-sim=no`, once with N calls and once with none, after the same setup and one warm-up call. N is 1,000, or 20 for a 1 MiB case. Ir/op is `(Ir[N] - Ir[0]) / N`, so process start, setup and exit cancel. The counts are exact, so N sets the run time, not the precision. The measured process gets an empty environment, so neither the shell nor a `VALGRIND_OPTS` changes what is counted. diff --git a/benches/perf_ir.rs b/benches/perf_ir.rs index aa37a31..d0e26da 100644 --- a/benches/perf_ir.rs +++ b/benches/perf_ir.rs @@ -156,7 +156,7 @@ fn operation(case: &str) -> Box usize> { "retrieve" => Box::new(move || storage.retrieve(black_box(&envelope)).unwrap().0.len()), // What the bindings call: retrieve without the format String. Under // fat LTO this second caller keeps decode_envelope out of line, which - // costs retrieve 68 to 77 Ir/op at 40 B to 1 KiB (README). + // costs retrieve 26 to 27 Ir/op at 40 B to 1 KiB (README). "retrieve_data" => { Box::new(move || storage.retrieve_data(black_box(&envelope)).unwrap().len()) }