Files

12 KiB
Raw Permalink Blame History

Software Quality Remediation Plan

Date: 2026-07-17 Scope: Address remaining open audit findings (F-H1 through F-H4, F-M1F-M5, F-L1F-L8) — all software-quality issues, not cryptographic-correctness gaps. Prerequisite: F-C1 (reclassified), F-C2/F-C3 (fixed), F-C4 (resolved) are done.


Priority tiers

The findings fall into three tiers by impact:

  • Tier 1 (must-fix for production): F-H1 (tests), F-H2 (id checks), F-H3 (XSS), F-H4 (CSP/SRI), F-L8 (doc leftover)
  • Tier 2 (should-fix): F-L2 (24-word default), F-L4 (e-tag id check), F-L5 (Content-Type), F-L6 (relay close-as-success), F-M5 (Falcon UI warning)
  • Tier 3 (nice-to-have / design decisions): F-M1 (canonical attestation), F-M2 (block_height verification), F-M3 (xpub enforcement), F-M4 (truncation construction), F-L1 (zeroing), F-L3 (input validation), F-L7 (vendor nostr-login-lite)

Tier 1: Must-fix

F-H1: Add a test suite

What: Create a test file using Node's built-in test runner (no new dependencies). Update package.json test script.

File: test/pq-crypto.test.mjs (new)

Tests to include:

  1. BIP32→PQ-seed derivation known-answer: fixed mnemonic → expected BIP32 child private keys (verify determinism + correct path).
  2. PQ keygen determinism: same seed → same pubkeys for all 5 algorithms.
  3. PQ sign→verify round-trip: for each of ML-DSA-44, ML-DSA-65, SLH-DSA-128s, Falcon-512: sign a test message, verify passes; tamper message → verify fails; tamper signature → verify fails; wrong pubkey → verify fails.
  4. verifyNIPQRContent accept/reject:
    • Accept: genuine kind 1 event with valid PQ sigs + valid secp256k1 sig + matching e/sha256 tags.
    • Reject: tampered content, tampered PQ sig, wrong pubkey, mismatched e tag, mismatched sha256 tag.
  5. computeEventId / verifyNostrEvent: known Nostr test vector (from NIP-01 spec or a known event).
  6. OTS parser tests: isDetachedOtsFile true on valid prefix, false on random bytes; parseOtsFile on hello-world.txt.ots → expected target digest + 1 Bitcoin attestation at height 358391; parseOtsFile on incomplete.txt.ots → pending attestation.
  7. verifyOtsProof F-C3 binding: wrong expected digest → verified: false with "Target digest mismatch".

package.json change: "test": "node --test test/"

F-H2: verifyNostrEvent check event.id

What: Add an id check to verifyNostrEvent.

Change in pq-crypto.mjs:

export function verifyNostrEvent(event) {
    const serialized = JSON.stringify([0, event.pubkey, event.created_at, event.kind, event.tags, event.content]);
    const hash = sha256(new TextEncoder().encode(serialized));
    const computedId = bytesToHex(hash);
    if (event.id && event.id.toLowerCase() !== computedId) return false;  // NEW
    const sig = hexToBytes(event.sig);
    const pubkey = hexToBytes(event.pubkey);
    return schnorr.verify(sig, hash, pubkey);
}

Also fix F-L4 in verifyNIPQRContent: assert kind1Event.id (if present) equals computed id, or ignore it and rely on the computed id.

F-H3: Fix XSS — replace innerHTML with textContent for dynamic strings

What: Audit all innerHTML usages in index.html and verify.html. Replace with textContent where the content includes variables derived from relay/localStorage/user input. Keep innerHTML only for static HTML structure (no variables).

Key sites:

  • setStatus — used with error.message from relays. Change to textContent.
  • www/index.html:750otsNotice.innerHTML with pendingOts.eventId. Change to build the element with textContent for the variable part.
  • setStatus — same pattern. Change to textContent.
  • www/verify.html OTS info/badge — uses innerHTML with hashMatchNote etc. Change to textContent.

