diff --git a/CHANGELOG.md b/CHANGELOG.md index 811fd9d..cc348e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,13 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ### Security +- **One stranger behind a shared proxy no longer locks everyone out of + signing in** (audit A1, A8, A9): sign-in counts failures per address and + username (a hundred per address alone), a right password no longer + counts, a link's page is limited per link, a reset request per address + and name, and an IPv6 client is one per /64. With every request coming + from one private address with `X-Forwarded-For` and `TRUSTED_PROXY_CIDR` + empty, the log says what to set. Refused sign-ins are logged. - **The TLS private key no longer has to be readable by Postfix** (audit I1): the guide gave it to group 10998 for the panel's backup, and `postfix` is in that group, so every Postfix daemon - an smtpd talking to diff --git a/docs/guide.md b/docs/guide.md index 0d92f6f..413c7b9 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -183,6 +183,19 @@ docker compose up -d The nginx/Caddy/Traefik fragments already include `docker compose up -d` — skip this if you ran one of those. +With Apache on the host, the panel sees every visitor at the Docker +network's gateway. Tell it that address is your proxy, so that each visitor +has a sign-in limit of their own, and recreate the container: + +```sh +docker network inspect "$(basename "$PWD")_default" -f '{{(index .IPAM.Config 0).Gateway}}' +``` + +Put what it prints into `.env` as `TRUSTED_PROXY_CIDR=` and run `docker +compose up -d` again. Until then the panel's log says, after the first +requests, that every visitor shares one limit and what to set; Server › +Health shows the same under *Your address*. + **Get the setup URL** — open it in a browser to create the admin account (see [Initial setup](#initial-setup)): diff --git a/docs/security-audit-2026-10-17.md b/docs/security-audit-2026-10-17.md index 782094f..8404f3f 100644 --- a/docs/security-audit-2026-10-17.md +++ b/docs/security-audit-2026-10-17.md @@ -762,21 +762,31 @@ the argument. cannot read `/etc/postfix/tls/privkey.pem`; security.md § Output and process lists the three groups and who is in each. Rollout: `chgrp 10997 /root/selfpost-deploy/certs/privkey.pem` and the same in - `cert-deploy-hook.sh` on the test server. — **Opus**, 2.5 h. + `cert-deploy-hook.sh` on the test server. — **Opus**, 2.5 h. *Done + 2026-10-17:* the e2e stage gives its key to 10997 at 0640 when the runner + is root and `testFullBackupRestore` checks both uids; the test server's + key at the rollout. ### Stage 3 — limits and defaults (~7 h) -- [ ] **A1.** (a) Compose and guide set `TRUSTED_PROXY_CIDR` for the +- [x] **A1.** (a) Compose and guide set `TRUSTED_PROXY_CIDR` for the Apache-on-host scenario (the Docker gateway, with the command that prints it) and the panel logs a warning at start when the first N requests all come from one private peer carrying XFF and the list is empty; (b) the login limiter also keyed by username with its own window, the link-page limiters by token hash; (c) successful sign-ins not counted. security.md records (a)-(c). Done when the reader's test passes with two clients - behind one untrusted peer. — **Opus**, 4 h. -- [ ] **A8.** One log line per refused sign-in or lockout, rate-limited per - key. — **Sonnet**, 0.5 h. -- [ ] **A9.** IPv6 keyed by /64. — **Sonnet**, 0.5 h. + behind one untrusted peer. — **Opus**, 4 h. *Done 2026-10-17:* (a) as a + runtime warning (`proxyWatch`) and the guide's install step with the + command - the shipped compose cannot know the gateway, so it is not set + there; (b) per address and username, per link, the address alone at ten + times that; (c) a right password gives its attempt back. + TestLoginBehindOneUntrustedProxyKeepsOthersIn, TestLoginSuccessesDoNotCount, + TestProxyWatchAndRefusalsAreLogged. +- [x] **A8.** One log line per refused sign-in or lockout, rate-limited per + key. — **Sonnet**, 0.5 h. *Done 2026-10-17* (`refusalLog`). +- [x] **A9.** IPv6 keyed by /64. — **Sonnet**, 0.5 h. *Done 2026-10-17:* + TestIPv6IsLimitedPerSlash64. - [ ] **A3.** After five unsolicited reset links in a day, one mail saying further requests are ignored for 24 h, then silence. — **Opus**, 1.5 h. - [ ] **I6.** `/healthz` answered only to loopback and trusted-proxy peers; diff --git a/docs/security.md b/docs/security.md index 435abed..db7bd18 100644 --- a/docs/security.md +++ b/docs/security.md @@ -89,7 +89,8 @@ The panel is exposed to the internet — the items below are **not optional**. - Token ≥128 bits (`crypto/rand`); mirrored to `/data/setup-token`. - Token lifetime — **10 minutes**; after expiry, or after a restart with setup unfinished, it is regenerated and logged again. -- Rate limiting on `/setup/` per IP, separate from login. +- Rate limiting on `/setup/` per client address (an IPv6 one per + /64), separate from login. - Token comparison is **constant-time** (`subtle.ConstantTimeCompare`). - Failed attempts do **not** invalidate the token early (protects setup from being DoS-ed). @@ -123,7 +124,18 @@ The panel is exposed to the internet — the items below are **not optional**. ### Authentication and sessions -- Rate limiting on login (per IP, with lockout or delay). The Account form's +- Rate limiting on login: ten failures in 15 minutes per client address and + username, and a hundred per address alone; a right password gives its + attempt back, so only failures count. An IPv6 client is one address per + /64. Behind a reverse proxy the panel does not trust (`TRUSTED_PROXY_CIDR` + empty) every visitor has the proxy's address: a stranger there can then + lock one name, or spray a hundred, but not lock everyone out at the + per-name size (until `2.14.4` the one key was the address, successes + counted, and ten requests of any body locked every administrator out - + security audit 2026-10-17, A1, A9). When the first requests after a start + all come from one private peer with `X-Forwarded-For` and no proxy is + trusted, the log says so with the value to set. A refused sign-in and a + lockout are logged, five lines per address in 15 minutes (A8). The Account form's current-password check has a limit of its own, ten in 15 minutes per user, so guesses there do not lock the address out of signing in (`2.5.4`). - An unknown username is refused in the time a wrong password is: the sign-in @@ -163,8 +175,10 @@ The panel is exposed to the internet — the items below are **not optional**. token. The request form answers the same for every name and sends the mail after answering, so neither the words nor the time tell whether a name exists or is confirmed; requests are limited to five in 15 minutes per - client address, mails to five an hour per account, and a link's page to 30 - requests in 15 minutes per client address. A link's page changes nothing + client address and username (fifty per address alone), mails to five an + hour per account, and a link's page to 30 requests in 15 minutes per link + (300 per address alone), so one stranger behind a shared proxy cannot stop + the others asking for or opening a link (`2.14.4`; audit A1). A link's page changes nothing on `GET` (a mail scanner opens links): the button does. Setting a password from a link ends every session of the account and every other reset link. Changing the account e-mail asks for the current password, as the password diff --git a/internal/web/auth/auth.go b/internal/web/auth/auth.go index 5621559..0efa808 100644 --- a/internal/web/auth/auth.go +++ b/internal/web/auth/auth.go @@ -31,16 +31,28 @@ type Module struct { sessions *sessionStore setup *setupManager - loginLimiter *rateLimiter - setupLimiter *rateLimiter - passwordLimiter *rateLimiter - // resetLimiter counts the requests for a reset link per client address, - // linkLimiter the requests a link's page gets per client address, and - // mailLimiter the links mailed to one user's address (links.go). - resetLimiter *rateLimiter - linkLimiter *rateLimiter - mailLimiter *rateLimiter - mailer Mailer + // loginLimiter counts failed sign-ins per address and username, + // loginAddrLimiter per address alone at ten times that; setupLimiter + // counts setup requests per address. Neither address bucket is one a + // stranger behind the same proxy fills for everyone at the per-user size + // (limitkeys.go). + loginLimiter *rateLimiter + loginAddrLimiter *rateLimiter + setupLimiter *rateLimiter + passwordLimiter *rateLimiter + // resetLimiter counts the requests for a reset link per address and + // username, resetAddrLimiter per address alone; linkLimiter the requests + // a link's page gets per link, linkAddrLimiter per address at a high + // ceiling; mailLimiter the links mailed to one user's address (links.go). + resetLimiter *rateLimiter + resetAddrLimiter *rateLimiter + linkLimiter *rateLimiter + linkAddrLimiter *rateLimiter + mailLimiter *rateLimiter + + proxyWatch proxyWatch + refusals *refusalLog + mailer Mailer // mailing counts the links being mailed after their answer, so a stop // waits for them (Wait) before the database closes. mailing sync.WaitGroup @@ -56,19 +68,23 @@ func New(st *store.Store, cfg Config, v *view.Engine, setupTokenPath string) *Mo idleDays = 7 } m := &Module{ - store: st, - cfg: cfg, - view: v, - sessions: newSessionStore(st, time.Duration(idleDays)*24*time.Hour), - setupLimiter: newRateLimiter(10, time.Minute), - loginLimiter: newRateLimiter(10, 15*time.Minute), + store: st, + cfg: cfg, + view: v, + sessions: newSessionStore(st, time.Duration(idleDays)*24*time.Hour), + setupLimiter: newRateLimiter(10, time.Minute), + loginLimiter: newRateLimiter(10, 15*time.Minute), + loginAddrLimiter: newRateLimiter(100, 15*time.Minute), // The Account form's current-password check has a bucket of its // own, per user: ten wrong guesses there used to lock the address // out of signing in (audit 2026-10-12, L15), and the one guessing // is whoever holds that user's session, wherever from. - passwordLimiter: newRateLimiter(10, 15*time.Minute), - resetLimiter: newRateLimiter(5, 15*time.Minute), - linkLimiter: newRateLimiter(30, 15*time.Minute), + passwordLimiter: newRateLimiter(10, 15*time.Minute), + resetLimiter: newRateLimiter(5, 15*time.Minute), + resetAddrLimiter: newRateLimiter(50, 15*time.Minute), + linkLimiter: newRateLimiter(30, 15*time.Minute), + linkAddrLimiter: newRateLimiter(300, 15*time.Minute), + refusals: newRefusalLog(), // Five links an hour to one address, of either kind: three resets // asked for and a couple of confirmations is ordinary use, a mailbox // full of them is not. @@ -77,6 +93,10 @@ func New(st *store.Store, cfg Config, v *view.Engine, setupTokenPath string) *Mo } m.setupLimiter.startSweeper() m.loginLimiter.startSweeper() + m.loginAddrLimiter.startSweeper() + m.resetAddrLimiter.startSweeper() + m.linkAddrLimiter.startSweeper() + m.refusals.limiter.startSweeper() m.passwordLimiter.startSweeper() m.resetLimiter.startSweeper() m.linkLimiter.startSweeper() diff --git a/internal/web/auth/handlers.go b/internal/web/auth/handlers.go index 27b4f41..4feb89c 100644 --- a/internal/web/auth/handlers.go +++ b/internal/web/auth/handlers.go @@ -101,8 +101,17 @@ func (m *Module) renderLogin(w http.ResponseWriter, status int, formErr string) m.view.Render(w, status, "login", view.NewLogin(m.cfg.Hostname, formErr)) } +// submitLogin checks a sign-in. Its limits are per address and username, +// and per address alone at ten times that, and only a failure counts: one +// stranger behind the proxy everyone shares no longer locks every +// administrator out, nor does a day of signing in (security audit +// 2026-10-17, A1). A refusal and a lockout are logged, a few lines per +// address in a window (A8). func (m *Module) submitLogin(w http.ResponseWriter, r *http.Request) { - if !m.loginLimiter.Allow(clientIP(r, m.trustedProxies)) { + m.proxyWatch.see(r, m.trustedProxies) + addr := limitAddr(r, m.trustedProxies) + if !m.loginAddrLimiter.Allow(addr) { + m.refusals.printf(addr, "panel: sign-in from %s refused: too many attempts from the address", addr) m.renderLogin(w, http.StatusTooManyRequests, "Too many attempts. Please wait and try again.") return } @@ -112,6 +121,17 @@ func (m *Module) submitLogin(w http.ResponseWriter, r *http.Request) { } username := strings.TrimSpace(r.PostFormValue("username")) password := r.PostFormValue("password") + key := userKey(addr, username) + if !m.loginLimiter.Allow(key) { + m.loginAddrLimiter.Refund(addr) // counted once, against the name + m.refusals.printf(addr, "panel: sign-in as %q from %s refused: too many attempts", logName(username), addr) + m.renderLogin(w, http.StatusTooManyRequests, "Too many attempts. Please wait and try again.") + return + } + refused := func() { + m.refusals.printf(addr, "panel: sign-in as %q from %s refused: wrong username or password", logName(username), addr) + m.renderLogin(w, http.StatusUnauthorized, "Invalid username or password.") + } user, err := m.store.GetUserByUsername(username) if err != nil { @@ -122,14 +142,17 @@ func (m *Module) submitLogin(w http.ResponseWriter, r *http.Request) { // a wrong password is: the answer's delay told which names exist // (audit 2026-10-12, M8). _ = bcrypt.CompareHashAndPassword(noSuchUserHash(), []byte(password)) - m.renderLogin(w, http.StatusUnauthorized, "Invalid username or password.") + refused() return } if err := bcrypt.CompareHashAndPassword([]byte(user.PasswordHash), []byte(password)); err != nil { - m.renderLogin(w, http.StatusUnauthorized, "Invalid username or password.") + refused() return } + // A right password is no guess: its attempts are given back. + m.loginLimiter.Refund(key) + m.loginAddrLimiter.Refund(addr) // The session the browser presents, if any, ends here: a sign-in issues // a new token and leaves no other behind it - one planted before the @@ -195,7 +218,8 @@ func (m *Module) HandleLogout(w http.ResponseWriter, r *http.Request) { // HandleSetup serves the one-time administrator creation flow at // /setup/ (security.md). func (m *Module) HandleSetup(w http.ResponseWriter, r *http.Request) { - if !m.setupLimiter.Allow(clientIP(r, m.trustedProxies)) { + m.proxyWatch.see(r, m.trustedProxies) + if !m.setupLimiter.Allow(limitAddr(r, m.trustedProxies)) { http.Error(w, "too many requests", http.StatusTooManyRequests) return } diff --git a/internal/web/auth/limitkeys.go b/internal/web/auth/limitkeys.go new file mode 100644 index 0000000..93e32e8 --- /dev/null +++ b/internal/web/auth/limitkeys.go @@ -0,0 +1,123 @@ +package auth + +import ( + "crypto/sha256" + "encoding/hex" + "net" + "net/http" + "strings" + "sync" + "time" + "unicode" +) + +// The keys the public routes are limited by (security audit 2026-10-17, A1, +// A8, A9). +// +// A client's address is the one clientIP reads, an IPv6 address cut to its +// /64: a client with a /64 to itself rotates addresses inside it at will +// and never met a per-address limit (A9). Behind a reverse proxy the panel +// does not trust, every visitor has the proxy's address; so no limit that a +// stranger can fill is keyed on the address alone at a size that stops the +// others: sign-in counts per address and username, the address alone only +// at a ceiling ten times higher; a link page counts per link; and a right +// password gives its attempt back (A1). proxyWatch says in the log when the +// shared-address case is what the panel sees. + +// limitAddr is the client's address as the limiters key it. +func limitAddr(r *http.Request, trusted []*net.IPNet) string { + host := clientIP(r, trusted) + ip := net.ParseIP(host) + if ip == nil || ip.To4() != nil { + return host + } + return (&net.IPNet{IP: ip.Mask(net.CIDRMask(64, 128)), Mask: net.CIDRMask(64, 128)}).String() +} + +// userKey is a limiter key for one username tried from one address. The +// name is folded as the store folds it (usernames are matched without +// regard to case). +func userKey(addr, username string) string { + return addr + "|" + strings.ToLower(strings.TrimSpace(username)) +} + +// tokenKey is a limiter key for one link: its hash, so a limiter's map holds +// no token. +func tokenKey(token string) string { + sum := sha256.Sum256([]byte(token)) + return "link:" + hex.EncodeToString(sum[:8]) +} + +// logName is a typed username as a log line may hold it: printable ASCII, +// cut. It is the stranger's text. +func logName(s string) string { + s = strings.Map(func(r rune) rune { + if r > unicode.MaxASCII || !unicode.IsPrint(r) { + return '?' + } + return r + }, s) + if len(s) > 64 { + s = s[:64] + "…" + } + return s +} + +// proxyWatchRequests is how many requests in a row must come from one +// private peer with X-Forwarded-For, while no proxy is trusted, before the +// panel says so. +const proxyWatchRequests = 20 + +// proxyWatch looks at the first requests after the start. When they all +// come from one private (or loopback) peer that carries X-Forwarded-For and +// TRUSTED_PROXY_CIDR is empty, every visitor shares that peer's sign-in +// limits: it says so once, with the value to set. Any other request - no +// header, another peer, a public one - ends the watch silently. +type proxyWatch struct { + mu sync.Mutex + peer string + n int + done bool +} + +func (w *proxyWatch) see(r *http.Request, trusted []*net.IPNet) { + if len(trusted) > 0 { + return + } + w.mu.Lock() + defer w.mu.Unlock() + if w.done { + return + } + peer, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + peer = r.RemoteAddr + } + ip := net.ParseIP(peer) + if ip == nil || !(ip.IsPrivate() || ip.IsLoopback()) || r.Header.Get("X-Forwarded-For") == "" || + (w.peer != "" && w.peer != peer) { + w.done = true + return + } + w.peer = peer + if w.n++; w.n >= proxyWatchRequests { + w.done = true + logf("panel: the last %d requests all came from %s with X-Forwarded-For, which is ignored while TRUSTED_PROXY_CIDR is empty: "+ + "every visitor shares one sign-in limit. If %s is your reverse proxy, set TRUSTED_PROXY_CIDR=%s (guide § Environment variables)", + w.n, peer, peer, peer) + } +} + +// refusalLog lets a refused sign-in or a lockout be logged a few times per +// address in a window, so the log shows a brute force without becoming one. +type refusalLog struct{ limiter *rateLimiter } + +func newRefusalLog() *refusalLog { + return &refusalLog{limiter: newRateLimiter(5, 15*time.Minute)} +} + +func (l *refusalLog) printf(addr, format string, args ...any) { + if l.limiter.Allow(addr) { + logf(format, args...) + } +} diff --git a/internal/web/auth/limitkeys_test.go b/internal/web/auth/limitkeys_test.go new file mode 100644 index 0000000..f14343f --- /dev/null +++ b/internal/web/auth/limitkeys_test.go @@ -0,0 +1,116 @@ +package auth + +import ( + "log" + "net/http" + "net/http/httptest" + "os" + "strconv" + "strings" + "testing" +) + +// Behind a reverse proxy the panel does not trust, every visitor has the +// proxy's address. A stranger there who guesses one name, or sprays many, +// must not lock out an administrator signing in from behind the same proxy +// (security audit 2026-10-17, A1: the reader's test, two clients behind one +// untrusted peer). +func TestLoginBehindOneUntrustedProxyKeepsOthersIn(t *testing.T) { + m := moduleWithAdmin(t) + const proxy = "172.18.0.1:40000" + for i := 0; i < 15; i++ { + postLogin(m, proxy, "admin2", "guess") + } + if rec := postLogin(m, proxy, "admin", testPassword); rec.Code != http.StatusSeeOther { + t.Fatalf("the administrator behind the same proxy = %d after a stranger's guesses at another name", rec.Code) + } + // Guessing the administrator's own name locks that name from there, + // not the others. + for i := 0; i < 10; i++ { + postLogin(m, proxy, "admin", "guess") + } + if rec := postLogin(m, proxy, "admin", testPassword); rec.Code != http.StatusTooManyRequests { + t.Errorf("the guessed name after ten failures = %d, want 429", rec.Code) + } + // Spraying names: the address alone is stopped at a hundred failures. + m = moduleWithAdmin(t) + for i := 0; i < 100; i++ { + postLogin(m, "203.0.113.50:1", "spray"+strings.Repeat("x", i%40)+string(rune('a'+i%26)), "guess") + } + if rec := postLogin(m, "203.0.113.50:1", "admin", testPassword); rec.Code != http.StatusTooManyRequests { + t.Errorf("after a hundred failures from one address = %d, want 429", rec.Code) + } +} + +// A right password is no guess: a day of signing in uses up no limit +// (security audit 2026-10-17, A1 (c)). +func TestLoginSuccessesDoNotCount(t *testing.T) { + m := moduleWithAdmin(t) + for i := 0; i < 25; i++ { + if rec := postLogin(m, "203.0.113.7:1", "admin", testPassword); rec.Code != http.StatusSeeOther { + t.Fatalf("sign-in %d = %d", i+1, rec.Code) + } + } +} + +// An IPv6 client is one client per /64: rotating addresses inside it +// gives no fresh limit (security audit 2026-10-17, A9). +func TestIPv6IsLimitedPerSlash64(t *testing.T) { + m := moduleWithAdmin(t) + for i := 0; i < 10; i++ { + postLogin(m, "[2001:db8:1:2::"+strconv.Itoa(100+i)+"]:443", "admin", "guess") + } + if rec := postLogin(m, "[2001:db8:1:2:ffff::1]:443", "admin", testPassword); rec.Code != http.StatusTooManyRequests { + t.Errorf("another address in the same /64 = %d, want 429", rec.Code) + } + if rec := postLogin(m, "[2001:db8:1:3::1]:443", "admin", testPassword); rec.Code != http.StatusSeeOther { + t.Errorf("another /64 = %d, want a sign-in", rec.Code) + } + if got := limitAddr(httptest.NewRequest(http.MethodGet, "/", nil), nil); got != "192.0.2.1" { + t.Errorf("an IPv4 address is keyed as %q", got) + } +} + +// When every request comes from one private peer with X-Forwarded-For and no +// proxy is trusted, the log says so once, with the value to set; a request +// without the header ends the watch (security audit 2026-10-17, A1 (a)). +// Refusals are logged, a few per address (A8). +func TestProxyWatchAndRefusalsAreLogged(t *testing.T) { + var buf strings.Builder + log.SetOutput(&buf) + t.Cleanup(func() { log.SetOutput(os.Stderr) }) + + var w proxyWatch + req := func(xff bool) *http.Request { + r := httptest.NewRequest(http.MethodGet, "/login", nil) + r.RemoteAddr = "172.18.0.1:5000" + if xff { + r.Header.Set("X-Forwarded-For", "198.51.100.4") + } + return r + } + for i := 0; i < proxyWatchRequests+5; i++ { + w.see(req(true), nil) + } + if n := strings.Count(buf.String(), "set TRUSTED_PROXY_CIDR=172.18.0.1"); n != 1 { + t.Errorf("the warning was logged %d times:\n%s", n, buf.String()) + } + buf.Reset() + var quiet proxyWatch + quiet.see(req(false), nil) + for i := 0; i < proxyWatchRequests+5; i++ { + quiet.see(req(true), nil) + } + if buf.Len() != 0 { + t.Errorf("a watch ended by a request without the header logged:\n%s", buf.String()) + } + + m := moduleWithAdmin(t) + for i := 0; i < 8; i++ { + postLogin(m, "203.0.113.9:1", "ad\x1bmin", "guess") + } + got := buf.String() + if n := strings.Count(got, "refused: wrong username or password"); n != 5 || !strings.Contains(got, `sign-in as "ad?min" from 203.0.113.9`) { + t.Errorf("refusals logged %d times (want 5), or the name unescaped:\n%s", n, got) + } +} diff --git a/internal/web/auth/links.go b/internal/web/auth/links.go index 27cb5eb..48ad296 100644 --- a/internal/web/auth/links.go +++ b/internal/web/auth/links.go @@ -132,8 +132,16 @@ func (m *Module) HandleReset(w http.ResponseWriter, r *http.Request) { case http.MethodGet: m.view.Render(w, http.StatusOK, "login-reset", view.NewReset(m.cfg.Hostname, "")) case http.MethodPost: - if !m.resetLimiter.Allow(clientIP(r, m.trustedProxies)) { + // Per address and username, and per address alone at ten times + // that: one stranger behind a shared proxy cannot stop everyone + // asking for a link (security audit 2026-10-17, A1). + m.proxyWatch.see(r, m.trustedProxies) + addr := limitAddr(r, m.trustedProxies) + tooMany := func() { m.view.Render(w, http.StatusTooManyRequests, "login-reset", view.NewReset(m.cfg.Hostname, "Too many attempts. Please wait and try again.")) + } + if !m.resetAddrLimiter.Allow(addr) { + tooMany() return } if err := r.ParseForm(); err != nil { @@ -141,6 +149,10 @@ func (m *Module) HandleReset(w http.ResponseWriter, r *http.Request) { return } username := strings.TrimSpace(r.PostFormValue("username")) + if !m.resetLimiter.Allow(userKey(addr, username)) { + tooMany() + return + } m.later(func() { m.sendReset(username) }) http.Redirect(w, r, "/login?reset=sent", http.StatusSeeOther) default: @@ -201,15 +213,26 @@ func linkToken(path, prefix string) string { const deadLink = "This link has expired or was already used." +// allowLink is the limit of a link's page: per link, so that nobody can use up +// another's, and per address at a ceiling high enough for everyone behind one +// proxy (security audit 2026-10-17, A1). +func (m *Module) allowLink(r *http.Request, token string) bool { + m.proxyWatch.see(r, m.trustedProxies) + if !m.linkAddrLimiter.Allow(limitAddr(r, m.trustedProxies)) { + return false + } + return token == "" || m.linkLimiter.Allow(tokenKey(token)) +} + // HandleResetLink serves /login/reset/: the new password's form (GET) // and setting it (POST). Setting it takes the link, ends every session of the // account and every other reset link, and leaves the person at sign-in. func (m *Module) HandleResetLink(w http.ResponseWriter, r *http.Request) { - if !m.linkLimiter.Allow(clientIP(r, m.trustedProxies)) { + token := linkToken(r.URL.Path, "/login/reset/") + if !m.allowLink(r, token) { http.Error(w, "too many requests", http.StatusTooManyRequests) return } - token := linkToken(r.URL.Path, "/login/reset/") if token == "" { http.NotFound(w, r) return @@ -305,11 +328,11 @@ func (m *Module) setPasswordFromLink(w http.ResponseWriter, r *http.Request, tok // a mistyped address would hand the account to whoever reads it, by a reset // link (security review, 2026-10-17). func (m *Module) HandleConfirm(w http.ResponseWriter, r *http.Request) { - if !m.linkLimiter.Allow(clientIP(r, m.trustedProxies)) { + token := linkToken(r.URL.Path, "/confirm/") + if !m.allowLink(r, token) { http.Error(w, "too many requests", http.StatusTooManyRequests) return } - token := linkToken(r.URL.Path, "/confirm/") if token == "" { http.NotFound(w, r) return