internal: filter misdirected TLS requests

TLS routes are specialized to a unique virtual hostname. However, if
wildcard certificates are being used, browsers will aggressively coalesce
and reuse server connections even when the full origin hostname doesn't
match. This results on 404 responses because each TLS virtual host only
has routes for one host.

We can avoid this behaviour bleeding out to users by generating a 421
Misdirected Request response if the request authority doesn't match
the FQDN of the virtual host. In this case, the browser is supposed
to understand that the request wasn't processed and re-send it on a
new connection.

This fixes #1493.

Signed-off-by: James Peach <jpeach@vmware.com>

_integration/testsuite/httpproxy/004-https-sni-enforcement.yaml

@@ -117,7 +117,7 @@ fatal_proxy_is_not_present[msg] {
   msg := sprintf("HTTPProxy for %q is not present", [ fqdn ])
 }
 
---- 
+---
 
 Name := "echo-one"
 
@@ -243,7 +243,7 @@ fatal_proxy_is_not_present[msg] {
   msg := sprintf("HTTPProxy for %q is not present", [ fqdn ])
 }
 
---- 
+---
 
 Name := "echo-two"
 
@@ -309,9 +309,10 @@ error_path_routing_mismatch[msg] {
 ---
 
 import data.contour.http.client
+import data.contour.http.response
 
 # Ensure that sending a request to "echo-one" with the SNI from "echo-two"
-# generates a 404.
+# generates a 4xx response status.
 
 Response := client.Get({
   "url": sprintf("https://%s/https-sni-enforcement/%d", [
@@ -324,23 +325,18 @@ Response := client.Get({
   "tls_server_name": "echo-two.projectcontour.io",
 })
 
-Wanted := 404
-
-error_non_404_response [msg] {
-  Response.status_code != Wanted
-
-  msg :=  sprintf("got status %d, wanted %d", [
-    Response.status_code, Wanted
-  ])
+error_non_400_response [msg] {
+  not response.is_4xx(Response)
+  msg :=  sprintf("got status %d, wanted 4xx", [ Response.status_code ])
 }
 
-
 ---
 
 import data.contour.http.client
+import data.contour.http.response
 
 # Ensure that sending a request to "echo-two" with the SNI from "echo-one"
-# generates a 404.
+# generates a 4xx response status.
 
 Response := client.Get({
   "url": sprintf("https://%s/https-sni-enforcement/%d", [
@@ -353,18 +349,7 @@ Response := client.Get({
   "tls_server_name": "echo-one.projectcontour.io",
 })
 
-Wanted := 404
-
-error_no_response {
-  not Response
-}
-
 error_non_404_response [msg] {
-  Response.status_code != Wanted
-
-  msg :=  sprintf("got status %d, wanted %d", [
-    Response.status_code, Wanted
-  ])
+  not response.is_4xx(Response)
+  msg :=  sprintf("got status %d, wanted 4xx", [ Response.status_code ])
 }
-
-

_integration/testsuite/httpproxy/009-https-misdirected-request.yaml

@@ -0,0 +1,192 @@
+# Copyright 2020 VMware, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License.  You may obtain
+# a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import data.contour.resources
+
+# Ensure that cert-manager is installed.
+# Version check the certificates resource.
+
+Group := "cert-manager.io"
+Version := "v1alpha2"
+
+have_certmanager_version {
+  v := resources.versions["certificates"]
+  v[_].Group == Group
+  v[_].Version == Version
+}
+
+skip[msg] {
+  not resources.is_supported("certificates")
+  msg := "cert-manager is not installed"
+}
+
+skip[msg] {
+  not have_certmanager_version
+
+  avail := resources.versions["certificates"]
+
+  msg := concat("\n", [
+    sprintf("cert-manager version %s/%s is not installed", [Group, Version]),
+    "available versions:",
+    yaml.marshal(avail)
+  ])
+}
+
+---
+
+# Create a self-signed issuer to give us secrets.
+
+apiVersion: cert-manager.io/v1alpha2
+kind: Issuer
+metadata:
+  name: selfsigned
+spec:
+  selfSigned: {}
+
+---
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: ingress-conformance-echo
+$apply:
+  fixture:
+    as: echo
+
+---
+
+apiVersion: v1
+kind: Service
+metadata:
+  name: ingress-conformance-echo
+$apply:
+  fixture:
+    as: echo
+
+---
+
+apiVersion: cert-manager.io/v1alpha2
+kind: Certificate
+metadata:
+  name: echo-cert
+spec:
+  dnsNames:
+  - echo.projectcontour.io
+  secretName: echo
+  issuerRef:
+    name: selfsigned
+
+---
+
+apiVersion: projectcontour.io/v1
+kind: HTTPProxy
+metadata:
+  name: echo
+spec:
+  virtualhost:
+    fqdn: echo.projectcontour.io
+    tls:
+      secretName: echo
+  routes:
+  - services:
+    - name: echo
+      port: 80
+
+---
+
+import data.contour.resources
+
+Name := "echo"
+
+fatal_proxy_is_not_present[msg] {
+  not resources.is_present("httpproxies", Name)
+  msg := sprintf("HTTPProxy for %q is not present", [ Name ])
+}
+
+---
+
+import data.contour.resources
+
+Name := "echo"
+
+fatal_proxy_is_not_valid[msg] {
+  status := resources.status("httpproxies", Name)
+
+  object.get(status, "currentStatus", "") != "valid"
+
+  msg := sprintf("HTTPProxy %q is not valid\n%s", [
+    Name, yaml.marshal(status)
+  ])
+}
+
+---
+
+import data.contour.http.client
+import data.contour.http.request
+import data.contour.http.response
+
+Response := client.Get({
+  "url": sprintf("https://%s/misdirected/%d", [
+     client.target_addr, time.now_ns()
+  ]),
+  "headers": {
+    "Host": "echo.projectcontour.io",
+    "User-Agent": client.ua("misdirected-request"),
+  },
+  "tls_insecure_skip_verify": true,
+})
+
+error_non_200_response [msg] {
+  not response.status_is(Response, 200)
+  msg := sprintf("got status %d, wanted %d", [Response.status_code, 200])
+}
+
+error_wrong_routing [msg] {
+  not response.has_testid(Response)
+  msg := "response has missing body or test ID"
+}
+
+error_wrong_routing[msg] {
+  wanted := "echo"
+  testid := response.testid(Response)
+  testid != wanted
+  msg := sprintf("got test ID %q, wanted %q", [testid, wanted])
+}
+
+---
+
+import data.contour.http.client
+import data.contour.http.request
+import data.contour.http.response
+
+# Send a request with a Host header that doesn't match the SNI name that
+# we have for the proxy document. We expect the mismatch will generate a
+# 421 response, not 404.
+
+Response := client.Get({
+  "url": sprintf("https://%s/misdirected/%d", [
+     client.target_addr, time.now_ns()
+  ]),
+  "headers": {
+    "Host": "echo-two.projectcontour.io",
+    "User-Agent": client.ua("misdirected-request"),
+  },
+  "tls_server_name": "echo.projectcontour.io",
+  "tls_insecure_skip_verify": true,
+})
+
+error_non_421_response [msg] {
+  not response.status_is(Response, 421)
+  msg := sprintf("got status %d, wanted %d", [Response.status_code, 421])
+}

_integration/testsuite/policies/contour-client.rego

@@ -38,4 +38,12 @@ Get(params) = response {
   }
 
   response := http.send(object.union(to_send, params))
+} else = response {
+  # If the Get wasn't evaluated for any reason, return a dummy object to ensure
+  # subsequent field references are valid.
+  response := {
+    "status_code": 0,
+    "body": {},
+    "headers": {},
+  }
 }

_integration/testsuite/policies/contour-resources.rego

@@ -81,3 +81,14 @@ get(resource, name) = obj {
 } else = obj {
   obj := {}
 }
+
+# status returns the status field of the named resource. If the resource
+# is not present, and empty object is returned. Implemented in terms of
+# 'get', so namespace syntax works for the object name.
+#
+# Examples:
+#   resources.status("httpproxies", "foo")
+status(resource, name) = s {
+  r := get(resource, name)
+  s := object.get(r, "status", {})
+}

_integration/testsuite/policies/contour-response.rego

@@ -44,3 +44,20 @@ testid(resp) = value {
   b := body(resp)
   value := object.get(b, "TestId", "")
 }
+
+# Return true if the response status matches.
+status_is(resp, expected_code) = true {
+  status_code := object.get(resp, "status_code", 0)
+  status_code == expected_code
+} else = false {
+  true
+}
+
+# Return true if the response status is in the 4xx range.
+is_4xx(resp) = true {
+  status_code := object.get(resp, "status_code", 0)
+  status_code >= 400
+  status_code < 500
+} else = false {
+  true
+}

internal/contour/listener.go

@@ -1,4 +1,4 @@
-// Copyright © 2019 VMware
+// Copyright © 2020 VMware
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
@@ -298,6 +298,7 @@ func visitListeners(root dag.Vertex, lvc *ListenerVisitorConfig) map[string]*v2.
 	if lv.http {
 		// Add a listener if there are vhosts bound to http.
 		cm := envoy.HTTPConnectionManagerBuilder().
+			DefaultFilters().
 			RouteConfigName(ENVOY_HTTP_LISTENER).
 			MetricsPrefix(ENVOY_HTTP_LISTENER).
 			AccessLoggers(lvc.newInsecureAccessLog()).
@@ -366,6 +367,8 @@ func (v *listenerVisitor) visit(vertex dag.Vertex) {
 			// coded into monitoring dashboards.
 			filters = envoy.Filters(
 				envoy.HTTPConnectionManagerBuilder().
+					AddFilter(envoy.FilterMisdirectedRequests(vh.VirtualHost.Name)).
+					DefaultFilters().
 					RouteConfigName(path.Join("https", vh.VirtualHost.Name)).
 					MetricsPrefix(ENVOY_HTTPS_LISTENER).
 					AccessLoggers(v.ListenerVisitorConfig.newSecureAccessLog()).
@@ -403,10 +406,12 @@ func (v *listenerVisitor) visit(vertex dag.Vertex) {
 		v.listeners[ENVOY_HTTPS_LISTENER].FilterChains = append(v.listeners[ENVOY_HTTPS_LISTENER].FilterChains,
 			envoy.FilterChainTLS(vh.VirtualHost.Name, downstreamTLS, filters))
 
-		// If this VirtualHost has enabled the fallback certificate then set a default FilterChain which will allow
-		// routes with this vhost to accept non SNI TLS requests
+		// If this VirtualHost has enabled the fallback certificate then set a default
+		// FilterChain which will allow routes with this vhost to accept non-SNI TLS requests.
+		// Note that we don't add the misdirected requests filter on this chain because at this
+		// point we don't actually know the full set of server names that will be bound to the
+		// filter chain through the ENVOY_FALLBACK_ROUTECONFIG route configuration.
 		if vh.FallbackCertificate != nil && !envoy.ContainsFallbackFilterChain(v.listeners[ENVOY_HTTPS_LISTENER].FilterChains) {
-
 			// Construct the downstreamTLSContext passing the configured fallbackCertificate. The TLS minProtocolVersion will use
 			// the value defined in the Contour Configuration file if defined.
 			downstreamTLS = envoy.DownstreamTLSContext(

internal/contour/listener_test.go

@@ -1,4 +1,4 @@
-// Copyright © 2019 VMware
+// Copyright © 2020 VMware
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
@@ -129,6 +129,8 @@ func TestListenerCacheQuery(t *testing.T) {
 func TestListenerVisit(t *testing.T) {
 	httpsFilterFor := func(vhost string) *envoy_api_v2_listener.Filter {
 		return envoy.HTTPConnectionManagerBuilder().
+			AddFilter(envoy.FilterMisdirectedRequests(vhost)).
+			DefaultFilters().
 			MetricsPrefix(ENVOY_HTTPS_LISTENER).
 			RouteConfigName(path.Join("https", vhost)).
 			AccessLoggers(envoy.FileAccessLogEnvoy(DEFAULT_HTTP_ACCESS_LOG)).
@@ -790,6 +792,8 @@ func TestListenerVisit(t *testing.T) {
 					},
 					TransportSocket: transportSocket("secret", envoy_api_v2_auth.TlsParameters_TLSv1_1, "h2", "http/1.1"),
 					Filters: envoy.Filters(envoy.HTTPConnectionManagerBuilder().
+						AddFilter(envoy.FilterMisdirectedRequests("whatever.example.com")).
+						DefaultFilters().
 						MetricsPrefix(ENVOY_HTTPS_LISTENER).
 						RouteConfigName(path.Join("https", "whatever.example.com")).
 						AccessLoggers(envoy.FileAccessLogEnvoy("/tmp/https_access.log")).