path: root/docs/second-review.md

# MeshBay — Second Architecture & Security Review

> Date: 2026-08-13
> Scope: architecture and security design review of the hub ↔ node ↔ client protocol,
> as specified in draft v4 and the Phase 1–12 log (both archived in `old-draft.md`), `devel-phases-next.md`,
> and as **implemented** in `packages/` (Phases 1–12 + 10b/10c).
>
> Unlike `first-review.md` (2026-08-10), which was a design-level review, this one reads
> the code that implements the protocol: `protocol.py`, `webrtc_server.py`, `quic_server.py`,
> `server.py`, `http_server.py`, `daemon.py`, `bundle_store.py`, `ui/app.py`, the hub API
> routers, and the browser client (`transport.js`, `crypto.js`, `keyderive.js`, `app.js`).
>
> Finding numbering is **independent** of `first-review.md`. All C1/H1/M1 references below
> are new.
>
> Not verified: the test suite could not be run (`pytest` is not installed in `.venv`), so
> the "191 tests" claim is taken at face value. No live testing against meshbay.org was done.
> This is a code and design review, not a penetration test.

---

## 1. Executive summary

The cryptographic core remains sound: ECIES GEK wrapping, HKDF domain separation, AEAD
chunk encryption, Ed25519 JWT with `jti`, refresh-token rotation. The *ideas* added since
the first review — GEK-HMAC handshake proof, DTLS channel binding, Ed25519 admin
challenge-response, password split, node sovereignty — are the right ideas, and several of
them are genuinely clever.

**But the implementation does not enforce the model the documents describe.** The
protection added in Phases 11–12 lives almost entirely on the WebRTC path, while three
other paths into the same node (HTTP API, QUIC, TCP) were left as they were. The most
serious result is that **every private group served by a node daemon is exposed in
plaintext, without any authentication, over the node's HTTP API on `0.0.0.0`**. That single
defect nullifies the entire GEK-proof / node-sovereignty layer for anyone who can reach the
node's HTTP port.

Answering the question directly:

> *"The client and the node want to communicate safely, with everything encrypted and
> unreadable by other parties, even the hub. Does it do what it claims?"*

**Partially, and not today.**

- Against a **passive/honest-but-curious hub**: yes for file content. The hub never sees
  the GEK, never sees chunks, and is out of the data path after signaling. This part works.
- Against an **active malicious hub**: **no.** The hub is the public-key directory. When a
  member invites someone, the inviter fetches the invitee's `pk_x25519` *from the hub* and
  wraps the GEK for it (`app.js:1399-1410`). A hub that returns its own key gets the GEK for
  that group. This is documented as trust assumption **T2** but it is not a residual risk —
  it is a complete break of the confidentiality claim, requiring no exotic capability.
- **"Everything encrypted"**: **no.** Chat messages are plaintext on the wire (application
  layer) and plaintext at rest in SQLite. The Mesh Group Index is sent in cleartext over the
  WebRTC DataChannel. Uploads are transmitted and stored in plaintext. Files are stored in
  plaintext on the node by design.
- **"Unreadable by other parties"**: it is readable by every group member, by the node
  operator, and — via the findings below — by anyone who can reach the node's HTTP port or
  who can hijack a node's signaling registration on the hub.

There are **6 critical** and **7 high** findings. Most are not exotic crypto issues; they
are missing authorization checks and paths that were never brought up to the level of the
newest path. None of them invalidate the architecture — all are fixable inside the existing
design — but the current build should not be described as end-to-end secure, and should not
host real private data until C1–C6 are closed.

---

## 2. What is solid

Worth recording, because the delta since the first review is real:

1. **GEK-HMAC handshake proof with DTLS channel binding** (`webrtc_server.py:294-335`,
   `crypto.js:235-246`). Binding `HMAC(GEK, nonce ‖ offer_fp ‖ answer_fp)` to the DTLS
   fingerprints of both sides is a correct, well-chosen defence: a signaling relay that
   substitutes its own fingerprints cannot produce a proof the node accepts. The Chrome
   raw-SDP workaround (`transport.js:127`) shows this was actually made to work, not just
   specified.
2. **Deny-by-default on destructive operations** (`webrtc_server.py:750-754`). If no key is
   pinned, deletion is refused. Correct posture.
3. **Ed25519 node→hub authentication** with a domain-separated message
   (`meshbay:node_auth:{username}:{timestamp}`, `nodes.py:55`) and a node-scoped JWT that
   `require_user_scope` refuses for mutations. Clean.
4. **Refresh-token family rotation with reuse detection** (`users.py:213-267`). Textbook
   OAuth BCP.
5. **HKDF domain separation** is consistent and the AES/ChaCha20 variants are properly
   separated by info string (`:aes` suffix), so the two ciphers can never derive the same
   key from one GEK.
6. **AEAD-only chunk wire format.** Dropping per-chunk Ed25519 signatures in favour of
   AES-GCM tags (Phase 9.15) is defensible: the tag authenticates the ciphertext under a key
   only members hold. (It does have a consequence — see H3.)
7. **The trust-domain separation in §4.2.x of draft-v4 is the right model.** "The hub
   certifies identity; the node authorizes content operations" is exactly the correct
   framing for this system. The problem is enforcement coverage, not the model.

---

## 3. Critical findings

### C1 — Private group content is served in plaintext with no authentication (node HTTP API)

**Location:** `transport/http_server.py:115-160`, wired in `daemon.py:341-366`

The daemon starts `create_http_app()` for **every configured group**, private ones included,
bound to `0.0.0.0:http_port` (default 19001).

Two endpoints have **no authentication of any kind** — no JWT, no group check, no GEK proof:

```python
@app.get("/index")            # http_server.py:115 — full file listing, no auth
@app.get("/file/{file_id}")   # http_server.py:141 — FileResponse(path) — raw plaintext file
```

`download_file` reads the file straight off disk and streams it. The `gek` parameter is only
consulted by the *chunk* endpoint (`/file/{id}/{chunk}`), and even that one accepts **any**
JWT signed by the hub — no group-membership check, no GEK proof.

