diff --git a/DECISIONS.md b/DECISIONS.md index 3bb73fe4..1cfade1b 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -33723,3 +33723,23 @@ Evidence: PR #107 (https://github.com/QuantumShieldLabs/qsl-protocol/pull/107) m - Both formatter test shadow copies (`identity_binding.rs:37`, `identity_foundation_contract_na0217d.rs:133`) must be kept in lockstep with the production formatter forever and must remain duplicates rather than imports. - qsl-desktop's displayed verification code changes when it next bumps its qsc pin; `DESIGN_SPEC_AppendixD.md` and `ui/style.css` go stale at that moment. Desktop pin-bump lane's concern. - **References:** NA-0669; D605 (`QSL-DIR-2026-07-23-605`, approved 2026-07-23, sha256 `4852f471e7bf10d91bd9a8d95d7e08f6af46b8276d816b081e71a74de9274b73`, 374 lines); operator ruling (A) extending §5a to `tests/identity_foundation_contract_na0217d.rs` for the one lockstep edit; D-1295 (the implementation decision this closes, merged as `ccc6c008` over head `3d32a1c0`, PR #1630); D-1294 (NA-0668, whose standing method this lane discharges by observation); the 2026-07-22 independent audit (findings C-1a, C-4, C-6; C-1b, C-2 and the target-width decision deliberately out of scope); `docs/governance/evidence/NA-0669_as_built.md`; `tests/NA-0669_audit_client_security_fixes_testplan.md`; `docs/ops/ROLLING_OPERATIONS_JOURNAL.md` (2026-07-23 entry + closeout). **Nothing was filed in `docs/ops/IMPROVEMENT_LEDGER.md` — deliberately.** + +- **ID:** D-1297 + - **Title:** NA-0670 closeout — the last unfixed HIGH of the 2026-07-22 independent audit (C-2) shipped: qsl-server `auth_ok` now compares the bearer token in constant time (SHA-256 both sides, then the client's `hs_ct_eq_32` XOR-fold), closing a remote timing oracle on the one network-exposed component; like NA-0669 and unlike the four docs-only lanes before it, the qsl-server implementation PR changed `src/` so its required `rust` check RAN and its green IS evidence for the behavioural half + - **Date:** 2026-07-23 + - **Status:** Accepted + - **Goal IDs:** G4 + - **Decision:** Close NA-0670 per **QSL-DIR-2026-07-23-606 (D606)**. The qsl-server implementation (satellite decision **D-0014**) merged as `5235c2bf` (PR #64) over base `b4f86a3c`; the queue returns to `READY=NONE | HIGHEST_NA=0670 | HIGHEST_D=1297`. **THE FIX, IN PLAIN TERMS:** `auth_ok`'s final `provided == token` used `str::eq`, which short-circuits on the first differing byte — a **remote timing oracle** on the one component deliberately exposed to the network, in the low-jitter LAN/tailnet regime where byte-at-a-time statistical amplification is actually practical. It is now `ct_eq_secret(provided, token)`, a private helper that reduces both sides to a fixed 32-byte SHA-256 digest and folds them with an XOR-accumulate loop over all 32 bytes (`diff |= da[i] ^ db[i]`, then `diff == 0`) — the exact shape of the qsc client's `hs_ct_eq_32` (ENG-0003) and consistent with this file's own `route_key_for`, which already hashes the **other** secret (the route token), so the relay now treats both its secrets the same way (the strongest form of the fix, §1b). Hashing normalises both inputs to 32 bytes, so the fold does identical work for every input, **closing the length leak** the audit offered as optional (§2c). The two `return false` guards (missing / malformed `Authorization`) are kept — they branch on attacker-known request shape, not on the secret. **NO dependency change:** `sha2` is already a direct dependency and `use sha2::{Digest, Sha256}` already imported; `subtle` was **deliberately NOT added** (it is present in `Cargo.lock` only via rustls under the reqwest DEV-dependency; a direct use would newly enter the production graph, and — being defined for equal-length slices — would not close the length leak without a length-visible branch; §2b/§2c) — the `Cargo.toml`/`Cargo.lock` diff is verified empty. **NO wire/API/protocol/schema/env change;** the four gated handlers (`server-info`, `push`, `pull`, `pull/ack`) still reject-before-mutation because `auth_ok` remains their first statement. **⚠ THIS LANE'S GREEN IS EVIDENCE, AND THE DISTINCTION IS STATED SO THE CLOSEOUT DOES NOT INHERIT THE WRONG CAVEAT:** the qsl-server implementation PR changed `src/lib.rs`, so `classify_ci_scope` returns runtime/non-docs and the required `rust` check **RAN** — green on `82df531`, the **exact reviewed commit** (run head == PR head == local HEAD, 1m36s). That green is real evidence for the **behavioural** half (auth still accepts the right token and rejects wrong ones, including the same-length case, under CI). It is the inverse of the queue-promotion PR (docs_only, green proved nothing) and of the four docs-only lanes NA-0664/0666/0667/0668. **The constant-time property ITSELF is STRUCTURAL — fixed work over the full 32-byte digest, no data-dependent early return — verified by READING, not by measurement; no timing proof was run or claimed (§3.4 held).** A **same-length wrong-token test** was added (`"topsecreX"` vs `"topsecret"`, both 9 bytes): the pre-existing wrong-token test uses different-length tokens, so `==` rejected on length before comparing a single byte and **passed against the buggy code** — the same-length case is the only behavioural test that exercises the fold, and it proves the fold's ANSWER (reject + no mutation), not its timing. Phase 0 re-verified every §0/§1 anchor at the seating base `b4f86a3c` (`provided == token` at `src/lib.rs:507` exactly; `sha2` imported at `:10`; `route_key_for` at `:486`; the `==` body-sweep confirming `:507` is the **sole** secret comparison — so no second fix is owed, §1c). **FOUR LEDGER FILINGS, NONE FIXED (scope discipline held — one function, nothing else touched):** ENG-0063 (precompute `Sha256::digest(token)` once at `AppState` construction — a refinement riding the `new_with_auth*` constructor chain, §5), ENG-0064 (the two-repo seat leaked qsl-protocol's `CARGO_TARGET_DIR` into the qsl-server seat, defeating the per-repo cache separation NA-0667's registration established — worked around with an explicit pin, not fixed), ENG-0065 (a pre-existing flaky log-capture test, `tests::logs_do_not_contain_raw_channel`, **surfaced not caused** by the required new test at 6-core contention — CI-irrelevant at ≤4 threads), ENG-0066 (qsl-server `TRACEABILITY.md` lapsed for three satellite lanes). Commit identity on the qsl-server implementation commit and on this closeout is the GH007 noreply (`238594419+Tebbens4832@users.noreply.github.com`), author and committer, trailers empty, verified on the object (WF-0029 per-commit override). Queue returns to `READY=NONE`; the operator promotes; the executor cannot self-promote. + - **Rationale:** The last unfixed HIGH of the independent audit is shipped on the single network-exposed component, in the file's own existing idiom (SHA-256 then fold), with no dependency, wire, or behavioural change. Because the implementation PR changed `src/`, its required `rust` check ran and its green is real evidence for the behavioural half — while the constant-time property remains a structural, read-verified claim, never a measured one, holding the near-miss line before there is a near-miss. Unlike NA-0669, this lane DID file — four ledger entries — because each records a real, decision-bearing finding that would otherwise evaporate in a relay (an observation that lives only in a turn record never gets scheduled), and filing them is recording, not a second fix; scope discipline barred doing any of that work inside a one-function lane, and none was done. + - **Security invariants introduced/changed:** The relay's bearer-token comparison is now **constant-time** (fixed work over a 32-byte digest, no data-dependent early return) and **length-independent**, removing a remote timing oracle on the network-exposed shared secret. `auth_ok` accepts and rejects EXACTLY the tokens it did before. No protocol, wire, handshake, ratchet, vault, or cryptographic-primitive change; no marker-vocabulary change; no new error code; no new dependency. A residual **constant per-deployment** timing offset from re-hashing the token on each request remains (proportional to the token's block count, not per-guess and not content-revealing); it is filed as ENG-0063, not folded in. This closeout ships governance records only. + - **Alternatives considered:** + - **`subtle::ConstantTimeEq` on the raw bytes.** Declined (§2b/§2c) — it would newly enter the production dependency graph, and being defined for equal-length slices it would not close the length leak without a length-visible branch; hashing-first closes it for free and yields the `hs_ct_eq_32` idiom the file already uses. + - **HMAC with a random per-process key.** Declined (§2d) — its random key defends against precomputation, but an attacker who already holds a candidate token can simply send it; plain SHA-256 delivers the property actually needed (comparison time independent of matching-prefix length) with no added machinery. + - **Precomputing the token digest at `AppState` construction.** Deferred and filed as ENG-0063 — it removes the secret from the per-request path and erases the block-count offset, but touches the multi-constructor `new_with_auth*` chain, correctly out of a one-function lane. + - **Filing nothing (the NA-0669 posture).** Declined — the four findings are real and decision-bearing; recording them at closeout is the correct home and does not reopen scope discipline. + - **Implications for spec/impl/tests:** + - Queue at `READY=NONE | HIGHEST_NA=0670 | HIGHEST_D=1297`; helper `READY_COUNT 0` with exit 2 is CORRECT at `READY=NONE` (`--allow-nonready-count` not passed) and is not a failure. + - qsl-server records satellite decision **D-0014** and a `TRACEABILITY.md` row; the required `rust` check ran green on the implementation PR (#64, head `82df531`, merged `5235c2bf`). + - No qsl-desktop, qsc, or protocol/spec change. The 2026-07-22 audit finding **C-2 is now closed**, and with it the last audit HIGH; the route-token path (`route_key_for`) is named as evidence, not a target, and is byte-unchanged. + - Four new `IMPROVEMENT_LEDGER.md` entries (ENG-0063…ENG-0066), all `open`, none fixed by this lane. + - **References:** NA-0670; D606 (`QSL-DIR-2026-07-23-606`, approved 2026-07-23, sha256 `ef3bd063f936564935b86e806e459e2bc93be6a4b52fdba685611e43e8a177ca`, 284 lines); D-0014 (qsl-server satellite decision, merged as `5235c2bf`, PR #64 over base `b4f86a3c`); the 2026-07-22 independent audit finding **C-2** (the last unfixed HIGH, and the only one outside the spine); qsl-protocol **ENG-0003** (client `hs_ct_eq_32`); this closeout's four filings **ENG-0063 / ENG-0064 / ENG-0065 / ENG-0066**; `docs/governance/evidence/NA-0670_as_built.md`; `tests/NA-0670_qsl_server_c2_constant_time_bearer_testplan.md`; `docs/ops/ROLLING_OPERATIONS_JOURNAL.md` (2026-07-23 NA-0670 closeout entry). diff --git a/NEXT_ACTIONS.md b/NEXT_ACTIONS.md index 46ceea40..10473c5e 100644 --- a/NEXT_ACTIONS.md +++ b/NEXT_ACTIONS.md @@ -6,8 +6,9 @@ Goals: G4 (primary), drives G1–G3 delivery ## LIVE QUEUE -`STATE: READY=NA-0670 | HIGHEST_NA=0670 | HIGHEST_D=1296 | BACKLOG_SOURCE=docs/ops/IMPROVEMENT_LEDGER.md` - +`STATE: READY=NONE | HIGHEST_NA=0670 | HIGHEST_D=1297 | BACKLOG_SOURCE=docs/ops/IMPROVEMENT_LEDGER.md` + + @@ -36208,7 +36209,7 @@ Begins at D-1295. TWO PRs, two decisions: D-1295 implementation, D-1296 closeout ### NA-0670 — Constant-time bearer-token comparison in the relay (D606, CROSS-REPO, SHIPPING CODE): fix audit C-2 — replace `auth_ok`'s short-circuiting `provided == token` with a length-independent constant-time check (SHA-256 both sides, then the in-house `hs_ct_eq_32` XOR-fold), on the one component deliberately exposed to the network — ITS `rust` CHECK RUNS AND ITS GREEN IS EVIDENCE -Status: READY +Status: DONE Goals: G4 Wire/behavior change allowed? NO observable wire/API/protocol/schema/env/behaviour change — `auth_ok` accepts and rejects EXACTLY the tokens it did; only the internal byte-comparison method changes (first-differing-byte short-circuit → fixed-width XOR-fold over a 32-byte digest). No dependency or lockfile motion (`sha2` is already a direct dep and already imported; `subtle` MUST NOT be added — it is dev-only in the lock). Crypto/state-machine change allowed? NO protocol/handshake/vault crypto or state-machine change. SHA-256 (already a direct dependency) is used purely as a length-normaliser for a LOCAL constant-time comparison; no key, no wire byte, no stored-format byte changes. diff --git a/TRACEABILITY.md b/TRACEABILITY.md index 5a278017..3547a24a 100644 --- a/TRACEABILITY.md +++ b/TRACEABILITY.md @@ -1636,3 +1636,4 @@ NA-0056 (public demo/client v1): apps/qsl-tui + scripts/demo + DOC-TST-RELAY-TUI - 2026-07-23: NA-0668 closeout addendum (D-1294, recorded after PR #1624 merged as `565d480c`) — **ACCEPTANCE §5.B.6 CLOSED AT 4/4 ON REAL MIRRORS.** The operator refreshed `qsl-protocol` and re-ran `check_repo_registration.sh`: `4 repos checked, 0 issue(s), 0 unverified`, exit 0, each repo printing `mirror CURRENT at `. **The lane therefore holds a COMPLETE FAIL → refresh → PASS pair against PRODUCTION mirrors rather than only the throwaway fixture**, the fail half having been caught **unprompted on the check's first live run** against a staleness created by the lane's own promotion and merge — exactly the acceptance the lane was defined by. **⚠ AND IT WENT STALE A THIRD TIME WITHIN THE HOUR, RECORDED AS DATA NOT A DEFECT:** merging #1624 advanced origin `8a05c1a3` → `565d480c`, so **every spine merge re-stales the spine mirror**; three staleness events on 2026-07-23 alone. Deliberately not chased — §3b self-heals it at the next lane seat and the downstream origin asserts prevent a wrong checkout regardless — but it is the **measured decay rate**, and it means *"is the mirror set 4/4 right now?"* has **a shelf life of one merge**. **NON-FATAL REFRESH RULING AFFIRMED**, reasoning recorded because fatal is the superficially safer-looking choice: `qwork.sh:372` and `qshell.sh:137` hard-assert `head_equals_origin_main` **after** the mirror seeds the clone, so **a stale mirror CANNOT produce a wrong checkout — the refresh is hygiene, not a correctness gate**; fatal would block work on a network blip for **zero correctness gain**, *"a gate that cannot pass teaches bypass"* arriving inside the fix for it, while absent-mirror-stays-fatal preserves the real invariant. **COSTUMES 6 AND 7 ACCEPTED AND THEIR UNIFYING SHAPE NAMED: in both cases the tool answered a question ADJACENT to the one intended, and the answer was shaped like an answer to the intended question** — `test -f` on a path missing its label suffix answered *"does this nonexistent path exist?"* (no), not *"did B0 back up these files?"*; `check-ignore` on a **tracked** file answered *"is this tracked file ignored?"* (no, tracked files are exempt), not *"is this directory ignored?"*. **Opposite directions — a false alarm against completed work, and a false all-clear that nearly shipped a PR with no as-built — same defect: the instrument was pointed slightly off the question**, and neither was catchable by re-reading output that was well-formed and internally consistent. **THE STANDING METHOD'S FINAL CLAUSE IS GENERALIZED, SUPERSEDING ITS NARROWER FIRST DRAFT: A NEGATIVE RESULT IS ONLY EVIDENCE IF THE INSTRUMENT COULD HAVE RETURNED POSITIVE** — the earlier *"…if the PATH it was measured against exists"* covers costume 6 and **misses costume 7 entirely**; discharged by running a **positive control**. **⚠ THE OPERATOR'S QUESTION ANSWERED: NOTHING RUNS `check_repo_registration.sh` AUTOMATICALLY, AND NOTHING EVEN TELLS ANYONE TO** — not cron (`no crontab for victor`), not any of the 16 systemd timers (the two project ones run `qsl-backup daily` and `qbuild-ssd-maintenance`), not a Claude Code hook (`settings.json` has **no `hooks` key at all** — the WF-0036 gap), not spine CI, and not any of the 15 tools scripts, **whose only two mentions of it are comments NA-0668 itself wrote**; it is named in **zero** read-first documents or checklists, every spine reference being narrative rather than procedural. **The negative was validated with POSITIVE CONTROLS** (the same greps returned 8 and 3 real references for sibling scripts; the same doc sweep returned 3 and 2 `qwork` mentions). **Consequence: the detector inherits the exact shape of the artifact it replaced — §3a made the assertion correct but did not give it an OWNER, so it reads as health whenever nobody looks.** **Bounded honestly:** §3b defuses the operational half, and the residual risk is a repo in which **no lane is ever seated** (`qsl-attachments`), which receives no §3b refresh and would drift unseen. **FILED AS WF-0041 with four unchosen options — timer / `qwork`-invoked / Claude Code hook (which would also resolve WF-0036 and should be decided with it) / checklist, the last explicitly the weakest since it re-implements the memory dependency it exists to remove — and DELIBERATELY NOT FIXED, per operator instruction.** Closeout verified on **both layers**: helper `READY_COUNT 0` with `NA-0668 DONE` and **exit 2, correct at `READY=NONE`, `--allow-nonready-count` NOT passed**; STATE `READY=NONE | HIGHEST_NA=0668 | HIGHEST_D=1294`; `D-1294` present once, `DUPLICATE_COUNT 0`. The 2026-07-23 journal observation **remains an observation and is NOT promoted to a principle.** Evidence: `docs/governance/evidence/NA-0668_as_built.md` §10; `tests/NA-0668_mirror_freshness_testplan.md` §6.1/§6.2/§8. - 2026-07-23: NA-0669 (the three client security fixes of the 2026-07-22 independent audit, D-1295 implementation + D-1296 closeout) — **THE FIRST SHIPPING-CODE LANE IN FIVE, AND THE FIRST WHOSE CI GREEN CARRIES INFORMATION.** `scripts/ci/classify_ci_scope.sh` over the implementation PR returns `docs_only=false | runtime_critical=true | scope_class=runtime_critical`, so **both full suites RUN and a green COULD HAVE BEEN RED** — verified with a **positive control** (the same classifier still returns `docs_only=true` for a docs-only file set), which discharges NA-0668's standing method (*a negative result is only evidence if the instrument could have returned positive*) **by construction rather than by argument**, and inverts NA-0664/NA-0667/NA-0668, whose spine diffs all classified `docs_only=true` and whose greens proved nothing. Executed per QSL-DIR-2026-07-23-605 (D605, APPROVED 2026-07-23, sha256 `4852f471e7bf10d91bd9a8d95d7e08f6af46b8276d816b081e71a74de9274b73`, 374 lines) in the directive's order — C-6, then C-1a with its lockstep test update, then C-4. **THREE FILES, THREE LINES OF BEHAVIOUR.** **C-6:** `crate::fsync_dir_best_effort(parent)` after the rename in `write_vault_atomic`, byte-parallel to `vault_init_core` — `sync_all()` persisted the tmp file's CONTENTS, never the DIRECTORY ENTRY the rename creates; the function has **THREE callers** (`secret_set`, `secret_set_with_passphrase`, `persist_session`), i.e. **every steady-state vault mutation in the product**, and was the **ONLY** rename-then-nothing site in the crate. **⚠ ITS EVIDENCE BOUNDARY IS STATED RATHER THAN BLURRED: no test in the suite can observe C-6** — directory-entry durability is only visible across a power failure — so **the green proves NON-REGRESSION for C-6 and nothing more**, and what supports it is the parity argument, not the check. **C-1a:** strip `QSCFP-` before the alphanumeric filter, which had dropped the prefix's hyphen but KEPT its five letters, making five of sixteen displayed characters constant so every code ever shown began `QSCF-P`; width 16 and `4-4-4-4-checksum` grouping unchanged. **MEASURED THROUGH THE REAL PRODUCTION FUNCTION, NOT A MODEL** — five real identities from the real built binary plus 4000 real-shaped fingerprints through the `qsc` library linked as an external consumer crate, built once from the base tree and once from the fixed tree: constant display positions `[0,1,2,3,4,5,9,14,19]` → `[4,9,14,19]`, varying hex **11 → 16**, i.e. **44 → 64 bits**, position-for-position identical to the directive's modeled table. **⚠ 64 BITS AT WIDTH 16, NOT THE INTENT'S 88/22** (16 hex = 64 bits; 88 needs 22, which needs the excluded width change and is not divisible by the four-character grouping), **and the directive's own "20-fold reduction" phrasing is a 20-BIT reduction — 2²⁰ ≈ 1,048,576 — corrected in the record rather than quietly fixed**, with the bit figures and every ruling resting on them unaffected. **⚠ THE SHADOW COPY (`tests/identity_binding.rs:37`) WAS UPDATED IN LOCKSTEP AND DELIBERATELY KEPT AS A DUPLICATE** rather than replaced by an import — its separateness is what makes the test a real check instead of a tautology — and **no assertion was weakened and no expected value needed editing, for a stronger reason than the directive anticipated: the test COMPUTES its expected code rather than pinning a literal, so the formula update IS the value update.** **A THIRD CALL SITE THE DIRECTIVE DID NOT NAME** (`tests/NA_0649_gui_surface.rs:311-312`) was found by sweeping all callers rather than working from its list, and is unaffected. **C-4: RULED REJECT, NOT RAW BYTES.** Enumerating the COMPLETE single-byte input space through Rust's real `from_utf8_lossy` gives **129 distinct outcomes and Shannon H = 4.500 bits of 8** (all 128 bytes `0x80-0xFF` collapse to one character), so `head -c 32 /dev/urandom > pass.txt` produced a vault the operator believed held 256 bits and which held **≈144** — the audit's 145–160 confirmed — corroborated over 200,000 random 32-byte trials of which **ZERO were valid UTF-8**. Raw-bytes-end-to-end is the better design and was rejected as **uncontained** (~10 sites, two `pub` signature changes on the qsl-desktop-facing surface) and deferred to the vault-format lane; reject is one line, the existing error code, and converts a **silent degradation into a loud, diagnosable failure at the moment of use**. **OBSERVED WITH A POSITIVE CONTROL:** the base binary **silently accepted** the raw file (exit 0), the fixed binary **rejects** it (`vault_passphrase_file_read_failed`, exit 1), and base64 of the same 32 bytes round-trips with **all 256 bits reaching Argon2id byte-for-byte** (the pre-fix path handed Argon2id **64 bytes** for a 32-byte file — a longer string carrying less information). **BOTH MIGRATION BREAKS ACCEPTED, NO MIGRATION, KNOWINGLY RATHER THAN BY DISCOVERY:** every existing **code-pinned contact** now yields `peer_mismatch` (a property of changing the FORMAT, not the matcher, so it happens with `:562` untouched), and every existing **vault created from a non-UTF-8 passphrase file is permanently unopenable** — demonstrated live rather than asserted, reject being the kinder candidate because it fails with the named error rather than as an indistinguishable wrong key. **F1 = (a): `identity_pin_matches_seen:562` NOT TOUCHED** — codes ARE stored as pins today (`contacts/mod.rs:908`/`:918` verbatim; the `(None,None)` arm performs no key check at all) and a named test covers it, so deleting the branch is a **product decision**, not a one-line security fix; **(b) declined explicitly** (no marker-vocabulary change). **Recorded because it cuts the other way:** the adversarial oracle `binding_fuzz.rs:396` is a strict full-fingerprint compare with **no code branch**, and the production result is computed into `_canonical_pin_matches` and **DISCARDED** in its favour — the strict semantics are already the modeled and attested ones, and the production leniency is a **known, unasserted divergence from the model**; the oracle was NOT edited. **PHASE 0 RE-ANCHORED EVERY file:line AT THE SEATING BASE** (`14a2464c` → `62118874`): `git diff --name-status` between them returns **`M NEXT_ACTIONS.md` and nothing else**, each anchor then confirmed individually, so nothing was edited at a shifted line. **SCOPE DISCIPLINE WAS THE SECOND DELIVERABLE:** `Cargo.toml`/`Cargo.lock` verified untouched **including after building an external consumer crate against the tree**; `binding_fuzz.rs` and `:562` byte-unchanged; **zero new rustfmt drift** (the crate's pre-existing 42-file drift is the known owed micro-lane, verified identical before and after); and **NOTHING NEW WAS FILED in `IMPROVEMENT_LEDGER.md`** — observations went to the operator relay. **CROSS-REPO, REPORT ONLY:** C-1a changes the code qsl-desktop displays **only when desktop bumps its qsc pin**, a separate lane; `docs/DESIGN_SPEC_AppendixD.md:100`/`:221` and `ui/style.css:280` go stale at that moment, **no desktop test pins the format so desktop CI will not break**, and no spine doc pins it either. **C-2 (constant-time bearer compare) remains a separate `qsl-server` lane.** Successor: the **vault-format lane**, now owning C-1b, the target-width decision, the `identity_pin_matches_seen` product ruling, the C-4 raw-bytes form and the newline stripper. - 2026-07-23: NA-0669 CLOSEOUT (D-1296, after PR #1630 merged as `ccc6c008` over head `3d32a1c0`) — **THE THREE 2026-07-22 AUDIT CLIENT SECURITY FIXES SHIPPED, AND THIS LANE'S GREEN IS EVIDENCE.** In plain terms: **C-1a** takes the verification code from **44 to 64 bits** by stripping the constant `QSCFP-` prefix before truncating; **C-6** adds the parent-directory fsync that `vault_init_core` already had to `write_vault_atomic`, closing a crash window that could lose the whole store while the tmp file's contents were durable; **C-4** rejects non-UTF-8 passphrase files instead of silently discarding ~44% of their entropy (H=4.500 bits of 8 per random byte). **⚠ THE CONTRAST IS EXPLICIT: unlike the four docs-only lanes before it (NA-0664/0666/0667/0668), whose skipped suites made their greens empty, this lane changed `src/`, classified `runtime_critical`, and BOTH FULL SUITES + THE FORMAL PROOFS RAN** (34 checks green on #1630, the 2 skips being the known NA-0633 PR-skips). **AND THE SUITE WAS DEMONSTRATED TO REJECT A REAL DEFECT BEFORE IT WAS TRUSTED TO ACCEPT:** the first full `cargo test -p qsc` went RED on a **third copy of the formatter the census missed** (shadow #2, `verification_code_from_fingerprint` at `identity_foundation_contract_na0217d.rs:133`, named without the `format_` prefix so symbol-name sweeps missed it — only a body sweep for the Crockford constant finds all three), fixed under operator ruling (A) with the identical lockstep prefix-strip (assertion unchanged), re-run clean (434 passed, 0 failed, cargo exit 0). Root-cause lesson, recorded not filed: **hunt duplicate implementations by a distinctive body fragment, not the symbol name** — same class as NA-0668's costume set. No fourth copy (qsl-desktop body-swept clean, calls the qsc symbol, pins no format literal). **OWED, RIDING THE VAULT WORK (operator §9.7) AND NONE A VAULT-FORMAT CHANGE — correcting the D-1295 "vault-format lane" phrasing:** the C-1 target-width decision (20/24), the C-1b checksum transposition weakness, C-4's raw-bytes form, and the trailing-newline stripper; `identity_pin_matches_seen:562` stays untouched per F1(a), to be ruled with the width decision. C-2 remains a separate `qsl-server` lane. Nothing new filed in `IMPROVEMENT_LEDGER.md`. Queue closes at `READY=NONE | HIGHEST_NA=0669 | HIGHEST_D=1296`. +- 2026-07-23: NA-0670 (D606, C-2 constant-time bearer-token comparison in the relay; D-0014 qsl-server implementation + D-1297 spine closeout) — **THE LAST UNFIXED HIGH OF THE 2026-07-22 INDEPENDENT AUDIT SHIPPED, AND ITS GREEN IS EVIDENCE.** Executed per QSL-DIR-2026-07-23-606 (D606, approved 2026-07-23, sha256 `ef3bd063f936564935b86e806e459e2bc93be6a4b52fdba685611e43e8a177ca`, 284 lines). **ONE FUNCTION:** qsl-server `auth_ok`'s short-circuiting `provided == token` (`str::eq`, a remote timing oracle on the network-exposed shared bearer token) became `ct_eq_secret`, which SHA-256s both sides to a fixed 32-byte digest and folds them with the client's `hs_ct_eq_32` XOR-accumulate shape (ENG-0003) over all 32 bytes with no data-dependent early return — the same idiom the file already uses for its OTHER secret in `route_key_for`; hashing normalises both inputs to 32 bytes and closes the length leak for free. **NO dependency change** (`sha2` already direct; `subtle` deliberately NOT added; `Cargo.toml`/`Cargo.lock` diff empty), **NO wire/API/protocol/schema/env change**; the two non-secret `Authorization`-header guards kept; the four gated handlers still reject-before-mutation. Added the same-length wrong-token test (`"topsecreX"` vs `"topsecret"`, both 9 bytes) — the only behavioural test exercising the fold, since the pre-existing wrong-token test differs in LENGTH and passed against the buggy `==`. **⚠ THE qsl-server IMPLEMENTATION PR CHANGED `src/lib.rs`, so the required `rust` check RAN and its GREEN on `82df531` (the EXACT reviewed commit; run head == PR head == local HEAD, 1m36s) IS EVIDENCE for the behavioural half** — the inverse of the docs_only promotion/closeout greens; the constant-time property itself is STRUCTURAL, read-verified, NOT measured (no timing proof run or claimed). Phase 0 re-verified every §0/§1 anchor at the seating base `b4f86a3c` (`provided == token` at `src/lib.rs:507`; the `==` body-sweep confirming `:507` the SOLE secret comparison — no second fix owed). qsl-server implementation **PR #64 merged as `5235c2bf`** over base `b4f86a3c` (records satellite decision D-0014 + a TRACEABILITY row). **FOUR LEDGER FILINGS, NONE FIXED:** ENG-0063 (precompute the token digest at `AppState` construction — refinement), ENG-0064 (the two-repo seat leaked qsl-protocol's `CARGO_TARGET_DIR` into the qsl-server seat — the first real exercise of the NA-0667 two-repo seat path, defeating per-repo cache separation; worked around with an explicit pin, not fixed), ENG-0065 (a pre-existing flaky log-capture test, `tests::logs_do_not_contain_raw_channel`, surfaced not caused by the required new test — 0/20 pristine vs 8/25 at 6 cores, but 0/30 at `RUST_TEST_THREADS=2` and `=4` so CI's 2-vCPU runner is reliably green), ENG-0066 (qsl-server `TRACEABILITY.md` lapsed for three satellite lanes). Commit identity GH007 noreply (`238594419+Tebbens4832@users.noreply.github.com`) on both lane commits, author and committer, trailers empty, verified on the object. Evidence: `docs/governance/evidence/NA-0670_as_built.md`; `tests/NA-0670_qsl_server_c2_constant_time_bearer_testplan.md`. The 2026-07-22 audit's **C-2 is CLOSED** — no audit HIGHs remain. Queue closes at `READY=NONE | HIGHEST_NA=0670 | HIGHEST_D=1297`. diff --git a/docs/governance/evidence/NA-0670_as_built.md b/docs/governance/evidence/NA-0670_as_built.md new file mode 100644 index 00000000..17351f0e --- /dev/null +++ b/docs/governance/evidence/NA-0670_as_built.md @@ -0,0 +1,138 @@ +# NA-0670 — as built + +**Lane:** NA-0670 · **Directive:** QSL-DIR-2026-07-23-606 (D606, approved 2026-07-23, sha256 `ef3bd063f936564935b86e806e459e2bc93be6a4b52fdba685611e43e8a177ca`, 284 lines) · **Decisions:** D-1297 (spine closeout) · D-0014 (qsl-server satellite) · **Finding:** 2026-07-22 independent audit **C-2** (the last unfixed HIGH, and the only one outside the governance spine). + +**Result class:** `QSL_SERVER_C2_CONSTANT_TIME_BEARER_PASS`. + +**Shape:** ONE qsl-server implementation PR (**#64**, merged `5235c2bf` over base `b4f86a3c`) — `src/lib.rs` + `DECISIONS.md` (D-0014) + `TRACEABILITY.md`, merge-commit only, single required `rust` check — then this SEPARATE spine governance closeout (D-1297). + +--- + +## ⚠ 1. THE CI GREEN ON THE IMPLEMENTATION PR *IS* EVIDENCE — AND THE DISTINCTION IS THE POINT + +The qsl-server implementation PR changed `src/lib.rs`, so `classify_ci_scope` returns runtime/non-docs and the required **`rust`** check RAN and could have gone RED. It went green on head `82df531` (`success`, 1m36s) — **run head SHA == PR head == the exact reviewed commit**, not a stale run. That green is **real evidence for the BEHAVIOURAL half**: auth still accepts the right token and rejects wrong ones, including the same-length case, under CI. + +This is the **inverse** of: +- the qsl-server **queue-promotion** PR and this **spine closeout** PR — both `docs_only`, so their greens prove nothing about the code; and +- the four docs-only lanes NA-0664/0666/0667/0668, whose skipped suites made their greens empty. + +### 1.1 — the structural claim is NOT measured, and is not claimed to be + +The constant-time property itself — **fixed work over the full 32-byte digest, no data-dependent early return** — is a claim about **structure**, verified by **reading the code**. It is **not** a measured claim. No microbenchmark was run; a microbenchmark on a loaded build host would not reliably show the difference, and asserting it did would be the near-miss pattern this project records. **§3.4 held: no timing measurement was run or claimed.** Holding that line here is the near-miss rule working before there is a near-miss. + +--- + +## 2. What landed (qsl-server `src/lib.rs`, one function + one helper + one test) + +`auth_ok`'s final `provided == token` (`str::eq`, which short-circuits on the first differing byte) became `ct_eq_secret(provided, token)`. A new private helper was added, and one behavioural test. `DECISIONS.md` gained D-0014; `TRACEABILITY.md` gained the NA-0670 row. **`Cargo.toml`/`Cargo.lock` diff is empty.** + +The three-caller reject-before-mutation structure is undisturbed: `auth_ok` remains the **first statement** of every gated handler (`server_info`, `push`, `pull`, `pull/ack`), so the two `_no_mutation` tests still assert that rejects precede any queue mutation. + +--- + +## 3. The fix — SHA-256 both sides, then the in-house XOR-accumulate fold + +The call site (with the recorded maintainer note on why NOT `subtle`): + +```rust + // Constant-time credential check. `str::eq` short-circuits on the first + // differing byte -> a remote timing oracle on a network-exposed bearer + // token (cf. the client's `hs_ct_eq_32`, ENG-0003, and this file's own + // `route_key_for`). Reducing both sides to a fixed 32-byte digest first + // also removes length dependence: the fold does identical work for every + // input. NB: `subtle` is present in Cargo.lock only via rustls under the + // reqwest DEV-dependency, so it is NOT in the production graph -- do not + // reach for it here; `sha2` is already a direct dependency of this crate. + ct_eq_secret(provided, token) +``` + +The helper: + +```rust +/// Constant-time equality of two secrets, folded over a fixed 32-byte SHA-256 +/// digest so the comparison leaks neither content nor length. Same XOR-accumulate +/// shape as the client handshake's `hs_ct_eq_32` (ENG-0003). +fn ct_eq_secret(a: &str, b: &str) -> bool { + let da = Sha256::digest(a.as_bytes()); + let db = Sha256::digest(b.as_bytes()); + let mut diff = 0u8; + for i in 0..32 { + diff |= da[i] ^ db[i]; + } + diff == 0 +} +``` + +- The loop iterates **all 32 bytes unconditionally**, ORs the per-byte XOR into `diff`, and only after the full loop returns `diff == 0`. **No early return; no data-dependent branch.** +- The idiom is the qsc client's `hs_ct_eq_32` (**ENG-0003**), and the relay's own `route_key_for` (`src/lib.rs:486`) already hashes the OTHER secret (the route token). So **the relay now treats both of its secrets the same way** — the strongest form of the fix (D606 §1b): not "a constant-time compare was added" but "the relay treats both secrets alike." + +--- + +## 4. Why the in-house fold and NOT `subtle`, and NOT HMAC (recorded so the absences are decisions) + +- **`subtle::ConstantTimeEq`** — declined (§2b/§2c). `subtle 2.6.1` is in `Cargo.lock` **only** via rustls under the reqwest DEV-dependency; a direct `[dependencies]` use would newly pull it into the shipped relay. Decisively, though: `ConstantTimeEq::ct_eq` is defined for **equal-length** slices, so on the raw `&[u8]` it would force a **length-visible branch** for unequal lengths — it would NOT close the length leak. Hashing-first closes it for free. +- **HMAC with a random per-process key** — declined (§2d). HMAC's random key defends against **precomputation**; here the attacker already holds the candidate token, and if they have one to precompute against they can simply **send** it. Plain SHA-256 delivers the property actually needed — comparison time independent of matching-prefix length — with no added machinery. + +--- + +## 5. The length leak — CLOSED (a property of hashing first, not extra work) + +Hashing normalises both inputs to 32 bytes, so the fold is length-independent for free. A bare `ct_eq` on the raw bytes would not close it (§4). **Residual, honestly:** `Sha256::digest(token)` runs in time proportional to the token's block count — a **constant per-deployment offset** on every request, not a per-guess oracle and not content-revealing. The refinement that erases even that (precompute at construction) is deferred and **filed as ENG-0063**, not folded in. + +--- + +## 6. The one added test, and its precise limit + +`auth_enabled_wrong_token_same_length_401_no_mutation`: configured token `"topsecret"`, provided `"topsecreX"` — **both 9 bytes**. Asserts push → 401 `ERR_UNAUTHORIZED`, then a correct-token pull → 204 (queue unmutated). + +**Why it is the sharp one:** the pre-existing `auth_enabled_wrong_token_401_no_mutation` uses `"wrong"` (5 bytes) vs `"topsecret"` (9 bytes) — different lengths — so the old `==` rejected on **length** before comparing a single byte, and **that test passes against the buggy code**. The same-length case is the only behavioural test that exercises the fold. **Its limit, stated in-comment:** it proves the fold returns the right ANSWER (reject + no mutation), not that it runs in constant TIME. + +--- + +## 7. Acceptance (D606 §3) + +1. **Structural constant-time** — CONFIRMED by reading: fixed work over the full 32-byte digest, no data-dependent early return; the only early returns left are on non-secret header presence/format. +2. **Existing behaviour preserved under real CI** — the required `rust` check is GREEN on `82df531`. The four required tests pass unchanged (`auth_enabled_correct_token_allows_roundtrip`, `auth_enabled_wrong_token_401_no_mutation`, `auth_enabled_missing_token_401_no_mutation`, `auth_disabled_allows_push_pull`) plus the new same-length test. The two `_no_mutation` tests ARE the "rejects precede any queue mutation" acceptance. +3. **The one added test** — added, passes, limit recorded (§6). +4. **No timing proof required or claimed** — none run, none asserted (§1.1). +5. **Scope held** — one function; no file outside D606 §4; the only things filed are the closeout ENG lines (§10). + +--- + +## 8. CI + local gate + +- **CI (required `rust`):** `success`, 1m36s, head `82df531` (PR #64). `cargo test -q` on GitHub's 2-vCPU Linux runner. +- **Local gate** (`CARGO_TARGET_DIR=/srv/qbuild/cache/targets/qsl-server`, set explicitly per ENG-0064): `cargo fmt --check` clean; `cargo test` **14/14**; `cargo clippy --all-targets -- -D warnings` exit 0; `git diff Cargo.toml Cargo.lock` empty. + +--- + +## 9. Phase 0 — anchors re-verified at the seating base `b4f86a3c` + +| Anchor | D606 | Verified | +|---|---|---| +| `provided == token` | `src/lib.rs:507` | `:507` exactly | +| `use sha2::{Digest, Sha256};` | already imported | `:10` | +| `route_key_for` (hashes the route token) | `:486` | `:486`, `Sha256::digest` | +| `sha2` direct dep / `subtle` absent from `Cargo.toml` | yes / yes | `sha2 = "0.10"` at `:13`; `subtle` absent | +| DECISIONS top D-0013, D-0014 absent | yes | confirmed | +| `==` body-sweep: `:507` sole secret comparison | §1c | `lib.rs:47,55,62,192,672` + `store.rs:35,42,409,474` all numeric zero/threshold — `:507` is the only secret compare; **no second fix owed** | + +--- + +## 10. The four ledger filings (none fixed — recording, not a second fix) + +- **ENG-0063** — precompute `Sha256::digest(token)` at `AppState` construction (removes the secret from the per-request path; erases the block-count offset). Refinement, rides `new_with_auth*`. (D606 §5.) +- **ENG-0064** — the two-repo seat leaked `qsl-protocol`'s `CARGO_TARGET_DIR` into the `qsl-server` seat (classified inherited value as "explicit" and preserved it), so `qsl-server` built into `/srv/qbuild/cache/targets/qsl-PROTOCOL/…` and its registered per-repo cache went unused. First real exercise of the NA-0667 two-repo seat path. Worked around with an explicit per-build `CARGO_TARGET_DIR` pin; `qwork` NOT touched. +- **ENG-0065** — `tests::logs_do_not_contain_raw_channel` flakes under core contention (reads its buffer without synchronising on the server's on-response log). Measured: pristine 0/20, with-change 8/25 on 6 cores (always that test), 0/30 at 2 & 4 threads → CI reliably green. Surfaced not caused by the required new test. The "always passes where merges are decided" class. +- **ENG-0066** — `qsl-server` `TRACEABILITY.md` ends at NA-0012; D-0011/0012/0013 have no rows. Only the NA-0670 row was added; the three-row back-fill is filed, not done. + +--- + +## 11. What this lane did NOT do + +- Did **not** add any dependency, in particular **not `subtle`**; `Cargo.toml`/`Cargo.lock` byte-unchanged. +- Did **not** touch `route_key_for` or the route-token path (named as evidence, not a target), `src/store.rs`, `src/main.rs`, any route/wire/API/schema/env, any `.github/**`, or the `new_with_auth*` constructor chain. +- Did **not** fix the four filed items — they are refinements/defects for their own lanes. +- Did **not** run or claim any timing measurement. +- Did **not** back-fill the three missing qsl-server TRACEABILITY rows (ENG-0066) — out of scope. +- The executor did **not** self-promote; stopped at the open implementation PR, then (after the operator merged it) at this open closeout PR. diff --git a/docs/ops/IMPROVEMENT_LEDGER.md b/docs/ops/IMPROVEMENT_LEDGER.md index 1d6609aa..033415ce 100644 --- a/docs/ops/IMPROVEMENT_LEDGER.md +++ b/docs/ops/IMPROVEMENT_LEDGER.md @@ -2119,3 +2119,43 @@ Title; Problem; Recommended change; Status; Originating/last lane; Last-updated. - Sequencing: independent. **Pairs with WF-0041** as above; both are "detection depends on memory" and could be decided in one pass. - **⚠ PAIR — ONE LANE RETIRES BOTH: WF-0042 + WF-0026.** Same file (`scripts/ci/qsl_evidence_helper.py`), same class: **a parser accepting a form its sibling rejects.** `parse_queue()` takes `NA-\d+[A-Z]?`; `decision_ids()` takes no suffix at all. **ONE DIFF FIXES BOTH.** ⚠ **And both nearly bit in the same week:** a `D-1294-A` decision entry was almost created on 2026-07-23 and was caught only by running the helper, while `NA-0216AA` (`NEXT_ACTIONS.md:9875`) is **invisible to the parser right now.** **This pairing line is the shared handle — recorded deliberately INSTEAD of a taxonomy entry, because the answer to several entries sharing a theme is to fix some so the set shrinks, not to add an artifact describing the set.** - Status: open — filed 2026-07-23 **at operator instruction**, as a docs-only correction against a `READY=NONE` queue. **Nothing in `scripts/ci/qsl_evidence_helper.py` was changed.** + +### ENG-0063 — qsl-server `auth_ok` re-hashes the bearer token on every request, leaving a constant per-deployment timing offset the fix does not need to carry +- Severity: P3 (defence-in-depth refinement; **NO runtime, protocol, or security defect** — the shipped fix is correct and constant-time in the per-guess sense; this only removes a constant offset) +- Exact surfaces: `qsl-server` `src/lib.rs` — `ct_eq_secret` (called from `auth_ok`) computes `Sha256::digest(token)` on the configured secret **per request**; the token enters `AppState` through the `new_with_auth*` constructor chain (`relay_token: Option`). +- Description: NA-0670 (D-1297, C-2) made the comparison constant-time by digesting both sides to 32 bytes and folding. The digest of the *provided* value must be per-request, but the digest of the *configured* token is invariant — re-computing it each request adds `Sha256::digest(token)`, whose cost is proportional to the token's 64-byte block count, to every gated request. Precomputing it once at `AppState` construction and storing the `[u8; 32]` would remove the secret from the per-request path entirely and **erase the constant block-count offset** the current form leaves (D606 §2c residual). +- Consequence: the offset is **constant across requests** (not a per-guess oracle) and **not content-revealing** (it leaks at most the token's block count), so this is a refinement, not a defect. Left unfiled it would simply never be scheduled — the ledger's own most-repeated lesson. +- Recommended change (minimal, not a redesign): store `Sha256::digest(relay_token)` as an `Option<[u8; 32]>` on `AppState` at construction; `ct_eq_secret` then digests only the provided value and folds against the stored digest. Small and cheap for whoever is next in the `new_with_auth*` constructor chain. +- Proof gap: nothing asserts the configured-token digest is computed off the per-request path. +- Sequencing: independent; rides the `new_with_auth*` constructor chain, which is why it was correctly **out** of a one-function lane. Recommended directive shape: implementation-only. +- Status: open — filed 2026-07-23 by NA-0670 (D-1297), per D606 §5. **Not a defect, a refinement; `new_with_auth*` byte-unchanged by NA-0670.** + +### ENG-0064 — the two-repo seat leaks the first repo's `CARGO_TARGET_DIR` into the second, so the satellite builds into the spine's target dir and its own registered per-repo cache goes unused +- Severity: P2 (build/CI-cache correctness; **no runtime, protocol, or security impact** — but it defeats the per-repo cache separation registration established, and an observation living only in a relay evaporates) +- Exact surfaces: the two-repo seat path (`qwork qsl-protocol qsl-server`, the NA-0667 cross-repo convention) and its `CARGO_TARGET_DIR` classification in `env_qbuild.sh` (the same dispatch touched by WF-0035). At the NA-0670 seat, the qsl-server seat came up with `cargo_target_mode=explicit · cargo_target_source=preexisting-env · cargo_target_dir=/srv/qbuild/cache/targets/qsl-PROTOCOL/… · explicit_target_preserved=yes`. +- Description: the `qsl-protocol` seat set `CARGO_TARGET_DIR` moments earlier in the same invocation; the subsequent `qsl-server` seat classified that **inherited** value as "explicit"/`preexisting-env` and **preserved it**, so `qsl-server` was pointed at the SPINE's target dir. `/srv/qbuild/cache/targets/qsl-server/` — which NA-0667 (D-1293) created and `check_repo_registration.sh` verifies — went **unused**. This is the **first real exercise of the two-repo seat path NA-0667 enabled**. +- Consequence: cross-repo cache pollution — the satellite's builds land in the spine's tree, defeating the per-repo cache separation registration was meant to establish; the registered per-repo cache is provisioned and verified but never written. **NA-0670 worked around it explicitly** by setting `CARGO_TARGET_DIR=/srv/qbuild/cache/targets/qsl-server` for every local qsl-server build in the lane and **saying so** rather than working around it silently. +- Recommended change: the second-and-later repo in a multi-repo seat should **derive `CARGO_TARGET_DIR` per-repo from the registration** rather than inherit an ambient value and classify it as "explicit"; equivalently, an inherited value should not be treated as an explicit per-repo choice when a registered per-repo target exists. +- Proof gap: nothing asserts that each repo in a multi-repo seat is pointed at ITS OWN registered target dir; the seat reports the value it chose but not whether that value is the per-repo one. +- Sequencing: independent; a `qbuild` tooling lane (the fix is outside every product repository, the NA-0667/NA-0668 shape). Recommended directive shape: implementation-only, in `/srv/qbuild/tools/`. +- Status: open — filed 2026-07-23 by NA-0670 (D-1297). **Not a defect in NA-0670's own work, and `qwork` was NOT touched by this lane** — the workaround was an explicit per-build `CARGO_TARGET_DIR` pin. + +### ENG-0065 — a qsl-server log-capture test reads its buffer without synchronising on the server's on-response log, so it flakes under core contention — but always passes on the 2-vCPU runner that decides merges, so it generates no pressure to fix +- Severity: P2 (test-synchronisation correctness; **no runtime, protocol, or security impact** — but the class is *a defect that always passes in the environment that decides merges*, the same shape as NA-0664's "a recovery convention that always works removes pressure to fix") +- Exact surfaces: `qsl-server` `src/lib.rs` — `tests::logs_do_not_contain_raw_channel`. It calls `handle.abort()` and then **immediately** reads its capture buffer and asserts `logged.contains("channel_id=")`, without synchronising on the server task having emitted the on-response `channel_id=` log line. +- Description: **the measurement is the argument.** On a 6-core build box: **pristine base `b4f86a3c` (13 tests) 0/20 failures; with NA-0670's required same-length test (14 tests) 8/25 failures, and ALWAYS this same unrelated log-capture test, never an auth test.** Single-threaded: clean. Isolated: clean. `RUST_TEST_THREADS=2` → **0/30**; `RUST_TEST_THREADS=4` → **0/30**. Mechanism (**inference**): `#[tokio::test]` runs a current-thread runtime, cooperatively scheduled, so under high core-count contention the server task lags and its on-response log lands **after** the immediate buffer read; the added 14th server-spawning test raised concurrency past the threshold **on a 6-core box only**. GitHub's standard Linux runner is **2 vCPU** and CI runs `cargo test -q`, so the required `rust` check is reliably green (0/30 at 2 threads) — which is exactly why the defect **generates no pressure to fix**. +- Consequence: the test can produce a false failure on higher-core CI or local dev, and — because it is invisible on the 2-vCPU runner — it will sit latent indefinitely. NA-0670's required test SURFACED it (raised the failure probability), it did **not** cause it: the defect is entirely in the test's own missing synchronisation. +- Recommended change: the test should **wait for the server to have flushed the expected log** before asserting — poll/await for the `channel_id=` line (bounded), or flush the subscriber, rather than reading the buffer immediately after `abort()`. +- Proof gap: nothing awaits the server's on-response log before the buffer assertion; nothing runs the suite at high `--test-threads` where the race is visible. +- Sequencing: independent; a `qsl-server` test-hardening micro-lane. Recommended directive shape: implementation-only (test-only). +- Status: open — filed 2026-07-23 by NA-0670 (D-1297). **NOT fixed — NA-0670 touched only `auth_ok`, the `ct_eq_secret` helper, and the one same-length test.** + +### ENG-0066 — qsl-server `TRACEABILITY.md` stopped tracking at NA-0012, so three accepted satellite decisions (D-0011/D-0012/D-0013) have no traceability row +- Severity: P3 (traceability completeness; **no runtime, protocol, or security impact** — but "documented but not asserted" back-fills only get scheduled if written down, the WF-0041/WF-0042 class one repo over) +- Exact surfaces: `qsl-server` `TRACEABILITY.md` — ends at the `NA-0012` rows; `DECISIONS.md` carries `D-0011` (NA-0642, durability), `D-0012` (NA-0652, `server-info`), `D-0013` (NA-0655, community-health) with **no** corresponding TRACEABILITY rows. +- Description: the "keep TRACEABILITY in sync" discipline lapsed across three satellite lanes. NA-0670 **correctly added only its own** `NA-0670`/`D-0014` row (adding it was in scope; back-filling the three missing rows was not). +- Consequence: the satellite's traceability is incomplete for three decisions; anyone reconstructing the qsl-server decision→PR trail from `TRACEABILITY.md` alone will miss durability, `server-info`, and community-health. Filing it makes the back-fill **schedulable rather than remembered**. +- Recommended change: a docs-only `qsl-server` micro-lane back-filling the three rows from their merge SHAs; optionally an assertion that every `Accepted` qsl-server decision has a TRACEABILITY row. +- Proof gap: nothing asserts that every accepted qsl-server decision id appears in `TRACEABILITY.md`. +- Sequencing: independent; a `qsl-server` docs micro-lane. Recommended directive shape: docs-evidence-only. +- Status: open — filed 2026-07-23 by NA-0670 (D-1297). **The three missing rows were NOT back-filled by this lane** (out of scope); only the `NA-0670` row was added. diff --git a/docs/ops/ROLLING_OPERATIONS_JOURNAL.md b/docs/ops/ROLLING_OPERATIONS_JOURNAL.md index 0e3f8804..7da4ff93 100644 --- a/docs/ops/ROLLING_OPERATIONS_JOURNAL.md +++ b/docs/ops/ROLLING_OPERATIONS_JOURNAL.md @@ -44647,3 +44647,21 @@ Recorded at operator instruction as a **general practice, not a one-off** (2026- **⚠ CORRECTION OF THE D-1295 PHRASING (operator §9.7).** D-1295 called the successor "the vault-format lane" and said it "owns" the deferred items; that is imprecise. **The four owed items ride the vault WORK, and NONE of them is a vault-FORMAT change:** (1) the C-1 verification-code **target-width decision** (20/24, to reach beyond 64 bits), (2) the **C-1b checksum transposition weakness**, (3) **C-4's raw-bytes form** (the uncontained ~10-site passphrase-pipeline change deferred here), (4) the **trailing-newline stripper** (a `while` loop self-consistent at init and unlock, so any fix must move both together). Separately, **`identity_pin_matches_seen:562` stays untouched per F1(a)** and is to be ruled together with the width decision. **C-2** (constant-time bearer compare) remains a separate `qsl-server` lane. - Scope discipline held to the end: four files touched (three planned + `na0217d.rs` by ruling (A)), three lines of behavior plus two lockstep test updates, **nothing new filed in `IMPROVEMENT_LEDGER.md`** — observations went to the operator relay. Commit identity on both lane commits verified on the object as the GH007 noreply (`238594419+Tebbens4832@users.noreply.github.com`), trailers empty (WF-0029 per-commit override). Queue returns to `READY=NONE`; the operator promotes; the executor cannot self-promote. + +## 2026-07-23 — NA-0670 CLOSEOUT (D-1297, after qsl-server PR #64 merged as `5235c2bf` over base `b4f86a3c`) + +- Implementation (qsl-server satellite decision **D-0014**) merged as `5235c2bf` (PR #64) over base `b4f86a3c`. The required **`rust`** check went green on head `82df531` (`success`, 1m36s) — run head SHA == PR head == the exact reviewed commit. Queue closes at `READY=NONE | HIGHEST_NA=0670 | HIGHEST_D=1297`; helper `READY_COUNT 0` / exit 2 CORRECT at `READY=NONE` (`--allow-nonready-count` not passed), `DUPLICATE_COUNT 0`, `LATEST_DECISION_ENTRY D-1297`. + +**THE FIX, IN PLAIN TERMS FOR THE RECORD.** `qsl-server` `auth_ok`'s final `provided == token` used `str::eq`, which **short-circuits on the first differing byte** — a remote timing oracle on the one component deliberately exposed to the network, and the LAN/tailnet deployment posture is exactly the low-jitter regime where byte-at-a-time statistical amplification is practical. It is now `ct_eq_secret(provided, token)`, which reduces both sides to a fixed **32-byte SHA-256 digest** and folds them with an XOR-accumulate loop over all 32 bytes (`diff |= da[i] ^ db[i]`, then `diff == 0`) — the exact shape of the qsc client's `hs_ct_eq_32` (**ENG-0003**), and consistent with this file's own `route_key_for`, which already hashes the OTHER secret (the route token). **The relay now treats both of its secrets the same way** — the strongest form of the fix (D606 §1b). Hashing normalises both inputs to 32 bytes, so the fold does identical work for every input, **closing the length leak** the audit offered as optional (§2c). The two `return false` guards (missing / malformed `Authorization`) are kept — they branch on attacker-known request shape, not on the secret. + +**⚠ THIS LANE'S GREEN IS EVIDENCE — STATED SO THE CLOSEOUT DOES NOT INHERIT THE WRONG CAVEAT.** Unlike the docs_only queue-promotion PR (and the four docs-only lanes NA-0664/0666/0667/0668, whose skipped suites made their greens empty), the qsl-server **implementation** PR changed `src/lib.rs`, so `classify_ci_scope` returns runtime/non-docs and the required **`rust`** check RAN. Its green on `82df531` is **real evidence for the BEHAVIOURAL half** — auth still accepts the right token and rejects wrong ones, including the same-length case, under CI. **The constant-time property ITSELF is STRUCTURAL** — fixed work over the full 32-byte digest, no data-dependent early return — verified by READING, not by measurement. **No timing measurement was run or claimed (§3.4 held)** — holding the near-miss line before there is a near-miss. + +**THE ONE ADDED TEST, AND WHY IT IS THE SHARP ONE.** `auth_enabled_wrong_token_same_length_401_no_mutation` (`"topsecreX"` vs the configured `"topsecret"`, both 9 bytes) asserts 401 `ERR_UNAUTHORIZED` + queue unmutated. The pre-existing wrong-token test uses `"wrong"` (5 bytes), so the old `==` rejected on **length** before comparing a single byte and **passed against the buggy code** — the same-length case is the only behavioural test that exercises the fold. Its limit is stated in-comment: it proves the fold's ANSWER, not its timing. Phase 0 re-anchored every §0/§1 file:line at the seating base `b4f86a3c` (`provided == token` at `src/lib.rs:507`; `sha2` imported at `:10`; `route_key_for` at `:486`; the `==` body-sweep across `lib.rs` and `store.rs` confirming `:507` the **sole** secret comparison — so **no second fix is owed**, §1c). + +**NO DEPENDENCY, NO WIRE.** `sha2` is already a direct dependency and `use sha2::{Digest, Sha256}` already imported; `subtle` was **deliberately NOT added** — it is present in `Cargo.lock` only via rustls under the reqwest DEV-dependency, and a direct use would newly enter the production graph and (being defined for equal-length slices) would not close the length leak without a length-visible branch. The `Cargo.toml`/`Cargo.lock` diff is **empty**. No wire/API/protocol/schema/env change; the four gated handlers (`server-info`, `push`, `pull`, `pull/ack`) still reject-before-mutation because `auth_ok` remains their first statement. + +**⚠ UNLIKE NA-0669, THIS LANE FILED — FOUR LEDGER ENTRIES, NONE FIXED, AND THAT IS CORRECT.** Scope discipline bars **doing** a second piece of work inside a one-function lane; it does not bar **recording** real, decision-bearing findings at closeout, and an observation that lives only in a relay evaporates. **ENG-0063** — precompute `Sha256::digest(token)` once at `AppState` construction to remove the secret from the per-request path and erase the constant block-count offset re-hashing leaves; a refinement riding the `new_with_auth*` chain, correctly out of a one-function lane (D606 §5). **ENG-0064** — the two-repo seat leaked `qsl-protocol`'s `CARGO_TARGET_DIR` into the `qsl-server` seat, which classified the inherited value as "explicit" and preserved it, so `qsl-server` built into `/srv/qbuild/cache/targets/qsl-PROTOCOL/…` and its own registered per-repo cache (NA-0667/D-1293) went unused — **the first real exercise of the two-repo seat path NA-0667 enabled**, defeating the per-repo cache separation registration established; **worked around explicitly** in this lane by pinning `CARGO_TARGET_DIR=/srv/qbuild/cache/targets/qsl-server` for every local build and **saying so**, not silently. **ENG-0065** — a pre-existing flaky log-capture test (`tests::logs_do_not_contain_raw_channel`) **surfaced, not caused**, by the required new test: **0/20** on the pristine base, **8/25** with the change on a 6-core box, and **always that same unrelated test, never an auth test**; single-threaded and isolated clean; **`RUST_TEST_THREADS=2` → 0/30, `=4` → 0/30**, so CI's 2-vCPU runner is reliably green. Mechanism (inference): the test reads its capture buffer immediately after `handle.abort()` without synchronising on the server's on-response `channel_id=` log, and cooperatively-scheduled `#[tokio::test]` current-thread runtimes let the log land after the read under contention. **The class is the familiar one: a defect that always passes in the environment that decides merges (2 vCPU) and therefore generates no pressure to fix — the same shape as NA-0664's "a recovery convention that always works removes pressure to fix."** **ENG-0066** — `qsl-server` `TRACEABILITY.md` ends at `NA-0012` while `D-0011/D-0012/D-0013` (NA-0642/0652/0655) have no rows; the keep-in-sync discipline lapsed across three satellite lanes. Only the `NA-0670`/`D-0014` row was added; filing makes the back-fill schedulable rather than remembered — the WF-0041/WF-0042 "detection depends on memory" class, one repo over. + +**⚠ OBS-3 CLOSED AT PHASE 0.** D606 §0 recorded spine main as `cf9a5852` while the seated worktree was `76eb566a`; fetching `origin` first (not the mirror) showed **`76eb566a` IS live `origin/main`** — the gap was the promotion PR #1632 (`b647f50c` → `76eb566a`) merging **between** D606 drafting and seating, not mirror staleness. The closeout branch was cut off `76eb566a`. NA-0668's mirror-freshness lesson was applied by fetching first; it did not bite this time. + +- Scope discipline held: the qsl-server implementation touched exactly `src/lib.rs` + `DECISIONS.md` (D-0014) + `TRACEABILITY.md`; this spine closeout touches only governance records (`DECISIONS.md` D-1297, `TRACEABILITY.md`, this journal, `IMPROVEMENT_LEDGER.md` ENG-0063…0066, `NEXT_ACTIONS.md`, the as-built and testplan). `route_key_for` and the route-token path named as evidence, not a target, byte-unchanged. Commit identity on the qsl-server implementation commit and on this closeout is the GH007 noreply (`238594419+Tebbens4832@users.noreply.github.com`), author and committer, trailers empty, verified on the object (WF-0029 per-commit override). The 2026-07-22 audit's **C-2 is closed — no audit HIGHs remain.** Queue returns to `READY=NONE`; the operator promotes; the executor cannot self-promote. Evidence: `docs/governance/evidence/NA-0670_as_built.md`; `tests/NA-0670_qsl_server_c2_constant_time_bearer_testplan.md`. diff --git a/tests/NA-0670_qsl_server_c2_constant_time_bearer_testplan.md b/tests/NA-0670_qsl_server_c2_constant_time_bearer_testplan.md new file mode 100644 index 00000000..24187084 --- /dev/null +++ b/tests/NA-0670_qsl_server_c2_constant_time_bearer_testplan.md @@ -0,0 +1,99 @@ +# NA-0670 — constant-time bearer-token comparison (audit C-2): test plan and executed results + +**Lane:** NA-0670 · **Directive:** QSL-DIR-2026-07-23-606 (D606) · **Repo under test:** qsl-server (satellite) · **Implementation PR:** #64 (merged `5235c2bf` over base `b4f86a3c`). + +--- + +## ⚠ 0. WHAT CI CAN AND CANNOT PROVE HERE + +The qsl-server implementation PR changes `src/lib.rs`, so the required **`rust`** check RUNS (`cargo test -q`) and its green **is** evidence for the **behavioural** half — auth accepts the right token and rejects wrong ones, including the same-length case. **CI cannot prove the constant-TIME property**: that is a **structural** claim (fixed work over the full 32-byte digest, no data-dependent early return), verified by **reading** the code. **No timing measurement is run or claimed** (D606 §3.4). If one were ever run it would be indicative, not proof. + +--- + +## 1. Harness + +- Local: `cargo fmt --check`, `cargo test`, `cargo clippy --all-targets -- -D warnings`, run with `CARGO_TARGET_DIR=/srv/qbuild/cache/targets/qsl-server` set **explicitly** (per ENG-0064 — the two-repo seat otherwise leaks the spine's target dir). +- CI: `.github/workflows/ci.yml` job `rust` — `cargo fmt`, `cargo test -q`, `cargo clippy`. GitHub 2-vCPU Linux runner. + +--- + +## 2. The behavioural change under test + +`auth_ok`'s `provided == token` → `ct_eq_secret(provided, token)`: + +```rust +fn ct_eq_secret(a: &str, b: &str) -> bool { + let da = Sha256::digest(a.as_bytes()); + let db = Sha256::digest(b.as_bytes()); + let mut diff = 0u8; + for i in 0..32 { diff |= da[i] ^ db[i]; } + diff == 0 +} +``` + +Structural acceptance (read-verified): the loop runs all 32 bytes unconditionally; `diff` accumulates by `|=`; the sole return is `diff == 0` **after** the loop. No early return, no data-dependent branch. `Sha256::digest` processes the full input (the acknowledged constant per-deployment offset, filed ENG-0063). + +--- + +## 3. The added test — the only one that exercises the fold (§7.1) + +`auth_enabled_wrong_token_same_length_401_no_mutation`: configured `"topsecret"`, provided `"topsecreX"` (both 9 bytes) → assert push 401 `ERR_UNAUTHORIZED`, then correct-token pull → 204 (queue unmutated). + +**Why this is the sharp case, stated plainly:** the pre-existing `auth_enabled_wrong_token_401_no_mutation` uses `"wrong"` (5 bytes) vs `"topsecret"` (9 bytes). With the buggy `==`, `str::eq` compares length first and rejects **before comparing a single byte** — so that test **passes against the buggy code** and proves nothing about the fold. The same-length case forces the byte comparison. **Its limit: it proves the fold's ANSWER (reject + no mutation), not its timing.** + +--- + +## 4. Regression — existing behaviour preserved (§7.1) + +These pass **unchanged** (assertions not weakened): +- `auth_enabled_correct_token_allows_roundtrip` — right token accepted. +- `auth_enabled_wrong_token_401_no_mutation` — wrong token 401, queue unmutated. +- `auth_enabled_missing_token_401_no_mutation` — missing header 401, no mutation. +- `auth_disabled_allows_push_pull` — `relay_token = None` still open. + +The two `_no_mutation` tests ARE the "rejects precede any queue mutation" acceptance (D606 §3.2). + +--- + +## 5. Scope verification (§7.2) + +- `git diff Cargo.toml Cargo.lock` — **empty** (no dependency change; `subtle` NOT added). +- Changed files: exactly `src/lib.rs`, `DECISIONS.md`, `TRACEABILITY.md`. +- `route_key_for` / route-token path, `src/store.rs`, `src/main.rs`, `.github/**`, `new_with_auth*` — untouched. +- Phase 0: `provided == token` was at `src/lib.rs:507` at the seating base `b4f86a3c`; the `==` body-sweep confirmed `:507` the sole secret comparison (no second fix owed). + +--- + +## 6. ⚠ The pre-existing flaky test surfaced by this lane (ENG-0065) — measured, not hand-waved + +`tests::logs_do_not_contain_raw_channel` is **unrelated to `auth_ok`** (it checks that request logging redacts the raw channel). It flakes under high core-count contention because it reads its capture buffer immediately after `handle.abort()` without synchronising on the server task having emitted its on-response `channel_id=` log. + +| Condition | Failures | Note | +|---|---|---| +| pristine base `b4f86a3c` (13 tests), parallel | **0 / 20** | flake not reproduced without the change | +| with NA-0670's test (14 tests), parallel, 6 cores | **8 / 25** | always `logs_do_not_contain_raw_channel`, never an auth test | +| single-threaded (`--test-threads=1`) | **0** | deterministic | +| the log test in isolation | **0** | passes alone | +| `RUST_TEST_THREADS=2` (GitHub 2-vCPU runner) | **0 / 30** | CI-relevant parallelism | +| `RUST_TEST_THREADS=4` | **0 / 30** | | + +**Conclusion:** the added 14th server-spawning test raises parallel CPU load past a threshold **on a 6-core box only**, surfacing a latent race in a different test; CI runs at ≤ 2 vCPU, where the required `rust` check is **reliably green**. Surfaced, not caused. Out of this lane's scope to fix (touch only `auth_ok`/helper/the one test); filed as ENG-0065. + +--- + +## 7. Executed suite results + +### 7.1 Local `cargo test` (14 tests) +- Single-threaded: **14 passed, 0 failed** — including the new same-length test and the four required auth tests. +- `cargo fmt --check`: clean. `cargo clippy --all-targets -- -D warnings`: exit 0. Dependency diff: empty. + +### 7.2 CI required `rust` check +- PR #64, head `82df531`: **`success`, 1m36s** — run head SHA == PR head == the exact reviewed commit. This green is evidence for the behavioural half. + +--- + +## 8. What this plan does not cover + +- The constant-TIME property by measurement — deliberately not attempted; structural/read-verified only. +- Directory-durability or power-loss behaviour — not in scope (that was NA-0669's C-6). +- The ENG-0063 precompute refinement, the ENG-0064 seat fix, the ENG-0065 test-synchronisation fix, and the ENG-0066 TRACEABILITY back-fill — all filed, none done here.