Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

https redirect doesn't occur when AuthService applied #4620

Closed
LanceEa opened this issue Oct 13, 2022 · 0 comments
Closed

https redirect doesn't occur when AuthService applied #4620

LanceEa opened this issue Oct 13, 2022 · 0 comments
Assignees
Labels
t:bug Something isn't working

Comments

@LanceEa
Copy link
Contributor

LanceEa commented Oct 13, 2022

Summary

By default, Emissary-ingress adds http->https redirect routes for every host and the wild-card host (Note: this behavior can be overridden on a Host. However, in the v3 series it no longer works when an AuthService is applied because Envoy will try to call the service configured with ext_authz http filter prior to doing the http--> https redirect.

Below is the investigation on the behavior change.

TL;DR;

  • Envoy Behavior change in 1.20
  • V3 series of Emissary-ingress upgraded to 1.22+ adopting new behavior that breaks http --> https redirection
  • Emissary-ingress need to disable the ext_authz filter on a per route basis to restore previous behavior.

Understanding Envoy Behavior Change with Timeline:

July 27, 2021
envoyproxy/envoy#17502
Issue outlining the fact that a DirectResponse Route (issue doesn't say it but redirect response is in the same category) should always call Ext_Authz. This happens because the following check:

// Emissary-ingress (<= 2.4.2) which is based on Envoy 1.17.4
if (route == nullptr || route->routeEntry() == nullptr) {
return PerRouteFlags{true /*skip_check_*/, false /*skip_request_body_buffering_*/};
}

The skip_check_ causes the ext_authz filter to be skipped and continue execution along to the next filter in the HTTP Filter chain. If we take a look at sample Envoy Configuration generated by Emissary-ingress:

// removed extra stuff to reduce noise
// example of the FilterChain within the 8080 Listener that would enable http --> https redirect
{
  "filter_chain_match": {},
  "filters": [
    {
      "name": "envoy.filters.network.http_connection_manager",
      "typed_config": {
        "@type": "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager",
        "http_filters": [
          {
            "name": "envoy.filters.http.ext_authz",
            "typed_config": {
              "@type": "type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthz",
              "clear_route_cache": true,
              "grpc_service": {
                "envoy_grpc": {
                  "cluster_name": "cluster_extauth_127_0_0_1_8500_ambassador"
                },
                "timeout": "5.000s"
              },
              "transport_api_version": "V3"
            }
          },
          {
            "name": "envoy.filters.http.router"
          }
        ],
        "route_config": {
          "virtual_hosts": [
            {
              "domains": [
                "*"
              ],
              "name": "emissary-ingress-listener-8080-*",
              "routes": [
                {
                  "match": {
                    "case_sensitive": true,
                    "headers": [
                      {
                        "exact_match": "https",
                        "name": "x-forwarded-proto"
                      }
                    ],
                    "prefix": "/backend/",
                    "runtime_fraction": {
                      "default_value": {
                        "denominator": "HUNDRED",
                        "numerator": 100
                      },
                      "runtime_key": "routing.traffic_shift.cluster_quote_default_default"
                    }
                  },
                  "route": {
                    "cluster": "cluster_quote_default_default",
                    "prefix_rewrite": "/",
                    "priority": null,
                    "timeout": "3.000s"
                  }
                },
                // This is the "route" that is matched by HCM  on non-tls requests
                {
                  "match": {
                    "case_sensitive": true,
                    "prefix": "/backend/",
                    "runtime_fraction": {
                      "default_value": {
                        "denominator": "HUNDRED",
                        "numerator": 100
                      },
                      "runtime_key": "routing.traffic_shift.cluster_quote_default_default"
                    }
                  },
                  "redirect": {
                    "https_redirect": true
                  }
                }
              ]
            }
          ]
        }
      }
    }
  ]
}

Based on the above ext_authz logic when the HCM finds the Route in our configuration with the https_redirect it gets passed to this if (route == nullptr || route->routeEntry() == nullptr) logic. Since we have a Route it will pass the first logical test since it is not a nullpty. The call to route-->routeEntry() == nullptr is always true when it is a DirectResponse or a RedirectResponse. This can be seen here:

Because routeEntry() always returns nullptr it means that PerRouteFlags{true /*skip_check_*/, false /*skip_request_body_buffering_*/} is always returned, skipping the ext_authz filter and allowing the http-->https redirect.

August 10th, 2021

The following PR
envoyproxy/envoy#17546 landed fixing the issue and changing Envoy behavior as of 1.20 (released on Oct. 5th 2021). Here you can see the changelog where it is mentioned:
https://www.envoyproxy.io/docs/envoy/v1.23.1/version_history/v1.20/v1.20.0#minor-behavior-changes.

The override flag that would have reverted to the old behavior was removed in Envoy 1.23 which is the current version of Envoy that is shipped with v3.2.0.

How do we address this?

PerRouteFlags setting the skip rules for the ext_authz filter can be added to our http --> https redirect Routes when a AuthService exist.

We already provide a RouteSpecific toggle for a Mapping via the bypass_auth flag but this would remove it for a whole route regardless of TLS or non-tls connection. Therefore, we would want it to be applied only to the RedirectRoutes that are added.

An example can be found here:
https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/ext_authz_filter#per-route-configuration

@LanceEa LanceEa added the t:bug Something isn't working label Oct 13, 2022
@LanceEa LanceEa self-assigned this Oct 13, 2022
@LanceEa LanceEa changed the title http to https redirect doesn't occur when AuthService applied https redirect doesn't occur when AuthService applied Oct 13, 2022
@LanceEa LanceEa changed the title https redirect doesn't occur when AuthService applied bug: https redirect doesn't occur when AuthService applied Oct 13, 2022
@LanceEa LanceEa changed the title bug: https redirect doesn't occur when AuthService applied https redirect doesn't occur when AuthService applied Oct 13, 2022
LanceEa pushed a commit that referenced this issue Oct 13, 2022
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>
LanceEa pushed a commit that referenced this issue Oct 13, 2022
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>
LanceEa pushed a commit that referenced this issue Oct 14, 2022
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>
LanceEa pushed a commit that referenced this issue Oct 14, 2022
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>
LanceEa pushed a commit that referenced this issue Oct 15, 2022
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>
LanceEa pushed a commit that referenced this issue Oct 15, 2022
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>
LanceEa pushed a commit that referenced this issue Oct 16, 2022
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>
LanceEa pushed a commit that referenced this issue Oct 16, 2022
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>
LanceEa pushed a commit that referenced this issue Oct 16, 2022
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>
LanceEa pushed a commit that referenced this issue Oct 16, 2022
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>
LanceEa pushed a commit that referenced this issue Oct 17, 2022
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant