security: parse X-Forwarded-For from trusted proxies for rate-limit key
Resolves plan item A.1 (option б): login/setup rate-limiting used RemoteAddr only, which behind the default reverse proxy is the proxy's own address, making the limiter effectively global and enabling a lockout-DoS. Now, when the request's direct peer matches the new TRUSTED_PROXY_CIDR list (comma-separated CIDRs, env, empty by default), the last X-Forwarded-For entry is used instead, giving a real per-client limit. Unset behaviour is unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -7,6 +7,10 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
|
||||
|
||||
- ci: disable provenance attestation on release image push, so the ghcr.io
|
||||
manifest list shows only `linux/amd64`/`linux/arm64` (no `unknown/unknown`).
|
||||
- security: optionally honour `X-Forwarded-For` for login/setup rate-limiting
|
||||
when the request's direct peer is in the new `TRUSTED_PROXY_CIDR` list,
|
||||
giving real per-client limits behind a reverse proxy instead of one global
|
||||
bucket. Unset by default (unchanged `RemoteAddr`-only behaviour).
|
||||
|
||||
## [0.1.0] - 2026-07-15
|
||||
|
||||
|
||||
@@ -32,6 +32,7 @@ func serveHTTP(ctx context.Context, cfg config, st *store.Store) error {
|
||||
DataDir: cfg.dataDir,
|
||||
DBPath: cfg.dbPath,
|
||||
Version: buildinfo.Version,
|
||||
TrustedProxyCIDRs: cfg.trustedProxies,
|
||||
}, cfg.setupTokenPath)
|
||||
if err != nil {
|
||||
return err
|
||||
|
||||
@@ -12,10 +12,12 @@ import (
|
||||
"flag"
|
||||
"fmt"
|
||||
"log"
|
||||
"net"
|
||||
"os"
|
||||
"os/signal"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"syscall"
|
||||
|
||||
@@ -56,6 +58,7 @@ type config struct {
|
||||
setupTokenPath string
|
||||
hostname string
|
||||
cookieSecure bool
|
||||
trustedProxies []*net.IPNet
|
||||
|
||||
opendkimDir string
|
||||
dkimSelectorDef string
|
||||
@@ -83,6 +86,11 @@ func loadConfig() config {
|
||||
// Secure cookies by default (spec 7.6.6); PANEL_COOKIE_SECURE=false is a
|
||||
// development-only escape hatch for testing over plain HTTP.
|
||||
cookieSecure: envDefault("PANEL_COOKIE_SECURE", "true") != "false",
|
||||
// Reverse-proxy addresses allowed to supply X-Forwarded-For for
|
||||
// rate-limiting (plan.md item A.1). Empty by default: an untrusted peer's
|
||||
// XFF header is trivially forgeable, so it's ignored unless the panel is
|
||||
// told which proxy to trust.
|
||||
trustedProxies: parseTrustedProxies(os.Getenv("TRUSTED_PROXY_CIDR")),
|
||||
|
||||
// Per-domain DKIM state (spec 6). The directory layout matches what
|
||||
// entrypoint.sh prepares (setgid, shared `selfpost` group).
|
||||
@@ -131,6 +139,37 @@ func envInt(key string, def int) int {
|
||||
return def
|
||||
}
|
||||
|
||||
// parseTrustedProxies parses a comma-separated list of CIDRs (bare IPs are
|
||||
// accepted and treated as /32 or /128). Invalid entries are logged and
|
||||
// skipped rather than failing startup, matching envInt's tolerance of
|
||||
// misconfiguration.
|
||||
func parseTrustedProxies(raw string) []*net.IPNet {
|
||||
if raw == "" {
|
||||
return nil
|
||||
}
|
||||
var nets []*net.IPNet
|
||||
for _, part := range strings.Split(raw, ",") {
|
||||
cidr := strings.TrimSpace(part)
|
||||
if cidr == "" {
|
||||
continue
|
||||
}
|
||||
if !strings.Contains(cidr, "/") {
|
||||
if ip := net.ParseIP(cidr); ip != nil && ip.To4() != nil {
|
||||
cidr += "/32"
|
||||
} else {
|
||||
cidr += "/128"
|
||||
}
|
||||
}
|
||||
_, n, err := net.ParseCIDR(cidr)
|
||||
if err != nil {
|
||||
log.Printf("ignoring invalid TRUSTED_PROXY_CIDR entry %q: %v", part, err)
|
||||
continue
|
||||
}
|
||||
nets = append(nets, n)
|
||||
}
|
||||
return nets
|
||||
}
|
||||
|
||||
// run starts the panel's three roles and blocks until a shutdown signal or the
|
||||
// first fatal error from any role. A signal triggers a clean stop of all roles;
|
||||
// a role error cancels the others and is returned so the process exits non-zero
|
||||
|
||||
@@ -15,3 +15,11 @@ RATE_LIMIT_WINDOW_SECONDS=3600
|
||||
|
||||
# Days of send-log history kept before the background sweep deletes rows.
|
||||
SEND_LOG_RETENTION_DAYS=90
|
||||
|
||||
# Comma-separated CIDRs (bare IPs allowed) of reverse proxies allowed to
|
||||
# supply X-Forwarded-For for login/setup rate-limiting. Leave unset unless
|
||||
# you know the exact address of your reverse proxy — trusting the wrong
|
||||
# source lets a client spoof its rate-limit key. Behind the default Apache
|
||||
# host-network setup this is typically the Docker bridge gateway, e.g.
|
||||
# TRUSTED_PROXY_CIDR=172.18.0.1
|
||||
|
||||
|
||||
@@ -22,7 +22,7 @@
|
||||
|
||||
### A. Безопасность — hardening сверх обязательного 7.6
|
||||
|
||||
1. **Rate-limit за обратным прокси кеится по `RemoteAddr`** ([internal/web/web.go](../internal/web/web.go) `clientIP`). За дефолтным Apache это адрес прокси → лимитеры логина и `/setup` фактически **глобальны**. Осознанный выбор: не парсить `X-Forwarded-For` (иначе тривиально обходится подделкой заголовка). По ТЗ 7.6.1 реальная защита setup — 128-бит энтропии токена, rate-limit там defense-in-depth. **Побочный эффект:** атакующий может исчерпать общий bucket логина (10 попыток/15 мин) и на 15 минут заблокировать вход легитимному админу (lockout-DoS). **Варианты:** (а) оставить как есть (просто, по ТЗ достаточно); (б) парсить XFF **только от доверенного прокси** (`TRUSTED_PROXY_CIDR`) → настоящий per-client лимит; (в) вместо жёсткой блокировки — экспоненциальная задержка ответа, чтобы brute-force тормозился, но легитимный вход не блокировался.
|
||||
1. **Rate-limit за обратным прокси кеился по `RemoteAddr`** ([internal/web/web.go](../internal/web/web.go) `clientIP`). **Решено:** вариант (б) — парсить `X-Forwarded-For`, но только когда прямой peer (`RemoteAddr`) входит в `TRUSTED_PROXY_CIDR` (список CIDR через запятую, env, по умолчанию пусто); тогда используется последний элемент XFF (адрес, добавленный самим доверенным прокси). Без настройки `TRUSTED_PROXY_CIDR` поведение не меняется (лимит по `RemoteAddr`, глобальный за прокси). См. `deploy/.env.example`.
|
||||
2. **Нет security-заголовков ответа** — панель не шлёт `Strict-Transport-Security`, `Content-Security-Policy`, `X-Frame-Options`/`frame-ancestors`, `X-Content-Type-Options: nosniff`, `Referrer-Policy`. XSS уже закрыт автоэкранированием `html/template` (7.6.7), CSRF — `SameSite=Lax`, но заголовки — дешёвый второй эшелон (clickjacking, downgrade, sniffing). **Вопрос:** эмитить их из панели (единый мидлварь, ~10 строк) или переложить на reverse-proxy и задокументировать в `deploy/`? Рекомендация — минимальный набор из панели (HSTS/nosniff/`frame-ancestors 'none'`/строгий CSP `default-src 'self'`), т.к. панель знает свою модель контента, а прокси у всех разный.
|
||||
3. **CSRF — только `SameSite=Lax`, без токенов.** Достаточно для современных браузеров (все мутации — POST, все GET read-only), но не защищает при downgrade до старого браузера/особых прокси и не даёт защиты на уровне «per-request». **Вопрос:** считать `SameSite=Lax` достаточным для single-admin панели (моя рекомендация — да) или добавить double-submit CSRF-токен.
|
||||
4. **Cookie без префикса `__Host-`.** Сейчас `selfpost_session` (`Secure`/`HttpOnly`/`SameSite=Lax`/`Path=/`). Префикс `__Host-` дал бы браузерный гарант «только HTTPS, только этот хост, без Domain». Мелочь, но бесплатная. **Вопрос:** переименовать (учесть dev-режим `PANEL_COOKIE_SECURE=false` — `__Host-` требует `Secure`, т.е. только когда secure включён).
|
||||
|
||||
@@ -50,7 +50,7 @@ func (s *Server) renderLogin(w http.ResponseWriter, status int, formErr string)
|
||||
|
||||
func (s *Server) submitLogin(w http.ResponseWriter, r *http.Request) {
|
||||
// Brute-force throttle by client IP (spec 7.6.5).
|
||||
if !s.loginLimiter.Allow(clientIP(r)) {
|
||||
if !s.loginLimiter.Allow(clientIP(r, s.trustedProxies)) {
|
||||
s.renderLogin(w, http.StatusTooManyRequests, "Too many attempts. Please wait and try again.")
|
||||
return
|
||||
}
|
||||
|
||||
@@ -13,7 +13,7 @@ import (
|
||||
// page, also 404.
|
||||
func (s *Server) handleSetup(w http.ResponseWriter, r *http.Request) {
|
||||
// Route-specific rate limit, separate from login (spec 7.6.1).
|
||||
if !s.setupLimiter.Allow(clientIP(r)) {
|
||||
if !s.setupLimiter.Allow(clientIP(r, s.trustedProxies)) {
|
||||
http.Error(w, "too many requests", http.StatusTooManyRequests)
|
||||
return
|
||||
}
|
||||
|
||||
+41
-6
@@ -8,6 +8,7 @@ import (
|
||||
"log"
|
||||
"net"
|
||||
"net/http"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"codeberg.org/mix/selfpost/internal/app"
|
||||
@@ -37,6 +38,12 @@ type Config struct {
|
||||
DataDir string
|
||||
DBPath string
|
||||
Version string
|
||||
// TrustedProxyCIDRs are the reverse-proxy addresses allowed to supply
|
||||
// X-Forwarded-For (plan.md item A.1: TRUSTED_PROXY_CIDR). A request whose
|
||||
// direct peer (RemoteAddr) is not in this list never has its XFF header
|
||||
// honoured, so the header can't be spoofed by anyone but a trusted proxy.
|
||||
// Empty (the default) keeps rate-limiting keyed on RemoteAddr only.
|
||||
TrustedProxyCIDRs []*net.IPNet
|
||||
}
|
||||
|
||||
// Server is the panel HTTP application.
|
||||
@@ -51,6 +58,8 @@ type Server struct {
|
||||
|
||||
loginLimiter *rateLimiter
|
||||
setupLimiter *rateLimiter
|
||||
|
||||
trustedProxies []*net.IPNet
|
||||
}
|
||||
|
||||
// New builds the panel server. setupTokenPath is where the current setup token
|
||||
@@ -74,6 +83,8 @@ func New(st *store.Store, domains *domain.Service, apps *app.Service, cfg Config
|
||||
setupLimiter: newRateLimiter(10, time.Minute),
|
||||
// Login: throttle brute-force by IP (spec 7.6.5).
|
||||
loginLimiter: newRateLimiter(10, 15*time.Minute),
|
||||
|
||||
trustedProxies: cfg.TrustedProxyCIDRs,
|
||||
}
|
||||
s.setup = newSetupManager(st, cfg.Hostname, setupTokenPath)
|
||||
return s, nil
|
||||
@@ -144,18 +155,42 @@ func handleHealth(w http.ResponseWriter, _ *http.Request) {
|
||||
_, _ = w.Write([]byte("ok\n"))
|
||||
}
|
||||
|
||||
// clientIP extracts the peer IP for rate-limiting. It uses the transport peer
|
||||
// (RemoteAddr), not client-supplied headers, so it cannot be spoofed; behind a
|
||||
// reverse proxy this is the proxy address, which is an acceptable backstop for
|
||||
// a single-admin panel.
|
||||
func clientIP(r *http.Request) string {
|
||||
// clientIP extracts the peer IP for rate-limiting. By default it is the
|
||||
// transport peer (RemoteAddr), which cannot be spoofed. If RemoteAddr matches
|
||||
// one of trustedProxies, the last entry of X-Forwarded-For is used instead —
|
||||
// that is the address the trusted proxy itself appended, so a client can't
|
||||
// forge it by sending its own XFF header (plan.md item A.1). With no trusted
|
||||
// proxies configured, behind a reverse proxy this is the proxy's own address,
|
||||
// which is an acceptable backstop for a single-admin panel.
|
||||
func clientIP(r *http.Request, trustedProxies []*net.IPNet) string {
|
||||
host, _, err := net.SplitHostPort(r.RemoteAddr)
|
||||
if err != nil {
|
||||
return r.RemoteAddr
|
||||
host = r.RemoteAddr
|
||||
}
|
||||
|
||||
if len(trustedProxies) > 0 {
|
||||
if peer := net.ParseIP(host); peer != nil && ipInAny(peer, trustedProxies) {
|
||||
if xff := r.Header.Get("X-Forwarded-For"); xff != "" {
|
||||
parts := strings.Split(xff, ",")
|
||||
if ip := net.ParseIP(strings.TrimSpace(parts[len(parts)-1])); ip != nil {
|
||||
return ip.String()
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return host
|
||||
}
|
||||
|
||||
func ipInAny(ip net.IP, nets []*net.IPNet) bool {
|
||||
for _, n := range nets {
|
||||
if n.Contains(ip) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// logf is a thin wrapper so handlers log with a consistent prefix.
|
||||
func logf(format string, args ...any) {
|
||||
log.Printf(format, args...)
|
||||
|
||||
Reference in New Issue
Block a user