The docstring says "Note: this server handles PUBLIC content only", but nothing in the code
enforces it: the daemon passes the private group's `shared_root` and index unconditionally.

**Impact.** Complete bypass of the entire Phase 12 sovereignty layer. Anyone who can reach
the port gets the full private index and every private file in cleartext:
- anyone on the node operator's LAN/VLAN (guest WiFi, roommate, compromised IoT device);
- anyone on the Internet if the operator forwarded the port (the docs encourage port
  forwarding for NAT edge cases) or has a permissive IPv6 firewall;
- any local process/user on the machine.

No JWT forgery, no hub compromise, no GEK required. This is the single most severe issue in
the codebase and it silently negates NS1/NS3/R22 in the documentation.

**Fix.** Bind to `127.0.0.1` at minimum. Then: refuse to start the HTTP app at all for
groups with `visibility = "private"`; require a valid JWT *and* group membership on every
endpoint including `/index` and `/file/{id}`; never serve plaintext bytes for a group that
has a GEK. Better: delete this server. It predates the WebRTC/QUIC paths and duplicates them
without any of their controls.

---

### C2 — Any authenticated user can hijack a node's identity on the hub (WebSocket)

**Location:** `api/revocation.py:131-163`

```python
decoded = decode_access_token(msg["token"])
node_id = msg.get("node_id") or decoded.get("sub", "unknown")   # ← client-supplied
_connected_nodes[node_id] = ws
group_ids = msg.get("group_ids", [])                            # ← client-supplied
_node_groups[node_id] = group_ids
```

The hub accepts whatever `node_id` and `group_ids` the connecting party claims. There is no
check that the JWT subject owns that node record, and no check that `scope == "node"`.

**Impact — this is a full client-impersonation primitive.** Any registered user can:

1. Connect to `/v1/nodes/ws` with their ordinary user JWT and claim the `node_id` of a
   victim node, overwriting the legitimate entry in `_connected_nodes`.
2. All subsequent `POST /v1/nodes/{node_id}/webrtc/offer` requests from browsers are relayed
   to the **attacker** (`signaling.py:56`), who answers with their own SDP.
3. The victim's browser now has a DataChannel to the attacker, believing it is the node.

The DTLS channel binding does *not* help here: the attacker is the endpoint, not a relay.
The browser sends its GEK proof to the attacker, who simply ignores it and replies
`handshake_ack` — `transport.js:204` only checks `ack.type === 'handshake_ack'`.

The attacker then receives:
- the victim's **encrypted keypair bundle** (`storeKeypairBundle`, `app.js:850`) → offline
  password brute-force target (see C4);
- every chat message the victim sends (plaintext);
- every file the victim uploads (plaintext);
- and can serve a forged index and forged chat history.

Also: `group_ids` is attacker-controlled, so the attacker can advertise as an online node for
any group and appear in `GET /v1/groups/{id}/nodes` — the browser picks `nodes[0]`
(`app.js:822`) with no further verification.

**Fix.** Require `scope == "node"`; look up the `Node` row and verify `node.user_id ==
payload["sub"]`; derive `group_ids` from the database (`GroupMember` for that user), never
from the message; reject a second registration for an already-connected `node_id` instead of
overwriting it.

---

### C3 — The node never authenticates itself to the client

**Location:** `transport.js:56-215`, `app.js:808-827`, `webrtc_server.py:351-362`

Authentication is one-directional. The client proves its identity (JWT) and its membership
(GEK-HMAC). The node proves *nothing*:

- `handshake_ack` carries `node_pk` but there is no signature over anything — possession of
  `sk_node` is never demonstrated.
- The browser fetches `pk_node` from `GET /v1/groups/{id}/nodes` and then **discards it**;
  `app.js:822` uses only `nodes[0].node_id`.
- Per-chunk Ed25519 signatures were removed in Phase 9.15, so no later message proves node
  identity either.

The only implicit authentication is possession of the GEK, and it only covers *file chunks*
(they will not decrypt otherwise). Everything else — the index, chat history, `is_node_admin`,
`handshake_challenge`, and everything the client *pushes* — is unauthenticated.

**Impact.** Enables C2 end-to-end, and independently means a hub that returns an attacker's
`node_id` for a group achieves the same result. `is_node_admin` is trusted by the SPA
(`app.js:829`) to decide which controls to display, and it comes from an unauthenticated
peer.

**Fix.** Mutual proof in the handshake. Simplest correct version: the node returns, alongside
its challenge, `HMAC(GEK, "meshbay:node_proof:v1" ‖ nonce_c ‖ offer_fp ‖ answer_fp)` over a
client-supplied nonce, and the client verifies it before sending anything sensitive. Add
`Ed25519(sk_node)` over the same transcript and have the client pin `pk_node` from the hub
(TOFU + change alerts), so that node identity does not rest on a group-shared secret.

---

### C4 — Users' encrypted private-key bundles are handed to third parties, and are only PBKDF2-protected

**Location:** `webrtc_server.py:194-197, 451-492`, `bundle_store.py:84-100`,
`keyderive.js:74-87`, `app.js:848-856`

Phase 12 moved keypair bundles off the hub and onto nodes. Three problems compound:

1. **The bundle is served before the GEK proof.** In `_handle_message`, both
   `GEK_BUNDLE_FETCH` and `KEYPAIR_BUNDLE_FETCH` are dispatched on the condition
   `self._gek_challenge is not None` — i.e. after JWT verification but **before**
   `_do_handshake_response` has validated anything. (`_gek_challenge` is even set on the
   error path where the group has no GEK, `webrtc_server.py:279-292`.) A hub that forges a
   JWT for user X — trivial, it holds the signing key — retrieves X's encrypted keypair
   bundle without ever possessing the GEK. This is a chicken-and-egg the design has to solve,
   but as written the pre-proof window is a data-disclosure window.

2. **The bundle is pushed to every node the user connects to.** `app.js:848` pushes
   `_pendingBundlePush` to whichever node the group connection landed on. Join five groups
   hosted by five different people and five unrelated operators now hold your private-key
   bundle on their disk.

