From 80f2f67db605b70ad2dbc146f48a4c4265f1b35a Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Sun, 9 Aug 2026 03:29:25 +0200 Subject: [PATCH] feat(dpop): server request dpop on all /api/* --- docs/plan/dpop.md | 10 +- src/interfaces/middleware/dpop.rs | 180 +----------------------------- 2 files changed, 7 insertions(+), 183 deletions(-) diff --git a/docs/plan/dpop.md b/docs/plan/dpop.md index 4bfd2e75..eac7fb2b 100644 --- a/docs/plan/dpop.md +++ b/docs/plan/dpop.md @@ -221,7 +221,11 @@ DPoP (RFC 9449) closes this gap by binding the session to a browser-held private - After 2-4 weeks of clean opportunistic-mode telemetry, flip default to `required` in a later release. Pre-existing app-password / legacy sessions with `dpop_jkt IS NULL` still work; they're exempt at the middleware. - **Documentation deliverable**: operator guide entry explaining the flag, the modes, the observability signals, the upgrade path. -## Gate C — Content-serve + streaming allowlist (browser-direct GETs) +## Gate C — Content-serve + streaming allowlist (SUPERSEDED by Service Worker) + +**Status: retired.** A Service Worker at `frontend/src/service-worker.ts` now intercepts every same-origin `/api/*` request the browser initiates and attaches a DPoP proof — including ``, ``, and `EventSource` streams that JS-space fetch can never touch. The middleware allowlist that this section documented has been deleted (`matches_content_path`, `is_content_serve_get`, `looks_like_uuid`, the enforcement branch, plus 9 tests). No exempt paths remain; the DPoP posture is uniform across the whole `/api/*` surface. + +Left in place for the historical rationale: Discovered during the `required`-mode rollout: some SPA endpoints are fetched by the **browser itself**, not by JS via `fetch()`. These paths have no JS in the loop to sign a DPoP proof: @@ -242,9 +246,7 @@ The allowlist exempts ONLY the missing-proof reject. Proofs that ARE sent on the **Security posture (accepted trade-off).** An attacker with a stolen cookie can GET one of these URLs *if and only if* they already know a specific 128-bit UUID. Every listing / discovery endpoint (`/api/folders/{id}/children`, `/api/photos`, `/api/files/by-hash`, `/search`, …) still requires a DPoP proof — those go through `apiFetch`. So a bare stolen cookie gives the attacker "download the exact IDs you already know" — effectively nothing without prior knowledge. Plugin log SSE additionally has admin-only AuthZ at the handler. -**Refactor pending (near-term).** The allowlist currently lives in the DPoP middleware as a slice-pattern matcher over path segments (`matches_content_path` in `src/interfaces/middleware/dpop.rs`). Cleaner: split the router — exempt routes on one sub-router without the `require_dpop_layer`, protected routes with it, merge. Declares exemption next to the route registration rather than centrally in the matcher; deletes the matcher and its 9 tests. Effort: 0.5 day. Behaviour-preserving. - -**Long-term evolution (Option B).** See Deferred to Phase 2 — signed short-lived URL tokens replace the allowlist entirely. +**Refactor considered, superseded.** A split-router variant (exempt routes on one sub-router without `require_dpop_layer`) and a signed short-lived URL token variant (Option B) were both scoped in earlier revisions. Both are moot now that the Service Worker intercepts uniformly. Option B remains in the Deferred section as a fallback if SW registration ever needs to be optional. ## Gate 10 — Observability + admin UX diff --git a/src/interfaces/middleware/dpop.rs b/src/interfaces/middleware/dpop.rs index 1b33c24c..38bc756e 100644 --- a/src/interfaces/middleware/dpop.rs +++ b/src/interfaces/middleware/dpop.rs @@ -49,91 +49,6 @@ use crate::infrastructure::services::dpop_verifier::{ use crate::interfaces::errors::AppError; use crate::interfaces::middleware::auth::CurrentUser; -/// Content-serving GETs the browser fetches directly from ``, -/// ``, `` — no JS in the loop, so -/// there's no way to attach a DPoP proof (the fetch interceptor never -/// runs). Exempt them from the "bound session missing proof" hard -/// enforcement so the SPA can still show thumbnails, play videos, and -/// download files after DPoP=required flips on. -/// -/// Security posture: an attacker with a stolen cookie can GET these -/// URLs BUT ONLY if they already know a specific 128-bit UUID. -/// Every listing / discovery endpoint (`GET /api/folders//children`, -/// `/search`, `/api/files/by-hash`, `/api/photos`, …) still requires -/// DPoP because it's called via `apiFetch`. So a bare stolen cookie -/// gives the attacker "download the exact IDs you already know" — -/// effectively nothing without prior knowledge. -/// -/// Proofs that ARE sent on these paths (e.g. an image preloader that -/// went through `apiFetch` and blob'd) still get fully verified — -/// this only bypasses the missing-proof reject, not the verifier -/// itself. -/// -/// Long-term Option B (signed short-lived URL tokens) would let us -/// remove this allowlist entirely; tracked as a separate task. -fn is_content_serve_get(method: &axum::http::Method, path: &str) -> bool { - if method != axum::http::Method::GET { - return false; - } - // Note: `path` is nest-stripped by axum (`/api` prefix removed by - // the `/api` nest), so we match against the inner segment. - // `/files/` (download / inline) - // `/files//thumbnail/` (thumbnails) - // `/folders//download` (zip download) - // `/photos//preview` (photo preview, if present) - matches_content_path(path) -} - -fn matches_content_path(path: &str) -> bool { - // Collect segments into a fixed stack buffer, then match all - // allowed shapes as slice patterns — reads left-to-right like - // the paths themselves. Any path deeper than the buffer or not - // matching a listed shape falls through to `false`. No alloc, - // no regex, no leading-slash surprises. - // - // UUID guards (`if looks_like_uuid(id)`) disambiguate - // `/files/` (content) from `/files/by-hash` (a listing - // endpoint that MUST stay behind DPoP for anti-enumeration). - // Plugin slugs aren't UUIDs so the SSE arm just accepts any - // segment there — the endpoint enforces its own admin AuthZ. - let mut buf: [&str; 6] = [""; 6]; - let mut n = 0; - for seg in path.trim_start_matches('/').split('/') { - if n == buf.len() { - return false; - } - buf[n] = seg; - n += 1; - } - match &buf[..n] { - // SSE: EventSource can't attach headers → cookie-only auth - // (RFC 9449 known gap for streaming). Same posture as content- - // serve — attacker still needs the exact target id. - ["admin", "plugins", _id, "logs", "stream"] => true, - // Content-serve GETs — browser fetches directly from - // ``, `img src`, `video src`, ``. - ["files", id] if looks_like_uuid(id) => true, - ["files", id, "thumbnail", _size] if looks_like_uuid(id) => true, - ["folders", id, "download"] if looks_like_uuid(id) => true, - ["photos", id, "preview"] if looks_like_uuid(id) => true, - _ => false, - } -} - -/// Cheap UUID-shape check: 36 chars, hyphens at positions 8, 13, 18, -/// 23, everything else hex. Not a full parse (nothing here needs the -/// bytes) — just enough to distinguish `` from named endpoints -/// like `by-hash`, `upload`, `search`. -fn looks_like_uuid(s: &str) -> bool { - if s.len() != 36 { - return false; - } - s.bytes().enumerate().all(|(i, b)| match i { - 8 | 13 | 18 | 23 => b == b'-', - _ => b.is_ascii_hexdigit(), - }) -} - /// Resolve the request's external `(scheme, host)` — what the client /// sees the URL as, which is what its DPoP proof's `htu` was built /// from. Behind a reverse proxy, the internal request scheme + @@ -221,9 +136,7 @@ pub async fn require_dpop_layer( .to_owned(); let Some(proof) = dpop_header else { match (mode, expected_jkt) { - (DpopMode::Required, Some(_)) - if !is_content_serve_get(request.method(), request.uri().path()) => - { + (DpopMode::Required, Some(_)) => { // Distinct event name: `dpop.proof_missing` means // no header was on the wire at all — no verification // happened. `dpop.verify_failed` is reserved for @@ -480,95 +393,4 @@ mod tests { ); } - // Content-serve allowlist — the paths browsers fetch directly - // from anchors / img / video without JS in the loop. Under - // `required` mode + bound session, missing-proof must NOT reject - // these or the SPA breaks (downloads, thumbnails, video streaming - // all silently 401). - - fn get(path: &str) -> bool { - is_content_serve_get(&axum::http::Method::GET, path) - } - fn post(path: &str) -> bool { - is_content_serve_get(&axum::http::Method::POST, path) - } - - #[test] - fn content_serve_matches_file_download() { - assert!(get("/files/8f8e4390-1234-4a5b-8c9d-abcdef012345")); - } - - #[test] - fn content_serve_matches_thumbnail() { - assert!(get( - "/files/8f8e4390-1234-4a5b-8c9d-abcdef012345/thumbnail/icon" - )); - assert!(get( - "/files/8f8e4390-1234-4a5b-8c9d-abcdef012345/thumbnail/large" - )); - } - - #[test] - fn content_serve_matches_folder_zip() { - assert!(get( - "/folders/8f8e4390-1234-4a5b-8c9d-abcdef012345/download" - )); - } - - #[test] - fn content_serve_matches_photo_preview() { - assert!(get("/photos/8f8e4390-1234-4a5b-8c9d-abcdef012345/preview")); - } - - #[test] - fn content_serve_rejects_non_get() { - // Attacker with stolen cookie can't mutate — POST/DELETE - // to the SAME url still requires DPoP. - assert!(!post("/files/8f8e4390-1234-4a5b-8c9d-abcdef012345")); - assert!(!is_content_serve_get( - &axum::http::Method::DELETE, - "/files/8f8e4390-1234-4a5b-8c9d-abcdef012345" - )); - } - - #[test] - fn content_serve_rejects_listing_endpoints() { - // Discovery endpoints (children, by-hash, search) MUST NOT be - // allowlisted — that's the whole security argument. Attacker - // needs to already know the UUID; can't enumerate. - assert!(!get( - "/folders/8f8e4390-1234-4a5b-8c9d-abcdef012345/children" - )); - assert!(!get("/files/by-hash")); - assert!(!get("/search")); - assert!(!get("/auth/me")); - } - - #[test] - fn content_serve_rejects_root_or_id_only() { - assert!(!get("/")); - assert!(!get("/files")); - assert!(!get("/folders")); - } - - #[test] - fn content_serve_matches_plugin_log_sse_stream() { - // Server-Sent Events endpoint. Browser uses EventSource, - // which can't attach custom headers — cookie-only auth is - // the RFC 9449 known gap for streaming. Allowlisted. - assert!(get("/admin/plugins/com.example.hello/logs/stream")); - assert!(get("/admin/plugins/some-slug/logs/stream")); - } - - #[test] - fn content_serve_rejects_other_admin_plugin_endpoints() { - // Only the SSE stream is allowlisted. Every other plugin - // endpoint (list, install, uninstall, config) stays behind - // DPoP for anti-enumeration and mutation protection. - assert!(!get("/admin/plugins")); - assert!(!get("/admin/plugins/some-slug")); - assert!(!get("/admin/plugins/some-slug/logs")); - assert!(!get("/admin/plugins/some-slug/config")); - assert!(!post("/admin/plugins/some-slug/logs/stream")); - } }