Honour forwarded headers only from MEALQUEST_TRUSTED_PROXY
CI / ci (pull_request) Successful in 1m26s
Independent review Codex gpt-6-sol high: CHANGES_REQUIRED (1 high, 1 medium)
CI / ci (push) Successful in 1m25s

Add internal/forwarded: parse the trusted proxy list (comma-separated IPs
and CIDRs, validated at load), resolve each request's client IP and scheme,
and expose them through context helpers. X-Forwarded-For and
X-Forwarded-Proto are honoured only when the TCP peer is trusted; the client
is the rightmost X-Forwarded-For address that is not a trusted proxy.
Malformed forwarded headers from a trusted peer get 400.

The router resolves every request first; a slog handler adds client_ip to
request-scoped log lines. The auth routes refuse requests whose resolved
scheme differs from MEALQUEST_PUBLIC_URL's, so a proxy that is not trusted
fails loudly instead of breaking Secure cookies. The OIDC redirect URI is
still built only from MEALQUEST_PUBLIC_URL.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-03 10:57:23 -04:00
co-authored by Claude Opus 5.5
parent 36b975ff5b
commit 7e49b72ad4
14 changed files with 943 additions and 15 deletions
+33 -1
View File
@@ -19,6 +19,7 @@ import (
"github.com/go-chi/chi/v5"
"golang.org/x/oauth2"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secureurl"
)
@@ -83,7 +84,10 @@ type Auth struct {
// endSessionURL is the provider's end_session_endpoint; logout ends
// there so the provider's own session ends too.
endSessionURL string
logger *slog.Logger
// publicScheme is the public URL's scheme; requests to the auth routes
// must have arrived over it.
publicScheme string
logger *slog.Logger
}
// New discovers the provider at cfg.Issuer and returns the login flow. It
@@ -155,6 +159,7 @@ func New(ctx context.Context, cfg Config, sessions Sessions, logger *slog.Logger
cookie: newLoginCookie(cfg.SecretKey),
sessions: sessions,
endSessionURL: discovered.EndSessionURI,
publicScheme: cfg.PublicURL.Scheme,
logger: logger,
}, nil
}
@@ -193,12 +198,39 @@ func checkRedirect(req *http.Request, via []*http.Request) error {
}
// Mount registers GET LoginPath, GET CallbackPath and POST LogoutPath on r.
// The routes need the request's resolved scheme in the context (see package
// forwarded) and refuse requests that did not arrive over the public URL's
// scheme.
func (a *Auth) Mount(r chi.Router) {
r = r.With(a.requirePublicScheme)
r.Get(LoginPath, a.login)
r.Get(CallbackPath, a.callback)
r.Post(LogoutPath, a.logout)
}
// requirePublicScheme refuses a request whose resolved scheme differs from
// the public URL's. The login and session cookies are Secure, and the
// redirect URI is always built from the public URL, so a sign-in reaching the
// service over another scheme cannot work: behind a TLS-terminating proxy
// this means the proxy's X-Forwarded-Proto was not honoured, because the
// proxy is not a trusted peer.
func (a *Auth) requirePublicScheme(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
scheme, ok := forwarded.Scheme(r.Context())
switch {
case !ok:
a.logger.ErrorContext(r.Context(), "auth route reached without a resolved request scheme")
http.Error(w, "internal error", http.StatusInternalServerError)
case scheme != a.publicScheme:
a.logger.WarnContext(r.Context(), "auth request scheme differs from the public URL; is the proxy a trusted peer?",
"scheme", scheme, "public_scheme", a.publicScheme)
http.Error(w, "sign-in must use the service's public URL", http.StatusBadRequest)
default:
next.ServeHTTP(w, r)
}
})
}
// EndSessionURL is the provider's end_session_endpoint.
func (a *Auth) EndSessionURL() string { return a.endSessionURL }
+148 -4
View File
@@ -17,6 +17,7 @@ import (
"math/big"
"net/http"
"net/http/httptest"
"net/netip"
"net/url"
"slices"
"strings"
@@ -26,6 +27,8 @@ import (
"time"
"github.com/go-chi/chi/v5"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded"
)
const (
@@ -347,11 +350,32 @@ func newHarness(t *testing.T) *harness {
if err != nil {
t.Fatal(err)
}
r := chi.NewRouter()
a.Mount(r)
r := publicRouter(a, testPublicURL.Scheme)
return &harness{provider: p, sessions: sessions, router: r, logs: logs}
}
// publicRouter mounts a's routes behind the request-origin resolution the
// server installs, with every request taken to have arrived over scheme.
// Requests with noOrigin set skip the resolution.
func publicRouter(a *Auth, scheme string) http.Handler {
r := chi.NewRouter()
r.Use(func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
if req.Header.Get(noOrigin) == "" {
o := forwarded.Origin{ClientIP: netip.MustParseAddr("192.0.2.1"), Scheme: scheme}
req = req.WithContext(forwarded.NewContext(req.Context(), o))
}
next.ServeHTTP(w, req)
})
})
a.Mount(r)
return r
}
// noOrigin is a test-only request header telling publicRouter to leave the
// request's origin unresolved.
const noOrigin = "X-Test-No-Origin"
func (h *harness) get(target string, cookies ...*http.Cookie) *http.Response {
req := httptest.NewRequest(http.MethodGet, target, nil)
for _, c := range cookies {
@@ -433,6 +457,127 @@ func TestLoginRedirect(t *testing.T) {
}
}
func TestAuthRoutesRequirePublicScheme(t *testing.T) {
t.Parallel()
p := newFakeProvider(t)
logs := &syncBuffer{}
a, err := New(context.Background(), Config{
Issuer: p.issuer(),
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("server-secret-key"),
AllowedGroups: testAllowedGroups,
}, &memorySessions{}, slog.New(slog.NewTextHandler(logs, nil)))
if err != nil {
t.Fatal(err)
}
// The proxy's X-Forwarded-Proto was not honoured: the request looks like
// plain http while the public URL is https.
plain := publicRouter(a, "http")
for _, req := range []*http.Request{
httptest.NewRequest(http.MethodGet, LoginPath, nil),
httptest.NewRequest(http.MethodGet, CallbackPath+"?code=c&state=s", nil),
httptest.NewRequest(http.MethodPost, LogoutPath, nil),
} {
rec := httptest.NewRecorder()
plain.ServeHTTP(rec, req)
if rec.Code != http.StatusBadRequest {
t.Errorf("%s %s over http = %d, want 400", req.Method, req.URL.Path, rec.Code)
}
if loc := rec.Header().Get("Location"); loc != "" {
t.Errorf("%s %s over http redirected to %q", req.Method, req.URL.Path, loc)
}
if c := rec.Result().Cookies(); len(c) != 0 {
t.Errorf("%s %s over http set cookies %v", req.Method, req.URL.Path, c)
}
}
if !strings.Contains(logs.String(), "trusted peer") {
t.Errorf("logs = %q, want the scheme mismatch explained", logs.String())
}
bare := chi.NewRouter()
a.Mount(bare)
rec := httptest.NewRecorder()
bare.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, LoginPath, nil))
if rec.Code != http.StatusInternalServerError {
t.Errorf("login without a resolved origin = %d, want 500", rec.Code)
}
}
// TestLoginBehindTraefik resolves each request with forwarded.Resolve, as
// the server's middleware does, trusting Traefik only.
func TestLoginBehindTraefik(t *testing.T) {
t.Parallel()
p := newFakeProvider(t)
a, err := New(context.Background(), Config{
Issuer: p.issuer(),
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("server-secret-key"),
AllowedGroups: testAllowedGroups,
}, &memorySessions{}, slog.New(slog.NewTextHandler(io.Discard, nil)))
if err != nil {
t.Fatal(err)
}
trusted, err := forwarded.ParseTrusted("10.60.1.33")
if err != nil {
t.Fatal(err)
}
r := chi.NewRouter()
r.Use(func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
o, err := forwarded.Resolve(req, trusted)
if err != nil {
t.Errorf("Resolve: %v", err)
return
}
next.ServeHTTP(w, req.WithContext(forwarded.NewContext(req.Context(), o)))
})
})
a.Mount(r)
login := func(peer string) *http.Response {
// Traefik reaches the service over plain http on its internal
// address; the host headers must not leak into the redirect URI.
req := httptest.NewRequest(http.MethodGet, "http://mealquest:8080"+LoginPath, nil)
req.RemoteAddr = peer
req.Header.Set("X-Forwarded-For", "203.0.113.7")
req.Header.Set("X-Forwarded-Proto", "https")
req.Header.Set("X-Forwarded-Host", "evil.example.com")
rec := httptest.NewRecorder()
r.ServeHTTP(rec, req)
return rec.Result()
}
resp := login("10.60.1.33:41000")
if resp.StatusCode != http.StatusFound {
t.Fatalf("login via Traefik status = %d, want 302", resp.StatusCode)
}
loc, err := url.Parse(resp.Header.Get("Location"))
if err != nil {
t.Fatal(err)
}
if got, want := loc.Query().Get("redirect_uri"), "https://meals.example.com/auth/callback"; got != want {
t.Errorf("redirect_uri = %q, want %q from the public URL", got, want)
}
cookies := resp.Cookies()
if len(cookies) != 1 || cookies[0].Name != loginCookieName || !cookies[0].Secure {
t.Errorf("login cookies = %v, want one Secure %s", cookies, loginCookieName)
}
// The same headers from an untrusted peer are ignored: the request is
// plain http, so sign-in is refused rather than setting a Secure cookie
// the browser would drop.
resp = login("198.51.100.9:5000")
if resp.StatusCode != http.StatusBadRequest || len(resp.Cookies()) != 0 {
t.Errorf("login spoofing X-Forwarded-Proto = %d with cookies %v, want 400 and none",
resp.StatusCode, resp.Cookies())
}
}
func TestLoginStateIsFreshEachTime(t *testing.T) {
t.Parallel()
h := newHarness(t)
@@ -904,8 +1049,7 @@ func TestCallbackRefusesTokenRedirectToHTTP(t *testing.T) {
}
// Trust the test server's certificate; the redirect policy stays.
a.client.Transport = tlsToken.Client().Transport
r := chi.NewRouter()
a.Mount(r)
r := publicRouter(a, testPublicURL.Scheme)
h := &harness{provider: p, sessions: sessions, router: r, logs: logs}
l := h.login(t, LoginPath)
+1 -4
View File
@@ -13,8 +13,6 @@ import (
"testing"
"time"
"github.com/go-chi/chi/v5"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations"
)
@@ -72,8 +70,7 @@ func newSessionHarness(t *testing.T) *sessionHarness {
if err != nil {
t.Fatal(err)
}
r := chi.NewRouter()
a.Mount(r)
r := publicRouter(a, testPublicURL.Scheme)
h.harness = &harness{provider: p, router: r, logs: logs}
h.auth = a
return h
+21
View File
@@ -12,6 +12,7 @@ import (
"strings"
"time"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secureurl"
)
@@ -30,6 +31,8 @@ const (
EnvSessionIdle = "MEALQUEST_SESSION_IDLE"
EnvSessionMax = "MEALQUEST_SESSION_MAX"
EnvTrustedProxy = "MEALQUEST_TRUSTED_PROXY"
)
// Session lifetime defaults (D-004): long enough that the kitchen tablet
@@ -58,6 +61,10 @@ type Config struct {
SessionIdle time.Duration
// SessionMax ends every session this long after sign-in, used or not.
SessionMax time.Duration
// TrustedProxy lists the reverse proxies whose X-Forwarded-For and
// X-Forwarded-Proto headers are honoured. Empty trusts no peer.
TrustedProxy forwarded.Trusted
}
// PublicOrigin returns the scheme and host of PublicURL, the only parts of it
@@ -88,6 +95,8 @@ func load(lookup func(string) (string, bool)) (Config, error) {
SessionIdle: r.sessionDuration(EnvSessionIdle, DefaultSessionIdle),
SessionMax: r.sessionDuration(EnvSessionMax, DefaultSessionMax),
TrustedProxy: r.trustedProxy(EnvTrustedProxy),
}
if cfg.SessionIdle > 0 && cfg.SessionMax > 0 && cfg.SessionMax < cfg.SessionIdle {
r.fail(EnvSessionMax, "must be at least %s (%s)", EnvSessionIdle, cfg.SessionIdle)
@@ -177,6 +186,18 @@ func (r *reader) list(name string) []string {
return items
}
// trustedProxy parses an optional comma-separated list of IP addresses and
// CIDR prefixes with forwarded.ParseTrusted. Unset or blank trusts no peer.
func (r *reader) trustedProxy(name string) forwarded.Trusted {
v, _ := r.lookup(name)
t, err := forwarded.ParseTrusted(v)
if err != nil {
r.fail(name, "%v", err)
return nil
}
return t
}
func (r *reader) listenAddr(name string) string {
v, ok := r.required(name)
if !ok {
+54
View File
@@ -3,6 +3,7 @@ package config
import (
"errors"
"fmt"
"net/netip"
"net/url"
"os"
"path/filepath"
@@ -11,6 +12,8 @@ import (
"strings"
"testing"
"time"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded"
)
func TestLoad(t *testing.T) {
@@ -293,6 +296,48 @@ func TestLoad(t *testing.T) {
env: with(map[string]string{EnvSessionMax: "719h"}),
badVars: []string{EnvSessionMax},
},
{
name: "trusted proxy IP",
env: with(map[string]string{EnvTrustedProxy: "10.60.1.33"}),
},
{
name: "trusted proxy IPs and CIDRs",
env: with(map[string]string{EnvTrustedProxy: "10.60.1.33, 172.16.0.0/12,2001:db8::/32,::1"}),
},
{
name: "blank trusted proxy trusts no peer",
env: with(map[string]string{EnvTrustedProxy: " "}),
},
{
name: "trusted proxy not an IP",
env: with(map[string]string{EnvTrustedProxy: "traefik"}),
badVars: []string{EnvTrustedProxy},
},
{
name: "trusted proxy invalid CIDR",
env: with(map[string]string{EnvTrustedProxy: "10.60.1.0/33"}),
badVars: []string{EnvTrustedProxy},
},
{
name: "trusted proxy CIDR with host bits",
env: with(map[string]string{EnvTrustedProxy: "10.60.1.33/24"}),
badVars: []string{EnvTrustedProxy},
},
{
name: "trusted proxy empty entry",
env: with(map[string]string{EnvTrustedProxy: "10.60.1.33,,10.60.1.34"}),
badVars: []string{EnvTrustedProxy},
},
{
name: "trusted proxy trusting everyone",
env: with(map[string]string{EnvTrustedProxy: "0.0.0.0/0"}),
badVars: []string{EnvTrustedProxy},
},
{
name: "trusted proxy with port",
env: with(map[string]string{EnvTrustedProxy: "10.60.1.33:443"}),
badVars: []string{EnvTrustedProxy},
},
{
name: "several invalid values are all named",
env: with(map[string]string{
@@ -402,8 +447,13 @@ func TestLoadValues(t *testing.T) {
t.Errorf("SessionIdle, SessionMax = %v, %v, want defaults 720h, 4320h", cfg.SessionIdle, cfg.SessionMax)
}
if cfg.TrustedProxy != nil {
t.Errorf("TrustedProxy = %v, want none when unset", cfg.TrustedProxy)
}
env[EnvSessionIdle] = "12h"
env[EnvSessionMax] = "36h"
env[EnvTrustedProxy] = "10.60.1.33, 172.16.0.0/12"
cfg, err = load(func(k string) (string, bool) { v, ok := env[k]; return v, ok })
if err != nil {
t.Fatalf("load() error = %v", err)
@@ -411,6 +461,10 @@ func TestLoadValues(t *testing.T) {
if cfg.SessionIdle != 12*time.Hour || cfg.SessionMax != 36*time.Hour {
t.Errorf("SessionIdle, SessionMax = %v, %v, want 12h, 36h", cfg.SessionIdle, cfg.SessionMax)
}
want := forwarded.Trusted{netip.MustParsePrefix("10.60.1.33/32"), netip.MustParsePrefix("172.16.0.0/12")}
if !slices.Equal(cfg.TrustedProxy, want) {
t.Errorf("TrustedProxy = %v, want %v", cfg.TrustedProxy, want)
}
}
func TestSecretRedacted(t *testing.T) {
+221
View File
@@ -0,0 +1,221 @@
// Package forwarded resolves where a request really came from. The service
// runs behind a reverse proxy (Traefik), so the TCP peer is usually the proxy;
// the X-Forwarded-For and X-Forwarded-Proto headers it adds are believed only
// when the peer is a trusted proxy. From anyone else they are ignored, so a
// client cannot spoof its address or scheme.
package forwarded
import (
"context"
"errors"
"fmt"
"log/slog"
"net/http"
"net/netip"
"strings"
)
// Header names honoured from trusted proxies.
const (
HeaderFor = "X-Forwarded-For"
HeaderProto = "X-Forwarded-Proto"
)
// ErrInvalidHeader marks a malformed forwarded header from a trusted peer.
var ErrInvalidHeader = errors.New("invalid forwarded header")
// Trusted is the set of proxy addresses whose forwarded headers are honoured.
// The zero value trusts no peer.
type Trusted []netip.Prefix
// ParseTrusted parses a comma-separated list of IP addresses and CIDR
// prefixes. A blank list trusts no peer. Each entry must be a plain address
// or a prefix with no host bits set; zones, IPv4-mapped IPv6 forms and
// zero-length prefixes, which would trust every peer, are rejected.
func ParseTrusted(s string) (Trusted, error) {
if strings.TrimSpace(s) == "" {
return nil, nil
}
var t Trusted
for _, entry := range strings.Split(s, ",") {
entry = strings.TrimSpace(entry)
if entry == "" {
return nil, fmt.Errorf("empty entry in comma-separated list %q", s)
}
p, err := parseEntry(entry)
if err != nil {
return nil, err
}
t = append(t, p)
}
return t, nil
}
func parseEntry(entry string) (netip.Prefix, error) {
var p netip.Prefix
if strings.Contains(entry, "/") {
var err error
p, err = netip.ParsePrefix(entry)
if err != nil {
return netip.Prefix{}, fmt.Errorf("invalid CIDR %q", entry)
}
if p != p.Masked() {
return netip.Prefix{}, fmt.Errorf("CIDR %q has host bits set (did you mean %s?)", entry, p.Masked())
}
} else {
a, err := netip.ParseAddr(entry)
if err != nil {
return netip.Prefix{}, fmt.Errorf("invalid IP address %q", entry)
}
if a.Zone() != "" {
return netip.Prefix{}, fmt.Errorf("IP address %q must not have a zone", entry)
}
p = netip.PrefixFrom(a, a.BitLen())
}
switch {
case p.Addr().Is4In6():
return netip.Prefix{}, fmt.Errorf("%q is IPv4-mapped IPv6; write the IPv4 form", entry)
case p.Bits() == 0:
return netip.Prefix{}, fmt.Errorf("%q would trust every peer", entry)
}
return p, nil
}
// Contains reports whether a is a trusted proxy address. IPv4-mapped IPv6
// addresses, as a dual-stack listener reports IPv4 peers, match their IPv4
// form.
func (t Trusted) Contains(a netip.Addr) bool {
a = normalize(a)
for _, p := range t {
if p.Contains(a) {
return true
}
}
return false
}
func normalize(a netip.Addr) netip.Addr { return a.Unmap().WithZone("") }
// Origin is where a request came from.
type Origin struct {
// ClientIP is the client's address.
ClientIP netip.Addr
// Scheme is "https" or "http": the scheme the client used.
Scheme string
}
// Resolve works out r's Origin. When the TCP peer (r.RemoteAddr) is not in
// trusted, both forwarded headers are ignored: the client is the peer and the
// scheme is that of the connection.
//
// When the peer is trusted:
// - X-Forwarded-For entries (across all its header lines, in order) are
// walked right to left, skipping trusted proxy addresses; the first
// untrusted address is the client. Entries left of it are client-supplied
// and not examined. If every entry is trusted the leftmost is the client;
// with no X-Forwarded-For the peer is the client.
// - X-Forwarded-Proto, if present, must be a single "http" or "https" and
// is the scheme; without it the connection's scheme is used.
//
// A malformed header from a trusted peer is an error wrapping
// ErrInvalidHeader; any other error means RemoteAddr is not an IP and port.
func Resolve(r *http.Request, trusted Trusted) (Origin, error) {
peer, err := netip.ParseAddrPort(r.RemoteAddr)
if err != nil {
return Origin{}, fmt.Errorf("peer address %q is not an IP and port", r.RemoteAddr)
}
o := Origin{ClientIP: normalize(peer.Addr()), Scheme: "http"}
if r.TLS != nil {
o.Scheme = "https"
}
if !trusted.Contains(o.ClientIP) {
return o, nil
}
if values := r.Header.Values(HeaderFor); len(values) > 0 {
o.ClientIP, err = clientFromChain(strings.Split(strings.Join(values, ","), ","), trusted)
if err != nil {
return Origin{}, err
}
}
switch values := r.Header.Values(HeaderProto); len(values) {
case 0:
case 1:
switch scheme := strings.ToLower(strings.TrimSpace(values[0])); scheme {
case "http", "https":
o.Scheme = scheme
default:
return Origin{}, fmt.Errorf("%w: %s %q is not http or https", ErrInvalidHeader, HeaderProto, values[0])
}
default:
return Origin{}, fmt.Errorf("%w: %d %s headers, want one", ErrInvalidHeader, len(values), HeaderProto)
}
return o, nil
}
// clientFromChain returns the rightmost untrusted address in chain, or the
// leftmost address when all are trusted.
func clientFromChain(chain []string, trusted Trusted) (netip.Addr, error) {
var client netip.Addr
for i := len(chain) - 1; i >= 0; i-- {
a, err := netip.ParseAddr(strings.TrimSpace(chain[i]))
if err != nil {
return netip.Addr{}, fmt.Errorf("%w: %s entry %q is not an IP address", ErrInvalidHeader, HeaderFor, strings.TrimSpace(chain[i]))
}
client = normalize(a)
if !trusted.Contains(client) {
break
}
}
return client, nil
}
type contextKey struct{}
// NewContext returns ctx carrying o.
func NewContext(ctx context.Context, o Origin) context.Context {
return context.WithValue(ctx, contextKey{}, o)
}
// FromContext returns the Origin ctx carries; ok is false when the request
// was not resolved.
func FromContext(ctx context.Context) (o Origin, ok bool) {
o, ok = ctx.Value(contextKey{}).(Origin)
return o, ok
}
// ClientIP returns the resolved client address ctx carries.
func ClientIP(ctx context.Context) (netip.Addr, bool) {
o, ok := FromContext(ctx)
return o.ClientIP, ok
}
// Scheme returns the resolved request scheme ctx carries.
func Scheme(ctx context.Context) (string, bool) {
o, ok := FromContext(ctx)
return o.Scheme, ok
}
// LogAttr is the log attribute key for the resolved client address.
const LogAttr = "client_ip"
// LogHandler wraps h so every record logged with a resolved request's
// context carries the client address as LogAttr.
func LogHandler(h slog.Handler) slog.Handler { return logHandler{h} }
type logHandler struct{ slog.Handler }
func (h logHandler) Handle(ctx context.Context, rec slog.Record) error {
if ip, ok := ClientIP(ctx); ok {
rec = rec.Clone()
rec.AddAttrs(slog.String(LogAttr, ip.String()))
}
return h.Handler.Handle(ctx, rec)
}
func (h logHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
return logHandler{h.Handler.WithAttrs(attrs)}
}
func (h logHandler) WithGroup(name string) slog.Handler {
return logHandler{h.Handler.WithGroup(name)}
}
+260
View File
@@ -0,0 +1,260 @@
package forwarded
import (
"bytes"
"context"
"crypto/tls"
"errors"
"log/slog"
"net/http"
"net/http/httptest"
"net/netip"
"slices"
"strings"
"testing"
)
func TestParseTrusted(t *testing.T) {
t.Parallel()
tests := []struct {
in string
want []string
wantErr string
}{
{in: "", want: nil},
{in: " ", want: nil},
{in: "10.60.1.33", want: []string{"10.60.1.33/32"}},
{in: "10.60.1.33, 172.16.0.0/12", want: []string{"10.60.1.33/32", "172.16.0.0/12"}},
{in: "2001:db8::1,2001:db8::/32", want: []string{"2001:db8::1/128", "2001:db8::/32"}},
{in: "traefik", wantErr: "invalid IP address"},
{in: "10.60.1.33:443", wantErr: "invalid IP address"},
{in: "10.60.1.0/33", wantErr: "invalid CIDR"},
{in: "10.60.1.33/24", wantErr: "did you mean 10.60.1.0/24"},
{in: "10.60.1.33,,10.60.1.34", wantErr: "empty entry"},
{in: "10.60.1.33,", wantErr: "empty entry"},
{in: "fe80::1%eth0", wantErr: "zone"},
{in: "::ffff:10.60.1.33", wantErr: "IPv4-mapped"},
{in: "0.0.0.0/0", wantErr: "every peer"},
{in: "::/0", wantErr: "every peer"},
}
for _, tt := range tests {
got, err := ParseTrusted(tt.in)
if tt.wantErr != "" {
if err == nil || !strings.Contains(err.Error(), tt.wantErr) {
t.Errorf("ParseTrusted(%q) error = %v, want one containing %q", tt.in, err, tt.wantErr)
}
continue
}
if err != nil {
t.Errorf("ParseTrusted(%q) error = %v", tt.in, err)
continue
}
var gotStr []string
for _, p := range got {
gotStr = append(gotStr, p.String())
}
if !slices.Equal(gotStr, tt.want) {
t.Errorf("ParseTrusted(%q) = %v, want %v", tt.in, gotStr, tt.want)
}
}
}
func TestTrustedContainsMappedIPv4(t *testing.T) {
t.Parallel()
trusted := mustTrusted(t, "10.60.1.33")
if !trusted.Contains(netip.MustParseAddr("::ffff:10.60.1.33")) {
t.Error("IPv4-mapped form of a trusted address is not trusted")
}
if Trusted(nil).Contains(netip.MustParseAddr("10.60.1.33")) {
t.Error("an empty set trusts a peer")
}
}
func mustTrusted(t *testing.T, s string) Trusted {
t.Helper()
tr, err := ParseTrusted(s)
if err != nil {
t.Fatal(err)
}
return tr
}
func TestResolve(t *testing.T) {
t.Parallel()
// 10.60.1.33 is Traefik; 10.60.2.0/24 is an inner proxy tier.
trusted := mustTrusted(t, "10.60.1.33,10.60.2.0/24")
tests := []struct {
name string
peer string
tls bool
xff []string
xfp []string
wantIP string
wantScheme string
}{
{
name: "untrusted peer, no headers",
peer: "198.51.100.9:5000", wantIP: "198.51.100.9", wantScheme: "http",
},
{
name: "untrusted peer over TLS",
peer: "198.51.100.9:5000", tls: true, wantIP: "198.51.100.9", wantScheme: "https",
},
{
name: "untrusted peer spoofing both headers is ignored",
peer: "198.51.100.9:5000", xff: []string{"203.0.113.7"}, xfp: []string{"https"},
wantIP: "198.51.100.9", wantScheme: "http",
},
{
name: "untrusted peer's malformed headers are ignored, not rejected",
peer: "198.51.100.9:5000", xff: []string{"not-an-ip"}, xfp: []string{"gopher"},
wantIP: "198.51.100.9", wantScheme: "http",
},
{
name: "trusted peer: client from X-Forwarded-For, scheme from X-Forwarded-Proto",
peer: "10.60.1.33:41000", xff: []string{"203.0.113.7"}, xfp: []string{"https"},
wantIP: "203.0.113.7", wantScheme: "https",
},
{
name: "trusted peer seen as IPv4-mapped IPv6",
peer: "[::ffff:10.60.1.33]:41000", xff: []string{"203.0.113.7"}, xfp: []string{"https"},
wantIP: "203.0.113.7", wantScheme: "https",
},
{
name: "trusted peer without headers is itself the client",
peer: "10.60.1.33:41000", wantIP: "10.60.1.33", wantScheme: "http",
},
{
name: "rightmost untrusted address wins over a client-supplied prefix",
peer: "10.60.1.33:41000", xff: []string{"1.2.3.4, 203.0.113.7"}, xfp: []string{"https"},
wantIP: "203.0.113.7", wantScheme: "https",
},
{
name: "trusted proxies in the chain are skipped",
peer: "10.60.1.33:41000", xff: []string{"1.2.3.4, 203.0.113.7, 10.60.2.5"}, xfp: []string{"https"},
wantIP: "203.0.113.7", wantScheme: "https",
},
{
name: "several header lines form one chain",
peer: "10.60.1.33:41000", xff: []string{"1.2.3.4", "203.0.113.7, 10.60.2.5"},
wantIP: "203.0.113.7", wantScheme: "http",
},
{
name: "garbage left of the client is not examined",
peer: "10.60.1.33:41000", xff: []string{"spoofed, 203.0.113.7"},
wantIP: "203.0.113.7", wantScheme: "http",
},
{
name: "all trusted: leftmost is the client",
peer: "10.60.1.33:41000", xff: []string{"10.60.2.5, 10.60.2.6"},
wantIP: "10.60.2.5", wantScheme: "http",
},
{
name: "IPv6 client",
peer: "10.60.1.33:41000", xff: []string{"2001:db8::7"}, xfp: []string{"HTTPS"},
wantIP: "2001:db8::7", wantScheme: "https",
},
{
name: "trusted peer may downgrade to http",
peer: "10.60.1.33:41000", tls: true, xfp: []string{"http"},
wantIP: "10.60.1.33", wantScheme: "http",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
r := request(tt.peer, tt.xff, tt.xfp)
if tt.tls {
r.TLS = &tls.ConnectionState{}
}
o, err := Resolve(r, trusted)
if err != nil {
t.Fatalf("Resolve error = %v", err)
}
if o.ClientIP.String() != tt.wantIP || o.Scheme != tt.wantScheme {
t.Errorf("Resolve = %s %s, want %s %s", o.ClientIP, o.Scheme, tt.wantIP, tt.wantScheme)
}
})
}
}
func TestResolveRejectsMalformedHeadersFromTrustedPeer(t *testing.T) {
t.Parallel()
trusted := mustTrusted(t, "10.60.1.33")
for _, tt := range []struct {
name string
xff, xfp []string
}{
{"X-Forwarded-For not an IP", []string{"not-an-ip"}, nil},
{"X-Forwarded-For with a port", []string{"203.0.113.7:5000"}, nil},
{"X-Forwarded-For empty entry", []string{"203.0.113.7, "}, nil},
{"X-Forwarded-For empty", []string{""}, nil},
{"X-Forwarded-Proto unknown scheme", nil, []string{"ftp"}},
{"X-Forwarded-Proto list", nil, []string{"https, http"}},
{"X-Forwarded-Proto repeated", nil, []string{"https", "https"}},
} {
_, err := Resolve(request("10.60.1.33:41000", tt.xff, tt.xfp), trusted)
if !errors.Is(err, ErrInvalidHeader) {
t.Errorf("%s: Resolve error = %v, want ErrInvalidHeader", tt.name, err)
}
}
}
func TestResolveBadPeerAddress(t *testing.T) {
t.Parallel()
r := request("@", nil, nil)
_, err := Resolve(r, nil)
if err == nil || errors.Is(err, ErrInvalidHeader) {
t.Errorf("Resolve error = %v, want a peer address error", err)
}
}
func request(peer string, xff, xfp []string) *http.Request {
r := httptest.NewRequest(http.MethodGet, "/", nil)
r.RemoteAddr = peer
for _, v := range xff {
r.Header.Add(HeaderFor, v)
}
for _, v := range xfp {
r.Header.Add(HeaderProto, v)
}
return r
}
func TestContextHelpers(t *testing.T) {
t.Parallel()
if _, ok := ClientIP(context.Background()); ok {
t.Error("ClientIP ok on an unresolved context")
}
if _, ok := Scheme(context.Background()); ok {
t.Error("Scheme ok on an unresolved context")
}
ctx := NewContext(context.Background(), Origin{ClientIP: netip.MustParseAddr("203.0.113.7"), Scheme: "https"})
if ip, ok := ClientIP(ctx); !ok || ip.String() != "203.0.113.7" {
t.Errorf("ClientIP = %s %v, want 203.0.113.7 true", ip, ok)
}
if s, ok := Scheme(ctx); !ok || s != "https" {
t.Errorf("Scheme = %q %v, want https true", s, ok)
}
}
func TestLogHandler(t *testing.T) {
t.Parallel()
var buf bytes.Buffer
logger := slog.New(LogHandler(slog.NewTextHandler(&buf, nil))).With("component", "test")
ctx := NewContext(context.Background(), Origin{ClientIP: netip.MustParseAddr("203.0.113.7"), Scheme: "https"})
logger.InfoContext(ctx, "resolved")
logger.InfoContext(context.Background(), "unresolved")
lines := strings.Split(strings.TrimSpace(buf.String()), "\n")
if len(lines) != 2 {
t.Fatalf("logged %d lines, want 2: %q", len(lines), buf.String())
}
if !strings.Contains(lines[0], "client_ip=203.0.113.7") || !strings.Contains(lines[0], "component=test") {
t.Errorf("resolved line = %q, want client_ip and the With attribute", lines[0])
}
if strings.Contains(lines[1], "client_ip") {
t.Errorf("unresolved line = %q, want no client_ip", lines[1])
}
}
+34
View File
@@ -0,0 +1,34 @@
package server
import (
"errors"
"log/slog"
"net/http"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded"
)
// resolveOrigin resolves every request's client address and scheme with
// forwarded.Resolve and puts them in the context (read them with
// forwarded.ClientIP and forwarded.Scheme). Forwarded headers are honoured
// only from a peer in trusted. A malformed forwarded header from a trusted
// peer gets 400 bad_request.
func resolveOrigin(logger *slog.Logger, trusted forwarded.Trusted) func(http.Handler) http.Handler {
badRequest := apiErrorHandler(logger, http.StatusBadRequest, "bad_request", "invalid forwarded headers")
internal := apiErrorHandler(logger, http.StatusInternalServerError, "internal", "internal error")
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
o, err := forwarded.Resolve(r, trusted)
switch {
case errors.Is(err, forwarded.ErrInvalidHeader):
logger.WarnContext(r.Context(), "rejected forwarded headers", "peer", r.RemoteAddr, "error", err)
badRequest(w, r)
case err != nil:
logger.ErrorContext(r.Context(), "resolve request origin", "error", err)
internal(w, r)
default:
next.ServeHTTP(w, r.WithContext(forwarded.NewContext(r.Context(), o)))
}
})
}
}
+145
View File
@@ -0,0 +1,145 @@
package server
import (
"bytes"
"encoding/json"
"fmt"
"io"
"log/slog"
"net/http"
"net/http/httptest"
"testing"
"testing/fstest"
"github.com/go-chi/chi/v5"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded"
)
// traefik is the production proxy's address.
const traefik = "10.60.1.33"
// probeAuth mounts GET /auth/probe, which reports the request's resolved
// origin, in place of the real auth flow.
type probeAuth struct{}
func (probeAuth) Mount(r chi.Router) {
r.Get("/auth/probe", func(w http.ResponseWriter, r *http.Request) {
ip, ipOK := forwarded.ClientIP(r.Context())
scheme, schemeOK := forwarded.Scheme(r.Context())
if !ipOK || !schemeOK {
http.Error(w, "origin not resolved", http.StatusInternalServerError)
return
}
fmt.Fprintf(w, "%s %s", ip, scheme)
})
}
// newOriginRouter builds the production router trusting Traefik, logging
// through forwarded.LogHandler as cmd/server does.
func newOriginRouter(t *testing.T, logs io.Writer) http.Handler {
t.Helper()
trusted, err := forwarded.ParseTrusted(traefik)
if err != nil {
t.Fatal(err)
}
logger := slog.New(forwarded.LogHandler(slog.NewJSONHandler(logs, nil)))
r, err := NewRouter(logger, "dev", trusted, probeAuth{}, stubSessions{},
fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}})
if err != nil {
t.Fatal(err)
}
return r
}
func originRequest(target, peer string, headers map[string]string) *http.Request {
req := httptest.NewRequest(http.MethodGet, target, nil)
req.RemoteAddr = peer
for k, v := range headers {
req.Header.Set(k, v)
}
return req
}
func TestRouterResolvesOrigin(t *testing.T) {
t.Parallel()
spoofed := map[string]string{forwarded.HeaderFor: "203.0.113.7", forwarded.HeaderProto: "https"}
tests := []struct {
name string
peer string
want string
}{
{"trusted proxy's forwarded headers are honoured", traefik + ":41000", "203.0.113.7 https"},
{"untrusted peer spoofing them is ignored", "198.51.100.9:5000", "198.51.100.9 http"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
rec := httptest.NewRecorder()
newOriginRouter(t, io.Discard).ServeHTTP(rec, originRequest("/auth/probe", tt.peer, spoofed))
if rec.Code != http.StatusOK || rec.Body.String() != tt.want {
t.Errorf("probe = %d %q, want 200 %q", rec.Code, rec.Body.String(), tt.want)
}
})
}
}
func TestRouterLogsResolvedClientIP(t *testing.T) {
t.Parallel()
spoofed := map[string]string{forwarded.HeaderFor: "203.0.113.7", forwarded.HeaderProto: "https"}
tests := []struct {
name string
peer string
want string
}{
{"trusted proxy", traefik + ":41000", "203.0.113.7"},
{"untrusted peer spoofing", "198.51.100.9:5000", "198.51.100.9"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
var logs bytes.Buffer
// A failed session lookup is logged with the request's context.
req := originRequest("/api/v1/me", tt.peer, spoofed)
req.AddCookie(brokenSessionCookie)
rec := httptest.NewRecorder()
newOriginRouter(t, &logs).ServeHTTP(rec, req)
if rec.Code != http.StatusInternalServerError {
t.Fatalf("status = %d, want 500", rec.Code)
}
var line map[string]any
if err := json.Unmarshal(logs.Bytes(), &line); err != nil {
t.Fatalf("log %q: %v", logs.String(), err)
}
if line["msg"] != "look up session" || line[forwarded.LogAttr] != tt.want {
t.Errorf("log = %v, want the session error with %s=%s", line, forwarded.LogAttr, tt.want)
}
})
}
}
func TestRouterRejectsMalformedForwardedHeaders(t *testing.T) {
t.Parallel()
const badRequestJSON = `{"error":{"code":"bad_request","message":"invalid forwarded headers","retryable":false}}`
for _, headers := range []map[string]string{
{forwarded.HeaderFor: "not-an-ip"},
{forwarded.HeaderProto: "ftp"},
} {
var logs bytes.Buffer
rec := httptest.NewRecorder()
newOriginRouter(t, &logs).ServeHTTP(rec, originRequest("/health", traefik+":41000", headers))
if rec.Code != http.StatusBadRequest || rec.Body.String() != badRequestJSON {
t.Errorf("%v: response = %d %s, want 400 %s", headers, rec.Code, rec.Body.String(), badRequestJSON)
}
if !bytes.Contains(logs.Bytes(), []byte("rejected forwarded headers")) {
t.Errorf("%v: logs = %q, want the rejection logged", headers, logs.String())
}
// The same headers from an untrusted peer are not examined at all.
rec = httptest.NewRecorder()
newOriginRouter(t, io.Discard).ServeHTTP(rec, originRequest("/health", "198.51.100.9:5000", headers))
if rec.Code != http.StatusOK {
t.Errorf("%v from an untrusted peer: status = %d, want 200", headers, rec.Code)
}
}
}
+6 -2
View File
@@ -9,6 +9,7 @@ import (
"github.com/go-chi/chi/v5"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/frontend"
)
@@ -21,12 +22,14 @@ type RouteMounter interface {
Mount(r chi.Router)
}
// NewRouter returns the HTTP handler for the service. version is reported by
// NewRouter returns the HTTP handler for the service. Every request first
// has its client address and scheme resolved, honouring forwarded headers
// only from a peer in trusted (see resolveOrigin). version is reported by
// GET /health; auth mounts the /auth routes; sessions guards /api/v1 and
// /media; assets is the web build served for every other GET. /health, /auth
// and the web build are public. Requests to /api/v1 other than GET, HEAD and
// OPTIONS also need the session's CSRF token in CSRFHeader.
func NewRouter(logger *slog.Logger, version string, auth RouteMounter, sessions SessionLookup, assets fs.FS) (http.Handler, error) {
func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, auth RouteMounter, sessions SessionLookup, assets fs.FS) (http.Handler, error) {
notFound := apiErrorHandler(logger, http.StatusNotFound, "not_found", "not found")
methodNotAllowed := apiErrorHandler(logger, http.StatusMethodNotAllowed, "method_not_allowed", "method not allowed")
@@ -36,6 +39,7 @@ func NewRouter(logger *slog.Logger, version string, auth RouteMounter, sessions
}
r := chi.NewRouter()
r.Use(resolveOrigin(logger, trusted))
r.MethodNotAllowed(methodNotAllowed)
r.NotFound(notFound)
+1 -1
View File
@@ -27,7 +27,7 @@ func newTestRouter(t *testing.T, version string) http.Handler {
"index.html": {Data: []byte(fixtureIndex)},
"assets/index-AbC123.js": {Data: []byte(fixtureAsset)},
}
r, err := NewRouter(logger, version, stubAuth{}, stubSessions{}, assets)
r, err := NewRouter(logger, version, nil, stubAuth{}, stubSessions{}, assets)
if err != nil {
t.Fatal(err)
}