3. **The bundle is protected only by PBKDF2-SHA512, 600 000 iterations**
   (`keyderive.js:23,74-87`), salted with `SHA-256("meshbay:bundle:v1:" + username)` — a
   deterministic, non-random salt.

Consequence: the "password split" (T1) does not deliver what §4.2.x claims. It is true that
the hub cannot *derive* `bundle_key` from `auth_key`. It is not true that the hub is
therefore locked out: the hub obtains the bundle by forging a JWT (path 1) and then runs an
offline dictionary attack that costs **only PBKDF2**, not the Argon2id-256MB the hub's own
password verifier is protected by. The user's password is the last line of defence, and it
is defended by the *cheaper* of the two KDFs. Recovering it yields `sk_ed25519` and
`sk_x25519` → unwrapping every GEK bundle → all groups, all content, plus the ability to
sign as that user.

Every node operator whose group you join gets the same offline target (path 2).

**Fix.** Ranked:
- **Do not store keypair bundles on other people's machines.** This is the wrong home for
  them. A native client keeps keys in a local OS-protected keystore; the browser can keep them
  in IndexedDB with an explicit, user-initiated encrypted export.
- If the bundle must be remotely recoverable, protect it with Argon2id (256 MB) via WASM, not
  PBKDF2, and use a random per-user salt fetched alongside the bundle.
- Serve it only *after* a successful GEK proof, and only from the user's own node.
- Separate the bundle key from the login password entirely (recovery phrase), so that
  cracking one does not yield the other.

---

### C5 — Any group member can overwrite arbitrary files in the shared directory, and can seize the group key

Two independent authorization gaps in the MNP handlers, both reachable by any authenticated
group member (the GEK proof does not distinguish members from each other).

**C5a — Upload overwrites anything** (`webrtc_server.py:683-726`)

```python
safe_name = filename.replace("/", "_").replace("\\", "_").replace("..", "_")
...
final_path = shared_root / safe_name
tmp_path.rename(final_path)          # unconditional overwrite
```

Path traversal is blocked, but nothing prevents overwriting an existing file. There is no
size limit, no quota, no per-user restriction, no operator approval. So:
- any member can destroy or replace any file at the root of the shared directory —
  a direct violation of "the node operator is the sole authority over content";
- and this **bypasses the deletion controls entirely**: overwrite the victim's file, then
  `_register_uploader` (`:727-736`) tags the entry with *your* `uploader_pk`, after which you
  can legitimately delete it via the uploader path (`:798-806`);
- disk-fill DoS is unconstrained.

**C5b — GEK bundle store has no authorization, and auto-activates**
(`webrtc_server.py:391-449`)

`_do_gek_bundle_store` writes whatever `(group_id, user_id, bundle)` the caller supplies, with
no check that the caller is the group admin or the node operator, and `INSERT OR REPLACE`
overwrites existing bundles. Then:

```python
if node_user_id and target_user_id == node_user_id and group_id:
    await self._try_activate_gek(group_id, target_user_id)   # unwraps and swaps the live GEK
```

The node operator's `pk_x25519` is public (it is even handed out in `handshake_ack` as
`node_pk_x25519`, `:359-361`). So any member can wrap a **GEK of their own choosing** for the
operator's key, store it, and the node will unwrap it and replace the group's active GEK.
Result: all existing content becomes undecryptable for the legitimate members, and the
attacker controls the key used from that point on. A member can also silently overwrite other
members' bundles to lock them out.

**Fix.** Uploads: quarantine to `.uploads/{user_id}/`, refuse to overwrite an existing index
entry, enforce quotas and a max file size, and require operator opt-in for writes outside the
upload directory. GEK bundles: require an Ed25519 challenge-response against the pinned admin
key for `gek_bundle_store`, and never auto-activate a GEK from a peer message — GEK
initialization belongs to the local admin UI only, which is already implemented
(`ui/app.py:175-260`).

---

### C6 — The GEK proof only exists on the WebRTC path; QUIC and TCP accept a JWT alone

**Location:** `quic_server.py:148-186`, `server.py:134-165` vs `webrtc_server.py:242-335`

Draft-v4 §4.2.x states: *"ALL operations require passing the GEK proof first."* That is true
only for `webrtc_server.py`. The QUIC server (started on `::` port 19000) and the TCP server
(started on `0.0.0.0` port 18001) still perform the Phase 7 handshake: verify JWT → check
`groups` claim → `handshake_ack`. No challenge, no proof.

**Impact.** A forged JWT (hub) or a stolen JWT reaches the node over QUIC/TCP and can:
- fetch index and chunks — these are GEK-encrypted, so confidentiality holds *there*;
- **inject chat messages** into the group store — `_do_chat_message_sync` in `quic_server.py`
  stores and broadcasts plaintext payloads without any GEK involvement. Chat injection and
  impersonation of the group's discussion with nothing but a hub-signed token.
- consume node resources without ever holding the group key.

It also means the denylist/GEK/sovereignty story has to be reasoned about per-transport,
which is exactly the kind of divergence that produces the next C1.

**Fix.** Factor the handshake (JWT → denylist → group claim → GEK challenge → proof → ack)
into one function in `meshbay_common` and call it from all three transports. If native
clients are not using QUIC/TCP yet, disable those listeners by default until they are brought
to parity.

---

## 4. High findings

### H1 — Cross-group data leakage on multi-group nodes (chat store and peer set)

**Location:** `daemon.py:249`, `webrtc_server.py:601-681, 617, 343-345`

The daemon builds a proper per-group context (`groups_ctx[gid]["chat_store"]`,
`daemon.py:219-226`) and then sets a single global one:

```python
self._webrtc._ctx["chat_store"] = first.get("chat_store")     # daemon.py:249 — the FIRST group
```

Both chat handlers read from the *top-level* context, not the group context:

```python
chat_store = self._ctx.get("chat_store")    # webrtc_server.py:602 and :650
```

So on a node hosting several groups, **all groups write into the first group's chat database,
and `chat_hist` serves that database to members of every group.** Members of group B read
group A's private conversation.

The same bug affects broadcast: `_peers` lives in the shared `_ctx` (`:974`, `:343-345`), so
`_do_chat_message` (`:617-632`) fans out every message to **all connected peers on the node,
regardless of group**.

