From a420db8b8a3597b56bcc6dc7d1fc5f5d7d932448 Mon Sep 17 00:00:00 2001 From: Aurelia Date: Fri, 25 Apr 2025 13:59:55 +0200 Subject: feat: more elaborate XFF compute (#350) * feat: more elaborate XFF compute #328 followup now featuring configuration and defaults that shouldn't break most setups. fixes #344 * refactor: obvious condition eval order optimization * feat: add StripLLU implementation * chore: I'm sorry it's 7 AM * test: add test environment for unix socket serving Signed-off-by: Xe Iaso * test(unix-socket-xff): comment out the shell script more Signed-off-by: Xe Iaso * fix(internal): fix logic bug in XFF computation, add tests Signed-off-by: Xe Iaso * fix(internal): prevent panic in local testing Signed-off-by: Xe Iaso * fix(internal): shuffle around return values to flow better Signed-off-by: Xe Iaso --------- Signed-off-by: Xe Iaso Co-authored-by: Xe Iaso --- internal/headers.go | 124 ++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 102 insertions(+), 22 deletions(-) (limited to 'internal/headers.go') diff --git a/internal/headers.go b/internal/headers.go index 4516b40..639d2b1 100644 --- a/internal/headers.go +++ b/internal/headers.go @@ -1,15 +1,29 @@ package internal import ( + "errors" + "fmt" "log/slog" "net" "net/http" + "net/netip" "strings" "github.com/TecharoHQ/anubis" "github.com/sebest/xff" ) +// TODO: move into config +type XFFComputePreferences struct { + StripPrivate bool + StripLoopback bool + StripCGNAT bool + StripLLU bool + Flatten bool +} + +var CGNat = netip.MustParsePrefix("100.64.0.0/10") + // UnchangingCache sets the Cache-Control header to cache a response for 1 year if // and only if the application is compiled in "release" mode by Docker. func UnchangingCache(next http.Handler) http.Handler { @@ -71,40 +85,106 @@ func XForwardedForUpdate(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { defer next.ServeHTTP(w, r) - remoteIP, _, err := net.SplitHostPort(r.RemoteAddr) + pref := XFFComputePreferences{ + StripPrivate: true, + StripLoopback: true, + StripCGNAT: true, + Flatten: true, + StripLLU: true, + } + + remoteAddr := r.RemoteAddr + origXFFHeader := r.Header.Get("X-Forwarded-For") - if parsedRemoteIP := net.ParseIP(remoteIP); parsedRemoteIP != nil && parsedRemoteIP.IsLoopback() { - // anubis is likely deployed behind a local reverse proxy - // pass header as-is to not break existing applications + if remoteAddr == "@" { + // remote is a unix socket + // do not touch chain return } + xffHeaderString, err := computeXFFHeader(remoteAddr, origXFFHeader, pref) if err != nil { - slog.Warn("The default format of request.RemoteAddr should be IP:Port", "remoteAddr", r.RemoteAddr) + slog.Debug("computing X-Forwarded-For header failed", "err", err) return } - if xff := r.Header.Get("X-Forwarded-For"); xff != "" { - forwardedList := strings.Split(",", xff) - forwardedList = append(forwardedList, remoteIP) - // this behavior is equivalent to - // ingress-nginx "compute-full-forwarded-for" - // https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#compute-full-forwarded-for - // - // this would be the correct place to strip and/or flatten this list - // - // strip - iterate backwards and eliminate configured trusted IPs - // flatten - only return the last element to avoid spoofing confusion - // - // many applications handle this in different ways, but - // generally they'd be expected to do these two things on - // their own end to find the first non-spoofed IP - r.Header.Set("X-Forwarded-For", strings.Join(forwardedList, ",")) + + if len(xffHeaderString) == 0 { + r.Header.Del("X-Forwarded-For") } else { - r.Header.Set("X-Forwarded-For", remoteIP) + r.Header.Set("X-Forwarded-For", xffHeaderString) } }) } +var ( + ErrCantSplitHostParse = errors.New("internal: unable to net.SplitHostParse") + ErrCantParseRemoteIP = errors.New("internal: unable to parse remote IP") +) + +func computeXFFHeader(remoteAddr string, origXFFHeader string, pref XFFComputePreferences) (string, error) { + remoteIP, _, err := net.SplitHostPort(remoteAddr) + if err != nil { + return "", fmt.Errorf("%w: %w", ErrCantSplitHostParse, err) + } + parsedRemoteIP, err := netip.ParseAddr(remoteIP) + if err != nil { + return "", fmt.Errorf("%w: %w", ErrCantParseRemoteIP, err) + } + + origForwardedList := make([]string, 0, 4) + if origXFFHeader != "" { + origForwardedList = strings.Split(origXFFHeader, ",") + } + origForwardedList = append(origForwardedList, parsedRemoteIP.String()) + forwardedList := make([]string, 0, len(origForwardedList)) + // this behavior is equivalent to + // ingress-nginx "compute-full-forwarded-for" + // https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#compute-full-forwarded-for + // + // this would be the correct place to strip and/or flatten this list + // + // strip - iterate backwards and eliminate configured trusted IPs + // flatten - only return the last element to avoid spoofing confusion + // + // many applications handle this in different ways, but + // generally they'd be expected to do these two things on + // their own end to find the first non-spoofed IP + for i := len(origForwardedList) - 1; i >= 0; i-- { + segmentIP, err := netip.ParseAddr(origForwardedList[i]) + if err != nil { + // can't assess this element, so the remainder of the chain + // can't be trusted. not a fatal error, since anyone can + // spoof an XFF header + slog.Debug("failed to parse XFF segment", "err", err) + break + } + if pref.StripPrivate && segmentIP.IsPrivate() { + continue + } + if pref.StripLoopback && segmentIP.IsLoopback() { + continue + } + if pref.StripLLU && segmentIP.IsLinkLocalUnicast() { + continue + } + if pref.StripCGNAT && CGNat.Contains(segmentIP) { + continue + } + forwardedList = append([]string{segmentIP.String()}, forwardedList...) + } + var xffHeaderString string + if len(forwardedList) == 0 { + xffHeaderString = "" + return xffHeaderString, nil + } + if pref.Flatten { + xffHeaderString = forwardedList[len(forwardedList)-1] + } else { + xffHeaderString = strings.Join(forwardedList, ",") + } + return xffHeaderString, nil +} + // NoStoreCache sets the Cache-Control header to no-store for the response. func NoStoreCache(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { -- cgit v1.2.3