Believe X-Forwarded-* only from router.trusted_proxies
A new internal/clientip says who sent a request and how: the connection's address, scheme and Host, or through a configured proxy what it forwarded. The base URL in pages and setup scripts, isHTTPS for cookies, and the per-address limits of auth and the sign-in forms use it. /setup scripts are no-store. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
44cd58005a
commit
fe5b7f88f0
14 files changed
+231
-34
No files matched your search
@@ -136,6 +136,15 @@ APIs change between minors without a deprecation period.
|
||||
30 a minute, signup 5), and a password hash that cannot get a turn within
|
||||
ten seconds answers "busy" instead of holding the request. `server init`
|
||||
now writes `signup_open: false`.
|
||||
- **Forwarding headers are believed only from the proxies named.**
|
||||
`X-Forwarded-Host` and `-Proto` were taken from anyone, and decide the
|
||||
gateway address written into the `/setup` scripts clients pipe to a
|
||||
shell; behind a caching proxy one request could poison them for others.
|
||||
They — and `X-Forwarded-For`, now used for the per-address limits — are
|
||||
believed only from `router.trusted_proxies` (addresses or CIDR prefixes).
|
||||
**Behind a reverse proxy, set it**, or the pages and scripts name the
|
||||
address the proxy reached and every client counts as the proxy. `/setup`
|
||||
scripts are served `no-store`.
|
||||
|
||||
### Everything that can be changed on a running cluster, from the console
|
||||
|
||||
|
||||
@@ -152,6 +152,7 @@ rather than repeated.
|
||||
| `inject_usage` | `bool` | [Router](#router) |
|
||||
| `tenant_label` | `string` | [Router](#router) |
|
||||
| `max_body_bytes` | `int64` | [Router](#router) |
|
||||
| `trusted_proxies` | `[]string` | [Router](#router) |
|
||||
| `timeouts` | `Timeouts` | [Router](#router) |
|
||||
| `scoring` | `Scoring` | [Router](#router) |
|
||||
| `admission` | `Admission` | [Router](#router) |
|
||||
@@ -1226,6 +1227,15 @@ team is what an alert is usually about; "team" is the default.
|
||||
|
||||
MaxBodyBytes bounds the request body the router will read.
|
||||
|
||||
### `trusted_proxies`
|
||||
|
||||
`[]string`
|
||||
|
||||
TrustedProxies are the reverse proxies in front of the client port,
|
||||
as addresses or CIDR prefixes. Their X-Forwarded-For, -Proto and -Host
|
||||
are believed; anyone else's are not, since they decide the address
|
||||
written into the setup scripts and whose attempts are counted where.
|
||||
|
||||
### `timeouts`
|
||||
|
||||
`Timeouts`
|
||||
|
||||
@@ -4,9 +4,10 @@ import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"log/slog"
|
||||
"net"
|
||||
"net/http"
|
||||
"strings"
|
||||
|
||||
"gitea.mynodes.xyz/ilker/llmgw/internal/clientip"
|
||||
)
|
||||
|
||||
// Middleware authenticates every request before it reaches the router.
|
||||
@@ -109,11 +110,6 @@ func WriteForbiddenModel(w http.ResponseWriter, model string) {
|
||||
}})
|
||||
}
|
||||
|
||||
// clientAddr is the address a request came from, without its port.
|
||||
func clientAddr(r *http.Request) string {
|
||||
host, _, err := net.SplitHostPort(r.RemoteAddr)
|
||||
if err != nil {
|
||||
return r.RemoteAddr
|
||||
}
|
||||
return host
|
||||
}
|
||||
// clientAddr is the address a request came from: through a trusted proxy,
|
||||
// the client's.
|
||||
func clientAddr(r *http.Request) string { return clientip.Addr(r) }
|
||||
@@ -16,6 +16,7 @@ import (
|
||||
"syscall"
|
||||
"time"
|
||||
|
||||
"gitea.mynodes.xyz/ilker/llmgw/internal/clientip"
|
||||
"gitea.mynodes.xyz/ilker/llmgw/internal/control/inventory"
|
||||
"gitea.mynodes.xyz/ilker/llmgw/internal/control/modelserve"
|
||||
"gitea.mynodes.xyz/ilker/llmgw/internal/gguf"
|
||||
@@ -361,6 +362,9 @@ func newServerCmd() *cobra.Command {
|
||||
if err := checkAdminListen(cfg); err != nil {
|
||||
return err
|
||||
}
|
||||
if bad := clientip.SetTrusted(cfg.Router.TrustedProxies); len(bad) > 0 {
|
||||
return fmt.Errorf("router.trusted_proxies: %q is not an address or a CIDR prefix", bad[0])
|
||||
}
|
||||
adminToken, err := ensureAdminToken(adminTokenPath(configPath, cfg))
|
||||
if err != nil {
|
||||
return err
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
// Package clientip says who sent a request and how, believing a proxy's
|
||||
// X-Forwarded-* headers only when the request came from a proxy the
|
||||
// configuration names.
|
||||
//
|
||||
// Believed from anyone, X-Forwarded-Host decides the address written into the
|
||||
// setup scripts clients pipe to a shell, and X-Forwarded-For lets one client
|
||||
// claim to be any number of others to whatever counts attempts per address.
|
||||
// Believed from nobody, every client behind a proxy is one address, and a
|
||||
// limit per address is a limit on all of them together.
|
||||
package clientip
|
||||
|
||||
import (
|
||||
"net"
|
||||
"net/http"
|
||||
"net/netip"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
)
|
||||
|
||||
var trusted atomic.Pointer[[]netip.Prefix]
|
||||
|
||||
// SetTrusted names the proxies whose forwarding headers are believed:
|
||||
// addresses or CIDR prefixes. Anything unparsable is skipped and returned.
|
||||
func SetTrusted(list []string) (skipped []string) {
|
||||
var out []netip.Prefix
|
||||
for _, s := range list {
|
||||
s = strings.TrimSpace(s)
|
||||
if p, err := netip.ParsePrefix(s); err == nil {
|
||||
out = append(out, p.Masked())
|
||||
continue
|
||||
}
|
||||
if a, err := netip.ParseAddr(s); err == nil {
|
||||
out = append(out, netip.PrefixFrom(a, a.BitLen()))
|
||||
continue
|
||||
}
|
||||
skipped = append(skipped, s)
|
||||
}
|
||||
trusted.Store(&out)
|
||||
return skipped
|
||||
}
|
||||
|
||||
func isTrusted(addr string) bool {
|
||||
a, err := netip.ParseAddr(addr)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
a = a.Unmap()
|
||||
list := trusted.Load()
|
||||
if list == nil {
|
||||
return false
|
||||
}
|
||||
for _, p := range *list {
|
||||
if p.Contains(a) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// peer is the address the connection came from.
|
||||
func peer(r *http.Request) string {
|
||||
host, _, err := net.SplitHostPort(r.RemoteAddr)
|
||||
if err != nil {
|
||||
return r.RemoteAddr
|
||||
}
|
||||
return host
|
||||
}
|
||||
|
||||
// fromProxy reports whether the request came through a trusted proxy.
|
||||
func fromProxy(r *http.Request) bool { return isTrusted(peer(r)) }
|
||||
|
||||
// Addr is the client's address: the connection's, or through trusted
|
||||
// proxies the nearest address in X-Forwarded-For that is not one of them.
|
||||
func Addr(r *http.Request) string {
|
||||
p := peer(r)
|
||||
if !isTrusted(p) {
|
||||
return p
|
||||
}
|
||||
hops := strings.Split(r.Header.Get("X-Forwarded-For"), ",")
|
||||
for i := len(hops) - 1; i >= 0; i-- {
|
||||
h := strings.TrimSpace(hops[i])
|
||||
if h == "" {
|
||||
continue
|
||||
}
|
||||
if !isTrusted(h) {
|
||||
return h
|
||||
}
|
||||
}
|
||||
return p
|
||||
}
|
||||
|
||||
// Proto is "https" when the client spoke TLS: to this server, or to a
|
||||
// trusted proxy that says so.
|
||||
func Proto(r *http.Request) string {
|
||||
if r.TLS != nil {
|
||||
return "https"
|
||||
}
|
||||
if fromProxy(r) {
|
||||
if v := first(r.Header.Get("X-Forwarded-Proto")); v != "" {
|
||||
return strings.ToLower(v)
|
||||
}
|
||||
}
|
||||
return "http"
|
||||
}
|
||||
|
||||
// Host is the host the client asked for: Host, or what a trusted proxy says
|
||||
// it was asked for.
|
||||
func Host(r *http.Request) string {
|
||||
if fromProxy(r) {
|
||||
if v := first(r.Header.Get("X-Forwarded-Host")); v != "" {
|
||||
return v
|
||||
}
|
||||
}
|
||||
return r.Host
|
||||
}
|
||||
|
||||
func first(v string) string {
|
||||
if i := strings.IndexByte(v, ','); i >= 0 {
|
||||
v = v[:i]
|
||||
}
|
||||
return strings.TrimSpace(v)
|
||||
}
|
||||
@@ -0,0 +1,34 @@
|
||||
package clientip
|
||||
|
||||
import (
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestForwardingHeadersAreBelievedOnlyFromTrustedProxies(t *testing.T) {
|
||||
r := httptest.NewRequest("GET", "/setup/x", nil)
|
||||
r.RemoteAddr = "203.0.113.9:4000"
|
||||
r.Header.Set("X-Forwarded-For", "10.9.9.9")
|
||||
r.Header.Set("X-Forwarded-Host", "evil.example")
|
||||
r.Header.Set("X-Forwarded-Proto", "https")
|
||||
r.Host = "gw.example:8080"
|
||||
|
||||
SetTrusted(nil)
|
||||
if Addr(r) != "203.0.113.9" || Host(r) != "gw.example:8080" || Proto(r) != "http" {
|
||||
t.Errorf("believed an untrusted client: %s %s %s", Addr(r), Host(r), Proto(r))
|
||||
}
|
||||
|
||||
if bad := SetTrusted([]string{"203.0.113.0/24", "10.0.0.5", "nonsense"}); len(bad) != 1 {
|
||||
t.Errorf("skipped %v", bad)
|
||||
}
|
||||
r.Header.Set("X-Forwarded-For", "198.51.100.7, 10.0.0.5")
|
||||
if Addr(r) != "198.51.100.7" || Host(r) != "evil.example" || Proto(r) != "https" {
|
||||
t.Errorf("through trusted proxies: %s %s %s", Addr(r), Host(r), Proto(r))
|
||||
}
|
||||
// A client prepending addresses gains nothing: the nearest untrusted one
|
||||
// is the one the proxy saw.
|
||||
r.Header.Set("X-Forwarded-For", "1.2.3.4, 198.51.100.7")
|
||||
if Addr(r) != "198.51.100.7" {
|
||||
t.Errorf("spoofed: %s", Addr(r))
|
||||
}
|
||||
}
|
||||
@@ -409,6 +409,12 @@ type Router struct {
|
||||
// MaxBodyBytes bounds the request body the router will read.
|
||||
MaxBodyBytes int64 `koanf:"max_body_bytes"`
|
||||
|
||||
// TrustedProxies are the reverse proxies in front of the client port,
|
||||
// as addresses or CIDR prefixes. Their X-Forwarded-For, -Proto and -Host
|
||||
// are believed; anyone else's are not, since they decide the address
|
||||
// written into the setup scripts and whose attempts are counted where.
|
||||
TrustedProxies []string `koanf:"trusted_proxies"`
|
||||
|
||||
Timeouts Timeouts `koanf:"timeouts"`
|
||||
Scoring Scoring `koanf:"scoring"`
|
||||
Admission Admission `koanf:"admission"`
|
||||
|
||||
@@ -182,6 +182,7 @@ var Reference = []Block{
|
||||
{Key: "inject_usage", Type: "bool", Doc: "InjectUsage asks engines for usage even when the client did not, so token\naccounting works regardless of client behaviour.", Nested: ""},
|
||||
{Key: "tenant_label", Type: "string", Doc: "TenantLabel is the level of the tree the per-tenant request series are\nlabelled by — llmgw_tenant_requests_total and its siblings: \"org\",\n\"team\" or \"project\", or \"none\" for no such series. A label per project\non a cluster of thousands is more series than Prometheus wants, and a\nteam is what an alert is usually about; \"team\" is the default.", Nested: ""},
|
||||
{Key: "max_body_bytes", Type: "int64", Doc: "MaxBodyBytes bounds the request body the router will read.", Nested: ""},
|
||||
{Key: "trusted_proxies", Type: "[]string", Doc: "TrustedProxies are the reverse proxies in front of the client port,\nas addresses or CIDR prefixes. Their X-Forwarded-For, -Proto and -Host\nare believed; anyone else's are not, since they decide the address\nwritten into the setup scripts and whose attempts are counted where.", Nested: ""},
|
||||
{Key: "timeouts", Type: "Timeouts", Doc: "", Nested: "config.Timeouts"},
|
||||
{Key: "scoring", Type: "Scoring", Doc: "", Nested: "config.Scoring"},
|
||||
{Key: "admission", Type: "Admission", Doc: "", Nested: "config.Admission"},
|
||||
|
||||
@@ -14,6 +14,7 @@ import (
|
||||
"time"
|
||||
|
||||
"gitea.mynodes.xyz/ilker/llmgw/internal/account"
|
||||
"gitea.mynodes.xyz/ilker/llmgw/internal/clientip"
|
||||
"gitea.mynodes.xyz/ilker/llmgw/internal/store"
|
||||
)
|
||||
|
||||
@@ -569,10 +570,7 @@ func clearSessionCookie(w http.ResponseWriter, r *http.Request) {
|
||||
// Secure when the connection is plain — would break sign-in over http://, which
|
||||
// is why it is not simply hardcoded.
|
||||
func isHTTPS(r *http.Request) bool {
|
||||
if r.TLS != nil {
|
||||
return true
|
||||
}
|
||||
return strings.EqualFold(firstValue(r.Header.Get("X-Forwarded-Proto")), "https")
|
||||
return clientip.Proto(r) == "https"
|
||||
}
|
||||
|
||||
func noStore(w http.ResponseWriter) {
|
||||
|
||||
@@ -131,6 +131,7 @@ func strip(s string) string {
|
||||
}
|
||||
|
||||
func TestTheExamplesUseTheAddressTheUserReached(t *testing.T) {
|
||||
trustTestProxy(t)
|
||||
f := &fakeClaims{secret: "sk-llmgw-thesecret"}
|
||||
mux := http.NewServeMux()
|
||||
mux.Handle("POST /claim/{token}", Claim(f, "v1", nil))
|
||||
|
||||
@@ -176,6 +176,9 @@ func (p *AccountPages) connect(w http.ResponseWriter, r *http.Request) {
|
||||
// was there. Served without a session — it holds nothing a stranger could
|
||||
// use: this gateway's address, which they already typed, and a model name.
|
||||
func (p *AccountPages) setupScript(w http.ResponseWriter, r *http.Request) {
|
||||
// Never from a cache: the script names this gateway's address, and one
|
||||
// cached for another request's Host would send a client's key elsewhere.
|
||||
w.Header().Set("Cache-Control", "no-store")
|
||||
tool := r.PathValue("tool")
|
||||
model := r.URL.Query().Get("model")
|
||||
if !safeModel.MatchString(model) {
|
||||
|
||||
@@ -4,10 +4,11 @@ import (
|
||||
"crypto/rand"
|
||||
"crypto/subtle"
|
||||
"encoding/hex"
|
||||
"net"
|
||||
"net/http"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"gitea.mynodes.xyz/ilker/llmgw/internal/clientip"
|
||||
)
|
||||
|
||||
// The sign-in and signup forms are the two a browser posts before it has a
|
||||
@@ -101,11 +102,6 @@ func rationed(q *ration, next http.HandlerFunc) http.HandlerFunc {
|
||||
}
|
||||
}
|
||||
|
||||
// clientAddr is the address a request came from, without its port.
|
||||
func clientAddr(r *http.Request) string {
|
||||
host, _, err := net.SplitHostPort(r.RemoteAddr)
|
||||
if err != nil {
|
||||
return r.RemoteAddr
|
||||
}
|
||||
return host
|
||||
}
|
||||
// clientAddr is the address a request came from: through a trusted proxy,
|
||||
// the client's.
|
||||
func clientAddr(r *http.Request) string { return clientip.Addr(r) }
|
||||
+6
-12
@@ -4,6 +4,8 @@ import (
|
||||
"net/http"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"gitea.mynodes.xyz/ilker/llmgw/internal/clientip"
|
||||
)
|
||||
|
||||
// LandingData is what the landing page shows. It is deliberately almost
|
||||
@@ -58,18 +60,10 @@ func Landing(version string, authEnabled, signupOpen bool, contact, notice strin
|
||||
// is why they are only reflected into a displayed URL and never used for a
|
||||
// decision.
|
||||
func baseURL(r *http.Request) string {
|
||||
scheme := "http"
|
||||
switch {
|
||||
case r.TLS != nil:
|
||||
scheme = "https"
|
||||
case r.Header.Get("X-Forwarded-Proto") != "":
|
||||
scheme = firstValue(r.Header.Get("X-Forwarded-Proto"))
|
||||
}
|
||||
|
||||
host := r.Host
|
||||
if fwd := r.Header.Get("X-Forwarded-Host"); fwd != "" {
|
||||
host = firstValue(fwd)
|
||||
}
|
||||
// What a trusted proxy says it was asked for, else what this server was:
|
||||
// the address goes into scripts clients pipe to a shell, so it is not
|
||||
// taken from whoever sends a header.
|
||||
scheme, host := clientip.Proto(r), clientip.Host(r)
|
||||
if host == "" {
|
||||
host = "localhost:8080"
|
||||
}
|
||||
|
||||
@@ -12,6 +12,7 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"gitea.mynodes.xyz/ilker/llmgw/internal/clientip"
|
||||
"gitea.mynodes.xyz/ilker/llmgw/internal/control/adminapi"
|
||||
"gitea.mynodes.xyz/ilker/llmgw/internal/control/fabrics"
|
||||
"gitea.mynodes.xyz/ilker/llmgw/internal/store"
|
||||
@@ -65,6 +66,7 @@ func TestLandingPageWarnsWhenTheGatewayIsOpen(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestTheExampleURLSurvivesAReverseProxy(t *testing.T) {
|
||||
trustTestProxy(t)
|
||||
// The process sees http and an internal host; the client used https and a
|
||||
// public name. An example naming the internal address is one nobody can
|
||||
// paste anywhere.
|
||||
@@ -81,6 +83,7 @@ func TestTheExampleURLSurvivesAReverseProxy(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestForwardedHeaderChainsTakeTheFirstEntry(t *testing.T) {
|
||||
trustTestProxy(t)
|
||||
// A chain of proxies appends rather than replaces.
|
||||
_, body := get(t, Landing("v1", true, false, "", ""), "http://internal/", map[string]string{
|
||||
"X-Forwarded-Proto": "https, http",
|
||||
@@ -582,6 +585,7 @@ func TestEveryClientExampleIsInThePageItself(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestAHostileForwardedHostCannotWriteMarkup(t *testing.T) {
|
||||
trustTestProxy(t)
|
||||
// Nothing overwrites X-Forwarded-Host when the gateway is reached directly,
|
||||
// so its value is whatever the caller sent, and it is now reflected into
|
||||
// several places on the page rather than one. Escaping is the template's
|
||||
@@ -687,3 +691,22 @@ type emptyFabrics struct{ fakeSource }
|
||||
|
||||
func (emptyFabrics) Summary() adminapi.Summary { return adminapi.Summary{} }
|
||||
func (emptyFabrics) Fabrics() fabrics.Report { return fabrics.Report{} }
|
||||
|
||||
// trustTestProxy believes forwarding headers from where httptest's requests
|
||||
// come from, as a deployment behind a proxy it names would.
|
||||
func trustTestProxy(t *testing.T) {
|
||||
t.Helper()
|
||||
clientip.SetTrusted([]string{"192.0.2.1"})
|
||||
t.Cleanup(func() { clientip.SetTrusted(nil) })
|
||||
}
|
||||
|
||||
func TestForwardingHeadersFromAnUntrustedClientAreIgnored(t *testing.T) {
|
||||
clientip.SetTrusted(nil)
|
||||
_, body := get(t, Landing("v1", true, false, "", ""), "http://llmgw.example/", map[string]string{
|
||||
"X-Forwarded-Proto": "https",
|
||||
"X-Forwarded-Host": "attacker.example",
|
||||
})
|
||||
if strings.Contains(body, "attacker.example") || !strings.Contains(body, "http://llmgw.example/v1") {
|
||||
t.Error("a client's own X-Forwarded-Host decided the address on the page")
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user