Skip to content

Commit

Permalink
Apply Disable ExtAuth from GlobalExtAuth and Remove Auth from HTTP Up…
Browse files Browse the repository at this point in the history 
…grade (#6661)

* fix #6617 and #6659

Changes:
- use dagRoute's AuthContext and AuthDisabled in HTTPS-Upgrade to fix 6659
- Use globalExtAuth.AuthPolicy.Disabled to calculate dagRoute.AuthDisabled
- Fix Tests

Signed-off-by: Saman Mahdanian <saman@mahdanian.xyz>

* add changelog

Signed-off-by: Saman Mahdanian <saman@mahdanian.xyz>

* fix indentation

Signed-off-by: Saman Mahdanian <saman@mahdanian.xyz>

* fix indentation in route.go

Signed-off-by: Saman Mahdanian <saman@mahdanian.xyz>

* disable ext_auth when upgrading to HTTPS

Signed-off-by: Saman Mahdanian <saman@mahdanian.xyz>

* fix tests for "disable ext_auth when upgrading to HTTPS"

Signed-off-by: Saman Mahdanian <saman@mahdanian.xyz>

* fix CHANGELOG for "disable ext_auth when upgrading to HTTPS"

Signed-off-by: Saman Mahdanian <saman@mahdanian.xyz>

* add tests for globalExtAuth.AuthPolicy.disabled proper behaviour

Signed-off-by: Saman Mahdanian <saman@mahdanian.xyz>

* fix gofumpt issue with global_authorization_test.go

Signed-off-by: Saman Mahdanian <saman@mahdanian.xyz>

* fix nil authorization extref issue while overwriting GlobalExtAuth

Signed-off-by: Saman Mahdanian <saman@mahdanian.xyz>

* fix linting issue

Signed-off-by: Saman Mahdanian <saman@mahdanian.xyz>

* Update changelogs/unreleased/6661-SamMHD-minor.md

Signed-off-by: Saman Mahdanian <saman@mahdanian.xyz>

---------

Signed-off-by: Saman Mahdanian <saman@mahdanian.xyz>
  • Loading branch information
@SamMHD
SamMHD authored Nov 22, 2024
1 parent 4607ac2 commit 3589fc0
Show file tree
Hide file tree
Showing 11 changed files with 408 additions and 206 deletions.
5 changes: 5 additions & 0 deletions apis/projectcontour/v1/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ func (v *VirtualHost) DisableAuthorization() bool {
return false
}

// IsConfigured returns whether service ref is configured
func (r *ExtensionServiceReference) IsConfigured() bool {
return r.Name != ""
}

// AuthorizationContext returns the authorization policy context (if present).
func (v *VirtualHost) AuthorizationContext() map[string]string {
if v.AuthorizationConfigured() {
Expand Down
10 changes: 10 additions & 0 deletions changelogs/unreleased/6661-SamMHD-minor.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
## Disable ExtAuth by default if GlobalExtAuth.AuthPolicy.Disabled is set

Global external authorization can now be disabled by default and enabled by overriding the vhost and route level auth policies.
This is achieved by setting the `globalExtAuth.authPolicy.disabled` in the configuration file or `ContourConfiguration` CRD to `true`, and setting the `authPolicy.disabled` to `false` in the vhost and route level auth policies.
The final authorization state is determined by the most specific policy applied at the route level.

## Disable External Authorization in HTTPS Upgrade

When external authorization is enabled, no authorization check will be performed for HTTP to HTTPS redirection.
Previously, external authorization was checked before redirection, which could result in a 401 Unauthorized error instead of a 301 Moved Permanently status code.
25 changes: 20 additions & 5 deletions internal/dag/httpproxy_processor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -852,16 +852,31 @@ func (p *HTTPProxyProcessor) computeRoutes(
// enable it on the route and propagate defaults
// downwards.
if rootProxy.Spec.VirtualHost.AuthorizationConfigured() || p.GlobalExternalAuthorization != nil {
// Global external or vhost-level authorization is enabled by default
// unless an AuthPolicy explicitly disables it. By default, `disabled`
// is set to false, meaning authorization is active. This global setting
// can be overridden by vhost-level AuthPolicy, which can further be
// overridden by route-specific AuthPolicy.
// Therefore, the final authorization state is determined by the
// most specific policy applied at the route level.
disabled := false

if p.GlobalExternalAuthorization != nil && p.GlobalExternalAuthorization.AuthPolicy != nil {
disabled = p.GlobalExternalAuthorization.AuthPolicy.Disabled
}

// When the ext_authz filter is added to a
// vhost, it is in enabled state, but we can
// disable it per route. We emulate disabling
// it at the vhost layer by defaulting the state
// from the root proxy.
disabled := rootProxy.Spec.VirtualHost.DisableAuthorization()
if rootProxy.Spec.VirtualHost.AuthorizationConfigured() {
disabled = rootProxy.Spec.VirtualHost.DisableAuthorization()
}

// Take the default for enabling authorization
// from the virtual host. If this route has a
// policy, let that override.
// from the virtualhost/global-extauth. If this
// route has a policy, let that override.
if route.AuthPolicy != nil {
disabled = route.AuthPolicy.Disabled
}
Expand Down Expand Up @@ -1441,14 +1456,14 @@ func determineExternalAuthTimeout(responseTimeout string, validCond *contour_v1.
}

func (p *HTTPProxyProcessor) computeSecureVirtualHostAuthorization(validCond *contour_v1.DetailedCondition, httpproxy *contour_v1.HTTPProxy, svhost *SecureVirtualHost) bool {
if httpproxy.Spec.VirtualHost.AuthorizationConfigured() && !httpproxy.Spec.VirtualHost.DisableAuthorization() {
if httpproxy.Spec.VirtualHost.AuthorizationConfigured() && !httpproxy.Spec.VirtualHost.DisableAuthorization() && httpproxy.Spec.VirtualHost.Authorization.ExtensionServiceRef.IsConfigured() {
authorization := p.computeVirtualHostAuthorization(httpproxy.Spec.VirtualHost.Authorization, validCond, httpproxy)
if authorization == nil {
return false
}

svhost.ExternalAuthorization = authorization
} else if p.GlobalExternalAuthorization != nil && !httpproxy.Spec.VirtualHost.DisableAuthorization() {
} else if p.GlobalExternalAuthorization != nil {
globalAuthorization := p.computeVirtualHostAuthorization(p.GlobalExternalAuthorization, validCond, httpproxy)
if globalAuthorization == nil {
return false
Expand Down
21 changes: 19 additions & 2 deletions internal/envoy/v3/route.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,14 +121,18 @@ func buildRoute(dagRoute *dag.Route, vhostName string, secure bool) *envoy_confi
// envoy.RouteRoute. Currently the DAG processor adds any HTTP->HTTPS
// redirect routes to *both* the insecure and secure vhosts.
route.Action = UpgradeHTTPS()

// Disable External Authorization it is being redirected to HTTPS route
route.TypedPerFilterConfig = map[string]*anypb.Any{}
route.TypedPerFilterConfig[ExtAuthzFilterName] = routeAuthzDisabled()
case dagRoute.DirectResponse != nil:
route.TypedPerFilterConfig = map[string]*anypb.Any{}

// Apply per-route authorization policy modifications.
if dagRoute.AuthDisabled {
route.TypedPerFilterConfig["envoy.filters.http.ext_authz"] = routeAuthzDisabled()
route.TypedPerFilterConfig[ExtAuthzFilterName] = routeAuthzDisabled()
} else if len(dagRoute.AuthContext) > 0 {
route.TypedPerFilterConfig["envoy.filters.http.ext_authz"] = routeAuthzContext(dagRoute.AuthContext)
route.TypedPerFilterConfig[ExtAuthzFilterName] = routeAuthzContext(dagRoute.AuthContext)
}

route.Action = routeDirectResponse(dagRoute.DirectResponse)
Expand Down Expand Up @@ -592,6 +596,19 @@ func UpgradeHTTPS() *envoy_config_route_v3.Route_Redirect {
}
}

// DisabledExtAuthConfig returns a route TypedPerFilterConfig that disables ExtAuth
func DisabledExtAuthConfig() map[string]*anypb.Any {
return map[string]*anypb.Any{
ExtAuthzFilterName: protobuf.MustMarshalAny(
&envoy_filter_http_ext_authz_v3.ExtAuthzPerRoute{
Override: &envoy_filter_http_ext_authz_v3.ExtAuthzPerRoute_Disabled{
Disabled: true,
},
},
),
}
}

// headerValueList creates a list of Envoy HeaderValueOptions from the provided map.
func headerValueList(hvm map[string]string, app bool) []*envoy_config_core_v3.HeaderValueOption {
var hvs []*envoy_config_core_v3.HeaderValueOption
Expand Down
40 changes: 19 additions & 21 deletions internal/featuretests/v3/authorization_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,13 +267,6 @@ func authzOverrideDisabled(t *testing.T, rh ResourceEventHandlerWrapper, c *Cont
// same authorization enablement as the root proxy, and
// the other path should have the opposite enablement.

disabledConfig := withFilterConfig(envoy_v3.ExtAuthzFilterName,
&envoy_filter_http_ext_authz_v3.ExtAuthzPerRoute{
Override: &envoy_filter_http_ext_authz_v3.ExtAuthzPerRoute_Disabled{
Disabled: true,
},
})

c.Request(routeType).Equals(&envoy_service_discovery_v3.DiscoveryResponse{
TypeUrl: routeType,
Resources: resources(t,
Expand All @@ -287,7 +280,7 @@ func authzOverrideDisabled(t *testing.T, rh ResourceEventHandlerWrapper, c *Cont
&envoy_config_route_v3.Route{
Match: routePrefix("/default"),
Action: routeCluster("default/app-server/80/da39a3ee5e"),
TypedPerFilterConfig: disabledConfig,
TypedPerFilterConfig: envoy_v3.DisabledExtAuthConfig(),
},
),
),
Expand All @@ -297,7 +290,7 @@ func authzOverrideDisabled(t *testing.T, rh ResourceEventHandlerWrapper, c *Cont
&envoy_config_route_v3.Route{
Match: routePrefix("/disabled"),
Action: routeCluster("default/app-server/80/da39a3ee5e"),
TypedPerFilterConfig: disabledConfig,
TypedPerFilterConfig: envoy_v3.DisabledExtAuthConfig(),
},
&envoy_config_route_v3.Route{
Match: routePrefix("/default"),
Expand All @@ -309,22 +302,26 @@ func authzOverrideDisabled(t *testing.T, rh ResourceEventHandlerWrapper, c *Cont
"ingress_http",
envoy_v3.VirtualHost(disabled,
&envoy_config_route_v3.Route{
Match: routePrefix("/enabled"),
Action: withRedirect(),
Match: routePrefix("/enabled"),
Action: withRedirect(),
TypedPerFilterConfig: envoy_v3.DisabledExtAuthConfig(),
},
&envoy_config_route_v3.Route{
Match: routePrefix("/default"),
Action: withRedirect(),
Match: routePrefix("/default"),
Action: withRedirect(),
TypedPerFilterConfig: envoy_v3.DisabledExtAuthConfig(),
},
),
envoy_v3.VirtualHost(enabled,
&envoy_config_route_v3.Route{
Match: routePrefix("/disabled"),
Action: withRedirect(),
Match: routePrefix("/disabled"),
Action: withRedirect(),
TypedPerFilterConfig: envoy_v3.DisabledExtAuthConfig(),
},
&envoy_config_route_v3.Route{
Match: routePrefix("/default"),
Action: withRedirect(),
Match: routePrefix("/default"),
Action: withRedirect(),
TypedPerFilterConfig: envoy_v3.DisabledExtAuthConfig(),
},
),
),
Expand Down Expand Up @@ -406,8 +403,9 @@ func authzMergeRouteContext(t *testing.T, rh ResourceEventHandlerWrapper, c *Con
"ingress_http",
envoy_v3.VirtualHost(fqdn,
&envoy_config_route_v3.Route{
Match: routePrefix("/"),
Action: withRedirect(),
Match: routePrefix("/"),
Action: withRedirect(),
TypedPerFilterConfig: envoy_v3.DisabledExtAuthConfig(),
},
),
),
Expand All @@ -433,8 +431,8 @@ func authzInvalidReference(t *testing.T, rh ResourceEventHandlerWrapper, c *Cont

invalid.Spec.VirtualHost.Authorization.ExtensionServiceRef = contour_v1.ExtensionServiceReference{
APIVersion: "foo/bar",
Namespace: "",
Name: "",
Namespace: "missing",
Name: "extension",
}

rh.OnDelete(invalid)
Expand Down
5 changes: 3 additions & 2 deletions internal/featuretests/v3/envoy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,9 @@ func routeHostRewriteHeader(cluster, hostnameHeader string) *envoy_config_route_

func upgradeHTTPS(match *envoy_config_route_v3.RouteMatch) *envoy_config_route_v3.Route {
return &envoy_config_route_v3.Route{
Match: match,
Action: envoy_v3.UpgradeHTTPS(),
Match: match,
Action: envoy_v3.UpgradeHTTPS(),
TypedPerFilterConfig: envoy_v3.DisabledExtAuthConfig(),
}
}

Expand Down
Loading

0 comments on commit 3589fc0

Please sign in to comment.