Repository navigation
ORE v2 (2/n): core refactor — width abstraction, seed/tag separation, template RO keys - #79
Conversation
ab17b5a to
7170514
Compare
7170514 to
220c498
Compare
220c498 to
7a2d438
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Cryptographic state handling and wire-compatibility guarantees warrant final human review.
0 open findings
What changed in this PR
Refactors ORE internals to support future block widths while preserving the legacy wire format.
Changes:
- Introduces block-width abstractions and 6-bit decomposition.
- Separates sensitive PRP seeds from serializable ciphertext state.
- Reworks random-oracle buffers and removes
lazy_static.
| File | Description |
|---|---|
packages/ore-rs/src/scheme/width.rs |
Defines width-dependent types and operations. |
packages/ore-rs/src/scheme/decompose.rs |
Adds tested 6-bit decomposition. |
packages/ore-rs/src/scheme/bit2.rs |
Refactors seed and random-oracle buffer handling. |
packages/ore-rs/src/scheme.rs |
Registers internal modules. |
packages/ore-rs/Cargo.toml |
Removes lazy_static. |
Cargo.lock |
Updates resolved dependencies. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
08a308e to
6cf2d39
Compare
6cf2d39 to
bf65c60
Compare
freshtonic
left a comment
There was a problem hiding this comment.
Security review of this layer. One medium finding. The legacy PRP seeds collide over runs of zero bytes, so one Left ciphertext shows the magnitude of an integer. The new TODO gives the wrong condition. The legacy wire format is frozen, so the fix is documentation and guidance. See the inline comment.
I checked these items and found no problem: behaviour is equal to legacy main (the compat vectors pass on this head and on the merge base), the RO-key template, PRF₁/PRF₂ separation, width.rs and decompose.rs.
🤖 Generated with Claude Code
bf65c60 to
fd45b44
Compare
Introduces the ORE v2 core abstractions (plan §1) with byte-identical output, verified by the PR 1 compatibility vectors: - BlockWidth trait (Bit8 today; Bit6 arrives with its scheme) carrying the domain-sized types stable Rust can't express const-generically. - 6-bit MSB-first block decomposition with order-preservation property tests, ahead of the scheme that consumes it. - PRP seeds (PRF2 outputs, key-equivalent material) now live in a dedicated zeroize-on-drop SeedBuf, never in the serialisable Left; tag inputs are built directly into Left.f. - RO keys maintained as an incrementally-updated template plus a work buffer for the in-place PRF/hash passes (3 byte-writes per entry per block instead of a full rebuild), both zeroized after use. - Indicator encoding extracted behind encode_right_block (naive form retained; bulk form lands in the next PR). Drops the now-unused lazy_static dependency. Part of the ORE v2 program (docs/plans/2026-06-12-ore-v2-architecture.md, PR 2).
BITS was declared but never read, which stable clippy reports as dead code (CI's 1.78 does not). DOMAIN is documented as 1 << BITS, but each impl spelled both out, so they could disagree. DOMAIN is now a default of 1 << Self::BITS and impls declare only BITS.
The TODO on derive_prp_seeds gave the wrong condition. Seeds for blocks n < m collide when x[n..m] is all zero, not over runs of identical bytes, and the leak shows in a single ciphertext: zero runs repeat π(0) in xt, and leading zero bytes give an integer's magnitude as the same value under every key. The wire format is frozen, so the code keeps it. Replace the TODO with the correct condition and a link to the PR whose schemes bind the block index, document the leak in the crate README and the v2 plan, and stop the README claiming that the block number avoids repeated prefixes (it is bound into the left tag only).
fd45b44 to
ad9e19d
Compare
freshtonic
left a comment
There was a problem hiding this comment.
Approved after the round-2 security re-review. The round-1 findings for this PR are fixed or have no remaining blocking items.
🤖 Generated with Claude Code
Stacked on #78. Plan §1 (
docs/plans/2026-06-12-ore-v2-architecture.md).What
BlockWidthtrait (scheme/width.rs): sealed trait carryingBITS/DOMAINand the domain-sized associated types (RightBlock,Prp,RoKeyBuf) that stable Rust can't express const-generically.Bit8is the only width today;Bit6arrives with its scheme (PR 5).scheme/decompose.rs): MSB-first bit-packing with quickcheck properties for order preservation, domain bounds, and injectivity — landed ahead of the scheme that consumes it so the packing is independently pinned.SeedBufand never touch the serialisableLeft. Previously the only thing separating seeds from published tags was statement ordering insideencrypt.lazy_staticblock each iteration; a work buffer takes the in-place PRF/hash passes. Both zeroized after use.lazy_staticdependency dropped.encode_right_block: the naive per-bit indicator loop extracted behind a width-generic function — deliberately not optimized here (risk staging); the permutation-direct bulk form is PR 3.Byte identity
All 12 compatibility vector tests from #78 pass unchanged — the AES input bytes are identical to before; only buffer management moved.
Note for review
OreCipher's public shape is unchanged in this PR; the associated-type change flagged in the plan lands with the Bit6 scheme instead, keeping this diff purely internal.