diff --git a/api/v1alpha1/authorization_types.go b/api/v1alpha1/authorization_types.go index 628cda7c773..2c43d1954c6 100644 --- a/api/v1alpha1/authorization_types.go +++ b/api/v1alpha1/authorization_types.go @@ -248,7 +248,9 @@ type AuthorizationHeaderMatch struct { type JWTPrincipal struct { // Provider is the name of the JWT provider that used to verify the JWT token. // In order to use JWT claims for authorization, you must configure the JWT - // authentication with the same provider in the same `SecurityPolicy`. + // authentication with the same provider in the same `SecurityPolicy`, or, + // when `mergeType` is set, in the parent `SecurityPolicy` this policy is + // merged into. // // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=253 diff --git a/api/v1alpha1/securitypolicy_types.go b/api/v1alpha1/securitypolicy_types.go index bf8f4f8fe89..36d7952b09c 100644 --- a/api/v1alpha1/securitypolicy_types.go +++ b/api/v1alpha1/securitypolicy_types.go @@ -50,7 +50,7 @@ type SecurityPolicy struct { // +kubebuilder:validation:XValidation:rule="has(self.targetRefs) ? self.targetRefs.all(ref, ref.group == 'gateway.networking.k8s.io') : true ", message="this policy can only have a targetRefs[*].group of gateway.networking.k8s.io" // +kubebuilder:validation:XValidation:rule="has(self.targetRefs) ? self.targetRefs.all(ref, ref.kind in ['Gateway', 'ListenerSet', 'HTTPRoute', 'GRPCRoute', 'TCPRoute']) : true ", message="this policy can only have a targetRefs[*].kind of Gateway/ListenerSet/HTTPRoute/GRPCRoute/TCPRoute" // +kubebuilder:validation:XValidation:rule="!has(self.mergeType) || ((!has(self.targetRef) || self.targetRef.kind in ['HTTPRoute', 'GRPCRoute', 'TCPRoute']) && (!has(self.targetRefs) || self.targetRefs.all(ref, ref.kind in ['HTTPRoute', 'GRPCRoute', 'TCPRoute'])) && (!has(self.targetSelectors) || self.targetSelectors.all(sel, sel.kind in ['HTTPRoute', 'GRPCRoute', 'TCPRoute'])))", message="mergeType can only be used with xRoute targets" -// +kubebuilder:validation:XValidation:rule="(has(self.authorization) && has(self.authorization.rules) && self.authorization.rules.exists(r, has(r.principal) ? has(r.principal.jwt) : false)) ? has(self.jwt) : true", message="if authorization.rules.principal.jwt is used, jwt must be defined" +// +kubebuilder:validation:XValidation:rule="(!has(self.mergeType) && has(self.authorization) && has(self.authorization.rules) && self.authorization.rules.exists(r, has(r.principal) ? has(r.principal.jwt) : false)) ? has(self.jwt) : true", message="if authorization.rules.principal.jwt is used, jwt must be defined, unless mergeType is set and the jwt providers are inherited from the parent policy" type SecurityPolicySpec struct { PolicyTargetReferences `json:",inline"` diff --git a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml index 7fc9d74ed3b..a01faa249b2 100644 --- a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -518,7 +518,9 @@ spec: description: |- Provider is the name of the JWT provider that used to verify the JWT token. In order to use JWT claims for authorization, you must configure the JWT - authentication with the same provider in the same `SecurityPolicy`. + authentication with the same provider in the same `SecurityPolicy`, or, + when `mergeType` is set, in the parent `SecurityPolicy` this policy is + merged into. maxLength: 253 minLength: 1 type: string @@ -8114,9 +8116,11 @@ spec: || self.targetRefs.all(ref, ref.kind in [''HTTPRoute'', ''GRPCRoute'', ''TCPRoute''])) && (!has(self.targetSelectors) || self.targetSelectors.all(sel, sel.kind in [''HTTPRoute'', ''GRPCRoute'', ''TCPRoute''])))' - - message: if authorization.rules.principal.jwt is used, jwt must be defined - rule: '(has(self.authorization) && has(self.authorization.rules) && - self.authorization.rules.exists(r, has(r.principal) ? has(r.principal.jwt) + - message: if authorization.rules.principal.jwt is used, jwt must be defined, + unless mergeType is set and the jwt providers are inherited from the + parent policy + rule: '(!has(self.mergeType) && has(self.authorization) && has(self.authorization.rules) + && self.authorization.rules.exists(r, has(r.principal) ? has(r.principal.jwt) : false)) ? has(self.jwt) : true' status: description: Status defines the current status of SecurityPolicy. diff --git a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml index 2430d5b595f..5259bc8b8ee 100644 --- a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -517,7 +517,9 @@ spec: description: |- Provider is the name of the JWT provider that used to verify the JWT token. In order to use JWT claims for authorization, you must configure the JWT - authentication with the same provider in the same `SecurityPolicy`. + authentication with the same provider in the same `SecurityPolicy`, or, + when `mergeType` is set, in the parent `SecurityPolicy` this policy is + merged into. maxLength: 253 minLength: 1 type: string @@ -8113,9 +8115,11 @@ spec: || self.targetRefs.all(ref, ref.kind in [''HTTPRoute'', ''GRPCRoute'', ''TCPRoute''])) && (!has(self.targetSelectors) || self.targetSelectors.all(sel, sel.kind in [''HTTPRoute'', ''GRPCRoute'', ''TCPRoute''])))' - - message: if authorization.rules.principal.jwt is used, jwt must be defined - rule: '(has(self.authorization) && has(self.authorization.rules) && - self.authorization.rules.exists(r, has(r.principal) ? has(r.principal.jwt) + - message: if authorization.rules.principal.jwt is used, jwt must be defined, + unless mergeType is set and the jwt providers are inherited from the + parent policy + rule: '(!has(self.mergeType) && has(self.authorization) && has(self.authorization.rules) + && self.authorization.rules.exists(r, has(r.principal) ? has(r.principal.jwt) : false)) ? has(self.jwt) : true' status: description: Status defines the current status of SecurityPolicy. diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index b598766fde4..46c77511231 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -511,28 +511,30 @@ func (t *Translator) processSecurityPolicyForRoute( return } - // Protocol-specific validation: pick the appropriate validator and message, - // then run it once to keep the flow linear and easier to read. + // Protocol-specific validation: pick the appropriate validator and message. + // A merging policy is only complete once it has been merged - its authorization + // rules may reference a JWT provider that the parent policy defines - so each + // branch below validates at the point where the policy is whole. validator := validateSecurityPolicy errMsg := "invalid SecurityPolicy" if currTarget.Kind == resource.KindTCPRoute { validator = validateSecurityPolicyForTCP errMsg = "invalid SecurityPolicy for TCP route" } - if err := validator(policy); err != nil { - status.SetTranslationErrorForPolicyAncestors(&policy.Status, - ancestorRefs, - t.GatewayControllerName, - policy.Generation, - status.Error2ConditionMsg(fmt.Errorf("%s: %w", errMsg, err)), - ) - - return - } // Check if merging is enabled if policy.Spec.MergeType == nil { - // No merging - use existing translation logic + // No merging - validate and translate the policy as it was written. + if err := validator(policy); err != nil { + status.SetTranslationErrorForPolicyAncestors(&policy.Status, + ancestorRefs, + t.GatewayControllerName, + policy.Generation, + status.Error2ConditionMsg(fmt.Errorf("%s: %w", errMsg, err)), + ) + return + } + if err := t.translateSecurityPolicyForRoute(policy, &securityPolicyOwners{}, targetedRoute, currTarget, resources, xdsIR, nil); err != nil { status.SetTranslationErrorForPolicyAncestors(&policy.Status, ancestorRefs, @@ -589,7 +591,20 @@ func (t *Translator) processSecurityPolicyForRoute( } if parentPolicy == nil { - // No parent policy found, fall back to current policy + // No parent policy found, fall back to the current policy. Nothing was + // inherited, so validate the policy exactly as it was written. + if err := validator(policy); err != nil { + status.SetConditionForPolicyAncestor(&policy.Status, + &ancestorRef, + t.GatewayControllerName, + gwapiv1.PolicyConditionAccepted, metav1.ConditionFalse, + egv1a1.PolicyReasonInvalid, + status.Error2ConditionMsg(err), + policy.Generation, + ) + continue + } + if err := t.translateSecurityPolicyForRoute(policy, &securityPolicyOwners{}, targetedRoute, currTarget, resources, xdsIR, listener); err != nil { status.SetConditionForPolicyAncestor(&policy.Status, &ancestorRef, @@ -968,9 +983,41 @@ func validateSecurityPolicy(p *egv1a1.SecurityPolicy) error { return err } } + + if err := validateAuthorizationJWTProviders(p); err != nil { + return err + } return nil } +// validateAuthorizationJWTProviders ensures every JWT principal in the authorization +// rules references a JWT provider that the policy defines. +func validateAuthorizationJWTProviders(p *egv1a1.SecurityPolicy) error { + if p.Spec.Authorization == nil { + return nil + } + + providers := sets.New[string]() + if p.Spec.JWT != nil { + for _, provider := range p.Spec.JWT.Providers { + providers.Insert(provider.Name) + } + } + + var errs []error + for i, rule := range p.Spec.Authorization.Rules { + if rule.Principal == nil || rule.Principal.JWT == nil { + continue + } + if !providers.Has(rule.Principal.JWT.Provider) { + errs = append(errs, fmt.Errorf( + "authorization rule %d: jwt provider %q is not defined in jwt.providers", + i, rule.Principal.JWT.Provider)) + } + } + return errors.Join(errs...) +} + // validateSecurityPolicyForTCP ensures SecurityPolicy usage on TCP is compatible. // // TCP supports Authorization with ClientCIDRs ONLY. diff --git a/internal/gatewayapi/securitypolicy_test.go b/internal/gatewayapi/securitypolicy_test.go index a3c12ffa4ee..9fbe8f8fc8b 100644 --- a/internal/gatewayapi/securitypolicy_test.go +++ b/internal/gatewayapi/securitypolicy_test.go @@ -778,6 +778,86 @@ func Test_OIDC_PassThroughAuthHeader(t *testing.T) { } } +func TestValidateAuthorizationJWTProviders(t *testing.T) { + jwtWith := func(names ...string) *egv1a1.JWT { + providers := make([]egv1a1.JWTProvider, 0, len(names)) + for _, name := range names { + providers = append(providers, egv1a1.JWTProvider{Name: name}) + } + return &egv1a1.JWT{Providers: providers} + } + authorizationWith := func(providers ...string) *egv1a1.Authorization { + rules := make([]egv1a1.AuthorizationRule, 0, len(providers)) + for _, provider := range providers { + rules = append(rules, egv1a1.AuthorizationRule{ + Action: egv1a1.AuthorizationActionAllow, + Principal: &egv1a1.Principal{JWT: &egv1a1.JWTPrincipal{Provider: provider}}, + }) + } + return &egv1a1.Authorization{Rules: rules} + } + + tests := []struct { + name string + jwt *egv1a1.JWT + authorization *egv1a1.Authorization + wantError bool + }{ + { + name: "provider defined in the same policy", + jwt: jwtWith("example"), + authorization: authorizationWith("example"), + }, + { + name: "no authorization rules", + jwt: jwtWith("example"), + authorization: nil, + }, + { + name: "principal without a jwt is ignored", + authorization: &egv1a1.Authorization{ + Rules: []egv1a1.AuthorizationRule{ + { + Action: egv1a1.AuthorizationActionAllow, + Principal: &egv1a1.Principal{ClientCIDRs: []egv1a1.CIDR{"10.0.0.0/8"}}, + }, + }, + }, + }, + { + name: "unknown provider", + jwt: jwtWith("example"), + authorization: authorizationWith("does-not-exist"), + wantError: true, + }, + { + name: "no jwt providers at all", + authorization: authorizationWith("example"), + wantError: true, + }, + { + name: "one known and one unknown provider", + jwt: jwtWith("example"), + authorization: authorizationWith("example", "does-not-exist"), + wantError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + policy := &egv1a1.SecurityPolicy{ + Spec: egv1a1.SecurityPolicySpec{ + JWT: tt.jwt, + Authorization: tt.authorization, + }, + } + + err := validateAuthorizationJWTProviders(policy) + require.Equal(t, tt.wantError, err != nil, "validateAuthorizationJWTProviders() error = %v", err) + }) + } +} + func ToPointer[T any](v T) *T { return &v } diff --git a/internal/gatewayapi/testdata/securitypolicy-with-merge-jwt-provider.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-merge-jwt-provider.in.yaml new file mode 100644 index 00000000000..43498cefafe --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-merge-jwt-provider.in.yaml @@ -0,0 +1,171 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-1 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: All + # No SecurityPolicy targets this listener, so a route attached to it has no + # parent policy to merge into. + - name: http-no-policy + protocol: HTTP + port: 8080 + allowedRoutes: + namespaces: + from: All +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-inherited-provider + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/inherited" + backendRefs: + - name: service-1 + port: 8080 +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-unknown-provider + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/unknown" + backendRefs: + - name: service-1 + port: 8080 +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-no-parent-policy + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http-no-policy + rules: + - matches: + - path: + value: "/no-parent" + backendRefs: + - name: service-1 + port: 8080 +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: envoy-gateway + name: policy-for-gateway + spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + sectionName: http + jwt: + providers: + - name: example + issuer: https://www.example.com + remoteJWKS: + uri: https://www.example.com/.well-known/jwks.json +# Accepted: the jwt providers are inherited from the Gateway-level policy, so this +# policy does not have to define them itself. +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: default + name: policy-inherited-provider + spec: + mergeType: StrategicMerge + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-inherited-provider + authorization: + defaultAction: Deny + rules: + - name: allow-mygroup + action: Allow + principal: + jwt: + provider: example + claims: + - name: roles + valueType: StringArray + values: + - mygroup +# Invalid: neither this policy nor the Gateway-level policy it merges into defines +# this provider. +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: default + name: policy-unknown-provider + spec: + mergeType: StrategicMerge + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-unknown-provider + authorization: + defaultAction: Deny + rules: + - name: allow-mygroup + action: Allow + principal: + jwt: + provider: does-not-exist + claims: + - name: roles + valueType: StringArray + values: + - mygroup +# Invalid: the route's listener has no parent policy, so there is nothing to +# inherit the provider from. +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: default + name: policy-no-parent-policy + spec: + mergeType: StrategicMerge + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-no-parent-policy + authorization: + defaultAction: Deny + rules: + - name: allow-mygroup + action: Allow + principal: + jwt: + provider: example + claims: + - name: roles + valueType: StringArray + values: + - mygroup diff --git a/internal/gatewayapi/testdata/securitypolicy-with-merge-jwt-provider.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-merge-jwt-provider.out.yaml new file mode 100644 index 00000000000..a3690f303f6 --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-merge-jwt-provider.out.yaml @@ -0,0 +1,547 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + - allowedRoutes: + namespaces: + from: All + name: http-no-policy + port: 8080 + protocol: HTTP + status: + listeners: + - attachedRoutes: 2 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http-no-policy + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + name: httproute-inherited-provider + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: /inherited + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + name: httproute-unknown-provider + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: /unknown + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + name: httproute-no-parent-policy + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http-no-policy + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: /no-parent + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http-no-policy +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - name: envoy-gateway/gateway-1/http + ports: + - containerPort: 10080 + name: http-80 + protocol: HTTP + servicePort: 80 + - name: envoy-gateway/gateway-1/http-no-policy + ports: + - containerPort: 8080 + name: http-8080 + protocol: HTTP + servicePort: 8080 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + ownerReference: + kind: GatewayClass + name: envoy-gateway-class + name: envoy-gateway/gateway-1 + namespace: envoy-gateway-system +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + name: policy-inherited-provider + namespace: default + spec: + authorization: + defaultAction: Deny + rules: + - action: Allow + name: allow-mygroup + principal: + jwt: + claims: + - name: roles + valueType: StringArray + values: + - mygroup + provider: example + mergeType: StrategicMerge + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-inherited-provider + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + conditions: + - lastTransitionTime: null + message: Merged with policy envoy-gateway/policy-for-gateway + reason: Merged + status: "True" + type: Merged + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + name: policy-unknown-provider + namespace: default + spec: + authorization: + defaultAction: Deny + rules: + - action: Allow + name: allow-mygroup + principal: + jwt: + claims: + - name: roles + valueType: StringArray + values: + - mygroup + provider: does-not-exist + mergeType: StrategicMerge + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-unknown-provider + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + conditions: + - lastTransitionTime: null + message: 'Authorization rule 0: jwt provider "does-not-exist" is not defined + in jwt.providers.' + reason: Invalid + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + name: policy-no-parent-policy + namespace: default + spec: + authorization: + defaultAction: Deny + rules: + - action: Allow + name: allow-mygroup + principal: + jwt: + claims: + - name: roles + valueType: StringArray + values: + - mygroup + provider: example + mergeType: StrategicMerge + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-no-parent-policy + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http-no-policy + conditions: + - lastTransitionTime: null + message: 'Authorization rule 0: jwt provider "example" is not defined in jwt.providers.' + reason: Invalid + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + name: policy-for-gateway + namespace: envoy-gateway + spec: + jwt: + providers: + - issuer: https://www.example.com + name: example + remoteJWKS: + uri: https://www.example.com/.well-known/jwks.json + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + sectionName: http + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: 'This policy is being merged by other securityPolicies for these + routes: [default/httproute-inherited-provider]' + reason: Merged + status: "True" + type: Merged + - lastTransitionTime: null + message: 'This policy is being overridden by other securityPolicies for these + routes: [default/httproute-unknown-provider]' + reason: Overridden + status: "True" + type: Overridden + controllerName: gateway.envoyproxy.io/gatewayclass-controller +xdsIR: + envoy-gateway/gateway-1: + accessLog: + json: + - path: /dev/stdout + globalResources: + proxyServiceCluster: + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + settings: + - addressType: IP + endpoints: + - host: 7.6.5.4 + port: 8080 + zone: zone1 + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + protocol: TCP + http: + - address: 0.0.0.0 + externalPort: 80 + hostnames: + - '*' + metadata: + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + name: envoy-gateway/gateway-1/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - destination: + metadata: + kind: HTTPRoute + name: httproute-inherited-provider + namespace: default + name: httproute/default/httproute-inherited-provider/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + metadata: + kind: Service + name: service-1 + namespace: default + sectionName: "8080" + name: httproute/default/httproute-inherited-provider/rule/0/backend/0 + protocol: HTTP + weight: 1 + hostname: '*' + isHTTP2: false + metadata: + kind: HTTPRoute + name: httproute-inherited-provider + namespace: default + name: httproute/default/httproute-inherited-provider/rule/0/match/0/* + pathMatch: + distinct: false + name: "" + prefix: /inherited + security: + authorization: + defaultAction: Deny + rules: + - action: Allow + name: allow-mygroup + principal: + jwt: + claims: + - name: roles + valueType: StringArray + values: + - mygroup + provider: example + jwt: + providers: + - issuer: https://www.example.com + name: example + remoteJWKS: + uri: https://www.example.com/.well-known/jwks.json + - destination: + metadata: + kind: HTTPRoute + name: httproute-unknown-provider + namespace: default + name: httproute/default/httproute-unknown-provider/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + metadata: + kind: Service + name: service-1 + namespace: default + sectionName: "8080" + name: httproute/default/httproute-unknown-provider/rule/0/backend/0 + protocol: HTTP + weight: 1 + hostname: '*' + isHTTP2: false + metadata: + kind: HTTPRoute + name: httproute-unknown-provider + namespace: default + name: httproute/default/httproute-unknown-provider/rule/0/match/0/* + pathMatch: + distinct: false + name: "" + prefix: /unknown + security: + jwt: + providers: + - issuer: https://www.example.com + name: example + remoteJWKS: + uri: https://www.example.com/.well-known/jwks.json + - address: 0.0.0.0 + externalPort: 8080 + hostnames: + - '*' + metadata: + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http-no-policy + name: envoy-gateway/gateway-1/http-no-policy + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 8080 + routes: + - destination: + metadata: + kind: HTTPRoute + name: httproute-no-parent-policy + namespace: default + name: httproute/default/httproute-no-parent-policy/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + metadata: + kind: Service + name: service-1 + namespace: default + sectionName: "8080" + name: httproute/default/httproute-no-parent-policy/rule/0/backend/0 + protocol: HTTP + weight: 1 + hostname: '*' + isHTTP2: false + metadata: + kind: HTTPRoute + name: httproute-no-parent-policy + namespace: default + name: httproute/default/httproute-no-parent-policy/rule/0/match/0/* + pathMatch: + distinct: false + name: "" + prefix: /no-parent + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 diff --git a/release-notes/current/bug_fixes/9953-securitypolicy-merge-jwt-provider.md b/release-notes/current/bug_fixes/9953-securitypolicy-merge-jwt-provider.md new file mode 100644 index 00000000000..3409cad8582 --- /dev/null +++ b/release-notes/current/bug_fixes/9953-securitypolicy-merge-jwt-provider.md @@ -0,0 +1 @@ +Fixed a route-level `SecurityPolicy` with `mergeType` set being rejected on admission when its `authorization` rules reference a JWT provider defined by the parent `SecurityPolicy` it merges into (`if authorization.rules.principal.jwt is used, jwt must be defined`). The CRD validation ran against the policy before the merge, so the JWT providers had to be duplicated on every route. The reference is now resolved against the merged policy, and an unknown provider is reported as an `Accepted: False` condition on the policy instead of silently producing an authorization rule that never matches. diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 2d2c7343a86..dd1c4fbccbf 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -3812,7 +3812,7 @@ _Appears in:_ | Field | Type | Required | Default | Description | | --- | --- | --- | --- | --- | -| `provider` | _string_ | true | | Provider is the name of the JWT provider that used to verify the JWT token.
In order to use JWT claims for authorization, you must configure the JWT
authentication with the same provider in the same `SecurityPolicy`. | +| `provider` | _string_ | true | | Provider is the name of the JWT provider that used to verify the JWT token.
In order to use JWT claims for authorization, you must configure the JWT
authentication with the same provider in the same `SecurityPolicy`, or,
when `mergeType` is set, in the parent `SecurityPolicy` this policy is
merged into. | | `claims` | _[JWTClaim](#jwtclaim) array_ | false | | Claims are the claims in a JWT token.
If multiple claims are specified, all claims must match for the rule to match.
For example, if there are two claims: one for the audience and one for the issuer,
the rule will match only if both the audience and the issuer match. | | `scopes` | _[JWTScope](#jwtscope) array_ | false | | Scopes are a special type of claim in a JWT token that represents the permissions of the client.
The value of the scopes field should be a space delimited string that is expected in the
scope (or scp) claim, as defined in RFC 6749: https://datatracker.ietf.org/doc/html/rfc6749#page-23.
If multiple scopes are specified, all scopes must match for the rule to match. | diff --git a/test/cel-validation/securitypolicy_test.go b/test/cel-validation/securitypolicy_test.go index 5f06cd33482..2cd8c705f85 100644 --- a/test/cel-validation/securitypolicy_test.go +++ b/test/cel-validation/securitypolicy_test.go @@ -1732,6 +1732,46 @@ func TestSecurityPolicyTarget(t *testing.T) { }, wantErrors: []string{"if authorization.rules.principal.jwt is used, jwt must be defined"}, }, + { + desc: "authorization-jwt-claims-without-jwt-authn-but-merging", + mutate: func(sp *egv1a1.SecurityPolicy) { + sp.Spec = egv1a1.SecurityPolicySpec{ + PolicyTargetReferences: egv1a1.PolicyTargetReferences{ + TargetSelectors: []egv1a1.TargetSelector{ + { + Group: new(gwapiv1.Group("gateway.networking.k8s.io")), + Kind: "HTTPRoute", + MatchLabels: map[string]string{ + "eg/namespace": "reference-apps", + }, + }, + }, + }, + // The jwt providers are inherited from the parent policy this one + // merges into, so they are not required here. + MergeType: new(egv1a1.StrategicMerge), + Authorization: &egv1a1.Authorization{ + Rules: []egv1a1.AuthorizationRule{ + { + Action: egv1a1.AuthorizationActionAllow, + Principal: &egv1a1.Principal{ + JWT: &egv1a1.JWTPrincipal{ + Provider: "example", + Claims: []egv1a1.JWTClaim{ + { + Name: "iss", + Values: []string{"https://example.com"}, + }, + }, + }, + }, + }, + }, + }, + } + }, + wantErrors: []string{}, + }, { desc: "authorization-jwt-empty-principal", mutate: func(sp *egv1a1.SecurityPolicy) { diff --git a/test/helm/gateway-crds-helm/all.out.yaml b/test/helm/gateway-crds-helm/all.out.yaml index 197aec2547f..6b90d9ab536 100644 --- a/test/helm/gateway-crds-helm/all.out.yaml +++ b/test/helm/gateway-crds-helm/all.out.yaml @@ -54439,7 +54439,9 @@ spec: description: |- Provider is the name of the JWT provider that used to verify the JWT token. In order to use JWT claims for authorization, you must configure the JWT - authentication with the same provider in the same `SecurityPolicy`. + authentication with the same provider in the same `SecurityPolicy`, or, + when `mergeType` is set, in the parent `SecurityPolicy` this policy is + merged into. maxLength: 253 minLength: 1 type: string @@ -62035,9 +62037,11 @@ spec: || self.targetRefs.all(ref, ref.kind in [''HTTPRoute'', ''GRPCRoute'', ''TCPRoute''])) && (!has(self.targetSelectors) || self.targetSelectors.all(sel, sel.kind in [''HTTPRoute'', ''GRPCRoute'', ''TCPRoute''])))' - - message: if authorization.rules.principal.jwt is used, jwt must be defined - rule: '(has(self.authorization) && has(self.authorization.rules) && - self.authorization.rules.exists(r, has(r.principal) ? has(r.principal.jwt) + - message: if authorization.rules.principal.jwt is used, jwt must be defined, + unless mergeType is set and the jwt providers are inherited from the + parent policy + rule: '(!has(self.mergeType) && has(self.authorization) && has(self.authorization.rules) + && self.authorization.rules.exists(r, has(r.principal) ? has(r.principal.jwt) : false)) ? has(self.jwt) : true' status: description: Status defines the current status of SecurityPolicy. diff --git a/test/helm/gateway-crds-helm/e2e.out.yaml b/test/helm/gateway-crds-helm/e2e.out.yaml index 32f54c37669..27d1747855d 100644 --- a/test/helm/gateway-crds-helm/e2e.out.yaml +++ b/test/helm/gateway-crds-helm/e2e.out.yaml @@ -30377,7 +30377,9 @@ spec: description: |- Provider is the name of the JWT provider that used to verify the JWT token. In order to use JWT claims for authorization, you must configure the JWT - authentication with the same provider in the same `SecurityPolicy`. + authentication with the same provider in the same `SecurityPolicy`, or, + when `mergeType` is set, in the parent `SecurityPolicy` this policy is + merged into. maxLength: 253 minLength: 1 type: string @@ -37973,9 +37975,11 @@ spec: || self.targetRefs.all(ref, ref.kind in [''HTTPRoute'', ''GRPCRoute'', ''TCPRoute''])) && (!has(self.targetSelectors) || self.targetSelectors.all(sel, sel.kind in [''HTTPRoute'', ''GRPCRoute'', ''TCPRoute''])))' - - message: if authorization.rules.principal.jwt is used, jwt must be defined - rule: '(has(self.authorization) && has(self.authorization.rules) && - self.authorization.rules.exists(r, has(r.principal) ? has(r.principal.jwt) + - message: if authorization.rules.principal.jwt is used, jwt must be defined, + unless mergeType is set and the jwt providers are inherited from the + parent policy + rule: '(!has(self.mergeType) && has(self.authorization) && has(self.authorization.rules) + && self.authorization.rules.exists(r, has(r.principal) ? has(r.principal.jwt) : false)) ? has(self.jwt) : true' status: description: Status defines the current status of SecurityPolicy. diff --git a/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml b/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml index db1da4ef9c1..e778854b65c 100644 --- a/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml +++ b/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml @@ -30377,7 +30377,9 @@ spec: description: |- Provider is the name of the JWT provider that used to verify the JWT token. In order to use JWT claims for authorization, you must configure the JWT - authentication with the same provider in the same `SecurityPolicy`. + authentication with the same provider in the same `SecurityPolicy`, or, + when `mergeType` is set, in the parent `SecurityPolicy` this policy is + merged into. maxLength: 253 minLength: 1 type: string @@ -37973,9 +37975,11 @@ spec: || self.targetRefs.all(ref, ref.kind in [''HTTPRoute'', ''GRPCRoute'', ''TCPRoute''])) && (!has(self.targetSelectors) || self.targetSelectors.all(sel, sel.kind in [''HTTPRoute'', ''GRPCRoute'', ''TCPRoute''])))' - - message: if authorization.rules.principal.jwt is used, jwt must be defined - rule: '(has(self.authorization) && has(self.authorization.rules) && - self.authorization.rules.exists(r, has(r.principal) ? has(r.principal.jwt) + - message: if authorization.rules.principal.jwt is used, jwt must be defined, + unless mergeType is set and the jwt providers are inherited from the + parent policy + rule: '(!has(self.mergeType) && has(self.authorization) && has(self.authorization.rules) + && self.authorization.rules.exists(r, has(r.principal) ? has(r.principal.jwt) : false)) ? has(self.jwt) : true' status: description: Status defines the current status of SecurityPolicy.