Approach: Create a helper function setText(element, text) that uses textContent, and replace all innerHTML calls that include dynamic data. For cases that need HTML structure (e.g., a link inside a status message), build the DOM with createElement + textContent for the variable parts.

F-H4: Add CSP and SRI

What:

  1. Add a CSP meta tag to both index.html and verify.html:
    <meta http-equiv="Content-Security-Policy" content="default-src 'self'; script-src 'self'; connect-src wss: https:; img-src 'self' data:; style-src 'self' 'unsafe-inline'; font-src 'self' data:;">
    
    (Note: style-src 'unsafe-inline' is needed because the pages have inline <style> blocks. Ideally these would be extracted to CSS files, but that's a larger refactor. The CSP still prevents script injection.)
  2. Add SRI integrity attributes to the bundle script tag. Compute the SHA-384 hash of www/pq-crypto.bundle.js and add:
    <script type="module" integrity="sha384-<hash>" src="./pq-crypto.bundle.js"></script>
    
    Wait — module scripts with integrity need the crossorigin attribute too. Since the bundle is same-origin, this should work. The /nostr-login-lite/ scripts are not in the repo so we can't compute their SRI — flag this as F-L7 (vendor them or document the deployment dependency).

Note: The CSP connect-src must allow wss: (relays), https: (OTS calendars, Bitcoin APIs, mempool.space, blockstream.info). The script-src 'self' blocks inline scripts — but both pages have inline <script type="module"> blocks. This is a problem. Options:

  • (a) Extract the inline scripts to separate .js files (cleaner, but larger change).
  • (b) Use a CSP nonce: generate a per-page-load nonce and add nonce="<random>" to the inline script tags + 'nonce-<random>' to the CSP. This is the standard approach for inline scripts.
  • (c) Use 'unsafe-inline' for script-src (defeats much of the CSP purpose for XSS, but still blocks external script injection).

Recommendation: Option (b) — CSP nonce for the inline module scripts. This gives real XSS protection (an injected script without the nonce won't run) while keeping the inline scripts. The nonce is generated server-side or per-page-load in the HTML. Since this is a static site, the nonce can be generated in a small inline bootstrap script that sets it on the CSP meta tag and the module script tag. Actually, for a static site, the cleanest approach is (a) — extract the inline scripts to .js files. This is more work but gives the strongest CSP.

Decision needed: extract inline scripts to files (stronger, more work) or use nonces (pragmatic)? I'll plan for (a) extraction since it's the right long-term answer, but note (b) as an alternative.

F-L8: Fix why_what_how.md Component 4 leftover

What: why_what_how.md:64 says "the new identity signs to endorse the PQ keys" — successor-model language.

Fix: Reword to: "The old identity signs to authorize the migration. (The raw-nsec migration is a design proposal, not yet implemented.)" Or add a "Design only" note.


Tier 2: Should-fix

F-L2: Default to 24-word mnemonics (or warn)

What: generateSeedPhrase defaults to 128 bits (12 words). Change to 256 bits (24 words), or add a UI warning that 12-word gives only ~64-bit PQ security.

Recommendation: Add a UI warning rather than changing the default — 12 words is more user-friendly for a demo, and the warning educates. Add a note near the seed display: "For maximum post-quantum security, consider a 24-word seed phrase (12 words provides ~64-bit PQ security under Grover's algorithm)."

F-L4: e tag validation check kind1Event.id

What: In verifyNIPQRContent, assert kind1Event.id (if present) equals the computed id.

Fix: Add after computing computedKind1Id:

if (kind1Event.id && kind1Event.id.toLowerCase() !== computedKind1Id.toLowerCase()) {
    eTagValid = false;
}

F-L5: Fix timestampEvent Content-Type

What: timestampEvent sends Content-Type: application/x-www-form-urlencoded with a binary body.

Fix: Change to Content-Type: application/octet-stream.

F-L6: Relay publishing — don't treat close as success

What: publishToRelays treats ws.onclose as success.

Fix: Treat close-without-OK as failure or "unknown". Only count as success on an explicit OK message.

F-M5: Falcon-512 draft-status UI warning

What: Add a note in the UI (near the Falcon key display) that Falcon-512 is based on FIPS 206 (draft) and may need re-issuing if the standard changes.


Tier 3: Nice-to-have / design decisions

F-M1: Canonical structured attestation

What: Sign a canonical JSON object (or its hash) instead of human-readable prose. This is a design change that affects the event format — it would require re-issuing any existing events. Recommend deferring unless there's a concrete need (e.g., the prose text needs to change).

F-M2: block_height verification

What: Verify the block height was current at signing time. This requires OTS verification (now implemented) to establish the signing time, then comparing to the claimed block height. Low priority — the block height is informational; the real anchor is the OTS proof.

F-M3: xpub enforcement

What: Add a code-level warning or check that the xpub at m/44'/1237'/0'/0' is never exported. Already documented — code enforcement is difficult since the xpub isn't published by this app. Recommend deferring.

F-M4: Truncation construction

What: Replace "take first N bytes" with HKDF-Expand on the concatenated children. Design change that would break existing key derivation — recommend deferring unless doing a major version bump.

F-L1: Zero secrets on sign-out

What: Clear pqKeys, pqSeed, pqMnemonic references on sign-out. The sign-out flow already clears some state; ensure all secret references are nulled. (JS can't guarantee memory zeroing, but clearing references helps.)

F-L3: Input validation for hexToBytes/base64ToBytes

What: Validate input format and length; throw clear errors on malformed input.

F-L7: Vendor /nostr-login-lite/ scripts

What: Copy the nostr-login-lite scripts into the repo (with SRI) or document the deployment dependency. This is a deployment/reproducibility issue, not a security issue per se.


Execution order

  1. F-H2 + F-L4: verifyNostrEvent id check + e tag id check (small, high-value, in pq-crypto.mjs)
  2. F-L5: Fix Content-Type (one-line change in pq-crypto.mjs)
  3. F-L6: Fix relay close-as-success (in index.html)
  4. F-H3: XSS — replace innerHTML with textContent (in both HTML files)
  5. F-H4: CSP + SRI — extract inline scripts to .js files, add CSP meta tag, add SRI to bundle (both HTML files)
  6. F-L8: Fix why_what_how.md leftover (one-line doc fix)
  7. F-L2: Add 12-word PQ security warning (UI text in index.html)
  8. F-M5: Add Falcon draft-status warning (UI text in index.html)
  9. F-L1: Clear secret references on sign-out (in index.html)
  10. F-L3: Input validation (in pq-crypto.mjs)
  11. F-H1: Add test suite (new file + package.json)
  12. Rebuild bundle and verify tests pass

Tier 3 items (F-M1, F-M2, F-M3, F-M4, F-L7) are deferred unless you want them included.


Mermaid diagram

flowchart TD
    H2[F-H2: verifyNostrEvent id check] --> L4[F-L4: e-tag id check]
    L4 --> L5[F-L5: Content-Type fix]
    L5 --> L6[F-L6: relay close-as-success]
    L6 --> H3[F-H3: XSS - textContent]
    H3 --> H4[F-H4: CSP + SRI]
    H4 --> L8[F-L8: doc leftover]
    L8 --> L2[F-L2: 12-word warning]
    L2 --> M5[F-M5: Falcon warning]
    M5 --> L1[F-L1: clear secrets]
    L1 --> L3[F-L3: input validation]
    L3 --> H1[F-H1: test suite]
    H1 --> BUILD[Rebuild bundle + run tests]

    style H2 fill:#fcc
    style H3 fill:#fcc
    style H4 fill:#fcc
    style H1 fill:#fcc
    style BUILD fill:#cfc