Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions docs/PHI.md
Original file line number Diff line number Diff line change
Expand Up @@ -409,10 +409,20 @@ application log files (`[logging].log_dir`).
([store/backup_codec.py](../messagefoundry/store/backup_codec.py), whose own docstring says the
cipher *mechanism* is net-new): chunked AES-256-GCM under the store DEK resolved directly by
`resolve_active_key`, with a per-chunk AAD of
`header_sha256 ‖ frame_counter(uint64) ‖ final_flag(uint8)` — **not** a per-cell AAD. Because the
`header_sha256 ‖ frame_counter(uint64) ‖ final_flag(uint8)` — **not** a per-cell AAD. Because that
key is resolved by `resolve_active_key` and not `build_store_cipher`, `cipher_provider =
vault_transit` **never applies to a backup**. And `[backup].allow_unencrypted = true` writes a
**CLEARTEXT `.mfbak.plain`** — a plaintext PHI-body archive on disk.
vault_transit` **never applies to sealing or unsealing an archive**. And
`[backup].allow_unencrypted = true` writes a **CLEARTEXT `.mfbak.plain`** — a plaintext PHI-body
archive on disk.
- *Scope of that sentence.* It is about the **archive**, not about every step of a backup run.
`[backup].full_restore_verify` opens the *extracted* snapshot and decrypts its cells to prove the
PHI is still readable, and reading a store cell is what the store cipher is for — so that step
**does** go through `build_store_cipher`, and therefore through Transit under `vault_transit`.
Two paths, two key uses, one key source; the split is recorded once in
[ADR 0049](adr/0049-turnkey-dr-backup-restore-verify.md) §"Encryption boundary" and pinned by
`tests/test_phi_at_rest_inventory.py::test_the_mfbak_seal_never_reaches_for_the_store_cipher`.
That pass decrypts into memory and reports a **count** of cells opened; it writes no plaintext,
so it adds no at-rest tier to this inventory.
- *File-connector spill dirs* — **plaintext on disk**; there is no cipher on that path, only
volume/share encryption and the directory ACL.
- **Integrity.** The per-value GCM tag is the tamper-evidence. For `attachment_chunk.ciphertext` each
Expand Down
66 changes: 62 additions & 4 deletions docs/adr/0049-turnkey-dr-backup-restore-verify.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,35 @@ below), containing:
escape (parallel to `[store].allow_unencrypted_phi`) for a synthetic/non-PHI box. A synthetic instance with
no key may back up in the clear; a PHI instance may not, silently.

#### Two key USES, one key SOURCE — which path holds which (BACKLOG #1561)

`pipeline/dr_backup.py` reaches key material in two places, and they are different operations. Stated here
so the next reader does not re-derive it from the imports, and because a guard was once written on the
premise that there was only one:

| Operation | Where | Key material | Does `cipher_provider` apply? |
|---|---|---|---|
| **Seal / unseal the `.mfbak`** | `_do_backup` to `_resolve_key` to `_build_archive_blocking` to `encrypt_stream`; `_verify_archive_blocking` to `decrypt_stream` | **Raw DEK bytes** from `resolve_active_key`, handed to `store/backup_codec.py` | **No.** The codec takes bytes and builds its own `AESGCM`. `vault_transit` is never consulted, so a Transit posture does not protect (or reach) the archive frames |
| **Read the extracted snapshot's cells** (`full_restore_verify` only) | `_full_open_check` to `open_store` / `_decrypt_check` | **The store cipher** from `build_store_cipher`, with the store's own per-cell AAD | **Yes.** It is a store read, so it dispatches on `cipher_provider` exactly as a live read does — including Transit |

Three consequences worth stating rather than inferring:

- **The snapshot read is not a second at-rest tier.** `_decrypt_check` decrypts into memory and returns a
**count** of cells opened, never a plaintext, and writes nothing. The extracted `store.db` keeps the
store's own column cipher; `docs/PHI.md` §2 already inventories that staging dir.
- **`open_store` builds a store cipher too.** That is why the table lists it beside `build_store_cipher`:
a "does this file touch the store cipher" question answered by searching for one name gets the wrong
answer. The full verify has called `open_store` since this ADR shipped; what #1561 changed is that it
now passes the LIVE settings, so an encrypted snapshot opens under a real key instead of the identity
cipher.
- **The guard is scoped to the seal, and it is an AST call-path check, not a token scan.**
`tests/test_phi_at_rest_inventory.py::test_the_mfbak_seal_never_reaches_for_the_store_cipher` asserts
that every archive-codec call sits in the seal/unseal region, that no store-cipher constructor sits
in it, and that the bytes handed to `encrypt_stream` are the unmodified `resolve_active_key` DEK.
"Sits in" is lexical and deliberate: the guard follows no calls, so a cipher construction moved into
a helper called from the seal lands OUTSIDE the permitted region and reds. Both seams are registered
in `scripts/security/crypto_inventory_check.py` (ASVS 11.1.3).

> **Key-availability consequence for #61's cold seed (addressed by design).** Because the archive is encrypted
> under the store DEK, **the DR site must have that DEK available to restore the cold seed.** ADR 0048's cold
> path **requires the same `KeyProvider` posture at the DR site** — env var / DPAPI key file / or reachability
Expand Down Expand Up @@ -239,7 +268,7 @@ retention_keep = 7 # keep-N: prune the oldest archives beyond N af
snapshot_method = "vacuum_into" # "vacuum_into" (default, writer-lock under off-peak schedule) | "online_backup" (low-contention)
include_config = true # bundle the loaded --config dir into the archive
verify_after_backup = true # run the lightweight restore-verify after each backup (default ON)
full_restore_verify = false # the heavier restore-to-temp through open_store; on-demand / opt-in extra
full_restore_verify = false # the heavier open through open_store + a decrypt pass; on-demand / opt-in extra
config_only_on_server_db = true # on postgres/sqlserver, back up config only; the DB is DBA-delegated (#52)
allow_unencrypted = false # audited escape: permit a clear archive ONLY for a no-key synthetic instance
```
Expand Down Expand Up @@ -325,9 +354,28 @@ The owner-locked posture is **lightweight verification after each backup**, with
the keep-N prune does not count it as the latest good backup** (so a failing backup never silently evicts
the last *good* one).

