fix: disable ext_authz filter on https redirect route

Fix regression introduced in v3 series when Envoy was upgraded to a
version after 1.20 (behavior change introduced). This commit restores
the expected behavior that an https redirect will occur prior to
calling the ext_authz service.

fixes #4620

Signed-off-by: Lance Austin <laustin@datawire.io>

CHANGELOG.md

@@ -93,6 +93,17 @@ it will be removed; but as it won't be user-visible this isn't considered a brea
 - Security: Updated Golang to 1.19.2 to address the CVEs: CVE-2022-2879, CVE-2022-2880,
   CVE-2022-41715.
 
+- Bugfix: By default Emissary-ingress adds routes for http to https redirection. When an AuthService
+  is applied in v2.Y of Emissary-ingress, Envoy would skip the ext_authz call for non-tls http
+  request and would perform the https  redirect. In Envoy 1.20+ the behavior has changed where Envoy
+  will  always call the ext_authz filter and must be disabled on a per route  basis. 
+  This new
+  behavior change introduced a regression in v3.0 of  Emissary-ingress when it was upgraded to Envoy
+  1.22. The http to https  redirection no longer works when an AuthService was applied. This fix 
+  restores the previous behavior by disabling the ext_authz call on the  https redirect routes. ([#4620])
+
+[#4620]: https://github.com/emissary-ingress/emissary/issues/4620
+
 ## [3.2.0] September 26, 2022
 [3.2.0]: https://github.com/emissary-ingress/emissary/compare/v3.1.0...v3.2.0
 

docs/releaseNotes.yml

@@ -41,6 +41,25 @@ items:
         body: >-
           Updated Golang to 1.19.2 to address the CVEs: CVE-2022-2879, CVE-2022-2880, CVE-2022-41715.
 
+      - title: Fix regression in http to https redirects with AuthService
+        type: bugfix
+        body: >-
+          By default $productName$ adds routes for http to https redirection. When
+          an AuthService is applied in v2.Y of $productName$, Envoy would skip the
+          ext_authz call for non-tls http request and would perform the https 
+          redirect. In Envoy 1.20+ the behavior has changed where Envoy will 
+          always call the ext_authz filter and must be disabled on a per route 
+          basis. 
+          
+          This new behavior change introduced a regression in v3.0 of 
+          $productName$ when it was upgraded to Envoy 1.22. The http to https 
+          redirection no longer works when an AuthService was applied. This fix 
+          restores the previous behavior by disabling the ext_authz call on the 
+          https redirect routes.
+        github:
+        - title: "#4620"
+          link: https://github.com/emissary-ingress/emissary/issues/4620
+
   - version: 3.2.0
     prevVersion: 3.1.0
     date: '2022-09-26'

python/ambassador/envoy/v3/v3route.py

@@ -238,6 +238,16 @@ def action_route(self, variant) -> None:
     def action_redirect(self, variant) -> None:
         variant.pop("route", None)
         variant["redirect"] = {"https_redirect": True}
+        for filter in self.route._group.ir.filters:
+            if filter.kind == "IRAuth":
+                # Required to ensure that the redirect occurs prior to calling ext_authz
+                # when an AuthService is applied
+                variant["typed_per_filter_config"] = {
+                    "envoy.filters.http.ext_authz": {
+                        "@type": "type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthzPerRoute",
+                        "disabled": True,
+                    }
+                }
 
 
 # Model an Envoy route.

python/tests/kat/t_extauth.py

@@ -1026,6 +1026,9 @@ def queries(self):
         # [5]
         yield Query(self.url("target/"), headers={"X-Forwarded-Proto": "https"}, expected=200)
 
+        # [6] verify http-->https redirect
+        yield Query(self.url("target/"), insecure=True, expected=301)
+
     def check(self):
         # [0] Verifies all request headers sent to the authorization server.
         assert self.results[0].backend
@@ -1113,6 +1116,16 @@ def check(self):
         except ValueError as e:
             assert False, "could not parse Extauth header '%s': %s" % (eahdr, e)
 
+        # [6] verify http-->https redirect occurs without calling ext_authz
+        #
+        # For http --> https redirect routes we disable ext_authz calls so that
+        # envoy can redirect to the https route
+        r6 = self.results[6]
+        assert r6.status == 301
+        location_header = r6.headers["location"]
+        assert location_header
+        assert location_header.startswith("https://")
+
         # TODO(gsagula): Write tests for all UCs which request header headers
         # are overridden, e.g. Authorization.
 

python/tests/unit/test_irauth.py

@@ -170,3 +170,72 @@ def test_irauth_grpcservice_version_default():
         errors[0]["error"]
         == 'AuthService: protocol_version v2 is unsupported, protocol_version must be "v3"'
     )
+
+
+@pytest.mark.compilertest
+def test_basic_http_redirect_with_no_authservice():
+    """Test that http --> https redirect route exists when no AuthService is provided
+    and verify that the typed_per_filter_config is NOT included
+    """
+
+    yaml = """
+apiVersion: getambassador.io/v3alpha1
+kind: Mapping
+metadata:
+  name: ambassador
+  namespace: default
+spec:
+  hostname: "*"
+  prefix: /httpbin/
+  service: httpbin
+    """
+    econf = _get_envoy_config(yaml)
+
+    for rv in econf.route_variants:
+        if rv.route.get("match").get("prefix") == "/httpbin/":
+            xfp_http_redirect = rv.variants.get("xfp-http-redirect")
+            assert xfp_http_redirect
+            assert "redirect" in xfp_http_redirect
+            assert "typed_per_filter_config" not in xfp_http_redirect
+
+
+@pytest.mark.compilertest
+def test_basic_http_redirect_disables_ext_authz():
+    """Test that http --> https redirect route exists along with
+    typed_per_filter_config for disabling ext_authz when an AuthService exists
+    """
+
+    if EDGE_STACK:
+        pytest.xfail("XFailing for now, custom AuthServices not supported in Edge Stack")
+
+    yaml = """
+---
+apiVersion: getambassador.io/v3alpha1
+kind: AuthService
+metadata:
+  name:  mycoolauthservice
+  namespace: default
+spec:
+  auth_service: someservice
+  proto: grpc
+---
+apiVersion: getambassador.io/v3alpha1
+kind: Mapping
+metadata:
+  name: ambassador
+  namespace: default
+spec:
+  hostname: "*"
+  prefix: /httpbin/
+  service: httpbin
+    """
+    econf = _get_envoy_config(yaml)
+
+    for rv in econf.route_variants:
+        if rv.route.get("match").get("prefix") == "/httpbin/":
+            xfp_http_redirect = rv.variants.get("xfp-http-redirect")
+            assert xfp_http_redirect
+            assert "redirect" in xfp_http_redirect
+            per_filter_config = xfp_http_redirect.get("typed_per_filter_config")
+            assert per_filter_config.get("envoy.filters.http.ext_authz")
+            assert per_filter_config.get("envoy.filters.http.ext_authz").get("disabled") == True