From 5e4ed4d508dc4db5a8731b676f7ba3ad9a72c131 Mon Sep 17 00:00:00 2001 From: silverwind Date: Fri, 23 Oct 2026 11:25:04 +0200 Subject: [PATCH] fix: correctly parse and validate forwarded client IPs and remove unmaintained `chi-middleware/proxy` (#39190) Replace the unmaintained `chi-middleware/proxy` with an in-tree handler that detects client IPs correctly behind common reverse proxies. 1. Handle repeated header lines and comma-separated proxy hops. 2. Accept and normalize IPs, including `ip:port`. 3. Keep the client port from the header value or `X-Real-Port` instead of logging port 0. 4. Prefer the last `X-Real-IP` line. 5. Keep the proxy's address when the selected value is invalid. 6. Read `Forwarded` only when `X-Forwarded-For` is absent. Docs PR: https://gitea.com/gitea/docs/pulls/560 Fixes: https://github.com/go-gitea/gitea/issues/35682 Fixes: https://github.com/go-gitea/gitea/issues/36814 Co-authored-by: bircni --- assets/go-licenses.json | 5 - custom/conf/app.example.ini | 3 +- go.mod | 1 - go.sum | 3 - routers/common/forwardedheaders.go | 156 ++++++++++++++++++++++++ routers/common/forwardedheaders_test.go | 57 +++++++++ routers/common/middleware.go | 14 --- 7 files changed, 215 insertions(+), 24 deletions(-) create mode 100644 routers/common/forwardedheaders.go create mode 100644 routers/common/forwardedheaders_test.go diff --git a/assets/go-licenses.json b/assets/go-licenses.json index 34edbbaacd4..71cd1704588 100644 --- a/assets/go-licenses.json +++ b/assets/go-licenses.json @@ -284,11 +284,6 @@ "path": "github.com/charmbracelet/git-lfs-transfer/LICENSE", "licenseText": "MIT License\n\nCopyright (c) 2022-2023 Charmbracelet, Inc\n\nPermission is hereby granted, free of charge, to any person obtaining a copy\nof this software and associated documentation files (the \"Software\"), to deal\nin the Software without restriction, including without limitation the rights\nto use, copy, modify, merge, publish, distribute, sublicense, and/or sell\ncopies of the Software, and to permit persons to whom the Software is\nfurnished to do so, subject to the following conditions:\n\nThe above copyright notice and this permission notice shall be included in all\ncopies or substantial portions of the Software.\n\nTHE SOFTWARE IS PROVIDED \"AS IS\", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR\nIMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,\nFITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE\nAUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER\nLIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,\nOUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE\nSOFTWARE.\n" }, - { - "name": "github.com/chi-middleware/proxy", - "path": "github.com/chi-middleware/proxy/LICENSE", - "licenseText": "Copyright (c) 2020 Lauris BH\n\nPermission is hereby granted, free of charge, to any person obtaining a copy\nof this software and associated documentation files (the \"Software\"), to deal\nin the Software without restriction, including without limitation the rights\nto use, copy, modify, merge, publish, distribute, sublicense, and/or sell\ncopies of the Software, and to permit persons to whom the Software is\nfurnished to do so, subject to the following conditions:\n\nThe above copyright notice and this permission notice shall be included in\nall copies or substantial portions of the Software.\n\nTHE SOFTWARE IS PROVIDED \"AS IS\", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR\nIMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,\nFITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE\nAUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER\nLIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,\nOUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN\nTHE SOFTWARE.\n" - }, { "name": "github.com/clipperhouse/displaywidth", "path": "github.com/clipperhouse/displaywidth/LICENSE", diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 2211607d9de..249d05e29fe 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -471,7 +471,8 @@ INTERNAL_TOKEN = ;REVERSE_PROXY_AUTHENTICATION_EMAIL = X-WEBAUTH-EMAIL ;REVERSE_PROXY_AUTHENTICATION_FULL_NAME = X-WEBAUTH-FULLNAME ;; -;; Interpret X-Forwarded-For header or the X-Real-IP header and set this as the remote IP for the request +;; Interpret the X-Real-IP, X-Forwarded-For or Forwarded header, in that order, and set this as the remote IP for the request. +;; The client port is taken from the header value or, with X-Real-IP, from X-Real-Port. ;REVERSE_PROXY_LIMIT = 1 ;; ;; List of IP addresses and networks separated by comma of trusted proxy servers. Use `*` to trust all. diff --git a/go.mod b/go.mod index 4768c85549a..9a5b2501fe3 100644 --- a/go.mod +++ b/go.mod @@ -25,7 +25,6 @@ require ( github.com/buildkite/terminal-to-html/v3 v3.17.1 github.com/caddyserver/certmagic v0.25.6 github.com/charmbracelet/git-lfs-transfer v0.1.1-0.20260812203852-971c0284dc33 - github.com/chi-middleware/proxy v1.1.1 github.com/coder/websocket v1.8.15 github.com/dlclark/regexp2/v2 v2.8.4 github.com/dsnet/compress v0.0.2-0.20230904184137-39efe44ab707 diff --git a/go.sum b/go.sum index 25a5c77be24..a62eae07100 100644 --- a/go.sum +++ b/go.sum @@ -138,8 +138,6 @@ github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UF github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/charmbracelet/git-lfs-transfer v0.1.1-0.20260812203852-971c0284dc33 h1:YG5mXlv9SxS4rdtSxK3RuH146x1HIQzsbVJqSp/Rvfc= github.com/charmbracelet/git-lfs-transfer v0.1.1-0.20260812203852-971c0284dc33/go.mod h1:R+SetERD4+IL7QH0WHp9MLifvITvh9gL3g8vX1j2Fcs= -github.com/chi-middleware/proxy v1.1.1 h1:4HaXUp8o2+bhHr1OhVy+VjN0+L7/07JDcn6v7YrTjrQ= -github.com/chi-middleware/proxy v1.1.1/go.mod h1:jQwMEJct2tz9VmtCELxvnXoMfa+SOdikvbVJVHv/M+0= github.com/chromedp/cdproto v0.0.0-20230802225258-3cf4e6d46a89/go.mod h1:GKljq0VrfU4D5yc+2qA6OVr8pmO/MBbPEWqWQ/oqGEs= github.com/chromedp/chromedp v0.9.2/go.mod h1:LkSXJKONWTCHAfQasKFUZI+mxqS4tZqhmtGzzhLsnLs= github.com/chromedp/sysutil v1.0.0/go.mod h1:kgWmDdq8fTzXYcKIBqIYvRRTnYb9aNS9moAV0xufSww= @@ -200,7 +198,6 @@ github.com/git-lfs/pktline v0.0.0-20230103162542-ca444d533ef1 h1:mtDjlmloH7ytdbl github.com/git-lfs/pktline v0.0.0-20230103162542-ca444d533ef1/go.mod h1:fenKRzpXDjNpsIBhuhUzvjCKlDjKam0boRAenTE0Q6A= github.com/go-asn1-ber/asn1-ber v1.5.8 h1:H9AZkK22UOmfX8J84ubyaZxKJZ3FMHVwn8swoMML7iQ= github.com/go-asn1-ber/asn1-ber v1.5.8/go.mod h1:hEBeB/ic+5LoWskz+yKT7vGhhPYkProFKoKdwZRWMe0= -github.com/go-chi/chi/v5 v5.0.1/go.mod h1:DslCQbL2OYiznFReuXYUmQ2hGd1aDpCnlMNITLSKoi8= github.com/go-chi/chi/v5 v5.4.0 h1:+ufzvFTGKyin26Or5DLrlIDq4ygIDBVcj7oVNeeaYFE= github.com/go-chi/chi/v5 v5.4.0/go.mod h1:d1IG7GtKPSjPyBP365PH8sAGVHk3qfB/IhjFNRH1wWM= github.com/go-chi/cors v1.2.2 h1:Jmey33TE+b+rB7fT8MUy1u0I4L+NARQlK6LhzKPSyQE= diff --git a/routers/common/forwardedheaders.go b/routers/common/forwardedheaders.go new file mode 100644 index 00000000000..c5d836052d2 --- /dev/null +++ b/routers/common/forwardedheaders.go @@ -0,0 +1,156 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package common + +import ( + "net/http" + "net/netip" + "slices" + "strconv" + "strings" + + "gitea.dev/modules/log" +) + +func ForwardedHeadersHandler(limit int, trustedProxies []string) func(h http.Handler) http.Handler { + trustAll := slices.Contains(trustedProxies, "*") + var trusted []netip.Prefix + for _, s := range trustedProxies { + if s == "*" { + continue + } + prefix, err := netip.ParsePrefix(s) + if err != nil { + addr, _ := netip.ParseAddr(s) + prefix = netip.PrefixFrom(addr, addr.BitLen()) + } + if prefix.Addr().Is4In6() && prefix.Bits() >= 96 { + prefix = netip.PrefixFrom(prefix.Addr().Unmap(), prefix.Bits()-96) + } + if !prefix.IsValid() { + log.Error("Ignoring invalid trusted proxy %q", s) + continue + } + trusted = append(trusted, prefix) + } + return func(h http.Handler) http.Handler { + return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { + if req.RemoteAddr == "@" { // unix socket + req.RemoteAddr = "127.0.0.1:0" + } + peer, _ := netip.ParseAddrPort(req.RemoteAddr) + if trustAll || slices.ContainsFunc(trusted, func(p netip.Prefix) bool { return p.Contains(peer.Addr().Unmap()) }) { + if client := forwardedClient(req.Header, limit); client.IsValid() { + req.RemoteAddr = netip.AddrPortFrom(client.Addr().Unmap().WithZone(""), client.Port()).String() + } + } + h.ServeHTTP(resp, req) + }) + } +} + +// forwardedClient reads Forwarded only without X-Forwarded-For, so proxies passing a client-sent Forwarded through stay safe +func forwardedClient(header http.Header, limit int) netip.AddrPort { + if realIPs := header["X-Real-Ip"]; len(realIPs) > 0 && realIPs[len(realIPs)-1] != "" { + client := parseNode(realIPs[len(realIPs)-1]) // the closest hop wrote the last value + if realPorts := header["X-Real-Port"]; client.Port() == 0 && len(realPorts) > 0 { + if port, err := strconv.ParseUint(realPorts[len(realPorts)-1], 10, 16); err == nil { + client = netip.AddrPortFrom(client.Addr(), uint16(port)) + } + } + return client + } + if xff := header["X-Forwarded-For"]; len(xff) > 0 { + return parseNode(elementFromEnd(xff, limit)) + } + return parseNode(forwardedFor(elementFromEnd(header["Forwarded"], limit))) +} + +// elementFromEnd skips empty elements, which RFC 9110 doesn't count +func elementFromEnd(values []string, n int) string { + list, entry := strings.Join(values, ","), "" + for list != "" && n > 0 { + var element string + list, element = cutLast(list, ',') + if element = strings.TrimSpace(element); element != "" { + entry, n = element, n-1 + } + } + return entry +} + +// forwardedFor returns nothing on a repeated "for", which RFC 7239 forbids +func forwardedFor(element string) string { + node, found := "", false + for element != "" { + var pair string + element, pair = cutLast(element, ';') + key, value, _ := strings.Cut(strings.TrimSpace(pair), "=") + if !strings.EqualFold(key, "for") { + continue + } + if found { + return "" + } + node, found = unquote(value), true + } + return node +} + +// cutLast scans from the right so a malformed client-written prefix can't shift proxy-written elements +func cutLast(s string, sep byte) (before, after string) { + if i := strings.LastIndexByte(s, sep); !strings.Contains(s[i+1:], `"`) { + return s[:max(i, 0)], s[i+1:] + } + quoted := false + for i := len(s) - 1; i >= 0; i-- { + switch s[i] { + case '"': + quoted = !quoted || (i-len(strings.TrimRight(s[:i], `\`)))%2 == 1 + case sep: + if !quoted { + return s[:i], s[i+1:] + } + } + } + return "", s +} + +func unquote(s string) string { + if len(s) < 2 || s[0] != '"' || s[len(s)-1] != '"' { + return s + } + s = s[1 : len(s)-1] + if !strings.Contains(s, `\`) { + return s + } + var b strings.Builder + for i := 0; i < len(s); i++ { + if s[i] == '\\' && i+1 < len(s) { + i++ + } + b.WriteByte(s[i]) + } + return b.String() +} + +// parseNode keeps numeric ports and drops RFC 7239 brackets and obfuscated ports +func parseNode(node string) netip.AddrPort { + if node == "" { + return netip.AddrPort{} + } + if addr, err := netip.ParseAddr(node); err == nil { + return netip.AddrPortFrom(addr, 0) + } + if addrPort, err := netip.ParseAddrPort(node); err == nil { + return addrPort + } + if strings.HasPrefix(node, "[") { + node, _, _ = strings.Cut(node[1:], "]") + } else { + node, _, _ = strings.Cut(node, ":") + } + addr, _ := netip.ParseAddr(node) + return netip.AddrPortFrom(addr, 0) +} diff --git a/routers/common/forwardedheaders_test.go b/routers/common/forwardedheaders_test.go new file mode 100644 index 00000000000..1bfaa8fd92c --- /dev/null +++ b/routers/common/forwardedheaders_test.go @@ -0,0 +1,57 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package common + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestForwardedHeadersHandler(t *testing.T) { + defaultProxies := []string{"127.0.0.0/8", "::1/128"} + cases := []struct { + name string + remoteAddr string + limit int + proxies []string + header http.Header + expected string + }{ + {"chain shorter than limit", "127.0.0.1:1234", 3, defaultProxies, http.Header{"X-Forwarded-For": {"1.1.1.1, 2.2.2.2"}}, "1.1.1.1:0"}, + {"repeated header without spaces", "127.0.0.1:1234", 2, defaultProxies, http.Header{"X-Forwarded-For": {"1.1.1.1,2.2.2.2", "3.3.3.3"}}, "2.2.2.2:0"}, + {"non-ip entry", "127.0.0.1:1234", 1, defaultProxies, http.Header{"X-Forwarded-For": {"evil,