The review of 4433688..0808779 (read-only, a separate agent) found one medium and some lows; all fixed here. Medium, proven by the reviewer: A1's userKey put the typed username into the limiter map whole, and the public forms had no body bound beyond net/http's 10 MiB for a parsed form - each refused request kept up to 10 MiB in memory for fifteen minutes. userKey now holds a 96-bit hash of the folded name, and sign-in, reset, a reset link's page and setup read at most 64 KiB (limitForm, http.MaxBytesReader). TestPublicFormsAreBoundedAndKeysShort. Lows: - Refund skipped the request that filled a bucket (count < max), so a right password at the ceiling left the shared address blocked; Refund now always undoes the Allow it follows. - The reset form did not give the address its attempt back when the name's bucket refused (sign-in did); it does. - A3's day of five links is spent only by a link that goes out, and the price of the pause - anyone knowing a username can pause that owner's reset for a day - is recorded in security.md. - longHeaderLine counted a trailing CR on a header's first line. - test.yml declares permissions: contents: read. Not changed, as the review's nits: the store matches names exactly while the limiter folds them (stricter, said in the comment now); a session from a longer window is honoured until its next Touch (the next request). go test ./internal/web/auth ./internal/milter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
138 lines
4.5 KiB
Go
138 lines
4.5 KiB
Go
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 the stranger's text, of any length a form carries: the key holds
|
|
// its hash, not the name (the review of stages 2-6 kept a 5 MB name for
|
|
// fifteen minutes per request). Case is folded, so "Admin" and "admin" share
|
|
// a bucket - stricter than the store, which matches names exactly.
|
|
func userKey(addr, username string) string {
|
|
sum := sha256.Sum256([]byte(strings.ToLower(strings.TrimSpace(username))))
|
|
return addr + "|" + hex.EncodeToString(sum[:12])
|
|
}
|
|
|
|
// maxPublicForm bounds the body of a form a stranger can send - sign-in,
|
|
// reset, a link's page, setup - far below net/http's 10 MiB for a parsed
|
|
// form (net/http, parsePostForm: 10 << 20 when the body is not already a
|
|
// MaxBytesReader).
|
|
const maxPublicForm = 64 << 10
|
|
|
|
// limitForm caps the body ParseForm will read on a public form.
|
|
func limitForm(w http.ResponseWriter, r *http.Request) {
|
|
r.Body = http.MaxBytesReader(w, r.Body, maxPublicForm)
|
|
}
|
|
|
|
// 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...)
|
|
}
|
|
}
|