perf(byte-storage): decode the envelope in one bounded pass on retrieve (LAB-12100)

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.
 #

README.md

@@ -258,13 +258,18 @@ Malicious payloads claiming `original_size: 500GB` with 100 bytes of data are re
 <details>
 <summary><strong>Envelope Decode Bounds</strong></summary>
 
-`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/<size>`, `retrieve/<size>` | `ByteStorage::store` / `retrieve` on the realistic msgpack payload `hot_path` uses |
 | `retrieve_data/<size>` | `ByteStorage::retrieve_data`, the bindings' call, on the same envelope as `retrieve` |
-| `prescan/<size>` | `check_msgpack_structure` on the envelope, the pre-scan `retrieve` runs first |
+| `prescan/<size>` | `check_msgpack_structure` on the envelope, the pre-scan `retrieve` and `retrieve_data` run on every envelope their one-pass read declines |
 | `encrypt/<size>`, `decrypt/<size>` | `ZeroKnowledgeEncryptor` AES-256-GCM |
 | `keyring_decrypt/<size>` | `Keyring::decrypt_indexed`, which re-derives the tenant key (HKDF) on every call |
 | `tenant_keyring_decrypt/<size>` | `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.
 

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.
 

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<String> {
@@ -156,7 +156,7 @@ fn operation(case: &str) -> Box<dyn FnMut() -> 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())
         }

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)

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::<StorageEnvelope>(data) {
+        let _ = envelope.extract();
+    }
 
-    // Try to deserialize fuzz input as MessagePack StorageEnvelope
-    match rmp_serde::from_slice::<StorageEnvelope>(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);
 });