`full_restore_verify` (opt-in / on-demand) additionally restores the snapshot to a throwaway temp DB and opens
it through the real `open_store` path (cipher + migrations) to prove an end-to-end restore — heavier, so not
the per-backup default.
### Full restore-verify (opt-in / on-demand)

`full_restore_verify` additionally opens the extracted snapshot through the real `open_store` path — **under
this instance's LIVE `[store]` settings, with only the path and the backend substituted** — and then decrypts
**and authenticates** every cipher-covered cell it holds. Heavier, so not the per-backup default.

Two properties of that sentence are load-bearing, and the shipped code got both wrong until they were named
here:

- **The settings must be the live ones.** A bare `StoreSettings(path=…)` resolves no key, so an encrypted
snapshot opens under the identity cipher. Substituting *only* the path is what carries `cipher_provider`,
`key_provider`, the active + retired keyring and `aad_bind` into the verify, and it is why this is a
`model_copy` rather than a rebuilt object: a field added to `StoreSettings` later rides along instead of
being silently dropped. `backend` is the one other substitution — the archive member is a SQLite file by
construction, so an instance that has since moved to a server DB still verifies its older SQLite archive
against SQLite.
- **Opening the store is not reading the PHI.** `PRAGMA quick_check` and the row counts are blind to a
bit-flipped AEAD cell, so without the decrypt pass a full verify would report `PASS` on an archive whose
bodies no longer decrypt. Each cell is opened with the same cell-bound AAD the store writes (ASVS 11.3.3),
and the result reports a **count** of cells opened — never a plaintext — so a `PASS` states which claim it
is making. The covered cells are the store's own `_CIPHER_COLUMNS` declaration; its cipher-covered tables
whose AAD binds to a composite/natural key are out of scope until the store publishes them as data too.

