From 696ae6e8d332fcc1652b3f72c0a056247502cbc5 Mon Sep 17 00:00:00 2001 From: Alex Savanovich <40720931+savme@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:00:00 +0100 Subject: [PATCH 1/2] fix: add misdirected request e2e tests --- Taskfile.test-infra.yml | 2 +- test/e2e-edge/README.md | 10 +- .../misdirected-request/chainsaw-test.yaml | 355 ++++++++++++++++++ 3 files changed, 365 insertions(+), 2 deletions(-) create mode 100644 test/e2e-edge/misdirected-request/chainsaw-test.yaml diff --git a/Taskfile.test-infra.yml b/Taskfile.test-infra.yml index 3cab60ab..6c4f6035 100644 --- a/Taskfile.test-infra.yml +++ b/Taskfile.test-infra.yml @@ -1100,7 +1100,7 @@ tasks: vars: # Scenarios authored against this env's cluster names and downstream path. # Older fixtures targeting the previous cluster names are excluded here. - DEFAULT_SCENARIOS: extension-server-smoke waf-enforcement waf-section-scoping branded-error-page connector-offline-503 atomic-reject-isolation oidc-missing-secret-isolation networkservice-endpoints + DEFAULT_SCENARIOS: extension-server-smoke waf-enforcement waf-section-scoping branded-error-page connector-offline-503 atomic-reject-isolation oidc-missing-secret-isolation networkservice-endpoints misdirected-request # CLI_ARGS (after --) wins; else SCENARIOS env; else the default set. SELECTED: '{{.CLI_ARGS | default .SCENARIOS | default .DEFAULT_SCENARIOS}}' deps: diff --git a/test/e2e-edge/README.md b/test/e2e-edge/README.md index 4c9fbdc3..ba8a826f 100644 --- a/test/e2e-edge/README.md +++ b/test/e2e-edge/README.md @@ -66,6 +66,14 @@ upgrade gate: it is green on a fail-safe version (v1.7.x, v1.9.2) and flips red EG runs v1.8.x while the upstream bug is unfixed, so "green" never gets misread as "the upstream bug is fixed". +### A site never answers for another site's connection +Hostnames on the edge share an IP and a certificate, so a browser reuses one +site's connection for another. A request that arrives on the wrong site's +connection must be answered 421, which tells the browser to retry on a +connection of its own, not 404. The test negotiates TLS for one site, asks for +another, and expects 421, while each site still serves 200 on its own and an +unknown hostname over plain HTTP still gets 404. + ## Running them The scenarios run against the production-fidelity environment: @@ -83,7 +91,7 @@ environment these assume. - Scenario folders (`waf-enforcement/`, `connector-offline-503/`, `branded-error-page/`, `atomic-reject-isolation/`, - `oidc-missing-secret-isolation/`) — one guarantee each. + `oidc-missing-secret-isolation/`, `misdirected-request/`) — one guarantee each. - `_steps/` — shared, reusable checks (send-a-request, confirm-configuration, capture-the-build-marker) so every scenario asserts behavior the same way. - `_fixtures/` — the supporting pieces a scenario needs (a sample backend, an diff --git a/test/e2e-edge/misdirected-request/chainsaw-test.yaml b/test/e2e-edge/misdirected-request/chainsaw-test.yaml new file mode 100644 index 00000000..7c0cbe20 --- /dev/null +++ b/test/e2e-edge/misdirected-request/chainsaw-test.yaml @@ -0,0 +1,355 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +# +# A request for one site on another site's connection is answered 421. +# +# Hostnames on the edge share an IP and a certificate, so a browser reuses one +# site's HTTP/2 or HTTP/3 connection for another. The request then arrives in the +# first site's route configuration, which has no virtual host for it. Without +# the extension server's catch-all that is a 404 the browser cannot recover +# from; with it, a 421 tells the browser to retry on a connection of its own. +# +# Two gateways stand in for two tenants: one HTTPS listener each, both serving +# the same certificate, merged onto the edge's port 443 as production merges +# them. HTTP/1.1 exercises the same route configuration a reused HTTP/2 or +# HTTP/3 connection does: TLS is negotiated for site A, the request names site B. +# +# The gateways are created on the downstream cluster directly, as in +# atomic-reject-isolation; the namespace carries the gateway-watch label at +# creation. +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: misdirected-request +spec: + concurrent: false + cluster: nso-upstream + bindings: + - name: downstreamGatewayClass + value: datum-downstream-gateway-e2e + - name: siteA + value: misdirect-a.e2e.env.datum.net + - name: siteB + value: misdirect-b.e2e.env.datum.net + steps: + - name: Two tenants' gateways sharing one certificate + try: + - create: + cluster: nso-downstream + resource: + apiVersion: v1 + kind: Namespace + metadata: + name: (join('-', ['md', $namespace])) + labels: + meta.datumapis.com/upstream-cluster-name: cluster-single + - apply: + cluster: nso-downstream + resource: + apiVersion: v1 + kind: Namespace + metadata: + name: datum-downstream-gateway-hostnames + - create: + cluster: nso-downstream + resource: + apiVersion: cert-manager.io/v1 + kind: Issuer + metadata: + name: md-selfsigned + namespace: (join('-', ['md', $namespace])) + spec: + selfSigned: {} + - create: + cluster: nso-downstream + resource: + apiVersion: cert-manager.io/v1 + kind: Certificate + metadata: + name: md-shared + namespace: (join('-', ['md', $namespace])) + spec: + secretName: md-shared-tls + commonName: ($siteA) + dnsNames: + - ($siteA) + - ($siteB) + issuerRef: + name: md-selfsigned + kind: Issuer + - assert: + timeout: 90s + cluster: nso-downstream + resource: + apiVersion: cert-manager.io/v1 + kind: Certificate + metadata: + name: md-shared + namespace: (join('-', ['md', $namespace])) + status: + conditions: + - type: Ready + status: "True" + - create: + cluster: nso-downstream + resource: + apiVersion: v1 + kind: Pod + metadata: + name: echo-backend + namespace: (join('-', ['md', $namespace])) + labels: + purpose: md-echo + spec: + containers: + - name: backend + image: ghcr.io/mccutchen/go-httpbin:2.18.1 + command: ["/bin/go-httpbin"] + args: ["-host", "0.0.0.0", "-port", "8080"] + terminationGracePeriodSeconds: 0 + - create: + cluster: nso-downstream + resource: + apiVersion: v1 + kind: Service + metadata: + name: echo-backend + namespace: (join('-', ['md', $namespace])) + spec: + ports: + - name: http + port: 8080 + targetPort: 8080 + selector: + purpose: md-echo + - script: + cluster: nso-downstream + env: + - name: MD_NS + value: (join('-', ['md', $namespace])) + - name: GATEWAY_CLASS + value: ($downstreamGatewayClass) + - name: SITE_A + value: ($siteA) + - name: SITE_B + value: ($siteB) + content: | + set -eu + for site in a b; do + host=$SITE_A; [ "$site" = b ] && host=$SITE_B + cat </dev/null || true) + echo "attempt ${i}: ${host} -> ${code}" + if [ "${code}" = "200" ]; then ok=yes; break; fi + sleep 5 + done + [ -n "$ok" ] || { echo "ERROR: ${host} never served 200"; exit 1; } + done + + - name: A request for site B on site A's connection is answered 421 + description: | + The verdict. TLS is negotiated for site A and the request names site B, + which is what a browser's reused connection carries. Before the + catch-all this was a 404. + try: + - script: + timeout: 300s + cluster: nso-downstream + env: + - name: OWNING_GC + value: ($downstreamGatewayClass) + - name: SITE_A + value: ($siteA) + - name: SITE_B + value: ($siteB) + content: | + set -eu + IP=$(kubectl get svc -n datum-downstream-gateway \ + -l gateway.envoyproxy.io/owning-gatewayclass=$OWNING_GC \ + -o jsonpath='{.items[0].spec.clusterIP}') + for i in $(seq 1 24); do + code=$(kubectl -n default exec conntest -- \ + curl -ksS -o /dev/null -w '%{http_code}' --max-time 10 \ + --resolve "${SITE_A}:443:${IP}" -H "Host: ${SITE_B}" \ + "https://${SITE_A}/status/200" 2>/dev/null || true) + echo "attempt ${i}: SNI ${SITE_A}, Host ${SITE_B} -> ${code}" + if [ "${code}" = "421" ]; then echo "OK: misdirected request answered 421"; exit 0; fi + sleep 5 + done + echo "ERROR: misdirected request was not answered 421" + exit 1 + + - name: The proxy runs the configuration the extension server built + description: | + The 421 is only the guard's if the proxy carries what the extension + server produced, not stale or partially rejected configuration. + use: + template: ../_steps/assert-config-dump.yaml + with: + bindings: + - { name: envoyPodSelector, value: gateway.envoyproxy.io/owning-gatewayclass=datum-downstream-gateway-e2e } + - { name: envoyNamespace, value: datum-downstream-gateway } + - { name: extServerSelector, value: app.kubernetes.io/component=envoy-gateway-extension-server } + - { name: extServerNamespace, value: network-services-operator-system } + - { name: dataplaneCluster, value: nso-downstream } + + - name: Plain HTTP is left alone + description: | + Without SNI a plaintext request cannot be misdirected, so an unknown + hostname on port 80 must not become a 421 a client would retry for + nothing. + try: + - script: + timeout: 120s + cluster: nso-downstream + env: + - name: OWNING_GC + value: ($downstreamGatewayClass) + content: | + set -eu + IP=$(kubectl get svc -n datum-downstream-gateway \ + -l gateway.envoyproxy.io/owning-gatewayclass=$OWNING_GC \ + -o jsonpath='{.items[0].spec.clusterIP}') + code=$(kubectl -n default exec conntest -- \ + curl -sS -o /dev/null -w '%{http_code}' --max-time 10 \ + -H "Host: nowhere.e2e.env.datum.net" "http://${IP}/" 2>/dev/null || true) + echo "plain HTTP, unknown host -> ${code}" + [ "${code}" != "421" ] || { echo "ERROR: plaintext must not answer 421"; exit 1; } + catch: + - script: + cluster: nso-downstream + env: + - name: OWNING_GC + value: ($downstreamGatewayClass) + content: | + kubectl -n datum-downstream-gateway logs -l gateway.envoyproxy.io/owning-gatewayclass=$OWNING_GC -c envoy --tail=100 From 368aaa31d0fbc79ab8f97c33e6adad521286e589 Mon Sep 17 00:00:00 2001 From: Alex Savanovich <40720931+savme@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:06:49 +0100 Subject: [PATCH 2/2] fix: add misdirected request guard --- internal/extensionserver/metrics/metrics.go | 9 ++ .../extensionserver/mutate/misdirected.go | 90 ++++++++++++++ .../mutate/misdirected_test.go | 114 ++++++++++++++++++ internal/extensionserver/server/server.go | 7 ++ 4 files changed, 220 insertions(+) create mode 100644 internal/extensionserver/mutate/misdirected.go create mode 100644 internal/extensionserver/mutate/misdirected_test.go diff --git a/internal/extensionserver/metrics/metrics.go b/internal/extensionserver/metrics/metrics.go index 47033245..24031c5c 100644 --- a/internal/extensionserver/metrics/metrics.go +++ b/internal/extensionserver/metrics/metrics.go @@ -112,6 +112,15 @@ var ( }, ) + // MisdirectedGuardsTotal counts route configurations given the 421 + // Misdirected Request catch-all, across all hook invocations. + MisdirectedGuardsTotal = promauto.NewCounter( + prometheus.CounterOpts{ + Name: "nso_extension_misdirected_guards_total", + Help: "Total route configurations given the 421 Misdirected Request catch-all across all hook invocations.", + }, + ) + // WAFRouteMutationsTotal counts total per-route TrafficProtectionPolicy (WAF) // config applications across all hook invocations. WAFRouteMutationsTotal = promauto.NewCounter( diff --git a/internal/extensionserver/mutate/misdirected.go b/internal/extensionserver/mutate/misdirected.go new file mode 100644 index 00000000..e29cbc5f --- /dev/null +++ b/internal/extensionserver/mutate/misdirected.go @@ -0,0 +1,90 @@ +package mutate + +import ( + "slices" + + listenerv3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" + routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" + hcmv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3" + "k8s.io/apimachinery/pkg/util/sets" +) + +const ( + misdirectedVirtualHostName = "datum-misdirected-request" + misdirectedStatus = 421 +) + +// GuardMisdirectedRequests answers 421 instead of 404 when a browser reuses one +// site's connection for another, so it retries on a connection of its own. +func GuardMisdirectedRequests(listeners []*listenerv3.Listener, routes []*routev3.RouteConfiguration) int { + secure, plaintext := sets.New[string](), sets.New[string]() + for _, l := range listeners { + chains := append([]*listenerv3.FilterChain{}, l.GetFilterChains()...) + if dfc := l.GetDefaultFilterChain(); dfc != nil { + chains = append(chains, dfc) + } + for _, fc := range chains { + for _, name := range rdsRouteConfigNames(fc) { + if fc.GetTransportSocket() != nil { + secure.Insert(name) + } else { + plaintext.Insert(name) + } + } + } + } + + guarded := 0 + for _, rc := range routes { + name := rc.GetName() + // A second "*" domain would make Envoy reject the whole update. + if !secure.Has(name) || plaintext.Has(name) || hasCatchAllDomain(rc) { + continue + } + rc.VirtualHosts = append(rc.VirtualHosts, misdirectedVirtualHost()) + guarded++ + } + return guarded +} + +func rdsRouteConfigNames(fc *listenerv3.FilterChain) []string { + var names []string + for _, f := range fc.GetFilters() { + if f.GetName() != hcmFilterName || f.GetTypedConfig() == nil { + continue + } + hcm := &hcmv3.HttpConnectionManager{} + if err := f.GetTypedConfig().UnmarshalTo(hcm); err != nil { + continue + } + if name := hcm.GetRds().GetRouteConfigName(); name != "" { + names = append(names, name) + } + } + return names +} + +func hasCatchAllDomain(rc *routev3.RouteConfiguration) bool { + for _, vh := range rc.GetVirtualHosts() { + if slices.Contains(vh.GetDomains(), "*") { + return true + } + } + return false +} + +func misdirectedVirtualHost() *routev3.VirtualHost { + return &routev3.VirtualHost{ + Name: misdirectedVirtualHostName, + Domains: []string{"*"}, + Routes: []*routev3.Route{{ + Name: misdirectedVirtualHostName, + Match: &routev3.RouteMatch{ + PathSpecifier: &routev3.RouteMatch_Prefix{Prefix: "/"}, + }, + Action: &routev3.Route_DirectResponse{ + DirectResponse: &routev3.DirectResponseAction{Status: misdirectedStatus}, + }, + }}, + } +} diff --git a/internal/extensionserver/mutate/misdirected_test.go b/internal/extensionserver/mutate/misdirected_test.go new file mode 100644 index 00000000..69bf9a16 --- /dev/null +++ b/internal/extensionserver/mutate/misdirected_test.go @@ -0,0 +1,114 @@ +package mutate + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" + listenerv3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" + routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" + hcmv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3" + "google.golang.org/protobuf/types/known/anypb" + "google.golang.org/protobuf/types/known/wrapperspb" +) + +// withRDS gives the chain an HCM that takes its routes from the named route +// configuration. +func withRDS(t *testing.T, fc *listenerv3.FilterChain, routeConfig string) *listenerv3.FilterChain { + t.Helper() + hcmAny, err := anypb.New(&hcmv3.HttpConnectionManager{ + StatPrefix: routeConfig, + RouteSpecifier: &hcmv3.HttpConnectionManager_Rds{Rds: &hcmv3.Rds{RouteConfigName: routeConfig}}, + }) + require.NoError(t, err) + fc.Filters = append(fc.Filters, &listenerv3.Filter{ + Name: hcmFilterName, + ConfigType: &listenerv3.Filter_TypedConfig{TypedConfig: hcmAny}, + }) + return fc +} + +func siteRouteConfig(name, host string) *routev3.RouteConfiguration { + return &routev3.RouteConfiguration{ + Name: name, + VirtualHosts: []*routev3.VirtualHost{{Name: host, Domains: []string{host}}}, + } +} + +func findVirtualHost(rc *routev3.RouteConfiguration, name string) *routev3.VirtualHost { + for _, vh := range rc.GetVirtualHosts() { + if vh.GetName() == name { + return vh + } + } + return nil +} + +func TestGuardMisdirectedRequests(t *testing.T) { + quicChain := withRDS(t, &listenerv3.FilterChain{ + Name: "quic-b", + TransportSocket: &corev3.TransportSocket{Name: "envoy.transport_sockets.quic"}, + }, "https-b") + + listeners := []*listenerv3.Listener{ + { + Name: "tcp-443", + FilterChains: []*listenerv3.FilterChain{ + withRDS(t, tlsChain(t, "a.example.net", "secret-a"), "https-a"), + withRDS(t, tlsChain(t, "shared.example.net", "secret-shared"), "shared"), + withRDS(t, tlsChain(t, "any.example.net", "secret-any"), "has-catch-all"), + }, + }, + {Name: "udp-443", FilterChains: []*listenerv3.FilterChain{quicChain}}, + { + Name: "tcp-80", + FilterChains: []*listenerv3.FilterChain{withRDS(t, plainChain("shared-plain"), "shared")}, + DefaultFilterChain: withRDS(t, plainChain("http-80"), "http-80"), + }, + } + + catchAll := siteRouteConfig("has-catch-all", "any.example.net") + catchAll.VirtualHosts[0].Domains = []string{"*"} + routes := []*routev3.RouteConfiguration{ + siteRouteConfig("https-a", "a.example.net"), + siteRouteConfig("https-b", "b.example.net"), + siteRouteConfig("shared", "shared.example.net"), + catchAll, + siteRouteConfig("http-80", "plain.example.net"), + } + + require.Equal(t, 2, GuardMisdirectedRequests(listeners, routes)) + + for _, rc := range routes[:2] { + vh := findVirtualHost(rc, misdirectedVirtualHostName) + require.NotNil(t, vh, "%s is only reached over TLS or QUIC and must answer 421", rc.GetName()) + assert.Equal(t, []string{"*"}, vh.GetDomains()) + assert.EqualValues(t, misdirectedStatus, vh.GetRoutes()[0].GetDirectResponse().GetStatus()) + assert.Equal(t, rc.GetVirtualHosts()[0].GetName(), rc.GetVirtualHosts()[0].GetDomains()[0], + "the site's own virtual host stays first and unchanged") + } + for _, rc := range routes[2:] { + assert.Nil(t, findVirtualHost(rc, misdirectedVirtualHostName), + "%s is reached in plaintext or already has a catch-all and must be left alone", rc.GetName()) + } + + assert.Zero(t, GuardMisdirectedRequests(listeners, routes), + "a second pass must not add a duplicate \"*\" domain") +} + +func TestGuardMisdirectedRequestsSkipsUnreadableHCM(t *testing.T) { + notAnHCM, err := anypb.New(wrapperspb.String("not an HCM")) + require.NoError(t, err) + fc := tlsChain(t, "a.example.net", "secret-a") + fc.Filters = []*listenerv3.Filter{{ + Name: hcmFilterName, + ConfigType: &listenerv3.Filter_TypedConfig{TypedConfig: notAnHCM}, + }} + listeners := []*listenerv3.Listener{{Name: "tcp-443", FilterChains: []*listenerv3.FilterChain{fc}}} + routes := []*routev3.RouteConfiguration{siteRouteConfig("https-a", "a.example.net")} + + assert.Zero(t, GuardMisdirectedRequests(listeners, routes)) + assert.Len(t, routes[0].GetVirtualHosts(), 1) +} diff --git a/internal/extensionserver/server/server.go b/internal/extensionserver/server/server.go index cfa7fac5..bcd35b29 100644 --- a/internal/extensionserver/server/server.go +++ b/internal/extensionserver/server/server.go @@ -104,6 +104,7 @@ const ProgrammedSetEndpointPath = programmedSetEndpointPath // 3. ReplaceConnectorClusters — replace online-connector clusters with // STATIC internal-upstream clusters. // 4. ApplyConnectorRoutes — prepend CONNECT routes, append target domains. +// 5. GuardMisdirectedRequests — answer 421 for requests on another site's connection. func (s *Server) PostTranslateModify( ctx context.Context, req *pb.PostTranslateModifyRequest, @@ -299,6 +300,11 @@ func (s *Server) PostTranslateModify( ) connRoutesSpan.End() + _, misdirectedSpan := tr.Start(mctx, "misdirected.routes") + misdirectedCount := mutate.GuardMisdirectedRequests(listeners, routes) + misdirectedSpan.SetAttributes(attribute.Int("route_configs.misdirected_guarded", misdirectedCount)) + misdirectedSpan.End() + // --- VPC pod family (#856) --- // Binds a vpcPod backend's cluster to its tenant's VRF device // (SO_BINDTODEVICE) so the shared multi-tenant Envoy fleet resolves the @@ -374,6 +380,7 @@ func (s *Server) PostTranslateModify( // rate() in Prometheus / PromQL to derive per-build averages. extmetrics.WAFHCMMutationsTotal.Add(float64(hcmCount)) extmetrics.LocalReplyMutationsTotal.Add(float64(localReplyCount)) + extmetrics.MisdirectedGuardsTotal.Add(float64(misdirectedCount)) extmetrics.WAFRouteMutationsTotal.Add(float64(tppCount)) extmetrics.ConnectorClustersTotal.Add(float64(len(replaced))) extmetrics.ConnectorRoutesTotal.Add(float64(vhCount))