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

Draft: feat(translation): expand sources of JWKS required to validate JWTs #4684

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 73 additions & 3 deletions api/v1alpha1/jwt_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

package v1alpha1

import gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"

// JWT defines the configuration for JSON Web Token (JWT) authentication.
type JWT struct {
// Optional determines whether a missing JWT is acceptable, defaulting to false if not specified.
Expand All @@ -23,6 +25,8 @@

// JWTProvider defines how a JSON Web Token (JWT) can be verified.
// +kubebuilder:validation:XValidation:rule="(has(self.recomputeRoute) && self.recomputeRoute) ? size(self.claimToHeaders) > 0 : true", message="claimToHeaders must be specified if recomputeRoute is enabled"
// +kubebuilder:validation:XValidation:rule="((has(self.remoteJWKS) && self.remoteJWKS) && (has(self.jwksSource) && self.jwksSource)", message="only one of jwksSource or remoteJWKS, should be specfied. remoteJWKS can be replaced by jwksSource."

Check failure on line 28 in api/v1alpha1/jwt_types.go

View workflow job for this annotation

GitHub Actions / lint

specfied ==> specified
// +kubebuilder:validation:XValidation:rule="(!has(self.remoteJWKS) && !has(self.jwksSource))", message="a jwksSource is required."
type JWTProvider struct {
// Name defines a unique name for the JWT provider. A name can have a variety of forms,
// including RFC1123 subdomains, RFC 1123 labels, or RFC 1035 labels.
Expand All @@ -49,8 +53,17 @@
Audiences []string `json:"audiences,omitempty"`

// RemoteJWKS defines how to fetch and cache JSON Web Key Sets (JWKS) from a remote
// HTTP/HTTPS endpoint.
RemoteJWKS RemoteJWKS `json:"remoteJWKS"`
// HTTP/HTTPS endpoint. This field is deprecated, https retrieval is supported by the uri
// type in JWKSSource
//
// +optional
RemoteJWKS *RemoteJWKS `json:"remoteJWKS"`

// JWKSSource defines how to fetch and cache JSON Web Key Sets (JWKS) from a given source.
// Inline, URI and Configmap/Secret contents are supported.
//
// +optional
JWKSSource *JWKSSource `json:"jwksSource"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the jwks.Type idea !
we have been meaning to also add a backendRefs field to remoteJwks so upstream connection/traffic parameters can be controlled, relates to #3536

so the 2 options here are

  1. Represent backendRefs as a Type
  2. Keep the remoteJwks field, and add a new one localJwks to support the inline and valueRef cases

cc @envoyproxy/gateway-maintainers

Copy link
Member

@zhaohuabing zhaohuabing Nov 15, 2024

Choose a reason for hiding this comment

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

+1 for 2 as the structure is more straightfoward and less levels. It's a bit messy to add BackendCluster, URL, inline, and valueRef in a single structure.

      remoteJWKS:
        backendRefs:
        - group: gateway.envoyproxy.io
          kind: Backend
          name: backend-fqdn
          port: 443
        backendSettings:
          retry:
            numRetries: 3
            perRetry:
              backOff:
                baseInterval: 1s
                maxInterval: 5s
            retryOn:
              triggers: ["5xx", "gateway-error", "reset"]
      localJWKS:
        type: inline
        vaule: xxxxxxx

    jwksSource:
      type: BackendCluster
      backendCluster:
        backendRefs:
        - group: gateway.envoyproxy.io
          kind: Backend
          name: backend-fqdn
          port: 443
        backendSettings:
          retry:
            numRetries: 3
            perRetry:
              backOff:
                baseInterval: 1s
                maxInterval: 5s
            retryOn:
              triggers: ["5xx", "gateway-error", "reset"]

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for 2

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for sharing your thoughts folks, lets go with 2.


// ClaimToHeaders is a list of JWT claims that must be extracted into HTTP request headers
// For examples, following config:
Expand All @@ -76,7 +89,7 @@
}

// RemoteJWKS defines how to fetch and cache JSON Web Key Sets (JWKS) from a remote
// HTTP/HTTPS endpoint.
// HTTP/HTTPS endpoint. Note this has been replaced by JWKSSource and is deprecated.
type RemoteJWKS struct {
// URI is the HTTPS URI to fetch the JWKS. Envoy's system trust bundle is used to
// validate the server certificate.
Expand All @@ -88,6 +101,63 @@
// TODO: Add TBD remote JWKS fields based on defined use cases.
}

// JWKSSourceType defines the types of values for the source of JSON Web Key Sets (JWKS) for use with a JWTProvider.
// +kubebuilder:validation:Enum=Inline;URI;ValueRef
type JWKSSourceType string

const (
// JWKSSourceTypeInline defines the "Inline" type, indicating that the JWKS json is provided inline
JWKSSourceTypeInline JWKSSourceType = "Inline"

// JWKSSourceTypeURI defines the "URI" type, indicating that the JWKS json should be retrieved from the given uri
// typically a https endpoint or a local file.
JWKSSourceTypeURI JWKSSourceType = "URI"

// JWKSSourceTypeValueRefdefines the "ValueRef" type, indicating that the JWKS json should be loaded from
// the given LocalObjectReference
JWKSSourceTypeValueRef JWKSSourceType = "ValueRef"
)

// JWKSSource defines how to load a JSON Web Key Sets (JWKS) from various source locations.
//
// +kubebuilder:validation:XValidation:message="uri must be set for type URI",rule="(!has(self.type) || self.type == 'URI')? has(self.path) : true"
// +kubebuilder:validation:XValidation:message="inline must be set for type Inline",rule="(!has(self.type) || self.type == 'Inline')? has(self.inline) : true"
// +kubebuilder:validation:XValidation:message="valueRef must be set for type ValueRef",rule="(has(self.type) && self.type == 'ValueRef')? has(self.valueRef) : true"
type JWKSSource struct {

// Type is the type of method to use to read the contents of the JWKS.
// Valid values are Inline, URI and ValueRef, default is URI.
//
// +kubebuilder:default=Inline
// +kubebuilder:validation:Enum=Inline;URI;ValueRef
// +unionDiscriminator
Type *JWKSSourceType `json:"type"`

// URI is a location of the JWKS for File or HTTPS based retrieval. The location of the contents
// are determined by examining the scheme of the given URI. This covers the HTTPS usecase supported
// by RemoteJWKS
//
// Envoy's system trust bundle is used to validate the server certificate for HTTPS retrieval
//
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
// +optional
URI *string `json:"uri,omitempty"`

// Inline contains the value as an inline string.
//
// +optional
Inline *string `json:"inline,omitempty"`

// ValueRef contains the contents of the JWKS specified as a local object reference specifically
// a ConfigMap or Secret.
//
// The first value in the Secret/ConfigMap will be used as the contents for the JWKS
//
// +optional
ValueRef *gwapiv1.LocalObjectReference `json:"valueRef,omitempty"`
}

// ClaimToHeader defines a configuration to convert JWT claims into HTTP headers
type ClaimToHeader struct {
// Header defines the name of the HTTP request header that the JWT Claim will be saved into.
Expand Down
59 changes: 43 additions & 16 deletions api/v1alpha1/validation/securitypolicy_validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestValidateSecurityPolicy(t *testing.T) {
Name: "test",
Issuer: "https://www.test.local",
Audiences: []string{"test.local"},
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "https://test.local/jwt/public-key/jwks.json",
},
},
Expand Down Expand Up @@ -86,7 +86,7 @@ func TestValidateSecurityPolicy(t *testing.T) {
Name: "test",
Issuer: "[email protected]",
Audiences: []string{"test.local"},
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "https://test.local/jwt/public-key/jwks.json",
},
},
Expand Down Expand Up @@ -114,7 +114,7 @@ func TestValidateSecurityPolicy(t *testing.T) {
Name: "test",
Issuer: "[email protected]",
Audiences: []string{"test.local"},
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "https://test.local/jwt/public-key/jwks.json",
},
ClaimToHeaders: []egv1a1.ClaimToHeader{
Expand Down Expand Up @@ -148,7 +148,7 @@ func TestValidateSecurityPolicy(t *testing.T) {
Name: "unqualified_...",
Issuer: "https://www.test.local",
Audiences: []string{"test.local"},
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "https://test.local/jwt/public-key/jwks.json",
},
},
Expand Down Expand Up @@ -176,7 +176,7 @@ func TestValidateSecurityPolicy(t *testing.T) {
Name: "",
Issuer: "https://www.test.local",
Audiences: []string{"test.local"},
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "https://test.local/jwt/public-key/jwks.json",
},
},
Expand Down Expand Up @@ -204,23 +204,23 @@ func TestValidateSecurityPolicy(t *testing.T) {
Name: "unique",
Issuer: "https://www.test.local",
Audiences: []string{"test.local"},
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "https://test.local/jwt/public-key/jwks.json",
},
},
{
Name: "non-unique",
Issuer: "https://www.test.local",
Audiences: []string{"test.local"},
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "https://test.local/jwt/public-key/jwks.json",
},
},
{
Name: "non-unique",
Issuer: "https://www.test.local",
Audiences: []string{"test.local"},
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "https://test.local/jwt/public-key/jwks.json",
},
},
Expand Down Expand Up @@ -248,7 +248,7 @@ func TestValidateSecurityPolicy(t *testing.T) {
Name: "test",
Issuer: "http://invalid url.local",
Audiences: []string{"test.local"},
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "http://www.test.local",
},
},
Expand Down Expand Up @@ -276,7 +276,7 @@ func TestValidateSecurityPolicy(t *testing.T) {
Name: "test",
Issuer: "test@!123...",
Audiences: []string{"test.local"},
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "https://test.local/jwt/public-key/jwks.json",
},
},
Expand Down Expand Up @@ -304,7 +304,7 @@ func TestValidateSecurityPolicy(t *testing.T) {
Name: "test",
Issuer: "http://www.test.local",
Audiences: []string{"test.local"},
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "invalid/local",
},
},
Expand All @@ -331,7 +331,7 @@ func TestValidateSecurityPolicy(t *testing.T) {
{
Name: "test",
Audiences: []string{"test.local"},
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "",
},
},
Expand Down Expand Up @@ -359,7 +359,7 @@ func TestValidateSecurityPolicy(t *testing.T) {
Name: "test",
Issuer: "[email protected]",
Audiences: []string{"test.local"},
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "https://test.local/jwt/public-key/jwks.json",
},
ClaimToHeaders: []egv1a1.ClaimToHeader{
Expand Down Expand Up @@ -393,7 +393,7 @@ func TestValidateSecurityPolicy(t *testing.T) {
Name: "test",
Issuer: "[email protected]",
Audiences: []string{"test.local"},
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "https://test.local/jwt/public-key/jwks.json",
},
ClaimToHeaders: []egv1a1.ClaimToHeader{
Expand Down Expand Up @@ -426,7 +426,7 @@ func TestValidateSecurityPolicy(t *testing.T) {
{
Name: "test",
Audiences: []string{"test.local"},
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "https://test.local/jwt/public-key/jwks.json",
},
},
Expand All @@ -453,7 +453,34 @@ func TestValidateSecurityPolicy(t *testing.T) {
{
Name: "test",
Issuer: "https://www.test.local",
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "https://test.local/jwt/public-key/jwks.json",
},
},
},
},
},
},
expected: true,
},
{
name: "unspecified audiences",
policy: &egv1a1.SecurityPolicy{
TypeMeta: metav1.TypeMeta{
Kind: egv1a1.KindSecurityPolicy,
APIVersion: egv1a1.GroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Namespace: "test",
Name: "test",
},
Spec: egv1a1.SecurityPolicySpec{
JWT: &egv1a1.JWT{
Providers: []egv1a1.JWTProvider{
{
Name: "test",
Issuer: "https://www.test.local",
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "https://test.local/jwt/public-key/jwks.json",
},
},
Expand Down
4 changes: 2 additions & 2 deletions internal/ir/xds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ var (
Providers: []egv1a1.JWTProvider{
{
Name: "test1",
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "https://test1.local",
},
},
Expand Down Expand Up @@ -1256,7 +1256,7 @@ func TestValidateJWT(t *testing.T) {
Name: "test",
Issuer: "https://test.local",
Audiences: []string{"test1", "test2"},
RemoteJWKS: egv1a1.RemoteJWKS{
RemoteJWKS: &egv1a1.RemoteJWKS{
URI: "https://test.local",
},
},
Expand Down
Loading