From 9bde314cd2f700ce09eb9df5c940385963d530f7 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Mon, 30 Oct 2023 17:45:50 +0800 Subject: [PATCH 1/8] add oidc to securitypolicy api Signed-off-by: huabing zhao --- api/v1alpha1/groupversion_info.go | 5 +- api/v1alpha1/oidc_types.go | 64 ++++ api/v1alpha1/securitypolicy_types.go | 5 + api/v1alpha1/zz_generated.deepcopy.go | 42 +++ ...ateway.envoyproxy.io_securitypolicies.yaml | 87 +++++ internal/gatewayapi/securitypolicy.go | 234 ++++++++++-- .../testdata/securitypolicy-with-cors.in.yaml | 2 +- .../securitypolicy-with-cors.out.yaml | 4 +- ...itypolicy-with-oidc-invalid-issuer.in.yaml | 40 ++ ...typolicy-with-oidc-invalid-issuer.out.yaml | 96 +++++ ...policy-with-oidc-invalid-secretref.in.yaml | 116 ++++++ ...olicy-with-oidc-invalid-secretref.out.yaml | 286 ++++++++++++++ .../testdata/securitypolicy-with-oidc.in.yaml | 162 ++++++++ .../securitypolicy-with-oidc.out.yaml | 356 ++++++++++++++++++ internal/gatewayapi/translator.go | 28 +- internal/gatewayapi/validate.go | 61 +++ internal/gatewayapi/validate_test.go | 38 ++ internal/ir/xds.go | 31 +- internal/ir/zz_generated.deepcopy.go | 27 ++ internal/provider/kubernetes/controller.go | 58 +++ internal/status/conditions.go | 31 ++ internal/status/conditions_test.go | 25 ++ internal/status/securitypolicy.go | 9 +- site/content/en/latest/api/extension_types.md | 35 ++ 24 files changed, 1783 insertions(+), 59 deletions(-) create mode 100644 api/v1alpha1/oidc_types.go create mode 100644 internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml create mode 100755 internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.out.yaml create mode 100644 internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml create mode 100755 internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml create mode 100644 internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml create mode 100755 internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml create mode 100644 internal/gatewayapi/validate_test.go diff --git a/api/v1alpha1/groupversion_info.go b/api/v1alpha1/groupversion_info.go index 1b13b602e87..be4c68a538d 100644 --- a/api/v1alpha1/groupversion_info.go +++ b/api/v1alpha1/groupversion_info.go @@ -10,9 +10,12 @@ import ( "sigs.k8s.io/controller-runtime/pkg/scheme" ) +const GroupName = "gateway.envoyproxy.io" + var ( + // GroupVersion is group version used to register these objects - GroupVersion = schema.GroupVersion{Group: "gateway.envoyproxy.io", Version: "v1alpha1"} + GroupVersion = schema.GroupVersion{Group: GroupName, Version: "v1alpha1"} // SchemeBuilder is used to add go types to the GroupVersionKind scheme SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} diff --git a/api/v1alpha1/oidc_types.go b/api/v1alpha1/oidc_types.go new file mode 100644 index 00000000000..749b56b4d73 --- /dev/null +++ b/api/v1alpha1/oidc_types.go @@ -0,0 +1,64 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +package v1alpha1 + +import ( + gwapiv1b1 "sigs.k8s.io/gateway-api/apis/v1beta1" +) + +const OIDCClientSecretKey = "client_secret" + +// OIDC defines the configuration for the OpenID Connect (OIDC) authentication. +type OIDC struct { + // The OIDC Provider configuration. + Provider OIDCProvider `json:"provider"` + + // The client ID assigned to this policy to be used in the OIDC + // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + // + // +kubebuilder:validation:MinLength=1 + ClientID string `json:"clientID"` + + // The Kubernetes secret which contains the OIDC client secret assigned to + // the filter to be used in the + // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + // + // This is an Opaque secret. The client secret should be stored in the key + // "client_secret". + // +kubebuilder:validation:Required + ClientSecret gwapiv1b1.SecretObjectReference `json:"clientSecret"` + + // The OIDC scopes to be used in the + // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + // The "openid" scope is always added to the list of scopes if not already + // specified. + // +optional + Scopes []string `json:"scopes,omitempty"` +} + +// OIDCProvider defines the OIDC Provider configuration. +type OIDCProvider struct { + // The OIDC Provider's [issuer identifier](https://openid.net/specs/openid-connect-discovery-1_0.html#IssuerDiscovery). + // Issuer MUST be a URI RFC 3986 [RFC3986] with a scheme component that MUST + // be https, a host component, and optionally, port and path components and + // no query or fragment components. + // +kubebuilder:validation:MinLength=1 + Issuer string `json:"issuer"` + + // TODO zhaohuabing validate the issuer + + // The OIDC Provider's [authorization endpoint](https://openid.net/specs/openid-connect-core-1_0.html#AuthorizationEndpoint). + // If not provided, EG will try to discover it from the provider's [Well-Known Configuration Endpoint](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationResponse). + // + // +optional + AuthorizationEndpoint string `json:"authorizationEndpoint,omitempty"` + + // The OIDC Provider's [token endpoint](https://openid.net/specs/openid-connect-core-1_0.html#TokenEndpoint). + // If not provided, EG will try to discover it from the provider's [Well-Known Configuration Endpoint](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationResponse). + // + // +optional + TokenEndpoint string `json:"tokenEndpoint,omitempty"` +} diff --git a/api/v1alpha1/securitypolicy_types.go b/api/v1alpha1/securitypolicy_types.go index e5c60882003..3d4efcebbc7 100644 --- a/api/v1alpha1/securitypolicy_types.go +++ b/api/v1alpha1/securitypolicy_types.go @@ -56,6 +56,11 @@ type SecurityPolicySpec struct { // // +optional JWT *JWT `json:"jwt,omitempty"` + + // OIDC defines the configuration for the OpenID Connect (OIDC) authentication. + // + // +optional + OIDC *OIDC `json:"oidc,omitempty"` } // SecurityPolicyStatus defines the state of SecurityPolicy diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 60ad334e9a5..eb884086ce8 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -1522,6 +1522,43 @@ func (in *LoadBalancer) DeepCopy() *LoadBalancer { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OIDC) DeepCopyInto(out *OIDC) { + *out = *in + out.Provider = in.Provider + in.ClientSecret.DeepCopyInto(&out.ClientSecret) + if in.Scopes != nil { + in, out := &in.Scopes, &out.Scopes + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OIDC. +func (in *OIDC) DeepCopy() *OIDC { + if in == nil { + return nil + } + out := new(OIDC) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OIDCProvider) DeepCopyInto(out *OIDCProvider) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OIDCProvider. +func (in *OIDCProvider) DeepCopy() *OIDCProvider { + if in == nil { + return nil + } + out := new(OIDCProvider) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OpenTelemetryEnvoyProxyAccessLog) DeepCopyInto(out *OpenTelemetryEnvoyProxyAccessLog) { *out = *in @@ -2099,6 +2136,11 @@ func (in *SecurityPolicySpec) DeepCopyInto(out *SecurityPolicySpec) { *out = new(JWT) (*in).DeepCopyInto(*out) } + if in.OIDC != nil { + in, out := &in.OIDC, &out.OIDC + *out = new(OIDC) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecurityPolicySpec. diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml index 7fefb8b9d77..e0fabfc1cf4 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -186,6 +186,93 @@ spec: required: - providers type: object + oidc: + description: OIDC defines the configuration for the OpenID Connect + (OIDC) authentication. + properties: + clientID: + description: The client ID assigned to this policy to be used + in the OIDC [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + minLength: 1 + type: string + clientSecret: + description: "The Kubernetes secret which contains the OIDC client + secret assigned to the filter to be used in the [Authentication + Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + \n This is an Opaque secret. The client secret should be stored + in the key \"client_secret\"." + properties: + group: + default: "" + description: Group is the group of the referent. For example, + "gateway.networking.k8s.io". When unspecified or empty string, + core API group is inferred. + maxLength: 253 + pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + kind: + default: Secret + description: Kind is kind of the referent. For example "Secret". + maxLength: 63 + minLength: 1 + pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$ + type: string + name: + description: Name is the name of the referent. + maxLength: 253 + minLength: 1 + type: string + namespace: + description: "Namespace is the namespace of the referenced + object. When unspecified, the local namespace is inferred. + \n Note that when a namespace different than the local namespace + is specified, a ReferenceGrant object is required in the + referent namespace to allow that namespace's owner to accept + the reference. See the ReferenceGrant documentation for + details. \n Support: Core" + maxLength: 63 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ + type: string + required: + - name + type: object + provider: + description: The OIDC Provider configuration. + properties: + authorizationEndpoint: + description: The OIDC Provider's [authorization endpoint](https://openid.net/specs/openid-connect-core-1_0.html#AuthorizationEndpoint). + If not provided, EG will try to discover it from the provider's + [Well-Known Configuration Endpoint](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationResponse). + type: string + issuer: + description: The OIDC Provider's [issuer identifier](https://openid.net/specs/openid-connect-discovery-1_0.html#IssuerDiscovery). + Issuer MUST be a URI RFC 3986 [RFC3986] with a scheme component + that MUST be https, a host component, and optionally, port + and path components and no query or fragment components. + minLength: 1 + type: string + tokenEndpoint: + description: The OIDC Provider's [token endpoint](https://openid.net/specs/openid-connect-core-1_0.html#TokenEndpoint). + If not provided, EG will try to discover it from the provider's + [Well-Known Configuration Endpoint](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationResponse). + type: string + required: + - issuer + type: object + scopes: + description: The OIDC scopes to be used in the [Authentication + Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + The "openid" scope is always added to the list of scopes if + not already specified. + items: + type: string + type: array + required: + - clientID + - clientSecret + - provider + type: object targetRef: description: TargetRef is the name of the Gateway resource this policy is being attached to. This Policy and the TargetRef MUST be in the diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index 801f2837078..8094fd0f224 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -6,10 +6,13 @@ package gatewayapi import ( + "encoding/json" "fmt" + "net/http" "sort" "strings" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" gwv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -24,6 +27,7 @@ import ( func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.SecurityPolicy, gateways []*GatewayContext, routes []RouteContext, + resources *Resources, xdsIR XdsIRMap) []*egv1a1.SecurityPolicy { var res []*egv1a1.SecurityPolicy @@ -68,13 +72,20 @@ func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.Security continue } - t.translateSecurityPolicyForRoute(policy, route, xdsIR) - - message := "SecurityPolicy has been accepted." - status.SetSecurityPolicyAcceptedIfUnset(&policy.Status, message) + err := t.translateSecurityPolicyForRoute(policy, route, resources, xdsIR) + if err != nil { + status.SetSecurityPolicyCondition(policy, + gwv1a2.PolicyConditionAccepted, + metav1.ConditionFalse, + gwv1a2.PolicyReasonInvalid, + status.Error2ConditionMsg(err), + ) + } else { + message := "SecurityPolicy has been accepted." + status.SetSecurityPolicyAccepted(&policy.Status, message) + } } } - // Process the policies targeting Gateways for _, policy := range securityPolicies { if policy.Spec.TargetRef.Kind == KindGateway { @@ -87,17 +98,27 @@ func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.Security continue } - t.translateSecurityPolicyForGateway(policy, gateway, xdsIR) - - message := "SecurityPolicy has been accepted." - status.SetSecurityPolicyAcceptedIfUnset(&policy.Status, message) + err := t.translateSecurityPolicyForGateway(policy, gateway, resources, xdsIR) + if err != nil { + status.SetSecurityPolicyCondition(policy, + gwv1a2.PolicyConditionAccepted, + metav1.ConditionFalse, + gwv1a2.PolicyReasonInvalid, + status.Error2ConditionMsg(err), + ) + } else { + message := "SecurityPolicy has been accepted." + status.SetSecurityPolicyAccepted(&policy.Status, message) + } } } return res } -func resolveSecurityPolicyGatewayTargetRef(policy *egv1a1.SecurityPolicy, gateways map[types.NamespacedName]*policyGatewayTargetContext) *GatewayContext { +func resolveSecurityPolicyGatewayTargetRef( + policy *egv1a1.SecurityPolicy, + gateways map[types.NamespacedName]*policyGatewayTargetContext) *GatewayContext { targetNs := policy.Spec.TargetRef.Namespace // If empty, default to namespace of policy if targetNs == nil { @@ -107,7 +128,8 @@ func resolveSecurityPolicyGatewayTargetRef(policy *egv1a1.SecurityPolicy, gatewa // Ensure Policy and target are in the same namespace if policy.Namespace != string(*targetNs) { - message := fmt.Sprintf("Namespace:%s TargetRef.Namespace:%s, SecurityPolicy can only target a resource in the same namespace.", + message := fmt.Sprintf( + "Namespace:%s TargetRef.Namespace:%s, SecurityPolicy can only target a resource in the same namespace.", policy.Namespace, *targetNs) status.SetSecurityPolicyCondition(policy, gwv1a2.PolicyConditionAccepted, @@ -158,7 +180,9 @@ func resolveSecurityPolicyGatewayTargetRef(policy *egv1a1.SecurityPolicy, gatewa return gateway.GatewayContext } -func resolveSecurityPolicyRouteTargetRef(policy *egv1a1.SecurityPolicy, routes map[policyTargetRouteKey]*policyRouteTargetContext) RouteContext { +func resolveSecurityPolicyRouteTargetRef( + policy *egv1a1.SecurityPolicy, + routes map[policyTargetRouteKey]*policyRouteTargetContext) RouteContext { targetNs := policy.Spec.TargetRef.Namespace // If empty, default to namespace of policy if targetNs == nil { @@ -168,7 +192,8 @@ func resolveSecurityPolicyRouteTargetRef(policy *egv1a1.SecurityPolicy, routes m // Ensure Policy and target are in the same namespace if policy.Namespace != string(*targetNs) { - message := fmt.Sprintf("Namespace:%s TargetRef.Namespace:%s, SecurityPolicy can only target a resource in the same namespace.", + message := fmt.Sprintf( + "Namespace:%s TargetRef.Namespace:%s, SecurityPolicy can only target a resource in the same namespace.", policy.Namespace, *targetNs) status.SetSecurityPolicyCondition(policy, gwv1a2.PolicyConditionAccepted, @@ -189,7 +214,10 @@ func resolveSecurityPolicyRouteTargetRef(policy *egv1a1.SecurityPolicy, routes m // Route not found if !ok { - message := fmt.Sprintf("%s/%s/%s not found.", policy.Spec.TargetRef.Kind, string(*targetNs), policy.Spec.TargetRef.Name) + message := fmt.Sprintf( + "%s/%s/%s not found.", + policy.Spec.TargetRef.Kind, + string(*targetNs), policy.Spec.TargetRef.Name) status.SetSecurityPolicyCondition(policy, gwv1a2.PolicyConditionAccepted, @@ -202,7 +230,9 @@ func resolveSecurityPolicyRouteTargetRef(policy *egv1a1.SecurityPolicy, routes m // Check if another policy targeting the same xRoute exists if route.attached { - message := fmt.Sprintf("Unable to target %s, another SecurityPolicy has already attached to it", string(policy.Spec.TargetRef.Kind)) + message := fmt.Sprintf( + "Unable to target %s, another SecurityPolicy has already attached to it", + string(policy.Spec.TargetRef.Kind)) status.SetSecurityPolicyCondition(policy, gwv1a2.PolicyConditionAccepted, @@ -220,22 +250,39 @@ func resolveSecurityPolicyRouteTargetRef(policy *egv1a1.SecurityPolicy, routes m return route.RouteContext } -func (t *Translator) translateSecurityPolicyForRoute(policy *egv1a1.SecurityPolicy, route RouteContext, xdsIR XdsIRMap) { +func (t *Translator) translateSecurityPolicyForRoute( + policy *egv1a1.SecurityPolicy, route RouteContext, + resources *Resources, xdsIR XdsIRMap) error { // Build IR var ( cors *ir.CORS jwt *ir.JWT + oidc *ir.OIDC + err error ) if policy.Spec.CORS != nil { - cors = t.buildCORS(policy) + cors, err = t.buildCORS(policy.Spec.CORS) + if err != nil { + return err + } } if policy.Spec.JWT != nil { - jwt = t.buildJWT(policy) + jwt = t.buildJWT(policy.Spec.JWT) + } + + if policy.Spec.OIDC != nil { + oidc, err = t.buildOIDC(policy, resources) + if err != nil { + return err + } } // Apply IR to all relevant routes + // It can be difficult to reason about the state of the system if we apply + // part of the policy and not the rest. Therefore, we either apply all of it + // or none of it (when get errors when translating the policy) prefix := irRoutePrefix(route) for _, ir := range xdsIR { for _, http := range ir.HTTP { @@ -246,31 +293,50 @@ func (t *Translator) translateSecurityPolicyForRoute(policy *egv1a1.SecurityPoli if strings.HasPrefix(r.Name, prefix) { r.CORS = cors r.JWT = jwt + r.OIDC = oidc } } } - } + return nil } -func (t *Translator) translateSecurityPolicyForGateway(policy *egv1a1.SecurityPolicy, gateway *GatewayContext, xdsIR XdsIRMap) { +func (t *Translator) translateSecurityPolicyForGateway( + policy *egv1a1.SecurityPolicy, gateway *GatewayContext, + resources *Resources, xdsIR XdsIRMap) error { // Build IR var ( cors *ir.CORS jwt *ir.JWT + oidc *ir.OIDC + err error ) if policy.Spec.CORS != nil { - cors = t.buildCORS(policy) + cors, err = t.buildCORS(policy.Spec.CORS) + if err != nil { + return err + } } if policy.Spec.JWT != nil { - jwt = t.buildJWT(policy) + jwt = t.buildJWT(policy.Spec.JWT) + } + + if policy.Spec.OIDC != nil { + oidc, err = t.buildOIDC(policy, resources) + if err != nil { + return err + } } // Apply IR to all the routes within the specific Gateway // If the feature is already set, then skip it, since it must have be // set by a policy attaching to the route + // + // It can be difficult to reason about the state of the system if we apply + // part of the policy and not the rest. Therefore, we either apply all of it + // or none of it (when get errors when translating the policy) irKey := t.getIRKey(gateway.Gateway) // Should exist since we've validated this ir := xdsIR[irKey] @@ -284,15 +350,18 @@ func (t *Translator) translateSecurityPolicyForGateway(policy *egv1a1.SecurityPo if r.JWT == nil { r.JWT = jwt } + if r.OIDC == nil { + r.OIDC = oidc + } } } - + return nil } -func (t *Translator) buildCORS(policy *egv1a1.SecurityPolicy) *ir.CORS { +func (t *Translator) buildCORS(cors *egv1a1.CORS) (*ir.CORS, error) { var allowOrigins []*ir.StringMatch - for _, origin := range policy.Spec.CORS.AllowOrigins { + for _, origin := range cors.AllowOrigins { origin := origin.DeepCopy() // matchType default to exact @@ -316,23 +385,122 @@ func (t *Translator) buildCORS(policy *egv1a1.SecurityPolicy) *ir.CORS { Suffix: &origin.Value, }) case egv1a1.StringMatchRegularExpression: + if err := validateRegex(origin.Value); err != nil { + return nil, err // TODO zhaohuabing: also check regex in other places + } allowOrigins = append(allowOrigins, &ir.StringMatch{ - SafeRegex: &origin.Value, // TODO zhaohuabing: check if the value is a valid regex + SafeRegex: &origin.Value, }) } } return &ir.CORS{ AllowOrigins: allowOrigins, - AllowMethods: policy.Spec.CORS.AllowMethods, - AllowHeaders: policy.Spec.CORS.AllowHeaders, - ExposeHeaders: policy.Spec.CORS.ExposeHeaders, - MaxAge: policy.Spec.CORS.MaxAge, - } + AllowMethods: cors.AllowMethods, + AllowHeaders: cors.AllowHeaders, + ExposeHeaders: cors.ExposeHeaders, + MaxAge: cors.MaxAge, + }, nil } -func (t *Translator) buildJWT(policy *egv1a1.SecurityPolicy) *ir.JWT { +func (t *Translator) buildJWT(jwt *egv1a1.JWT) *ir.JWT { return &ir.JWT{ - Providers: policy.Spec.JWT.Providers, + Providers: jwt.Providers, + } +} + +func (t *Translator) buildOIDC( + policy *egv1a1.SecurityPolicy, + resources *Resources) (*ir.OIDC, error) { + var ( + oidc = policy.Spec.OIDC + clientSecret *v1.Secret + err error + ) + + from := crossNamespaceFrom{ + group: egv1a1.GroupName, + kind: KindSecurityPolicy, + namespace: policy.Namespace, } + if clientSecret, err = t.validateSecretRef(from, oidc.ClientSecret, resources); err != nil { + return nil, err + } + + val, ok := clientSecret.Data[egv1a1.OIDCClientSecretKey] + if !ok || len(val) == 0 { + return nil, fmt.Errorf("client secret not found in secret %s/%s", clientSecret.Namespace, clientSecret.Name) + } + + // Discover the token and authorization endpoints from the issuer's + // well-known url if not explicitly specified + provider := oidc.Provider.DeepCopy() + if err := discoverEndpointsFromIssuer(provider); err != nil { + return nil, err + } + + scopes := appendOpenidScopeIfNotExist(oidc.Scopes) + + return &ir.OIDC{ + Provider: *provider, + ClientID: oidc.ClientID, + ClientSecret: oidc.ClientSecret, + Scopes: scopes, + }, nil +} + +// appendOpenidScopeIfNotExist appends the openid scope to the provided scopes +// if it is not already present. +// `openid` is a required scope for OIDC. +// see https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims +func appendOpenidScopeIfNotExist(scopes []string) []string { + const authScopeOpenID = "openid" + + hasOpenIDScope := false + for _, scope := range scopes { + if scope == authScopeOpenID { + hasOpenIDScope = true + } + } + if !hasOpenIDScope { + scopes = append(scopes, authScopeOpenID) + } + return scopes +} + +type OpenIDConfig struct { + TokenEndpoint string `json:"token_endpoint"` + AuthorizationEndpoint string `json:"authorization_endpoint"` +} + +// discoverEndpointsFromIssuer discovers the token and authorization endpoints from the issuer's well-known url +// return error if failed to fetch the well-known configuration +func discoverEndpointsFromIssuer(provider *egv1a1.OIDCProvider) error { + if provider.TokenEndpoint == "" || provider.AuthorizationEndpoint == "" { + tokenEndpoint, authorizationEndpoint, err := fetchEndpointsFromIssuer(provider.Issuer) + if err != nil { + return fmt.Errorf("error fetching endpoints from issuer: %w", err) + } + provider.TokenEndpoint = tokenEndpoint + provider.AuthorizationEndpoint = authorizationEndpoint + } + return nil +} + +func fetchEndpointsFromIssuer(issuerURL string) (string, string, error) { + // Fetch the OpenID configuration from the issuer URL + resp, err := http.Get(fmt.Sprintf("%s/.well-known/openid-configuration", issuerURL)) + if err != nil { + return "", "", err + } + defer resp.Body.Close() + + // Parse the OpenID configuration response + var config OpenIDConfig + err = json.NewDecoder(resp.Body).Decode(&config) + if err != nil { + return "", "", err + } + + return config.TokenEndpoint, config.AuthorizationEndpoint, nil } diff --git a/internal/gatewayapi/testdata/securitypolicy-with-cors.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-cors.in.yaml index fd6c99debd1..3cc8004e602 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-cors.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-cors.in.yaml @@ -77,7 +77,7 @@ securityPolicies: cors: allowOrigins: - type: RegularExpression - value: "*.example.com" + value: "FooBar[0-9]+" - type: Exact value: foo.bar.com allowMethods: diff --git a/internal/gatewayapi/testdata/securitypolicy-with-cors.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-cors.out.yaml index 9e2fc0e1fb6..153f26be923 100755 --- a/internal/gatewayapi/testdata/securitypolicy-with-cors.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-cors.out.yaml @@ -233,7 +233,7 @@ securityPolicies: - POST allowOrigins: - type: RegularExpression - value: '*.example.com' + value: FooBar[0-9]+ - type: Exact value: foo.bar.com exposeHeaders: @@ -278,7 +278,7 @@ xdsIR: allowOrigins: - distinct: false name: "" - safeRegex: '*.example.com' + safeRegex: FooBar[0-9]+ - distinct: false exact: foo.bar.com name: "" diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml new file mode 100644 index 00000000000..75802df896d --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml @@ -0,0 +1,40 @@ +secrets: +- apiVersion: v1 + kind: Secret + metadata: + namespace: default + name: client1-secret + data: + client_secret: Y2xpZW50MTpzZWNyZXQK +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: default + name: gateway-1 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: All +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: default + name: policy-non-exist-secretRef + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + oidc: + provider: + issuer: "https://httpbin.org/" + clientID: "client1.apps.foo.bar.com" + clientSecret: + name: "client1-secret" diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.out.yaml new file mode 100755 index 00000000000..8e21d7060ec --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.out.yaml @@ -0,0 +1,96 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-1 + namespace: default + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 0 + 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 +infraIR: + default/gateway-1: + proxy: + listeners: + - address: "" + ports: + - containerPort: 10080 + name: http + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: default + name: default/gateway-1 +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + creationTimestamp: null + name: policy-non-exist-secretRef + namespace: default + spec: + oidc: + clientID: client1.apps.foo.bar.com + clientSecret: + group: null + kind: null + name: client1-secret + provider: + issuer: https://httpbin.org/ + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + status: + conditions: + - lastTransitionTime: null + message: 'Error fetching endpoints from issuer: invalid character ''<'' looking + for beginning of value.' + reason: Invalid + status: "False" + type: Accepted +xdsIR: + default/gateway-1: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: default/gateway-1/http + port: 10080 diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml new file mode 100644 index 00000000000..34c15f88a51 --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml @@ -0,0 +1,116 @@ +secrets: +- apiVersion: v1 + kind: Secret + metadata: + namespace: envoy-gateway + name: client2-secret + data: + client_secret: Y2xpZW50MTpzZWNyZXQK +- apiVersion: v1 + kind: Secret + metadata: + namespace: default + name: client3-secret + data: + invalid_client_secret_key: Y2xpZW50MTpzZWNyZXQK + +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: default + name: gateway-1 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: All +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: default + name: gateway-2 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: All +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: default + name: gateway-3 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: All +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: default + name: policy-non-exist-secretRef + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + oidc: + provider: + issuer: "https://accounts.google.com" + authorizationEndpoint: "https://accounts.google.com/o/oauth2/v2/auth" + tokenEndpoint: "https://oauth2.googleapis.com/token" + clientID: "client1.apps.googleusercontent.com" + clientSecret: + name: "client1-secret" +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: default + name: policy-no-referenceGrant + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + oidc: + provider: + issuer: "https://accounts.google.com" + authorizationEndpoint: "https://accounts.google.com/o/oauth2/v2/auth" + tokenEndpoint: "https://oauth2.googleapis.com/token" + clientID: "client1.apps.googleusercontent.com" + clientSecret: + namespace: envoy-gateway + name: "client2-secret" +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: default + name: policy-no-client-secret-key + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-3 + oidc: + provider: + issuer: "https://accounts.google.com" + authorizationEndpoint: "https://accounts.google.com/o/oauth2/v2/auth" + tokenEndpoint: "https://oauth2.googleapis.com/token" + clientID: "client1.apps.googleusercontent.com" + clientSecret: + namespace: default + name: "client3-secret" diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml new file mode 100755 index 00000000000..f57844492bc --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml @@ -0,0 +1,286 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-1 + namespace: default + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 0 + 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 +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-2 + namespace: default + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 0 + 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 +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-3 + namespace: default + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 0 + 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 +infraIR: + default/gateway-1: + proxy: + listeners: + - address: "" + ports: + - containerPort: 10080 + name: http + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: default + name: default/gateway-1 + default/gateway-2: + proxy: + listeners: + - address: "" + ports: + - containerPort: 10080 + name: http + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-2 + gateway.envoyproxy.io/owning-gateway-namespace: default + name: default/gateway-2 + default/gateway-3: + proxy: + listeners: + - address: "" + ports: + - containerPort: 10080 + name: http + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-3 + gateway.envoyproxy.io/owning-gateway-namespace: default + name: default/gateway-3 +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + creationTimestamp: null + name: policy-non-exist-secretRef + namespace: default + spec: + oidc: + clientID: client1.apps.googleusercontent.com + clientSecret: + group: null + kind: null + name: client1-secret + provider: + authorizationEndpoint: https://accounts.google.com/o/oauth2/v2/auth + issuer: https://accounts.google.com + tokenEndpoint: https://oauth2.googleapis.com/token + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + status: + conditions: + - lastTransitionTime: null + message: Secret default/client1-secret does not exist. + reason: Invalid + status: "False" + type: Accepted +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + creationTimestamp: null + name: policy-no-referenceGrant + namespace: default + spec: + oidc: + clientID: client1.apps.googleusercontent.com + clientSecret: + group: null + kind: null + name: client2-secret + namespace: envoy-gateway + provider: + authorizationEndpoint: https://accounts.google.com/o/oauth2/v2/auth + issuer: https://accounts.google.com + tokenEndpoint: https://oauth2.googleapis.com/token + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + status: + conditions: + - lastTransitionTime: null + message: Certificate ref to secret envoy-gateway/client2-secret not permitted + by any ReferenceGrant. + reason: Invalid + status: "False" + type: Accepted +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + creationTimestamp: null + name: policy-no-client-secret-key + namespace: default + spec: + oidc: + clientID: client1.apps.googleusercontent.com + clientSecret: + group: null + kind: null + name: client3-secret + namespace: default + provider: + authorizationEndpoint: https://accounts.google.com/o/oauth2/v2/auth + issuer: https://accounts.google.com + tokenEndpoint: https://oauth2.googleapis.com/token + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-3 + status: + conditions: + - lastTransitionTime: null + message: Client secret not found in secret default/client3-secret. + reason: Invalid + status: "False" + type: Accepted +xdsIR: + default/gateway-1: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: default/gateway-1/http + port: 10080 + default/gateway-2: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: default/gateway-2/http + port: 10080 + default/gateway-3: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: default/gateway-3/http + port: 10080 diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml new file mode 100644 index 00000000000..91272ac2e97 --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml @@ -0,0 +1,162 @@ +secrets: +- apiVersion: v1 + kind: Secret + metadata: + namespace: envoy-gateway + name: client1-secret + data: + client_secret: Y2xpZW50MTpzZWNyZXQK +- apiVersion: v1 + kind: Secret + metadata: + namespace: default + name: client2-secret + data: + client_secret: Y2xpZW50MTpzZWNyZXQK +- apiVersion: v1 + kind: Secret + metadata: + namespace: envoy-gateway + name: client3-secret + data: + client_secret: Y2xpZW50MTpzZWNyZXQK +referenceGrants: +- apiVersion: gateway.networking.k8s.io/v1alpha2 + kind: ReferenceGrant + metadata: + namespace: envoy-gateway + name: referencegrant-1 + spec: + from: + - group: gateway.envoyproxy.io + kind: SecurityPolicy + namespace: default + to: + - group: "" + kind: Secret +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 +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-1 + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/foo" + backendRefs: + - name: service-1 + port: 8080 +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-2 + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/bar" + backendRefs: + - name: service-1 + port: 8080 +grpcRoutes: +- apiVersion: gateway.networking.k8s.io/v1alpha2 + kind: GRPCRoute + metadata: + namespace: default + name: grpcroute-1 + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: envoy-gateway + name: policy-for-gateway-discover-endpoints # This policy should attach httproute-2 + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + oidc: + provider: + issuer: "https://accounts.google.com" + clientID: "client1.apps.googleusercontent.com" + clientSecret: + name: "client1-secret" +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: default + name: policy-for-http-route # This policy should attach httproute-1 + spec: + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-1 + namespace: default + oidc: + provider: + issuer: "https://oauth.foo.com" + authorizationEndpoint: "https://oauth.foo.com/oauth2/v2/auth" + tokenEndpoint: "https://oauth.foo.com/token" + clientID: "client2.oauth.foo.com" + clientSecret: + name: "client2-secret" + scopes: ["openid", "email", "profile"] +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: default + name: policy-cross-namespace-secretRef # This policy should attach grpcroute-1 + spec: + targetRef: + group: gateway.networking.k8s.io + kind: GRPCRoute + name: grpcroute-1 + oidc: + provider: + issuer: "https://oauth.bar.com" + authorizationEndpoint: "https://oauth.bar.com/oauth2/v2/auth" + tokenEndpoint: "https://oauth.bar.com/token" + clientID: "client3.bar.foo.com" + clientSecret: + namespace: envoy-gateway + name: "client3-secret" diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml new file mode 100755 index 00000000000..302fe418d7b --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml @@ -0,0 +1,356 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 3 + 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 +grpcRoutes: +- apiVersion: gateway.networking.k8s.io/v1alpha2 + kind: GRPCRoute + metadata: + creationTimestamp: null + name: grpcroute-1 + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + 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 +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-1 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: /foo + 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: + creationTimestamp: null + name: httproute-2 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: /bar + 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 +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - address: "" + ports: + - containerPort: 10080 + name: http + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + name: envoy-gateway/gateway-1 +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + creationTimestamp: null + name: policy-for-http-route + namespace: default + spec: + oidc: + clientID: client2.oauth.foo.com + clientSecret: + group: null + kind: null + name: client2-secret + provider: + authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth + issuer: https://oauth.foo.com + tokenEndpoint: https://oauth.foo.com/token + scopes: + - openid + - email + - profile + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-1 + namespace: default + status: + conditions: + - lastTransitionTime: null + message: SecurityPolicy has been accepted. + reason: Accepted + status: "True" + type: Accepted +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + creationTimestamp: null + name: policy-cross-namespace-secretRef + namespace: default + spec: + oidc: + clientID: client3.bar.foo.com + clientSecret: + group: null + kind: null + name: client3-secret + namespace: envoy-gateway + provider: + authorizationEndpoint: https://oauth.bar.com/oauth2/v2/auth + issuer: https://oauth.bar.com + tokenEndpoint: https://oauth.bar.com/token + targetRef: + group: gateway.networking.k8s.io + kind: GRPCRoute + name: grpcroute-1 + status: + conditions: + - lastTransitionTime: null + message: SecurityPolicy has been accepted. + reason: Accepted + status: "True" + type: Accepted +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + creationTimestamp: null + name: policy-for-gateway-discover-endpoints + namespace: envoy-gateway + spec: + oidc: + clientID: client1.apps.googleusercontent.com + clientSecret: + group: null + kind: null + name: client1-secret + provider: + issuer: https://accounts.google.com + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + status: + conditions: + - lastTransitionTime: null + message: SecurityPolicy has been accepted. + reason: Accepted + status: "True" + type: Accepted +xdsIR: + envoy-gateway/gateway-1: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: true + name: envoy-gateway/gateway-1/http + port: 10080 + routes: + - backendWeights: + invalid: 0 + valid: 0 + destination: + name: httproute/default/httproute-1/rule/0 + settings: + - endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: gateway.envoyproxy.io + name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io + oidc: + clientID: client2.oauth.foo.com + clientSecret: + group: null + kind: null + name: client2-secret + provider: + authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth + issuer: https://oauth.foo.com + tokenEndpoint: https://oauth.foo.com/token + scopes: + - openid + - email + - profile + pathMatch: + distinct: false + name: "" + prefix: /foo + - backendWeights: + invalid: 0 + valid: 0 + destination: + name: httproute/default/httproute-2/rule/0 + settings: + - endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: gateway.envoyproxy.io + name: httproute/default/httproute-2/rule/0/match/0/gateway_envoyproxy_io + oidc: + clientID: client1.apps.googleusercontent.com + clientSecret: + group: null + kind: null + name: client1-secret + provider: + authorizationEndpoint: https://accounts.google.com/o/oauth2/v2/auth + issuer: https://accounts.google.com + tokenEndpoint: https://oauth2.googleapis.com/token + scopes: + - openid + pathMatch: + distinct: false + name: "" + prefix: /bar + - backendWeights: + invalid: 0 + valid: 0 + destination: + name: grpcroute/default/grpcroute-1/rule/0 + settings: + - endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: GRPC + weight: 1 + hostname: '*' + name: grpcroute/default/grpcroute-1/rule/0/match/-1/* + oidc: + clientID: client3.bar.foo.com + clientSecret: + group: null + kind: null + name: client3-secret + namespace: envoy-gateway + provider: + authorizationEndpoint: https://oauth.bar.com/oauth2/v2/auth + issuer: https://oauth.bar.com + tokenEndpoint: https://oauth.bar.com/token + scopes: + - openid diff --git a/internal/gatewayapi/translator.go b/internal/gatewayapi/translator.go index f5d6b54bc22..198b30680db 100644 --- a/internal/gatewayapi/translator.go +++ b/internal/gatewayapi/translator.go @@ -14,18 +14,19 @@ import ( ) const ( - KindEnvoyProxy = "EnvoyProxy" - KindGateway = "Gateway" - KindGatewayClass = "GatewayClass" - KindGRPCRoute = "GRPCRoute" - KindHTTPRoute = "HTTPRoute" - KindNamespace = "Namespace" - KindTLSRoute = "TLSRoute" - KindTCPRoute = "TCPRoute" - KindUDPRoute = "UDPRoute" - KindService = "Service" - KindServiceImport = "ServiceImport" - KindSecret = "Secret" + KindEnvoyProxy = "EnvoyProxy" + KindGateway = "Gateway" + KindGatewayClass = "GatewayClass" + KindGRPCRoute = "GRPCRoute" + KindHTTPRoute = "HTTPRoute" + KindNamespace = "Namespace" + KindTLSRoute = "TLSRoute" + KindTCPRoute = "TCPRoute" + KindUDPRoute = "UDPRoute" + KindService = "Service" + KindServiceImport = "ServiceImport" + KindSecret = "Secret" + KindSecurityPolicy = "SecurityPolicy" GroupMultiClusterService = "multicluster.x-k8s.io" // OwningGatewayNamespaceLabel is the owner reference label used for managed infra. @@ -187,9 +188,10 @@ func (t *Translator) Translate(resources *Resources) *TranslateResult { // Process BackendTrafficPolicies backendTrafficPolicies := t.ProcessBackendTrafficPolicies( resources.BackendTrafficPolicies, gateways, routes, xdsIR) + // Process SecurityPolicies securityPolicies := t.ProcessSecurityPolicies( - resources.SecurityPolicies, gateways, routes, xdsIR) + resources.SecurityPolicies, gateways, routes, resources, xdsIR) // Sort xdsIR based on the Gateway API spec sortXdsIRMap(xdsIR) diff --git a/internal/gatewayapi/validate.go b/internal/gatewayapi/validate.go index 1ada34033a0..b8e0c34925d 100644 --- a/internal/gatewayapi/validate.go +++ b/internal/gatewayapi/validate.go @@ -6,8 +6,10 @@ package gatewayapi import ( + "errors" "fmt" "net/netip" + "regexp" "strings" v1 "k8s.io/api/core/v1" @@ -283,6 +285,7 @@ func (t *Translator) validateTerminateModeAndGetTLSSecrets(listener *ListenerCon secrets := make([]*v1.Secret, 0) for _, certificateRef := range listener.TLS.CertificateRefs { + // TODO zhaohuabing: reuse validateSecretRef if certificateRef.Group != nil && string(*certificateRef.Group) != "" { listener.SetCondition( gwapiv1.ListenerConditionResolvedRefs, @@ -722,3 +725,61 @@ func (t *Translator) validateHostname(hostname string) error { return nil } + +// validateSecretRef checks three things: +// 1. Dose the secret reference has valid Group and kind +// 2. If the secret reference is a cross namespace reference, is it permitted by +// any ReferenceGrant +// 3. Does the secret exist +func (t *Translator) validateSecretRef( + from crossNamespaceFrom, + secretRef gwapiv1b1.SecretObjectReference, + resources *Resources) (*v1.Secret, error) { + if secretRef.Group != nil && string(*secretRef.Group) != "" { + return nil, errors.New("secret ref group must be unspecified/empty") + } + + if secretRef.Kind != nil && string(*secretRef.Kind) != KindSecret { + return nil, fmt.Errorf("secret ref kind must be %s", KindSecret) + } + + secretNamespace := from.namespace + + if secretRef.Namespace != nil && + string(*secretRef.Namespace) != "" && + string(*secretRef.Namespace) != from.namespace { + if !t.validateCrossNamespaceRef( + from, + crossNamespaceTo{ + group: "", + kind: KindSecret, + namespace: string(*secretRef.Namespace), + name: string(secretRef.Name), + }, + resources.ReferenceGrants, + ) { + return nil, + fmt.Errorf( + "certificate ref to secret %s/%s not permitted by any ReferenceGrant", + *secretRef.Namespace, secretRef.Name) + } + + secretNamespace = string(*secretRef.Namespace) + } + + secret := resources.GetSecret(secretNamespace, string(secretRef.Name)) + + if secret == nil { + return nil, fmt.Errorf( + "secret %s/%s does not exist", secretNamespace, secretRef.Name) + } + + return secret, nil +} + +func validateRegex(regex string) error { + if _, err := regexp.Compile(regex); err != nil { + return fmt.Errorf("regex %q is invalid: %v", regex, err) + } + return nil +} diff --git a/internal/gatewayapi/validate_test.go b/internal/gatewayapi/validate_test.go new file mode 100644 index 00000000000..86dd9317025 --- /dev/null +++ b/internal/gatewayapi/validate_test.go @@ -0,0 +1,38 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +package gatewayapi + +import ( + "testing" +) + +func Test_validateRegex(t *testing.T) { + tests := []struct { + name string + regex string + wantErr bool + }{ + { + name: "valid regex", + regex: "^[a-zA-Z0-9-]+$", + wantErr: false, + }, + { + name: "invalid regex", + regex: "*.foo.com", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateRegex(tt.regex) + if (err != nil) != tt.wantErr { + t.Errorf("validateRegex() error = %v, wantErr %v", err, tt.wantErr) + return + } + }) + } +} diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 9411edba70d..6a4608f5c66 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -6,13 +6,17 @@ package ir import ( - "cmp" "errors" "net" "reflect" + "cmp" + "github.com/tetratelabs/multierror" "golang.org/x/exp/slices" + + gwapiv1b1 "sigs.k8s.io/gateway-api/apis/v1beta1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -281,6 +285,8 @@ type HTTPRoute struct { CORS *CORS `json:"cors,omitempty" yaml:"cors,omitempty"` // JWT defines the schema for authenticating HTTP requests using JSON Web Tokens (JWT). JWT *JWT `json:"jwt,omitempty" yaml:"jwt,omitempty"` + // OIDC defines the schema for authenticating HTTP requests using OpenID Connect (OIDC). + OIDC *OIDC `json:"oidc,omitempty" yaml:"oidc,omitempty"` // ExtensionRefs holds unstructured resources that were introduced by an extension and used on the HTTPRoute as extensionRef filters ExtensionRefs []*UnstructuredRef `json:"extensionRefs,omitempty" yaml:"extensionRefs,omitempty"` } @@ -319,6 +325,29 @@ type JWT struct { Providers []egv1a1.JWTProvider `json:"providers,omitempty" yaml:"providers,omitempty"` } +// OIDC defines the schema for authenticating HTTP requests using +// OpenID Connect (OIDC). +// +// +k8s:deepcopy-gen=true +type OIDC struct { + // The OIDC Provider configuration. + Provider egv1a1.OIDCProvider `json:"provider"` + + // The OIDC client ID assigned to the filter to be used in the + // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + ClientID string `json:"clientID"` + + // The Kubernetes secret which contains the OIDC client secret assigned to the filter to be used in the + // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + // + // This is an Opaque secret. The client secret should be stored in the key "client_secret". + ClientSecret gwapiv1b1.SecretObjectReference `json:"clientSecret"` + + // The OIDC scopes to be used in the + // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + Scopes []string `json:"scopes,omitempty"` +} + // Validate the fields within the HTTPRoute structure func (h HTTPRoute) Validate() error { var errs error diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index 7ebe840dd5a..4697ce4cafa 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -461,6 +461,11 @@ func (in *HTTPRoute) DeepCopyInto(out *HTTPRoute) { *out = new(JWT) (*in).DeepCopyInto(*out) } + if in.OIDC != nil { + in, out := &in.OIDC, &out.OIDC + *out = new(OIDC) + (*in).DeepCopyInto(*out) + } if in.ExtensionRefs != nil { in, out := &in.ExtensionRefs, &out.ExtensionRefs *out = make([]*UnstructuredRef, len(*in)) @@ -682,6 +687,28 @@ func (in *Metrics) DeepCopy() *Metrics { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OIDC) DeepCopyInto(out *OIDC) { + *out = *in + out.Provider = in.Provider + in.ClientSecret.DeepCopyInto(&out.ClientSecret) + if in.Scopes != nil { + in, out := &in.Scopes, &out.Scopes + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OIDC. +func (in *OIDC) DeepCopy() *OIDC { + if in == nil { + return nil + } + out := new(OIDC) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OpenTelemetryAccessLog) DeepCopyInto(out *OpenTelemetryAccessLog) { *out = *in diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index 97ade72126b..cd54c0a4dd3 100644 --- a/internal/provider/kubernetes/controller.go +++ b/internal/provider/kubernetes/controller.go @@ -324,6 +324,9 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques resourceTree.SecurityPolicies = append(resourceTree.SecurityPolicies, &policy) } + // Add the referenced Secrets in SecurityPolicies to the resourceTree + r.processSecurityPolicySecretRefs(ctx, resourceTree, resourceMap) + // For this particular Gateway, and all associated objects, check whether the // namespace exists. Add to the resourceTree. for ns := range resourceMap.allAssociatedNamespaces { @@ -388,6 +391,61 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques return reconcile.Result{}, nil } +// processSecurityPolicySecretRefs adds the referenced Secrets in SecurityPolicies +// to the resourceTree +func (r *gatewayAPIReconciler) processSecurityPolicySecretRefs( + ctx context.Context, resourceTree *gatewayapi.Resources, resourceMap *resourceMappings) { + for _, policy := range resourceTree.SecurityPolicies { + oidc := policy.Spec.OIDC + if oidc != nil { + secret := new(corev1.Secret) + secretNamespace := gatewayapi.NamespaceDerefOr(oidc.ClientSecret.Namespace, policy.Namespace) + err := r.client.Get(ctx, + types.NamespacedName{Namespace: secretNamespace, Name: string(oidc.ClientSecret.Name)}, + secret, + ) + if err != nil && !kerrors.IsNotFound(err) { + r.log.Error(err, "unable to find the Secret for OIDC client secret") + // we don't return an error here, because we want to continue + // reconciling the rest of the resources despite that this + // SecurityPolicy is invalid. + // This SecurityPolicy will be marked as invalid in its status + // when translating to IR because the referenced secret can't be + // found. + } + + if secretNamespace != policy.Namespace { + from := ObjectKindNamespacedName{ + kind: v1alpha1.KindSecurityPolicy, + namespace: policy.Namespace, + name: policy.Name, + } + to := ObjectKindNamespacedName{ + kind: gatewayapi.KindSecret, + namespace: secretNamespace, + name: secret.Name, + } + refGrant, err := r.findReferenceGrant(ctx, from, to) + switch { + case err != nil: + r.log.Error(err, "failed to find ReferenceGrant") + case refGrant == nil: + r.log.Info("no matching ReferenceGrants found", "from", from.kind, + "from namespace", from.namespace, "target", to.kind, "target namespace", to.namespace) + default: + // RefGrant found + resourceMap.allAssociatedRefGrants[utils.NamespacedName(refGrant)] = refGrant + r.log.Info("added ReferenceGrant to resource map", "namespace", refGrant.Namespace, + "name", refGrant.Name) + } + } + resourceMap.allAssociatedNamespaces[secretNamespace] = struct{}{} // TODO Zhaohuabing do we need this line? + resourceTree.Secrets = append(resourceTree.Secrets, secret) + r.log.Info("processing Secret", "namespace", secretNamespace, "name", string(oidc.ClientSecret.Name)) + } + } +} + func (r *gatewayAPIReconciler) gatewayClassUpdater(ctx context.Context, gc *gwapiv1.GatewayClass, accepted bool, reason, msg string) error { if r.statusUpdater != nil { r.statusUpdater.Send(status.Update{ diff --git a/internal/status/conditions.go b/internal/status/conditions.go index bd4b3114b5e..eb6419d1541 100644 --- a/internal/status/conditions.go +++ b/internal/status/conditions.go @@ -16,6 +16,7 @@ package status import ( "fmt" "time" + "unicode" appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -138,3 +139,33 @@ func conditionChanged(a, b metav1.Condition) bool { (a.Message != b.Message) || (a.ObservedGeneration != b.ObservedGeneration) } + +// Error2ConditionMsg format the error string to a Status condition message. +// * Convert the first letter to capital +// * Append "." to the string if it doesn't exit +func Error2ConditionMsg(err error) string { + if err == nil { + return "" + } + + message := err.Error() + if message == "" { + return message + } + + // Convert the string to a rune slice for easier manipulation + runes := []rune(message) + + // Check if the first rune is a letter and convert it to uppercase + if unicode.IsLetter(runes[0]) { + runes[0] = unicode.ToUpper(runes[0]) + } + + // check if the last rune is . + if runes[len(runes)-1] != '.' { + return string(runes) + "." + } + + // Convert the rune slice back to a string + return string(runes) +} diff --git a/internal/status/conditions_test.go b/internal/status/conditions_test.go index 69a67590add..e1a24821f18 100644 --- a/internal/status/conditions_test.go +++ b/internal/status/conditions_test.go @@ -14,6 +14,7 @@ package status import ( + "errors" "testing" "time" @@ -341,3 +342,27 @@ func TestGatewayReadyCondition(t *testing.T) { }) } } + +func TestError2ConditionMsg(t *testing.T) { + testCases := []struct { + name string + err error + expect string + }{ + { + name: "nil error", + err: nil, + expect: "", + }, + { + name: "error with message", + err: errors.New("something is wrong"), + expect: "Something is wrong.", + }, + } + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + assert.Equalf(t, tt.expect, Error2ConditionMsg(tt.err), "Error2ConditionMsg(%v)", tt.err) + }) + } +} diff --git a/internal/status/securitypolicy.go b/internal/status/securitypolicy.go index c538a972308..cf9e478c6a1 100644 --- a/internal/status/securitypolicy.go +++ b/internal/status/securitypolicy.go @@ -19,14 +19,7 @@ func SetSecurityPolicyCondition(c *egv1a1.SecurityPolicy, conditionType gwv1a2.P c.Status.Conditions = MergeConditions(c.Status.Conditions, cond) } -func SetSecurityPolicyAcceptedIfUnset(s *egv1a1.SecurityPolicyStatus, message string) { - // Return early if Accepted condition is already set - for _, c := range s.Conditions { - if c.Type == string(gwv1a2.PolicyConditionAccepted) { - return - } - } - +func SetSecurityPolicyAccepted(s *egv1a1.SecurityPolicyStatus, message string) { cond := newCondition(string(gwv1a2.PolicyConditionAccepted), metav1.ConditionTrue, string(gwv1a2.PolicyReasonAccepted), message, time.Now(), 0) s.Conditions = MergeConditions(s.Conditions, cond) } diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 174ab89e45a..0b4c0b3f665 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -1071,6 +1071,40 @@ _Appears in:_ +#### OIDC + + + +OIDC defines the configuration for the OpenID Connect (OIDC) authentication. + +_Appears in:_ +- [SecurityPolicySpec](#securitypolicyspec) + +| Field | Description | +| --- | --- | +| `provider` _[OIDCProvider](#oidcprovider)_ | The OIDC Provider configuration. | +| `clientID` _string_ | The client ID assigned to this policy to be used in the OIDC [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). | +| `clientSecret` _[SecretObjectReference](https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1.SecretObjectReference)_ | The Kubernetes secret which contains the OIDC client secret assigned to the filter to be used in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + This is an Opaque secret. The client secret should be stored in the key "client_secret". | +| `scopes` _string array_ | The OIDC scopes to be used in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). The "openid" scope is always added to the list of scopes if not already specified. | + + +#### OIDCProvider + + + +OIDCProvider defines the OIDC Provider configuration. + +_Appears in:_ +- [OIDC](#oidc) + +| Field | Description | +| --- | --- | +| `issuer` _string_ | The OIDC Provider's [issuer identifier](https://openid.net/specs/openid-connect-discovery-1_0.html#IssuerDiscovery). Issuer MUST be a URI RFC 3986 [RFC3986] with a scheme component that MUST be https, a host component, and optionally, port and path components and no query or fragment components. | +| `authorizationEndpoint` _string_ | The OIDC Provider's [authorization endpoint](https://openid.net/specs/openid-connect-core-1_0.html#AuthorizationEndpoint). If not provided, EG will try to discover it from the provider's [Well-Known Configuration Endpoint](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationResponse). | +| `tokenEndpoint` _string_ | The OIDC Provider's [token endpoint](https://openid.net/specs/openid-connect-core-1_0.html#TokenEndpoint). If not provided, EG will try to discover it from the provider's [Well-Known Configuration Endpoint](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationResponse). | + + #### OpenTelemetryEnvoyProxyAccessLog @@ -1557,6 +1591,7 @@ _Appears in:_ | `targetRef` _[PolicyTargetReferenceWithSectionName](#policytargetreferencewithsectionname)_ | TargetRef is the name of the Gateway resource this policy is being attached to. This Policy and the TargetRef MUST be in the same namespace for this Policy to have effect and be applied to the Gateway. TargetRef | | `cors` _[CORS](#cors)_ | CORS defines the configuration for Cross-Origin Resource Sharing (CORS). | | `jwt` _[JWT](#jwt)_ | JWT defines the configuration for JSON Web Token (JWT) authentication. | +| `oidc` _[OIDC](#oidc)_ | OIDC defines the configuration for the OpenID Connect (OIDC) authentication. | From 0911b38130a1785a0185ef0c7ea8ebbfd1795e80 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Sat, 11 Nov 2023 05:23:04 +0800 Subject: [PATCH 2/8] address comments Signed-off-by: huabing zhao --- api/v1alpha1/oidc_types.go | 19 +++++++++++++++---- api/v1alpha1/zz_generated.deepcopy.go | 12 +++++++++++- internal/gatewayapi/securitypolicy.go | 6 +++--- ...itypolicy-with-oidc-invalid-issuer.in.yaml | 2 +- ...policy-with-oidc-invalid-secretref.in.yaml | 2 +- .../testdata/securitypolicy-with-oidc.in.yaml | 8 ++++---- internal/ir/xds.go | 5 ++--- internal/ir/zz_generated.deepcopy.go | 2 +- site/content/en/latest/api/extension_types.md | 8 ++++++-- 9 files changed, 44 insertions(+), 20 deletions(-) diff --git a/api/v1alpha1/oidc_types.go b/api/v1alpha1/oidc_types.go index 749b56b4d73..11288a542ae 100644 --- a/api/v1alpha1/oidc_types.go +++ b/api/v1alpha1/oidc_types.go @@ -9,7 +9,7 @@ import ( gwapiv1b1 "sigs.k8s.io/gateway-api/apis/v1beta1" ) -const OIDCClientSecretKey = "client_secret" +const OIDCClientSecretKey = "clientSecret" // OIDC defines the configuration for the OpenID Connect (OIDC) authentication. type OIDC struct { @@ -27,7 +27,7 @@ type OIDC struct { // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). // // This is an Opaque secret. The client secret should be stored in the key - // "client_secret". + // "clientSecret". // +kubebuilder:validation:Required ClientSecret gwapiv1b1.SecretObjectReference `json:"clientSecret"` @@ -40,6 +40,17 @@ type OIDC struct { } // OIDCProvider defines the OIDC Provider configuration. +// To make the EG OIDC config easy to use, some of the low-level ouath2 filter +// configuration knobs are hidden from the user, and default values will be provided +// when translating to XDS. For example: +// +// * redirect_uri: uses a default redirect URI "%REQ(x-forwarded-proto)%://%REQ(:authority)%/oauth2/callback" +// +// * signout_path: uses a default signout path "/signout" +// +// * redirect_path_matcher: uses a default redirect path matcher "/oauth2/callback" +// +// If we get requests to expose these knobs, we can always do so later. type OIDCProvider struct { // The OIDC Provider's [issuer identifier](https://openid.net/specs/openid-connect-discovery-1_0.html#IssuerDiscovery). // Issuer MUST be a URI RFC 3986 [RFC3986] with a scheme component that MUST @@ -54,11 +65,11 @@ type OIDCProvider struct { // If not provided, EG will try to discover it from the provider's [Well-Known Configuration Endpoint](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationResponse). // // +optional - AuthorizationEndpoint string `json:"authorizationEndpoint,omitempty"` + AuthorizationEndpoint *string `json:"authorizationEndpoint,omitempty"` // The OIDC Provider's [token endpoint](https://openid.net/specs/openid-connect-core-1_0.html#TokenEndpoint). // If not provided, EG will try to discover it from the provider's [Well-Known Configuration Endpoint](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationResponse). // // +optional - TokenEndpoint string `json:"tokenEndpoint,omitempty"` + TokenEndpoint *string `json:"tokenEndpoint,omitempty"` } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index eb884086ce8..9054a1e40bd 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -1525,7 +1525,7 @@ func (in *LoadBalancer) DeepCopy() *LoadBalancer { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OIDC) DeepCopyInto(out *OIDC) { *out = *in - out.Provider = in.Provider + in.Provider.DeepCopyInto(&out.Provider) in.ClientSecret.DeepCopyInto(&out.ClientSecret) if in.Scopes != nil { in, out := &in.Scopes, &out.Scopes @@ -1547,6 +1547,16 @@ func (in *OIDC) DeepCopy() *OIDC { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OIDCProvider) DeepCopyInto(out *OIDCProvider) { *out = *in + if in.AuthorizationEndpoint != nil { + in, out := &in.AuthorizationEndpoint, &out.AuthorizationEndpoint + *out = new(string) + **out = **in + } + if in.TokenEndpoint != nil { + in, out := &in.TokenEndpoint, &out.TokenEndpoint + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OIDCProvider. diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index 8094fd0f224..1b0842e3b1e 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -476,13 +476,13 @@ type OpenIDConfig struct { // discoverEndpointsFromIssuer discovers the token and authorization endpoints from the issuer's well-known url // return error if failed to fetch the well-known configuration func discoverEndpointsFromIssuer(provider *egv1a1.OIDCProvider) error { - if provider.TokenEndpoint == "" || provider.AuthorizationEndpoint == "" { + if provider.TokenEndpoint == nil || provider.AuthorizationEndpoint == nil { tokenEndpoint, authorizationEndpoint, err := fetchEndpointsFromIssuer(provider.Issuer) if err != nil { return fmt.Errorf("error fetching endpoints from issuer: %w", err) } - provider.TokenEndpoint = tokenEndpoint - provider.AuthorizationEndpoint = authorizationEndpoint + provider.TokenEndpoint = &tokenEndpoint + provider.AuthorizationEndpoint = &authorizationEndpoint } return nil } diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml index 75802df896d..f95fe91ca4f 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml @@ -5,7 +5,7 @@ secrets: namespace: default name: client1-secret data: - client_secret: Y2xpZW50MTpzZWNyZXQK + clientSecret: Y2xpZW50MTpzZWNyZXQK gateways: - apiVersion: gateway.networking.k8s.io/v1 kind: Gateway diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml index 34c15f88a51..11e5942e4c1 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml @@ -5,7 +5,7 @@ secrets: namespace: envoy-gateway name: client2-secret data: - client_secret: Y2xpZW50MTpzZWNyZXQK + clientSecret: Y2xpZW50MTpzZWNyZXQK - apiVersion: v1 kind: Secret metadata: diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml index 91272ac2e97..40fa3130037 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml @@ -5,21 +5,21 @@ secrets: namespace: envoy-gateway name: client1-secret data: - client_secret: Y2xpZW50MTpzZWNyZXQK + clientSecret: Y2xpZW50MTpzZWNyZXQK - apiVersion: v1 kind: Secret metadata: namespace: default name: client2-secret data: - client_secret: Y2xpZW50MTpzZWNyZXQK + clientSecret: Y2xpZW50MTpzZWNyZXQK - apiVersion: v1 kind: Secret metadata: namespace: envoy-gateway name: client3-secret data: - client_secret: Y2xpZW50MTpzZWNyZXQK + clientSecret: Y2xpZW50MTpzZWNyZXQK referenceGrants: - apiVersion: gateway.networking.k8s.io/v1alpha2 kind: ReferenceGrant @@ -108,7 +108,7 @@ securityPolicies: kind: SecurityPolicy metadata: namespace: envoy-gateway - name: policy-for-gateway-discover-endpoints # This policy should attach httproute-2 + name: policy-for-gateway-discover-endpoints # This policy should attach httproute-2 spec: targetRef: group: gateway.networking.k8s.io diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 6a4608f5c66..e1d298a67cf 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -6,12 +6,11 @@ package ir import ( + "cmp" "errors" "net" "reflect" - "cmp" - "github.com/tetratelabs/multierror" "golang.org/x/exp/slices" @@ -340,7 +339,7 @@ type OIDC struct { // The Kubernetes secret which contains the OIDC client secret assigned to the filter to be used in the // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). // - // This is an Opaque secret. The client secret should be stored in the key "client_secret". + // This is an Opaque secret. The client secret should be stored in the key "clientSecret". ClientSecret gwapiv1b1.SecretObjectReference `json:"clientSecret"` // The OIDC scopes to be used in the diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index 4697ce4cafa..a1fb5e25266 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -690,7 +690,7 @@ func (in *Metrics) DeepCopy() *Metrics { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OIDC) DeepCopyInto(out *OIDC) { *out = *in - out.Provider = in.Provider + in.Provider.DeepCopyInto(&out.Provider) in.ClientSecret.DeepCopyInto(&out.ClientSecret) if in.Scopes != nil { in, out := &in.Scopes, &out.Scopes diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 0b4c0b3f665..88397de7208 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -1085,7 +1085,7 @@ _Appears in:_ | `provider` _[OIDCProvider](#oidcprovider)_ | The OIDC Provider configuration. | | `clientID` _string_ | The client ID assigned to this policy to be used in the OIDC [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). | | `clientSecret` _[SecretObjectReference](https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1.SecretObjectReference)_ | The Kubernetes secret which contains the OIDC client secret assigned to the filter to be used in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). - This is an Opaque secret. The client secret should be stored in the key "client_secret". | + This is an Opaque secret. The client secret should be stored in the key "clientSecret". | | `scopes` _string array_ | The OIDC scopes to be used in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). The "openid" scope is always added to the list of scopes if not already specified. | @@ -1093,7 +1093,11 @@ _Appears in:_ -OIDCProvider defines the OIDC Provider configuration. +OIDCProvider defines the OIDC Provider configuration. To make the EG OIDC config easy to use, some of the low-level ouath2 filter configuration knobs are hidden from the user, and default values will be provided when translating to XDS. For example: + * redirect_uri: uses a default redirect URI "%REQ(x-forwarded-proto)%://%REQ(:authority)%/oauth2/callback" + * signout_path: uses a default signout path "/signout" + * redirect_path_matcher: uses a default redirect path matcher "/oauth2/callback" + If we get requests to expose these knobs, we can always do so later. _Appears in:_ - [OIDC](#oidc) From 95903a7e4455d7bf4d5b90da31ecea6e2854c22e Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Sat, 11 Nov 2023 16:50:35 +0800 Subject: [PATCH 3/8] address comments Signed-off-by: huabing zhao --- api/v1alpha1/oidc_types.go | 5 ++--- ...ateway.envoyproxy.io_securitypolicies.yaml | 9 ++++----- internal/gatewayapi/securitypolicy.go | 6 +++--- internal/gatewayapi/validate.go | 7 +++---- internal/ir/xds.go | 20 +++++++++++-------- internal/ir/zz_generated.deepcopy.go | 6 +++++- site/content/en/latest/api/extension_types.md | 4 ++-- 7 files changed, 31 insertions(+), 26 deletions(-) diff --git a/api/v1alpha1/oidc_types.go b/api/v1alpha1/oidc_types.go index 11288a542ae..817638fde93 100644 --- a/api/v1alpha1/oidc_types.go +++ b/api/v1alpha1/oidc_types.go @@ -16,14 +16,13 @@ type OIDC struct { // The OIDC Provider configuration. Provider OIDCProvider `json:"provider"` - // The client ID assigned to this policy to be used in the OIDC + // The client ID to be used in the OIDC // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). // // +kubebuilder:validation:MinLength=1 ClientID string `json:"clientID"` - // The Kubernetes secret which contains the OIDC client secret assigned to - // the filter to be used in the + // The Kubernetes secret which contains the OIDC client secret to be used in the // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). // // This is an Opaque secret. The client secret should be stored in the key diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml index e0fabfc1cf4..1168f856bcc 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -191,16 +191,15 @@ spec: (OIDC) authentication. properties: clientID: - description: The client ID assigned to this policy to be used - in the OIDC [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + description: The client ID to be used in the OIDC [Authentication + Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). minLength: 1 type: string clientSecret: description: "The Kubernetes secret which contains the OIDC client - secret assigned to the filter to be used in the [Authentication - Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + secret to be used in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). \n This is an Opaque secret. The client secret should be stored - in the key \"client_secret\"." + in the key \"clientSecret\"." properties: group: default: "" diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index 1b0842e3b1e..79df3fc5ae4 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -427,8 +427,8 @@ func (t *Translator) buildOIDC( return nil, err } - val, ok := clientSecret.Data[egv1a1.OIDCClientSecretKey] - if !ok || len(val) == 0 { + clientSecretBytes, ok := clientSecret.Data[egv1a1.OIDCClientSecretKey] + if !ok || len(clientSecretBytes) == 0 { return nil, fmt.Errorf("client secret not found in secret %s/%s", clientSecret.Namespace, clientSecret.Name) } @@ -444,7 +444,7 @@ func (t *Translator) buildOIDC( return &ir.OIDC{ Provider: *provider, ClientID: oidc.ClientID, - ClientSecret: oidc.ClientSecret, + ClientSecret: clientSecretBytes, Scopes: scopes, }, nil } diff --git a/internal/gatewayapi/validate.go b/internal/gatewayapi/validate.go index b8e0c34925d..7a5d18bbb4e 100644 --- a/internal/gatewayapi/validate.go +++ b/internal/gatewayapi/validate.go @@ -727,10 +727,9 @@ func (t *Translator) validateHostname(hostname string) error { } // validateSecretRef checks three things: -// 1. Dose the secret reference has valid Group and kind -// 2. If the secret reference is a cross namespace reference, is it permitted by -// any ReferenceGrant -// 3. Does the secret exist +// 1. Does the secret reference have a valid Group and kind +// 2. If the secret reference is a cross-namespace reference, is it permitted by any ReferenceGrant +// 3. Does the secret exist func (t *Translator) validateSecretRef( from crossNamespaceFrom, secretRef gwapiv1b1.SecretObjectReference, diff --git a/internal/ir/xds.go b/internal/ir/xds.go index e1d298a67cf..7998f1e2bb6 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -14,8 +14,6 @@ import ( "github.com/tetratelabs/multierror" "golang.org/x/exp/slices" - gwapiv1b1 "sigs.k8s.io/gateway-api/apis/v1beta1" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -152,6 +150,11 @@ func (x Xds) Printable() *Xds { for _, listener := range out.HTTP { // Omit field listener.TLS = nil + + for _, route := range listener.Routes { + // Omit field + route.OIDC.ClientSecret = []byte{} + } } return out } @@ -330,21 +333,22 @@ type JWT struct { // +k8s:deepcopy-gen=true type OIDC struct { // The OIDC Provider configuration. - Provider egv1a1.OIDCProvider `json:"provider"` + Provider egv1a1.OIDCProvider `json:"provider" yaml:"provider"` - // The OIDC client ID assigned to the filter to be used in the + // The OIDC client ID to be used in the // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). - ClientID string `json:"clientID"` + ClientID string `json:"clientID" yaml:"clientID"` - // The Kubernetes secret which contains the OIDC client secret assigned to the filter to be used in the + // The OIDC client secret to be used in the // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). // // This is an Opaque secret. The client secret should be stored in the key "clientSecret". - ClientSecret gwapiv1b1.SecretObjectReference `json:"clientSecret"` + + ClientSecret []byte `json:"clientSecret,omitempty" yaml:"clientSecret,omitempty"` // The OIDC scopes to be used in the // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). - Scopes []string `json:"scopes,omitempty"` + Scopes []string `json:"scopes,omitempty" yaml:"scopes,omitempty"` } // Validate the fields within the HTTPRoute structure diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index a1fb5e25266..edf7e1146cf 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -691,7 +691,11 @@ func (in *Metrics) DeepCopy() *Metrics { func (in *OIDC) DeepCopyInto(out *OIDC) { *out = *in in.Provider.DeepCopyInto(&out.Provider) - in.ClientSecret.DeepCopyInto(&out.ClientSecret) + if in.ClientSecret != nil { + in, out := &in.ClientSecret, &out.ClientSecret + *out = make([]byte, len(*in)) + copy(*out, *in) + } if in.Scopes != nil { in, out := &in.Scopes, &out.Scopes *out = make([]string, len(*in)) diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 88397de7208..08b52cdc8c5 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -1083,8 +1083,8 @@ _Appears in:_ | Field | Description | | --- | --- | | `provider` _[OIDCProvider](#oidcprovider)_ | The OIDC Provider configuration. | -| `clientID` _string_ | The client ID assigned to this policy to be used in the OIDC [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). | -| `clientSecret` _[SecretObjectReference](https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1.SecretObjectReference)_ | The Kubernetes secret which contains the OIDC client secret assigned to the filter to be used in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). +| `clientID` _string_ | The client ID to be used in the OIDC [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). | +| `clientSecret` _[SecretObjectReference](https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1.SecretObjectReference)_ | The Kubernetes secret which contains the OIDC client secret to be used in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). This is an Opaque secret. The client secret should be stored in the key "clientSecret". | | `scopes` _string array_ | The OIDC scopes to be used in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). The "openid" scope is always added to the list of scopes if not already specified. | From 2a8b7becb2eb483f28313a5d2baa5572415fb5b3 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Sat, 11 Nov 2023 16:59:30 +0800 Subject: [PATCH 4/8] address comments Signed-off-by: huabing zhao --- internal/gatewayapi/securitypolicy.go | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index 79df3fc5ae4..3480e3e66cf 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -22,6 +22,7 @@ import ( "github.com/envoyproxy/gateway/internal/ir" "github.com/envoyproxy/gateway/internal/status" "github.com/envoyproxy/gateway/internal/utils/ptr" + "github.com/tetratelabs/multierror" ) func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.SecurityPolicy, @@ -255,16 +256,15 @@ func (t *Translator) translateSecurityPolicyForRoute( resources *Resources, xdsIR XdsIRMap) error { // Build IR var ( - cors *ir.CORS - jwt *ir.JWT - oidc *ir.OIDC - err error + cors *ir.CORS + jwt *ir.JWT + oidc *ir.OIDC + err, errs error ) if policy.Spec.CORS != nil { - cors, err = t.buildCORS(policy.Spec.CORS) - if err != nil { - return err + if cors, err = t.buildCORS(policy.Spec.CORS); err != nil { + errs = multierror.Append(errs, err) } } @@ -273,16 +273,20 @@ func (t *Translator) translateSecurityPolicyForRoute( } if policy.Spec.OIDC != nil { - oidc, err = t.buildOIDC(policy, resources) - if err != nil { - return err + if oidc, err = t.buildOIDC(policy, resources); err != nil { + errs = multierror.Append(errs, err) } } - // Apply IR to all relevant routes // It can be difficult to reason about the state of the system if we apply // part of the policy and not the rest. Therefore, we either apply all of it // or none of it (when get errors when translating the policy) + + if errs != nil { + return errs + } + + // Apply IR to all relevant routes prefix := irRoutePrefix(route) for _, ir := range xdsIR { for _, http := range ir.HTTP { From 30e72f939aee37fd01c3f2b4d9ffe9e2f3651cc8 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Sat, 11 Nov 2023 17:43:52 +0800 Subject: [PATCH 5/8] fix test Signed-off-by: huabing zhao --- internal/gatewayapi/securitypolicy.go | 3 ++- .../testdata/securitypolicy-with-oidc.out.yaml | 16 +++------------- internal/ir/xds.go | 4 +++- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index 3480e3e66cf..277cb642bee 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -18,11 +18,12 @@ import ( gwv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2" gwv1b1 "sigs.k8s.io/gateway-api/apis/v1beta1" + "github.com/tetratelabs/multierror" + egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/ir" "github.com/envoyproxy/gateway/internal/status" "github.com/envoyproxy/gateway/internal/utils/ptr" - "github.com/tetratelabs/multierror" ) func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.SecurityPolicy, diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml index 302fe418d7b..49ffa545332 100755 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml @@ -283,10 +283,7 @@ xdsIR: name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io oidc: clientID: client2.oauth.foo.com - clientSecret: - group: null - kind: null - name: client2-secret + clientSecret: Y2xpZW50MTpzZWNyZXQK provider: authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth issuer: https://oauth.foo.com @@ -314,10 +311,7 @@ xdsIR: name: httproute/default/httproute-2/rule/0/match/0/gateway_envoyproxy_io oidc: clientID: client1.apps.googleusercontent.com - clientSecret: - group: null - kind: null - name: client1-secret + clientSecret: Y2xpZW50MTpzZWNyZXQK provider: authorizationEndpoint: https://accounts.google.com/o/oauth2/v2/auth issuer: https://accounts.google.com @@ -343,11 +337,7 @@ xdsIR: name: grpcroute/default/grpcroute-1/rule/0/match/-1/* oidc: clientID: client3.bar.foo.com - clientSecret: - group: null - kind: null - name: client3-secret - namespace: envoy-gateway + clientSecret: Y2xpZW50MTpzZWNyZXQK provider: authorizationEndpoint: https://oauth.bar.com/oauth2/v2/auth issuer: https://oauth.bar.com diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 7998f1e2bb6..f09490a8178 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -153,7 +153,9 @@ func (x Xds) Printable() *Xds { for _, route := range listener.Routes { // Omit field - route.OIDC.ClientSecret = []byte{} + if route.OIDC != nil { + route.OIDC.ClientSecret = []byte{} + } } } return out From bafdd2dd6f1b68ec6151cc261a8a5de20c248922 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Mon, 13 Nov 2023 10:55:14 -0800 Subject: [PATCH 6/8] forbid cross-namespace ref to secret for now Signed-off-by: huabing zhao --- internal/gatewayapi/securitypolicy.go | 7 +++++-- internal/gatewayapi/validate.go | 14 +++++++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index 277cb642bee..567e0d89fc3 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -428,13 +428,16 @@ func (t *Translator) buildOIDC( kind: KindSecurityPolicy, namespace: policy.Namespace, } - if clientSecret, err = t.validateSecretRef(from, oidc.ClientSecret, resources); err != nil { + if clientSecret, err = t.validateSecretRef( + false, from, oidc.ClientSecret, resources); err != nil { return nil, err } clientSecretBytes, ok := clientSecret.Data[egv1a1.OIDCClientSecretKey] if !ok || len(clientSecretBytes) == 0 { - return nil, fmt.Errorf("client secret not found in secret %s/%s", clientSecret.Namespace, clientSecret.Name) + return nil, fmt.Errorf( + "client secret not found in secret %s/%s", + clientSecret.Namespace, clientSecret.Name) } // Discover the token and authorization endpoints from the issuer's diff --git a/internal/gatewayapi/validate.go b/internal/gatewayapi/validate.go index 7a5d18bbb4e..8acc278a46d 100644 --- a/internal/gatewayapi/validate.go +++ b/internal/gatewayapi/validate.go @@ -727,10 +727,12 @@ func (t *Translator) validateHostname(hostname string) error { } // validateSecretRef checks three things: -// 1. Does the secret reference have a valid Group and kind -// 2. If the secret reference is a cross-namespace reference, is it permitted by any ReferenceGrant -// 3. Does the secret exist +// 1. Does the secret reference have a valid Group and kind +// 2. If the secret reference is a cross-namespace reference, +// is it permitted by any ReferenceGrant +// 3. Does the secret exist func (t *Translator) validateSecretRef( + allowCrossNamespace bool, from crossNamespaceFrom, secretRef gwapiv1b1.SecretObjectReference, resources *Resources) (*v1.Secret, error) { @@ -747,6 +749,12 @@ func (t *Translator) validateSecretRef( if secretRef.Namespace != nil && string(*secretRef.Namespace) != "" && string(*secretRef.Namespace) != from.namespace { + if !allowCrossNamespace { + return nil, fmt.Errorf( + "secret ref namespace must be unspecified/empty or %s", + from.namespace) + } + if !t.validateCrossNamespaceRef( from, crossNamespaceTo{ From 223fe9d1bc1a66ce5d146d38422a11293cb6b1e9 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Mon, 13 Nov 2023 12:19:12 -0800 Subject: [PATCH 7/8] address comments Signed-off-by: huabing zhao --- api/v1alpha1/oidc_types.go | 4 +- ...ateway.envoyproxy.io_securitypolicies.yaml | 2 +- internal/gatewayapi/securitypolicy.go | 30 +- ...typolicy-with-jwt-and-invalid-oidc.in.yaml | 111 +++++++ ...ypolicy-with-jwt-and-invalid-oidc.out.yaml | 279 ++++++++++++++++++ ...itypolicy-with-oidc-invalid-issuer.in.yaml | 2 +- ...policy-with-oidc-invalid-secretref.in.yaml | 2 +- ...olicy-with-oidc-invalid-secretref.out.yaml | 3 +- .../testdata/securitypolicy-with-oidc.in.yaml | 24 +- .../securitypolicy-with-oidc.out.yaml | 2 +- internal/ir/xds.go | 2 +- site/content/en/latest/api/extension_types.md | 2 +- 12 files changed, 417 insertions(+), 46 deletions(-) create mode 100644 internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.in.yaml create mode 100755 internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.out.yaml diff --git a/api/v1alpha1/oidc_types.go b/api/v1alpha1/oidc_types.go index 817638fde93..f690cfd7c56 100644 --- a/api/v1alpha1/oidc_types.go +++ b/api/v1alpha1/oidc_types.go @@ -9,7 +9,7 @@ import ( gwapiv1b1 "sigs.k8s.io/gateway-api/apis/v1beta1" ) -const OIDCClientSecretKey = "clientSecret" +const OIDCClientSecretKey = "client-secret" // OIDC defines the configuration for the OpenID Connect (OIDC) authentication. type OIDC struct { @@ -26,7 +26,7 @@ type OIDC struct { // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). // // This is an Opaque secret. The client secret should be stored in the key - // "clientSecret". + // "client-secret". // +kubebuilder:validation:Required ClientSecret gwapiv1b1.SecretObjectReference `json:"clientSecret"` diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml index 1168f856bcc..bc4833e5cb3 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -199,7 +199,7 @@ spec: description: "The Kubernetes secret which contains the OIDC client secret to be used in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). \n This is an Opaque secret. The client secret should be stored - in the key \"clientSecret\"." + in the key \"client-secret\"." properties: group: default: "" diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index 567e0d89fc3..977ff683740 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -60,7 +60,7 @@ func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.Security // Translate // 1. First translate Policies targeting xRoutes - // 2.. Finally, the policies targeting Gateways + // 2. Finally, the policies targeting Gateways // Process the policies targeting xRoutes for _, policy := range securityPolicies { @@ -279,15 +279,9 @@ func (t *Translator) translateSecurityPolicyForRoute( } } - // It can be difficult to reason about the state of the system if we apply - // part of the policy and not the rest. Therefore, we either apply all of it - // or none of it (when get errors when translating the policy) - - if errs != nil { - return errs - } - // Apply IR to all relevant routes + // Note: there are multiple features in a security policy, even if some of them + // are invalid, we still want to apply the valid ones. prefix := irRoutePrefix(route) for _, ir := range xdsIR { for _, http := range ir.HTTP { @@ -303,7 +297,7 @@ func (t *Translator) translateSecurityPolicyForRoute( } } } - return nil + return errs } func (t *Translator) translateSecurityPolicyForGateway( @@ -311,16 +305,16 @@ func (t *Translator) translateSecurityPolicyForGateway( resources *Resources, xdsIR XdsIRMap) error { // Build IR var ( - cors *ir.CORS - jwt *ir.JWT - oidc *ir.OIDC - err error + cors *ir.CORS + jwt *ir.JWT + oidc *ir.OIDC + err, errs error ) if policy.Spec.CORS != nil { cors, err = t.buildCORS(policy.Spec.CORS) if err != nil { - return err + errs = multierror.Append(errs, err) } } @@ -331,7 +325,7 @@ func (t *Translator) translateSecurityPolicyForGateway( if policy.Spec.OIDC != nil { oidc, err = t.buildOIDC(policy, resources) if err != nil { - return err + errs = multierror.Append(errs, err) } } @@ -342,6 +336,8 @@ func (t *Translator) translateSecurityPolicyForGateway( // It can be difficult to reason about the state of the system if we apply // part of the policy and not the rest. Therefore, we either apply all of it // or none of it (when get errors when translating the policy) + // Note: there are multiple features in a security policy, even if some of them + // are invalid, we still want to apply the valid ones. irKey := t.getIRKey(gateway.Gateway) // Should exist since we've validated this ir := xdsIR[irKey] @@ -360,7 +356,7 @@ func (t *Translator) translateSecurityPolicyForGateway( } } } - return nil + return errs } func (t *Translator) buildCORS(cors *egv1a1.CORS) (*ir.CORS, error) { diff --git a/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.in.yaml new file mode 100644 index 00000000000..741b5fd670e --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.in.yaml @@ -0,0 +1,111 @@ +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 +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-1 + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/foo" + backendRefs: + - name: service-1 + port: 8080 +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-2 + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/bar" + backendRefs: + - name: service-1 + port: 8080 +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: envoy-gateway + name: policy-for-gateway # This policy should attach httproute-2 + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + jwt: + providers: + - name: example1 + issuer: https://one.example.com + audiences: + - one.foo.com + remoteJWKS: + uri: https://one.example.com/jwt/public-key/jwks.json + claimToHeaders: + - header: one-route-example-key + claim: claim1 + oidc: + provider: + issuer: "https://accounts.google.com" + clientID: "client1.apps.googleusercontent.com" + clientSecret: + name: "client1-secret" +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: default + name: policy-for-http-route # This policy should attach httproute-2 + spec: + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-2 + namespace: default + jwt: + providers: + - name: example2 + issuer: https://two.example.com + audiences: + - two.foo.com + remoteJWKS: + uri: https://one.example.com/jwt/public-key/jwks.json + claimToHeaders: + - header: one-route-example-key + claim: claim2 + oidc: + provider: + issuer: "https://accounts.google.com" + clientID: "client1.apps.googleusercontent.com" + clientSecret: + name: "client2-secret" diff --git a/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.out.yaml new file mode 100755 index 00000000000..5ed5a760aff --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.out.yaml @@ -0,0 +1,279 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + 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 +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-1 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: /foo + 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: + creationTimestamp: null + name: httproute-2 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: /bar + 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 +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - address: "" + ports: + - containerPort: 10080 + name: http + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + name: envoy-gateway/gateway-1 +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + creationTimestamp: null + name: policy-for-http-route + namespace: default + spec: + jwt: + providers: + - audiences: + - two.foo.com + claimToHeaders: + - claim: claim2 + header: one-route-example-key + issuer: https://two.example.com + name: example2 + remoteJWKS: + uri: https://one.example.com/jwt/public-key/jwks.json + oidc: + clientID: client1.apps.googleusercontent.com + clientSecret: + group: null + kind: null + name: client2-secret + provider: + issuer: https://accounts.google.com + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-2 + namespace: default + status: + conditions: + - lastTransitionTime: null + message: Secret default/client2-secret does not exist. + reason: Invalid + status: "False" + type: Accepted +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + creationTimestamp: null + name: policy-for-gateway + namespace: envoy-gateway + spec: + jwt: + providers: + - audiences: + - one.foo.com + claimToHeaders: + - claim: claim1 + header: one-route-example-key + issuer: https://one.example.com + name: example1 + remoteJWKS: + uri: https://one.example.com/jwt/public-key/jwks.json + oidc: + clientID: client1.apps.googleusercontent.com + clientSecret: + group: null + kind: null + name: client1-secret + provider: + issuer: https://accounts.google.com + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + status: + conditions: + - lastTransitionTime: null + message: Secret envoy-gateway/client1-secret does not exist. + reason: Invalid + status: "False" + type: Accepted +xdsIR: + envoy-gateway/gateway-1: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: envoy-gateway/gateway-1/http + port: 10080 + routes: + - backendWeights: + invalid: 0 + valid: 0 + destination: + name: httproute/default/httproute-1/rule/0 + settings: + - endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: gateway.envoyproxy.io + jwt: + providers: + - audiences: + - one.foo.com + claimToHeaders: + - claim: claim1 + header: one-route-example-key + issuer: https://one.example.com + name: example1 + remoteJWKS: + uri: https://one.example.com/jwt/public-key/jwks.json + name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io + pathMatch: + distinct: false + name: "" + prefix: /foo + - backendWeights: + invalid: 0 + valid: 0 + destination: + name: httproute/default/httproute-2/rule/0 + settings: + - endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: gateway.envoyproxy.io + jwt: + providers: + - audiences: + - two.foo.com + claimToHeaders: + - claim: claim2 + header: one-route-example-key + issuer: https://two.example.com + name: example2 + remoteJWKS: + uri: https://one.example.com/jwt/public-key/jwks.json + name: httproute/default/httproute-2/rule/0/match/0/gateway_envoyproxy_io + pathMatch: + distinct: false + name: "" + prefix: /bar diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml index f95fe91ca4f..9f49012c528 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml @@ -5,7 +5,7 @@ secrets: namespace: default name: client1-secret data: - clientSecret: Y2xpZW50MTpzZWNyZXQK + client-secret: Y2xpZW50MTpzZWNyZXQK gateways: - apiVersion: gateway.networking.k8s.io/v1 kind: Gateway diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml index 11e5942e4c1..c3c86142e0b 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml @@ -5,7 +5,7 @@ secrets: namespace: envoy-gateway name: client2-secret data: - clientSecret: Y2xpZW50MTpzZWNyZXQK + client-secret: Y2xpZW50MTpzZWNyZXQK - apiVersion: v1 kind: Secret metadata: diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml index f57844492bc..5a30d9f3c6b 100755 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml @@ -216,8 +216,7 @@ securityPolicies: status: conditions: - lastTransitionTime: null - message: Certificate ref to secret envoy-gateway/client2-secret not permitted - by any ReferenceGrant. + message: Secret ref namespace must be unspecified/empty or default. reason: Invalid status: "False" type: Accepted diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml index 40fa3130037..889fbfbfc8e 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml @@ -5,35 +5,21 @@ secrets: namespace: envoy-gateway name: client1-secret data: - clientSecret: Y2xpZW50MTpzZWNyZXQK + client-secret: Y2xpZW50MTpzZWNyZXQK - apiVersion: v1 kind: Secret metadata: namespace: default name: client2-secret data: - clientSecret: Y2xpZW50MTpzZWNyZXQK + client-secret: Y2xpZW50MTpzZWNyZXQK - apiVersion: v1 kind: Secret metadata: - namespace: envoy-gateway + namespace: default name: client3-secret data: - clientSecret: Y2xpZW50MTpzZWNyZXQK -referenceGrants: -- apiVersion: gateway.networking.k8s.io/v1alpha2 - kind: ReferenceGrant - metadata: - namespace: envoy-gateway - name: referencegrant-1 - spec: - from: - - group: gateway.envoyproxy.io - kind: SecurityPolicy - namespace: default - to: - - group: "" - kind: Secret + client-secret: Y2xpZW50MTpzZWNyZXQK gateways: - apiVersion: gateway.networking.k8s.io/v1 kind: Gateway @@ -158,5 +144,5 @@ securityPolicies: tokenEndpoint: "https://oauth.bar.com/token" clientID: "client3.bar.foo.com" clientSecret: - namespace: envoy-gateway + namespace: default name: "client3-secret" diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml index 49ffa545332..a334ddd8693 100755 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml @@ -212,7 +212,7 @@ securityPolicies: group: null kind: null name: client3-secret - namespace: envoy-gateway + namespace: default provider: authorizationEndpoint: https://oauth.bar.com/oauth2/v2/auth issuer: https://oauth.bar.com diff --git a/internal/ir/xds.go b/internal/ir/xds.go index f09490a8178..2c5179d9d23 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -344,7 +344,7 @@ type OIDC struct { // The OIDC client secret to be used in the // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). // - // This is an Opaque secret. The client secret should be stored in the key "clientSecret". + // This is an Opaque secret. The client secret should be stored in the key "client-secret". ClientSecret []byte `json:"clientSecret,omitempty" yaml:"clientSecret,omitempty"` diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 08b52cdc8c5..8cb5a5fcf71 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -1085,7 +1085,7 @@ _Appears in:_ | `provider` _[OIDCProvider](#oidcprovider)_ | The OIDC Provider configuration. | | `clientID` _string_ | The client ID to be used in the OIDC [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). | | `clientSecret` _[SecretObjectReference](https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1.SecretObjectReference)_ | The Kubernetes secret which contains the OIDC client secret to be used in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). - This is an Opaque secret. The client secret should be stored in the key "clientSecret". | + This is an Opaque secret. The client secret should be stored in the key "client-secret". | | `scopes` _string array_ | The OIDC scopes to be used in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). The "openid" scope is always added to the list of scopes if not already specified. | From 08fb74fef78f2dd51258634a3078ff85a41c17b4 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Mon, 13 Nov 2023 12:22:46 -0800 Subject: [PATCH 8/8] fix lint Signed-off-by: huabing zhao --- ...typolicy-with-jwt-and-invalid-oidc.in.yaml | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.in.yaml index 741b5fd670e..833c2b70c13 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.in.yaml @@ -66,15 +66,15 @@ securityPolicies: namespace: envoy-gateway jwt: providers: - - name: example1 - issuer: https://one.example.com - audiences: - - one.foo.com - remoteJWKS: - uri: https://one.example.com/jwt/public-key/jwks.json - claimToHeaders: - - header: one-route-example-key - claim: claim1 + - name: example1 + issuer: https://one.example.com + audiences: + - one.foo.com + remoteJWKS: + uri: https://one.example.com/jwt/public-key/jwks.json + claimToHeaders: + - header: one-route-example-key + claim: claim1 oidc: provider: issuer: "https://accounts.google.com" @@ -94,15 +94,15 @@ securityPolicies: namespace: default jwt: providers: - - name: example2 - issuer: https://two.example.com - audiences: - - two.foo.com - remoteJWKS: - uri: https://one.example.com/jwt/public-key/jwks.json - claimToHeaders: - - header: one-route-example-key - claim: claim2 + - name: example2 + issuer: https://two.example.com + audiences: + - two.foo.com + remoteJWKS: + uri: https://one.example.com/jwt/public-key/jwks.json + claimToHeaders: + - header: one-route-example-key + claim: claim2 oidc: provider: issuer: "https://accounts.google.com"