The proxy sets Forwarded, X-Forwarded-For/-Host/-Proto and X-Real-IP from the connection, but applied a route's request header filters after that, so whoever could write a route to a backend could tell it any client address (a backend that trusts the gateway's X-Forwarded-For believes it). An HTTPRoute whose request header filter sets, adds or removes one of them (names compared case-insensitively) is now not accepted (UnsupportedValue), like the other filters we do not serve. The proxy also sets them after the filters, clearing them first: SetXForwarded appends to an X-Forwarded-For already there. The proxy test's backend now prints every X-Forwarded-For value; it printed only the first, which hid an appended one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHXzGrsxfkGk8LmwrBbHNZ
This commit is contained in:
1 parent
e7583c4177
commit
5d6b0719b3
6 files changed
+124
-9
No files matched your search
+41
-1
@@ -47,6 +47,42 @@ const (
|
|||||||
pathPrefix = "PathPrefix"
|
pathPrefix = "PathPrefix"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// forwardingHeaders are the request headers the proxy sets itself, from the connection: who the client is and what it
|
||||||
|
// asked for, over what. No route filter may change them, or whoever can write a route to a backend could tell that
|
||||||
|
// backend any client address.
|
||||||
|
var forwardingHeaders = []string{"Forwarded", "X-Forwarded-For", "X-Forwarded-Host", "X-Forwarded-Proto", "X-Real-IP"}
|
||||||
|
|
||||||
|
// forwardingHeader is the forwarding header a name denotes, if it denotes one (header names are case-insensitive).
|
||||||
|
func forwardingHeader(name string) (string, bool) {
|
||||||
|
for _, h := range forwardingHeaders {
|
||||||
|
if strings.EqualFold(name, h) {
|
||||||
|
return h, true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return "", false
|
||||||
|
}
|
||||||
|
|
||||||
|
// touchesForwarding is why a request header filter cannot be served: it sets, adds or removes a forwarding header.
|
||||||
|
func touchesForwarding(f *HTTPHeaderFilter) string {
|
||||||
|
if f == nil {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
var names []string
|
||||||
|
for _, h := range f.Set {
|
||||||
|
names = append(names, h.Name)
|
||||||
|
}
|
||||||
|
for _, h := range f.Add {
|
||||||
|
names = append(names, h.Name)
|
||||||
|
}
|
||||||
|
names = append(names, f.Remove...)
|
||||||
|
for _, n := range names {
|
||||||
|
if h, ok := forwardingHeader(n); ok {
|
||||||
|
return fmt.Sprintf("a request header filter may not change %s: the gateway sets it from the connection", h)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
|
||||||
// GatewayInput is what the API shows that the gateway policy reads.
|
// GatewayInput is what the API shows that the gateway policy reads.
|
||||||
type GatewayInput struct {
|
type GatewayInput struct {
|
||||||
Classes []*GatewayClass
|
Classes []*GatewayClass
|
||||||
@@ -638,7 +674,11 @@ func unsupportedHTTP(r *HTTPRoute) string {
|
|||||||
okFilter := func(fs []HTTPRouteFilter) string {
|
okFilter := func(fs []HTTPRouteFilter) string {
|
||||||
for _, f := range fs {
|
for _, f := range fs {
|
||||||
switch f.Type {
|
switch f.Type {
|
||||||
case filterRequestHeaderModifier, filterResponseHeaderModifier, filterRequestRedirect, filterURLRewrite:
|
case filterRequestHeaderModifier:
|
||||||
|
if msg := touchesForwarding(f.RequestHeaderModifier); msg != "" {
|
||||||
|
return msg
|
||||||
|
}
|
||||||
|
case filterResponseHeaderModifier, filterRequestRedirect, filterURLRewrite:
|
||||||
default:
|
default:
|
||||||
return fmt.Sprintf("filter %s is not supported", f.Type)
|
return fmt.Sprintf("filter %s is not supported", f.Type)
|
||||||
}
|
}
|
||||||
|
|||||||
+10
-5
@@ -293,11 +293,6 @@ func (p *Proxy) serve(s *portServer, w http.ResponseWriter, r *http.Request) {
|
|||||||
Rewrite: func(pr *httputil.ProxyRequest) {
|
Rewrite: func(pr *httputil.ProxyRequest) {
|
||||||
pr.SetURL(&url.URL{Scheme: "http", Host: ep.String()})
|
pr.SetURL(&url.URL{Scheme: "http", Host: ep.String()})
|
||||||
pr.Out.Host = pr.In.Host // forwarded unmodified unless a filter says otherwise
|
pr.Out.Host = pr.In.Host // forwarded unmodified unless a filter says otherwise
|
||||||
// SetXForwarded replaces whatever the client sent: only this hop's view is trusted
|
|
||||||
pr.SetXForwarded()
|
|
||||||
if ip, _, err := net.SplitHostPort(pr.In.RemoteAddr); err == nil {
|
|
||||||
pr.Out.Header.Set("X-Real-IP", ip)
|
|
||||||
}
|
|
||||||
for _, f := range filters {
|
for _, f := range filters {
|
||||||
switch {
|
switch {
|
||||||
case f.Type == filterRequestHeaderModifier && f.RequestHeaderModifier != nil:
|
case f.Type == filterRequestHeaderModifier && f.RequestHeaderModifier != nil:
|
||||||
@@ -312,6 +307,16 @@ func (p *Proxy) serve(s *portServer, w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// After the filters, so none can change these (the model already refuses routes that try): only this
|
||||||
|
// hop's view of the connection is passed on. Cleared first, as SetXForwarded appends to an
|
||||||
|
// X-Forwarded-For already there.
|
||||||
|
for _, h := range forwardingHeaders {
|
||||||
|
pr.Out.Header.Del(h)
|
||||||
|
}
|
||||||
|
pr.SetXForwarded()
|
||||||
|
if ip, _, err := net.SplitHostPort(pr.In.RemoteAddr); err == nil {
|
||||||
|
pr.Out.Header.Set("X-Real-IP", ip)
|
||||||
|
}
|
||||||
},
|
},
|
||||||
ModifyResponse: func(resp *http.Response) error {
|
ModifyResponse: func(resp *http.Response) error {
|
||||||
for _, f := range filters {
|
for _, f := range filters {
|
||||||
|
|||||||
@@ -53,7 +53,7 @@ func named(t *testing.T, name string) (*httptest.Server, Endpoint) {
|
|||||||
w.Header().Set("X-Backend", name)
|
w.Header().Set("X-Backend", name)
|
||||||
n, _ := io.Copy(io.Discard, r.Body)
|
n, _ := io.Copy(io.Discard, r.Body)
|
||||||
fmt.Fprintf(w, "%s path=%s host=%s xff=%s proto=%s xhost=%s realip=%s added=%s removed=%s body=%d",
|
fmt.Fprintf(w, "%s path=%s host=%s xff=%s proto=%s xhost=%s realip=%s added=%s removed=%s body=%d",
|
||||||
name, r.URL.Path, r.Host, r.Header.Get("X-Forwarded-For"), r.Header.Get("X-Forwarded-Proto"),
|
name, r.URL.Path, r.Host, strings.Join(r.Header.Values("X-Forwarded-For"), ","), r.Header.Get("X-Forwarded-Proto"),
|
||||||
r.Header.Get("X-Forwarded-Host"), r.Header.Get("X-Real-IP"), strings.Join(r.Header.Values("X-Added"), ","),
|
r.Header.Get("X-Forwarded-Host"), r.Header.Get("X-Real-IP"), strings.Join(r.Header.Values("X-Added"), ","),
|
||||||
r.Header.Get("X-Remove-Me"), n)
|
r.Header.Get("X-Remove-Me"), n)
|
||||||
}))
|
}))
|
||||||
@@ -258,6 +258,25 @@ func TestProxyRedirectsAndForwards(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The model refuses routes whose filters touch the forwarding headers; the proxy holds the line anyway.
|
||||||
|
func TestProxyForwardingHeadersSurviveFilters(t *testing.T) {
|
||||||
|
tp := newTestProxy(t)
|
||||||
|
_, a := named(t, "A")
|
||||||
|
routes := []ServeRoute{{Key: "ns/r", Rules: []ServeRule{{Matches: []HTTPRouteMatch{prefix("/")}, Backends: backend("A", a),
|
||||||
|
Filters: []HTTPRouteFilter{{Type: filterRequestHeaderModifier, RequestHeaderModifier: &HTTPHeaderFilter{
|
||||||
|
Set: []HTTPHeader{{Name: "X-Real-IP", Value: "6.6.6.6"}, {Name: "x-forwarded-host", Value: "evil.example"}},
|
||||||
|
Add: []HTTPHeader{{Name: "X-Forwarded-For", Value: "6.6.6.6"}},
|
||||||
|
Remove: []string{"X-Forwarded-Proto"},
|
||||||
|
}}}}}}}
|
||||||
|
tp.Apply([]ServeListener{{Gateway: "ns/gw", Name: "http", Address: net.ParseIP("192.0.2.1"), Port: 80, Routes: routes}})
|
||||||
|
_, body, _ := get(t, "http://"+tp.at(80)+"/", "git.levk.dev", map[string]string{"X-Forwarded-For": "7.7.7.7"})
|
||||||
|
for _, want := range []string{"xff=127.0.0.1 ", "proto=http ", "xhost=git.levk.dev ", "realip=127.0.0.1 "} {
|
||||||
|
if !strings.Contains(body, want) {
|
||||||
|
t.Fatalf("a filter changed the forwarding headers: %q (want %q)", body, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestProxyAnswersWhatItCannotRoute(t *testing.T) {
|
func TestProxyAnswersWhatItCannotRoute(t *testing.T) {
|
||||||
tp := newTestProxy(t)
|
tp := newTestProxy(t)
|
||||||
routes := []ServeRoute{{Key: "ns/r", Rules: []ServeRule{
|
routes := []ServeRoute{{Key: "ns/r", Rules: []ServeRule{
|
||||||
|
|||||||
@@ -397,6 +397,18 @@ func TestRouteAttachment(t *testing.T) {
|
|||||||
"regular expression path": {func(in *GatewayInput) {
|
"regular expression path": {func(in *GatewayInput) {
|
||||||
in.HTTPRoutes[0].Spec.Rules[0].Matches = []HTTPRouteMatch{{Path: &HTTPPathMatch{Type: ptr("RegularExpression"), Value: ptr("/.*")}}}
|
in.HTTPRoutes[0].Spec.Rules[0].Matches = []HTTPRouteMatch{{Path: &HTTPPathMatch{Type: ptr("RegularExpression"), Value: ptr("/.*")}}}
|
||||||
}, "gitea", "UnsupportedValue"},
|
}, "gitea", "UnsupportedValue"},
|
||||||
|
"filter sets X-Forwarded-For": {func(in *GatewayInput) {
|
||||||
|
in.HTTPRoutes[0].Spec.Rules[0].Filters = []HTTPRouteFilter{{Type: filterRequestHeaderModifier,
|
||||||
|
RequestHeaderModifier: &HTTPHeaderFilter{Set: []HTTPHeader{{Name: "X-Forwarded-For", Value: "6.6.6.6"}}}}}
|
||||||
|
}, "gitea", "UnsupportedValue"},
|
||||||
|
"backend filter adds x-real-ip": {func(in *GatewayInput) {
|
||||||
|
in.HTTPRoutes[0].Spec.Rules[0].BackendRefs[0].Filters = []HTTPRouteFilter{{Type: filterRequestHeaderModifier,
|
||||||
|
RequestHeaderModifier: &HTTPHeaderFilter{Add: []HTTPHeader{{Name: "x-real-ip", Value: "6.6.6.6"}}}}}
|
||||||
|
}, "gitea", "UnsupportedValue"},
|
||||||
|
"filter removes Forwarded": {func(in *GatewayInput) {
|
||||||
|
in.HTTPRoutes[0].Spec.Rules[0].Filters = []HTTPRouteFilter{{Type: filterRequestHeaderModifier,
|
||||||
|
RequestHeaderModifier: &HTTPHeaderFilter{Remove: []string{"FORWARDED"}}}}
|
||||||
|
}, "gitea", "UnsupportedValue"},
|
||||||
"wrong port": {func(in *GatewayInput) { in.TCPRoutes[0].Spec.ParentRefs[0].Port = ptr(int32(2222)) }, "gitea-ssh", "NoMatchingParent"},
|
"wrong port": {func(in *GatewayInput) { in.TCPRoutes[0].Spec.ParentRefs[0].Port = ptr(int32(2222)) }, "gitea-ssh", "NoMatchingParent"},
|
||||||
}
|
}
|
||||||
for name, c := range cases {
|
for name, c := range cases {
|
||||||
@@ -428,6 +440,29 @@ func TestRouteAttachment(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestForwardingHeadersAreTheGateways(t *testing.T) {
|
||||||
|
accepted := func(name string, value string) metav1.Condition {
|
||||||
|
in := giteaInput(t)
|
||||||
|
in.HTTPRoutes[0].Spec.Rules[0].Filters = []HTTPRouteFilter{{Type: filterRequestHeaderModifier,
|
||||||
|
RequestHeaderModifier: &HTTPHeaderFilter{Set: []HTTPHeader{{Name: name, Value: value}}}}}
|
||||||
|
for _, r := range ComputeGateways("cp", gwNodes(), in).Routes {
|
||||||
|
if r.Name == "gitea" {
|
||||||
|
return condOf(r.Status.Parents[0].Conditions, condAccepted)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
t.Fatalf("no status for route gitea")
|
||||||
|
return metav1.Condition{}
|
||||||
|
}
|
||||||
|
a := accepted("x-forwarded-proto", "https")
|
||||||
|
if a.Status != metav1.ConditionFalse || a.Reason != "UnsupportedValue" || !strings.Contains(a.Message, "X-Forwarded-Proto") {
|
||||||
|
t.Fatalf("a filter on X-Forwarded-Proto: %+v", a)
|
||||||
|
}
|
||||||
|
// a header that only looks like one of them is the route's to set
|
||||||
|
if a := accepted("X-Forwarded-Prefix", "/git"); a.Status != metav1.ConditionTrue {
|
||||||
|
t.Fatalf("a filter on X-Forwarded-Prefix: %+v", a)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestBackendNotFoundBreaksTheRule(t *testing.T) {
|
func TestBackendNotFoundBreaksTheRule(t *testing.T) {
|
||||||
in := giteaInput(t)
|
in := giteaInput(t)
|
||||||
in.HTTPRoutes[0].Spec.Rules[0].BackendRefs[0].Name = "nope"
|
in.HTTPRoutes[0].Spec.Rules[0].BackendRefs[0].Name = "nope"
|
||||||
|
|||||||
@@ -93,6 +93,20 @@ spec:
|
|||||||
- matches: [{path: {type: PathPrefix, value: /broken}}]
|
- matches: [{path: {type: PathPrefix, value: /broken}}]
|
||||||
backendRefs: [{name: missing, port: 80}]
|
backendRefs: [{name: missing, port: 80}]
|
||||||
---
|
---
|
||||||
|
# refused: the gateway sets the forwarding headers itself, so no route may change them
|
||||||
|
apiVersion: gateway.networking.k8s.io/v1
|
||||||
|
kind: HTTPRoute
|
||||||
|
metadata: {name: forge, namespace: gwtest}
|
||||||
|
spec:
|
||||||
|
parentRefs: [{name: gwtest, namespace: gateway-system, sectionName: https}]
|
||||||
|
hostnames: [gw.test]
|
||||||
|
rules:
|
||||||
|
- matches: [{path: {type: PathPrefix, value: /forge}}]
|
||||||
|
filters:
|
||||||
|
- type: RequestHeaderModifier
|
||||||
|
requestHeaderModifier: {set: [{name: X-Forwarded-For, value: 6.6.6.6}]}
|
||||||
|
backendRefs: [{name: echo-a, port: 80}]
|
||||||
|
---
|
||||||
apiVersion: gateway.networking.k8s.io/v1
|
apiVersion: gateway.networking.k8s.io/v1
|
||||||
kind: HTTPRoute
|
kind: HTTPRoute
|
||||||
metadata: {name: redirect, namespace: gwtest}
|
metadata: {name: redirect, namespace: gwtest}
|
||||||
|
|||||||
@@ -7,7 +7,8 @@
|
|||||||
# The machine running this script is the outside client. Asserts, in order:
|
# The machine running this script is the outside client. Asserts, in order:
|
||||||
# G0 GatewayClass reweave is accepted
|
# G0 GatewayClass reweave is accepted
|
||||||
# G1 the Gateway is accepted and programmed at ADDR, its listeners with their routes attached
|
# G1 the Gateway is accepted and programmed at ADDR, its listeners with their routes attached
|
||||||
# G2 route statuses: accepted and resolved, the broken one ResolvedRefs False (BackendNotFound)
|
# G2 route statuses: accepted and resolved, the broken one ResolvedRefs False (BackendNotFound), and the one whose
|
||||||
|
# filter sets X-Forwarded-For refused (Accepted False, UnsupportedValue)
|
||||||
# G3 TCP listener: forwarded in the kernel to a pod on another node, which sees this client's own address
|
# G3 TCP listener: forwarded in the kernel to a pod on another node, which sees this client's own address
|
||||||
# G4 HTTPS from outside: the listener's certificate by SNI, HTTP/2, routing by exact path, by prefix with a
|
# G4 HTTPS from outside: the listener's certificate by SNI, HTTP/2, routing by exact path, by prefix with a
|
||||||
# rewrite, and by default; the backend sees this client (the same address as G3) in X-Forwarded-For
|
# rewrite, and by default; the backend sees this client (the same address as G3) in X-Forwarded-For
|
||||||
@@ -84,7 +85,8 @@ for r in "httproute main" "httproute redirect" "tcproute raw"; do
|
|||||||
route_is $r ResolvedRefs "True/ResolvedRefs" || fail "G2: $r resolved: $(route $r ResolvedRefs)"
|
route_is $r ResolvedRefs "True/ResolvedRefs" || fail "G2: $r resolved: $(route $r ResolvedRefs)"
|
||||||
done
|
done
|
||||||
route_is httproute broken ResolvedRefs "False/BackendNotFound" || fail "G2: broken: $(route httproute broken ResolvedRefs)"
|
route_is httproute broken ResolvedRefs "False/BackendNotFound" || fail "G2: broken: $(route httproute broken ResolvedRefs)"
|
||||||
ok "G2. routes accepted and resolved; the broken one ResolvedRefs False (BackendNotFound)"
|
eventually 30 route_is httproute forge Accepted "False/UnsupportedValue" || fail "G2: forge: $(route httproute forge Accepted)"
|
||||||
|
ok "G2. routes accepted and resolved; the broken one ResolvedRefs False (BackendNotFound); the forging one refused"
|
||||||
|
|
||||||
tcp_hostname() { [ "$(curl -s --max-time 5 "http://$ADDR:18022/hostname")" = echo-b ]; }
|
tcp_hostname() { [ "$(curl -s --max-time 5 "http://$ADDR:18022/hostname")" = echo-b ]; }
|
||||||
eventually 30 tcp_hostname || fail "G3: TCP listener did not reach echo-b"
|
eventually 30 tcp_hostname || fail "G3: TCP listener did not reach echo-b"
|
||||||
|
|||||||
Reference in new issue
Block a user