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 88ab460..90fce53 100644 --- a/README.md +++ b/README.md @@ -258,13 +258,18 @@ Malicious payloads claiming `original_size: 500GB` with 100 bytes of data are re
Envelope Decode Bounds -`retrieve()`, `retrieve_data()` 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()`, `retrieve_data()` 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 +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 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 @@ -283,7 +288,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") @@ -360,13 +365,13 @@ The Criterion suite in `benches/hot_path.rs` needs the `encryption` feature (`ma |:-----|:-------------------| | `store/`, `retrieve/` | `ByteStorage::store` / `retrieve` on the realistic msgpack payload `hot_path` uses | | `retrieve_data/` | `ByteStorage::retrieve_data`, the bindings' call, on the same envelope as `retrieve` | -| `prescan/` | `check_msgpack_structure` on the envelope, the pre-scan `retrieve` runs first | +| `prescan/` | `check_msgpack_structure` on the envelope, the pre-scan `retrieve` and `retrieve_data` run on every envelope their 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` | | `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/SECURITY.md b/SECURITY.md index 822e3f5..208e3a8 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -173,11 +173,28 @@ 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). The pre-scan rejects: +are `spec/interop-mode.md` → Decode bounds). + +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 +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 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: | Rule | Bound | |:-----|:------| @@ -192,8 +209,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/benches/perf_ir.rs b/benches/perf_ir.rs index 101a78e..d0e26da 100644 --- a/benches/perf_ir.rs +++ b/benches/perf_ir.rs @@ -103,7 +103,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 { @@ -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()) } diff --git a/fuzz/README.md b/fuzz/README.md index 239c5a6..38bffc9 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; 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** - 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..0ec0a6c 100644 --- a/fuzz/fuzz_targets/byte_storage_corrupted_envelope.rs +++ b/fuzz/fuzz_targets/byte_storage_corrupted_envelope.rs @@ -1,34 +1,39 @@ #![no_main] +use cachekit_core::byte_storage::{assert_one_pass_matches_full_decode, StorageEnvelope}; +use cachekit_core::ByteStorage; use libfuzzer_sys::fuzz_target; -use cachekit_core::byte_storage::StorageEnvelope; + +// 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]| { - // Attack: Attempt to deserialize arbitrary bytes as StorageEnvelope - // Validates: Safe rejection of corrupted MessagePack serialization without panics + // 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(); + } - // 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 Some((edits, value)) = rest.split_at_checked(usize::from(plan % 4) * 3) 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 + assert_one_pass_matches_full_decode(&envelope); }); diff --git a/src/byte_storage.rs b/src/byte_storage.rs index 0469742..786e5ae 100644 --- a/src/byte_storage.rs +++ b/src/byte_storage.rs @@ -209,12 +209,13 @@ fn compress_with_thread_table(data: &[u8]) -> Result, ByteStorageError> /// Storage envelope for raw byte storage /// Contains compressed data with integrity checking // `retrieve`, `retrieve_data` and `validate` decode into its private borrowing twin, -// `borrowed::StorageEnvelope`. `store` encodes from another, `EnvelopeRef`, +// `borrowed::StorageEnvelope`, whose `decode_one_pass` hard-codes this field +// order and these encodings. `store` encodes from another, `EnvelopeRef`, // and writes the same bytes by hand for an input under 64 bytes with a format // of at most 255 bytes (`encode_literal_envelope`, sized by // `envelope_encoded_len`). Change all of them together; -// `literal_envelope_matches_rmp_serde` and `encoded_len_matches_rmp_serde` -// fail on a mismatch. +// `literal_envelope_matches_rmp_serde`, `encoded_len_matches_rmp_serde` and +// `one_pass_decode_takes_every_canonical_envelope` fail on a mismatch. #[derive(Serialize, Deserialize)] pub struct StorageEnvelope { /// Compressed payload data @@ -499,7 +500,13 @@ 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 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( compressed_data: &[u8], checksum: &[u8; 8], @@ -619,12 +626,18 @@ 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 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 + /// starts with `decode pre-scan: `, the same variant as any other bytes + /// that do not decode as a `StorageEnvelope`. /// /// A caller that does not use the format identifier should call /// [`retrieve_data`](Self::retrieve_data), which skips allocating it. @@ -693,18 +706,39 @@ impl Default for ByteStorage { } } -/// Decode untrusted envelope bytes: size cap, structural pre-scan, 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. +/// Decode untrusted envelope bytes: size cap, then `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> { - // Security: Check envelope size before deserializing + // Security: Check envelope size before deserializing. Ahead of both + // decodes: the one-pass decode ignores trailing bytes, so it would take an + // over-cap input that starts with a valid envelope. if envelope_bytes.len() > MAX_COMPRESSED_SIZE { return Err(ByteStorageError::InputTooLarge); } + match borrowed::StorageEnvelope::decode_one_pass(envelope_bytes) { + Some(envelope) => Ok(envelope), + None => decode_envelope_full(envelope_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 the `retrieve` +// and `retrieve_data` path. +#[cfg(all(feature = "compression", feature = "checksum", feature = "messagepack"))] +#[cold] +#[inline(never)] +fn decode_envelope_full( + envelope_bytes: &[u8], +) -> Result, ByteStorageError> { // Nesting bound for the envelope decode. The protocol requires 32..=1024; // 100 matches cachekit-rs and cachekit-ts. A legitimate envelope nests 2 deep. const MAX_DEPTH: usize = 100; @@ -715,6 +749,51 @@ fn decode_envelope( .map_err(|e| ByteStorageError::DeserializationFailed(e.to_string())) } +/// 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", + feature = "checksum", + feature = "messagepack" +))] +#[doc(hidden)] +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, _) => {} + } + // `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.to_string()))) + }; + 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`, `retrieve_data` and `validate` decode: `StorageEnvelope` /// with `compressed_data` borrowed from the input instead of copied out of it, /// below `LARGE_PAYLOAD`, and `format` borrowed, so only `retrieve`, which @@ -737,6 +816,7 @@ mod borrowed { use super::{extract_payload, ByteStorageError, LARGE_PAYLOAD}; #[derive(Deserialize)] + #[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]>, @@ -748,11 +828,95 @@ mod borrowed { pub(super) format: Cow<'a, str>, } - 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) } + + /// 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 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 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 + /// 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_one_pass(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)?, + 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()?; + Some(StorageEnvelope { + compressed_data: borrow_below_large(payload), + checksum, + original_size, + format: Cow::Borrowed(format), + }) + } + } + + 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` @@ -1190,11 +1354,18 @@ mod tests { fn test_envelope_size_validation() { let storage = ByteStorage::new(None); - // Create oversized envelope bytes - let oversized_envelope = vec![0u8; MAX_COMPRESSED_SIZE + 1]; + // Oversized envelope bytes that start with a valid envelope: the + // one-pass decode ignores trailing bytes, so only the size cap ahead + // of it rejects them. + let mut oversized_envelope = storage.store(b"data", None).unwrap(); + oversized_envelope.resize(MAX_COMPRESSED_SIZE + 1, 0); let result = storage.retrieve(&oversized_envelope); assert!(matches!(result, Err(ByteStorageError::InputTooLarge))); + assert_eq!( + storage.retrieve_data(&oversized_envelope), + Err(ByteStorageError::InputTooLarge) + ); } #[test] @@ -1319,6 +1490,137 @@ 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, str8, str16 and str32 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, 256), + (3, 3, 0x1_0000), + (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_one_pass(&bytes).is_some(), + "declined {:02x?}", + &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() { + let name = v["name"].as_str().unwrap(); + let bytes = hex::decode(v["envelope_hex"].as_str().unwrap()).unwrap(); + 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}]"); + } + } + + #[test] + fn one_pass_decode_matches_full_decode() { + // `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]| 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() { + 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 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); + + 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 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 { + 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, uint64, 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 retrieve_data_matches_retrieve() { // Same data and same error for every input; only the format is dropped. @@ -1338,27 +1640,32 @@ 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(); - assert!( - matches!(envelope.format, Cow::Borrowed(_)), - "[{name}] format copied" - ); - if name.ends_with("_bin") { - seen.0 += 1; + 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(); 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" + matches!(envelope.format, Cow::Borrowed(_)), + "[{name}] format copied" ); + 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!( @@ -1371,18 +1678,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}"); } } diff --git a/src/msgpack_bounds.rs b/src/msgpack_bounds.rs index c0e0a6c..9d1a98a 100644 --- a/src/msgpack_bounds.rs +++ b/src/msgpack_bounds.rs @@ -1,8 +1,9 @@ //! Structural pre-scan for untrusted MessagePack. //! //! `ByteStorage`'s shared read path (`decode_envelope`, under `retrieve`, -//! `retrieve_data` and `validate`) runs it before the envelope decode; it is exported -//! for SDK bindings that decode untrusted MessagePack themselves. The bounds are +//! `retrieve_data` and `validate`) 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/src/read_allocation_probe.rs b/src/read_allocation_probe.rs index 8f4fe49..ee91acd 100644 --- a/src/read_allocation_probe.rs +++ b/src/read_allocation_probe.rs @@ -17,9 +17,10 @@ //! this read's alone. //! //! The same probe pins what an accepted msgpack-`bin` read below -//! `LARGE_PAYLOAD` allocates: `retrieve_data` requests only what the pre-scan -//! and the LZ4 decoder do, and `retrieve` adds the format `String` and nothing -//! else. A legacy int-array payload, and a payload from `LARGE_PAYLOAD` up, +//! `LARGE_PAYLOAD` allocates: `retrieve_data` requests only what the LZ4 +//! decoder does (the one-pass decode reads these envelopes without +//! allocating), and `retrieve` adds the format `String` and nothing else. A +//! legacy int-array payload, and a payload from `LARGE_PAYLOAD` up, //! are copied out of the input by design. //! //! The same count, [`bytes_requested`], also pins the write side: the @@ -134,10 +135,14 @@ mod tests { let (envelope, original_size) = reject_vector(name); let (result, requested) = retrieve_counted(&envelope); assert_eq!(result.err(), Some(expected), "[{name}] wrong rejection"); - // The read allocates (the pre-scan's nesting stack; these - // vectors' bin payload is borrowed), so a probe that sees - // nothing is not counting this thread's allocations. - assert!(requested > 0, "[{name}] probe saw no allocation at all"); + // The read allocates nothing here: the one-pass decode borrows + // 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, \ @@ -146,12 +151,13 @@ mod tests { } } - /// On every `msgpack bin` vector, `retrieve_data` allocates what the - /// pre-scan and the LZ4 decoder do on their own and nothing else, and - /// `retrieve` adds only the format `String`. A legacy int-array payload is - /// copied out by design, so those vectors are skipped. + /// On every `msgpack bin` vector, `retrieve_data` allocates what the LZ4 + /// decoder does on its own and nothing else, since the one-pass decode + /// takes every such vector without allocating, and `retrieve` adds only + /// the format `String`. A legacy int-array payload is copied out by + /// design, so those vectors are skipped. #[test] - fn retrieve_data_allocates_only_prescan_and_output() { + fn retrieve_data_allocates_only_output() { let fixture: serde_json::Value = serde_json::from_str(FIXTURE).expect("wire-format.json fixture must parse"); let storage = ByteStorage::new(None); @@ -166,18 +172,16 @@ mod tests { .expect("envelope_hex must decode"); let decoded: StorageEnvelope = rmp_serde::from_slice(&envelope).expect("vector must decode"); - // The pre-scan's allocation depends on the nesting, not the bound. - let (_, prescan) = bytes_requested(|| crate::check_msgpack_structure(&envelope, 100)); let (_, output) = bytes_requested(|| { lz4_flex::decompress(&decoded.compressed_data, decoded.original_size as usize) }); let (data, requested) = bytes_requested(|| storage.retrieve_data(&envelope)); data.unwrap_or_else(|e| panic!("[{name}] {e}")); - assert_eq!(requested, prescan + output, "[{name}] retrieve_data"); + assert_eq!(requested, output, "[{name}] retrieve_data"); let (full, requested) = bytes_requested(|| storage.retrieve(&envelope)); full.unwrap_or_else(|e| panic!("[{name}] {e}")); let format = decoded.format.len(); - assert_eq!(requested, prescan + output + format, "[{name}] retrieve"); + assert_eq!(requested, output + format, "[{name}] retrieve"); } assert!(seen > 0, "fixture has msgpack bin vectors"); } diff --git a/tests/decode_bounds_vectors.rs b/tests/decode_bounds_vectors.rs index 225a2e0..5eefe3f 100644 --- a/tests/decode_bounds_vectors.rs +++ b/tests/decode_bounds_vectors.rs @@ -2,13 +2,14 @@ //! //! `retrieve` shares the untrusted envelope decode (`decode_envelope`) with //! `retrieve_data` and `validate`, the reads 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` +//! 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`