Files

214 lines
17 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Fable5 Audit Mitigation Plan
**Companion to:** [`audit.md`](audit.md:1)
**Date:** 2026-07-25
**Scope:** Remediate the findings from the Fable5 audit ([`audits/Fable5/audit.md`](audit.md:1)). This plan is scoped to the Fable5 findings only; it complements (does not supersede) the GPT-5.6 mitigation plan in [`audits/GPT5.6/audit_mitigation.md`](../GPT5.6/audit_mitigation.md:1), which addresses an overlapping but distinct set of earlier findings (G56-xx).
## Finding inventory
| ID | Severity | Title | Status |
|---|---|---|---|
| H-1 | High | Non-canonical JSON hashing makes the OTS digest fragile | Open |
| H-2 | High | NIP-09 deletion + relay churn attack on the genuine anchor | Open |
| M-1 | Medium | NIP vs implementation contradiction on unknown/missing algorithms | Open |
| M-2 | Medium | Falcon-512 not covered by `@noble/post-quantum` audit | Open |
| M-3 | Medium | Server-assisted OTS upgrade is a soft centralization point | Open |
| M-4 | Medium | Explorer-API trust for block headers (acknowledged) | Open |
| M-5 | Medium | Browser as the trust boundary (acknowledged, unavoidable for now) | Open |
| L-1 | Low | Documentation drift | Open |
| L-2 | Low | Pending-candidate fallback ranks by forgeable `created_at` | Open |
| L-3 | Low | `base64ToBytes` uses `atob` (lenient) | Open |
| L-4 | Low | Kind 9999 allocation is provisional | Open |
## Non-negotiable invariants (Fable5-specific)
These are the properties the remediation must establish. Each finding maps to at least one.
| Invariant | Required property | Findings covered |
|---|---|---|
| F-I1 Canonical digest | The OTS target digest is computed over a byte-exact, cross-implementation-reproducible serialization, not engine-dependent `JSON.stringify`. | H-1 |
| F-I2 Anchor availability | The genuine proof carrier remains discoverable and verifiable even if relays delete it or flood replacements. | H-2 |
| F-I3 Policy/version coherence | The mandatory-algorithm set is versioned and explicit; the NIP text and the verifier agree on how unknown and missing algorithms are treated. | M-1 |
| F-I4 Dependency transparency | Each PQ primitive's audit coverage and determinism guarantees are documented; the most provisional primitive is labeled as such. | M-2 |
| F-I5 Trust-mode transparency | Every verification result names its trust mode (Bitcoin-verified / API-checked / structural-only / pending-only) and the verifier re-checks any server-assisted upgrade before persisting. | M-3, M-4 |
| F-I6 Browser-boundary honesty | The UI and docs state plainly what the browser can and cannot guarantee; secrets are scoped and zeroized; out-of-band verification is the recommended continuity proof. | M-5 |
| F-I7 Doc/code agreement | Documentation matches the implemented defaults and constants. | L-1 |
| F-I8 No trust in forgeable clocks | Pending-only selections are never presented as trust decisions. | L-2 |
| F-I9 Strict input decoding | Base64/hex decoders reject malformed input uniformly across implementations. | L-3 |
| F-I10 Kind-allocation safety | The chosen event kind's replaceability semantics are documented as load-bearing for the append-only upgrade design. | L-4 |
## Phased remediation
### Phase F-0 — Documentation and claim hygiene (no code behavior change)
**Findings addressed:** L-1, L-4, parts of M-2, M-5.
These are cheap, low-risk, and protect the credibility the warning boxes have earned. Do them first.
1. **L-1 doc drift:**
- [`README.md`](../../README.md:746): change "Client generates a 12-word phrase" and the mermaid node at line 706 to "24-word (256-bit) BIP39 phrase" to match [`generateSeedPhrase()`](../../www/js/pq-crypto.mjs:82) default.
- [`pq-crypto.mjs:1618`](../../www/js/pq-crypto.mjs:1618): fix the comment listing the pending attestation tag as `83df830d1c9d4a51` — the correct constant at line 1688 is `83dfe30d2ef90c8e`. The code is right; the comment is wrong.
- Rename the deprecated alias [`buildKind11112Wrapper()`](../../www/js/pq-crypto.mjs:743) and the `kind11112Content`/`kind11112Tags` parameter names in [`verifyNIPQRContent()`](../../www/js/pq-crypto.mjs:828) to `proofCarrier`/`proofCarrierContent`/`proofCarrierTags`. Keep a thin deprecated alias for one release.
2. **L-4 kind allocation:** add a "Kind allocation" subsection to [`nip_proposal.md`](../../nip_proposal.md:171) stating that kind 9999's non-replaceable semantics (0–9999 regular-event range) are load-bearing for the append-only `upgrade_of` design, and that if maintainers allocate a different kind it MUST also be non-replaceable.
3. **M-2 Falcon transparency:** add a "Algorithm maturity" note to [`nip_proposal.md`](../../nip_proposal.md:273) and the README algorithm table marking Falcon-512 as the most provisional link (draft FIPS 206, float-FFT determinism risk, not in the audited subset of `@noble/post-quantum`).
4. **M-5 browser-boundary honesty:** the warning boxes already exist; add one explicit sentence to the README "Implementation Status" section stating that the committed bundle `www/pq-crypto.bundle.js` is a build artifact and that reproducible-build verification of the deployed bundle is on the roadmap.
**Acceptance:** `grep -rn "12-word" README.md why_what_how.md` returns nothing contradictory; the attestation-tag comment matches the constant; Falcon is labeled provisional in the NIP.
### Phase F-1 — Canonical OTS digest (the spec-breaking fix)
**Findings addressed:** H-1. This is the single most important change and must land before any second implementation or NIP submission.
**Design decision to make first:** choose the canonical serialization. Two viable options:
- **Option A (recommended, minimal):** hash the NIP-01 serialization array extended with `id` and `sig`. That is, the digest is `sha256(JSON.stringify([0, pubkey, created_at, kind, tags, content, id, sig]))` over the already-canonical NIP-01 array form. Key order is fixed by the array; number/string formatting is the only residual ambiguity, which is far smaller than object-key-order ambiguity and is already implicit in NIP-01 event-ID computation.
- **Option B (stronger, more work):** RFC 8785 JSON Canonicalization Scheme over the event object, or deterministic CBOR.
Recommend **Option A** because it reuses the existing NIP-01 canonicalization that every Nostr implementation already reproduces, minimizing the interop surface.
**Implementation steps:**
1. Add `canonicalEventDigest(event)` to [`pq-crypto.mjs`](../../www/js/pq-crypto.mjs:1) computing the Option-A digest.
2. Replace [`hashFullEvent()`](../../www/js/pq-crypto.mjs:686) usages:
- In [`buildProofCarrier()`](../../www/js/pq-crypto.mjs:715): the `sha256` tag becomes `canonicalEventDigest(kind1Event)`.
- In [`verifyNIPQRContent()`](../../www/js/pq-crypto.mjs:951) and [`selectCanonicalProofCarrier()`](../../www/js/pq-crypto.mjs:1182): verify the `sha256` tag against `canonicalEventDigest`.
- In [`timestampEvent()`](../../www/js/pq-crypto.mjs:1471): submit `canonicalEventDigest` to the calendars.
3. Update [`nip_proposal.md`](../../nip_proposal.md:207) "OpenTimestamps anchoring" section: replace "SHA-256 of the full signed kind 1 event JSON" with the normative canonical-serialization definition and a worked byte-level example.
4. Add a versioned `digest_version` tag (`['digest_version', '1']`) to the proof carrier so a future change is detectable. Verifiers accept version 1; unknown versions fail closed.
5. Keep `hashFullEvent()` as a deprecated internal alias for one release to avoid breaking already-published v0 events; mark them as legacy and verify them under a clearly-labeled legacy path.
**Required tests:**
- Known-answer: a fixed kind 1 event → expected canonical digest (pinned in the test file).
- Re-serialization invariance: `JSON.parse` then re-`canonicalEventDigest` yields the same digest as the original object.
- A relay-fetched event with reordered keys still verifies (the regression test for H-1).
- `digest_version` tag missing or unknown → `validForMigration: false`.
- Legacy v0 digest path is labeled and isolated.
**Acceptance:** a kind 1 event re-serialized with different key order produces the same canonical digest and still verifies; the NIP text contains a byte-level normative definition.
### Phase F-2 — Anchor availability and deletion resistance
**Findings addressed:** H-2.
1. **NIP normative language** (in [`nip_proposal.md`](../../nip_proposal.md:171)):
- Add a "Data availability" subsection stating:
- Verifiers MUST accept proof carriers supplied out-of-band (pasted JSON, imported archive file), not only from relays.
- Users SHOULD retain an offline copy of the signed kind 1 event JSON and the `.ots` proof.
- PQ-aware relays SHOULD ignore NIP-09 (kind 5) deletion requests targeting kind 9999 proof carriers.
- A proof carrier's validity does not depend on relay retention.
2. **Implementation:**
- In [`www/verify.html`](../../www/verify.html:1) and [`www/js/verify-app.mjs`](../../www/js/verify-app.mjs:1): ensure the existing paste-JSON verification path is presented as a first-class, equally-trusted input alongside relay queries. Add an "Import archive" entry point that accepts a `.json` or `.ots` file.
- In [`www/js/index-app.mjs`](../../www/js/index-app.mjs:1): after a successful publication, offer a "Download proof archive" button that saves `{kind1Event, proofCarrierEvent, otsProof, digestVersion, createdAt}` as a single JSON file the user can store offline and later feed to the verifier.
3. **Relay deletion-resistance:** document in the NIP that kind 9999 is in the regular (non-replaceable, non-deletable-by-policy) range and recommend PQ-aware relays treat kind 5 over kind 9999 as a no-op. This is a relay-policy recommendation, not a client code change.
**Required tests:**
- A proof carrier supplied via paste-JSON verifies identically to one fetched from a relay.
- An imported archive file with a valid kind 1 + proof carrier + `.ots` verifies end-to-end without any relay contact.
- A kind 5 deletion event referencing a kind 9999 does not affect the verifier's selection (the verifier never consults kind 5).
**Acceptance:** the verifier accepts out-of-band proofs as normatively equivalent to relay-fetched proofs; the NIP states this; the user can download and re-import a self-contained archive.
### Phase F-3 — Algorithm policy reconciliation
**Findings addressed:** M-1.
1. **Define a versioned policy object** in a new small module `www/js/nip-qr-policy.mjs`:
```js
export const POLICY_V1 = {
version: 1,
mandatorySignatureAlgorithms: ['ml-dsa-44', 'ml-dsa-65', 'slh-dsa-128s', 'falcon-512'],
kemAlgorithms: ['ml-kem-768'],
// algorithms not in either set are 'ignored' — neither credit nor failure
};
```
2. **Refactor [`verifyNIPQRContent()`](../../www/js/pq-crypto.mjs:828)** to take a `policy` argument (default `POLICY_V1`):
- Unknown algorithms are reported as `ignored` with `valid: null` (not `false`), and do NOT flip the legacy `valid` flag.
- Missing mandatory algorithms are policy failures.
- The NIP's revocation path (a new event omitting a broken scheme) is honored by allowing a future `POLICY_V2` that drops a scheme; the verifier rejects events that claim a policy version it doesn't know.
3. **Reconcile the NIP text** ([`nip_proposal.md`](../../nip_proposal.md:150) and the "Revocation" section): replace "verify whichever subset they support" with "verify against the versioned policy declared in the event; unknown algorithms are ignored; missing mandatory algorithms are failures." Add a `policy_version` tag to the kind 1 event.
4. **UI:** [`www/verify.html`](../../www/verify.html:1) must display the policy version and the per-algorithm verdict (`valid` / `invalid` / `ignored`).
**Required tests:**
- Event with all v1 mandatory algorithms present and valid → `policySufficient: true`.
- Event missing one mandatory algorithm → `policySufficient: false`.
- Event with an extra unknown algorithm → unknown reported as `ignored`, `policySufficient` still true if the rest pass.
- Event declaring `policy_version: 2` against a v1-only verifier → fail closed with "unsupported policy version".
- Event omitting a scheme (revocation scenario) under a future `POLICY_V2` that drops it → passes.
**Acceptance:** the NIP text and the verifier agree; the revocation path described in the NIP actually works under a versioned policy.
### Phase F-4 — Trust-mode transparency and upgrade-helper hardening
**Findings addressed:** M-3, M-4, L-2.
1. **M-3 upgrade helper:** in [`www/js/index-app.mjs`](../../www/js/index-app.mjs:1), after every [`upgradeOts()`](../../www/js/pq-crypto.mjs:1539) call, run [`verifyOtsProof()`](../../www/js/pq-crypto.mjs:1989) on the returned proof before persisting or republishing. Only persist if `verified: true` OR the proof is structurally valid and still pending. Never trust the helper's `confirmed`/`changed` fields alone. Document the helper as replaceable/self-hostable in the README.
2. **M-4 explorer trust:** the `trustMode` field already exists in [`verifyOtsProof()`](../../www/js/pq-crypto.mjs:1989). Surface it in the verify UI as a distinct, prominent label (`Bitcoin-verified` / `API-checked (2 providers)` / `API-checked (1 provider)` / `Structural only` / `Pending only`). Add a roadmap note pointing to the vendored `resources/javascript-opentimestamps/` for a future light-client path.
3. **L-2 pending fallback:** in [`selectCanonicalProofCarrier()`](../../www/js/pq-crypto.mjs:1296), keep the `created_at` ordering for pending candidates but mark the result with `canonicalTrustMode: 'pending-only'`. The verify UI must display a pending-only selection with a warning that it is NOT a trust decision and must not be treated as canonical until a Bitcoin attestation is verified.
**Required tests:**
- Upgrade helper returns `confirmed: true` but the proof fails `verifyOtsProof` → client does not persist/republish; error logged.
- Upgrade helper returns a still-pending proof → client persists as pending, does not label confirmed.
- `verifyOtsProof` result with `trustMode: 'single-explorer-checked'` → UI label reflects it.
- Pending-only canonical selection → result carries `canonicalTrustMode: 'pending-only'`; UI shows the warning.
**Acceptance:** no server-assisted claim becomes a published "confirmed" state without client-side cryptographic re-verification; every verification result displays its trust mode; pending-only selections are labeled as non-decisions.
### Phase F-5 — Input decoding strictness
**Findings addressed:** L-3.
1. Replace [`base64ToBytes()`](../../www/js/pq-crypto.mjs:360) with a strict implementation that rejects whitespace, missing padding, and non-base64 characters uniformly. Use `@scure/base` (already a dependency) `base64.decode` which is strict, or implement an explicit validator.
2. Audit all callers in [`pq-crypto.mjs`](../../www/js/pq-crypto.mjs:1) and the app files: ensure they treat a thrown `Error` from `base64ToBytes` as "malformed input → fail closed", which the verifier already does via `safeBase64ToBytes`.
**Required tests:**
- `base64ToBytes('')` throws.
- `base64ToBytes('abc')` (missing padding) throws.
- `base64ToBytes('aGVsbG8=\n')` (whitespace) throws.
- `base64ToBytes('aGVsbG8=')` returns `hello`.
- A proof carrier with a whitespace-padded base64 `ots` tag → `validForMigration: false` (not silently accepted).
**Acceptance:** malformed base64 is rejected uniformly; no lenient-decoding interop gap remains.
### Phase F-6 — Cross-implementation conformance vectors (the long-horizon fix)
**Findings addressed:** supports H-1, M-1, M-2; this is the "most valuable next step" from the audit conclusion.
1. Create `test/vectors/` with a pinned, versioned set of JSON files:
- `seed-to-pubkeys.v1.json`: a fixed mnemonic → all five derived pubkeys (hex). This pins the BIP32 truncation rule and the Falcon determinism question (M-2) concretely.
- `kind1-event.v1.json`: a fixed signed kind 1 event → its canonical digest (post-Phase-F-1) and the expected per-algorithm verification results.
- `proof-carrier.v1.json`: a fixed proof carrier → expected `verifyNIPQRContent` result including `policySufficient`, `identityBound`, `validForMigration`.
2. Add a test that loads these vectors and asserts the implementation reproduces them exactly.
3. Publish the vectors in the NIP repo so a second implementation (Rust/Python/Go) can be validated against them.
**Acceptance:** the test suite pins the canonical encoding, derivation, and verification results; a second implementation can be conformance-tested without reading the JS source.
## Sequencing and dependencies
```mermaid
flowchart LR
F0[F-0 Doc hygiene] --> F1[F-1 Canonical digest]
F1 --> F3[F-3 Policy reconciliation]
F1 --> F6[F-6 Conformance vectors]
F2[F-2 Anchor availability] --> F6
F3 --> F6
F4[F-4 Trust transparency] --> F6
F5[F-5 Strict decoding] --> F6
```
- F-0 first (cheap, protects credibility).
- F-1 next (spec-breaking; gates F-6 and any NIP submission).
- F-2, F-3, F-4, F-5 can proceed in parallel after F-1.
- F-6 last (depends on the canonical digest and policy decisions being frozen).
## Out of scope for this plan
- Light-client Bitcoin header verification (M-4 roadmap item; tracked separately).
- Hardware/native PQ signer (M-5 long-term fix; tracked in `research/amber_integration_plan.md`).
- PQ event authentication, rotation, revocation, encryption companion protocols (the project's own "What Remains Unsolved" section).
- The GPT-5.6 findings (G56-xx) already remediated or tracked in [`audits/GPT5.6/audit_mitigation.md`](../GPT5.6/audit_mitigation.md:1).