Commit graph

4 commits

Author SHA1 Message Date
mithral
6e04e957b8 Rate limit every surface that checks a password
The limiter covered only /api/v1/auth/login. Jellyfin's AuthenticateByName
and Subsonic's authenticate both called auth.Authenticate with no limiting,
so an attacker brute-forced through those instead: the same password, the
same argon2, no ceiling.

The limiter moves from auth.Handlers onto *Authenticator, which all three
surfaces already hold, so they share one budget and splitting guesses
across surfaces gains nothing. main.go builds one authenticator and hands
it to both compat layers, so the sharing is real rather than intended.

Subsonic needs care: it has no login step, authenticate runs on every
request, so a naive check would charge playback against the login budget
and lock a listener out mid-album. The limiter is consulted only when a
credential was actually supplied, and a success or an uncredentialed
request spends nothing.

Verified red-green on all three: removing the Jellyfin gate fails
TestAuthenticateByNameIsRateLimited with 401 instead of 429; removing the
Subsonic credential guard fails
TestSuccessAndUncredentialedDoNotConsumeTheBudget; neutering AllowLogin or
ResetLogin fails the auth tests.

Known gap, unchanged: the limiter is in-process, so it resets on restart
and does not span replicas. Fine for one binary against one SQLite file.
2026-10-12 00:57:56 -04:00
mithral
87c17d4854 Make the login limiter correct behind a reverse proxy
Some checks failed
CI / ci (push) Has been cancelled
ClientIP used r.RemoteAddr and ignored X-Forwarded-For. That was right
about not trusting the header from an untrusted peer, but it is only half
the design: behind a proxy every request arrives from the proxy, so the
limiter key in handlers.go was "<proxy-ip>|<username>" for every client on
the internet. One attacker locked out a username for everyone, an attacker
rotating usernames never hit a limit, and sessions.ip recorded the proxy
so the device list was useless.

- VELLUM_TRUSTED_PROXIES is a CIDR list. X-Forwarded-For is honoured only
  when RemoteAddr is inside it, and the chain is walked right-to-left so
  the first address that is not itself a trusted proxy wins. Everything to
  its left was appended by a client we do not control, so a spoofed
  leftmost entry is discarded.
- VELLUM_PUBLIC_URL is the external origin. CookieSecure now defaults to
  true when it is https://, so a TLS deployment that forgets
  VELLUM_COOKIE_SECURE no longer ships a plaintext session cookie.
- A malformed CIDR fails the boot rather than being skipped: silently
  dropping a proxy network widens the trust boundary, which is the exact
  failure this setting exists to prevent.

ClientIP became a method on Authenticator so it can see the trusted set,
which is why handlers.go and main.go changed as well.

Verified red-green: neutering the trusted-peer path fails five ClientIP
tests, including the spoofed-leftmost case.

Known gap, pre-existing and not fixed here: this limiter still covers only
/api/v1/auth/login. Jellyfin AuthenticateByName and Subsonic auth call
auth.Authenticate with no limiting, so the same brute force is available
through those two surfaces.
2026-10-12 00:45:27 -04:00
mithral
00c1c0d619 Wave 2: correctness and security
TouchSession throttle (internal/auth)
- Middleware wrote sessions.last_seen on EVERY authenticated request, taking
  SQLite's single write lock each time. For media that is severe: one seek in
  an audio file fires several Range requests, each taking the write lock and
  competing with the scanner and progress saves.
- Now touched at most once per 5 minutes, decided from the LastSeen already
  loaded by SessionByToken, so no extra query. A zero interval degrades to the
  old per-request behaviour, so a hand-built Authenticator cannot silently stop
  updating.
- Verified end to end with an AFTER UPDATE trigger: 20 requests now cause 0
  session writes, down from 20.

Response writer forwarding (internal/httpapi)
- statusRecorder wrapped every ResponseWriter but implemented only Write and
  WriteHeader, so it did not implement http.Flusher. That silently disabled
  early flushing (progressive audio/video) and io.ReaderFrom, which is what
  lets io.Copy use sendfile for multi-GB files. Now forwards Flush, Hijack and
  ReadFrom, and WriteHeader is idempotent.

Security headers (internal/httpapi)
- Added nosniff, X-Frame-Options: DENY, Referrer-Policy and a CSP. Outermost in
  the chain so a recovered panic's 500 still carries them.
- CORRECTION to an earlier claim: I said a malicious EPUB was stored XSS.
  That was wrong. epub.js sets iframe sandbox to "allow-same-origin" and only
  appends "allow-scripts" when settings.allowScriptedContent is true, which
  defaults to false and Reader.svelte never sets. EPUB content cannot execute
  script. The CSP is still worth having as defence in depth, but that specific
  justification does not hold.

Final flush on teardown (AudiobookPlayer, Reader)
- The last playback/reading position was saved with a fire-and-forget fetch in
  onDestroy, which the browser cancels on navigate-away — losing exactly the
  thing worth persisting. Now persisted via fetch with keepalive: true, wired
  to pagehide and visibilitychange(hidden), latched so the overlapping events
  fire one request, and reset when the page becomes visible again so a bfcache
  restore still flushes later. sendBeacon was not usable: it is POST-only and
  progress is PUT-only.
- Also removed the queueMicrotask track-swap race in AudiobookPlayer: play() is
  now driven by onLoadedMetadata once the new src has actually loaded.

Honest readers (Reader, ComicReader)
- registry.go advertises .mobi/.azw3/.fb2 under ebooks, but every ebooks item
  was handed to epub.js, which cannot parse them. Now only .epub uses epub.js;
  other containers show a message naming the format plus a download link. Items
  stay catalogued and downloadable rather than silently vanishing.
- .cbr/.rar comics say the archive format is unsupported instead of surfacing
  the server's raw 422 (the server reads zip only).

Verified in a real browser, not just compiled: all six reader surfaces render
(epub iframe, cbz images, audio element, photo, pdf canvas) with zero CSP
violations; .mobi shows the honest message with no epub.js iframe. gofmt/vet
clean, 14 packages pass, svelte-check 0/0.

Known issue, NOT introduced here and NOT fixed: the EPUB reader logs
"Error: No Element Provided" x3 and a TypeError in epub.js injectIdentifier
(doc.getElementsByTagName("head")[0] undefined). Proven pre-existing by A/B
against c50b3cf, and not caused by the test file (spec-correct XHTML) or by
caching (cold browser). Non-fatal: the book still renders. Needs its own
investigation.
2026-10-11 13:31:34 -04:00
mithral
9772e55c73 Phase 1: accounts, sessions, per-library grants
Multi-user from the first migration, as planned.

- auth: argon2id password hashing (PHC encoded, parameters travel with the
  hash), opaque session tokens stored only as SHA-256, login rate limiting,
  and account enumeration resistance (unknown user hashes anyway)
- sessions: cookie or Bearer auth, HttpOnly + SameSite=Lax, revocation on
  password change
- grants: library_grants is the single access-control chokepoint. Admins see
  every library; everyone else sees exactly their can_read grants
- bootstrap: VELLUM_ADMIN_USER/PASSWORD create the first admin only on an
  empty database. No default password is ever invented.
- httpapi: mounts modules on a shared mux, session middleware innermost
- httpx: shared JSON helpers, strict body decoding

Gate: two users see different libraries. Tests cover that, plus session
expiry, token-at-rest, grant upsert, and the full login/me/logout flow.
2026-10-09 16:31:24 -04:00