**Fix.** `chat_store` and `_peers` must come from `self._group_ctx()`, with one peer registry
per group. Add a test with two groups and two users that asserts isolation.

---

### H2 — Stored XSS in the node admin UI via uploaded filename → node takeover

**Location:** `ui/app.py:359-365` (and `:632-639` for the audit page)

```python
file_rows += f"<tr><td>{e.name}</td><td>{e.type}</td>..."
```

Filenames are interpolated into HTML with no escaping. The upload sanitizer
(`webrtc_server.py:701`) strips path separators but not `<`, `>`, `"`. Any group member can
upload a file named `<img src=x onerror="fetch('/api/groups/GID/gek',{method:'POST'})">`.

The local UI has **no authentication at all** (by design, "localhost only"). So when the
operator opens `http://localhost:18000`, attacker JavaScript runs with full access to the node
admin API: re-initialize/rotate the GEK, enumerate all groups and shared paths, read the whole
audit log (users, IPs, actions), read the config. The audit page builds rows with `innerHTML`
from `e.detail`, which also carries filenames — same vector, different page.

**Fix.** Escape all interpolated values (`html.escape`), use `textContent` in the audit page,
sanitize uploaded filenames to a conservative allowlist, and add a CSP header to the UI app.
Consider a localhost token in the URL to blunt DNS-rebinding against the unauthenticated UI.

---

### H3 — An active hub breaks confidentiality through key substitution (T2 is not a residual risk)

> **CLOSED 2026-08-14.** Not by the fix proposed below. The invite path no longer reads
> the directory at all: the node holds the GEK and wraps it for a key the recipient
> proves possession of over the authenticated channel, and identities are bound to
> accounts by one-time codes the hub never sees. Safety numbers would have made the
> substitution *detectable by a human who checks*; removing the lookup makes it
> impossible. See `invite-pairing-v1.md` and draft-v5 §5.5.
>
> `gek-init` had the same flaw with the node as the victim — it fetched every member's
> public key from the hub and wrapped for the answer. That is gone too.

**Location:** `app.js:1389-1415`, `users.py:340-358`, `users.py:310-337`

The invite flow is: fetch `pk_x25519` for the invitee **from the hub**, wrap the GEK for it,
store the bundle on the node. The hub is the sole key directory, and `PUT /v1/users/me/keys`
lets keys be replaced at any time.

A malicious hub returns its own X25519 key for the invitee. The inviting member wraps the GEK
for the hub. The hub now holds the group key and can decrypt every chunk it can obtain —
including chunks captured via C1, C2, or C6. No JWT forgery needed, no JS injection needed,
nothing detectable by the client.

The documents list this as **T2** under "remaining trust assumptions", alongside T3 (hub
serves the SPA). That framing understates it: with T2 open, the sentence "unreadable by other
parties, even the hub" is not true against an adversarial hub, and the GEK-HMAC/sovereignty
work in Phase 12 does not change that, because the hub obtains the GEK legitimately.

**Fix.** Out-of-band key verification is the only real answer: safety numbers / fingerprint
comparison, key-change warnings ("Alice's key changed on 2026-08-13 — verify before sharing"),
and key transparency (a signed append-only log of key bindings the client audits). Until then,
the honest claim is *"the hub cannot read your content unless it actively attacks you."*

---

### H4 — Group revocation never reaches nodes; jti denylist is volatile

**Location:** `daemon.py:316-330`, `revocation.py:82-96`

The hub signs revocation tokens with `target ∈ {"user", "group"}` and broadcasts them. The
node handler only implements two cases:

```python
if target == "user":  denylist.deny_user(tid)
elif target == "jti": denylist.deny_jti(tid)
# target == "group" → silently dropped
```

So `POST /v1/admin/revoke` for a group marks it revoked in the hub DB and does nothing on any
node. Combined with the fact that `webrtc_offer` (`signaling.py:44-85`) checks neither group
status nor membership, "suspend a group blocks signaling" (draft-v4 §4.2.x) is not true — a
client holding a `node_id` and a still-valid JWT keeps connecting. The denylist is also
in-memory only (`Denylist()`), so it is cleared by any node restart.

**Fix.** Handle `target == "group"` on the node (drop sessions, refuse handshakes for that
group); check group status in `webrtc_offer`; persist the denylist to `data_dir` with
expiry-based pruning.

---

### H5 — The Ed25519 admin challenge is an unbound signing oracle

**Location:** `webrtc_server.py:756-763`, `keyderive.js:249-256`

```python
challenge = os.urandom(32)      # node → client
```
```js
const sig = await crypto.subtle.sign('Ed25519', sk, challenge);   // client signs 32 raw bytes
```

The client signs 32 arbitrary bytes chosen by the node, with its long-term identity key,
with no domain separator, no context, and no length constraint. The signed payload does not
mention "file_delete", the `file_id`, the group, the node, or a timestamp.

Consequences:
- A malicious or compromised node can request a "deletion" and obtain a signature over any
  32-byte string it likes. `meshbay:node_auth:{username}:{timestamp}` is exactly 32 bytes for
  a 3-character username — currently not exploitable because node auth verifies against
  `pk_node_ed25519` rather than the user identity key, but that separation is a coincidence of
  the current schema, not a designed defence.
- Signatures are not bound to the operation, so a captured signature is reusable for any
  future challenge that happens to repeat (it will not, but nothing structurally prevents
  replay across contexts either).

**Fix.** Sign a structured, domain-separated transcript:
`Ed25519(sk, "meshbay:file_delete:v1" ‖ node_pk ‖ group_id ‖ file_id ‖ nonce ‖ timestamp)`,
and have the client display *what* it is signing. Apply the same rule to every future
challenge (this is a protocol-wide invariant, not a one-off fix).

---

### H6 — Unauthenticated resource exhaustion on nodes

Several unbounded paths, all reachable by any hub user (no group membership needed for some):

