From ee6573c57f721db8550e34e1c1c79c5922c62a4b Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Thu, 13 Aug 2026 03:56:30 +0200 Subject: docs: second security review + roadmap rewrite Second architecture and security review (second-review.md): 6 critical and 7 high findings against the Phase 12 implementation, plus an assessment of whether the system meets its end-to-end confidentiality claim. Roadmap rewritten against those findings (devel-phases-next.md): new blocking Phase 11.5 (security remediation), Phase 12 (hub minimization), Phase 13 (native desktop client). Old phases 12-17 renumbered to 14-19. tmp-decisions.md records two open decisions: whether the hub keeps serving the web UI, and browser extension vs native desktop client vs both. CLAUDE.md and devel-phases-next.md also carry pre-existing Phase 12 edits from the working tree that could not be cleanly separated from the review changes. Co-Authored-By: Claude Opus 5 --- devel-phases-next.md | 436 ++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 383 insertions(+), 53 deletions(-) (limited to 'devel-phases-next.md') diff --git a/devel-phases-next.md b/devel-phases-next.md index 9a8b7ef..04d7b9b 100644 --- a/devel-phases-next.md +++ b/devel-phases-next.md @@ -1,8 +1,17 @@ # MeshBay — Next Implementation Phases -> Base: Phases 1–11 complete (except 10.9 → Phase 16). 171 tests. Web SPA + admin panel + self-service UI + MSE video streaming live on meshbay.org. Node daemon is production-ready (WebRTC, WS, chat, HTTP, index push, swarm all wired). +> Base: Phases 1–12 complete (except 10.9 → Phase 18). Web SPA + admin panel + self-service UI + MSE video streaming live on meshbay.org. Node daemon is production-ready (WebRTC, WS, chat, HTTP, index push, swarm all wired). > Architecture reference: docs/meshbay-draft-v4.md > First security review: first-review.md (2026-08-10) +> **Second security review: second-review.md (2026-08-13) — 6 critical, 7 high findings.** +> +> ⛔ **Phase 11.5 is BLOCKING.** No feature phase starts until C1–C6 and H1–H7 are closed. +> The current build must not host real private data: the node's HTTP API serves private +> group content unauthenticated (C1), any user can hijack a node's signaling identity (C2), +> and an active hub can obtain any group key through the key directory it controls (H3). +> +> **Phases renumbered 2026-08-13** (old → new): 12→14, 13→15, 14→16, 15→17, 16→18, 17→19. +> New: 11.5 (security remediation), 12 (hub minimization), 13 (native desktop client). --- @@ -567,35 +576,278 @@ allows other nodes/clients to discover which nodes host which content. --- -## Phase 12 — Node CLI + management +## Phase 11.5 — Security remediation ⛔ BLOCKING + +> Source: `second-review.md` (2026-08-13). Finding IDs in brackets. +> **No other phase starts until section J acceptance criteria pass.** + +**Objective:** close the gap between what the documents describe and what the code +enforces. The Phase 12 sovereignty work (GEK-HMAC proof, DTLS channel binding, Ed25519 +admin challenge) is sound but was implemented on one of four paths into the node. This +phase reduces the node to two paths and brings both to the same standard. + +### Transport decision (settled 2026-08-13) + +| Listener | Fate | Reason | +|---|---|---| +| WebRTC DataChannel (aiortc) | **Primary** — browser + native | ICE/STUN is the only NAT traversal validated here (2 ISPs, 2 browsers, IPv4 STUN + IPv6, 4G CGNAT) | +| QUIC 19000 | **Kept, brought to parity** | LAN, port-forwarded, and hub-less `group://` direct access | +| TCP+TLS 18001 | **Removed** | Superseded; no GEK proof; nothing uses it | +| HTTP 19001 | **Removed** | Source of C1; duplicates MNP without any of its controls | + +> `punch_nat()` is a single UDP probe (`quic_server.py:446`) with no STUN client, no +> candidate gathering and no dual-stack fallback — `aioice` is pulled in by `aiortc` only. +> It is a direct-connection helper, **not** a traversal stack. ICE remains the primary path. + +### A — Reduce the node's exposed surface + +| # | Component | Finding | Done when | +|---|---|---|---| +| 11.5.1 | Delete `transport/http_server.py` + daemon wiring (`daemon.py:341-366`) | **C1** | No listener on `0.0.0.0` other than QUIC; no endpoint serves file bytes or an index without a completed handshake | +| 11.5.2 | Delete `transport/server.py` + `transport/client.py` (TCP+TLS) | C6 scope | `ChunkServer` gone from `daemon.py`; port 18001 unbound | +| 11.5.3 | Node admin UI stays loopback + gains a session token in the URL | H2 | UI unreachable without the token printed at daemon startup | + +### B — One handshake, two transports + +| # | Component | Finding | Done when | +|---|---|---|---| +| 11.5.4 | Extract `meshbay_common/handshake.py`: JWT verify → `scope == "user"` → denylist → **mandatory** `group_id` in claims → group hosted → GEK challenge → proof verify → ack | **C6**, M1, M9 | Single implementation; `webrtc_server.py` and `quic_server.py` contain no JWT logic of their own | +| 11.5.5 | Both transports call it; test parametrized over `[webrtc, quic]` | C6 | A test that adds a step to the handshake fails for any transport that skips it | +| 11.5.6 | **Spike:** channel binding for QUIC. No DTLS fingerprint exists — bind to the QUIC server certificate hash as the analogue (`sha256(server_cert) ‖ sha256(client_cert)`); prefer an RFC 5705 TLS exporter if `aioquic` can expose one | C6/NS5 | QUIC handshake proof is bound to the connection, not replayable across connections | + +### C — Mutual authentication + +| # | Component | Finding | Done when | +|---|---|---|---| +| 11.5.7 | Node proves GEK possession over a client nonce **and** signs the transcript with `sk_node`: `Ed25519(sk_node, "meshbay:node_proof:v1" ‖ nonce_c ‖ binding)` | **C3** | Client rejects a peer that cannot produce both | +| 11.5.8 | Client pins `pk_node` (TOFU on first connect, persisted); key change raises a blocking warning | C3 | Swapping the node's key surfaces to the user instead of silently succeeding | +| 11.5.9 | Node WS registration: require `scope == "node"`, verify `Node.user_id == payload["sub"]`, derive `group_ids` **from the DB**, refuse to overwrite a live registration | **C2** | A user-scoped token, or a mismatched `node_id`, is rejected at `/v1/nodes/ws` | +| 11.5.10 | `POST /v1/nodes/announce` requires proof of possession of `sk_node`; one active record per user | M8 | Announcing someone else's `pk_node` fails | + +### D — MNP authorization + +| # | Component | Finding | Done when | +|---|---|---|---| +| 11.5.11 | `gek_bundle_store` requires an Ed25519 admin challenge; **delete `_try_activate_gek`** — GEK activation is local-UI/CLI only | **C5b** | A member cannot change the group's active GEK | +| 11.5.12 | Upload: per-user quarantine `.uploads/{user_id}/`, refuse to overwrite an existing index entry, size cap + per-user quota, filename allowlist (`[A-Za-z0-9._-]`) | **C5a**, H2 | A member cannot replace another member's file, and cannot inject markup via a filename | +| 11.5.13 | Admin challenge becomes a structured transcript: `"meshbay:file_delete:v1" ‖ node_pk ‖ group_id ‖ file_id ‖ nonce ‖ ts`; client displays what it signs | **H5** | No path exists where a peer obtains a signature over bytes it fully chose | +| 11.5.14 | `gek_bundle_fetch` / `keypair_bundle_fetch` move **after** proof verification; interim rate-limit + audit on the pre-proof window | C4 (partial) | Pre-proof window serves nothing; full fix lands in 13.3 | + +### E — Isolation + +| # | Component | Finding | Done when | +|---|---|---|---| +| 11.5.15 | `chat_store` and `_peers` resolve from `_group_ctx()`, one peer registry per group (`daemon.py:249`, `webrtc_server.py:602,617,650`) | **H1** | Two-group / two-user test proves neither history nor broadcast crosses groups | + +### F — Node admin UI + +| # | Component | Finding | Done when | +|---|---|---|---| +| 11.5.16 | `html.escape()` on every interpolated value (`ui/app.py:363`), `textContent` in the audit page (`:632`), CSP header | **H2** | A file named `` renders as text | + +### G — Revocation + +| # | Component | Finding | Done when | +|---|---|---|---| +| 11.5.17 | Handle `target == "group"` on the node; persist the denylist to `data_dir`; check group status in `webrtc_offer` | **H4** | Revoking a group drops live sessions and blocks new signaling | + +### H — Privacy + +| # | Component | Finding | Done when | +|---|---|---|---| +| 11.5.18 | Swarm registers hashes for `visibility == "public"` groups only; fix the mis-mounted route (`/v1/groups/v1/swarm/...`); authenticate the lookup | **H7** | No private-group content hash ever reaches the hub | + +### I — Resource limits + +| # | Component | Finding | Done when | +|---|---|---|---| +| 11.5.19 | Pre-handshake buffer cap (a few KB, not 64 MB); `asyncio.Semaphore` around ffmpeg; delete the synchronous `subprocess.run` in `_do_stream_segment`; per-user signaling rate limit + membership check before relaying an offer; validate `peer_ip` against the request source | **H6** | One client cannot stall the daemon's event loop or exhaust its memory/CPU | + +### J — Crypto hygiene, hub fixes, acceptance + +| # | Component | Finding | Done when | +|---|---|---|---| +| 11.5.20 | Keystore Argon2id → 256 MB, parameters stored per-node in `node.toml` (not a `meshbay_common` constant); raise the password minimum | M2 | `calibrate-argon2` writes usable config; `crypto.py:173` no longer hardcodes 64 MB | +| 11.5.21 | Length-prefix every field in the HMAC transcript; **reject** empty DTLS fingerprints instead of proceeding | L4 | A missing fingerprint fails the handshake rather than degrading it to nonce-only | +| 11.5.22 | Hub: fix IPLog backfill (`users.py:118-122`), trusted-proxy XFF, scrub `str(e)` from peer-visible errors, drop `GEK_REQUEST`/`GEK_RESPONSE` constants, validate email | M6, M7, L3, L1, L6 | Compliance log attributes each row to the right account | +| 11.5.23 | Regression suite | all | See below | + +**Required regression tests (all must exist and fail on reintroduction):** + +``` +test_no_unauthenticated_content — every node listener refuses index/chunks pre-handshake +test_handshake_parity[webrtc,quic] — identical checks on both transports +test_group_isolation — 2 groups × 2 users: chat + peers never cross +test_upload_cannot_overwrite — member B cannot replace member A's file +test_gek_store_requires_admin — member cannot store/activate a GEK +test_ws_node_identity — user token / foreign node_id rejected +test_node_proof_required — client aborts when the node cannot prove GEK + sk_node +test_ui_escapes_filenames — markup in a filename renders inert +test_swarm_public_only — private hashes never registered +``` + +**Acceptance criteria for the phase:** with a hub whose signing key is in the attacker's +hands, an attacker who is not a group member obtains **no** index entry, **no** file byte, +**no** chat message, and cannot write to any node. A member who is not the node operator +cannot delete or overwrite another member's file, and cannot change the group key. + +--- + +## Phase 12 — Hub minimization: registrar and nothing more + +**Objective:** reduce the hub to its legitimate role and make that reduction *structural* +rather than a matter of good behaviour. The hub must not be able to see private keys, +unencrypted content, or file listings — not "does not currently", but "cannot". + +### What the hub is allowed to know + +| Category | Allowed | Notes | +|---|---|---| +| Account: username, encrypted email, public keys, status, role | ✅ | Required to be a registrar | +| Group registry: id, admin, visibility, join policy, membership | ✅ | Required to issue the `groups` claim | +| Public group name + description | ✅ | Required for discovery | +| IP logs | ✅ | Legal retention, 1 year | +| Signaling relay (SDP/ICE, in-memory, seconds) | ✅ | Never persisted | +| **Private keys, keypair bundles, GEK bundles** | ❌ | Removed in Phase 12 (old); 13.3 removes the last copies | +| **File content, file names, file hashes, index** | ❌ | H7 was leaking hashes; 11.5.18 closes it | +| **Message content or per-message metadata** | ❌ | `chat_notify` currently leaks it — 12.3 | +| **Private group name / description** | ❌ (target) | 12.5 | + +### Milestones + +| # | Component | Description | +|---|---|---| +| 12.1 | Route inventory + blindness test | Enumerate every hub route; assert no response body can contain key material, content, a file name or a content hash. Runs in CI, fails the build on regression | +| 12.2 | **Key transparency + safety numbers** [H3] | Append-only, hub-signed key log; clients pin the key they first saw and audit the log; key change raises a blocking warning; safety-number comparison UI between two members. This is the fix for the last structural way a hub can read content | +| 12.3 | Chat metadata minimization | `chat_notify` (`webrtc_server.py:634-644` → `revocation.py:101-128`) currently tells the hub *who* posted in *which* group and *when*. Drop `sender_name`, make notification opt-in per group, coalesce and delay to blunt timing correlation | +| 12.4 | Swarm hardening | Enforce 11.5.18 at the API layer too: reject registration for a group the hub knows is private; authenticate `GET /v1/swarm/{hash}` | +| 12.5 | Opaque private-group metadata | For `visibility == "private"`, store name/description as a member-encrypted blob; the hub holds an opaque value and an id. Public groups unchanged (discovery needs plaintext) | +| 12.6 | SPA integrity + honest labelling | Strict CSP, SRI on the bundle, hub publishes a signed digest of the served bundle that native clients and extensions can verify; `/app/` carries an explicit "reduced trust — this hub serves this code" notice | +| 12.7 | Remove dead crypto plumbing | Drop residual columns/migrations/constants from the pre-Phase-12 GEK era so the schema cannot be quietly repopulated | +| 12.8 | Written threat model | One page: passive hub, active hub, malicious node operator, malicious member, network attacker, local attacker — and for each claim, which adversary it holds against. Referenced from draft-v5 | + +**Acceptance criteria:** a hub operator holding root on the server, the full PostgreSQL +database, the Ed25519 signing key, and the ability to forge any JWT can obtain: no private +key, no GEK, no file content, no file name, no content hash, no message content, and no +private group name. Every remaining capability is on the list above and is documented in +12.8. Any attempt to substitute a public key is detectable by clients via 12.2. + +--- + +## Phase 13 — Native desktop client (pywebview + aiortc) + +> **Status (2026-08-13): 13.1 active, 13.2–13.11 DEFERRED to after Phase 15**, pending +> decision D2 in `tmp-decisions.md` (browser extension vs native client vs both). +> +> **13.1 (platform adapter split) proceeds regardless** — it is pure refactoring whose +> acceptance criterion is "the browser SPA behaves identically", and it is the prerequisite +> for every option under D2. + +**Objective:** ship a desktop application with durable key storage, hub-independent +`group://` access, and a better media path than the browser allows. + +> ⚠️ **Do not justify this phase as "the fix for T3".** An earlier draft of +> `second-review.md` claimed a native client makes code integrity independent of the hub. +> That was wrong: a binary downloaded from `meshbay.org` and signed with a key the hub +> operator holds relocates the trust rather than removing it. What native actually changes is +> **detectability** — an attack must ship as an artifact that can be hashed and compared +> instead of a one-off HTTP response — and that value is realised only by **18.7 reproducible +> builds** plus published hashes. Native also *costs* the browser sandbox, hands you patch +> velocity for WebKitGTK and every bundled dependency, and adds the loopback media server, +> the IPC bridge and the updater as new attack surface. +> +> The security-per-effort ranking is: **11.5 ≫ 12 ≫ 14 (CLI) ≫ 13.** This phase is justified +> on product grounds. It permanently closes **C4** as a side effect, but C4 can also be closed +> in a browser by not storing keypair bundles remotely at all. + +### Why this is cheap + +The SPA never touches a browser crypto or network primitive directly: `app.js` contains +**0** occurrences of `crypto.subtle` and **0** of `RTCPeerConnection`. All crypto and +transport go through three injected globals (`window.MeshBayCrypto`, `MeshBayKeys`, +`MeshBayTransport` — 16 call sites) and all hub I/O through one function (`hubFetch`, 30 +call sites). That is the seam. + +| Asset | Lines | Native | +|---|---|---| +| `style.css`, `i18n.js`, `vendor/htm-preact.js` | 1708 | **reuse as-is** | +| `app.js` — components, routing, theme, admin | ~2050 | **reuse as-is** | +| `app.js` — storage glue, `hubFetch`, download/upload callbacks, MSE `VideoPlayer` | ~550 | rewrite | +| `transport.js`, `crypto.js`, `keyderive.js` | 1145 | **delete** | + +≈ **69 % reused unchanged**, and the 31 % that is not is largely code `second-review.md` +says to delete anyway (WebCrypto AES variant, PBKDF2 password split, keypair bundles). + +### Non-negotiable + +**UI assets ship inside the package and load from disk.** A shell that points its WebView at +`https://meshbay.org/app/` is a browser with a different icon and fixes nothing. The hub is +used for the API only, and the bundle is covered by 13.9 signing. + +### Milestones + +| # | Component | Description | +|---|---|---| +| 13.1 | Platform adapter split | Extract `platform-web.js` (WebRTC/WebCrypto/fetch — today's behaviour) and `platform-native.js` (pywebview bridge). `app.js` imports neither directly. **Acceptance: the browser SPA is byte-for-byte functional after the split** — this lands first, on its own, with no native code | +| 13.2 | pywebview shell + Python bridge | `meshbay-client` package; `window.pywebview.api.*` implements the same surface as the three globals; single-instance, tray, window state | +| 13.3 | Local keystore + Ed25519 client auth | Reuse `keystore.py` (Argon2id 256 MB, OS keychain later). Client authenticates like the daemon does: signed timestamp, `POST /v1/users/auth`. **No password on the wire, no `auth_key`/`bundle_key`, no keypair bundle anywhere** → closes **C4** permanently | +| 13.4 | aiortc client transport | `RTCPeerConnection` + `createDataChannel` + `createOffer` in Python; ICE/STUN via `aioice` — the traversal path validated on 2 ISPs. Calls the unified handshake from 11.5.4. QUIC (`quic_client.py`) retained as opt-in for LAN / port-forwarded / hub-less `group://` | +| 13.5 | Local index cache | SQLite in the client profile dir, replacing IndexedDB (also restricted under `file://` in some WebViews) | +| 13.6 | Loopback media server | Python decrypts and serves with HTTP Range; `