diff options
| author | Christophe Besson <cbesson@gmail.com> | 2026-09-01 19:11:20 +0200 |
|---|---|---|
| committer | Christophe Besson <cbesson@gmail.com> | 2026-09-01 19:11:20 +0200 |
| commit | 9ee9d7dfca50a64257163236f11b3cd58021963f (patch) | |
| tree | 541aca25cb8c3d64bc15e71d871cfc43e6c24c74 | |
| parent | 0808d594a371e7caea54f45a71db586160ae75ad (diff) | |
| download | meshbay-9ee9d7dfca50a64257163236f11b3cd58021963f.tar.gz | |
docs: mark M3 fixed in the third security review
Link-preview SSRF surface bounded: per-connection + node-wide rate
limit, port allowlist, connect-address re-check, decompression-bomb
guard. Summary, findings table and action plan updated; original M3
text kept for the record.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011pG75yGK3NthNfyjH74omG
| -rw-r--r-- | docs/third-review.md | 53 |
1 files changed, 38 insertions, 15 deletions
diff --git a/docs/third-review.md b/docs/third-review.md index b6f8d3c..48e7ccb 100644 --- a/docs/third-review.md +++ b/docs/third-review.md @@ -61,8 +61,9 @@ and a few of the second-review fixes did not reach every path. unconditional when a captcha is configured; the desktop client renders the widget too. - **Chat link previews** are a real SSRF surface (correctly identified as such in - the module) but the gate has gaps: no per-member rate limit, no port - restriction, and DNS rebinding is left as a documented residual. + the module) but the gate had gaps: no per-member rate limit, no port + restriction, and DNS rebinding a documented residual. **Fixed 2026-09-01** + (rate limit, port allowlist, connect-address re-check, bomb guard). - **MHP federation** trusts any registered peer hub to push directory rows and revocations, never checks the token audience, and the revocation-propagation path is a silent no-op because nodes verify only against their own hub's key. @@ -71,10 +72,9 @@ and a few of the second-review fixes did not reach every path. OpenGraph images and metadata. None of this breaks the architecture. The cryptographic core and the trust model -are unchanged and still sound. H1, H2, M1, M2 and M6 were fixed on 2026-09-01. -The link-preview SSRF (M3) should be bounded before that feature runs on an -internet-facing node; what remains beyond it (M4 federation, M5 headers, the -L-list) is hardening, not a hole. +are unchanged and still sound. H1, H2, M1, M2, M3 and M6 were fixed on +2026-09-01. What remains — M4 (federation trust), M5 (SPA security headers) and +the L-list — is hardening, not a hole. --- @@ -352,6 +352,25 @@ handler until it is at parity. Update the stale docstring. ### M3 — Chat link previews: SSRF gate has no rate limit, no port restriction, and a known rebinding hole +> **Fixed 2026-09-01.** +> - **Rate limit:** `_do_link_preview_request` is now bounded per connection (15) +> and node-wide (60) over a 60 s window; a cache hit is free, and over the +> ceiling the reply is a plain `ok: false` (bare link), not cached. +> - **Port allowlist:** `safe_url()` restricts the port to `{80, 443, 8080, +> 8443}` — every real OpenGraph page, none of the SSH/mail/DB/cache/search +> ports. Deliberately not just 80/443, to keep legitimate sites on alt-HTTP +> working. +> - **DNS rebinding:** the connection's actual peer address is re-checked +> against the public-address rule (`_reject_if_rebound`) before the body is +> read. Best-effort — a full literal-pin with cert-for-name is noted as +> remaining hardening. +> - **Decompression bomb:** `_downscale` refuses an image whose header +> dimensions exceed ~40 MP before decode. +> +> Not extended to `MEDIA_META_REQ` / `TMDB_SEARCH_REQ`: those reach a fixed +> host, so they carry a quota concern but not an SSRF one — left for a separate +> pass. + **Location:** `node/linkpreview.py`, `webrtc_server.py:3588-3636` (`_do_link_preview_request`) @@ -581,15 +600,15 @@ Against the v6 §4 claims, updated for this review: | Suspending/revoking a group blocks connections | ✅ | ✅ | — | `webrtc_offer` checks status; node drops sessions on `revoke` | | Device linking safe against the hub | ✅ | ✅ | ⚠️ browser link inherits T3 (documented) | Countersignature by a pinned device; hub holds no user keys | | Chat authenticated between members | ❌ not yet | ❌ | ❌ | Sender Keys is Phase 15; today chat is node-asserted on every transport (M2a's wire-asserted QUIC path was closed 2026-09-01) | -| Node does not emit traffic on a member's behalf | — | — | ⚠️ link previews: unthrottled, ports open, rebinding (M3) | +| Node does not emit traffic on a member's behalf | — | — | ✅ since 2026-09-01 — link previews rate-limited, ports restricted, connect-address re-checked (was M3) | | Hub cannot be used to censor content | — | — | ✅ since 2026-09-01 — `POST /v1/reports` needs auth, distinct reporters, public groups on (was H2) | | Moderator ≠ administrator | ✅ | — | — | ✅ since 2026-09-01 — `admin_patch_user` split by field (was H1) | **One-sentence version:** *the E2E story between browser and node is now genuinely mutual and covers every path — the second review's critical gaps are -closed, and H1/H2/M1/M2/M6 were fixed the day this was written — leaving the -link-preview SSRF (M3) as the one item to bound before that feature faces the -internet, and MHP federation / SPA headers as hardening.* +closed, and H1/H2/M1/M2/M3/M6 were fixed the day this was written — leaving MHP +federation trust (M4) and the missing SPA security headers (M5) as hardening, +not holes.* --- @@ -601,7 +620,7 @@ internet, and MHP federation / SPA headers as hardening.* | H2 | Unauthenticated 2-report global blocklist | High | S | ✅ **fixed 2026-09-01** — auth + distinct-reporter + rate limit + public-groups gate | | M1 | Registration CAPTCHA inert | Medium | S | ✅ **fixed 2026-09-01** — gate unconditional; desktop renders the widget | | M2 | QUIC chat: `sender_id` spoof, cross-group broadcast, sync ffmpeg | Medium | M | ✅ **fixed 2026-09-01** — handlers at WebRTC parity + `quic_enabled` off by default | -| M3 | Link-preview SSRF: no rate limit, ports open, rebinding | Medium | M | Before enabling link previews on an internet-facing node | +| M3 | Link-preview SSRF: no rate limit, ports open, rebinding | Medium | M | ✅ **fixed 2026-09-01** — rate limit + port allowlist + connect-address re-check + bomb guard | | M4 | Federation: peer over-trust, `aud` unchecked, revoke no-op | Medium | M | Before enabling MHP with any non-self peer | | M5 | No CSP / security headers on the SPA | Medium | S | Opportunistic — cheap, high value given T3 | | M6 | `add_group_member` accepts node tokens | Medium | S | ✅ **fixed 2026-09-01** — dependency → `require_user_scope` | @@ -623,7 +642,10 @@ Two structural recommendations, both echoing the second review: adversary named in the same commit.** Link previews, TMDB/MusicBrainz search, `POST /v1/reports`, `receive_directory` — each added a way for a low-privilege party (a member, an anonymous caller, a peer hub) to make the node or hub do - work or accept state, and each shipped without a bound on how much. + work or accept state, and each shipped without a bound on how much. The + 2026-09-01 fixes added the bounds to reports and link previews; + `MEDIA_META_REQ` / `TMDB_SEARCH_REQ` still want a quota, and + `receive_directory` a cap. --- @@ -643,6 +665,7 @@ required exotic capability, and none of them were architectural — they were th cost of adding six subsystems faster than the authorization model grew to cover them. -H1, H2, M1, M2 and M6 were fixed the day this review was written. Link previews -should stay off on an internet-facing node until M3 is bounded. Past that, the -honest claims are strong ones and are now largely defensible. +H1, H2, M1, M2, M3 and M6 were fixed the day this review was written. What is +left — MHP federation trust (M4), the SPA's missing security headers (M5), and +the L-list — is hardening on a build whose honest claims are now strong and +largely defensible. |