| Vector | Location | Effect |
|---|---|---|
| `POST /v1/nodes/{id}/webrtc/offer` | `signaling.py:44` — any authenticated user, no membership check, no rate limit | Node allocates an `RTCPeerConnection` + ICE gathering per request; `_sessions` grows |
| DataChannel receive buffer | `webrtc_server.py:121-139` — `MAX_MSG = 64 MB`, buffer grows before handshake | Memory exhaustion by claiming a 64 MB frame and dribbling bytes, pre-auth |
| `stream_req` | `webrtc_server.py:828-906` — spawns `ffmpeg` per request, no concurrency cap | CPU/process exhaustion by any member |
| `stream_seg` | `webrtc_server.py:573-590` — **synchronous `subprocess.run(timeout=30)` inside the event loop** | One request blocks the entire node for up to 30 s |
| `file_upload` | `webrtc_server.py:683` — no size/quota limit | Disk fill |
| `POST /v1/nodes/{id}/incoming` | `revocation.py:202` — any user picks `peer_ip`/`peer_port` | Node emits UDP probes to arbitrary destinations (small reflection primitive) |

**Fix.** Per-user connection caps and rate limits on signaling; membership check before
relaying an offer; cap the pre-handshake buffer at a few KB; a semaphore around ffmpeg;
make `stream_seg` async or delete it (superseded by `stream_req`); upload quotas; validate
that `peer_ip` matches the requester's source address.

---

### H7 — Swarm registration publishes private-group file hashes to the hub (currently masked by a routing bug)

**Location:** `daemon.py:382-387, 501-506`, `groups.py:120`, `hub_client.py:277-294`

The daemon registers the blake3 hashes of **every group's** files with the hub swarm table,
private groups included — there is no visibility filter. Draft-v4 §7.3 describes the swarm as
a *public content* mechanism.

Right now this fails silently: the route is declared as `@router.post("/v1/swarm/register")`
on a router with `prefix="/v1/groups"`, so it is mounted at `/v1/groups/v1/swarm/register`,
while the node posts to `/v1/swarm/register` → 404, swallowed by `except Exception: pass`.

**Impact.** The bug is currently protecting privacy. Fixing the path without adding a filter
would immediately leak, to the hub, a content-identifier fingerprint of every private file
on every node — enough for the hub (or anyone with `GET /v1/swarm/{hash}`, which requires no
auth) to confirm "does this known file exist in the network, and which node has it". That is
precisely the metadata the "hub stores no content metadata" claim rules out.

**Fix.** Register hashes only for groups with `visibility == "public"`, fix the route, and
require authentication on the lookup endpoint.

---

## 5. Medium findings

**M1 — `group_id` is optional in the handshake, which skips the membership check.**
`webrtc_server.py:257,261` guard on `if group_id and ...`. With `group_id = ""` both checks
are skipped and `_group_ctx()` (`:506-509`) falls back to `self._ctx`, which the daemon
populates with the **first group's** gek/index/shared_root (`daemon.py:238-247`). Access still
requires that group's GEK, so it is not a full bypass — but a user removed from the group on
the hub who kept the GEK regains access, and the JWT `groups` claim stops being authoritative.
Make `group_id` mandatory.

**M2 — Argon2id in the node keystore is still 64 MB.** `crypto.py:173-174`
(`ARGON2_MEMORY_COST = 65536`) with a comment saying to raise it. Only the hub's password
verifier got the 256 MB bump (`auth.py:30-35`). The docs record R1/8.10 as done, which is true
for the hub and false for the keystore. Also, `create_keystore` accepts an 8-character
minimum password, and the calibration command prints instructions to hand-edit a constant in
`meshbay_common` rather than writing a per-node parameter — so the keystore parameters cannot
actually be tuned per hardware as §4.2.1 promises.

**M3 — Node operator cannot delete files in the default configuration.** *(CLOSED
2026-08-14 — the auto-pin is deleted; authority comes from the node's roster, established
locally by `meshbay-node operator pair`. Asking the hub for the operator's key, the
obvious-looking fix, would have let the hub install itself as node administrator.)*
`_resolve_admin_pk`
(`daemon.py:451-465`) auto-pins the **node keystore's** Ed25519 key, while the browser signs
challenges with the **user identity** key from the keypair bundle (`app.js:983`). These are
different keys, so verification fails unless the operator manually sets `admin_pk_ed25519` to
their browser key. Fails closed, so it is a correctness problem rather than a hole — but the
sovereignty feature is effectively inert as shipped, and the mismatch will invite the wrong
fix (relaxing the check) unless it is documented.

> **C4 — REDUCED 2026-08-14, not closed.** The bundle's KDF moved from PBKDF2-SHA512
> 600k to Argon2id 128 MB/t=3 in the browser (vendored WebAssembly), so an operator
> attacking one offline no longer enjoys the GPU economics of a compute-only KDF. The
> pre-proof window is unchanged and still bounded. What remains: bundles are still stored
> on every node their owner joins, and a weak passphrase still loses — draft-v5 §7.1 gives
> the measured numbers. It closes at 13.3.

**M4 — Response-to-request matching by arrival order.** `transport.js:390-422` resolves the
**oldest** pending promise with whatever message arrives, ignoring type. With the 8-deep
pipelined download window, a node that reorders responses (or an `error` message arriving
mid-flight) resolves the wrong promise. Add a request id (`rid`) to MNP and echo it in
responses — cheap, and it also removes the `index_sync` special case at `:408-415`.