## Acceptance Criteria

Expand Down Expand Up @@ -386,6 +434,16 @@ the per-backup default.
- **AC-12** — WHILE clustered (active-passive HA) AND not the leader, THE SYSTEM SHALL NOT take a backup or
prune the shared destination; WHILE single-node (`NullCoordinator`), THE SYSTEM SHALL always run.
→ `tests/test_backup_runner.py::test_backup_is_leader_gated`
- **AC-13** — WHEN `full_restore_verify` runs, THE SYSTEM SHALL open the extracted snapshot under this
instance's live `[store]` settings (only the path and the backend substituted) AND decrypt **and
authenticate** every cipher-covered cell in it, reporting the number of cells opened; IF a cell fails its
AEAD tag, OR the settings resolve no key for a snapshot that holds sealed cells, OR no live settings are
supplied, THEN THE SYSTEM SHALL return `FAIL` naming that cause. A good encrypted archive SHALL verify
`PASS`, and a good unencrypted archive SHALL verify `PASS` with zero cells opened.
→ `tests/test_restore_verify.py::test_full_verify_passes_on_a_good_encrypted_archive`
→ `tests/test_restore_verify.py::test_full_verify_fails_on_a_corrupted_aead_cell`
→ `tests/test_restore_verify.py::test_full_verify_fails_when_the_snapshot_opens_without_its_key`
→ `tests/test_restore_verify.py::test_full_verify_passes_on_a_good_unencrypted_archive`

## Options considered

Expand Down
12 changes: 10 additions & 2 deletions messagefoundry/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4953,8 +4953,10 @@
"""Verify an existing ``.mfbak`` archive WITHOUT activating it (ADR 0049, #60 — 0049's owned
primitive that ADR 0048's cold-seed activation calls): key-fingerprint precheck (a clean
``KEY_MISMATCH`` before any decrypt) -> decrypt -> open the embedded store read-only ->
``integrity_check`` + per-table row-count vs the manifest. Reports ``PASS``/``FAIL``/
``KEY_MISMATCH``; PHI-safe (counts + a reason only, never a body)."""
``integrity_check`` + per-table row-count vs the manifest. ``--full`` additionally re-opens the
snapshot under THIS instance's real store settings (cipher, keyring, key provider) and decrypts +
authenticates its cipher-covered cells. Reports ``PASS``/``FAIL``/``KEY_MISMATCH``; PHI-safe (counts
+ a reason only, never a body)."""
import asyncio
from pathlib import Path

Expand All @@ -4981,6 +4983,10 @@
"integrity_ok": result.integrity_ok,
"row_counts": result.row_counts,
"manifest_counts": result.manifest_counts,
# A count of the cipher-covered cells --full decrypted AND authenticated (0 on a lightweight
# verify, and on an unencrypted store). It is what separates "the snapshot opened" from "its
# PHI was readable", so an operator can see which claim a PASS is making.
"decrypted_cells": result.decrypted_cells,
"reason": result.reason,
}
if args.json:
Expand All @@ -4989,6 +4995,8 @@
print(f"{result.status}: {result.reason or 'archive verified'}")
if result.row_counts:
print(f" row_counts={result.row_counts}")
if result.decrypted_cells:
print(f" decrypted_cells={result.decrypted_cells}")

Check notice on line 4999 in messagefoundry/__main__.py

View workflow job for this annotation

GitHub Actions / diff-coverage (advisory)

Missing Coverage

Line 4999 missing coverage
# exit 0 only on PASS; FAIL/KEY_MISMATCH are non-zero so a script/cold-seed activation can gate on it.
return 0 if result.ok else 1

Expand Down
Loading
Loading