Skip to content

fix(btree): bind node kind to authenticated envelope and bound-check node bodies - #11

Merged
farhan-syah merged 5 commits into
NodeDB-Lab:mainfrom
presempathy-awb:codex/upstream/authenticated-node-read-proof
Jul 26, 2026
Merged

fix(btree): bind node kind to authenticated envelope and bound-check node bodies#11
farhan-syah merged 5 commits into
NodeDB-Lab:mainfrom
presempathy-awb:codex/upstream/authenticated-node-read-proof

Conversation

@presempathy-awb

@presempathy-awb presempathy-awb commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Summary

A B+ tree node declares its kind twice: once in the AEAD-authenticated page
envelope, once in the encrypted body header. Reads decoded the body's
declaration and ignored the authenticated one, so a page authenticated as
BTreeInternal could be interpreted as a leaf, or the reverse.
BTree::read_node_guard now requires the two to agree and reports
Corruption(HeaderUnverifiable) when they don't.

Reviewing that led to a second, worse problem in the same threat model: the
node header's prefix_len, slot_count, and slot-directory entries are used
directly as slice indices with no validation, so a malformed-but-authenticated
page panics the library instead of reporting corruption. This PR fixes that
class too — see Bounds validation below.

The original diagnosis, the envelope/body binding, and its two-direction
regression are @presempathy-awb's. The bounds validation, the benchmark
correction, and the merge with current main came out of review.

Threat model

Worth stating precisely, because it scopes both fixes.

page_kind is part of the AAD (src/crypto/aad.rs), bound to the same AEAD tag
as the body. An attacker without the key cannot produce either a kind
mismatch or a malformed body — the tag check rejects it first. Both regressions
in this PR have to write their poisoned pages through the pager's own
write_main_page to construct the condition.

What remains, and why both fixes are still worth having:

  • a pagedb write-path bug emitting a self-inconsistent page;
  • a key-holding writer producing one — which is reachable in practice via
    Follower / apply_incremental / restore_from, where pages authored
    elsewhere by a holder of the same key are consumed.

So this is integrity hardening at a trust boundary, not a remote-attacker fix.

Envelope/body binding

read_node_guard receives the guard and the authenticated envelope kind from
the existing one-pass pager API, decodes the body header once, and rejects
disagreement. No extra read, decrypt, or cache lookup — the authenticated kind
already arrives with the page.

The non-node PageKind arm is unreachable today (KindBinding::Node already
restricts to the two node kinds on both the warm and cold pager paths). It is
kept, and now commented as such, so the boundary stays total if the pager ever
admits another kind.

Bounds validation

validate_node_body (src/btree/node.rs) structurally validates a body once:
the header fits, prefix and slot directory fit, and every slot's record extent
lies inside the body — for both the leaf and internal record layouts.

It runs in all four constructors that turn raw bytes into a node —
Leaf::decode, Internal::decode, LeafAccessor::new, InternalAccessor::new
— so the unchecked indexing downstream is sound by construction rather than by
inspection. The zero-copy accessors matter as much as the decoders here: they
are what the hot read path actually uses, and they had the same unchecked
indexing. Covering all four also catches the paths in maintenance.rs and
deep_walk.rs that decode bodies without going through read_node_guard.

Validation is deliberately extent-only. It proves each record lies inside
the body, not that the records are semantically sensible. A page with
overlapping or nonsensical offsets still decodes to garbage — authenticated
garbage is the writer's problem — but it cannot read out of bounds. Tightening
further (ordered, non-overlapping, past-the-directory offsets) would start
rejecting layouts a future encoder might legitimately produce.

OVERFLOW_SENTINEL moved from leaf.rs to node.rs, since the validator needs
the record format and that is where the layout is defined.

Tests

Each was confirmed to fail without its fix, by temporarily stubbing the check
out — not merely observed to pass:

Regression Unfixed behavior
envelope/body mismatch, both directions returns Some([118]) — the value read out of a mis-typed page
prefix_len past body panicked: range end index 60024 out of range for slice of length 4056
slot directory past body panic
slot offset past body panic
record overruns body panic
internal record overruns body panic

Coverage added: four unit tests on the validator in src/btree/node.rs, and
five malformed-page cases driven end-to-end through BTree::get in
tests/btree_basic.rs, alongside the existing two-direction mismatch test.

The pager test was renamed read_main_node_discovers_kind_in_a_single_read.
It passes with the envelope/body check removed, so its earlier name
overstated what it guards. It locks the single-read shape of the pager API —
worth keeping, because the agreement check is only free while the authenticated
kind arrives with the page — and now says so.

Benchmark

benches/authenticated_node_read.rs measures a cold authenticated descent
through a multi-level tree.

The first draft called evict_main_pages inside b.iter, charging cache
bookkeeping, the Db lock, and key construction to the read path. That is the
source of the wide spread in the original numbers (per-round deltas from
-8.6% to +12.1%, and -11% to +25% on the earlier diagnostic run) — the
harness, not the host. Eviction and key setup now happen in an untimed
iter_with_setup phase:

cold_tree_get   mean 862 ns   median 783 ns   p95 1.2 us
                1679 alloc bytes (73 allocs)

Not comparable to the ~604 ns in the original description: that figure and
this one measure different things, and the old one is no longer produced by any
code in this branch.

The bench also now uses the shared benches/common harness that landed with
#10, rather than re-rolling the runtime thread-local and tracking allocator.
Runs on MemVfs, so the figure is CPU + AEAD for an authenticated cold descent,
not the cost of reaching real storage.

Verification

On the merged branch: cargo fmt --all --check clean, cargo clippy --all-targets --all-features -- -D warnings clean, cargo nextest run --all-features 416 passed / 4 skipped, cargo bench --bench authenticated_node_read runs to completion.

Compatibility

Valid pages follow the same read, decrypt, decode, and accessor paths as
before. Newly rejected: a page whose two kind declarations disagree, and a page
whose header or slot directory describes records outside the body. Both were
previously accepted — the first silently, the second as a panic. No format,
public API, feature flag, dependency, or VFS contract movement.

presempathy-awb and others added 5 commits July 26, 2026 06:08
# Conflicts:
#	Cargo.toml
#	README.md
An authenticated page's bytes are only guaranteed to be what a key
holder wrote, not what a correct writer would write. Leaf and internal
decoders used prefix_len, slot_count, and slot-directory offsets
directly as slice indices, so a malformed-but-authenticated body could
panic the library instead of surfacing as corruption.

Add validate_node_body to structurally check the header, slot
directory, and every leaf/internal record fit within the body before
any accessor indexes into it, and route all node parse paths through
it.
Switch the authenticated cold node-read benchmark from a per-call
Arc<AsyncMutex<Db>> plus a locally built runtime to the shared
block_on/with_rt helpers and an Rc<Db>, since the workload is
single-threaded and read-only. Also move cache eviction and key
construction into iter_with_setup so only descent plus authentication
is timed.
@farhan-syah farhan-syah changed the title fix(btree): bind decoded node kind to authenticated envelope fix(btree): bind node kind to authenticated envelope and bound-check node bodies Jul 26, 2026
@farhan-syah
farhan-syah merged commit 7c12d64 into NodeDB-Lab:main Jul 26, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants