From 0edb01546fdcf06580083d18cab960ed1182e61f Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Tue, 16 Jun 2026 18:27:38 +1000 Subject: [PATCH 1/3] docs: ORE v2 security review records Review/validation artifacts from the v2 crypto review (zeroize audits for the ZA-0001 fix and PRs 82/83, the constant-time v2-applicability analysis, and the pl/pgSQL + pgcrypto validation of the v2 sigma-MMO comparison). ore-v2 is the forthcoming v2 release/integration branch; the stack rebases on top of this. --- .../2026-06-16-constant-time-analysis-pr79.md | 100 +++++++ .../2026-06-16-mmo-plpgsql-validation.md | 61 ++++ .../2026-06-16-mmo-plpgsql-validation.sql | 52 ++++ docs/reviews/2026-06-16-zeroize-audit-pr79.md | 282 ++++++++++++++++++ docs/reviews/2026-06-16-zeroize-audit-pr82.md | 61 ++++ docs/reviews/2026-06-16-zeroize-audit-pr83.md | 47 +++ 6 files changed, 603 insertions(+) create mode 100644 docs/reviews/2026-06-16-constant-time-analysis-pr79.md create mode 100644 docs/reviews/2026-06-16-mmo-plpgsql-validation.md create mode 100644 docs/reviews/2026-06-16-mmo-plpgsql-validation.sql create mode 100644 docs/reviews/2026-06-16-zeroize-audit-pr79.md create mode 100644 docs/reviews/2026-06-16-zeroize-audit-pr82.md create mode 100644 docs/reviews/2026-06-16-zeroize-audit-pr83.md diff --git a/docs/reviews/2026-06-16-constant-time-analysis-pr79.md b/docs/reviews/2026-06-16-constant-time-analysis-pr79.md new file mode 100644 index 00000000..4345f5e5 --- /dev/null +++ b/docs/reviews/2026-06-16-constant-time-analysis-pr79.md @@ -0,0 +1,100 @@ +# Constant-Time Analysis — ore-rs (PR #79 sweep, with v2 applicability) + +**Date:** 2026-06-16 +**Skill:** Trail of Bits `constant-time-testing` (Phase 1 — static analysis) +**Primary scope:** `feat/ore-v2-core-refactor` (#79) — the legacy `bit2`/`Bit8` +scheme (`KnuthShufflePRP` + `Aes128Prng`). +**Cross-checked against:** `feat/ore-v2-chained` (#83) — the new v2 schemes +(`bit2_w6`/Bit6 + chained), which use `LemireFyPrp<64>`. + +> Phase 2 (dudect statistical) and Phase 3 (timecop/Valgrind dynamic) were **not** +> run: Valgrind has no Apple-Silicon support, and dudect would require adding a +> dev-dependency + a CI-pinned core for stable signal. The static pass below is +> conclusive for the structural channels; dudect would empirically confirm CT-1. + +## Method + +Manual review of the comparator and permutation paths for the three classic +constant-time violation classes: secret-dependent branches, secret-indexed +array/memory accesses, and data-dependent loop bounds. Targets: `scheme/bit2.rs` +(comparator + encrypt), `primitives/prp.rs`, `primitives/prp/prng.rs`, +`scheme/width.rs`. + +## What is correct (legacy and v2) + +The comparator's first-differing-block **prefix scan is genuinely constant-time** +(`bit2.rs:240-247` on #79): it runs all `num_blocks` with `ct_eq` + +`conditional_assign`, **no early exit**, updating `l`/`is_equal` obliviously. The +branch *after* the scan (`if is_equal { return Equal }`) is on the comparison +**result**, which ORE reveals by design — not a secret-dependent timing branch. + +## Findings (legacy `bit2`/Bit8 on #79) + +| ID | Location (#79) | Class | Secret leaked | Severity | +|----|----------------|-------|---------------|----------| +| **CT-1** | `prp/prng.rs:54` `gen_range` rejection loop, driving the shuffle at `prp.rs:43` | data-dependent loop count | PRP seed = `PRF₂(x[0..n])` → **plaintext via encrypt timing** | High | +| **CT-2** | `prp.rs:64` `permute` → `inverse.get(input)` (and `invert` → `permutation.get`) | secret-indexed table read | plaintext byte `x[n]` | Med-High | +| **CT-3** | `prp.rs:44` `permutation.swap(i, j)` (j from `gen_range`) | secret-indexed array write | PRP seed (via keystream `j`) | Medium | +| **CT-4** | `bit2.rs:261` & `:323` `get_bit(block, xt[l])` | data-dependent array index | `xt[l]` (compare-side) | Medium | +| **CT-5** | `bit2.rs:258,260,261` accesses indexed by `l` | index by first-differing block | common-prefix length `l` | Low / by-design | + +Notes: +- **CT-1** is the "shuffle that leaked time": `gen_range` loops `next_byte()` until + `candidate <= max`, and `next_byte` triggers a fresh 16-block AES `generate()` + every 256 draws — so rejection count *and* re-keying frequency depend on the + key/plaintext-derived keystream. +- **CT-2** is mislabeled in source: `permute`/`invert` carry an "in constant time" + comment (`prp.rs:58,70`) but perform plaintext-indexed lookups into a 256-byte + (= 4 cache-line) table. +- **CT-5** is the inherent ORE *online* leakage (common-prefix length), not a defect. + +## Applicability to the new v2 schemes (Bit6 + chained, `LemireFyPrp<64>`) + +Verified against `feat/ore-v2-chained` (#83). + +| Finding | Applies to v2? | Evidence | +|---------|----------------|----------| +| **CT-1** rejection-sampling encrypt-timing | **No — fixed** | `LemireFyPrp` uses fixed-draw Fisher-Yates + Lemire multiply-high; **no rejection loop, no `%`** (`prp.rs:120-130`, `from_stream`). Still present in Bit8/`KnuthShufflePRP` (wire-frozen legacy). | +| **CT-4** compare-side cache-line leak | **No — fixed** | All comparator byte reads route through the oblivious `width::ct_select_byte` (`width.rs:77`): the free `get_bit` in `compare_raw_slices` for bit2/bit2_w6/chained (`bit2.rs:285`, `bit2_w6.rs:294`, `chained.rs:325`) **and** `RightBlock32::get_bit` used by typed `Ord::cmp` (`block_types.rs:45`). The `>> (bit%8)` is a constant-time variable shift. | +| **CT-2** plaintext-indexed `permute` lookup | **Partially — mitigated** | `permute` still does `inverse[input]` (`prp.rs:255`), but `#[repr(C, align(64))]` + `N=64` places `inverse` on exactly one 64-byte cache line → cache-line-constant access. Residual: MemJam 4-byte sub-line only. | +| **CT-3** secret-indexed FY swap (keygen) | **Partially — mitigated** | `permutation.swap(i, j)` (secret `j`) and `inverse[val]=…` (secret `val`) still exist (`prp.rs:211-215`); each `[u8;64]` table is one cache line under `align(64)`, so writes stay line-uniform. Same MemJam sub-line residual. | +| **CT-5** index by first-differing block | **Yes — by design** | Inherent ORE online leakage; applies to every scheme. Not a defect. | + +> **Superseded (2026-10-09):** the CT-2 and CT-3 residuals above are closed. Table reads are oblivious, and a timing study found the Fisher–Yates swap leaking through timing even without a co-tenant, so key generation is now register-resident with no secret address. See the crypto review brief, A4, "Status (2026-10-09)". + +## Bottom line + +- The two **genuinely actionable** legacy findings — **CT-1** (encrypt-timing) and + **CT-4** (compare-side cache leak) — **do not apply to the new v2 schemes.** They + are precisely what the `LemireFyPrp` switch and the `ct_select_byte` hardening + were built to close. +- What remains in v2 (**CT-2 / CT-3**) is **structural-but-mitigated**: the PRP still + performs secret-indexed table accesses, reduced from cache-line leaks to the + **MemJam sub-cache-line** residual (Intel-SMT co-resident attacker only; + ARM/AMD/non-SMT unaffected). This is the documented, deliberate default, with + **oblivious-swap-FY** as the wire-compatible high-assurance opt-in. + +## Caveats / follow-ups + +1. **The mitigation is load-bearing on `LemireFyPrp` staying at `N ≤ 64`.** If a + future width pushed it past 64 — or a `repr` change reordered the + `permutation`/`inverse` fields — CT-2/CT-3 would silently regress from + MemJam-only back to full cache-line leaks. The `const _: () = assert!(domain <= 64)` + (`prp.rs:176`) guards the width; **nothing guards against a `repr` change.** + Consider a comment / test pinning the field layout assumption. +2. **Correct the legacy "constant time" comment.** The + "performs the inverse permutation in constant time" comment is fair for + `LemireFyPrp<64>` at cache-line granularity, but the **identical comment on + `KnuthShufflePRP<256>` (`prp.rs:83` on #83 / `:70` on #79) is inaccurate** — the + 256-byte, unaligned table is a real 4-line lookup. Qualify or remove it so it + doesn't imply a guarantee the legacy type doesn't provide. +3. **dudect (Phase 2)** would empirically confirm CT-1 on the legacy scheme + (fixed-vs-random plaintext encrypt timing, Welch's t-test). Optional; needs a + `dudect-bencher` dev-dep and a pinned core. + +## Related + +- Zeroize audit of the same crate: `docs/reviews/2026-06-16-zeroize-audit-pr79.md` + (ZA-0001: `Aes128Prng` keystream not wiped on drop — a memory-hygiene, not + timing, issue, but in the same `prp/prng.rs`). +- v2 architecture / leakage model: `docs/plans/2026-06-12-ore-v2-architecture.md`. diff --git a/docs/reviews/2026-06-16-mmo-plpgsql-validation.md b/docs/reviews/2026-06-16-mmo-plpgsql-validation.md new file mode 100644 index 00000000..34f9bd85 --- /dev/null +++ b/docs/reviews/2026-06-16-mmo-plpgsql-validation.md @@ -0,0 +1,61 @@ +# Validation: v2 σ-MMO comparison in pl/pgSQL + pgcrypto + +**Date:** 2026-06-16 +**Question:** can the v2 (Bit6/chained) σ-MMO comparison function be implemented +in pl/pgSQL / SQL using only the `pgcrypto` extension, as an interim before the +Rust TLE lands? (The legacy bit2 comparator already lives in +`cipherstash/encrypt-query-language`; domain types separate the operators, so +this work targets the **new v2/MMO scheme**.) +**Answer: yes — validated empirically (12/12 vectors match Rust).** + +## Key finding: the comparison needs no CMAC / PRP / key derivation + +Those are all **encryption-side** (client/Rust). The comparator (`compare_views` +in `scheme/chained.rs`; the Bit6/bit2 `compare_raw_slices`) uses only: + +1. a byte-equality prefix scan over `xt` + the 16-byte `f` tags, +2. the σ-MMO 1-bit hash of the first differing block's left tag `f[l]` under the + stored nonce, and +3. an oblivious bit read of the right block at index `xt[l]`. + +So `pgcrypto` not providing CMAC is irrelevant — CMAC only derives per-block +secrets during encryption. + +## Primitive mapping + +| Need | Implementation | +|---|---| +| π (fixed public-key AES-128) | `encrypt(m, 'ORE-rs.v2.H-pi.1'::bytea, 'aes-ecb/pad:none')` (pgcrypto) — `pad:none` for a raw single block; the `PI_KEY` is public, so it's embedded | +| σ = GF(2¹²⁸) doubling | ~10 lines of pl/pgSQL: byte-wise shift-left-1 + conditional `# 0x87` (`gf128_double` below) — the only non-pgcrypto-native op | +| `m = σ(f) ⊕ nonce`, feedforward, LSB | byte XOR (`get_byte`/`set_byte`/`#`) + `& 1` | +| prefix scan, bit read | plain SQL (`substring`, `get_bit`) | + +## Empirical result + +The pl/pgSQL `mmo_hash_bit(f, nonce)` (see +`2026-06-16-mmo-plpgsql-validation.sql`) was checked against ground-truth from +the Rust `FixedPiZ2Hash::hash` on 12 `(f, nonce)` vectors (H mix +`1,0,0,0,0,1,0,0,0,0,1,0`): + +``` + total | matches | mismatches + 12 | 12 | 0 +``` + +Byte-for-byte agreement. (Postgres 15.12 + pgcrypto.) + +## Caveat — constant-time + +The pl/pgSQL comparator computes the correct order, but pl/pgSQL is **not a +constant-time environment**: the cache-line/sub-line obliviousness the Rust +comparator gets from `ct_select_byte`/`ct_bit` and the no-early-exit prefix scan +is not realistically achievable in SQL. The interim SQL comparator therefore +trades that hardening away; the Rust TLE restores it. Whether that's acceptable +is a threat-model call for the interim (server-side comparison over stored +ciphertexts) — flagged, not decided here. + +## Conclusion + +The requirement is met: the v2 σ-MMO comparison is implementable in pl/pgSQL with +pgcrypto. The encryption-side work (single-key derivation, CMAC accumulator, PRP) +is unaffected by it. Safe to resume the single-key change + Bit6 vector regen. diff --git a/docs/reviews/2026-06-16-mmo-plpgsql-validation.sql b/docs/reviews/2026-06-16-mmo-plpgsql-validation.sql new file mode 100644 index 00000000..a393b955 --- /dev/null +++ b/docs/reviews/2026-06-16-mmo-plpgsql-validation.sql @@ -0,0 +1,52 @@ +-- GF(2^128) doubling σ(x) = 2·x, reduction poly x^128+x^7+x^2+x+1 (0x87), +-- big-endian 16-byte field element. byte-wise shift-left-1 + conditional XOR. +CREATE OR REPLACE FUNCTION gf128_double(x bytea) RETURNS bytea AS $$ +DECLARE r bytea := x; carry int := 0; nc int; i int; topbit int; +BEGIN + topbit := (get_byte(x,0) >> 7) & 1; + FOR i IN REVERSE 15..0 LOOP + nc := (get_byte(r,i) >> 7) & 1; + r := set_byte(r, i, ((get_byte(r,i) << 1) | carry) & 255); + carry := nc; + END LOOP; + IF topbit = 1 THEN + r := set_byte(r, 15, get_byte(r,15) # 135); -- ^ 0x87 + END IF; + RETURN r; +END; $$ LANGUAGE plpgsql IMMUTABLE; + +CREATE OR REPLACE FUNCTION xor16(a bytea, b bytea) RETURNS bytea AS $$ +DECLARE r bytea := a; i int; +BEGIN + FOR i IN 0..15 LOOP r := set_byte(r, i, get_byte(a,i) # get_byte(b,i)); END LOOP; + RETURN r; +END; $$ LANGUAGE plpgsql IMMUTABLE; + +-- BHKR σ-MMO 1-bit hash: m = σ(f) ⊕ nonce; H = lsb(π(m)) ⊕ lsb(m). +-- π = AES-128-ECB under the fixed public key "ORE-rs.v2.H-pi.1". +CREATE OR REPLACE FUNCTION mmo_hash_bit(f bytea, nonce bytea) RETURNS int AS $$ +DECLARE m bytea; pim bytea; +BEGIN + m := xor16(gf128_double(f), nonce); + pim := encrypt(m, 'ORE-rs.v2.H-pi.1'::bytea, 'aes-ecb/pad:none'); + RETURN (get_byte(pim,0) & 1) # (get_byte(m,0) & 1); +END; $$ LANGUAGE plpgsql IMMUTABLE; + +WITH v(data, nonce, expected) AS (VALUES + ('\x00070e151c232a31383f464d545b6269'::bytea,'\x010e1b2835424f5c697683909daab7c4'::bytea,1), + ('\x1f262d343b424950575e656c737a8188'::bytea,'\x121f2c394653606d7a8794a1aebbc8d5'::bytea,0), + ('\x3e454c535a61686f767d848b9299a0a7'::bytea,'\x23303d4a5764717e8b98a5b2bfccd9e6'::bytea,0), + ('\x5d646b727980878e959ca3aab1b8bfc6'::bytea,'\x34414e5b6875828f9ca9b6c3d0ddeaf7'::bytea,0), + ('\x7c838a91989fa6adb4bbc2c9d0d7dee5'::bytea,'\x45525f6c798693a0adbac7d4e1eefb08'::bytea,0), + ('\x9ba2a9b0b7bec5ccd3dae1e8eff6fd04'::bytea,'\x5663707d8a97a4b1becbd8e5f2ff0c19'::bytea,1), + ('\xbac1c8cfd6dde4ebf2f900070e151c23'::bytea,'\x6774818e9ba8b5c2cfdce9f603101d2a'::bytea,0), + ('\xd9e0e7eef5fc030a11181f262d343b42'::bytea,'\x7885929facb9c6d3e0edfa0714212e3b'::bytea,0), + ('\xf8ff060d141b222930373e454c535a61'::bytea,'\x8996a3b0bdcad7e4f1fe0b1825323f4c'::bytea,0), + ('\x171e252c333a41484f565d646b727980'::bytea,'\x9aa7b4c1cedbe8f5020f1c293643505d'::bytea,0), + ('\x363d444b525960676e757c838a91989f'::bytea,'\xabb8c5d2dfecf90613202d3a4754616e'::bytea,1), + ('\x555c636a71787f868d949ba2a9b0b7be'::bytea,'\xbcc9d6e3f0fd0a1724313e4b5865727f'::bytea,0) +) +SELECT count(*) AS total, + count(*) FILTER (WHERE mmo_hash_bit(data,nonce) = expected) AS matches, + count(*) FILTER (WHERE mmo_hash_bit(data,nonce) <> expected) AS mismatches +FROM v; diff --git a/docs/reviews/2026-06-16-zeroize-audit-pr79.md b/docs/reviews/2026-06-16-zeroize-audit-pr79.md new file mode 100644 index 00000000..495a2741 --- /dev/null +++ b/docs/reviews/2026-06-16-zeroize-audit-pr79.md @@ -0,0 +1,282 @@ +# Zeroize Audit Report + +**Run ID:** `c212f1cdd302` +**Timestamp:** `2026-06-15T13:19:26Z` (assembled final: 2026-06-16) +**Repository:** `/Users/dan/Projects/CipherStash/ore.rs` +**Crate:** `ore-rs` v0.8.3 (manifest: `packages/ore-rs/Cargo.toml`) +**Compile DB:** none (Rust mode — crate root `packages/ore-rs/src/lib.rs`, TU hash `6f04dcee`) + +**Configuration:** + +| Setting | Value | +|---|---| +| Language mode | rust | +| Optimization levels | O0, O1, O2 | +| MCP mode | prefer | +| MCP available | yes (serena) | +| MCP required for advanced | yes | +| Assembly analysis | enabled (AArch64, experimental backend) | +| Semantic IR analysis | disabled | +| CFG analysis | disabled | +| Runtime tests | disabled | +| PoC generation | enabled | +| PoC validation | mandatory — completed, verified | + +--- + +## Executive Summary + +A single confirmed, medium-severity, secret-retention defect was found in the `ore-rs` +crate: `Aes128Prng` implements `Zeroize` but never triggers it on drop, leaving the +AES-CTR keystream (`data`) and position state (`ptr`/`ctr`) in the reclaimed stack +frame. The finding is corroborated by source, MIR, and LLVM-IR evidence and proven by a +verified, exploitable PoC (253/256 keystream bytes survive drop versus 0/256 with a +`ZeroizeOnDrop` control). + +Two of the original source claims were narrowed by compiler evidence: the AES **key +schedule** is in fact wiped on drop by the `aes` crate's own `ZeroizeOnDrop` and that +wipe **survives O2 on both the normal and unwind paths** — it is **not** optimized away. +No `OPTIMIZED_AWAY_ZEROIZE`, `STACK_RETENTION`, `REGISTER_SPILL`, or `SECRET_COPY` +defects were confirmed. + +| Metric | Count | +|---|---| +| Files scanned | 15 | +| Translation units analyzed | 1 of 1 | +| Sensitive objects identified | 6 | +| **Total findings** | **1** | + +### By Severity + +| Severity | Count | +|---|---| +| High | 0 | +| Medium | 1 | +| Low | 0 | + +### By Confidence + +| Confidence | Count | +|---|---| +| Confirmed | 1 | +| Likely | 0 | +| Needs review | 0 | + +### By Category + +| Category | Count | +|---|---| +| MISSING_SOURCE_ZEROIZE | 1 | +| OPTIMIZED_AWAY_ZEROIZE | 0 | +| PARTIAL_WIPE | 0 | +| NOT_ON_ALL_PATHS | 0 | +| STACK_RETENTION | 0 | +| REGISTER_SPILL | 0 | +| SECRET_COPY | 0 | +| INSECURE_HEAP_ALLOC | 0 | +| MISSING_ON_ERROR_PATH | 0 | +| LOOP_UNROLLED_INCOMPLETE | 0 | +| NOT_DOMINATING_EXITS | 0 | + +### PoC Validation + +| Metric | Count | +|---|---| +| Total findings | 1 | +| PoCs generated | 1 | +| PoCs validated (compiled + ran) | 1 | +| PoCs verified (claim proven) | 1 | +| Exploitable (confirmed) | 1 | +| Not exploitable | 0 | +| Rejected | 0 | +| Compile failures | 0 | +| No PoC generated | 0 | +| Verification failures | 0 | + +### MCP Availability and Impact + +MCP (serena) was available and used. Because MCP was present, no MCP-unavailable +confidence downgrades were applied to the advanced categories (`SECRET_COPY`, +`MISSING_ON_ERROR_PATH`, `NOT_DOMINATING_EXITS`). The one finding rests on three +independent non-MCP signals (source + MIR + IR) plus a verified PoC, so it would have +remained `confirmed` regardless. + +--- + +## Sensitive Objects Inventory + +| ID | Name | Type | Location | Confidence | Heuristic | Wipe Status | +|---|---|---|---|---|---|---| +| SO-5001 | OreAes128 | struct (prf1/prf2 PRF keys) | `packages/ore-rs/src/scheme/bit2.rs:34` | high | crypto_typed_field | OK — `derive(ZeroizeOnDrop)` → Drop; `rng` is `#[zeroize(skip)]` (correct) | +| SO-5002 | Aes128Prf | struct (aes::Aes128 key schedule) | `packages/ore-rs/src/primitives/prf.rs:7` | high | crypto_typed_field | OK — `derive(ZeroizeOnDrop)` → Drop | +| SO-5003 | Aes128Z2Hash | struct (aes::Aes128 keyed by RO nonce) | `packages/ore-rs/src/primitives/hash.rs:7` | high | crypto_typed_field | OK — `derive(ZeroizeOnDrop)` → Drop | +| SO-5004 | KnuthShufflePRP | struct (perm/inverse from PRP seed) | `packages/ore-rs/src/primitives/prp.rs:7` | high | crypto_typed_field | OK — Zeroize + manual Drop + ZeroizeOnDrop | +| SO-5005 | **Aes128Prng** | struct (aes::Aes128 + keystream) | `packages/ore-rs/src/primitives/prp/prng.rs:5` | high | crypto_typed_field | **FINDING (ZA-0001)** — manual Zeroize, no auto-trigger; data/ptr/ctr not wiped on drop | +| SO-5006 | SeedBuf | struct (key-equivalent PRP seeds) | `packages/ore-rs/src/scheme/bit2.rs:58` | high | sensitive_name_match (Seed) | OK — manual Drop zeroizes each AesBlock seed | + +Five of the six sensitive objects have approved, correctly-triggered wipes. Only +`Aes128Prng` (SO-5005) has an incomplete, never-auto-triggered wipe — the subject of +ZA-0001. Plaintext/ciphertext types (`CipherText`, `Left`, `Right`, `RightBlock32`) and +the width/decompose modules were deliberately excluded from scope as non-secret. + +--- + +## Findings + +### Medium Severity + +#### ZA-0001: MISSING_SOURCE_ZEROIZE — medium (confirmed) + +**Location:** `packages/ore-rs/src/primitives/prp/prng.rs:5` (struct `Aes128Prng`; manual `zeroize` body at prng.rs:12-18; construction at prng.rs:24-36) +**Object:** `Aes128Prng` (SO-5005) — `cipher: aes::Aes128`, `data: [GenericArray;16]` (256 bytes, stack-inline keystream, struct offset 0..256), `ptr: (usize, usize)`, `ctr: u32` + +**Summary.** `Aes128Prng` provides a manual `impl Zeroize` but has **no `Drop` and no +`derive(ZeroizeOnDrop)`**, so the wipe is never auto-triggered. The compiler-synthesised +drop glue delegates only to the `cipher` field. The AES-CTR keystream `data` (a +key-derived secret) and the `ptr`/`ctr` position state therefore persist in the reclaimed +stack frame after the PRNG is dropped. The PRNG is built per-block inside the PRP +(`Aes128Prng::init(key)`, prp.rs:27), so this drop happens frequently during operation. + +**Evidence:** + +- **[source]** prng.rs:12 — `impl Zeroize for Aes128Prng` exists but there is no `impl Drop` or `derive(ZeroizeOnDrop)`, so the manual zeroize is never called automatically. The manual body (prng.rs:13-17) loops over `self.data` only; `cipher`, `ptr`, and `ctr` are untouched. +- **[mir]** `6f04dcee.mir` — `Aes128Prng::zeroize` loops `self.data` only; no `Drop`/`ZeroizeOnDrop` impl present in MIR; the drop glue delegates only to `cipher` (F-RUST-MIR-0003). +- **[ir]** `6f04dcee.O0.ll:503-514` — `drop_in_place::` GEPs only to byte offset 256 (the `cipher` field) and calls `drop_in_place::`; offsets 0..256 (`data`) and `ptr`/`ctr` are never zeroized on the drop path (F-RUST-IR-0002). +- **[ir]** `6f04dcee.O2.ll:1823` — `data` is `@llvm.memset(..., 0, 256, false)` **non-volatile** at construction only; it is never wiped at drop (F-RUST-IR-0003). +- **[asm]** `6f04dcee.O2.s:1481-1491` — AArch64 corroboration that the key-schedule wipe (`cipher`) survives at O2; no zero-store covers `data`/`ptr`/`ctr` on the drop path (F-RUST-ASM-0001, experimental backend, hand-verified). +- **[poc]** PoC confirmed: secret persists after drop (exit code 0); verification passed — 253/256 keystream bytes survive drop versus 0/256 with a `ZeroizeOnDrop` control. + +**Compiler Evidence:** + +- Opt levels analyzed: O0, O1, O2 +- **O0:** `drop_in_place::` (O0.ll:503-514) drops only `cipher` (GEP offset 256); the `aes` `ZeroizeOnDrop` zeroes the 704-byte fixslice key schedule via `<[u64;88] as Zeroize>::zeroize`. `data`/`ptr`/`ctr` are never touched on drop. +- **O1:** Same drop-path structure as O0; manual `Aes128Prng::zeroize` still has no trigger. +- **O2:** The key-schedule wipe is inlined into a `store volatile i64 0` loop (offset 0..704, step 8) followed by `fence syncscope("singlethread") seq_cst` on **both** the normal **and** the unwind/cleanup paths (O2.ll:1854-1856) and **survives**. The `data` keystream is `memset(0)` only at construction (O2.ll:1823, non-volatile), never at drop. +- **Summary:** The `aes::Aes128` `ZeroizeOnDrop` key-schedule wipe is present at O0 and survives (inlined, volatile, fenced) at O2 on all paths — it is **not** optimized away. The genuine residual leak is `Aes128Prng`'s own `data` keystream + `ptr`/`ctr`, which the synthesised drop glue never zeroizes because there is no `Drop`/`ZeroizeOnDrop` to trigger the manual zeroize. + +**PoC Validation:** exploitable (verified) + +- Exit code: 0 (0 = secret persists = exploitable; 1 = wiped = not exploitable) +- PoC files: `poc/ZA-0001_missing_source_zeroize.rs` (test), `poc/ZA-0001_missing_source_zeroize_bin.rs` (binary), shared replica `poc/za_0001_replica.rs` +- Compiled and ran at debug (-O0) and release; deterministic. +- Verified: **yes** — all six verification checks passed (target_variable, target_function, technique, optimization_level, exit_code_interpretation, result_plausibility). +- Result: 253/256 keystream bytes survive drop with the verbatim (no-trigger) impl; the `Aes128PrngFixed` control with `ZeroizeOnDrop` yields 0/256, demonstrating both the leak and that the recommended fix closes it. +- Technique: version-pinned byte-for-byte replica (the real type is crate-private), `ManuallyDrop` + `ptr::drop_in_place` to run exactly the synthesised destructor, then `ptr::read_volatile` of the keystream storage — the correct primitive for a Rust `MISSING_SOURCE_ZEROIZE` PoC. Deps pinned to `aes=0.8.2` (zeroize feature) and `zeroize=1.5.7`, matching `ore-rs/Cargo.toml`. + +**Recommended Fix:** + +Add `#[derive(zeroize::ZeroizeOnDrop)]` to `Aes128Prng` so the manual `Zeroize` impl is +auto-triggered on drop, **and** extend the `zeroize` body to wipe all secret-derived +state, not just `data`: + +- keep the existing `data` keystream loop; +- additionally clear the position state, e.g. `self.ptr = (0, 0); self.ctr = 0;`. + +Do **not** manually re-wipe `cipher`: the AES key schedule is already zeroized by the +`aes` crate's own `ZeroizeOnDrop` (enabled via `aes = { features = ["zeroize"] }`), so +deriving `ZeroizeOnDrop` on `Aes128Prng` composes cleanly with it. After the change, +confirm at the IR layer that the synthesised drop glue GEPs offset 0..256 and zeroes the +keystream. The PoC's `Aes128PrngFixed` control already demonstrates this fix yields 0/256 +surviving bytes. + +--- + +## PoC Validation Results + +| Finding | Category | PoC File | Exit Code | Result | Verified | Impact | +|---|---|---|---|---|---|---| +| ZA-0001 | MISSING_SOURCE_ZEROIZE | `poc/ZA-0001_missing_source_zeroize_bin.rs` | 0 | exploitable | Yes | Confirmed — verified exploitable PoC reinforces `confirmed` confidence (253/256 keystream bytes survive drop vs 0/256 control) | + +--- + +## Superseded Findings + +Two source-level findings were partially superseded by IR/ASM hard evidence and re-scoped, +then folded into the single retained finding ZA-0001. No finding was silently dropped. + +| Superseded | Superseded By | Reason | +|---|---|---| +| F-RUST-SRC-0001 (MISSING_SOURCE_ZEROIZE, high) | F-RUST-IR-0002 (→ ZA-0001) | Source claimed "on drop nothing is wiped." IR/ASM show the synthesised drop glue **does** wipe the AES key schedule (`cipher`) via the `aes` crate's `ZeroizeOnDrop` (volatile loop + fence, surviving O2 on normal and unwind paths). The residual valid part — manual `zeroize` never triggered, `data`/`ptr`/`ctr` not wiped — was retained and re-scoped from high to medium. | +| F-RUST-SRC-0002 (PARTIAL_WIPE, medium) | F-RUST-IR-0002 (→ ZA-0001) | Source claimed `Aes128Prng::zeroize` ignores `cipher`, leaving AES round keys in memory. The manual zeroize does ignore `cipher` (confirmed F-RUST-MIR-0003), but it is never auto-triggered anyway, and the actual drop path wipes the 704-byte key schedule (F-RUST-IR-0002). So "AES round keys remain in memory" is **mitigated** for the drop case. The accurate residual gap is the `data` keystream (F-RUST-IR-0003) + `ptr`/`ctr`. Also corrected: `data` is a stack-inline array, not a heap Vec. | + +--- + +## Confidence Gate Summary + +| Finding / ID | Action | Reason | +|---|---|---| +| ZA-0001 | Confirmed (held) | 3+ independent signals agree on the residual gap: [source] manual Zeroize with no trigger, [mir] drop glue delegates only to `cipher`, [ir] `data` never wiped at drop + `drop_in_place` GEPs only `cipher`. ≥2 signals → confirmed. MCP available, so no MCP-unavailable downgrade. A verified exploitable PoC is a strong additional signal (can upgrade likely→confirmed; ZA-0001 was already confirmed). Severity set to medium because the primary key schedule **is** protected; residual exposure is the key-derived CTR keystream + counters (lower impact than raw key leakage). | +| F-RUST-IR-0001 (OPTIMIZED_AWAY_ZEROIZE, high) | Refuted — dropped from active findings | Automated false positive. `OPTIMIZED_AWAY_ZEROIZE` requires IR-diff evidence that a wipe was removed. Here the per-symbol volatile-store drop in the standalone `write_volatile::` helper is an **inlining artifact**: crate-wide `store volatile` count **rises** O0→O2 (2 → 138). No wipe was eliminated. Recorded in the refuted bucket for the audit trail. **No OPTIMIZED_AWAY_ZEROIZE finding is emitted in this run.** | +| F-RUST-MIR-0001 / F-RUST-MIR-0002 (hash_key) | Refuted — dropped from active findings | `hash_key` is `&GenericArray`, a borrow of caller-owned `b_right`, not an owned secret. No owned sensitive local leaks on unwind/Err paths. | +| F-RUST-ASM-0001 (info) | Corroborating-only — not emitted as a finding | Positive machine-level corroboration of F-RUST-IR-0002: the AArch64 checker returned 0 STACK_RETENTION / REGISTER_SPILL findings and the key-schedule volatile wipe survives to O2 machine code. This is evidence the key schedule **is** wiped, not a leak. (Experimental AArch64 backend; hand-verified.) No `STACK_RETENTION`/`REGISTER_SPILL` finding is created because no asm evidence of a non-wiped secret exists. | + +No rationalization-override attempts were made or rejected during this run. + +--- + +## Analysis Coverage + +| Metric | Value | +|---|---| +| TUs in scope | 1 (`packages/ore-rs/src/lib.rs`, hash `6f04dcee`, lib `ore-rs`) | +| TUs analyzed | 1 / 1 | +| TUs with sensitive objects | 1 | +| Module files scanned | 15 | +| Agent 1 (MCP/preflight resolver) | success — serena MCP available; nightly 1.95.0, uv 0.8.17; MIR/IR/ASM emit all passed | +| Agent 2b (Rust source analyzer) | success (with tool limitation — see below) — 6 sensitive objects, 2 source findings | +| Agent 3b (Rust compiler analyzer) | success — MIR + IR (O0/O1/O2) + ASM (O2) analyzed | +| Agent 4 (report assembler) | success (interim + final) | +| Agent 5 (PoC generator) | success — 1 PoC generated, compiled, validated, and verified | +| Agent 6 (test generator) | skipped — runtime tests disabled in config | + +**Features and impact:** + +- **Semantic IR analysis:** disabled (preflight `enable_semantic_ir=false`). No loop-unrolling / phi-node / SSA findings were produced; not required for this finding, which is grounded in drop-glue structure. +- **CFG analysis:** disabled (preflight `enable_cfg=false`). No dominator / all-paths findings. The unwind-path coverage that would normally come from CFG was instead established directly from the O2 IR (volatile wipe present on both normal and unwind paths). +- **Runtime tests:** disabled. PoC-based runtime validation was performed instead and is sufficient to confirm the finding. +- **Assembly analysis:** enabled on an **experimental AArch64 backend**; used only as positive corroboration (F-RUST-ASM-0001) and flagged for manual re-check. + +**Tooling incompatibility encountered during the run (actionable):** + +`semantic_audit.py` is **incompatible with rustdoc `format_version: 57`**. The current +nightly rustdoc JSON nests the item `kind` under `inner.{struct,enum}` instead of a +top-level `item['kind']`, and expands `#[derive(...)]` into the impls list rather than +emitting raw `attrs` strings. The shipped script reads the legacy layout, so it silently +**skipped all 14 structs/enums and produced 0 raw findings**. The two source findings +(F-RUST-SRC-0001/0002) and the 6 sensitive objects were recovered by a manual, +trait-aware pass over the same JSON. Recommended fix to the tool: (1) derive `kind` from +`inner` keys; (2) read derived traits from `impl.trait.path` in the impls list. This did +not reduce coverage for this run, but would cause silent under-reporting on future runs if +left unfixed. + +--- + +## Appendix: Evidence Files + +Paths are relative to the run working directory `/tmp/zeroize-audit-c212f1cdd302/`. + +| Finding | Evidence File | Description | +|---|---|---| +| ZA-0001 | `source-analysis/source-findings.json` | Source findings F-RUST-SRC-0001/0002 (re-scoped → ZA-0001) | +| ZA-0001 | `source-analysis/rust-semantic-findings.json` | Manual trait-aware semantic findings (format v57 fallback) | +| ZA-0001 | `source-analysis/sensitive-objects.json` | SO-5001..SO-5006 inventory (subject = SO-5005) | +| ZA-0001 | `rust-compiler-analysis/mir-findings.json` | MIR finding F-RUST-MIR-0003 (drop glue delegates only to cipher) | +| ZA-0001 | `rust-compiler-analysis/ir-findings.json` | IR findings F-RUST-IR-0001 (refuted), -0002, -0003 | +| ZA-0001 | `rust-compiler-analysis/asm-findings.json` | ASM corroboration F-RUST-ASM-0001 (AArch64, experimental) | +| ZA-0001 | `rust-compiler-analysis/superseded-findings.json` | Supersession records for F-RUST-SRC-0001/0002 | +| ZA-0001 | `rust-compiler-analysis/6f04dcee.mir` | MIR dump | +| ZA-0001 | `rust-compiler-analysis/6f04dcee.O0.ll` | LLVM IR at O0 (drop_in_place lines 503-514, 6643-6651) | +| ZA-0001 | `rust-compiler-analysis/6f04dcee.O1.ll` | LLVM IR at O1 | +| ZA-0001 | `rust-compiler-analysis/6f04dcee.O2.ll` | LLVM IR at O2 (data memset:1823; key-schedule volatile wipe + fence:1854-1856) | +| ZA-0001 | `rust-compiler-analysis/6f04dcee.O2.s` | AArch64 assembly at O2 (volatile zero-store loop:1481-1491) | +| ZA-0001 | `poc/ZA-0001_missing_source_zeroize.rs` | PoC test (vulnerable + control-fix) | +| ZA-0001 | `poc/ZA-0001_missing_source_zeroize_bin.rs` | PoC binary (exit 0 = exploitable) | +| ZA-0001 | `poc/za_0001_replica.rs` | Version-pinned byte-for-byte Aes128Prng replica | +| ZA-0001 | `poc/poc_validation_results.json` | Compile + run results | +| ZA-0001 | `poc/poc_verification.json` | Semantic verification (6/6 checks passed) | +| ZA-0001 | `poc/poc_final_results.json` | Merged validation + verification (input to this report) | +| run | `report/raw-findings.json` | All 9 findings pre-gating, original namespaced IDs | +| run | `report/id-mapping.json` | Namespaced → ZA mapping + refuted/superseded/corroborating buckets | +| run | `report/findings.json` | Final gated structured output (matches schemas/output.json) | +| run | `report/notes.md` | Assembly process notes | +| run | `preflight.json` | Run configuration and preflight checks | diff --git a/docs/reviews/2026-06-16-zeroize-audit-pr82.md b/docs/reviews/2026-06-16-zeroize-audit-pr82.md new file mode 100644 index 00000000..de9d341d --- /dev/null +++ b/docs/reviews/2026-06-16-zeroize-audit-pr82.md @@ -0,0 +1,61 @@ +# Zeroize Audit — ore-rs PR #82 (Bit6 scheme + v2 wire format) + +**Date:** 2026-06-16 +**Skill:** Trail of Bits `zeroize-audit` (preflight → Rust source → MIR/LLVM-IR/asm) +**Branch:** `feat/ore-v2-bit6` (#82), aarch64 host, `--cfg aes_armv8` (hardware AES) +**Verdict:** **Clean — no new zeroization gaps in #82's crypto code.** + +## Scope + +#82 adds the crypto core: `LemireFyPrp` (`primitives/prp.rs`), the `OreAes128Bit6` +cipher (`scheme/bit2_w6.rs`), the σ-MMO hash `FixedPiZ2Hash` (`primitives/hash.rs`), +and the v2 wire header. The audit asked whether the new key-derived material is +wiped on drop, and whether those wipes survive compiler optimization. + +## Source analysis (Phase 1) + +All new secret-bearing code zeroizes correctly: + +| Type | Status | +|---|---| +| `LemireFyPrp` (`prp.rs`) | OK — `Zeroize` + manual `Drop` + `ZeroizeOnDrop` marker (matches `KnuthShufflePRP`). The AES-CTR keystream built in `new()` is wiped after the permutation is constructed (`stream.zeroize()` + per-block `as_mut_slice().zeroize()`); the AES key schedule is covered by the `aes` crate's own `ZeroizeOnDrop`. | +| `OreAes128Bit6` (`bit2_w6.rs`) | OK — `#[derive(ZeroizeOnDrop)]` over `prf1`/`prf2`; `rng` correctly `#[zeroize(skip)]`. `SeedBuf` wipes each seed `AesBlock` on `Drop`. | +| Bit6 encrypt scratch | OK — per-block `template`/`work` RO-key buffers zeroized after the encrypt loop. | +| `FixedPiZ2Hash` (`hash.rs`) | Correctly **not** flagged — `PI_KEY` is a fixed, public nothing-up-my-sleeve AES key; the `Hash::new` parameter is a per-ciphertext nonce. Neither is a secret. | + +Dangerous-API scan (`mem::forget` / `ManuallyDrop` / `Box::leak` / `transmute` / +`ptr::write_bytes` / `mem::take` / secret-across-`.await`): **0 hits**, grep-confirmed. + +## Compiler analysis (Phase 2) — wipes survive `-O2` + +Crate-wide `store volatile` count rises **2 (O0) → 6 (O1) → 384 (O2)** and +`compiler_fence` **8 → 296**: the wipes *multiply* under inlining, they do not +disappear. **Zero `OPTIMIZED_AWAY_ZEROIZE`** (the 3 "volatile dropped" IR diffs +are un-inlined zeroize wrappers that fully inline at O2; the 204 other `memset` +hits are non-volatile init/alloc, no O0→O2 diff). + +Confirmed surviving at O2: +1. `LemireFyPrp::new` — 41 volatile stores + 26 `seq_cst` fences (keystream byte + wipes + the AES-block zeroize); Drop-triggered `permutation`/`inverse` wipe via + `drop_in_place` (22 volatile stores). +2. `OreAes128Bit6` — `template`/`work` + `SeedBuf` drop wipes inlined into callers. +3. `aes::Aes128` key schedule under `--cfg aes_armv8` — `drop_in_place::<…armv8…>` + retains volatile-store + fence at O2 (matches the #79 run). + +Assembly (aarch64, **experimental** backend): 0 `STACK_RETENTION` / `REGISTER_SPILL`. +MIR: 5 `needs_review` name-matches, all on the already-verified legacy bit2 path. + +## Findings + +| ID | Severity | Status | +|---|---|---| +| **ZA-0001** — legacy `Aes128Prng` lacks `Drop`/`ZeroizeOnDrop` | medium | Pre-existing, **not** a #82 issue. Fixed on **PR #84** (off `main`); the stack inherits it on rebase. | +| `SECRET_COPY` — `#[derive(Debug)]` on `OreAes128Bit6` | low | Benign (matches the bit2 sibling; `Aes128` Debug is opaque). **Addressed in #82** by replacing the derive with an explicit opaque `Debug` impl. | + +**No PoC phase:** the only exploitable finding (ZA-0001) is already PoC-verified +and fixed on #84. + +## Related + +- Constant-time analysis (v2 applicability): `docs/reviews/2026-06-16-constant-time-analysis-pr79.md` +- ZA-0001 zeroize audit + fix: `docs/reviews/2026-06-16-zeroize-audit-pr79.md`, PR #84 diff --git a/docs/reviews/2026-06-16-zeroize-audit-pr83.md b/docs/reviews/2026-06-16-zeroize-audit-pr83.md new file mode 100644 index 00000000..9af38e83 --- /dev/null +++ b/docs/reviews/2026-06-16-zeroize-audit-pr83.md @@ -0,0 +1,47 @@ +# Zeroize Audit — ore-rs PR #83 (chained-prefix variable-length scheme) + +**Date:** 2026-06-16 +**Skill:** Trail of Bits `zeroize-audit` (preflight → Rust source analysis) +**Branch:** `feat/ore-v2-chained` (#83), aarch64 host, `--cfg aes_armv8` +**Verdict:** two new gaps found and **fixed** in `f498990`; no remaining new gaps. + +## Scope + +#83 adds the CMAC accumulator (`primitives/cmac.rs`) and the chained +variable-length scheme (`scheme/chained.rs`), plus `LemireFyPrp::from_stream`. +The audit checked whether the new key-derived material — the accumulator subkeys +and state, the per-block RO-key tags, the PRP keystream, and the cipher key — is +wiped, on the struct Drop and in the per-block local buffers. + +## Source analysis + +| Item | Status | +|---|---| +| `CmacAccumulator` Drop (`k1`, `state`) | ✅ wiped; `cipher` via aes `ZeroizeOnDrop` | +| chained `k_acc` Drop | ✅ wiped | +| chained `stream` (PRP keystream, `prp_at`) | ✅ `stream.zeroize()` | +| `from_stream` consumed stream | ✅ caller-owned; both callers wipe | +| **`L = E_k(0)` in `CmacAccumulator::new`** | ❌→✅ **was a gap** — `K1 = dbl(L)`'s source, left on the stack. **Fixed:** `l.zeroize()` after deriving `k1`. | +| **chained `ro` RO-tag buffer (`encrypt_var`)** | ❌→✅ **was a gap** — 64 key-derived CMAC tags, never wiped (the sibling `bit2_w6` wipes its `template`/`work`). **Fixed:** per-block `ro` wipe after the right block is built. | + +Dangerous-API scan: 0 hits (`mem::forget`/`ManuallyDrop`/`Box::leak`/`transmute`/ +`write_bytes`/`mem::take`/secret-across-`.await`), grep-confirmed. No `Copy` on +secret types; `Clone`/`Debug` only on published-ciphertext containers. + +`FixedPiZ2Hash` `PI_KEY` correctly not flagged (fixed public key). **ZA-0001** +(legacy `Aes128Prng`) re-surfaced — pre-existing, fixed on **PR #84**, inherited +on rebase. + +## Compiler/PoC phases + +Skipped: the two findings were *missing*-wipe (source-conclusive, nothing for an +IR diff to do), and the existing wipes use the same `zeroize`-crate volatile+fence +API already IR-verified surviving `-O2` on #79/#82. After the fix, the new `L`/`ro` +wipes use that same API. + +## Related + +- Code review + constant-time on #83 (left-vs-right comparator added; Branch + enum; single-key init): see PR #83 discussion. +- ZA-0001 fix: `docs/reviews/2026-06-16-zeroize-audit-pr79.md`, PR #84. +- #82 zeroize audit: `docs/reviews/2026-06-16-zeroize-audit-pr82.md`. From 7b6317fb076cc09c805db7bf3b41595441d710df Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Tue, 16 Jun 2026 19:22:58 +1000 Subject: [PATCH 2/3] ci: run tests on all pull requests, not only those targeting main Drops the `branches: [main]` filter on the pull_request trigger so stacked PRs that target a feature/integration branch (the ore-v2 stack) run CI too. Lands on ore-v2 first so the whole rebased stack inherits it. --- .github/workflows/test.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d36e6063..65d4617c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -3,9 +3,10 @@ on: push: branches: - main + # Run on every pull request regardless of base branch, so stacked PRs that + # target a feature/integration branch (e.g. the ore-v2 stack) get CI too — + # not only PRs into `main`. pull_request: - branches: - - main workflow_dispatch: defaults: From 85323ea319a524a86207a827ed0483401be18ac0 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Thu, 8 Oct 2026 20:48:46 +1100 Subject: [PATCH 3/3] docs(reviews): address the #90 review of the audits and MMO validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The MMO validation claimed a pl/pgSQL comparator was validated; only the σ-MMO hash was implemented and checked (12 vectors). The doc now scopes its result to the hash and lists what a comparator still needs end-to-end vectors for. - "LSB" in both the SQL and Rust is bit 0 of byte 0 of the block (bit 120 of the big-endian field element), as in the frozen legacy hash. The SQL and summary now name that coordinate. - The #82 and #83 zeroize audits marked items clean that left key-derived temporaries unwiped (from_stream's per-draw copy; the CMAC mixed values). Both audits are amended, pointing at the fixes on #82 and #83, and say plainly that the wipes are source-level only. --- .../2026-06-16-mmo-plpgsql-validation.md | 35 +++++++++++++++---- .../2026-06-16-mmo-plpgsql-validation.sql | 3 ++ docs/reviews/2026-06-16-zeroize-audit-pr82.md | 10 +++++- docs/reviews/2026-06-16-zeroize-audit-pr83.md | 12 ++++++- 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/docs/reviews/2026-06-16-mmo-plpgsql-validation.md b/docs/reviews/2026-06-16-mmo-plpgsql-validation.md index 34f9bd85..95e314fc 100644 --- a/docs/reviews/2026-06-16-mmo-plpgsql-validation.md +++ b/docs/reviews/2026-06-16-mmo-plpgsql-validation.md @@ -6,7 +6,10 @@ in pl/pgSQL / SQL using only the `pgcrypto` extension, as an interim before the Rust TLE lands? (The legacy bit2 comparator already lives in `cipherstash/encrypt-query-language`; domain types separate the operators, so this work targets the **new v2/MMO scheme**.) -**Answer: yes — validated empirically (12/12 vectors match Rust).** +**Answer: the σ-MMO 1-bit hash, the one primitive the comparator needs beyond +plain SQL, is implementable and validated empirically (12/12 vectors match +Rust). The full comparator is not implemented or tested here** (see Scope of +the result). ## Key finding: the comparison needs no CMAC / PRP / key derivation @@ -27,7 +30,7 @@ secrets during encryption. |---|---| | π (fixed public-key AES-128) | `encrypt(m, 'ORE-rs.v2.H-pi.1'::bytea, 'aes-ecb/pad:none')` (pgcrypto) — `pad:none` for a raw single block; the `PI_KEY` is public, so it's embedded | | σ = GF(2¹²⁸) doubling | ~10 lines of pl/pgSQL: byte-wise shift-left-1 + conditional `# 0x87` (`gf128_double` below) — the only non-pgcrypto-native op | -| `m = σ(f) ⊕ nonce`, feedforward, LSB | byte XOR (`get_byte`/`set_byte`/`#`) + `& 1` | +| `m = σ(f) ⊕ nonce`, feedforward, output bit | byte XOR (`get_byte`/`set_byte`/`#`) + `get_byte(·, 0) & 1` | | prefix scan, bit read | plain SQL (`substring`, `get_bit`) | ## Empirical result @@ -44,10 +47,28 @@ the Rust `FixedPiZ2Hash::hash` on 12 `(f, nonce)` vectors (H mix Byte-for-byte agreement. (Postgres 15.12 + pgcrypto.) +**The output bit is a fixed coordinate, not the integer LSB.** The construction +is written `LSB(π(m) ⊕ m)`, but both Rust (`block[0] & 1`) and this SQL take +bit 0 of byte 0 of the 16-byte block, which for the big-endian field element is +bit 120. They agree with each other and with the frozen legacy bit2 hash, which +uses the same coordinate; any fixed output bit is equivalent for security. The +spec now names the coordinate rather than calling it the LSB. + +## Scope of the result + +Only `mmo_hash_bit` was implemented and checked. The rest of a comparator — +header and length handling, the prefix scan over `xt` and the `f` tags, the +shorter-sorts-first rule for prefixes, and the right-block bit read at +`xt[l]` — is plain SQL in principle (`substring`, `get_bit`, byte equality), +but none of it is implemented or executed here. A comparator built from these +pieces needs end-to-end comparison vectors (pairs of ciphertexts with their +expected order, including equal values, shared prefixes and different +lengths) before it can be called correct. + ## Caveat — constant-time -The pl/pgSQL comparator computes the correct order, but pl/pgSQL is **not a -constant-time environment**: the cache-line/sub-line obliviousness the Rust +A pl/pgSQL comparator built on this would not be constant-time: pl/pgSQL is +**not a constant-time environment**, and the cache-line/sub-line obliviousness the Rust comparator gets from `ct_select_byte`/`ct_bit` and the no-early-exit prefix scan is not realistically achievable in SQL. The interim SQL comparator therefore trades that hardening away; the Rust TLE restores it. Whether that's acceptable @@ -56,6 +77,8 @@ ciphertexts) — flagged, not decided here. ## Conclusion -The requirement is met: the v2 σ-MMO comparison is implementable in pl/pgSQL with -pgcrypto. The encryption-side work (single-key derivation, CMAC accumulator, PRP) +The blocking question is answered: the only cryptographic primitive the v2 +comparator needs, the σ-MMO 1-bit hash, is implementable in pl/pgSQL with +pgcrypto and matches Rust. The comparator itself remains to be written and +validated end to end. The encryption-side work (single-key derivation, CMAC accumulator, PRP) is unaffected by it. Safe to resume the single-key change + Bit6 vector regen. diff --git a/docs/reviews/2026-06-16-mmo-plpgsql-validation.sql b/docs/reviews/2026-06-16-mmo-plpgsql-validation.sql index a393b955..a1b19ddd 100644 --- a/docs/reviews/2026-06-16-mmo-plpgsql-validation.sql +++ b/docs/reviews/2026-06-16-mmo-plpgsql-validation.sql @@ -23,6 +23,9 @@ BEGIN END; $$ LANGUAGE plpgsql IMMUTABLE; -- BHKR σ-MMO 1-bit hash: m = σ(f) ⊕ nonce; H = lsb(π(m)) ⊕ lsb(m). +-- "lsb" is a fixed output coordinate, not the integer LSB: bit 0 of byte 0 +-- of the 16-byte block, i.e. bit 120 of the big-endian field element. This +-- matches the Rust `block[0] & 1` (and the frozen legacy bit2 hash). -- π = AES-128-ECB under the fixed public key "ORE-rs.v2.H-pi.1". CREATE OR REPLACE FUNCTION mmo_hash_bit(f bytea, nonce bytea) RETURNS int AS $$ DECLARE m bytea; pim bytea; diff --git a/docs/reviews/2026-06-16-zeroize-audit-pr82.md b/docs/reviews/2026-06-16-zeroize-audit-pr82.md index de9d341d..fd5f9373 100644 --- a/docs/reviews/2026-06-16-zeroize-audit-pr82.md +++ b/docs/reviews/2026-06-16-zeroize-audit-pr82.md @@ -5,6 +5,14 @@ **Branch:** `feat/ore-v2-bit6` (#82), aarch64 host, `--cfg aes_armv8` (hardware AES) **Verdict:** **Clean — no new zeroization gaps in #82's crypto code.** +> **Amended 2026-10-08** (review of #90): the verdict missed one gap. +> `LemireFyPrp::from_stream` copies each 8-byte keystream draw into a local +> `draw` array and decodes it into `x`, and wiped neither, so the `stream` +> and `blocks` wipes did not clear every source-level copy of the keystream. +> Fixed on #82 in "harden(prp): wipe the per-draw keystream copy in +> from_stream". Like the other wipes, it removes the source-level copy; it +> cannot rule out copies the compiler keeps in registers or spills. + ## Scope #82 adds the crypto core: `LemireFyPrp` (`primitives/prp.rs`), the `OreAes128Bit6` @@ -18,7 +26,7 @@ All new secret-bearing code zeroizes correctly: | Type | Status | |---|---| -| `LemireFyPrp` (`prp.rs`) | OK — `Zeroize` + manual `Drop` + `ZeroizeOnDrop` marker (matches `KnuthShufflePRP`). The AES-CTR keystream built in `new()` is wiped after the permutation is constructed (`stream.zeroize()` + per-block `as_mut_slice().zeroize()`); the AES key schedule is covered by the `aes` crate's own `ZeroizeOnDrop`. | +| `LemireFyPrp` (`prp.rs`) | OK — `Zeroize` + manual `Drop` + `ZeroizeOnDrop` marker (matches `KnuthShufflePRP`). The AES-CTR keystream built in `new()` is wiped after the permutation is constructed (`stream.zeroize()` + per-block `as_mut_slice().zeroize()`); the AES key schedule is covered by the `aes` crate's own `ZeroizeOnDrop`. **Amended:** the per-draw `draw`/`x` copies in `from_stream` were not wiped; fixed (see the note above). | | `OreAes128Bit6` (`bit2_w6.rs`) | OK — `#[derive(ZeroizeOnDrop)]` over `prf1`/`prf2`; `rng` correctly `#[zeroize(skip)]`. `SeedBuf` wipes each seed `AesBlock` on `Drop`. | | Bit6 encrypt scratch | OK — per-block `template`/`work` RO-key buffers zeroized after the encrypt loop. | | `FixedPiZ2Hash` (`hash.rs`) | Correctly **not** flagged — `PI_KEY` is a fixed, public nothing-up-my-sleeve AES key; the `Hash::new` parameter is a per-ciphertext nonce. Neither is a secret. | diff --git a/docs/reviews/2026-06-16-zeroize-audit-pr83.md b/docs/reviews/2026-06-16-zeroize-audit-pr83.md index 9af38e83..c57d5a9c 100644 --- a/docs/reviews/2026-06-16-zeroize-audit-pr83.md +++ b/docs/reviews/2026-06-16-zeroize-audit-pr83.md @@ -5,6 +5,15 @@ **Branch:** `feat/ore-v2-chained` (#83), aarch64 host, `--cfg aes_armv8` **Verdict:** two new gaps found and **fixed** in `f498990`; no remaining new gaps. +> **Amended 2026-10-08** (review of #90): the verdict missed two more gaps. +> `CmacAccumulator::absorb` and `finalize` each build an owned `mixed` value +> from the chain state (and, in `finalize`, `K1`) and returned without +> wiping it; and `from_stream`'s per-draw keystream copy (see the #82 audit) +> applies here too. Both are fixed: on #83 in "harden(cmac): wipe the mixed +> state temporaries in absorb and finalize", and on #82. The wipes remove the +> source-level copies; they cannot rule out copies the compiler keeps in +> registers or spills, and no IR evidence was gathered for them. + ## Scope #83 adds the CMAC accumulator (`primitives/cmac.rs`) and the chained @@ -18,9 +27,10 @@ wiped, on the struct Drop and in the per-block local buffers. | Item | Status | |---|---| | `CmacAccumulator` Drop (`k1`, `state`) | ✅ wiped; `cipher` via aes `ZeroizeOnDrop` | +| **`CmacAccumulator::absorb` / `finalize` `mixed` temporaries** | ❌→✅ **was a gap** (missed by this audit) — state/`K1`-derived values left on the stack. **Fixed** on #83 (see the note above). | | chained `k_acc` Drop | ✅ wiped | | chained `stream` (PRP keystream, `prp_at`) | ✅ `stream.zeroize()` | -| `from_stream` consumed stream | ✅ caller-owned; both callers wipe | +| `from_stream` consumed stream | ✅ caller-owned; both callers wipe. **Amended:** the per-draw `draw`/`x` copies inside `from_stream` were not wiped; fixed on #82. | | **`L = E_k(0)` in `CmacAccumulator::new`** | ❌→✅ **was a gap** — `K1 = dbl(L)`'s source, left on the stack. **Fixed:** `l.zeroize()` after deriving `k1`. | | **chained `ro` RO-tag buffer (`encrypt_var`)** | ❌→✅ **was a gap** — 64 key-derived CMAC tags, never wiped (the sibling `bit2_w6` wipes its `template`/`work`). **Fixed:** per-block `ro` wipe after the right block is built. |