Skip to content

feat(TC-887): seal KV blocks in the FileSystem store; prepare writes before the invoke lock - #333

Draft
samgbafa wants to merge 5 commits into
skgbafa/tc-887-s3-kv-sealedfrom
skgbafa/tc-887-s3b-fs-sealed
Draft

samgbafa wants to merge 5 commits into
skgbafa/tc-887-s3-kv-sealedfrom
skgbafa/tc-887-s3b-fs-sealed

Conversation

@samgbafa

@samgbafa samgbafa commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Part of TC-887 (encryption at rest), slice S3b: the FileSystem block store seals KV blocks. With at-rest mode off (the default), the store behaves as it does today.

Stack: base skgbafa/tc-887-s3-kv-sealed (#329) → #327/#326/#325 (S1 round 3). The diff below is S3b only. CI runs by workflow_dispatch on the PR branch (see comments).

Touches core's invoke path (tinycloud-core/src/db.rs, storage/{mod,either}.rs): a new hook runs before the writer lock. Please review that part with the invariant below in mind.

What changed

  • ImmutableWriteStore::prepare_write(space) (core): a new trait method, default no-op, delegated by Box and Either. The invocation path calls it for each space it writes, before the SQLite writer lock and the database transaction. persist still runs inside both. A prepare_write error fails the request before the transaction.
    • Invariant: no key-store I/O inside invoke's writer lock or transaction. Creating a space's data key inside them deadlocked SQLite nodes on the first sealed put: the key store takes the same writer mutex, and SQLite allows one writer.
  • FileSystem store (storage/file_system.rs):
    • prepare_write creates or loads the space's current data key (503 if keys are locked or encryption is off in a non-off mode).
    • Writes follow write_target. Sealed blocks go to v1/{space_tag}/{object_tag}, from either staging type: memory → seal_vec → temp file → atomic rename; temp file → seal_file → atomic rename. Only the sealed name dedups.
    • Reads and ranges follow read_plan: sealed, then legacy (refused in strict; a legacy miss while keys are locked → 503). The Readable type becomes Either<plaintext reader, DecryptReader>.
    • Sizes are plaintext bytes, joined by tag. remove takes the sealed copy first. contains follows the read plan.
  • Wiring (lib.rs, storage/mod.rs): storage::attach_at_rest links the opened store to AtRestRuntime through sealed::AtRestHandle, a per-operation AtRestSource, and calls S1's declare_seal_path() for the FileSystem store. The S3 block store doesn't seal yet and declares no seal path, so at_rest::setup refuses a non-off mode with S3 storage rather than write plaintext; S3c adds S3. S1's ceremony test now expects encrypt/strict to boot and encrypt on an FS node with a pinned, verified escrow, and the node at_rest.rs docs and docs/at-rest-escrow.md drop the "no storage seals" wording (agreed with S1).
  • did_resolution_single_entry: the FS store's existing range.resolve(total_size) moved into range_legacy, so its allowlist entry is renamed to match.
  • tinycloud-node-server dev-dependency parking_lot (already in the lock), for a switchable test state.

How it's tested

  • Core invariant test db::test::prepare_write_runs_before_the_writer_lock_and_persist_inside_it, on a SQLite file DB. A probe store records whether the writer lock was free in each hook: it requires prepare_write with the lock free, then persist with it held. With persist failing, the transaction rolls back after a successful prepare and leaves no KV row. Whatever prepare created stays unused; for real keys that's a DEK row the next put reuses. Without the hook the probe sees only persist, and the test fails.
  • Real app tests/at_rest_fs_node.rs: an FS node on a SQLite file DB with at_rest = {mode: "encrypt", escrow_none: true}, driven through /delegate and /invoke with SDK sessions.
    • A short value and a 200 000-byte value: put → get returns the value. On disk there are exactly 2 files, both v1/… and TCE1, with no marker, space name or address in them.
    • After a restart, both read back.
    • A 120 s timeout makes it the deadlock regression: before the hook, the first put hung until a 3000 s run limit stopped it.
    • Control: an off node stores the plaintext under the legacy name.
  • FS store tests (storage/file_system/sealed_tests.rs, 10):
    • both staging paths at 0, 10, 64Ki−1, 64Ki+5 and 3·64Ki;
    • ranges between all pairs of segment edges, plus the 416 plaintext total;
    • off with keys still writes legacy;
    • legacy readable in encrypt and refused in strict (500);
    • a legacy copy doesn't suppress the sealed write;
    • locked keys: writes and sealed reads → 503, legacy still served;
    • quota after unlock: after a restart, locked → legacy bytes only; unlocked → legacy + sealed; the next write adds to that;
    • swapped, tampered and TCE2 objects fail closed;
    • sealed remove;
    • DEK rotation keeps old blocks readable.
  • cargo test -p tinycloud-node --lib storage:: (27), -p tinycloud-core --lib storage::, did_resolution_single_entry and the at_rest_harness self-test pass. cargo clippy -p tinycloud-core -p tinycloud-node --all-targets -- -D warnings and cargo fmt --all --check are clean.

Review fixes (security 3c9d19cf, correctness d6a8c93b), commit 0cc5ce4

  • B1: with keys set up, a writing invocation is authorized (authorize_admitted / verify_and_authorize_with_graph, as on the read-only path) and its spaces' hosting is checked before any prepare hook runs, so a refused request creates no data key or mirror object. The transaction still authorizes as before.
  • B2:
    • every touched space gets the new ImmutableReadStore::prepare_read (→ DataKeyProvider::prepare_read: restores a lost DEK from its authenticated mirror, never creates one), and written spaces also get prepare_write;
    • inside the transaction, the store's get_kv/persist run in at_rest::write_locked, where the keyring answers NotPrepared (503) instead of any key-store write;
    • new trait items: ImmutableReadStore::{prepares, prepare_read}. The key-provider side (DataKeyProvider::prepare_read, DataKeyError::NotPrepared, write_locked, and the keyring guards) now lives in S1 feat(TC-887): at-rest keyring, key store and migration #326, unchanged.
  • N1: key-store error bodies carry reason codes; backend detail is logged. A stored DEK that does not unwrap is KeyInconsistent (500).
  • N2: integrity and unsupported failures are logged (rate-limited, no names), including mid-stream ones (LoggedDecrypt).
  • Correctness 4: FS contains checks only the name a write would use (INTERFACES §7).
  • seal_vec panics are Internal (500), and the in-memory sealed write is async.
  • INTERFACES §2 and §7 updated: the prepare invariant, errors, transition linkability, the downgrade window until strict, and remove.
  • Tests (RED on the pre-fix head 91a2a57, GREEN here):
    • at_rest_fs_node:
      • a_refused_put_creates_no_data_key_or_mirror_object;
      • a_mixed_read_and_delete_after_losing_the_data_key_row_completes, which hung 30 s before the fix.
    • Core: storage_prepares_hosted_spaces_before_the_writer_lock_and_persists_write_locked.
    • Keyring: under_the_write_lock_keys_are_only_looked_up.
    • FS sealed tests:
      • contains per §7;
      • off after encrypt, and off with locked keys;
      • strict with locked keys;
      • From/Suffix ranges at 2·64 KiB±1 and 3·64 KiB on both staging paths;
      • a failed sealed persist leaves no temp files.
    • The 503/500 mapping test now also covers /invoke reads and writes, and reason codes.

Delta round (security 6d578564, correctness 3f0b1aec), commit a6201d0, on S1 final #327 c1a040e

  • R1: new test an_unauthorized_put_to_a_hosted_space_creates_no_data_key. A session with only kv/get puts to its own hosted space: 401/403, 0 DEK rows, 0 mirror objects. It fails RED with the pre-check authorization removed (the never-hosted test does not), and is GREEN with it.
  • R2: integrity, unsupported and inconsistent-key failures are logged where the sealed name is known, with object = v1/{space tag}/{object tag} (keyed hashes only), including mid-stream ones. The route-level logging without a locator is gone.
  • CHANGELOG: sealed KV blocks, and the 401-instead-of-404 change on nodes with at-rest keys. Nodes without keys, and the SDK/e2e suites, are unaffected.
  • AtRestError::DekUnavailable (S1) maps to a 503 with its reason code.

@samgbafa

Copy link
Copy Markdown
Contributor Author

Rebased onto S1 round 3 (#327 daef3cf). Exact-head CI on 91a2a57 (dispatched with workflow_dispatch on the PR branch): run https://github.com/TinyCloudLabs/tinycloud-node/actions/runs/39032818172, all jobs green.

@samgbafa

Copy link
Copy Markdown
Contributor Author

REQUEST_CHANGES: see TC-887 (head 91a2a57744ebffc3912c3c137f928b739098c3d9)

@samgbafa

Copy link
Copy Markdown
Contributor Author

REQUEST_CHANGES: see TC-887 (head 91a2a57)

samgbafa added a commit that referenced this pull request Oct 17, 2026
…e-lock guards (#326)

- allow_resplit_before_verify no longer re-splits the same ARK: the
  node makes a fresh ARK (generation 1), splits it for the configured
  holders and retires the old one, so old shares rebuild only a key
  that never protected anything.
  - A W-authenticated tombstone per old ARK goes to the mirror first;
    load() then drops that ARK's key and escrow rows (also from a
    database restored to before the retirement) and ignores and deletes
    its ARK, escrow and record objects. Forged or junk tombstones are
    skipped; a database mixing retired and live rows is Inconsistent.
  - The database swaps keys in one transaction (replace_all_keys).
  - may_resplit also reads the mirror: any record- or dek- object, or
    any stored record, refuses the shortcut.
  - KeyMirror::delete (MemoryMirror implements it).
- Takes S3's #333 keyring changes (efa5bbb): write_locked /
  in_write_lock, DataKeyError::NotPrepared, the guards on restore_dek,
  mirror_dek and create_dek, DataKeyProvider::prepare_read and its
  test. D1 still holds: prepare_read's restore fails closed on a
  mirrored DEK that doesn't authenticate.
@samgbafa

Copy link
Copy Markdown
Contributor Author

APPROVE: see TC-887 (head efa5bbb6d6e5f5040005d2b8e04837b6e75c0dde)

@samgbafa

Copy link
Copy Markdown
Contributor Author

Review round (efa5bbb): exact-head CI run https://github.com/TinyCloudLabs/tinycloud-node/actions/runs/39045002838 — all jobs green.

@samgbafa

Copy link
Copy Markdown
Contributor Author

APPROVE: see TC-887 (head efa5bbb)

@samgbafa
samgbafa force-pushed the skgbafa/tc-887-s3-kv-sealed branch from 21aa0e9 to 843cea8 Compare October 17, 2026 16:52
@samgbafa
samgbafa force-pushed the skgbafa/tc-887-s3b-fs-sealed branch from efa5bbb to 4dbf283 Compare October 17, 2026 16:52
@samgbafa
samgbafa force-pushed the skgbafa/tc-887-s3-kv-sealed branch from 843cea8 to d05c261 Compare October 17, 2026 17:33
@samgbafa
samgbafa force-pushed the skgbafa/tc-887-s3b-fs-sealed branch from 4dbf283 to 8ba1e92 Compare October 17, 2026 17:33
@samgbafa

Copy link
Copy Markdown
Contributor Author

APPROVE: see TC-887 (head 8ba1e92)

@samgbafa

Copy link
Copy Markdown
Contributor Author

Rebased onto S1 #327 029c703. Exact-head CI on 8ba1e92: run https://github.com/TinyCloudLabs/tinycloud-node/actions/runs/39054264467, all jobs green.

…before the invoke lock

The FileSystem block store follows the at-rest mode and keys: sealed
writes under v1/{space_tag}/{object_tag} (in-memory and temp-file
staging; atomic rename), reads and ranges through the read plan
(sealed, then legacy unless strict; 503 while locked), plaintext sizes
by tag, sealed removal. lib.rs links the store to AtRestRuntime; an S3
block store refuses a non-off mode until it can seal.

New ImmutableWriteStore::prepare_write hook, which invoke calls for
each written space before the SQLite writer lock and transaction; the
FS store creates the space's data key there. Invariant: no key-store
I/O inside invoke's writer lock or transaction (a DEK insert there
deadlocked SQLite nodes on the first sealed put).
Box the decrypting reader inside FsReadable and the sealed read, range
and persist futures behind the async-trait methods. The larger inline
futures overflowed the 2 MiB test-thread stack in
policy_v3::tests::policy_owner_may_be_a_session_the_space_owner_delegated_to.
On S1 round 3, at_rest::setup refuses a non-off mode until storage
declares a seal path. storage::attach_at_rest declares it for the
FileSystem store, which seals every write; S3 declares none yet, so its
own refusal goes and setup's applies. The ceremony test now expects
encrypt and strict to boot and encrypt with a pinned, verified escrow;
the docs no longer say no storage seals. Drops the duplicate
parking_lot dev-dependency.
…te lock; keys only looked up under it (S3 review)
…ed-space put test; changelog (S3 review deltas)
@samgbafa
samgbafa force-pushed the skgbafa/tc-887-s3b-fs-sealed branch from 8ba1e92 to a6201d0 Compare October 17, 2026 18:48
@samgbafa
samgbafa force-pushed the skgbafa/tc-887-s3-kv-sealed branch from d05c261 to 8acfd5d Compare October 17, 2026 18:48

This branch has not been deployed

No deployments
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.

1 participant