214 lines
17 KiB
Markdown
214 lines
17 KiB
Markdown
# 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).
|