diff --git a/docs.md b/docs.md index e018db059..db2692818 100644 --- a/docs.md +++ b/docs.md @@ -51,7 +51,9 @@ If yes, the webhook redacts the role, so that it only grants a deletion permissi #### Escalation Prevention -Users can only create/update ClusterRoleTemplateBindings which grant permissions to RoleTemplates with rights less than or equal to those they currently possess. This is to prevent privilege escalation. +Users can only create/update ClusterRoleTemplateBindings which grant permissions to RoleTemplates with rights less than or equal to those they currently possess. This is to prevent privilege escalation. +For external RoleTemplates (RoleTemplates with `external` set to `true`), if the `external-rules` feature flag is enabled and `ExternalRules` is specified in the roleTemplate in `RoleTemplateName`, +`ExternalRules` will be used for authorization. Otherwise (if the feature flag is off or `ExternalRules` are nil), the rules from the backing `ClusterRole` in the local cluster will be used. #### Invalid Fields - Create @@ -90,6 +92,7 @@ In addition, as in the create validation, both a user subject and a group subjec #### On update The desired value must not change on new spec unless it's equal to the `lockedValue` or `lockedValue` is nil. +Due to the security impact of the `external-rules` feature flag, only users with admin permissions (`*` verbs on `*` resources in `*` APIGroups in all namespaces) can enable or disable this feature flag. ## FleetWorkspace @@ -207,6 +210,8 @@ Adds the authz.management.cattle.io/creator-role-bindings annotation. Users can only create/update ProjectRoleTemplateBindings with rights less than or equal to those they currently possess. This is to prevent privilege escalation. +For external RoleTemplates (RoleTemplates with `external` set to `true`), if the `external-rules` feature flag is enabled and `ExternalRules` is specified in the roleTemplate in `RoleTemplateName`, +`ExternalRules` will be used for authorization. Otherwise, if `ExternalRules` are nil when the feature flag is on, the rules from the backing `ClusterRole` in the local cluster will be used. #### Invalid Fields - Create @@ -256,11 +261,12 @@ Circular references to a `RoleTemplate` (a inherits b, b inherits a) are not all #### Rules Without Verbs, Resources, API groups -Rules without verbs, resources, or apigroups are not permitted. The `rules` included in a RoleTemplate are of the same type as the rules used by standard Kubernetes RBAC types (such as `Roles` from `rbac.authorization.k8s.io/v1`). Because of this, they inherit the same restrictions as these types, including this one. +Rules without verbs, resources, or apigroups are not permitted. The `rules` and `externalRules` included in a RoleTemplate are of the same type as the rules used by standard Kubernetes RBAC types (such as `Roles` from `rbac.authorization.k8s.io/v1`). Because of this, they inherit the same restrictions as these types, including this one. #### Escalation Prevention Users can only change RoleTemplates with rights less than or equal to those they currently possess. This prevents privilege escalation. +Users can't create external RoleTemplates (or update existing RoleTemplates) with `ExternalRules` without having the `escalate` verb on that RoleTemplate. #### Context Validation diff --git a/go.mod b/go.mod index 704a5baaf..e8b1deb2a 100644 --- a/go.mod +++ b/go.mod @@ -38,8 +38,8 @@ require ( github.com/gorilla/mux v1.8.0 github.com/rancher/dynamiclistener v0.4.0-rc2 github.com/rancher/lasso v0.0.0-20240123150939-7055397d6dfa - github.com/rancher/rancher/pkg/apis v0.0.0-20240507213626-07f244b8be3a - github.com/rancher/rke v1.5.9-rc2 + github.com/rancher/rancher/pkg/apis v0.0.0-20240611034301-19a4362e2243 + github.com/rancher/rke v1.5.9 github.com/rancher/wrangler/v2 v2.1.4 github.com/robfig/cron v1.2.0 github.com/sirupsen/logrus v1.9.3 diff --git a/go.sum b/go.sum index 4ea07ea69..cc17ad6ff 100644 --- a/go.sum +++ b/go.sum @@ -299,10 +299,10 @@ github.com/rancher/lasso v0.0.0-20240123150939-7055397d6dfa h1:eRhvQJjIpPxJunlS3 github.com/rancher/lasso v0.0.0-20240123150939-7055397d6dfa/go.mod h1:utdskbIL7kdVvPCUFPEJQDWJwPHGFpUCRfVkX2G2Xxg= github.com/rancher/norman v0.0.0-20240206180703-6eda4bc94b4c h1:ayqZqJ4AYYVaZGlBztLBij81z/QRsSFbQfxs9bzA+Tg= github.com/rancher/norman v0.0.0-20240206180703-6eda4bc94b4c/go.mod h1:WbNpu/HwChwKk54W0rWBdioxYVVZwVVz//UX84m/NvY= -github.com/rancher/rancher/pkg/apis v0.0.0-20240507213626-07f244b8be3a h1:D1X2mAW6LZdkZhQbmtwCKNND7jl2v+m3V4VuwaB+BiE= -github.com/rancher/rancher/pkg/apis v0.0.0-20240507213626-07f244b8be3a/go.mod h1:RZjaN8KMbRMF9zfpYEOkWUuwy6uZ6R/FKqK/gwOXnaQ= -github.com/rancher/rke v1.5.9-rc2 h1:DCovi6z3Q+GlxRy3mIRSR+cqLWoHD1OKOOdnR8HCaYg= -github.com/rancher/rke v1.5.9-rc2/go.mod h1:vojhOf8U8VCmw7y17OENWXSIfEFPEbXCMQcmI7xN7i8= +github.com/rancher/rancher/pkg/apis v0.0.0-20240611034301-19a4362e2243 h1:lDdMkc0klQ/0jA8l3QWRzKpK21nO6E4qIagaOBq2KdA= +github.com/rancher/rancher/pkg/apis v0.0.0-20240611034301-19a4362e2243/go.mod h1:5FRNHrG/G92wZ7XYKr9eFl/Sf2XqHown8CozHk+pmmU= +github.com/rancher/rke v1.5.9 h1:sCD2vWtkUQEQmNWyne7akZXuu4gZw+3vZVf8VRALXOE= +github.com/rancher/rke v1.5.9/go.mod h1:/z9oyKqYpFwgRBV9rfLxqUdjydz/VMCTcjld4uUt7uM= github.com/rancher/wrangler/v2 v2.1.4 h1:ohov0i6A9dJHHO6sjfsH4Dqv93ZTdm5lIJVJdPzVdQc= github.com/rancher/wrangler/v2 v2.1.4/go.mod h1:af5OaGU/COgreQh1mRbKiUI64draT2NN34uk+PALFY8= github.com/robfig/cron v1.2.0 h1:ZjScXvvxeQ63Dbyxy76Fj3AT3Ut0aKsyd2/tl3DTMuQ= diff --git a/pkg/auth/globalrole_test.go b/pkg/auth/globalrole_test.go index 83b38dca4..907bc37e9 100644 --- a/pkg/auth/globalrole_test.go +++ b/pkg/auth/globalrole_test.go @@ -153,7 +153,7 @@ func TestGlobalRulesFromRole(t *testing.T) { if test.stateSetup != nil { test.stateSetup(state) } - grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCacheMock, nil), nil) + grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCacheMock, nil, nil), nil) rules := grResolver.GlobalRulesFromRole(test.globalRole) require.Len(t, rules, len(test.wantRules)) @@ -264,7 +264,7 @@ func TestClusterRulesFromRole(t *testing.T) { if test.stateSetup != nil { test.stateSetup(state) } - grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCacheMock, nil), nil) + grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCacheMock, nil, nil), nil) rules, err := grResolver.ClusterRulesFromRole(test.globalRole) if test.wantErr { require.Error(t, err) @@ -337,7 +337,7 @@ func TestGetRoleTemplatesForGlobalRole(t *testing.T) { if test.stateSetup != nil { test.stateSetup(state) } - grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCacheMock, nil), nil) + grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCacheMock, nil, nil), nil) roleTemplates, err := grResolver.GetRoleTemplatesForGlobalRole(test.globalRole) if test.wantErr { require.Error(t, err) diff --git a/pkg/auth/roleTemplate.go b/pkg/auth/roleTemplate.go index a668a50e0..5b5d190e6 100644 --- a/pkg/auth/roleTemplate.go +++ b/pkg/auth/roleTemplate.go @@ -9,17 +9,21 @@ import ( rbacv1 "k8s.io/api/rbac/v1" ) +const ExternalRulesFeature = "external-rules" + // RoleTemplateResolver provides an interface to flatten role templates into slice of rules. type RoleTemplateResolver struct { roleTemplates v3.RoleTemplateCache clusterRoles v1.ClusterRoleCache + features v3.FeatureCache } // NewRoleTemplateResolver creates a newly allocated RoleTemplateResolver from the provided caches -func NewRoleTemplateResolver(roleTemplates v3.RoleTemplateCache, clusterRoles v1.ClusterRoleCache) *RoleTemplateResolver { +func NewRoleTemplateResolver(roleTemplates v3.RoleTemplateCache, clusterRoles v1.ClusterRoleCache, features v3.FeatureCache) *RoleTemplateResolver { return &RoleTemplateResolver{ roleTemplates: roleTemplates, clusterRoles: clusterRoles, + features: features, } } @@ -58,12 +62,29 @@ func (r *RoleTemplateResolver) RulesFromTemplate(roleTemplate *rancherv3.RoleTem func (r *RoleTemplateResolver) gatherRules(roleTemplate *rancherv3.RoleTemplate, rules []rbacv1.PolicyRule, seen map[string]bool) ([]rbacv1.PolicyRule, error) { seen[roleTemplate.Name] = true - if roleTemplate.External && roleTemplate.Context == "cluster" { - cr, err := r.clusterRoles.Get(roleTemplate.Name) + if roleTemplate.External { + externalRulesEnabled, err := r.isExternalRulesFeatureFlagEnabled() if err != nil { - return nil, fmt.Errorf("failed to get clusterRoles '%s': %w", roleTemplate.Name, err) + return nil, fmt.Errorf("failed to check externalRules feature flag: %w", err) + } + + if externalRulesEnabled { + if roleTemplate.ExternalRules != nil { + rules = append(rules, roleTemplate.ExternalRules...) + } else { + cr, err := r.clusterRoles.Get(roleTemplate.Name) + if err != nil { + return nil, fmt.Errorf("for external RoleTemplates, externalRules must be provided or a backing clusterRole must be installed to check for privilege escalations: failed to get ClusterRole %q: %w", roleTemplate.Name, err) + } + rules = append(rules, cr.Rules...) + } + } else if roleTemplate.Context == "cluster" { + cr, err := r.clusterRoles.Get(roleTemplate.Name) + if err != nil { + return nil, fmt.Errorf("failed to get ClusterRole %q: %w", roleTemplate.Name, err) + } + rules = append(rules, cr.Rules...) } - rules = append(rules, cr.Rules...) } rules = append(rules, roleTemplate.Rules...) @@ -84,3 +105,14 @@ func (r *RoleTemplateResolver) gatherRules(roleTemplate *rancherv3.RoleTemplate, } return rules, nil } + +func (r *RoleTemplateResolver) isExternalRulesFeatureFlagEnabled() (bool, error) { + f, err := r.features.Get(ExternalRulesFeature) + if err != nil { + return false, err + } + if f.Spec.Value == nil { + return f.Status.Default, nil + } + return *f.Spec.Value, nil +} diff --git a/pkg/auth/roleTemplate_test.go b/pkg/auth/roleTemplate_test.go index 94dc8d94d..b74a88469 100644 --- a/pkg/auth/roleTemplate_test.go +++ b/pkg/auth/roleTemplate_test.go @@ -54,14 +54,18 @@ func (r Rules) Equal(r2 Rules) bool { type RoleTemplateResolverSuite struct { suite.Suite - adminRT *apisv3.RoleTemplate - readNodesRT *apisv3.RoleTemplate - writeNodesRT *apisv3.RoleTemplate - inheritedRT *apisv3.RoleTemplate - externalRT *apisv3.RoleTemplate - invalidInhertedRT *apisv3.RoleTemplate + adminRT *apisv3.RoleTemplate + readNodesRT *apisv3.RoleTemplate + writeNodesRT *apisv3.RoleTemplate + inheritedRT *apisv3.RoleTemplate + externalRulesClusterRT *apisv3.RoleTemplate + externalRulesProjectRT *apisv3.RoleTemplate + externalClusterRT *apisv3.RoleTemplate + externalProjectRT *apisv3.RoleTemplate + invalidInhertedRT *apisv3.RoleTemplate readServiceCR *rbacv1.ClusterRole + writeNodesCR *rbacv1.ClusterRole } func TestRoleTemplateResolver(t *testing.T) { @@ -93,6 +97,10 @@ func (r *RoleTemplateResolverSuite) SetupSuite() { ObjectMeta: metav1.ObjectMeta{Name: "read-services"}, Rules: []rbacv1.PolicyRule{ruleReadServices}, } + r.writeNodesCR = &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{Name: "write-nodes"}, + Rules: []rbacv1.PolicyRule{ruleWriteNodes}, + } r.readNodesRT = &apisv3.RoleTemplate{ ObjectMeta: metav1.ObjectMeta{ @@ -121,7 +129,25 @@ func (r *RoleTemplateResolverSuite) SetupSuite() { Administrative: true, Context: "cluster", } - r.externalRT = &apisv3.RoleTemplate{ + r.externalRulesClusterRT = &apisv3.RoleTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: r.readServiceCR.Name, + }, + DisplayName: "External Role", + Context: "cluster", + External: true, + ExternalRules: []rbacv1.PolicyRule{ruleWriteNodes}, + } + r.externalRulesProjectRT = &apisv3.RoleTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: r.readServiceCR.Name, + }, + DisplayName: "External Role", + Context: "project", + External: true, + ExternalRules: []rbacv1.PolicyRule{ruleWriteNodes}, + } + r.externalClusterRT = &apisv3.RoleTemplate{ ObjectMeta: metav1.ObjectMeta{ Name: r.readServiceCR.Name, }, @@ -129,7 +155,14 @@ func (r *RoleTemplateResolverSuite) SetupSuite() { Context: "cluster", External: true, } - + r.externalProjectRT = &apisv3.RoleTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: r.readServiceCR.Name, + }, + DisplayName: "External Role", + Context: "project", + External: true, + } r.inheritedRT = &apisv3.RoleTemplate{ ObjectMeta: metav1.ObjectMeta{ Name: "inherited-role", @@ -153,7 +186,7 @@ func (r *RoleTemplateResolverSuite) SetupSuite() { func (r *RoleTemplateResolverSuite) TestRoleTemplateResolver() { type args struct { name string - caches func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache) + caches func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) } tests := []struct { name string @@ -165,12 +198,13 @@ func (r *RoleTemplateResolverSuite) TestRoleTemplateResolver() { { name: "Test simple Role Template", args: args{ - caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache) { + caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) { ctrl := gomock.NewController(r.T()) roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) roleTemplateCache.EXPECT().Get(r.adminRT.Name).Return(r.adminRT, nil).AnyTimes() clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) - return roleTemplateCache, clusterRoleCache + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + return roleTemplateCache, clusterRoleCache, featuresCache }, name: r.adminRT.Name, }, @@ -181,47 +215,319 @@ func (r *RoleTemplateResolverSuite) TestRoleTemplateResolver() { { name: "Test inherited Role Templates", args: args{ - caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache) { + caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) { ctrl := gomock.NewController(r.T()) roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) roleTemplateCache.EXPECT().Get(r.inheritedRT.Name).Return(r.inheritedRT, nil) roleTemplateCache.EXPECT().Get(r.readNodesRT.Name).Return(r.readNodesRT, nil) roleTemplateCache.EXPECT().Get(r.writeNodesRT.Name).Return(r.writeNodesRT, nil) clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) - return roleTemplateCache, clusterRoleCache + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + return roleTemplateCache, clusterRoleCache, featuresCache }, name: r.inheritedRT.Name, }, want: append(r.inheritedRT.Rules, append(r.readNodesRT.Rules, r.writeNodesRT.Rules...)...), wantErr: false, }, - // Get external role { - name: "Test external cluster role", + name: "Test externalRules (context=cluster) when feature flag is set to true", + args: args{ + caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) { + ctrl := gomock.NewController(r.T()) + roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) + roleTemplateCache.EXPECT().Get(r.externalRulesClusterRT.Name).Return(r.externalRulesClusterRT, nil) + clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + featuresCache.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + Spec: apisv3.FeatureSpec{ + Value: &[]bool{true}[0], + }, + Status: apisv3.FeatureStatus{ + Default: false, + }, + }, nil) + return roleTemplateCache, clusterRoleCache, featuresCache + }, + name: r.externalRulesClusterRT.Name, + }, + want: r.writeNodesCR.Rules, + wantErr: false, + }, + { + name: "Test externalRules (context=project) when feature flag is set to true", + args: args{ + caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) { + ctrl := gomock.NewController(r.T()) + roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) + roleTemplateCache.EXPECT().Get(r.externalRulesProjectRT.Name).Return(r.externalRulesProjectRT, nil) + clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + featuresCache.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + Spec: apisv3.FeatureSpec{ + Value: &[]bool{true}[0], + }, + Status: apisv3.FeatureStatus{ + Default: false, + }, + }, nil) + return roleTemplateCache, clusterRoleCache, featuresCache + }, + name: r.externalRulesProjectRT.Name, + }, + want: r.writeNodesCR.Rules, + wantErr: false, + }, + { + name: "Test externalRules (context=project) when feature flag is set to false", + args: args{ + caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) { + ctrl := gomock.NewController(r.T()) + roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) + roleTemplateCache.EXPECT().Get(r.externalRulesProjectRT.Name).Return(r.externalRulesProjectRT, nil) + clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + featuresCache.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + Spec: apisv3.FeatureSpec{ + Value: &[]bool{false}[0], + }, + }, nil) + return roleTemplateCache, clusterRoleCache, featuresCache + }, + name: r.externalRulesProjectRT.Name, + }, + want: nil, + wantErr: false, + }, + { + name: "Test externalRules (context=cluster) when feature flag is set to false", + args: args{ + caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) { + ctrl := gomock.NewController(r.T()) + roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) + roleTemplateCache.EXPECT().Get(r.externalRulesClusterRT.Name).Return(r.externalRulesClusterRT, nil) + clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) + clusterRoleCache.EXPECT().Get(r.readServiceCR.Name).Return(r.readServiceCR, nil) + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + featuresCache.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + Spec: apisv3.FeatureSpec{ + Value: &[]bool{false}[0], + }, + }, nil) + return roleTemplateCache, clusterRoleCache, featuresCache + }, + name: r.externalRulesClusterRT.Name, + }, + want: r.readServiceCR.Rules, + wantErr: false, + }, + { + name: "Test external cluster role (context=cluster) when feature flag defaults to true", + args: args{ + caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) { + ctrl := gomock.NewController(r.T()) + roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) + roleTemplateCache.EXPECT().Get(r.externalClusterRT.Name).Return(r.externalClusterRT, nil) + clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) + clusterRoleCache.EXPECT().Get(r.readServiceCR.Name).Return(r.readServiceCR, nil) + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + featuresCache.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + Status: apisv3.FeatureStatus{ + Default: true, + }, + }, nil) + return roleTemplateCache, clusterRoleCache, featuresCache + }, + name: r.externalClusterRT.Name, + }, + want: r.readServiceCR.Rules, + wantErr: false, + }, + { + name: "Test external cluster role (context=project) when feature flag defaults to true", args: args{ - caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache) { + caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) { ctrl := gomock.NewController(r.T()) roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) - roleTemplateCache.EXPECT().Get(r.externalRT.Name).Return(r.externalRT, nil) + roleTemplateCache.EXPECT().Get(r.externalProjectRT.Name).Return(r.externalProjectRT, nil) clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) clusterRoleCache.EXPECT().Get(r.readServiceCR.Name).Return(r.readServiceCR, nil) - return roleTemplateCache, clusterRoleCache + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + featuresCache.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + Status: apisv3.FeatureStatus{ + Default: true, + }, + }, nil) + return roleTemplateCache, clusterRoleCache, featuresCache }, - name: r.externalRT.Name, + name: r.externalProjectRT.Name, }, want: r.readServiceCR.Rules, wantErr: false, }, + { + name: "Test external cluster role (context=cluster) when feature flag is set to false", + args: args{ + caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) { + ctrl := gomock.NewController(r.T()) + roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) + roleTemplateCache.EXPECT().Get(r.externalClusterRT.Name).Return(r.externalClusterRT, nil) + clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) + clusterRoleCache.EXPECT().Get(r.readServiceCR.Name).Return(r.readServiceCR, nil) + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + featuresCache.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + Spec: apisv3.FeatureSpec{ + Value: &[]bool{false}[0], + }, + Status: apisv3.FeatureStatus{ + Default: true, + }, + }, nil) + return roleTemplateCache, clusterRoleCache, featuresCache + }, + name: r.externalClusterRT.Name, + }, + want: r.readServiceCR.Rules, + wantErr: false, + }, + { + name: "Test external cluster role (context=project) when feature flag is set to false", + args: args{ + caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) { + ctrl := gomock.NewController(r.T()) + roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) + roleTemplateCache.EXPECT().Get(r.externalProjectRT.Name).Return(r.externalProjectRT, nil) + clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + featuresCache.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + Spec: apisv3.FeatureSpec{ + Value: &[]bool{false}[0], + }, + Status: apisv3.FeatureStatus{ + Default: true, + }, + }, nil) + return roleTemplateCache, clusterRoleCache, featuresCache + }, + name: r.externalProjectRT.Name, + }, + want: nil, + wantErr: false, + }, + { + name: "Test non-existing external cluster role (context=cluster) when feature flag is set to true", + args: args{ + caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) { + ctrl := gomock.NewController(r.T()) + roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) + roleTemplateCache.EXPECT().Get(r.externalClusterRT.Name).Return(r.externalClusterRT, nil) + clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) + clusterRoleCache.EXPECT().Get(r.readServiceCR.Name).Return(nil, errExpected) + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + featuresCache.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + Spec: apisv3.FeatureSpec{ + Value: &[]bool{true}[0], + }, + Status: apisv3.FeatureStatus{ + Default: true, + }, + }, nil) + return roleTemplateCache, clusterRoleCache, featuresCache + }, + name: r.externalClusterRT.Name, + }, + wantErr: true, + }, + { + name: "Test non-existing external cluster role (context=cluster) when feature flag is set to false", + args: args{ + caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) { + ctrl := gomock.NewController(r.T()) + roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) + roleTemplateCache.EXPECT().Get(r.externalClusterRT.Name).Return(r.externalClusterRT, nil) + clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) + clusterRoleCache.EXPECT().Get(r.readServiceCR.Name).Return(nil, errExpected) + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + featuresCache.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + Spec: apisv3.FeatureSpec{ + Value: &[]bool{false}[0], + }, + }, nil) + return roleTemplateCache, clusterRoleCache, featuresCache + }, + name: r.externalClusterRT.Name, + }, + wantErr: true, + }, + { + name: "Test non-existing external cluster role (context=project) when feature flag is set to true", + args: args{ + caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) { + ctrl := gomock.NewController(r.T()) + roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) + roleTemplateCache.EXPECT().Get(r.externalProjectRT.Name).Return(r.externalProjectRT, nil) + clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) + clusterRoleCache.EXPECT().Get(r.readServiceCR.Name).Return(nil, errExpected) + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + featuresCache.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + Spec: apisv3.FeatureSpec{ + Value: &[]bool{true}[0], + }, + }, nil) + return roleTemplateCache, clusterRoleCache, featuresCache + }, + name: r.externalProjectRT.Name, + }, + wantErr: true, + }, + { + name: "Test err getting ClusterRole when feature flag is disabled", + args: args{ + caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) { + ctrl := gomock.NewController(r.T()) + roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) + roleTemplateCache.EXPECT().Get(r.externalClusterRT.Name).Return(r.externalClusterRT, nil) + clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) + clusterRoleCache.EXPECT().Get(r.readServiceCR.Name).Return(nil, errExpected) + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + featuresCache.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + Spec: apisv3.FeatureSpec{ + Value: &[]bool{false}[0], + }, + }, nil) + return roleTemplateCache, clusterRoleCache, featuresCache + }, + name: r.externalClusterRT.Name, + }, + wantErr: true, + }, + { + name: "Test err getting feature flag", + args: args{ + caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) { + ctrl := gomock.NewController(r.T()) + roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) + roleTemplateCache.EXPECT().Get(r.externalClusterRT.Name).Return(r.externalClusterRT, nil) + clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + featuresCache.EXPECT().Get(auth.ExternalRulesFeature).Return(nil, errExpected) + return roleTemplateCache, clusterRoleCache, featuresCache + }, + name: r.externalClusterRT.Name, + }, + wantErr: true, + }, // Get unknown role { name: "Test invalid template name", args: args{ - caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache) { + caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) { ctrl := gomock.NewController(r.T()) roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) roleTemplateCache.EXPECT().Get(invalidName).Return(nil, errExpected) clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) - return roleTemplateCache, clusterRoleCache + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + return roleTemplateCache, clusterRoleCache, featuresCache }, name: invalidName, }, @@ -232,13 +538,14 @@ func (r *RoleTemplateResolverSuite) TestRoleTemplateResolver() { { name: "Test invalid inherted template name", args: args{ - caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache) { + caches: func() (v3.RoleTemplateCache, wrbacv1.ClusterRoleCache, v3.FeatureCache) { ctrl := gomock.NewController(r.T()) roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) roleTemplateCache.EXPECT().Get(r.invalidInhertedRT.Name).Return(r.invalidInhertedRT, nil) roleTemplateCache.EXPECT().Get(invalidName).Return(nil, errExpected) clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) - return roleTemplateCache, clusterRoleCache + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + return roleTemplateCache, clusterRoleCache, featuresCache }, name: r.invalidInhertedRT.Name, }, @@ -269,6 +576,7 @@ func (r *RoleTemplateResolverSuite) TestGetCache() { ctrl := gomock.NewController(r.T()) roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) - resolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache) + featuresCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + resolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache, featuresCache) r.Equal(resolver.RoleTemplateCache(), roleTemplateCache, "Resolver did not correctly return cache") } diff --git a/pkg/clients/clients.go b/pkg/clients/clients.go index 27639cca8..ca61e4e90 100644 --- a/pkg/clients/clients.go +++ b/pkg/clients/clients.go @@ -66,7 +66,7 @@ func New(ctx context.Context, rest *rest.Config, mcmEnabled bool) (*Clients, err } if mcmEnabled { - result.RoleTemplateResolver = auth.NewRoleTemplateResolver(mgmt.Management().V3().RoleTemplate().Cache(), clients.RBAC.ClusterRole().Cache()) + result.RoleTemplateResolver = auth.NewRoleTemplateResolver(mgmt.Management().V3().RoleTemplate().Cache(), clients.RBAC.ClusterRole().Cache(), mgmt.Management().V3().Feature().Cache()) result.GlobalRoleResolver = auth.NewGlobalRoleResolver(result.RoleTemplateResolver, mgmt.Management().V3().GlobalRole().Cache()) } diff --git a/pkg/codegen/main.go b/pkg/codegen/main.go index 083e2ec30..a043ba17e 100644 --- a/pkg/codegen/main.go +++ b/pkg/codegen/main.go @@ -43,6 +43,7 @@ func main() { v3.ProjectRoleTemplateBinding{}, v3.Node{}, v3.Project{}, + v3.Feature{}, }, }, "provisioning.cattle.io": { diff --git a/pkg/generated/controllers/management.cattle.io/v3/feature.go b/pkg/generated/controllers/management.cattle.io/v3/feature.go new file mode 100644 index 000000000..e8c7877be --- /dev/null +++ b/pkg/generated/controllers/management.cattle.io/v3/feature.go @@ -0,0 +1,208 @@ +/* +Copyright 2024 Rancher Labs, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by codegen. DO NOT EDIT. + +package v3 + +import ( + "context" + "sync" + "time" + + v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" + "github.com/rancher/wrangler/v2/pkg/apply" + "github.com/rancher/wrangler/v2/pkg/condition" + "github.com/rancher/wrangler/v2/pkg/generic" + "github.com/rancher/wrangler/v2/pkg/kv" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// FeatureController interface for managing Feature resources. +type FeatureController interface { + generic.NonNamespacedControllerInterface[*v3.Feature, *v3.FeatureList] +} + +// FeatureClient interface for managing Feature resources in Kubernetes. +type FeatureClient interface { + generic.NonNamespacedClientInterface[*v3.Feature, *v3.FeatureList] +} + +// FeatureCache interface for retrieving Feature resources in memory. +type FeatureCache interface { + generic.NonNamespacedCacheInterface[*v3.Feature] +} + +// FeatureStatusHandler is executed for every added or modified Feature. Should return the new status to be updated +type FeatureStatusHandler func(obj *v3.Feature, status v3.FeatureStatus) (v3.FeatureStatus, error) + +// FeatureGeneratingHandler is the top-level handler that is executed for every Feature event. It extends FeatureStatusHandler by a returning a slice of child objects to be passed to apply.Apply +type FeatureGeneratingHandler func(obj *v3.Feature, status v3.FeatureStatus) ([]runtime.Object, v3.FeatureStatus, error) + +// RegisterFeatureStatusHandler configures a FeatureController to execute a FeatureStatusHandler for every events observed. +// If a non-empty condition is provided, it will be updated in the status conditions for every handler execution +func RegisterFeatureStatusHandler(ctx context.Context, controller FeatureController, condition condition.Cond, name string, handler FeatureStatusHandler) { + statusHandler := &featureStatusHandler{ + client: controller, + condition: condition, + handler: handler, + } + controller.AddGenericHandler(ctx, name, generic.FromObjectHandlerToHandler(statusHandler.sync)) +} + +// RegisterFeatureGeneratingHandler configures a FeatureController to execute a FeatureGeneratingHandler for every events observed, passing the returned objects to the provided apply.Apply. +// If a non-empty condition is provided, it will be updated in the status conditions for every handler execution +func RegisterFeatureGeneratingHandler(ctx context.Context, controller FeatureController, apply apply.Apply, + condition condition.Cond, name string, handler FeatureGeneratingHandler, opts *generic.GeneratingHandlerOptions) { + statusHandler := &featureGeneratingHandler{ + FeatureGeneratingHandler: handler, + apply: apply, + name: name, + gvk: controller.GroupVersionKind(), + } + if opts != nil { + statusHandler.opts = *opts + } + controller.OnChange(ctx, name, statusHandler.Remove) + RegisterFeatureStatusHandler(ctx, controller, condition, name, statusHandler.Handle) +} + +type featureStatusHandler struct { + client FeatureClient + condition condition.Cond + handler FeatureStatusHandler +} + +// sync is executed on every resource addition or modification. Executes the configured handlers and sends the updated status to the Kubernetes API +func (a *featureStatusHandler) sync(key string, obj *v3.Feature) (*v3.Feature, error) { + if obj == nil { + return obj, nil + } + + origStatus := obj.Status.DeepCopy() + obj = obj.DeepCopy() + newStatus, err := a.handler(obj, obj.Status) + if err != nil { + // Revert to old status on error + newStatus = *origStatus.DeepCopy() + } + + if a.condition != "" { + if errors.IsConflict(err) { + a.condition.SetError(&newStatus, "", nil) + } else { + a.condition.SetError(&newStatus, "", err) + } + } + if !equality.Semantic.DeepEqual(origStatus, &newStatus) { + if a.condition != "" { + // Since status has changed, update the lastUpdatedTime + a.condition.LastUpdated(&newStatus, time.Now().UTC().Format(time.RFC3339)) + } + + var newErr error + obj.Status = newStatus + newObj, newErr := a.client.UpdateStatus(obj) + if err == nil { + err = newErr + } + if newErr == nil { + obj = newObj + } + } + return obj, err +} + +type featureGeneratingHandler struct { + FeatureGeneratingHandler + apply apply.Apply + opts generic.GeneratingHandlerOptions + gvk schema.GroupVersionKind + name string + seen sync.Map +} + +// Remove handles the observed deletion of a resource, cascade deleting every associated resource previously applied +func (a *featureGeneratingHandler) Remove(key string, obj *v3.Feature) (*v3.Feature, error) { + if obj != nil { + return obj, nil + } + + obj = &v3.Feature{} + obj.Namespace, obj.Name = kv.RSplit(key, "/") + obj.SetGroupVersionKind(a.gvk) + + if a.opts.UniqueApplyForResourceVersion { + a.seen.Delete(key) + } + + return nil, generic.ConfigureApplyForObject(a.apply, obj, &a.opts). + WithOwner(obj). + WithSetID(a.name). + ApplyObjects() +} + +// Handle executes the configured FeatureGeneratingHandler and pass the resulting objects to apply.Apply, finally returning the new status of the resource +func (a *featureGeneratingHandler) Handle(obj *v3.Feature, status v3.FeatureStatus) (v3.FeatureStatus, error) { + if !obj.DeletionTimestamp.IsZero() { + return status, nil + } + + objs, newStatus, err := a.FeatureGeneratingHandler(obj, status) + if err != nil { + return newStatus, err + } + if !a.isNewResourceVersion(obj) { + return newStatus, nil + } + + err = generic.ConfigureApplyForObject(a.apply, obj, &a.opts). + WithOwner(obj). + WithSetID(a.name). + ApplyObjects(objs...) + if err != nil { + return newStatus, err + } + a.storeResourceVersion(obj) + return newStatus, nil +} + +// isNewResourceVersion detects if a specific resource version was already successfully processed. +// Only used if UniqueApplyForResourceVersion is set in generic.GeneratingHandlerOptions +func (a *featureGeneratingHandler) isNewResourceVersion(obj *v3.Feature) bool { + if !a.opts.UniqueApplyForResourceVersion { + return true + } + + // Apply once per resource version + key := obj.Namespace + "/" + obj.Name + previous, ok := a.seen.Load(key) + return !ok || previous != obj.ResourceVersion +} + +// storeResourceVersion keeps track of the latest resource version of an object for which Apply was executed +// Only used if UniqueApplyForResourceVersion is set in generic.GeneratingHandlerOptions +func (a *featureGeneratingHandler) storeResourceVersion(obj *v3.Feature) { + if !a.opts.UniqueApplyForResourceVersion { + return + } + + key := obj.Namespace + "/" + obj.Name + a.seen.Store(key, obj.ResourceVersion) +} diff --git a/pkg/generated/controllers/management.cattle.io/v3/interface.go b/pkg/generated/controllers/management.cattle.io/v3/interface.go index 5233282a7..b975a8476 100644 --- a/pkg/generated/controllers/management.cattle.io/v3/interface.go +++ b/pkg/generated/controllers/management.cattle.io/v3/interface.go @@ -33,6 +33,7 @@ func init() { type Interface interface { Cluster() ClusterController ClusterRoleTemplateBinding() ClusterRoleTemplateBindingController + Feature() FeatureController GlobalRole() GlobalRoleController GlobalRoleBinding() GlobalRoleBindingController Node() NodeController @@ -60,6 +61,10 @@ func (v *version) ClusterRoleTemplateBinding() ClusterRoleTemplateBindingControl return generic.NewController[*v3.ClusterRoleTemplateBinding, *v3.ClusterRoleTemplateBindingList](schema.GroupVersionKind{Group: "management.cattle.io", Version: "v3", Kind: "ClusterRoleTemplateBinding"}, "clusterroletemplatebindings", true, v.controllerFactory) } +func (v *version) Feature() FeatureController { + return generic.NewNonNamespacedController[*v3.Feature, *v3.FeatureList](schema.GroupVersionKind{Group: "management.cattle.io", Version: "v3", Kind: "Feature"}, "features", v.controllerFactory) +} + func (v *version) GlobalRole() GlobalRoleController { return generic.NewNonNamespacedController[*v3.GlobalRole, *v3.GlobalRoleList](schema.GroupVersionKind{Group: "management.cattle.io", Version: "v3", Kind: "GlobalRole"}, "globalroles", v.controllerFactory) } diff --git a/pkg/resolvers/mockAuthRuleResolver_test.go b/pkg/mocks/authRuleResolver.go similarity index 99% rename from pkg/resolvers/mockAuthRuleResolver_test.go rename to pkg/mocks/authRuleResolver.go index cbb8599d2..ff32d82ef 100644 --- a/pkg/resolvers/mockAuthRuleResolver_test.go +++ b/pkg/mocks/authRuleResolver.go @@ -2,7 +2,7 @@ // Source: k8s.io/kubernetes/pkg/registry/rbac/validation (interfaces: AuthorizationRuleResolver) // Package resolvers is a generated GoMock package. -package resolvers +package mocks import ( fmt "fmt" diff --git a/pkg/resolvers/aggregateResolver_test.go b/pkg/resolvers/aggregateResolver_test.go index 21d9cc444..3d5cb6bba 100644 --- a/pkg/resolvers/aggregateResolver_test.go +++ b/pkg/resolvers/aggregateResolver_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/golang/mock/gomock" + "github.com/rancher/webhook/pkg/mocks" "github.com/stretchr/testify/suite" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apiserver/pkg/authentication/user" @@ -55,7 +56,7 @@ func (a *AggregateResolverSuite) TestAggregateRuleResolverGetRules() { namespace: testNameSpace, resolvers: func(t *testing.T) ([]validation.AuthorizationRuleResolver, Rules) { expectedRules := []rbacv1.PolicyRule{a.ruleAdmin} - resolver := NewMockAuthorizationRuleResolver(gomock.NewController(t)) + resolver := mocks.NewMockAuthorizationRuleResolver(gomock.NewController(t)) resolver.EXPECT().VisitRulesFor(testUser, testNameSpace, gomock.Any()). DoAndReturn(func(_ user.Info, _ string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool) bool { for _, rule := range expectedRules { @@ -74,13 +75,13 @@ func (a *AggregateResolverSuite) TestAggregateRuleResolverGetRules() { wantErr: true, resolvers: func(t *testing.T) ([]validation.AuthorizationRuleResolver, Rules) { expectedRules := []rbacv1.PolicyRule{} - resolver := NewMockAuthorizationRuleResolver(gomock.NewController(t)) + resolver := mocks.NewMockAuthorizationRuleResolver(gomock.NewController(t)) resolver.EXPECT().VisitRulesFor(gomock.Any(), testNameSpace, gomock.Any()). DoAndReturn(func(_ user.Info, _ string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool) bool { visitor(nil, nil, errNotFound) return true }) - resolver2 := NewMockAuthorizationRuleResolver(gomock.NewController(t)) + resolver2 := mocks.NewMockAuthorizationRuleResolver(gomock.NewController(t)) resolver2.EXPECT().VisitRulesFor(gomock.Any(), testNameSpace, gomock.Any()). DoAndReturn(func(_ user.Info, _ string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool) bool { visitor(nil, nil, errNotFound) @@ -95,13 +96,13 @@ func (a *AggregateResolverSuite) TestAggregateRuleResolverGetRules() { namespace: testNameSpace, resolvers: func(t *testing.T) ([]validation.AuthorizationRuleResolver, Rules) { expectedRules := []rbacv1.PolicyRule{a.ruleReadPods} - resolver := NewMockAuthorizationRuleResolver(gomock.NewController(t)) + resolver := mocks.NewMockAuthorizationRuleResolver(gomock.NewController(t)) resolver.EXPECT().VisitRulesFor(testUser, testNameSpace, gomock.Any()). DoAndReturn(func(_ user.Info, _ string, _ func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool) bool { return true }) resolver.EXPECT().GetRoleReferenceRules(gomock.Any(), gomock.Any()).Return(expectedRules, nil) - resolver2 := NewMockAuthorizationRuleResolver(gomock.NewController(t)) + resolver2 := mocks.NewMockAuthorizationRuleResolver(gomock.NewController(t)) resolver2.EXPECT().VisitRulesFor(testUser, testNameSpace, gomock.Any()). DoAndReturn(func(_ user.Info, _ string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool) bool { for _, rule := range expectedRules { @@ -120,7 +121,7 @@ func (a *AggregateResolverSuite) TestAggregateRuleResolverGetRules() { resolvers: func(t *testing.T) ([]validation.AuthorizationRuleResolver, Rules) { expectedRules1 := []rbacv1.PolicyRule{a.ruleAdmin} expectedRules2 := []rbacv1.PolicyRule{a.ruleReadPods} - resolver := NewMockAuthorizationRuleResolver(gomock.NewController(t)) + resolver := mocks.NewMockAuthorizationRuleResolver(gomock.NewController(t)) resolver.EXPECT().VisitRulesFor(testUser, testNameSpace, gomock.Any()). DoAndReturn(func(_ user.Info, _ string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool) bool { for _, rule := range expectedRules1 { @@ -129,7 +130,7 @@ func (a *AggregateResolverSuite) TestAggregateRuleResolverGetRules() { return true }) resolver.EXPECT().GetRoleReferenceRules(gomock.Any(), gomock.Any()).Return(expectedRules1, nil) - resolver2 := NewMockAuthorizationRuleResolver(gomock.NewController(t)) + resolver2 := mocks.NewMockAuthorizationRuleResolver(gomock.NewController(t)) resolver2.EXPECT().VisitRulesFor(testUser, testNameSpace, gomock.Any()). DoAndReturn(func(_ user.Info, _ string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool) bool { for _, rule := range expectedRules2 { diff --git a/pkg/resolvers/crtbResolver_test.go b/pkg/resolvers/crtbResolver_test.go index 946879975..8b128f576 100644 --- a/pkg/resolvers/crtbResolver_test.go +++ b/pkg/resolvers/crtbResolver_test.go @@ -278,7 +278,7 @@ func (c *CRTBResolverSuite) NewTestCRTBResolver() *CRTBRuleResolver { roleTemplateCache.EXPECT().Get(c.readRT.Name).Return(c.readRT, nil).AnyTimes() roleTemplateCache.EXPECT().Get(c.writeRT.Name).Return(c.writeRT, nil).AnyTimes() roleTemplateCache.EXPECT().Get(invalidName).Return(nil, errNotFound).AnyTimes() - roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache) + roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache, nil) return NewCRTBRuleResolver(crtbCache, roleResolver) } diff --git a/pkg/resolvers/grbClusterResolver_test.go b/pkg/resolvers/grbClusterResolver_test.go index ba50688a0..7fc39f252 100644 --- a/pkg/resolvers/grbClusterResolver_test.go +++ b/pkg/resolvers/grbClusterResolver_test.go @@ -324,7 +324,7 @@ func (g *GRBClusterRuleResolverSuite) TestGRBClusterRuleResolver() { test.setup(state) } - grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCache, nil), state.grCache) + grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCache, nil, nil), state.grCache) grbResolver := NewGRBClusterRuleResolver(state.grbCache, grResolver) rules, err := grbResolver.RulesFor(g.userInfo, test.namespace) diff --git a/pkg/resolvers/prtbResolver_test.go b/pkg/resolvers/prtbResolver_test.go index 9de9c699d..0d6454525 100644 --- a/pkg/resolvers/prtbResolver_test.go +++ b/pkg/resolvers/prtbResolver_test.go @@ -281,7 +281,7 @@ func (p *PRTBResolverSuite) NewTestPRTBResolver() *PRTBRuleResolver { roleTemplateCache.EXPECT().Get(p.readRT.Name).Return(p.readRT, nil).AnyTimes() roleTemplateCache.EXPECT().Get(p.writeRT.Name).Return(p.writeRT, nil).AnyTimes() roleTemplateCache.EXPECT().Get(invalidName).Return(nil, errNotFound).AnyTimes() - roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache) + roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache, nil) return NewPRTBRuleResolver(PRTBCache, roleResolver) } diff --git a/pkg/resources/management.cattle.io/v3/clusterroletemplatebinding/ClusterRoleTemplateBinding.md b/pkg/resources/management.cattle.io/v3/clusterroletemplatebinding/ClusterRoleTemplateBinding.md index 591b70290..039406637 100644 --- a/pkg/resources/management.cattle.io/v3/clusterroletemplatebinding/ClusterRoleTemplateBinding.md +++ b/pkg/resources/management.cattle.io/v3/clusterroletemplatebinding/ClusterRoleTemplateBinding.md @@ -2,7 +2,9 @@ ### Escalation Prevention -Users can only create/update ClusterRoleTemplateBindings which grant permissions to RoleTemplates with rights less than or equal to those they currently possess. This is to prevent privilege escalation. +Users can only create/update ClusterRoleTemplateBindings which grant permissions to RoleTemplates with rights less than or equal to those they currently possess. This is to prevent privilege escalation. +For external RoleTemplates (RoleTemplates with `external` set to `true`), if the `external-rules` feature flag is enabled and `ExternalRules` is specified in the roleTemplate in `RoleTemplateName`, +`ExternalRules` will be used for authorization. Otherwise (if the feature flag is off or `ExternalRules` are nil), the rules from the backing `ClusterRole` in the local cluster will be used. ### Invalid Fields - Create diff --git a/pkg/resources/management.cattle.io/v3/clusterroletemplatebinding/validator_test.go b/pkg/resources/management.cattle.io/v3/clusterroletemplatebinding/validator_test.go index 3d4d7ced5..74d16c489 100644 --- a/pkg/resources/management.cattle.io/v3/clusterroletemplatebinding/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/clusterroletemplatebinding/validator_test.go @@ -34,13 +34,16 @@ const ( type ClusterRoleTemplateBindingSuite struct { suite.Suite - adminRT *apisv3.RoleTemplate - readNodesRT *apisv3.RoleTemplate - lockedRT *apisv3.RoleTemplate - projectRT *apisv3.RoleTemplate - adminCR *rbacv1.ClusterRole - writeNodeCR *rbacv1.ClusterRole - readServiceRole *rbacv1.Role + adminRT *apisv3.RoleTemplate + readNodesRT *apisv3.RoleTemplate + lockedRT *apisv3.RoleTemplate + projectRT *apisv3.RoleTemplate + externalRulesWriteNodesRT *apisv3.RoleTemplate + externalClusterRoleRT *v3.RoleTemplate + adminCR *rbacv1.ClusterRole + writeNodeCR *rbacv1.ClusterRole + readPodsCR *rbacv1.ClusterRole + readServiceRole *rbacv1.Role } func TestClusterRoleTemplateBindings(t *testing.T) { @@ -87,6 +90,27 @@ func (c *ClusterRoleTemplateBindingSuite) SetupSuite() { Administrative: true, Context: "cluster", } + c.externalRulesWriteNodesRT = &apisv3.RoleTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "external-rule-write-nodes", + }, + DisplayName: "External Role", + ExternalRules: []rbacv1.PolicyRule{ruleWriteNodes}, + External: true, + Builtin: true, + Administrative: true, + Context: "cluster", + } + c.externalClusterRoleRT = &apisv3.RoleTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "read-pods-role", + }, + DisplayName: "External Role", + External: true, + Builtin: true, + Administrative: true, + Context: "cluster", + } c.lockedRT = &apisv3.RoleTemplate{ ObjectMeta: metav1.ObjectMeta{ Name: "locked-role", @@ -118,6 +142,10 @@ func (c *ClusterRoleTemplateBindingSuite) SetupSuite() { ObjectMeta: metav1.ObjectMeta{Namespace: "namespace1", Name: "read-service"}, Rules: []rbacv1.PolicyRule{ruleReadServices}, } + c.readPodsCR = &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{Name: "read-pods-role"}, + Rules: []rbacv1.PolicyRule{ruleReadPods}, + } } func (c *ClusterRoleTemplateBindingSuite) Test_PrivilegeEscalation() { @@ -151,7 +179,8 @@ func (c *ClusterRoleTemplateBindingSuite) Test_PrivilegeEscalation() { roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*v3.RoleTemplate](ctrl) roleTemplateCache.EXPECT().Get(c.adminRT.Name).Return(c.adminRT, nil).AnyTimes() clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) - roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache) + featuresCache := fake.NewMockNonNamespacedCacheInterface[*v3.Feature](ctrl) + roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache, featuresCache) crtbCache := fake.NewMockCacheInterface[*apisv3.ClusterRoleTemplateBinding](ctrl) crtbCache.EXPECT().AddIndexer(gomock.Any(), gomock.Any()) crtbCache.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(crtbUser, newDefaultCRTB().ClusterName)).Return([]*apisv3.ClusterRoleTemplateBinding{ @@ -301,7 +330,8 @@ func (c *ClusterRoleTemplateBindingSuite) Test_UpdateValidation() { roleTemplateCache.EXPECT().Get(c.adminRT.Name).Return(c.adminRT, nil).AnyTimes() roleTemplateCache.EXPECT().List(gomock.Any()).Return([]*apisv3.RoleTemplate{c.adminRT}, nil).AnyTimes() clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) - roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache) + featuresCache := fake.NewMockNonNamespacedCacheInterface[*v3.Feature](ctrl) + roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache, featuresCache) crtbCache := fake.NewMockCacheInterface[*apisv3.ClusterRoleTemplateBinding](ctrl) crtbCache.EXPECT().AddIndexer(gomock.Any(), gomock.Any()) crtbCache.EXPECT().GetByIndex(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() @@ -623,12 +653,19 @@ func (c *ClusterRoleTemplateBindingSuite) Test_UpdateValidation() { } func (c *ClusterRoleTemplateBindingSuite) Test_Create() { + type testState struct { + featureCacheMock *fake.MockNonNamespacedCacheInterface[*apisv3.Feature] + clusterRoleCacheMock *fake.MockNonNamespacedCacheInterface[*rbacv1.ClusterRole] + } + ctrl := gomock.NewController(c.T()) const adminUser = "admin-userid" + const writeNodeUser = "write-node-userid" + const readPodUser = "read-pod-userid" const badRoleTemplateName = "bad-roletemplate" const missingCluster = "missing-cluster" const errorCluster = "error-cluster" const nilCluster = "nil-cluster" - clusterRoles := []*rbacv1.ClusterRole{c.adminCR} + clusterRoles := []*rbacv1.ClusterRole{c.adminCR, c.writeNodeCR, c.readPodsCR} clusterRoleBindings := []*rbacv1.ClusterRoleBinding{ { Subjects: []rbacv1.Subject{ @@ -636,6 +673,18 @@ func (c *ClusterRoleTemplateBindingSuite) Test_Create() { }, RoleRef: rbacv1.RoleRef{APIGroup: rbacv1.GroupName, Kind: "ClusterRole", Name: c.adminCR.Name}, }, + { + Subjects: []rbacv1.Subject{ + {Kind: rbacv1.UserKind, Name: writeNodeUser}, + }, + RoleRef: rbacv1.RoleRef{APIGroup: rbacv1.GroupName, Kind: "ClusterRole", Name: c.writeNodeCR.Name}, + }, + { + Subjects: []rbacv1.Subject{ + {Kind: rbacv1.UserKind, Name: readPodUser}, + }, + RoleRef: rbacv1.RoleRef{APIGroup: rbacv1.GroupName, Kind: "ClusterRole", Name: c.readPodsCR.Name}, + }, } validGRB := v3.GlobalRoleBinding{ @@ -654,57 +703,59 @@ func (c *ClusterRoleTemplateBindingSuite) Test_Create() { GlobalRoleName: "some-gr", } - resolver, _ := validation.NewTestRuleResolver(nil, nil, clusterRoles, clusterRoleBindings) - - ctrl := gomock.NewController(c.T()) - roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*v3.RoleTemplate](ctrl) - roleTemplateCache.EXPECT().Get(c.adminRT.Name).Return(c.adminRT, nil).AnyTimes() - roleTemplateCache.EXPECT().Get(c.lockedRT.Name).Return(c.lockedRT, nil).AnyTimes() - roleTemplateCache.EXPECT().Get(c.projectRT.Name).Return(c.projectRT, nil).AnyTimes() - expectedError := apierrors.NewNotFound(schema.GroupResource{}, "") - roleTemplateCache.EXPECT().Get(badRoleTemplateName).Return(nil, expectedError).AnyTimes() - roleTemplateCache.EXPECT().Get("").Return(nil, expectedError).AnyTimes() - clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) - roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache) - crtbCache := fake.NewMockCacheInterface[*apisv3.ClusterRoleTemplateBinding](ctrl) - crtbCache.EXPECT().AddIndexer(gomock.Any(), gomock.Any()) - crtbCache.EXPECT().GetByIndex(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() - grbCache := fake.NewMockNonNamespacedCacheInterface[*v3.GlobalRoleBinding](ctrl) - notFoundError := apierrors.NewNotFound(schema.GroupResource{ - Group: "management.cattle.io", - Resource: "globalrolebindings", - }, "not-found") - grbCache.EXPECT().Get(validGRB.Name).Return(&validGRB, nil).AnyTimes() - grbCache.EXPECT().Get(deletingGRB.Name).Return(&deletingGRB, nil).AnyTimes() - grbCache.EXPECT().Get("error").Return(nil, fmt.Errorf("server not available")).AnyTimes() - grbCache.EXPECT().Get("not-found").Return(nil, notFoundError).AnyTimes() - grbCache.EXPECT().Get("nil-grb").Return(nil, nil).AnyTimes() + validatorWithMocks := func(state testState) *clusterroletemplatebinding.Validator { + resolver, _ := validation.NewTestRuleResolver(nil, nil, clusterRoles, clusterRoleBindings) + roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*v3.RoleTemplate](ctrl) + roleTemplateCache.EXPECT().Get(c.adminRT.Name).Return(c.adminRT, nil).AnyTimes() + roleTemplateCache.EXPECT().Get(c.externalRulesWriteNodesRT.Name).Return(c.externalRulesWriteNodesRT, nil).AnyTimes() + roleTemplateCache.EXPECT().Get(c.externalClusterRoleRT.Name).Return(c.externalClusterRoleRT, nil).AnyTimes() + roleTemplateCache.EXPECT().Get(c.lockedRT.Name).Return(c.lockedRT, nil).AnyTimes() + roleTemplateCache.EXPECT().Get(c.projectRT.Name).Return(c.projectRT, nil).AnyTimes() + expectedError := apierrors.NewNotFound(schema.GroupResource{}, "") + roleTemplateCache.EXPECT().Get(badRoleTemplateName).Return(nil, expectedError).AnyTimes() + roleTemplateCache.EXPECT().Get("").Return(nil, expectedError).AnyTimes() + roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, state.clusterRoleCacheMock, state.featureCacheMock) + crtbCache := fake.NewMockCacheInterface[*apisv3.ClusterRoleTemplateBinding](ctrl) + crtbCache.EXPECT().AddIndexer(gomock.Any(), gomock.Any()) + crtbCache.EXPECT().GetByIndex(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + grbCache := fake.NewMockNonNamespacedCacheInterface[*v3.GlobalRoleBinding](ctrl) + notFoundError := apierrors.NewNotFound(schema.GroupResource{ + Group: "management.cattle.io", + Resource: "globalrolebindings", + }, "not-found") + grbCache.EXPECT().Get(validGRB.Name).Return(&validGRB, nil).AnyTimes() + grbCache.EXPECT().Get(deletingGRB.Name).Return(&deletingGRB, nil).AnyTimes() + grbCache.EXPECT().Get("error").Return(nil, fmt.Errorf("server not available")).AnyTimes() + grbCache.EXPECT().Get("not-found").Return(nil, notFoundError).AnyTimes() + grbCache.EXPECT().Get("nil-grb").Return(nil, nil).AnyTimes() - clusterCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Cluster](ctrl) - clusterCache.EXPECT().Get(defaultClusterID).Return(&apisv3.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: defaultClusterID, - }, - }, nil).AnyTimes() - clusterCache.EXPECT().Get(errorCluster).Return(nil, fmt.Errorf("server not available")).AnyTimes() - clusterCache.EXPECT().Get(missingCluster).Return(nil, apierrors.NewNotFound(schema.GroupResource{ - Group: "management.cattle.io", - Resource: "clusters", - }, missingCluster)).AnyTimes() - clusterCache.EXPECT().Get(nilCluster).Return(nil, nil).AnyTimes() + clusterCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Cluster](ctrl) + clusterCache.EXPECT().Get(defaultClusterID).Return(&apisv3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultClusterID, + }, + }, nil).AnyTimes() + clusterCache.EXPECT().Get(errorCluster).Return(nil, fmt.Errorf("server not available")).AnyTimes() + clusterCache.EXPECT().Get(missingCluster).Return(nil, apierrors.NewNotFound(schema.GroupResource{ + Group: "management.cattle.io", + Resource: "clusters", + }, missingCluster)).AnyTimes() + clusterCache.EXPECT().Get(nilCluster).Return(nil, nil).AnyTimes() - crtbResolver := resolvers.NewCRTBRuleResolver(crtbCache, roleResolver) - validator := clusterroletemplatebinding.NewValidator(crtbResolver, resolver, roleResolver, grbCache, clusterCache) + crtbResolver := resolvers.NewCRTBRuleResolver(crtbCache, roleResolver) + return clusterroletemplatebinding.NewValidator(crtbResolver, resolver, roleResolver, grbCache, clusterCache) + } type args struct { oldCRTB func() *apisv3.ClusterRoleTemplateBinding newCRTB func() *apisv3.ClusterRoleTemplateBinding username string } tests := []struct { - name string - args args - wantErr bool - allowed bool + name string + args args + wantErr bool + allowed bool + stateSetup func(state testState) }{ { name: "base test valid CRTB", @@ -970,6 +1021,104 @@ func (c *ClusterRoleTemplateBindingSuite) Test_Create() { }, allowed: false, }, + { + name: "external RT with externalRules valid CRTB creation when feature flag is on", + args: args{ + username: writeNodeUser, + oldCRTB: func() *apisv3.ClusterRoleTemplateBinding { + return nil + }, + newCRTB: func() *apisv3.ClusterRoleTemplateBinding { + baseCRTB := newDefaultCRTB() + baseCRTB.RoleTemplateName = "external-rule-write-nodes" + + return baseCRTB + }, + }, + stateSetup: func(state testState) { + state.featureCacheMock.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: apisv3.FeatureSpec{ + Value: &[]bool{true}[0], + }, + }, nil) + }, + allowed: true, + }, + { + name: "external RT with externalRules rejected when feature flag is on and there are not enough permissions", + args: args{ + username: readPodUser, + oldCRTB: func() *apisv3.ClusterRoleTemplateBinding { + return nil + }, + newCRTB: func() *apisv3.ClusterRoleTemplateBinding { + baseCRTB := newDefaultCRTB() + baseCRTB.RoleTemplateName = "external-rule-write-nodes" + + return baseCRTB + }, + }, + stateSetup: func(state testState) { + state.featureCacheMock.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: apisv3.FeatureSpec{ + Value: &[]bool{true}[0], + }, + }, nil) + }, + allowed: false, + }, + { + name: "external RT valid CRTB creation when feature flag is off", + args: args{ + username: adminUser, + oldCRTB: func() *apisv3.ClusterRoleTemplateBinding { + return nil + }, + newCRTB: func() *apisv3.ClusterRoleTemplateBinding { + baseCRTB := newDefaultCRTB() + baseCRTB.RoleTemplateName = "read-pods-role" + + return baseCRTB + }, + }, + stateSetup: func(state testState) { + state.featureCacheMock.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: apisv3.FeatureSpec{ + Value: &[]bool{false}[0], + }, + }, nil) + state.clusterRoleCacheMock.EXPECT().Get(c.readPodsCR.Name).Return(c.readPodsCR, nil) + }, + allowed: true, + }, + { + name: "external RT CRTB is rejected when there are not enough permissions when feature flag is off", + args: args{ + username: writeNodeUser, + oldCRTB: func() *apisv3.ClusterRoleTemplateBinding { + return nil + }, + newCRTB: func() *apisv3.ClusterRoleTemplateBinding { + basePRTB := newDefaultCRTB() + basePRTB.RoleTemplateName = "read-pods-role" + + return basePRTB + }, + }, + stateSetup: func(state testState) { + state.featureCacheMock.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: apisv3.FeatureSpec{ + Value: &[]bool{false}[0], + }, + }, nil) + state.clusterRoleCacheMock.EXPECT().Get(c.readPodsCR.Name).Return(c.readPodsCR, nil) + }, + allowed: false, + }, } for i := range tests { @@ -977,6 +1126,16 @@ func (c *ClusterRoleTemplateBindingSuite) Test_Create() { c.Run(test.name, func() { c.T().Parallel() req := createCRTBRequest(c.T(), test.args.oldCRTB(), test.args.newCRTB(), test.args.username) + featureCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) + state := testState{ + clusterRoleCacheMock: clusterRoleCache, + featureCacheMock: featureCache, + } + if test.stateSetup != nil { + test.stateSetup(state) + } + validator := validatorWithMocks(state) admitters := validator.Admitters() assert.Len(c.T(), admitters, 1) resp, err := admitters[0].Admit(req) diff --git a/pkg/resources/management.cattle.io/v3/feature/Feature.md b/pkg/resources/management.cattle.io/v3/feature/Feature.md index 076e155b0..702e69df4 100644 --- a/pkg/resources/management.cattle.io/v3/feature/Feature.md +++ b/pkg/resources/management.cattle.io/v3/feature/Feature.md @@ -3,3 +3,4 @@ ### On update The desired value must not change on new spec unless it's equal to the `lockedValue` or `lockedValue` is nil. +Due to the security impact of the `external-rules` feature flag, only users with admin permissions (`*` verbs on `*` resources in `*` APIGroups in all namespaces) can enable or disable this feature flag. diff --git a/pkg/resources/management.cattle.io/v3/feature/validator.go b/pkg/resources/management.cattle.io/v3/feature/validator.go index d455cb83d..b25a88fdd 100644 --- a/pkg/resources/management.cattle.io/v3/feature/validator.go +++ b/pkg/resources/management.cattle.io/v3/feature/validator.go @@ -6,11 +6,14 @@ import ( v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" "github.com/rancher/webhook/pkg/admission" + "github.com/rancher/webhook/pkg/auth" objectsv3 "github.com/rancher/webhook/pkg/generated/objects/management.cattle.io/v3" admissionv1 "k8s.io/api/admission/v1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/kubernetes/pkg/registry/rbac/validation" "k8s.io/utils/trace" ) @@ -26,9 +29,11 @@ type Validator struct { } // NewValidator returns a new validator for features. -func NewValidator() *Validator { +func NewValidator(ruleResolver validation.AuthorizationRuleResolver) *Validator { return &Validator{ - admitter: admitter{}, + admitter: admitter{ + ruleResolver: ruleResolver, + }, } } @@ -54,7 +59,9 @@ func (v *Validator) Admitters() []admission.Admitter { return []admission.Admitter{&v.admitter} } -type admitter struct{} +type admitter struct { + ruleResolver validation.AuthorizationRuleResolver +} // Admit handles the webhook admission request sent to this webhook. func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResponse, error) { @@ -78,11 +85,62 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp }, nil } + if newFeature.Name == auth.ExternalRulesFeature { + var rules []rbacv1.PolicyRule + oldFeatureValue := getEffectiveValue(oldFeature) + newFeatureValue := getEffectiveValue(newFeature) + + // if the feature value isn't changing, allow it + if oldFeatureValue == newFeatureValue { + return &admissionv1.AdmissionResponse{ + Allowed: true, + }, nil + } + + if !oldFeatureValue && newFeatureValue { + // enabling the feature requires the "security-enable" verb + rules = []rbacv1.PolicyRule{ + { + Verbs: []string{"security-enable"}, + APIGroups: []string{"management.cattle.io"}, + Resources: []string{"features"}, + ResourceNames: []string{"external-rules"}, + }, + } + } else { + // disabling the feature requires administrator permissions + rules = []rbacv1.PolicyRule{ + { + Verbs: []string{"*"}, + APIGroups: []string{"*"}, + Resources: []string{"*"}, + }, + } + } + err := auth.ConfirmNoEscalation(request, rules, "", a.ruleResolver) + if err != nil { + return admission.ResponseFailedEscalation(fmt.Sprintf("updating the 'external-rules' feature requires admin permissions: %s ", err.Error())), nil + } + } + return &admissionv1.AdmissionResponse{ Allowed: true, }, nil } +// getEffectiveValue considers a feature's default, value, and locked value to determine +// its effective value. +func getEffectiveValue(obj *v3.Feature) bool { + val := obj.Status.Default + if obj.Spec.Value != nil { + val = *obj.Spec.Value + } + if obj.Status.LockedValue != nil { + val = *obj.Status.LockedValue + } + return val +} + // isUpdateAllowed checks that the new value does not change on spec unless it's equal to the lockedValue, // or lockedValue is nil. func isUpdateAllowed(old, new *v3.Feature) bool { diff --git a/pkg/resources/management.cattle.io/v3/feature/validator_test.go b/pkg/resources/management.cattle.io/v3/feature/validator_test.go index 5d01fbcbe..71b66a963 100644 --- a/pkg/resources/management.cattle.io/v3/feature/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/feature/validator_test.go @@ -4,12 +4,16 @@ import ( "encoding/json" "testing" + "github.com/golang/mock/gomock" v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" "github.com/rancher/webhook/pkg/admission" + "github.com/rancher/webhook/pkg/auth" + "github.com/rancher/webhook/pkg/mocks" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" admissionv1 "k8s.io/api/admission/v1" authenicationv1 "k8s.io/api/authentication/v1" + rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -20,6 +24,11 @@ var ( ) func TestFeatureValueValid(t *testing.T) { + type testState struct { + authorizationRuleResolverMock *mocks.MockAuthorizationRuleResolver + } + ctrl := gomock.NewController(t) + t.Parallel() tests := []struct { name string @@ -27,6 +36,7 @@ func TestFeatureValueValid(t *testing.T) { oldFeature v3.Feature wantError bool wantAdmit bool + stateSetup func(state testState) }{ { name: "new feature locked with spec value changed", @@ -95,13 +105,319 @@ func TestFeatureValueValid(t *testing.T) { }, wantAdmit: true, }, + { + name: "external rules feature can be changed by admins", + oldFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: admission.Ptr(true), + }, + }, + newFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: admission.Ptr(false), + }, + }, + stateSetup: func(state testState) { + adminRules := []rbacv1.PolicyRule{ + { + Verbs: []string{"*"}, + APIGroups: []string{"*"}, + Resources: []string{"*"}, + }, + } + state.authorizationRuleResolverMock.EXPECT().RulesFor(gomock.Any(), gomock.Any()).Return(adminRules, nil) + }, + wantAdmit: true, + }, + { + name: "external rules feature can't be enabled by users with update permissions", + oldFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: admission.Ptr(false), + }, + }, + newFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: admission.Ptr(true), + }, + }, + stateSetup: func(state testState) { + adminRules := []rbacv1.PolicyRule{ + { + Verbs: []string{"update"}, + APIGroups: []string{"management.cattle.io"}, + Resources: []string{"features"}, + }, + } + state.authorizationRuleResolverMock.EXPECT().RulesFor(gomock.Any(), gomock.Any()).Return(adminRules, nil) + }, + wantAdmit: false, + }, + { + name: "external rules feature can't be disabled by users with update permissions", + oldFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: admission.Ptr(true), + }, + }, + newFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: admission.Ptr(false), + }, + }, + stateSetup: func(state testState) { + adminRules := []rbacv1.PolicyRule{ + { + Verbs: []string{"update"}, + APIGroups: []string{"management.cattle.io"}, + Resources: []string{"features"}, + }, + } + state.authorizationRuleResolverMock.EXPECT().RulesFor(gomock.Any(), gomock.Any()).Return(adminRules, nil) + }, + wantAdmit: false, + }, + { + name: "external rules feature can't be set to nil by users with update permissions", + oldFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: admission.Ptr(true), + }, + }, + newFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: nil, + }, + }, + stateSetup: func(state testState) { + adminRules := []rbacv1.PolicyRule{ + { + Verbs: []string{"update"}, + APIGroups: []string{"management.cattle.io"}, + Resources: []string{"features"}, + }, + } + state.authorizationRuleResolverMock.EXPECT().RulesFor(gomock.Any(), gomock.Any()).Return(adminRules, nil) + }, + wantAdmit: false, + }, + { + name: "external rules feature can be enabled for users with security-enable (RA)", + oldFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: admission.Ptr(false), + }, + }, + newFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: admission.Ptr(true), + }, + }, + stateSetup: func(state testState) { + state.authorizationRuleResolverMock.EXPECT().RulesFor(gomock.Any(), gomock.Any()).Return([]rbacv1.PolicyRule{ + { + Verbs: []string{"security-enable"}, + APIGroups: []string{"management.cattle.io"}, + Resources: []string{"features"}, + ResourceNames: []string{"external-rules"}, + }, + }, nil) + }, + wantAdmit: true, + }, + { + name: "external rules feature can't be disabled for users with security-enable (RA)", + oldFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: admission.Ptr(true), + }, + }, + newFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: admission.Ptr(false), + }, + }, + stateSetup: func(state testState) { + state.authorizationRuleResolverMock.EXPECT().RulesFor(gomock.Any(), gomock.Any()).Return([]rbacv1.PolicyRule{ + { + Verbs: []string{"security-enable", "update"}, + APIGroups: []string{"management.cattle.io"}, + Resources: []string{"features"}, + ResourceNames: []string{"external-rules"}, + }, + }, nil) + }, + wantAdmit: false, + }, + { + name: "external rules feature can be enabled with default value for users with security-enable (RA)", + oldFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: nil, + }, + Status: v3.FeatureStatus{ + Default: false, + }, + }, + newFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: admission.Ptr(true), + }, + }, + stateSetup: func(state testState) { + state.authorizationRuleResolverMock.EXPECT().RulesFor(gomock.Any(), gomock.Any()).Return([]rbacv1.PolicyRule{ + { + Verbs: []string{"security-enable"}, + APIGroups: []string{"management.cattle.io"}, + Resources: []string{"features"}, + ResourceNames: []string{"external-rules"}, + }, + }, nil) + }, + wantAdmit: true, + }, + { + name: "external rules feature can't be disabled for users with security-enable (RA)", + oldFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: nil, + }, + Status: v3.FeatureStatus{ + Default: true, + }, + }, + newFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: admission.Ptr(false), + }, + }, + stateSetup: func(state testState) { + state.authorizationRuleResolverMock.EXPECT().RulesFor(gomock.Any(), gomock.Any()).Return([]rbacv1.PolicyRule{ + { + Verbs: []string{"security-enable", "update"}, + APIGroups: []string{"management.cattle.io"}, + Resources: []string{"features"}, + ResourceNames: []string{"external-rules"}, + }, + }, nil) + }, + wantAdmit: false, + }, + { + name: "external rules feature can't be set to nil for users with security-enable (RA)", + oldFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: admission.Ptr(true), + }, + }, + newFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: nil, + }, + }, + stateSetup: func(state testState) { + state.authorizationRuleResolverMock.EXPECT().RulesFor(gomock.Any(), gomock.Any()).Return([]rbacv1.PolicyRule{ + { + Verbs: []string{"security-enable", "update"}, + APIGroups: []string{"management.cattle.io"}, + Resources: []string{"features"}, + ResourceNames: []string{"external-rules"}, + }, + }, nil) + }, + wantAdmit: false, + }, + { + name: "external rules feature can be modified if the value doesn't change", + oldFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + }, + Spec: v3.FeatureSpec{ + Value: admission.Ptr(true), + }, + }, + newFeature: v3.Feature{ + ObjectMeta: metav1.ObjectMeta{ + Name: auth.ExternalRulesFeature, + Annotations: map[string]string{ + "test-annotation": "test-value", + }, + }, + Spec: v3.FeatureSpec{ + Value: admission.Ptr(true), + }, + }, + wantAdmit: true, + }, } for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { t.Parallel() - admitters := NewValidator().Admitters() + authorizationRuleResolverMock := mocks.NewMockAuthorizationRuleResolver(ctrl) + state := testState{ + authorizationRuleResolverMock: authorizationRuleResolverMock, + } + if test.stateSetup != nil { + test.stateSetup(state) + } + admitters := NewValidator(state.authorizationRuleResolverMock).Admitters() assert.Len(t, admitters, 1) req := admission.Request{ @@ -137,7 +453,7 @@ func TestFeatureValueValid(t *testing.T) { func TestRejectsBadRequest(t *testing.T) { t.Parallel() - admitters := NewValidator().Admitters() + admitters := NewValidator(nil).Admitters() assert.Len(t, admitters, 1) req := admission.Request{ diff --git a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go index b24d8f5c0..2dbe46229 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go @@ -279,6 +279,6 @@ func newDefaultState(t *testing.T) testState { } func (m *testState) createBaseGRBResolver() *resolvers.GRBClusterRuleResolver { - grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(m.rtCacheMock, nil), m.grCacheMock) + grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(m.rtCacheMock, nil, nil), m.grCacheMock) return resolvers.NewGRBClusterRuleResolver(m.grbCacheMock, grResolver) } diff --git a/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go b/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go index c2259cedc..e09420270 100644 --- a/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go @@ -658,7 +658,7 @@ func TestAdmit(t *testing.T) { if test.args.stateSetup != nil { test.args.stateSetup(state) } - grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCacheMock, nil), state.grCacheMock) + grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCacheMock, nil, nil), state.grCacheMock) grbResolver := resolvers.NewGRBClusterRuleResolver(state.grbCacheMock, grResolver) admitters := globalrolebinding.NewValidator(state.resolver, grbResolver, state.sarMock).Admitters() require.Len(t, admitters, 1) @@ -679,7 +679,7 @@ func TestAdmit(t *testing.T) { func Test_UnexpectedErrors(t *testing.T) { t.Parallel() state := newDefaultState(t) - grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCacheMock, nil), state.grCacheMock) + grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCacheMock, nil, nil), state.grCacheMock) grbResolver := resolvers.NewGRBClusterRuleResolver(state.grbCacheMock, grResolver) validator := globalrolebinding.NewValidator(state.resolver, grbResolver, state.sarMock) admitters := validator.Admitters() diff --git a/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/ProjectRoleTemplateBinding.md b/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/ProjectRoleTemplateBinding.md index b2b4b6156..dc57d547e 100644 --- a/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/ProjectRoleTemplateBinding.md +++ b/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/ProjectRoleTemplateBinding.md @@ -4,6 +4,8 @@ Users can only create/update ProjectRoleTemplateBindings with rights less than or equal to those they currently possess. This is to prevent privilege escalation. +For external RoleTemplates (RoleTemplates with `external` set to `true`), if the `external-rules` feature flag is enabled and `ExternalRules` is specified in the roleTemplate in `RoleTemplateName`, +`ExternalRules` will be used for authorization. Otherwise, if `ExternalRules` are nil when the feature flag is on, the rules from the backing `ClusterRole` in the local cluster will be used. ### Invalid Fields - Create diff --git a/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/validator_test.go b/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/validator_test.go index 1aff9ee39..bcf08d0d6 100644 --- a/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/validator_test.go @@ -35,13 +35,16 @@ const ( type ProjectRoleTemplateBindingSuite struct { suite.Suite - adminRT *apisv3.RoleTemplate - readNodesRT *apisv3.RoleTemplate - lockedRT *apisv3.RoleTemplate - clusterContextRT *apisv3.RoleTemplate - adminCR *rbacv1.ClusterRole - writeNodeCR *rbacv1.ClusterRole - readServiceRole *rbacv1.Role + adminRT *apisv3.RoleTemplate + readNodesRT *apisv3.RoleTemplate + lockedRT *apisv3.RoleTemplate + clusterContextRT *apisv3.RoleTemplate + externalRulesWriteNodesRT *apisv3.RoleTemplate + externalClusterRoleRT *apisv3.RoleTemplate + adminCR *rbacv1.ClusterRole + writeNodeCR *rbacv1.ClusterRole + readPodsCR *rbacv1.ClusterRole + readServiceRole *rbacv1.Role } func TestProjectRoleTemplateBindings(t *testing.T) { @@ -88,6 +91,27 @@ func (p *ProjectRoleTemplateBindingSuite) SetupSuite() { Administrative: true, Context: "project", } + p.externalRulesWriteNodesRT = &apisv3.RoleTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "external-rule-write-nodes", + }, + DisplayName: "External Role", + ExternalRules: []rbacv1.PolicyRule{ruleWriteNodes}, + External: true, + Builtin: true, + Administrative: true, + Context: "project", + } + p.externalClusterRoleRT = &apisv3.RoleTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "read-pods-role", + }, + DisplayName: "External Role", + External: true, + Builtin: true, + Administrative: true, + Context: "project", + } p.lockedRT = &apisv3.RoleTemplate{ ObjectMeta: metav1.ObjectMeta{ Name: "locked-role", @@ -119,6 +143,10 @@ func (p *ProjectRoleTemplateBindingSuite) SetupSuite() { ObjectMeta: metav1.ObjectMeta{Namespace: "namespace1", Name: "read-service"}, Rules: []rbacv1.PolicyRule{ruleReadServices}, } + p.readPodsCR = &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{Name: "read-pods-role"}, + Rules: []rbacv1.PolicyRule{ruleReadPods}, + } } func (p *ProjectRoleTemplateBindingSuite) TestPrivilegeEscalation() { @@ -153,7 +181,7 @@ func (p *ProjectRoleTemplateBindingSuite) TestPrivilegeEscalation() { roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) roleTemplateCache.EXPECT().Get(p.adminRT.Name).Return(p.adminRT, nil).AnyTimes() clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) - roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache) + roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache, nil) prtbCache := fake.NewMockCacheInterface[*apisv3.ProjectRoleTemplateBinding](ctrl) prtbCache.EXPECT().AddIndexer(gomock.Any(), gomock.Any()) prtbCache.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(prtbUser, projectID)).Return([]*apisv3.ProjectRoleTemplateBinding{{ @@ -340,7 +368,7 @@ func (p *ProjectRoleTemplateBindingSuite) TestValidationOnUpdate() { roleTemplateCache.EXPECT().Get(p.adminRT.Name).Return(p.adminRT, nil).AnyTimes() roleTemplateCache.EXPECT().List(gomock.Any()).Return([]*apisv3.RoleTemplate{p.adminRT}, nil).AnyTimes() clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) - roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache) + roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache, nil) prtbCache := fake.NewMockCacheInterface[*apisv3.ProjectRoleTemplateBinding](ctrl) prtbCache.EXPECT().AddIndexer(gomock.Any(), gomock.Any()) prtbCache.EXPECT().GetByIndex(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() @@ -689,7 +717,14 @@ func (p *ProjectRoleTemplateBindingSuite) TestValidationOnUpdate() { } func (p *ProjectRoleTemplateBindingSuite) TestValidationOnCreate() { + type testState struct { + featureCacheMock *fake.MockNonNamespacedCacheInterface[*apisv3.Feature] + clusterRoleCacheMock *fake.MockNonNamespacedCacheInterface[*rbacv1.ClusterRole] + } + ctrl := gomock.NewController(p.T()) const adminUser = "admin-userid" + const writeNodeUser = "write-node-userid" + const readPodUser = "read-pod-userid" const badRoleTemplateName = "bad-roletemplate" const missingCluster = "missing-cluster" const nilCluster = "nil-cluster" @@ -698,7 +733,7 @@ func (p *ProjectRoleTemplateBindingSuite) TestValidationOnCreate() { const nilProject = "nil-project" const errProject = "error-project" const badSpecProject = "bad-spec" - clusterRoles := []*rbacv1.ClusterRole{p.adminCR} + clusterRoles := []*rbacv1.ClusterRole{p.adminCR, p.writeNodeCR, p.readPodsCR} clusterRoleBindings := []*rbacv1.ClusterRoleBinding{ { Subjects: []rbacv1.Subject{ @@ -706,78 +741,94 @@ func (p *ProjectRoleTemplateBindingSuite) TestValidationOnCreate() { }, RoleRef: rbacv1.RoleRef{APIGroup: rbacv1.GroupName, Kind: "ClusterRole", Name: p.adminCR.Name}, }, + { + Subjects: []rbacv1.Subject{ + {Kind: rbacv1.UserKind, Name: writeNodeUser}, + }, + RoleRef: rbacv1.RoleRef{APIGroup: rbacv1.GroupName, Kind: "ClusterRole", Name: p.writeNodeCR.Name}, + }, + { + Subjects: []rbacv1.Subject{ + {Kind: rbacv1.UserKind, Name: readPodUser}, + }, + RoleRef: rbacv1.RoleRef{APIGroup: rbacv1.GroupName, Kind: "ClusterRole", Name: p.readPodsCR.Name}, + }, } - resolver, _ := validation.NewTestRuleResolver(nil, nil, clusterRoles, clusterRoleBindings) - ctrl := gomock.NewController(p.T()) - roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) - roleTemplateCache.EXPECT().Get(p.adminRT.Name).Return(p.adminRT, nil).AnyTimes() - roleTemplateCache.EXPECT().Get(p.lockedRT.Name).Return(p.lockedRT, nil).AnyTimes() - roleTemplateCache.EXPECT().Get(p.clusterContextRT.Name).Return(p.clusterContextRT, nil).AnyTimes() - roleTemplateCache.EXPECT().Get(badRoleTemplateName).Return(nil, errExpected).AnyTimes() - roleTemplateCache.EXPECT().Get("").Return(nil, errExpected).AnyTimes() - clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) - roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache) - prtbCache := fake.NewMockCacheInterface[*apisv3.ProjectRoleTemplateBinding](ctrl) - prtbCache.EXPECT().AddIndexer(gomock.Any(), gomock.Any()) - prtbCache.EXPECT().GetByIndex(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() - crtbCache := fake.NewMockCacheInterface[*apisv3.ClusterRoleTemplateBinding](ctrl) - crtbCache.EXPECT().AddIndexer(gomock.Any(), gomock.Any()) - crtbCache.EXPECT().GetByIndex(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() - crtbResolver := resolvers.NewCRTBRuleResolver(crtbCache, roleResolver) - prtbResolver := resolvers.NewPRTBRuleResolver(prtbCache, roleResolver) + validatorWithMocks := func(state testState) *projectroletemplatebinding.Validator { + resolver, _ := validation.NewTestRuleResolver(nil, nil, clusterRoles, clusterRoleBindings) + roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.RoleTemplate](ctrl) + roleTemplateCache.EXPECT().Get(p.adminRT.Name).Return(p.adminRT, nil).AnyTimes() + roleTemplateCache.EXPECT().Get(p.externalRulesWriteNodesRT.Name).Return(p.externalRulesWriteNodesRT, nil).AnyTimes() + roleTemplateCache.EXPECT().Get(p.externalClusterRoleRT.Name).Return(p.externalClusterRoleRT, nil).AnyTimes() + roleTemplateCache.EXPECT().Get(p.lockedRT.Name).Return(p.lockedRT, nil).AnyTimes() + roleTemplateCache.EXPECT().Get(p.clusterContextRT.Name).Return(p.clusterContextRT, nil).AnyTimes() + roleTemplateCache.EXPECT().Get(badRoleTemplateName).Return(nil, errExpected).AnyTimes() + roleTemplateCache.EXPECT().Get("").Return(nil, errExpected).AnyTimes() + roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, state.clusterRoleCacheMock, state.featureCacheMock) + prtbCache := fake.NewMockCacheInterface[*apisv3.ProjectRoleTemplateBinding](ctrl) + prtbCache.EXPECT().AddIndexer(gomock.Any(), gomock.Any()) + prtbCache.EXPECT().GetByIndex(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + crtbCache := fake.NewMockCacheInterface[*apisv3.ClusterRoleTemplateBinding](ctrl) + crtbCache.EXPECT().AddIndexer(gomock.Any(), gomock.Any()) + crtbCache.EXPECT().GetByIndex(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + crtbResolver := resolvers.NewCRTBRuleResolver(crtbCache, roleResolver) + prtbResolver := resolvers.NewPRTBRuleResolver(prtbCache, roleResolver) - clusterCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Cluster](ctrl) - clusterCache.EXPECT().Get(clusterID).Return(&apisv3.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: clusterID, - }, - }, nil).AnyTimes() - clusterCache.EXPECT().Get(nilCluster).Return(nil, nil).AnyTimes() - clusterCache.EXPECT().Get(errCluster).Return(nil, fmt.Errorf("server not available")).AnyTimes() - clusterCache.EXPECT().Get(missingCluster).Return(nil, apierrors.NewNotFound(schema.GroupResource{ - Group: "management.cattle.io", - Resource: "clusters", - }, missingCluster)).AnyTimes() + clusterCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Cluster](ctrl) + clusterCache.EXPECT().Get(clusterID).Return(&apisv3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterID, + }, + }, nil).AnyTimes() + clusterCache.EXPECT().Get(nilCluster).Return(nil, nil).AnyTimes() + clusterCache.EXPECT().Get(errCluster).Return(nil, fmt.Errorf("server not available")).AnyTimes() + clusterCache.EXPECT().Get(missingCluster).Return(nil, apierrors.NewNotFound(schema.GroupResource{ + Group: "management.cattle.io", + Resource: "clusters", + }, missingCluster)).AnyTimes() - projectCache := fake.NewMockCacheInterface[*apisv3.Project](ctrl) - projectCache.EXPECT().Get(clusterID, projectID).Return(&apisv3.Project{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: clusterID, - Name: projectID, - }, - Spec: apisv3.ProjectSpec{ - ClusterName: clusterID, - }, - }, nil).AnyTimes() - projectCache.EXPECT().Get(clusterID, nilProject).Return(nil, nil).AnyTimes() - projectCache.EXPECT().Get(clusterID, errProject).Return(nil, fmt.Errorf("server not available")).AnyTimes() - projectCache.EXPECT().Get(clusterID, missingProject).Return(nil, apierrors.NewNotFound(schema.GroupResource{ - Group: "management.cattle.io", - Resource: "projects", - }, missingProject)).AnyTimes() + projectCache := fake.NewMockCacheInterface[*apisv3.Project](ctrl) + projectCache.EXPECT().Get(clusterID, projectID).Return(&apisv3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: clusterID, + Name: projectID, + }, + Spec: apisv3.ProjectSpec{ + ClusterName: clusterID, + }, + }, nil).AnyTimes() + projectCache.EXPECT().Get(clusterID, nilProject).Return(nil, nil).AnyTimes() + projectCache.EXPECT().Get(clusterID, errProject).Return(nil, fmt.Errorf("server not available")).AnyTimes() + projectCache.EXPECT().Get(clusterID, missingProject).Return(nil, apierrors.NewNotFound(schema.GroupResource{ + Group: "management.cattle.io", + Resource: "projects", + }, missingProject)).AnyTimes() - projectCache.EXPECT().Get(clusterID, badSpecProject).Return(&apisv3.Project{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: clusterID, - Name: projectID, - }, - Spec: apisv3.ProjectSpec{ - ClusterName: missingCluster, - }, - }, nil).AnyTimes() + projectCache.EXPECT().Get(clusterID, badSpecProject).Return(&apisv3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: clusterID, + Name: projectID, + }, + Spec: apisv3.ProjectSpec{ + ClusterName: missingCluster, + }, + }, nil).AnyTimes() + + return projectroletemplatebinding.NewValidator(prtbResolver, crtbResolver, resolver, roleResolver, clusterCache, projectCache) + } - validator := projectroletemplatebinding.NewValidator(prtbResolver, crtbResolver, resolver, roleResolver, clusterCache, projectCache) type args struct { oldPRTB func() *apisv3.ProjectRoleTemplateBinding newPRTB func() *apisv3.ProjectRoleTemplateBinding username string } tests := []struct { - name string - args args - wantErr bool - allowed bool + name string + args args + wantErr bool + allowed bool + stateSetup func(state testState) }{ { name: "base test valid PRTB creation", @@ -1121,6 +1172,102 @@ func (p *ProjectRoleTemplateBindingSuite) TestValidationOnCreate() { }, allowed: false, }, + { + name: "external RT with externalRules valid PRTB creation when feature flag is on", + args: args{ + username: writeNodeUser, + oldPRTB: func() *apisv3.ProjectRoleTemplateBinding { + return nil + }, + newPRTB: func() *apisv3.ProjectRoleTemplateBinding { + basePRTB := newBasePRTB() + basePRTB.RoleTemplateName = "external-rule-write-nodes" + + return basePRTB + }, + }, + stateSetup: func(state testState) { + state.featureCacheMock.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: apisv3.FeatureSpec{ + Value: &[]bool{true}[0], + }, + }, nil) + }, + allowed: true, + }, + { + name: "external RT with externalRules not created when feature flag is on and there are not enough permissions", + args: args{ + username: readPodUser, + oldPRTB: func() *apisv3.ProjectRoleTemplateBinding { + return nil + }, + newPRTB: func() *apisv3.ProjectRoleTemplateBinding { + basePRTB := newBasePRTB() + basePRTB.RoleTemplateName = "external-rule-write-nodes" + + return basePRTB + }, + }, + stateSetup: func(state testState) { + state.featureCacheMock.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: apisv3.FeatureSpec{ + Value: &[]bool{true}[0], + }, + }, nil) + }, + allowed: false, + }, + { + name: "external RT valid PRTB creation when feature flag is off", + args: args{ + username: adminUser, + oldPRTB: func() *apisv3.ProjectRoleTemplateBinding { + return nil + }, + newPRTB: func() *apisv3.ProjectRoleTemplateBinding { + basePRTB := newBasePRTB() + basePRTB.RoleTemplateName = "read-pods-role" + + return basePRTB + }, + }, + stateSetup: func(state testState) { + state.featureCacheMock.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: apisv3.FeatureSpec{ + Value: &[]bool{false}[0], + }, + }, nil) + }, + allowed: true, + }, + { + name: "external RT is created even when there are not enough permissions PRTB creation when feature flag is off", + args: args{ + username: writeNodeUser, + oldPRTB: func() *apisv3.ProjectRoleTemplateBinding { + return nil + }, + newPRTB: func() *apisv3.ProjectRoleTemplateBinding { + basePRTB := newBasePRTB() + basePRTB.RoleTemplateName = "read-pods-role" + + return basePRTB + }, + }, + stateSetup: func(state testState) { + state.featureCacheMock.EXPECT().Get(auth.ExternalRulesFeature).Return(&apisv3.Feature{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: apisv3.FeatureSpec{ + Value: &[]bool{false}[0], + }, + }, nil) + }, + allowed: true, + }, } for i := range tests { @@ -1128,6 +1275,16 @@ func (p *ProjectRoleTemplateBindingSuite) TestValidationOnCreate() { p.Run(test.name, func() { p.T().Parallel() req := createPRTBRequest(p.T(), test.args.oldPRTB(), test.args.newPRTB(), test.args.username) + featureCache := fake.NewMockNonNamespacedCacheInterface[*apisv3.Feature](ctrl) + clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) + state := testState{ + clusterRoleCacheMock: clusterRoleCache, + featureCacheMock: featureCache, + } + if test.stateSetup != nil { + test.stateSetup(state) + } + validator := validatorWithMocks(state) admitters := validator.Admitters() p.Len(admitters, 1) resp, err := admitters[0].Admit(req) diff --git a/pkg/resources/management.cattle.io/v3/roletemplate/RoleTemplate.md b/pkg/resources/management.cattle.io/v3/roletemplate/RoleTemplate.md index 51cf6acea..120d44cbc 100644 --- a/pkg/resources/management.cattle.io/v3/roletemplate/RoleTemplate.md +++ b/pkg/resources/management.cattle.io/v3/roletemplate/RoleTemplate.md @@ -8,11 +8,12 @@ Circular references to a `RoleTemplate` (a inherits b, b inherits a) are not all ### Rules Without Verbs, Resources, API groups -Rules without verbs, resources, or apigroups are not permitted. The `rules` included in a RoleTemplate are of the same type as the rules used by standard Kubernetes RBAC types (such as `Roles` from `rbac.authorization.k8s.io/v1`). Because of this, they inherit the same restrictions as these types, including this one. +Rules without verbs, resources, or apigroups are not permitted. The `rules` and `externalRules` included in a RoleTemplate are of the same type as the rules used by standard Kubernetes RBAC types (such as `Roles` from `rbac.authorization.k8s.io/v1`). Because of this, they inherit the same restrictions as these types, including this one. ### Escalation Prevention Users can only change RoleTemplates with rights less than or equal to those they currently possess. This prevents privilege escalation. +Users can't create external RoleTemplates (or update existing RoleTemplates) with `ExternalRules` without having the `escalate` verb on that RoleTemplate. ### Context Validation diff --git a/pkg/resources/management.cattle.io/v3/roletemplate/main_test.go b/pkg/resources/management.cattle.io/v3/roletemplate/main_test.go index 8751c00e2..a7f664fa9 100644 --- a/pkg/resources/management.cattle.io/v3/roletemplate/main_test.go +++ b/pkg/resources/management.cattle.io/v3/roletemplate/main_test.go @@ -8,6 +8,7 @@ import ( v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" "github.com/rancher/webhook/pkg/admission" + "github.com/rancher/wrangler/v2/pkg/generic/fake" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" v1 "k8s.io/api/admission/v1" @@ -21,12 +22,18 @@ import ( var errTest = errors.New("bad error") +type testState struct { + featureCacheMock *fake.MockNonNamespacedCacheInterface[*v3.Feature] + clusterRoleCacheMock *fake.MockNonNamespacedCacheInterface[*rbacv1.ClusterRole] +} + type tableTest struct { - wantRT func() *v3.RoleTemplate - name string - args args - wantError bool - allowed bool + wantRT func() *v3.RoleTemplate + name string + args args + stateSetup func(state testState) + wantError bool + allowed bool } type args struct { diff --git a/pkg/resources/management.cattle.io/v3/roletemplate/validator.go b/pkg/resources/management.cattle.io/v3/roletemplate/validator.go index 19f69aa95..187b2ac1b 100644 --- a/pkg/resources/management.cattle.io/v3/roletemplate/validator.go +++ b/pkg/resources/management.cattle.io/v3/roletemplate/validator.go @@ -127,6 +127,16 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp return admission.ResponseBadRequest(fmt.Sprintf("Circular Reference: RoleTemplate %s already inherits RoleTemplate %s", circularTemplate.Name, newRT.Name)), nil } + if newRT.ExternalRules != nil { + if !newRT.External { + return admission.ResponseBadRequest("ExternalRules can't be set in RoleTemplates with external=false"), nil + } + // verify external rules as per kubernetes rbac rules. + if err := common.ValidateRules(newRT.ExternalRules, false, fldPath.Child("externalRules")); err != nil { + return admission.ResponseBadRequest(fmt.Sprintf("Invalid externalRules: %v", err.Error())), nil + } + } + rules, err := a.roleTemplateResolver.RulesFromTemplate(newRT) if err != nil { return nil, fmt.Errorf("failed to get all rules for '%s': %w", newRT.Name, err) @@ -146,6 +156,11 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp return admission.ResponseAllowed(), nil } + if newRT.External && newRT.ExternalRules != nil { + // ExternalRules needs 'escalate' permissions. Request would have already been accepted if this user had 'escalate' permissions. + return admission.ResponseFailedEscalation("External RoleTemplates with ExternalRules can only be created for users with 'escalate' permissions"), nil + } + err = auth.ConfirmNoEscalation(request, rules, "", a.resolver) if err != nil { return admission.ResponseFailedEscalation(err.Error()), nil diff --git a/pkg/resources/management.cattle.io/v3/roletemplate/validator_test.go b/pkg/resources/management.cattle.io/v3/roletemplate/validator_test.go index 1ab076372..efe735caa 100644 --- a/pkg/resources/management.cattle.io/v3/roletemplate/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/roletemplate/validator_test.go @@ -54,15 +54,13 @@ func (r *RoleTemplateSuite) Test_PrivilegeEscalation() { ctrl := gomock.NewController(r.T()) roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*v3.RoleTemplate](ctrl) - roleTemplateCache.EXPECT().AddIndexer(expectedIndexerName, gomock.Any()) + roleTemplateCache.EXPECT().AddIndexer(expectedIndexerName, gomock.Any()).AnyTimes() roleTemplateCache.EXPECT().Get(r.adminRT.Name).Return(r.adminRT, nil).AnyTimes() roleTemplateCache.EXPECT().Get(r.readNodesRT.Name).Return(r.readNodesRT, nil).AnyTimes() roleTemplateCache.EXPECT().Get(notFoundRoleTemplateName).Return(nil, newNotFound(notFoundRoleTemplateName)).AnyTimes() roleTemplateCache.EXPECT().List(gomock.Any()).Return([]*v3.RoleTemplate{r.adminRT, r.readNodesRT}, nil).AnyTimes() - clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) - roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache) grCache := fake.NewMockNonNamespacedCacheInterface[*v3.GlobalRole](ctrl) - grCache.EXPECT().AddIndexer(expectedGlobalRefIndex, gomock.Any()) + grCache.EXPECT().AddIndexer(expectedGlobalRefIndex, gomock.Any()).AnyTimes() k8Fake := &k8testing.Fake{} fakeSAR := &k8fake.FakeSubjectAccessReviews{Fake: &k8fake.FakeAuthorizationV1{Fake: k8Fake}} @@ -78,9 +76,6 @@ func (r *RoleTemplateSuite) Test_PrivilegeEscalation() { spec.ResourceAttributes.Verb == "escalate" return true, review, nil }) - validator := roletemplate.NewValidator(resolver, roleResolver, fakeSAR, grCache) - admitters := validator.Admitters() - r.Len(admitters, 1, "wanted only one admitter") tests := []tableTest{ { @@ -153,12 +148,156 @@ func (r *RoleTemplateSuite) Test_PrivilegeEscalation() { }, allowed: false, }, + { + name: "user with escalate permissions can create external RoleTemplates with externalRules", + args: args{ + username: testUser, + oldRT: func() *v3.RoleTemplate { + return nil + }, + newRT: func() *v3.RoleTemplate { + baseRT := newDefaultRT() + baseRT.External = true + baseRT.ExternalRules = r.manageNodeRole.Rules + + return baseRT + }, + }, + stateSetup: func(state testState) { + state.featureCacheMock.EXPECT().Get(auth.ExternalRulesFeature).Return(&v3.Feature{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: v3.FeatureSpec{ + Value: &[]bool{true}[0], + }, + }, nil) + }, + allowed: true, + }, + { + name: "user without escalate permissions can't create external RoleTemplates with externalRules", + args: args{ + username: noPrivUser, + oldRT: func() *v3.RoleTemplate { + return nil + }, + newRT: func() *v3.RoleTemplate { + baseRT := newDefaultRT() + baseRT.External = true + baseRT.ExternalRules = r.manageNodeRole.Rules + + return baseRT + }, + }, + stateSetup: func(state testState) { + state.featureCacheMock.EXPECT().Get(auth.ExternalRulesFeature).Return(&v3.Feature{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: v3.FeatureSpec{ + Value: &[]bool{true}[0], + }, + }, nil) + }, + allowed: false, + }, + { + name: "user without escalate permissions can't create external RoleTemplates with externalRules when the feature flag is off", + args: args{ + username: noPrivUser, + oldRT: func() *v3.RoleTemplate { + return nil + }, + newRT: func() *v3.RoleTemplate { + baseRT := newDefaultRT() + baseRT.External = true + baseRT.ExternalRules = r.manageNodeRole.Rules + + return baseRT + }, + }, + stateSetup: func(state testState) { + state.featureCacheMock.EXPECT().Get(auth.ExternalRulesFeature).Return(&v3.Feature{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: v3.FeatureSpec{ + Value: &[]bool{false}[0], + }, + }, nil) + state.clusterRoleCacheMock.EXPECT().Get(newDefaultRT().Name).Return(&rbacv1.ClusterRole{}, nil) + }, + allowed: false, + }, + { + name: "user without escalate permissions can't update external RoleTemplates with externalRules", + args: args{ + username: noPrivUser, + oldRT: func() *v3.RoleTemplate { + baseRT := newDefaultRT() + + return baseRT + }, + newRT: func() *v3.RoleTemplate { + baseRT := newDefaultRT() + baseRT.External = true + baseRT.ExternalRules = r.manageNodeRole.Rules + + return baseRT + }, + }, + stateSetup: func(state testState) { + state.featureCacheMock.EXPECT().Get(auth.ExternalRulesFeature).Return(&v3.Feature{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: v3.FeatureSpec{ + Value: &[]bool{true}[0], + }, + }, nil) + }, + allowed: false, + }, + { + name: "user with escalate permissions ca update external RoleTemplates with externalRules", + args: args{ + username: testUser, + oldRT: func() *v3.RoleTemplate { + baseRT := newDefaultRT() + + return baseRT + }, + newRT: func() *v3.RoleTemplate { + baseRT := newDefaultRT() + baseRT.External = true + baseRT.ExternalRules = r.manageNodeRole.Rules + + return baseRT + }, + }, + stateSetup: func(state testState) { + state.featureCacheMock.EXPECT().Get(auth.ExternalRulesFeature).Return(&v3.Feature{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: v3.FeatureSpec{ + Value: &[]bool{true}[0], + }, + }, nil) + }, + allowed: true, + }, } for i := range tests { test := tests[i] r.Run(test.name, func() { r.T().Parallel() + featureCache := fake.NewMockNonNamespacedCacheInterface[*v3.Feature](ctrl) + clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) + + state := testState{ + clusterRoleCacheMock: clusterRoleCache, + featureCacheMock: featureCache, + } + if test.stateSetup != nil { + test.stateSetup(state) + } + roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache, state.featureCacheMock) + validator := roletemplate.NewValidator(resolver, roleResolver, fakeSAR, grCache) + admitters := validator.Admitters() + r.Len(admitters, 1, "wanted only one admitter") req := createRTRequest(r.T(), test.args.oldRT(), test.args.newRT(), test.args.username) resp, err := admitters[0].Admit(req) if r.NoError(err, "Admit failed") { @@ -185,7 +324,8 @@ func (r *RoleTemplateSuite) Test_UpdateValidation() { roleTemplateCache.EXPECT().AddIndexer(expectedIndexerName, gomock.Any()) roleTemplateCache.EXPECT().Get(r.adminRT.Name).Return(r.adminRT, nil).AnyTimes() clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) - roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache) + featureCache := fake.NewMockNonNamespacedCacheInterface[*v3.Feature](ctrl) + roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache, featureCache) grCache := fake.NewMockNonNamespacedCacheInterface[*v3.GlobalRole](ctrl) grCache.EXPECT().AddIndexer(expectedGlobalRefIndex, gomock.Any()) @@ -487,20 +627,14 @@ func (r *RoleTemplateSuite) Test_Create() { ctrl := gomock.NewController(r.T()) roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*v3.RoleTemplate](ctrl) - roleTemplateCache.EXPECT().AddIndexer(expectedIndexerName, gomock.Any()) + roleTemplateCache.EXPECT().AddIndexer(expectedIndexerName, gomock.Any()).AnyTimes() roleTemplateCache.EXPECT().Get(r.adminRT.Name).Return(r.adminRT, nil).AnyTimes() - clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) - roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache) grCache := fake.NewMockNonNamespacedCacheInterface[*v3.GlobalRole](ctrl) - grCache.EXPECT().AddIndexer(expectedGlobalRefIndex, gomock.Any()) + grCache.EXPECT().AddIndexer(expectedGlobalRefIndex, gomock.Any()).AnyTimes() k8Fake := &k8testing.Fake{} fakeSAR := &k8fake.FakeSubjectAccessReviews{Fake: &k8fake.FakeAuthorizationV1{Fake: k8Fake}} - validator := roletemplate.NewValidator(resolver, roleResolver, fakeSAR, grCache) - admitters := validator.Admitters() - r.Len(admitters, 1, "wanted only one admitter") - tests := []tableTest{ { name: "base test valid RT", @@ -580,6 +714,86 @@ func (r *RoleTemplateSuite) Test_Create() { }, allowed: false, }, + { + name: "create new external RoleTemplate when feature flag is off", + args: args{ + username: adminUser, + oldRT: func() *v3.RoleTemplate { + return nil + }, + newRT: func() *v3.RoleTemplate { + rt := newDefaultRT() + rt.External = true + return rt + }, + }, + stateSetup: func(state testState) { + state.featureCacheMock.EXPECT().Get(auth.ExternalRulesFeature).Return(&v3.Feature{ + Spec: v3.FeatureSpec{ + Value: &[]bool{false}[0], + }, + }, nil) + state.clusterRoleCacheMock.EXPECT().Get("rt-new").Return(&rbacv1.ClusterRole{}, nil) + }, + allowed: true, + }, + { + name: "create new external RoleTemplate when feature flag is off, context is project and backing ClusterRole doesn't exist", + args: args{ + username: adminUser, + oldRT: func() *v3.RoleTemplate { + return nil + }, + newRT: func() *v3.RoleTemplate { + rt := newDefaultRT() + rt.External = true + rt.Context = "project" + return rt + }, + }, + stateSetup: func(state testState) { + state.featureCacheMock.EXPECT().Get(auth.ExternalRulesFeature).Return(&v3.Feature{ + Spec: v3.FeatureSpec{ + Value: &[]bool{false}[0], + }, + }, nil) + }, + allowed: true, + }, + { + name: "invalid external rules", + args: args{ + username: adminUser, + oldRT: func() *v3.RoleTemplate { + return nil + }, + newRT: func() *v3.RoleTemplate { + rt := newDefaultRT() + rt.External = true + rt.ExternalRules = []rbacv1.PolicyRule{r.ruleEmptyVerbs} + return rt + }, + }, + allowed: false, + wantError: true, + }, + { + name: "ExternalRules can't be set in RoleTemplates with external=false", + args: args{ + username: adminUser, + oldRT: func() *v3.RoleTemplate { + return nil + }, + newRT: func() *v3.RoleTemplate { + rt := newDefaultRT() + rt.External = false + rt.ExternalRules = r.manageNodeRole.Rules + return rt + }, + }, + allowed: false, + wantError: true, + }, { // Ensure we're not blocking the creation of RTs with // NonResourceURLs included such as cluster-owner. @@ -608,8 +822,23 @@ func (r *RoleTemplateSuite) Test_Create() { test := tests[i] r.Run(test.name, func() { r.T().Parallel() + featureCache := fake.NewMockNonNamespacedCacheInterface[*v3.Feature](ctrl) + clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) + state := testState{ + clusterRoleCacheMock: clusterRoleCache, + featureCacheMock: featureCache, + } + if test.stateSetup != nil { + test.stateSetup(state) + } + roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache, state.featureCacheMock) + validator := roletemplate.NewValidator(resolver, roleResolver, fakeSAR, grCache) + admitters := validator.Admitters() + r.Len(admitters, 1, "wanted only one admitter") + req := createRTRequest(r.T(), test.args.oldRT(), test.args.newRT(), test.args.username) resp, err := admitters[0].Admit(req) + r.NoError(err, "Admit failed") r.Equalf(test.allowed, resp.Allowed, "Response was incorrectly validated wanted response.Allowed = '%v' got '%v' message=%+v", test.allowed, resp.Allowed, resp.Result) }) @@ -672,7 +901,7 @@ func (r *RoleTemplateSuite) Test_Delete() { grCache.EXPECT().AddIndexer(expectedGlobalRefIndex, gomock.Any()) grCache.EXPECT().GetByIndex(expectedGlobalRefIndex, gomock.Any()).Return([]*v3.GlobalRole{}, nil).AnyTimes() return testMocks{ - rtResolver: auth.NewRoleTemplateResolver(roleTemplateCache, nil), + rtResolver: auth.NewRoleTemplateResolver(roleTemplateCache, nil, nil), grCache: grCache, } }, @@ -724,7 +953,7 @@ func (r *RoleTemplateSuite) Test_Delete() { grCache.EXPECT().GetByIndex(expectedGlobalRefIndex, gomock.Any()).Return([]*v3.GlobalRole{}, nil).AnyTimes() return testMocks{ - rtResolver: auth.NewRoleTemplateResolver(roleTemplateCache, nil), + rtResolver: auth.NewRoleTemplateResolver(roleTemplateCache, nil, nil), grCache: grCache, } }, @@ -751,7 +980,7 @@ func (r *RoleTemplateSuite) Test_Delete() { grCache.EXPECT().AddIndexer(expectedGlobalRefIndex, gomock.Any()) grCache.EXPECT().GetByIndex(expectedGlobalRefIndex, gomock.Any()).Return([]*v3.GlobalRole{}, nil).AnyTimes() return testMocks{ - rtResolver: auth.NewRoleTemplateResolver(roleTemplateCache, nil), + rtResolver: auth.NewRoleTemplateResolver(roleTemplateCache, nil, nil), grCache: grCache, } }, @@ -787,7 +1016,8 @@ func (r *RoleTemplateSuite) Test_ErrorHandling() { roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*v3.RoleTemplate](ctrl) roleTemplateCache.EXPECT().AddIndexer(expectedIndexerName, gomock.Any()) clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) - roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache) + featureCache := fake.NewMockNonNamespacedCacheInterface[*v3.Feature](ctrl) + roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache, featureCache) grCache := fake.NewMockNonNamespacedCacheInterface[*v3.GlobalRole](ctrl) grCache.EXPECT().AddIndexer(expectedGlobalRefIndex, gomock.Any()) @@ -901,7 +1131,8 @@ func (r *RoleTemplateSuite) Test_CheckCircularRef() { req := createRTRequest(r.T(), nil, newRT, adminUser) clusterRoleCache := fake.NewMockNonNamespacedCacheInterface[*rbacv1.ClusterRole](ctrl) - roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache) + featureCache := fake.NewMockNonNamespacedCacheInterface[*v3.Feature](ctrl) + roleResolver := auth.NewRoleTemplateResolver(roleTemplateCache, clusterRoleCache, featureCache) validator := roletemplate.NewValidator(resolver, roleResolver, fakeSAR, grCache) admitters := validator.Admitters() diff --git a/pkg/server/handlers.go b/pkg/server/handlers.go index 10c8ce092..bde40ab88 100644 --- a/pkg/server/handlers.go +++ b/pkg/server/handlers.go @@ -26,7 +26,7 @@ import ( // Validation returns a list of all ValidatingAdmissionHandlers used by the webhook. func Validation(clients *clients.Clients) ([]admission.ValidatingAdmissionHandler, error) { handlers := []admission.ValidatingAdmissionHandler{ - feature.NewValidator(), + feature.NewValidator(clients.DefaultResolver), managementCluster.NewValidator(clients.K8s.AuthorizationV1().SubjectAccessReviews(), clients.Management.PodSecurityAdmissionConfigurationTemplate().Cache()), provisioningCluster.NewProvisioningClusterValidator(clients), machineconfig.NewValidator(),