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.

Sign up for GitHub

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

Jump to bottom

Authorization HTTPFilter #532

Closed
wants to merge 2 commits into from
Closed

Authorization HTTPFilter #532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c9816f8
Adds new RequestAuthorization HTTPFilter
jmprusi Jun 18, 2021
4ef30d9
make generate
jmprusi Jun 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
75 changes: 74 additions & 1 deletion apis/v1alpha1/httproute_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,13 @@ type HTTPRouteFilter struct {
// +optional
RequestMirror *HTTPRequestMirrorFilter `json:"requestMirror,omitempty"`

// RequestAuhtorization defines a schema for a filter that authorizes requests.
//
// Support: Extended
//
// +optional
RequestAuthorization *HTTPRequestAuthorization `json:"requestAuthorization,omitempty"`

// ExtensionRef is an optional, implementation-specific extension to the
// "filter" behavior. For example, resource "myroutefilter" in group
// "networking.acme.io"). ExtensionRef MUST NOT be used for core and
Expand All @@ -491,7 +498,7 @@ type HTTPRouteFilter struct {
}

// HTTPRouteFilterType identifies a type of HTTPRoute filter.
// +kubebuilder:validation:Enum=RequestHeaderModifier;RequestMirror;ExtensionRef
// +kubebuilder:validation:Enum=RequestHeaderModifier;RequestMirror;ExtensionRef;RequestAuthorization
type HTTPRouteFilterType string

const (
Expand All @@ -512,6 +519,13 @@ const (
// Support in HTTPRouteForwardTo: Extended
HTTPRouteFilterRequestMirror HTTPRouteFilterType = "RequestMirror"

// HTTPRouteFilterRequestAuthorization can be used to authorize a request with an
// external authorization endpoint before being sent to the upstream target.
//
// Support in HTTPRouteRule: Extended
// Support in HTTPRouteForwardTo: Extended
HTTPRouteFilterRequestAuthorization HTTPRouteFilterType = "RequestAuthorization"

// HTTPRouteFilterExtensionRef should be used for configuring custom
// HTTP filters.
//
Expand Down Expand Up @@ -631,6 +645,65 @@ type HTTPRequestMirrorFilter struct {
Port *PortNumber `json:"port,omitempty"`
}

// HTTPRequestAuthorization defines configuration for the RequestAuthorization filter.
type HTTPRequestAuthorization struct {
// URL specifies the address of the service used to perform the authorization check.
//
// Support: Extended
//
URL string `json:"url"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does configuration of how to reach this URl work? For example, if we need authentication, TLS, etc.

if it were a k8s service it would be "easy" since existing APIs defined here like BackendPolicy would apply. For a generic URL, I am not sure its easy.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's a good point. Thinking out loud, perhaps splitting the URL into a service reference and a type or protocol field to define the type of authorization (grpc/HTTP) could work?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That could potentially work, I think some sort of reference is probably more consistent with the rest of the API as well. FWIW we recently went through lots of debate with https://docs.google.com/document/d/1V4mCQCw7mlGp0zSQQXYoBdbKMDnkPOjeyUb85U07iSI/ about this exact topic in Istio.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @howardjohn! I will review that document and try to find a better API design.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just getting to review this one now, Contour added an ExtensionService to cover the "service reference" part of this (see https://projectcontour.io/docs/v1.12.0/config/api/#projectcontour.io/v1alpha1.ExtensionService), and then an AuthorizationServer struct that holds the ExtensionService reference. (https://projectcontour.io/docs/v1.12.0/config/api/#projectcontour.io/v1.AuthorizationServer). Both the ExtensionService and AuthorizationServer have places to put config.

I'm not suggesting doing the same thing, but I think the idea of a service reference and a type or protocol field would be great.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@youngnick It already has a protocol field, but for the service reference, we can follow the example of the other filters and replace URL with:

ServiceName *string `json:"serviceName,omitempty"`
BackendRef *LocalObjectReference `json:"backendRef,omitempty"`
Port *PortNumber `json:"port,omitempty"`

Or add a new structure that allows us to cross the namespace boundary, with additional information on how to connect to the external service, like for example a TLS client certificate to use.


// Protocol is used to define the Protocol that the gateway will use to communicate with the authorization
// service. If not set, it will default to "http" or inferred from the scheme in URL.
// Examples: "grpc","http2"
//
// Support: Extended
//
// +optional
// +kubebuilder:default=http
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add enum validation for allowed values?

Protocol string `json:"protocol,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For protocols over TLS, is there a way to tweak the server certificate validation?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that would great to have, but I don't see any example of this in the API. I looked at the HTTPRouteForwardTo struct, and it doesn't seem to support any TLS settings. All the other TLS settings seem to be focused on the "listener side," not starting a connection.

Any suggestion on how can we add this to the filter?


// FailModeAllow defines how this authorization filter should be handled if it timeouts or fails to
// contact the URL defined. This field accepts two options:
//
// - True: Fail open. Traffic will be allowed even if URL can't be contacted or timeouts
// - False: Fail closed. Default value. Traffic will be denied if the gateway can't reach the URL or timeouts.
//
// Support: Extended
//
// +optional
// +kubebuilder:default=false
FailModeAllow *bool `json:"failModeAllow,omitempty"`

// RequestBody allows to define if the body of the request should be sent to the authorization service.
//
// - True: The request body will be sent to the authorization endpoint.
// - False: Don't send the request body. Default value.
//
// Support: Extended
//
// +optional
// +kubebuilder:default=false
RequestBody *bool `json:"requestBody,omitempty"`

// TimeoutMs is the duration that the authorization call is allowed to operate.
// Default if unset is 100
//
// Support: Extended
//
// +optional
// +kubebuilder:default=100
TimeoutMs int `json:"timeoutMs,omitempty"`

// Metadata can be used to provide additional context information for the authorization
// service to make a decision.
//
// Support: Extended
//
// +optional
Metadata map[string]string `json:"metadata,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK, only the envoy GRPC protocol can pass arbitrary key value pairs here. How do you indicate missing or partial support for this field?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is for the implementor to decide; for example, in Nginx, this could be implemented as a custom HTTP header sent to the authorization service. Perhaps we should put some validation in place to make that easier, like a max length and allowed characters.

If the implementation doesn't support this, it can reject the route and set the proper status condition.

}

// HTTPRouteForwardTo defines how a HTTPRoute should forward a request.
type HTTPRouteForwardTo struct {
// ServiceName refers to the name of the Service to forward matched requests
Expand Down
37 changes: 37 additions & 0 deletions apis/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

110 changes: 110 additions & 0 deletions config/crd/bases/networking.x-k8s.io_httproutes.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.