**M5 — Index and chat are not encrypted on the WebRTC path.** `_do_index_sync`
(`webrtc_server.py:511-528`) sends entries as cleartext msgpack; the QUIC/TCP path uses
`GroupIndex.serialize()` which *is* GEK-encrypted. So the same object has two different
protection levels depending on transport, and draft-v4 §8.2 ("GEK-encrypted, hub stores
opaque") describes only one of them. With DTLS in place this is not remotely readable, but it
means the security property depends entirely on the transport rather than on the data.

**M6 — IP audit log corruption on registration.** `users.py:118-122`:

```python
await db.execute(IPLog.__table__.update().where(IPLog.user_id == None).values(user_id=user.id))
```

This backfills **every** IPLog row that has a NULL `user_id` — including failed-login rows for
other usernames and other users' registrations — with the newly created user's id. For logs
kept for a year specifically to answer legal requests, this is a data-integrity defect that
attributes other people's connections to the wrong account. Set `user_id` on the row you just
created (flush first, or add the row after `db.refresh(user)`).

**M7 — `X-Forwarded-For` is trusted unconditionally.** `users.py:361-365`, `groups.py:313`,
`nodes.py:131`. Behind Caddy this is fine today; if the hub is ever reachable directly, or a
second proxy is added, any client can forge the IP written into the compliance log and evade
per-IP rate limiting. Use a trusted-proxy list and take the rightmost untrusted hop.

**M8 — `announce_node` accepts any `pk_node`.** `nodes.py:88-109` — a user can announce a node
record containing someone else's public key, and node records accumulate without limit. Combine
with C2 for a more convincing impersonation. Verify possession (sign a challenge with
`sk_node`) and enforce one active node record per user unless multi-node is intended.

**M9 — Node accepts node-scoped tokens as client tokens.** All three transports call
`jwt.decode` without inspecting `scope` (`webrtc_server.py:246`, `quic_server.py:153`,
`server.py:141`). A node-scoped token (which is also issued with the full `groups` claim,
`nodes.py:66-71`) is accepted as a regular client anywhere. Check `scope == "user"` on the
client path.

---

## 6. Low findings / notes

- **L1 — Dead protocol constants.** `GEK_REQUEST`/`GEK_RESPONSE` remain in `protocol.py:32-33`
  though the handlers are gone (NS3 says "removed"). Delete them so the wire contract matches
  the docs.
- **L2 — No MNP version negotiation.** Every message carries `v: "0.1"` and nobody checks it
  (`webrtc_server.py`, `transport.js`). §3 of draft-v4 specifies range negotiation and an
  explicit refusal. Currently a version mismatch would fail in undefined ways. Phase 13.6
  covers this — keep it.
- **L3 — Error strings leak internals.** `webrtc_server.py:224-226` and `server.py:127` return
  `str(e)` to the peer, which includes filesystem paths and exception detail.
- **L4 — `hmacGEK` concatenates without length prefixes** (`crypto.js:235-246`,
  `webrtc_server.py:326`). With fixed-size inputs this is unambiguous today; if a fingerprint
  is ever missing (the extractors return empty on failure) the concatenation becomes ambiguous
  and the proof silently degrades to nonce-only. Prefix lengths, and **reject** empty
  fingerprints instead of proceeding.
- **L5 — No security headers / CSP on the hub** (`app.py:97-126`). For an application whose
  threat model explicitly includes "the hub could inject JS", a strict CSP plus
  `Subresource-Integrity` on the static bundle at least makes a *silent* injection harder and
  gives extensions something to pin against.
- **L6 — `EmailStr` imported but unused** (`users.py:8`, field typed `str`) — no email
  validation on registration.
- **L7 — Sender Keys is implemented but unreferenced.** `senderkeys.py` is exercised only by
  its own tests; no production code imports it. That matches the Phase 13 plan; noting it so
  the module is not mistaken for an active protection.
- **L8 — `_register_uploader` matches by name and root path only** (`webrtc_server.py:727-736`)
  — the first entry with a matching name at the root gets tagged, which is wrong when a file
  with the same name exists in a subdirectory.

---

## 7. Does the system do what it claims?

> This table is the verdict **on the code as it stood on 2026-08-13**, and is left as the
> record of what the review found. It is not the current state: Phase 11.5 closed C1–C6
> and H1–H7 except H3, and the invite redesign closed H3 and M3 on 2026-08-14. For what
> holds today, and against which adversary, read draft-v5 §2 — never this table.

| Claim (draft-v4) | Verdict | Why |
|---|---|---|
| Data never transits a central server | **Yes** | WebRTC DataChannel is genuinely P2P; hub relays SDP only. Well executed. |
| Hub stores no content, no index, no chat | **Yes** | Confirmed in the schema and routers. GEK bundles are gone from the hub since Phase 12. |
| E2E encryption for all private content (files, indexes, messages) | **No** | Files: yes. Index: cleartext on the WebRTC path (M5). Chat: plaintext on the wire and at rest (Phase 13 pending). Uploads: plaintext. |
| Content unreadable by the hub | **Passive hub: yes. Active hub: no** | H3 (key substitution at invite) and C4 (pre-proof keypair-bundle fetch + PBKDF2 cracking) both yield the GEK. T3 (hub-served SPA) is a third path. |
| Node operator is sole content authority | **No** | C5a (any member overwrites files), C5b (any member seizes the GEK), C1 (anyone reads everything), M3 (operator cannot actually delete). |
| Hub admin cannot read node content | **No** | C1, and C6 for chat injection. The GEK-proof defence is real but covers one of four paths. |
| Hub admin cannot delete files | **Yes** | Deny-by-default plus pinned key. Fails closed. Correct. |
| Suspending a group blocks new connections | **No** | H4 — group revocations are dropped by the node and signaling never checks group status. |
| Immediate revocation via jti denylist | **Partial** | Works while the node stays up; volatile, and group targets ignored (H4). |
| Node IPs not persisted | **Yes** | Signaling state is in-memory. But the node's own audit DB stores peer IPs — appropriate, just worth documenting to users. |

**The one-sentence honest version:** *content is encrypted between the browser and the node
with keys the hub does not hold, and the hub is out of the data path — but the node currently
gives that content away over an unauthenticated HTTP port, chat is not encrypted at all, and
a hub that chooses to attack can obtain the group key through the key directory it controls.*

---

## 8. Are the remaining phases enough?

**No — the roadmap does not contain fixes for the findings above.** Mapping the planned work
onto what was found:

| Planned phase | Addresses | Verdict |
|---|---|---|
| 12 — Node CLI + management | M3 partially (a CLI could pin the right admin key) | Useful, not security work |
| 13.1–13.4 — Sender Keys chat encryption | Part of "chat is plaintext"; nothing else | Necessary but narrower than it looks — see below |
| 13.5 — Chat retention | Data-minimization only | Good hygiene |
| 13.6 — MNP version negotiation | L2 | Correct as planned |
| 14 — Android client | Nothing directly; adds a fourth client to keep in parity | Neutral / new risk |
| 15 — Resilience (TURN, 0-RTT) | Nothing | Optional |
| 16 — Packaging + CI | Would catch regressions; 16.4 release signing matters a lot for a native client | Underrated — promote it |
| 17 — Extension sandbox | Adds a large new attack surface | Should be last, and needs its own review |

**Nothing in the plan addresses C1–C6, H1, H2, H4, H5, H6, or H7.**

A note on Phase 13 specifically, because it is presented as *the* remaining security item:
Sender Keys protects chat from *someone who is not a group member but holds the node's disk*
(a compromised node, a seized machine, a hosting provider). It does **not** protect chat from
the node operator, because on this platform the node operator is a group member and therefore
a sender-key recipient. It also does not help if the sender keys are distributed "via
GEK-wrapped channels" as §6.6 describes — that makes them a function of the GEK, so anyone
with the GEK (C5b, H3) gets them too. Real forward secrecy requires distributing sender keys
over per-member pairwise channels (the existing `ratchet.py`) keyed to identity keys, not to
the GEK. Worth settling before writing 13.1.

**Recommendation: insert a remediation phase before Phase 12.** Suggested content, in order:

```
Phase 11.5 — Security remediation (blocking)
  11.5.1  Disable/remove the node HTTP API for private groups; bind loopback     [C1]
  11.5.2  Authenticate the node WS registration (scope + ownership + DB groups)  [C2]
  11.5.3  Unify the handshake across WebRTC/QUIC/TCP into meshbay_common         [C6]
  11.5.4  Mutual handshake proof + pk_node pinning in the client                 [C3]
  11.5.5  Per-group chat_store and per-group peer registry                       [H1]
  11.5.6  Authorize gek_bundle_store; remove GEK auto-activation                 [C5b]
  11.5.7  Upload: no overwrite, per-user quarantine, quotas, filename allowlist  [C5a, H2]
  11.5.8  Escape all HTML in the node admin UI                                   [H2]
  11.5.9  Domain-separate the admin challenge transcript                         [H5]
  11.5.10 Handle group revocation on the node; persist the denylist              [H4]
  11.5.11 Rate limits and resource caps on signaling, uploads, ffmpeg            [H6]
  11.5.12 Swarm: public groups only                                              [H7]
  11.5.13 Decide the home of keypair bundles (see §9)                            [C4]
```

Add regression tests for each: two-group chat isolation, HTTP API refuses private groups,
handshake parity across transports, upload cannot overwrite, `gek_bundle_store` rejects
non-admins.

---

## 9. If you build a native client, what changes?

Short answer: **a native client removes the single most fundamental limitation (T3) and lets
you delete the machinery that exists only to work around the browser — but it does not remove
any of the findings above, and it adds obligations of its own.**

### What a native client genuinely fixes

- **T3 becomes detectable — it does not disappear.** *(Corrected 2026-08-13; the original
  text claimed "T3 disappears. Code integrity stops depending on the hub." That was wrong.)*
  A native client downloaded from `meshbay.org` and signed with a key the hub operator holds
  relocates the trust from "the JS they serve" to "the binary they serve". What genuinely
  changes is the **shape of an attack**: in a browser it is one HTTP response, aimed at one
  user, leaving no artifact — undetectable in principle. Natively it must ship as a build,
  which is hashable, archivable and comparable between users, so targeting one person means
  handing them a different binary. That is a real gain, but it is realised **only** by the
  verification machinery — reproducible builds, published hashes, independent rebuilds
  (Phase 18.7) — not by the packaging format. Native also costs the browser sandbox, transfers
  patch velocity for WebKitGTK and every bundled dependency onto the project, and adds new
  attack surface (loopback media server, IPC bridge, updater). A **browser extension**
  distributed through Mozilla/Chrome — a channel the hub operator does not control — achieves
  most of the same benefit while keeping the sandbox. See `tmp-decisions.md`.
- **Real key storage.** OS keychain / Argon2id-encrypted local keystore, already implemented
  in `keystore.py`. Keys never leave the device, so **C4 evaporates** — no keypair bundles,
  no PBKDF2-only protection, no third-party nodes holding your private keys.
- **Real crypto.** ChaCha20-Poly1305, Argon2id at 256 MB, constant-time primitives — no
  WebCrypto ceiling. The whole `webcrypto.py` / `:aes` dual-cipher split becomes unnecessary
  (keep it only while browser clients exist).
- **Password never transmitted.** The node already authenticates with Ed25519 challenge-
  response (`nodes.py:30-80`). Clients can do the same, and then `auth_key`/`bundle_key`, the
  password split, pw_versions and the legacy migration path all go away — a large reduction in
  code and in attack surface.
- **Key verification becomes practical.** Safety numbers, TOFU pinning of `pk_node` and of
  contacts' identity keys, and persistent warnings on key change — the fix for H3. This is
  achievable in a browser but far more credible in a client the hub does not serve.

### What becomes unnecessary (delete, don't port)

| Component | Reason |
|---|---|
| `keypair_bundle_store/fetch/resp` MNP messages, `keypair_bundles` table | Keys live locally (C4) |
| `deriveAuthKey` / `deriveEncryptionKey` password split, pw_version 3 | Replaced by Ed25519 auth |
| `webcrypto.py` AES variant + `:aes` HKDF suffix | Only needed for SubtleCrypto |
| MSE streaming path (`_probe_video`, ffmpeg fMP4 remux, `stream_init/data/end`) | A native player decrypts and plays directly; the node just serves chunks |
| Node HTTP file API | Already the source of C1; native clients speak MNP |
| ~~WebRTC transport, hub signaling relay, DTLS channel binding~~ | **Correction (2026-08-13): keep these.** The original text here said "QUIC + `punch_nat()` is already validated" — that oversold a single-ISP demo. `punch_nat()` (`quic_server.py:446`) is one UDP probe to one address: no STUN client (`aioice` is pulled in by `aiortc` only), no candidate gathering, no dual-stack fallback, and it requires the client to already know its own external IP:port and to connect from a fixed source port. ICE/STUN — validated on 2 ISPs, 2 browsers, IPv4 + IPv6 + 4G CGNAT — is the only NAT traversal actually proven in this project, and it lives in the WebRTC path. A native client keeps it by running `aiortc` in Python (`createDataChannel` + `createOffer`), which preserves every native benefit, since none of them come from the transport. QUIC is retained at parity for LAN, port-forwarded and hub-less `group://` access. |
| `_bundleKey` in IndexedDB, `_sessionKeys` in sessionStorage, `_pkFromSk` | Browser-specific persistence hacks |

Note what this means for Phase 12's own accounting: **T3 reduction phases 1–3 were largely
wasted motion.** Moving GEK and keypair bundles from the hub to nodes did not remove the
hub's access (it can still forge a JWT and fetch them, C4) and it *spread* the private-key
bundles across untrusted third-party machines. A native client makes the correct answer
available: the material should live on the user's own device, not on the hub *or* on other
people's nodes.

### What is still needed regardless of client type

- **All of C1–C6, H1, H2, H4–H7.** Every one of them is server/node-side. A native client
  changes none of them.
- **Phase 13 (Sender Keys)** — still required, and still needs the pairwise-distribution
  decision above.
- **Phase 12 (Node CLI)** — arguably *more* important with native clients, since group and
  GEK management moves out of the browser.
- **Phase 16 (packaging + CI + release signing)** — becomes **critical**, not optional. Once
  users install software instead of loading a page, your update channel is the new T3. You
  need signed releases, a documented key, ideally reproducible builds, and a client that
  verifies signatures. `16.4` should be promoted alongside the remediation phase.
- **H3 / safety numbers** — the hub remains the key directory even for native clients. Out-of-
  band verification is the fix, and it is not currently scheduled anywhere.

### Suggested sequencing

> **Superseded 2026-08-13** — the roadmap was rewritten against these findings.
> See `devel-phases-next.md` for the authoritative plan. Summary:

```
Phase 11.5  Security remediation          ⛔ blocking, everything else waits
Phase 12    Hub minimization              makes "the hub cannot read" structural
Phase 13    Native desktop client         Electron (2026-08-17); reduces T3, C4 partly
Phase 14    Node CLI                      (was Phase 12)
Phase 15    Sender Keys                   (was Phase 13) — 15.0 distribution decision first
Phase 16    Android                       (was Phase 14) — reuses the Phase 13 design
Phase 17    Resilience                    (was Phase 15)
Phase 18    Packaging + CI                (was Phase 16) — signing moved into 13.9
Phase 19    Extensions                    (was Phase 17)
```

Also worth an explicit decision: **do you keep the browser client?** Supporting both means
maintaining two transports, two crypto stacks, two key-storage models, and two handshake
implementations — which is exactly how C6 and M5 came about. If the browser client stays, it
should be positioned honestly as *"convenient access with a weaker trust model — the hub can
serve you modified code"*, with the native client as the recommended path for anything
sensitive.

---

## 10. Prioritized action plan

| # | Finding | Severity | Effort | When |
|---|---|---|---|---|
| C1 | Node HTTP API serves private content unauthenticated | Critical | S | Immediately — one-line bind change unblocks, proper fix same day |
| C2 | Node WS identity spoofing → client impersonation | Critical | S | Immediately |
| C5b | Any member can seize the group GEK | Critical | S | Immediately |
| C5a | Any member can overwrite shared files | Critical | S | Immediately |
| C6 | No GEK proof on QUIC/TCP transports | Critical | M | Before any further transport work |
| C3 | No node authentication to the client | Critical | M | With C2 |
| C4 | Keypair bundles on third-party nodes, PBKDF2-only | Critical | L | Needs the design decision in §9 |
| H1 | Cross-group chat leakage | High | S | Immediately |
| H2 | Stored XSS in node admin UI | High | S | Immediately |
| H3 | Hub key substitution (T2) | High | L | Safety numbers — schedule explicitly |
| H4 | Group revocation dropped; volatile denylist | High | S | Phase 11.5 |
| H5 | Unbound Ed25519 signing oracle | High | S | Phase 11.5 |
| H6 | Unauthenticated resource exhaustion | High | M | Phase 11.5 |
| H7 | Private hashes registered in swarm | High | S | Fix before repairing the route |
| M1–M9 | See §5 | Medium | S–M | Phase 11.5 / 12 |
| L1–L8 | See §6 | Low | S | Opportunistic |

---

## 11. Conclusion

The architecture is still the right architecture. Hub-as-registrar, node-as-host,
E2E-to-the-node, GEK-per-group, node sovereignty enforced by cryptography rather than policy —
these are good decisions, and the Phase 12 work (GEK-HMAC proof, DTLS channel binding,
Ed25519 admin challenge) shows real security engineering.

The gap is between the documents and the code. Draft-v4 describes a system where every
operation passes a GEK proof, where the hub admin can read nothing, where the node operator is
sovereign, and where private content is E2E encrypted. The code implements that on one of four
paths into the node. The other three — HTTP, QUIC, TCP — are at Phase 4/7 level, and the HTTP
one hands out private files to unauthenticated callers. Meanwhile the hub retains a decisive
lever it is documented as not having: it is the key directory, and whoever controls the key
directory controls the group key.

Two concrete recommendations beyond the fix list:

1. **Make transport parity a structural invariant, not a habit.** One shared handshake
   function in `meshbay_common`, called by every transport, with a test that fails if a
   transport skips a step. Every finding in the C6/C1 family exists because a new path was
   added and the old ones stayed behind.
2. **Write down the threat model explicitly** — one page: passive hub, active hub, malicious
   node operator, malicious group member, network attacker, local attacker — and mark for each
   claim which adversary it holds against. Most of the overstatements in the current docs
   ("unreadable by other parties, even the hub") come from not distinguishing the passive hub
   from the active one. Once that page exists, the honest claims are still strong ones, and
   they will be defensible.

The security posture is recoverable, and most of the critical work is small. But the current
build should not host real private data, and the project should not advertise end-to-end
confidentiality until at least C1, C2, C3, C5 and H1–H3 are closed.