From 525f42a3f5c711391b2e6df05637209ac1c05def Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" Date: Wed, 10 Sep 2025 16:22:34 +0200 Subject: [PATCH 1/3] UPSTREAM: : Add authentication.kcp.io/scopes to service accounts Signed-off-by: Nelo-T. Wallus Signed-off-by: Nelo-T. Wallus --- .../k8s.io/apiserver/pkg/authentication/serviceaccount/util.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util.go b/staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util.go index 9bcdbd4d8e3d5..e113ced880c4b 100644 --- a/staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util.go +++ b/staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util.go @@ -52,6 +52,8 @@ const ( NodeUIDKey = "authentication.kubernetes.io/node-uid" // ClusterNameKey is the logical cluster name this service-account comes from. ClusterNameKey = "authentication.kcp.io/cluster-name" + // ClusterScopeKey additionally sets the scope for the service-account to the cluster it belongs to. + ClusterScopeKey = "authentication.kcp.io/scopes" ) // MakeUsername generates a username from the given namespace and ServiceAccount name. @@ -170,6 +172,7 @@ func (sa *ServiceAccountInfo) UserInfo() user.Info { info.Extra = map[string][]string{} } info.Extra[ClusterNameKey] = []string{sa.ClusterName.String()} + info.Extra[ClusterScopeKey] = []string{"cluster:" + sa.ClusterName.String()} return info } From 3dfd955349ec0a149a8d44b36eb8f60768781ec7 Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" Date: Wed, 10 Sep 2025 16:24:54 +0200 Subject: [PATCH 2/3] UPSTREAM: : Add system:cluster: group to effective users Signed-off-by: Nelo-T. Wallus Signed-off-by: Nelo-T. Wallus --- pkg/registry/rbac/validation/kcp.go | 78 ++++++++++++--- pkg/registry/rbac/validation/kcp_test.go | 119 +++++++++++++++++++++-- 2 files changed, 177 insertions(+), 20 deletions(-) diff --git a/pkg/registry/rbac/validation/kcp.go b/pkg/registry/rbac/validation/kcp.go index 55d4a761d044f..4766fb7b5f5b6 100644 --- a/pkg/registry/rbac/validation/kcp.go +++ b/pkg/registry/rbac/validation/kcp.go @@ -3,6 +3,7 @@ package validation import ( "context" "fmt" + "slices" "strings" "github.com/kcp-dev/logicalcluster/v3" @@ -32,6 +33,10 @@ const ( // are and'ed. ScopeExtraKey = "authentication.kcp.io/scopes" + // ClusterExtraKey is the key used in a user's "extra" to specify + // the origin cluster of the user. + ClusterExtraKey = "authentication.kcp.io/cluster-name" + // ClusterPrefix is the prefix for cluster scopes. clusterPrefix = "cluster:" ) @@ -79,6 +84,35 @@ func withScopesAndWarrants(appliesToUser appliesToUserFunc) appliesToUserFuncCtx } } +// EffectiveScopes takes the scopes of a user and returns the +// intersection of all scopes. +func EffectiveScopes(in []string) []string { + if len(in) == 0 { + return in + } + + effectiveScopes := strings.Split(in[0], ",") + if len(effectiveScopes) == 0 || len(in) == 1 { + return effectiveScopes + } + + for _, further := range in[1:] { + furtherScopes := strings.Split(further, ",") + newEffective := make([]string, 0, len(effectiveScopes)) + for _, es := range effectiveScopes { + if slices.Contains(furtherScopes, es) { + newEffective = append(newEffective, es) + } + } + effectiveScopes = newEffective + if len(effectiveScopes) == 0 { + return []string{} + } + } + + return effectiveScopes +} + var ( authenticated = &user.DefaultInfo{Name: user.Anonymous, Groups: []string{user.AllAuthenticated}} unauthenticated = &user.DefaultInfo{Name: user.Anonymous, Groups: []string{user.AllUnauthenticated}} @@ -96,6 +130,10 @@ func EffectiveUsers(clusterName logicalcluster.Name, u user.Info) []user.Info { recursive = func(u user.Info) { if IsInScope(u, clusterName) { ret = append(ret, u) + // TODO(ntnn): Add system:cluster: group? + // Though the user is in scope so it shouldn't need the + // group added but it might be better for consistency? + // Leaving it out for now but could be revisited. } else { found := false for _, g := range u.GetGroups() { @@ -119,17 +157,29 @@ func EffectiveUsers(clusterName logicalcluster.Name, u user.Info) []user.Info { if g == user.AllAuthenticated { rewritten.Groups = append(rewritten.Groups, user.AllAuthenticated) } - // system:cluster:* groups are used to allow - // controlling binding APIExports between workspaces - // via RBAC - if strings.HasPrefix(g, "system:cluster:") { - rewritten.Groups = append(rewritten.Groups, g) + } + // Add the system:cluster: group if the SA has + // an effective scope for the cluster the effective + // users are calculated for. + for _, g := range EffectiveScopes(u.GetExtra()[ScopeExtraKey]) { + if strings.HasPrefix(g, clusterPrefix) { + rewritten.Groups = append(rewritten.Groups, "system:"+g) } } ret = append(ret, rewritten) } } + if len(u.GetExtra()[WarrantExtraKey]) > 0 && len(u.GetExtra()[ClusterExtraKey]) > 0 && !IsServiceAccount(u) { + // Users that are not SAs and have a cluster extra key + // cannot have warrants, as they would be coming from + // per-workspace authentication. + // If this happens it is either a misconfiguration from the + // admin or a malicious actor that tries to escalate + // privileges through warrants. + return + } + for _, v := range u.GetExtra()[WarrantExtraKey] { var w Warrant if err := json.Unmarshal([]byte(v), &w); err != nil { @@ -143,9 +193,17 @@ func EffectiveUsers(clusterName logicalcluster.Name, u user.Info) []user.Info { Extra: w.Extra, } if IsServiceAccount(wu) && len(w.Extra[authserviceaccount.ClusterNameKey]) == 0 { - // warrants must be scoped to a cluster + // For SAs warrants must be scoped to a cluster. continue } + // Add the system:cluster: group if the warrant has + // an effective scope for the cluster the effective + // users are calculated for. + for _, g := range EffectiveScopes(wu.GetExtra()[ScopeExtraKey]) { + if strings.HasPrefix(g, clusterPrefix) { + wu.Groups = append(wu.Groups, "system:"+g) + } + } recursive(wu) } } @@ -156,11 +214,9 @@ func EffectiveUsers(clusterName logicalcluster.Name, u user.Info) []user.Info { Name: user.Anonymous, Groups: []string{user.AllAuthenticated}, } - for _, g := range u.GetGroups() { - // system:cluster:* groups are used to allow controlling - // binding APIExports between workspaces via RBAC - if strings.HasPrefix(g, "system:cluster:") { - authed.Groups = append(authed.Groups, g) + for _, g := range EffectiveScopes(u.GetExtra()[ScopeExtraKey]) { + if strings.HasPrefix(g, clusterPrefix) { + authed.Groups = append(authed.Groups, "system:"+g) } } ret = append(ret, authed) diff --git a/pkg/registry/rbac/validation/kcp_test.go b/pkg/registry/rbac/validation/kcp_test.go index 5ee4113a84d6c..a40111ca0f334 100644 --- a/pkg/registry/rbac/validation/kcp_test.go +++ b/pkg/registry/rbac/validation/kcp_test.go @@ -172,12 +172,30 @@ func TestAppliesToUserWithWarrantsAndScopes(t *testing.T) { sub: rbacv1.Subject{Kind: "User", Name: "user-a"}, want: true, }, + { + name: "simple matching user with warrants and from this cluster", + user: &user.DefaultInfo{Name: "user-a", Extra: map[string][]string{ + WarrantExtraKey: {`{"user":"user-b"}`}, + ClusterExtraKey: {"this"}, + }}, + sub: rbacv1.Subject{Kind: "User", Name: "user-a"}, + want: true, // user is subject + }, { name: "simple non-matching user with matching warrants", user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{WarrantExtraKey: {`{"user":"user-a"}`}}}, sub: rbacv1.Subject{Kind: "User", Name: "user-a"}, want: true, }, + { + name: "simple non-matching user with matching warrants but with cluster-name", + user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{ + WarrantExtraKey: {`{"user":"user-a"}`}, + ClusterExtraKey: {"this"}, + }}, + sub: rbacv1.Subject{Kind: "User", Name: "user-a"}, + want: false, // Warrants are ineffective on users with cluster + }, { name: "simple non-matching user with non-matching warrants", user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{WarrantExtraKey: {`{"user":"user-b"}`}}}, @@ -190,6 +208,15 @@ func TestAppliesToUserWithWarrantsAndScopes(t *testing.T) { sub: rbacv1.Subject{Kind: "User", Name: "user-a"}, want: true, }, + { + name: "simple non-matching user with multiple warrants and cluster-name", + user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{ + WarrantExtraKey: {`{"user":"user-b"}`, `{"user":"user-a"}`, `{"user":"user-c"}`}, + ClusterExtraKey: {"this"}, + }}, + sub: rbacv1.Subject{Kind: "User", Name: "user-a"}, + want: false, // Warrants are ineffective on users with cluster + }, { name: "simple non-matching user with nested warrants", user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{WarrantExtraKey: {`{"user":"user-b","extra":{"authorization.kcp.io/warrant":["{\"user\":\"user-a\"}"]}}`}}}, @@ -206,18 +233,18 @@ func TestAppliesToUserWithWarrantsAndScopes(t *testing.T) { }, { name: "non-cluster-aware service account with this scope", - user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/scopes": {"cluster:this"}}}, + user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{ScopeExtraKey: {"cluster:this"}}}, sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"}, want: true, }, { name: "non-cluster-aware service account with other scope", - user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/scopes": {"cluster:other"}}}, + user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{ScopeExtraKey: {"cluster:other"}}}, sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"}, want: false, }, { - name: "non-cluster-aware service account as warrant", + name: "non-cluster-aware service account as warrant", // TODO what is this supposed to test? user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{WarrantExtraKey: {`{"user":"system:serviceaccount:ns:sa"}`}}}, sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"}, want: false, @@ -226,37 +253,46 @@ func TestAppliesToUserWithWarrantsAndScopes(t *testing.T) { // service accounts with cluster { name: "local service account", - user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/cluster-name": {"this"}}}, + user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{ClusterExtraKey: {"this"}}}, sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"}, want: true, }, { name: "foreign service account", - user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/cluster-name": {"other"}}}, + user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{ClusterExtraKey: {"other"}}}, sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"}, want: false, }, { name: "foreign service account with local warrant", - user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/cluster-name": {"other"}, WarrantExtraKey: {`{"user":"system:serviceaccount:ns:sa","extra":{"authentication.kcp.io/cluster-name":["this"]}}`}}}, + user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{ + ClusterExtraKey: {"other"}, + WarrantExtraKey: {`{"user":"system:serviceaccount:ns:sa","extra":{"authentication.kcp.io/cluster-name":["this"]}}`}, + }}, sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"}, want: true, }, { name: "foreign service account with foreign warrant", - user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/cluster-name": {"other"}, WarrantExtraKey: {`{"user":"system:serviceaccount:ns:sa","extra":{"authentication.kcp.io/cluster-name":["other"]}}`}}}, + user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{ + ClusterExtraKey: {"other"}, + WarrantExtraKey: {`{"user":"system:serviceaccount:ns:sa","extra":{"authentication.kcp.io/cluster-name":["other"]}}`}, + }}, sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"}, want: false, }, { name: "local service account with multiple clusters", - user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/cluster-name": {"this", "this"}}}, + user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{ClusterExtraKey: {"this", "this"}}}, sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"}, want: false, }, { name: "out-of-scope local service account", - user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/cluster-name": {"this"}, "authentication.kcp.io/scopes": {"cluster:other"}}}, + user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{ + ClusterExtraKey: {"this"}, + ScopeExtraKey: {"cluster:other"}, + }}, sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"}, want: false, }, @@ -487,3 +523,68 @@ func TestPrefixUser(t *testing.T) { }) } } + +func TestEffectiveUsers(t *testing.T) { + tests := map[string]struct { + in []string + want []string + }{ + "empty": { + in: []string{}, + want: []string{}, + }, + "one scope entry, one cluster": { + in: []string{"cluster:this"}, + want: []string{"cluster:this"}, + }, + "one scope entry, multiple clusters": { + in: []string{"cluster:this,cluster:that"}, + want: []string{"cluster:this", "cluster:that"}, + }, + "multiple scope entries, multiple clusters, empty result": { + in: []string{ + "cluster:this,cluster:that", + "cluster:other", + }, + want: []string{}, + }, + "multiple scope entries, multiple clusters, non-empty result": { + in: []string{ + "cluster:this,cluster:that", + "cluster:other,cluster:this", + }, + want: []string{"cluster:this"}, + }, + "multiple scopes entries, multiple clusters, multiple others": { + in: []string{ + "cluster:this,foo:bar", + "cluster:this,cluster:other,foo:bar", + "cluster:third,foo:bar,foo:baz", + }, + want: []string{ + "foo:bar", + }, + }, + "multiple equal scopes entries": { + in: []string{ + "cluster:this,cluster:other,foo:bar", + "cluster:this,cluster:other,foo:bar", + "cluster:this,cluster:other,foo:bar", + }, + want: []string{ + "cluster:this", + "cluster:other", + "foo:bar", + }, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + got := EffectiveScopes(tt.in) + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Errorf("EffectiveScopes() mismatch (-want +got):\n%s", diff) + } + }) + } +} From 74a7d5aacf87d69a39d65c2b2763afad0c752983 Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" <10514301+ntnn@users.noreply.github.com> Date: Mon, 15 Sep 2025 10:41:22 +0200 Subject: [PATCH 3/3] Update pkg/registry/rbac/validation/kcp_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/registry/rbac/validation/kcp_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/registry/rbac/validation/kcp_test.go b/pkg/registry/rbac/validation/kcp_test.go index a40111ca0f334..1bffbc58d0049 100644 --- a/pkg/registry/rbac/validation/kcp_test.go +++ b/pkg/registry/rbac/validation/kcp_test.go @@ -244,7 +244,7 @@ func TestAppliesToUserWithWarrantsAndScopes(t *testing.T) { want: false, }, { - name: "non-cluster-aware service account as warrant", // TODO what is this supposed to test? + name: "non-cluster-aware service account as warrant", user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{WarrantExtraKey: {`{"user":"system:serviceaccount:ns:sa"}`}}}, sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"}, want: false,