diff --git a/docs/configuration.md b/docs/configuration.md index a6d1193..d6d3e92 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -12,6 +12,25 @@ nowhere to put one that the operator would ever see. | `VERDANDE_DATA_DIR` | `/data` | Database, uploaded files and backups. | | `VERDANDE_DEV` | `false` | Human-readable logs at debug level, and a relaxed origin check so a Vite dev server can connect. Never in production. | +## Behind a proxy + +verdande keys its login rate limit and its audit log on the caller's address, so +that address has to be one the caller cannot forge. A forwarded-for header is +believed only when the machine that connected is a trusted proxy; a connection +straight off the internet is keyed on the address it really came from. + +| Variable | Default | What it does | +|---|---|---| +| `VERDANDE_TRUSTED_PROXIES` | private networks | Whose `X-Forwarded-For` to believe, as comma-separated CIDRs. The default trusts loopback and the private ranges — the shape of a reverse proxy or tunnel sharing the host or the docker bridge. Set to `none` for an instance exposed directly with no proxy, so no forwarded header is ever believed. | +| `VERDANDE_REAL_IP_HEADER` | `X-Forwarded-For` | The header the client address is read from, for a trusted proxy. | + +!!! warning "A proxy that does not overwrite the header" + The default is safe when your proxy replaces a client-supplied + `X-Forwarded-For` with the real address — nginx, Caddy and Cloudflare Tunnel all + do. If yours passes the client's own header through, narrow + `VERDANDE_TRUSTED_PROXIES` to just your proxy's address so a visitor cannot + choose the address they are rate-limited and logged as. + ## Sessions and links | Variable | Default | What it does | diff --git a/internal/config/config.go b/internal/config/config.go index afe7c2d..8f3a448 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -7,6 +7,7 @@ package config import ( "fmt" + "net/netip" "os" "path/filepath" "strconv" @@ -80,6 +81,26 @@ type Config struct { // TrashRetention is how long a soft-deleted task or project stays recoverable. TrashRetention time.Duration + // TrustedProxies are the peers whose forwarded-for header may be believed. + // + // The rate limiter and the audit log key on the caller's address, and that + // address is only as honest as wherever it was read. Read it from a header that + // anybody can set, and an attacker rotates it to get a fresh guess-bucket for + // every request and to write someone else's IP into the log. So the header is + // believed only when the machine that *connected* is one of these — a reverse + // proxy or tunnel the operator put there — and ignored when the connection came + // straight off the internet. + // + // The default is "the peer is on a private network", which is exactly the shape + // of a homelab behind Caddy, nginx or a Cloudflare Tunnel: the proxy shares the + // host or the docker bridge, so its address is loopback or RFC1918. A directly + // exposed instance sees a public peer, trusts nothing, and keys on the real + // connection. Override with VERDANDE_TRUSTED_PROXIES (comma-separated CIDRs), + // or "none" to trust no proxy at all. + TrustedProxies []netip.Prefix + // RealIPHeader is where the client address is read from, for a trusted peer. + RealIPHeader string + Dev bool } @@ -149,9 +170,70 @@ func Load() (*Config, error) { if c.DataDir, err = filepath.Abs(c.DataDir); err != nil { return nil, fmt.Errorf("VERDANDE_DATA_DIR: %w", err) } + + c.RealIPHeader = env("VERDANDE_REAL_IP_HEADER", "X-Forwarded-For") + if c.TrustedProxies, err = parseTrustedProxies(env("VERDANDE_TRUSTED_PROXIES", "")); err != nil { + return nil, err + } return c, nil } +// defaultTrustedProxies is "the peer is not on the public internet": loopback, +// the three RFC1918 ranges, Tailscale's CGNAT borrow, IPv6 unique-local and both +// families' link-local. A proxy an operator stands up in front of this server is +// on one of these; a stranger connecting directly is on none. +var defaultTrustedProxies = mustPrefixes( + "127.0.0.0/8", "::1/128", + "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", + "100.64.0.0/10", + "fc00::/7", + "169.254.0.0/16", "fe80::/10", +) + +// parseTrustedProxies reads the override. Empty keeps the private-network default; +// the literal "none" trusts no proxy at all, for an instance that is exposed +// directly and must never believe a forwarded header. +func parseTrustedProxies(raw string) ([]netip.Prefix, error) { + raw = strings.TrimSpace(raw) + if raw == "" { + return defaultTrustedProxies, nil + } + if strings.EqualFold(raw, "none") { + return nil, nil + } + var out []netip.Prefix + for _, part := range strings.Split(raw, ",") { + part = strings.TrimSpace(part) + if part == "" { + continue + } + // A bare address is a host route: 10.0.0.5 means 10.0.0.5/32, so an operator + // naming one proxy does not have to know CIDR notation to do it. + if !strings.Contains(part, "/") { + addr, err := netip.ParseAddr(part) + if err != nil { + return nil, fmt.Errorf("VERDANDE_TRUSTED_PROXIES: %q is not an address or CIDR", part) + } + out = append(out, netip.PrefixFrom(addr, addr.BitLen())) + continue + } + p, err := netip.ParsePrefix(part) + if err != nil { + return nil, fmt.Errorf("VERDANDE_TRUSTED_PROXIES: %q is not a CIDR", part) + } + out = append(out, p.Masked()) + } + return out, nil +} + +func mustPrefixes(cidrs ...string) []netip.Prefix { + out := make([]netip.Prefix, 0, len(cidrs)) + for _, c := range cidrs { + out = append(out, netip.MustParsePrefix(c)) + } + return out +} + // DBPath, FilesDir and BackupsDir are the three things that live on the /data volume. func (c *Config) DBPath() string { return filepath.Join(c.DataDir, "verdande.db") } func (c *Config) FilesDir() string { return filepath.Join(c.DataDir, "files") } diff --git a/internal/httpapi/csp_header_test.go b/internal/httpapi/csp_header_test.go index 3275dea..4608161 100644 --- a/internal/httpapi/csp_header_test.go +++ b/internal/httpapi/csp_header_test.go @@ -79,6 +79,10 @@ func TestHSTSTracksHTTPS(t *testing.T) { if got := resp.Header.Get("Permissions-Policy"); !strings.Contains(got, "camera=()") { t.Errorf("Permissions-Policy lukker ikke for kameraet: %q", got) } + // COOP isolerer browserkonteksten og står på hvert svar. + if got := resp.Header.Get("Cross-Origin-Opener-Policy"); got != "same-origin" { + t.Errorf("Cross-Origin-Opener-Policy er ikke same-origin: %q", got) + } } // Et bilag må ikke kunne blive læst som et dokument. diff --git a/internal/httpapi/realip.go b/internal/httpapi/realip.go new file mode 100644 index 0000000..bbfa3b5 --- /dev/null +++ b/internal/httpapi/realip.go @@ -0,0 +1,88 @@ +package httpapi + +import ( + "net" + "net/http" + "net/netip" + "strings" + + "github.com/kristianwind/verdande/internal/config" +) + +// realIP rewrites r.RemoteAddr to the client address a trusted proxy reported, +// and does nothing at all when the peer is not trusted. +// +// This replaces chi's middleware.RealIP, which is deprecated precisely because it +// believes a forwarded header from anyone: an attacker sets X-Forwarded-For to a +// new value on every request and gets a fresh rate-limit bucket each time, and +// writes whatever address they like into the audit log. The one control that +// exists for when a password is already gone is keyed on this address, so it has +// to be an address the caller cannot choose. +// +// The gate is the peer. A header is read only when the machine that opened the +// connection is one of cfg.TrustedProxies — a reverse proxy or tunnel the operator +// put there. A connection straight off the internet is trusted for nothing, so its +// header is ignored and it is keyed on the address it actually came from. Behind a +// proxy that overwrites the client's own header (nginx, Caddy and cloudflared all +// do) the value is then the real client and cannot be forged from outside. +func realIP(cfg *config.Config) func(http.Handler) http.Handler { + trusted := cfg.TrustedProxies + header := cfg.RealIPHeader + if header == "" { + header = "X-Forwarded-For" + } + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if ip := trustedClientIP(r, trusted, header); ip != "" { + // Port preserved off the original peer only for shape; downstream + // clientIP() splits it away, and the forwarded value carries none. + r.RemoteAddr = ip + } + next.ServeHTTP(w, r) + }) + } +} + +// trustedClientIP returns the forwarded client address if the peer may be believed, +// or "" to leave RemoteAddr untouched. +func trustedClientIP(r *http.Request, trusted []netip.Prefix, header string) string { + peer := r.RemoteAddr + if host, _, err := net.SplitHostPort(peer); err == nil { + peer = host + } + peerAddr, err := netip.ParseAddr(peer) + if err != nil || !isTrusted(peerAddr, trusted) { + return "" + } + + raw := r.Header.Get(header) + if raw == "" { + return "" + } + // The leftmost entry is the original client. That entry is only as trustworthy + // as the proxy that wrote it — but the trusted proxy is exactly the one that + // replaces a client-supplied header, so on the deployments this is written for + // it is the real client, and on a direct connection we never reach this line. + first := raw + if i := strings.IndexByte(raw, ','); i >= 0 { + first = raw[:i] + } + first = strings.TrimSpace(first) + if _, err := netip.ParseAddr(first); err != nil { + return "" + } + return first +} + +func isTrusted(addr netip.Addr, trusted []netip.Prefix) bool { + if !addr.IsValid() { + return false + } + addr = addr.Unmap() + for _, p := range trusted { + if p.Contains(addr) { + return true + } + } + return false +} diff --git a/internal/httpapi/realip_test.go b/internal/httpapi/realip_test.go new file mode 100644 index 0000000..2df7e6e --- /dev/null +++ b/internal/httpapi/realip_test.go @@ -0,0 +1,94 @@ +package httpapi + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/kristianwind/verdande/internal/config" +) + +// The address the rate limiter keys on must be one the caller cannot choose. A +// forwarded header is believed only from a trusted proxy, and ignored from a +// stranger connecting directly — the difference between a control and a suggestion. +func TestRealIPTrustsOnlyTheProxy(t *testing.T) { + cfg, err := config.Load() + if err != nil { + t.Fatal(err) + } + + seen := "" + h := realIP(cfg)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + seen = clientIP(r) + })) + + cases := []struct { + name string + remoteAddr string + xff string + want string + }{ + { + // A private peer is a proxy the operator put there: believe it. + name: "trusted proxy is believed", + remoteAddr: "10.1.2.3:5000", + xff: "203.0.113.9", + want: "203.0.113.9", + }, + { + // A public peer connected straight to us; its header is a guess. + name: "public peer header ignored", + remoteAddr: "198.51.100.7:5000", + xff: "203.0.113.9", + want: "198.51.100.7", + }, + { + // No header from a trusted proxy leaves the peer as the answer. + name: "trusted peer without header", + remoteAddr: "127.0.0.1:5000", + xff: "", + want: "127.0.0.1", + }, + { + // A spoofed value from a stranger cannot become the key. + name: "spoof from outside does not win", + remoteAddr: "198.51.100.7:5000", + xff: "127.0.0.1", + want: "198.51.100.7", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + r := httptest.NewRequest("POST", "/api/v1/auth/login", nil) + r.RemoteAddr = tc.remoteAddr + if tc.xff != "" { + r.Header.Set("X-Forwarded-For", tc.xff) + } + h.ServeHTTP(httptest.NewRecorder(), r) + if seen != tc.want { + t.Errorf("keyed on %q, want %q", seen, tc.want) + } + }) + } +} + +// "none" turns the trust off entirely, for an instance exposed directly. +func TestRealIPTrustNone(t *testing.T) { + t.Setenv("VERDANDE_TRUSTED_PROXIES", "none") + cfg, err := config.Load() + if err != nil { + t.Fatal(err) + } + seen := "" + h := realIP(cfg)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + seen = clientIP(r) + })) + r := httptest.NewRequest("POST", "/", nil) + r.RemoteAddr = "10.1.2.3:5000" // even a private peer is not trusted now + r.Header.Set("X-Forwarded-For", "203.0.113.9") + h.ServeHTTP(httptest.NewRecorder(), r) + if seen != "10.1.2.3" { + t.Errorf("keyed on %q, want the peer 10.1.2.3", seen) + } +} diff --git a/internal/httpapi/server.go b/internal/httpapi/server.go index 9ee7e37..b298302 100644 --- a/internal/httpapi/server.go +++ b/internal/httpapi/server.go @@ -110,7 +110,10 @@ func New(cfg *config.Config, db *store.DB, log *slog.Logger, web fs.FS) *Server r := chi.NewRouter() r.Use(middleware.RequestID) - r.Use(middleware.RealIP) + // Not middleware.RealIP: that believes a forwarded header from anyone, which + // lets a caller pick the address the rate limiter and audit log key on. This + // one reads the header only from a trusted proxy — see realip.go. + r.Use(realIP(cfg)) r.Use(requestLogger(log)) r.Use(middleware.Recoverer) r.Use(middleware.Timeout(30 * time.Second)) @@ -685,6 +688,11 @@ func (s *Server) securityHeaders(next http.Handler) http.Handler { // browser ignores is not a control. h.Set("Permissions-Policy", "camera=(), microphone=(), geolocation=(), "+ "payment=(), usb=(), interest-cohort=()") + // Isolate the browsing context: a window this page opened, or one that + // opened it, cannot keep a handle across the origin boundary. verdande + // never scripts another window, so there is nothing to lose and a whole + // class of cross-window attacks to close. + h.Set("Cross-Origin-Opener-Policy", "same-origin") next.ServeHTTP(w, r) }) }