From 7b50c8a510f2645219ee05da5195042c02552932 Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Thu, 1 May 2025 13:22:52 -0400 Subject: [PATCH] jwt: support CEL expressions with escaped names This is purely for consistency with other uses of CEL in the project. Using `[` for accessing claims or user data is preferred when names contain characters that would need to be escaped. CEL optionals via `?` can be used in places where `has` cannot be used, i.e. `claims[?"kubernetes.io"]` or `user.extra[?"domain.io/foo"]`. Signed-off-by: Monis Khan --- .../pkg/authenticator/token/oidc/oidc.go | 61 +++- .../pkg/authenticator/token/oidc/oidc_test.go | 265 +++++++++++++++++- 2 files changed, 321 insertions(+), 5 deletions(-) diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go index dfe1e98df0a..6edbe651c87 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go @@ -45,6 +45,7 @@ import ( celgo "github.com/google/cel-go/cel" "github.com/google/cel-go/common/types" "github.com/google/cel-go/common/types/ref" + "github.com/google/cel-go/common/types/traits" "k8s.io/apimachinery/pkg/util/net" "k8s.io/apimachinery/pkg/util/sets" @@ -55,6 +56,7 @@ import ( authenticationcel "k8s.io/apiserver/pkg/authentication/cel" authenticationtokenjwt "k8s.io/apiserver/pkg/authentication/token/jwt" "k8s.io/apiserver/pkg/authentication/user" + "k8s.io/apiserver/pkg/cel" "k8s.io/apiserver/pkg/cel/lazy" certutil "k8s.io/client-go/util/cert" "k8s.io/klog/v2" @@ -1037,7 +1039,7 @@ func newClaimsValue(c claims) *lazy.MapValue { if err := json.Unmarshal(data, &value); err != nil { return types.NewErr("claim %q failed to unmarshal: %w", name, err) } - return types.DefaultTypeAdapter.NativeToValue(value) + return nativeToValueWithUnescape(value) }) } return lazyMap @@ -1128,7 +1130,7 @@ func newUserInfoValue(info user.Info) *lazy.MapValue { field := func(name string, get func() any) { lazyMap.Append(name, func(_ *lazy.MapValue) ref.Val { value := get() - return types.DefaultTypeAdapter.NativeToValue(value) + return nativeToValueWithUnescape(value) }) } field("username", func() any { return info.GetName() }) @@ -1137,3 +1139,58 @@ func newUserInfoValue(info user.Info) *lazy.MapValue { field("extra", func() any { return info.GetExtra() }) return lazyMap } + +func nativeToValueWithUnescape(value any) ref.Val { + return unescapeWrapper(types.DefaultTypeAdapter.NativeToValue(value)) +} + +type unescapeMapper struct { + traits.Mapper +} + +func (m *unescapeMapper) Find(key ref.Val) (ref.Val, bool) { + name, ok := unescapedName(key) + if ok { + key = name + } + value, ok := m.Mapper.Find(key) + return unescapeWrapper(value), ok +} + +type unescapeLister struct { + traits.Lister +} + +func (l *unescapeLister) Get(index ref.Val) ref.Val { + return unescapeWrapper(l.Lister.Get(index)) +} + +// unescapeWrapper handles __dot__ based field access for native types that are converted into CEL values. +// This means we need to handle map lookups for our native types (the claims JSON and the user info data). +// User info is straightforward since it just has a single map field that needs the __dot__ support. The +// claims JSON is more complicated because maps can appear in deeply nested fields. This means that we need +// to account for both nested JSON objects and nested JSON arrays in all contexts where we return a CEL value. +// It is safe to pass any CEL value to this function, including nil (i.e. the caller can skip error checking). +func unescapeWrapper(value ref.Val) ref.Val { + switch v := value.(type) { + case traits.Mapper: + return &unescapeMapper{Mapper: v} // handle nested JSON objects + case traits.Lister: + return &unescapeLister{Lister: v} // handle nested JSON arrays + default: + return value + } +} + +func unescapedName(key ref.Val) (types.String, bool) { + n, ok := key.(types.String) + if !ok { + return "", false + } + ns := string(n) + name, ok := cel.Unescape(ns) + if !ok || name == ns { + return "", false + } + return types.String(name), true +} diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go index 9ef7d7a4499..5eea2022d9a 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go @@ -2520,7 +2520,7 @@ func TestToken(t *testing.T) { Expression: `"claus" in user.groups`, }, { - Expression: `user.extra["bio.snorlax.org/3"].size() == 4`, + Expression: `user.extra.bio__dot__snorlax__dot__org__slash__3.size() == 4`, }, }, }, @@ -2630,7 +2630,7 @@ func TestToken(t *testing.T) { Expression: `"claus" in user.groups`, }, { - Expression: `user.extra["bio.snorlax.org/3"].size() == 4`, + Expression: `user.extra.bio__dot__snorlax__dot__org__slash__3.size() == 4`, }, }, }, @@ -2740,7 +2740,7 @@ func TestToken(t *testing.T) { Expression: `"claus" in user.groups`, }, { - Expression: `user.extra["bio.snorlax.org/3"].size() == 4`, + Expression: `user.extra.bio__dot__snorlax__dot__org__slash__3.size() == 4`, }, }, }, @@ -2792,6 +2792,265 @@ func TestToken(t *testing.T) { }`, valid.Unix()), wantErr: "oidc: error evaluating group claim expression: expression 'claims.turtle.foo.bar.baz.a.b.c.d' resulted in error: no such key: a", }, + { + name: "claim mappings with expressions and deeply nested claim - success - service account payload", + options: Options{ + JWTAuthenticator: apiserver.JWTAuthenticator{ + Issuer: apiserver.Issuer{ + URL: "https://kubernetes.default.svc", + Audiences: []string{"https://kubernetes.default.svc"}, + }, + ClaimValidationRules: []apiserver.ClaimValidationRule{ + { + Expression: `claims.sub == ("system:serviceaccount:" + claims.kubernetes__dot__io.namespace + ":" + claims.kubernetes__dot__io.serviceaccount.name)`, + }, + { + Expression: `has(claims.kubernetes__dot__io.pod.uid)`, // has requires field based access + }, + { + Expression: `claims["kubernetes.io"]["node"]["uid"] != ""`, // the [ based syntax is preferred and is easier to read + }, + { + Expression: `claims[?"kubernetes.io"]["node"]["ip"].orValue(true)`, // optionals can be used even when has() cannot + }, + }, + ClaimMappings: apiserver.ClaimMappings{ + Username: apiserver.PrefixedClaimOrExpression{ + Expression: `"system:serviceaccount:" + claims.kubernetes__dot__io.namespace + ":" + claims.kubernetes__dot__io.serviceaccount.name`, + }, + UID: apiserver.ClaimOrExpression{ + Expression: "claims.kubernetes__dot__io.serviceaccount.uid", + }, + Groups: apiserver.PrefixedClaimOrExpression{ + Expression: `[ "system:serviceaccounts", "system:serviceaccounts:" + claims.kubernetes__dot__io.namespace ]`, + }, + Extra: []apiserver.ExtraMapping{ // use x-kubernetes.io since validation prevents the use of kubernetes.io + { + Key: "authentication.x-kubernetes.io/pod-name", + ValueExpression: "claims.kubernetes__dot__io.pod.name", + }, + { + Key: "authentication.x-kubernetes.io/pod-uid", + ValueExpression: "claims.kubernetes__dot__io.pod.uid", + }, + { + Key: "authentication.x-kubernetes.io/node-name", + ValueExpression: "claims.kubernetes__dot__io.node.name", + }, + { + Key: "authentication.x-kubernetes.io/node-uid", + ValueExpression: "claims.kubernetes__dot__io.node.uid", + }, + { + Key: "authentication.x-kubernetes.io/credential-id", + ValueExpression: `"JTI=" + claims.jti`, + }, + { + Key: "test.x-kubernetes.io/warnafter", + ValueExpression: "string(int(claims.kubernetes__dot__io.warnafter))", + }, + { + Key: "test.x-kubernetes.io/namespace", + ValueExpression: "claims.kubernetes__dot__io.namespace", + }, + }, + }, + UserValidationRules: []apiserver.UserValidationRule{ + { + Expression: `user.username.startsWith("system:serviceaccount:" + user.extra["test.x-kubernetes.io/namespace"][0] + ":")`, // the [ based syntax is preferred and is easier to read + }, + { + Expression: `user.uid != ""`, + }, + { + Expression: `"system:serviceaccounts" in user.groups && user.groups.size() == 2`, + }, + { + Expression: `user.extra.authentication__dot__x__dash__kubernetes__dot__io__slash__node__dash__name[0] == "127.0.0.1"`, + }, + { + Expression: `user.extra.authentication__dot__x__dash__kubernetes__dot__io__slash__credential__dash__id[0] == user.extra.authentication__dot__kubernetes__dot__io__slash__credential__dash__id[0]`, + }, + { + Expression: `user.extra["test.x-kubernetes.io/warnafter"][0] == "1700081020"`, + }, + { + Expression: `has(user.extra.authentication__dot__x__dash__kubernetes__dot__io__slash__pod__dash__name)`, // has requires field based access + }, + { + Expression: `user.extra[?"test.x-kubernetes.io/missing"].orValue([]) == []`, // optionals can be used even when has() cannot + }, + }, + }, + now: func() time.Time { return now }, + }, + signingKey: loadRSAPrivKey(t, "testdata/rsa_1.pem", jose.RS256), + pubKeys: []*jose.JSONWebKey{ + loadRSAKey(t, "testdata/rsa_1.pem", jose.RS256), + }, + claims: fmt.Sprintf(`{ + "aud": [ + "https://kubernetes.default.svc" + ], + "exp": %d, + "iat": 1700077413, + "iss": "https://kubernetes.default.svc", + "jti": "ea28ed49-2e11-4280-9ec5-bc3d1d84661a", + "kubernetes.io": { + "namespace": "kube-system", + "node": { + "name": "127.0.0.1", + "uid": "58456cb0-dd00-45ed-b797-5578fdceaced" + }, + "pod": { + "name": "coredns-69cbfb9798-jv9gn", + "uid": "778a530c-b3f4-47c0-9cd5-ab018fb64f33" + }, + "serviceaccount": { + "name": "coredns", + "uid": "a087d5a0-e1dd-43ec-93ac-f13d89cd13af" + }, + "warnafter": 1700081020 + }, + "nbf": %d, + "sub": "system:serviceaccount:kube-system:coredns" +}`, valid.Unix(), now.Unix()), + want: &user.DefaultInfo{ + Name: "system:serviceaccount:kube-system:coredns", + UID: "a087d5a0-e1dd-43ec-93ac-f13d89cd13af", + Groups: []string{ + "system:serviceaccounts", "system:serviceaccounts:kube-system", + }, + Extra: map[string][]string{ + "authentication.x-kubernetes.io/pod-name": { + "coredns-69cbfb9798-jv9gn", + }, + "authentication.x-kubernetes.io/pod-uid": { + "778a530c-b3f4-47c0-9cd5-ab018fb64f33", + }, + "authentication.x-kubernetes.io/node-name": { + "127.0.0.1", + }, + "authentication.x-kubernetes.io/node-uid": { + "58456cb0-dd00-45ed-b797-5578fdceaced", + }, + "authentication.kubernetes.io/credential-id": { + "JTI=ea28ed49-2e11-4280-9ec5-bc3d1d84661a", + }, + "authentication.x-kubernetes.io/credential-id": { + "JTI=ea28ed49-2e11-4280-9ec5-bc3d1d84661a", + }, + "test.x-kubernetes.io/warnafter": { + "1700081020", + }, + "test.x-kubernetes.io/namespace": { + "kube-system", + }, + }, + }, + }, + { + name: "claim mappings with expressions and deeply nested claim - success - claims payload with deep use of __dot__", + options: Options{ + JWTAuthenticator: apiserver.JWTAuthenticator{ + Issuer: apiserver.Issuer{ + URL: "https://auth.example.com", + Audiences: []string{"my-client"}, + }, + ClaimValidationRules: []apiserver.ClaimValidationRule{ // has requires field based access + { + Expression: `has(claims.turtle.foo.bar.baz.username__dot__io)`, + }, + { + Expression: `has(claims.turtle.foo.bar.baz.link[0].uid__dot__io)`, + }, + { + Expression: `has(claims.data[0].groups__dot__io)`, + }, + { + Expression: `claims.turtle.foo.bar.bee.username__dot__io.company__dot__io == "fun-corp"`, + }, + }, + ClaimMappings: apiserver.ClaimMappings{ + Username: apiserver.PrefixedClaimOrExpression{ + Expression: `claims.turtle.foo.bar.baz.username__dot__io`, + }, + UID: apiserver.ClaimOrExpression{ + Expression: `claims.turtle.foo.bar.baz.link[0].uid__dot__io`, + }, + Groups: apiserver.PrefixedClaimOrExpression{ + Expression: `claims.data[0].groups__dot__io`, + }, + }, + }, + now: func() time.Time { return now }, + }, + signingKey: loadRSAPrivKey(t, "testdata/rsa_1.pem", jose.RS256), + pubKeys: []*jose.JSONWebKey{ + loadRSAKey(t, "testdata/rsa_1.pem", jose.RS256), + }, + claims: fmt.Sprintf(`{ + "iss": "https://auth.example.com", + "aud": "my-client", + "exp": %d, + "data": [ + { + "groups.io": "gen1" + } + ], + "turtle": { + "foo": { + "1": "a", + "2": "b", + "other1": { + "bit1": true, + "bit2": false, + "bit3": 1 + }, + "bar": { + "3": "c", + "4": "d", + "other1": { + "bit1": true, + "bit2": false, + "bit3": 1 + }, + "bee": { + "username.io": { + "company.io": "fun-corp" + } + }, + "baz": { + "5": "e", + "6": "f", + "panda": [ + "snorlax", + "007", + "santa", + "claus" + ], + "other1": { + "bit1": true, + "bit2": false, + "bit3": 1 + }, + "username.io": "snorlax.io", + "link": [ + { + "uid.io": "143" + } + ] + } + } + } + } +}`, valid.Unix()), + want: &user.DefaultInfo{ + Name: "snorlax.io", + UID: "143", + Groups: []string{"gen1"}, + }, + }, { name: "groups claim mapping with expression", options: Options{