diff --git a/changelogs/unreleased/5437-sunjayBhatia-minor.md b/changelogs/unreleased/5437-sunjayBhatia-minor.md new file mode 100644 index 00000000000..721a6ef3dc7 --- /dev/null +++ b/changelogs/unreleased/5437-sunjayBhatia-minor.md @@ -0,0 +1,5 @@ +## Host header including port is passed through unmodified to backend + +Previously Contour would strip any port from the Host header in a downstream request for convenience in routing. +This resulted in backends not receiving the Host header with a port. +We no longer do this, for conformance with Gateway API (this change also applies to HTTPProxy and Ingress configuration). diff --git a/internal/dag/builder_test.go b/internal/dag/builder_test.go index 8d7ce987e53..311039e78fa 100644 --- a/internal/dag/builder_test.go +++ b/internal/dag/builder_test.go @@ -10359,7 +10359,7 @@ func TestDAGInsert(t *testing.T) { &Route{ PathMatchCondition: prefixString("/"), HeaderMatchConditions: []HeaderMatchCondition{ - {Name: ":authority", Value: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.projectcontour\\.io", MatchType: "regex", Invert: false}, + {Name: ":authority", Value: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.projectcontour\\.io(:[0-9]+)?", MatchType: "regex", Invert: false}, }, Clusters: clusters(service(s1)), }), @@ -10780,7 +10780,7 @@ func TestDAGInsert(t *testing.T) { { Name: ":authority", MatchType: HeaderMatchTypeRegex, - Value: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.example\\.com", + Value: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.example\\.com(:[0-9]+)?", }, }, Clusters: clusters(service(s1)), diff --git a/internal/dag/dag.go b/internal/dag/dag.go index bd640c636d7..ac53727f587 100644 --- a/internal/dag/dag.go +++ b/internal/dag/dag.go @@ -1170,6 +1170,14 @@ type ExtensionCluster struct { ClientCertificate *Secret } +const singleDNSLabelWildcardRegex = "^[a-z0-9]([-a-z0-9]*[a-z0-9])?" + +var _ = regexp.MustCompile(singleDNSLabelWildcardRegex) + +const ignorePortRegex = "(:[0-9]+)?" + +var _ = regexp.MustCompile(ignorePortRegex) + func wildcardDomainHeaderMatch(fqdn string) HeaderMatchCondition { return HeaderMatchCondition{ // Internally Envoy uses the HTTP/2 ":authority" header in @@ -1177,7 +1185,7 @@ func wildcardDomainHeaderMatch(fqdn string) HeaderMatchCondition { // See: https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#config-route-v3-headermatcher Name: ":authority", MatchType: HeaderMatchTypeRegex, - Value: singleDNSLabelWildcardRegex + regexp.QuoteMeta(fqdn[1:]), + Value: singleDNSLabelWildcardRegex + regexp.QuoteMeta(fqdn[1:]) + ignorePortRegex, } } diff --git a/internal/dag/ingress_processor.go b/internal/dag/ingress_processor.go index 755eca814a0..2789f92ddbc 100644 --- a/internal/dag/ingress_processor.go +++ b/internal/dag/ingress_processor.go @@ -14,7 +14,6 @@ package dag import ( - "regexp" "strconv" "strings" "time" @@ -203,10 +202,6 @@ func (p *IngressProcessor) computeIngressRule(ing *networking_v1.Ingress, rule n } } -const singleDNSLabelWildcardRegex = "^[a-z0-9]([-a-z0-9]*[a-z0-9])?" - -var _ = regexp.MustCompile(singleDNSLabelWildcardRegex) - // route builds a dag.Route for the supplied Ingress. func (p *IngressProcessor) route(ingress *networking_v1.Ingress, host string, path string, pathType networking_v1.PathType, service *Service, clientCertSecret *Secret, serviceName string, servicePort int32, log logrus.FieldLogger) (*Route, error) { log = log.WithFields(logrus.Fields{ diff --git a/internal/envoy/v3/listener.go b/internal/envoy/v3/listener.go index 44fe5391213..22f12274619 100644 --- a/internal/envoy/v3/listener.go +++ b/internal/envoy/v3/listener.go @@ -474,14 +474,6 @@ func (b *httpConnectionManagerBuilder) Get() *envoy_listener_v3.Filter { NormalizePath: wrapperspb.Bool(true), - // We can ignore any port number supplied in the Host/:authority header - // before processing by filters or routing. - // Note that the port a listener is bound to will already be selected - // and that the port is stripped from the header sent upstream as well. - StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{ - StripAnyHostPort: true, - }, - // issue #1487 pass through X-Request-Id if provided. PreserveExternalRequestId: true, MergeSlashes: b.mergeSlashes, @@ -713,6 +705,11 @@ function envoy_on_request(request_handle) local host = string.lower(headers:get(":authority")) local target = %s + s, e = string.find(host, ":", 1, true) + if s ~= nil then + host = string.sub(host, 1, s - 1) + end + if host ~= target then request_handle:respond( {[":status"] = "421"}, diff --git a/internal/envoy/v3/listener_test.go b/internal/envoy/v3/listener_test.go index a38d70b9d5c..f24c68435e2 100644 --- a/internal/envoy/v3/listener_test.go +++ b/internal/envoy/v3/listener_test.go @@ -655,9 +655,6 @@ func TestHTTPConnectionManager(t *testing.T) { AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), UseRemoteAddress: wrapperspb.Bool(true), NormalizePath: wrapperspb.Bool(true), - StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{ - StripAnyHostPort: true, - }, PreserveExternalRequestId: true, MergeSlashes: false, }), @@ -705,9 +702,6 @@ func TestHTTPConnectionManager(t *testing.T) { AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), UseRemoteAddress: wrapperspb.Bool(true), NormalizePath: wrapperspb.Bool(true), - StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{ - StripAnyHostPort: true, - }, RequestTimeout: durationpb.New(10 * time.Second), PreserveExternalRequestId: true, MergeSlashes: false, @@ -755,12 +749,9 @@ func TestHTTPConnectionManager(t *testing.T) { CommonHttpProtocolOptions: &envoy_core_v3.HttpProtocolOptions{ IdleTimeout: durationpb.New(90 * time.Second), }, - AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), - UseRemoteAddress: wrapperspb.Bool(true), - NormalizePath: wrapperspb.Bool(true), - StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{ - StripAnyHostPort: true, - }, + AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), + UseRemoteAddress: wrapperspb.Bool(true), + NormalizePath: wrapperspb.Bool(true), PreserveExternalRequestId: true, MergeSlashes: false, }), @@ -808,9 +799,6 @@ func TestHTTPConnectionManager(t *testing.T) { AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), UseRemoteAddress: wrapperspb.Bool(true), NormalizePath: wrapperspb.Bool(true), - StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{ - StripAnyHostPort: true, - }, PreserveExternalRequestId: true, MergeSlashes: false, StreamIdleTimeout: durationpb.New(90 * time.Second), @@ -858,12 +846,9 @@ func TestHTTPConnectionManager(t *testing.T) { CommonHttpProtocolOptions: &envoy_core_v3.HttpProtocolOptions{ MaxConnectionDuration: durationpb.New(90 * time.Second), }, - AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), - UseRemoteAddress: wrapperspb.Bool(true), - NormalizePath: wrapperspb.Bool(true), - StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{ - StripAnyHostPort: true, - }, + AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), + UseRemoteAddress: wrapperspb.Bool(true), + NormalizePath: wrapperspb.Bool(true), PreserveExternalRequestId: true, MergeSlashes: false, }), @@ -911,9 +896,6 @@ func TestHTTPConnectionManager(t *testing.T) { AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), UseRemoteAddress: wrapperspb.Bool(true), NormalizePath: wrapperspb.Bool(true), - StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{ - StripAnyHostPort: true, - }, PreserveExternalRequestId: true, MergeSlashes: false, }), @@ -961,9 +943,6 @@ func TestHTTPConnectionManager(t *testing.T) { AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), UseRemoteAddress: wrapperspb.Bool(true), NormalizePath: wrapperspb.Bool(true), - StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{ - StripAnyHostPort: true, - }, PreserveExternalRequestId: true, MergeSlashes: false, DelayedCloseTimeout: durationpb.New(90 * time.Second), @@ -1012,9 +991,6 @@ func TestHTTPConnectionManager(t *testing.T) { AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), UseRemoteAddress: wrapperspb.Bool(true), NormalizePath: wrapperspb.Bool(true), - StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{ - StripAnyHostPort: true, - }, PreserveExternalRequestId: true, MergeSlashes: false, DrainTimeout: durationpb.New(90 * time.Second), @@ -1065,9 +1041,6 @@ func TestHTTPConnectionManager(t *testing.T) { AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), UseRemoteAddress: wrapperspb.Bool(true), NormalizePath: wrapperspb.Bool(true), - StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{ - StripAnyHostPort: true, - }, PreserveExternalRequestId: true, MergeSlashes: false, DrainTimeout: durationpb.New(90 * time.Second), @@ -1116,9 +1089,6 @@ func TestHTTPConnectionManager(t *testing.T) { AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), UseRemoteAddress: wrapperspb.Bool(true), NormalizePath: wrapperspb.Bool(true), - StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{ - StripAnyHostPort: true, - }, PreserveExternalRequestId: true, MergeSlashes: true, }), @@ -1162,13 +1132,10 @@ func TestHTTPConnectionManager(t *testing.T) { // a Host: header. See #537. AcceptHttp_10: true, }, - CommonHttpProtocolOptions: &envoy_core_v3.HttpProtocolOptions{}, - AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), - UseRemoteAddress: wrapperspb.Bool(true), - NormalizePath: wrapperspb.Bool(true), - StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{ - StripAnyHostPort: true, - }, + CommonHttpProtocolOptions: &envoy_core_v3.HttpProtocolOptions{}, + AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), + UseRemoteAddress: wrapperspb.Bool(true), + NormalizePath: wrapperspb.Bool(true), PreserveExternalRequestId: true, ServerHeaderTransformation: http.HttpConnectionManager_PASS_THROUGH, }), @@ -1230,9 +1197,6 @@ func TestHTTPConnectionManager(t *testing.T) { AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), UseRemoteAddress: wrapperspb.Bool(true), NormalizePath: wrapperspb.Bool(true), - StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{ - StripAnyHostPort: true, - }, PreserveExternalRequestId: true, MergeSlashes: false, }), @@ -1290,9 +1254,6 @@ func TestHTTPConnectionManager(t *testing.T) { AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), UseRemoteAddress: wrapperspb.Bool(true), NormalizePath: wrapperspb.Bool(true), - StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{ - StripAnyHostPort: true, - }, PreserveExternalRequestId: true, MergeSlashes: false, }), @@ -1340,9 +1301,6 @@ func TestHTTPConnectionManager(t *testing.T) { AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), UseRemoteAddress: wrapperspb.Bool(true), NormalizePath: wrapperspb.Bool(true), - StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{ - StripAnyHostPort: true, - }, PreserveExternalRequestId: true, MergeSlashes: false, XffNumTrustedHops: uint32(1), diff --git a/internal/envoy/v3/route.go b/internal/envoy/v3/route.go index 9fe6e2caea1..bf3ce83dd71 100644 --- a/internal/envoy/v3/route.go +++ b/internal/envoy/v3/route.go @@ -649,6 +649,9 @@ func RouteConfiguration(name string, virtualhosts ...*envoy_route_v3.VirtualHost RequestHeadersToAdd: headers( appendHeader("x-request-start", "t=%START_TIME(%s.%3f)%"), ), + // We can ignore any port number supplied in the Host/:authority header + // when selecting a virtualhost. + IgnorePortInHostMatching: true, } } diff --git a/internal/envoy/v3/route_test.go b/internal/envoy/v3/route_test.go index d9ce5b193d5..b752ee48cb0 100644 --- a/internal/envoy/v3/route_test.go +++ b/internal/envoy/v3/route_test.go @@ -1070,6 +1070,7 @@ func TestRouteConfiguration(t *testing.T) { }, AppendAction: envoy_core_v3.HeaderValueOption_APPEND_IF_EXISTS_OR_ADD, }}, + IgnorePortInHostMatching: true, }, }, "one virtualhost": { @@ -1089,6 +1090,7 @@ func TestRouteConfiguration(t *testing.T) { }, AppendAction: envoy_core_v3.HeaderValueOption_APPEND_IF_EXISTS_OR_ADD, }}, + IgnorePortInHostMatching: true, }, }, } diff --git a/internal/featuretests/v3/wildcardhost_test.go b/internal/featuretests/v3/wildcardhost_test.go index 92fc5ee3d27..53aeae1f079 100644 --- a/internal/featuretests/v3/wildcardhost_test.go +++ b/internal/featuretests/v3/wildcardhost_test.go @@ -93,7 +93,7 @@ func TestIngressWildcardHostHTTP(t *testing.T) { StringMatch: &matcher.StringMatcher{ MatchPattern: &matcher.StringMatcher_SafeRegex{ SafeRegex: &matcher.RegexMatcher{ - Regex: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.foo\\.com", + Regex: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.foo\\.com(:[0-9]+)?", }, }, }, @@ -154,7 +154,7 @@ func TestHTTPProxyWildcardFQDN(t *testing.T) { StringMatch: &matcher.StringMatcher{ MatchPattern: &matcher.StringMatcher_SafeRegex{ SafeRegex: &matcher.RegexMatcher{ - Regex: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.projectcontour\\.io", + Regex: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.projectcontour\\.io(:[0-9]+)?", }, }, }, @@ -259,7 +259,7 @@ func TestIngressWildcardHostHTTPSWildcardSecret(t *testing.T) { StringMatch: &matcher.StringMatcher{ MatchPattern: &matcher.StringMatcher_SafeRegex{ SafeRegex: &matcher.RegexMatcher{ - Regex: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.foo-tls\\.com", + Regex: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.foo-tls\\.com(:[0-9]+)?", }, }, }, @@ -287,7 +287,7 @@ func TestIngressWildcardHostHTTPSWildcardSecret(t *testing.T) { StringMatch: &matcher.StringMatcher{ MatchPattern: &matcher.StringMatcher_SafeRegex{ SafeRegex: &matcher.RegexMatcher{ - Regex: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.foo-tls\\.com", + Regex: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.foo-tls\\.com(:[0-9]+)?", }, }, },