Skip to content

Commit

Permalink
fix: disable ext_authz filter on https redirect route
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Lance Austin committed Oct 17, 2022
1 parent b445f87 commit d5e9b91
Show file tree
Hide file tree
Showing 5 changed files with 206 additions and 0 deletions.
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
19 changes: 19 additions & 0 deletions docs/releaseNotes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
10 changes: 10 additions & 0 deletions python/ambassador/envoy/v3/v3route.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
97 changes: 97 additions & 0 deletions python/tests/kat/t_extauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -1348,3 +1348,100 @@ def check(self):
assert self.results[3].backend.request.headers["kat-resp-extauth-protocol-version"] == [
self.expected_protocol_version
]


class AuthenticationHTTPSRedirectTest(AmbassadorTest):
"""
AuthenticationHTTPSREdirect: We expect that the http-to-https will occur
without calling the AuthService (ext_authz).
"""

target: ServiceType
auth: ServiceType

def init(self):
if EDGE_STACK:
self.xfail = "custom AuthServices not supported in Edge Stack"
self.target = HTTP()
self.auth = AHTTP(name="auth")
self.add_default_http_listener = False
self.add_default_https_listener = False

def manifests(self) -> str:
return (
self.format(
"""
---
apiVersion: getambassador.io/v3alpha1
kind: Listener
metadata:
name: {self.path.k8s}
spec:
ambassador_id: [{self.ambassador_id}]
port: 8080
protocol: HTTP
securityModel: XFP
l7Depth: 1
hostBinding:
namespace:
from: ALL
---
apiVersion: getambassador.io/v3alpha1
kind: AuthService
metadata:
name: {self.auth.path.k8s}
spec:
ambassador_id: [ {self.ambassador_id} ]
auth_service: "{self.auth.path.fqdn}"
proto: http
protocol_version: "v3"
---
apiVersion: getambassador.io/v3alpha1
kind: Host
metadata:
name: {self.path.k8s}-host
labels:
kat-ambassador-id: {self.ambassador_id}
spec:
ambassador_id: [ {self.ambassador_id} ]
hostname: "*"
---
apiVersion: getambassador.io/v3alpha1
kind: Mapping
metadata:
name: {self.target.path.k8s}
spec:
ambassador_id: [{self.ambassador_id}]
hostname: "*"
prefix: /target/
service: {self.target.path.fqdn}
"""
)
+ super().manifests()
)

def requirements(self):
# The client doesn't follow redirects so we must force checks to
# match the XFP https route. The Listener is configured with
# l7depth: 1 so that Envoy trusts the header XFP header forwarded
# by the client.
yield (
"url",
Query(self.url("ambassador/v0/check_ready"), headers={"X-Forwarded-Proto": "https"}),
)
yield (
"url",
Query(self.url("ambassador/v0/check_alive"), headers={"X-Forwarded-Proto": "https"}),
)

def queries(self):
# send http request
yield Query(
self.url("target/", scheme="http"), headers={"X-Forwarded-Proto": "http"}, expected=301
)

def check(self):
# we should NOT make a call to the backend service, and we will
# rather envoy should have redirected to https
assert self.results[0].backend is None
assert self.results[0].headers["Location"] == [f"https://{self.path.fqdn}/target/"]
69 changes: 69 additions & 0 deletions python/tests/unit/test_irauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit d5e9b91

Please sign in to comment.