diff --git a/pkg/accesscontrol/access_store.go b/pkg/accesscontrol/access_store.go index 51f7269b..2380f12b 100644 --- a/pkg/accesscontrol/access_store.go +++ b/pkg/accesscontrol/access_store.go @@ -4,10 +4,12 @@ import ( "context" "crypto/sha256" "encoding/hex" + "hash" "sort" "time" v1 "github.com/rancher/wrangler/v3/pkg/generated/controllers/rbac/v1" + rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/util/cache" "k8s.io/apiserver/pkg/authentication/user" ) @@ -19,10 +21,21 @@ type AccessSetLookup interface { PurgeUserData(id string) } +type policyRules interface { + get(string) *AccessSet + getRoleBindings(string) []*rbacv1.RoleBinding + getClusterRoleBindings(string) []*rbacv1.ClusterRoleBinding +} + +type roleRevisions interface { + roleRevision(string, string) string +} + type AccessStore struct { - users *policyRuleIndex - groups *policyRuleIndex - cache *cache.LRUExpireCache + usersPolicyRules policyRules + groupsPolicyRules policyRules + roles roleRevisions + cache *cache.LRUExpireCache } type roleKey struct { @@ -31,10 +44,10 @@ type roleKey struct { } func NewAccessStore(ctx context.Context, cacheResults bool, rbac v1.Interface) *AccessStore { - revisions := newRoleRevision(ctx, rbac) as := &AccessStore{ - users: newPolicyRuleIndex(true, revisions, rbac), - groups: newPolicyRuleIndex(false, revisions, rbac), + usersPolicyRules: newPolicyRuleIndex(true, rbac), + groupsPolicyRules: newPolicyRuleIndex(false, rbac), + roles: newRoleRevision(ctx, rbac), } if cacheResults { as.cache = cache.NewLRUExpireCache(50) @@ -53,9 +66,9 @@ func (l *AccessStore) AccessFor(user user.Info) *AccessSet { } } - result := l.users.get(user.GetName()) + result := l.usersPolicyRules.get(user.GetName()) for _, group := range user.GetGroups() { - result.Merge(l.groups.get(group)) + result.Merge(l.groupsPolicyRules.get(group)) } if l.cache != nil { @@ -73,16 +86,30 @@ func (l *AccessStore) PurgeUserData(id string) { func (l *AccessStore) CacheKey(user user.Info) string { d := sha256.New() - l.users.addRolesToHash(d, user.GetName()) - groupBase := user.GetGroups() groups := make([]string, len(groupBase)) copy(groups, groupBase) sort.Strings(groups) + l.addRolesToHash(d, user.GetName(), l.usersPolicyRules) for _, group := range groups { - l.groups.addRolesToHash(d, group) + l.addRolesToHash(d, group, l.groupsPolicyRules) } return hex.EncodeToString(d.Sum(nil)) } + +func (l *AccessStore) addRolesToHash(digest hash.Hash, subjectName string, rules policyRules) { + for _, crb := range rules.getClusterRoleBindings(subjectName) { + digest.Write([]byte(crb.RoleRef.Name)) + digest.Write([]byte(l.roles.roleRevision("", crb.RoleRef.Name))) + } + + for _, rb := range rules.getRoleBindings(subjectName) { + digest.Write([]byte(rb.RoleRef.Name)) + if rb.Namespace != "" { + digest.Write([]byte(rb.Namespace)) + } + digest.Write([]byte(l.roles.roleRevision(rb.Namespace, rb.RoleRef.Name))) + } +} diff --git a/pkg/accesscontrol/access_store_test.go b/pkg/accesscontrol/access_store_test.go new file mode 100644 index 00000000..bdaf5d51 --- /dev/null +++ b/pkg/accesscontrol/access_store_test.go @@ -0,0 +1,328 @@ +package accesscontrol + +import ( + "fmt" + "slices" + "testing" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/cache" + "k8s.io/apiserver/pkg/authentication/user" +) + +func TestAccessStore_CacheKey(t *testing.T) { + testUser := &user.DefaultInfo{ + Name: "user-12345", + Groups: []string{ + "users", + "mygroup", + }, + } + + tests := []struct { + name string + store *AccessStore + verify func(t *testing.T, store *AccessStore, res string) + }{ + { + name: "consistently produces the same value", + store: &AccessStore{ + usersPolicyRules: &policyRulesMock{ + getRBFunc: func(s string) []*rbacv1.RoleBinding { + return []*rbacv1.RoleBinding{ + makeRB("testns", "testrb", testUser.Name, "testrole"), + } + }, + getCRBFunc: func(s string) []*rbacv1.ClusterRoleBinding { + return []*rbacv1.ClusterRoleBinding{ + makeCRB("testcrb", testUser.Name, "testclusterrole"), + } + }, + }, + groupsPolicyRules: &policyRulesMock{}, + roles: roleRevisionsMock(func(ns, name string) string { + return fmt.Sprintf("%s%srev", ns, name) + }), + }, + verify: func(t *testing.T, store *AccessStore, res string) { + // iterate enough times to make possibly random iterators repeating order by coincidence + for range 5 { + if res != store.CacheKey(testUser) { + t.Fatal("CacheKey is not the same on consecutive runs") + } + } + }, + }, + { + name: "group permissions are taken into account", + store: &AccessStore{ + usersPolicyRules: &policyRulesMock{}, + groupsPolicyRules: &policyRulesMock{ + getRBFunc: func(s string) []*rbacv1.RoleBinding { + return []*rbacv1.RoleBinding{ + makeRB("testns", "testrb", testUser.Name, "testrole"), + } + }, + getCRBFunc: func(s string) []*rbacv1.ClusterRoleBinding { + return []*rbacv1.ClusterRoleBinding{ + makeCRB("testcrb", testUser.Name, "testclusterrole"), + } + }, + }, + roles: roleRevisionsMock(func(ns, name string) string { + return fmt.Sprintf("%s%srev", ns, name) + }), + }, + verify: func(t *testing.T, store *AccessStore, res string) { + // remove users + testUserAlt := *testUser + testUserAlt.Groups = []string{} + + if store.CacheKey(&testUserAlt) == res { + t.Fatal("CacheKey does not use groups for hashing") + } + }, + }, + { + name: "different groups order produces the same value", + store: &AccessStore{ + usersPolicyRules: &policyRulesMock{}, + groupsPolicyRules: &policyRulesMock{ + getRBFunc: func(s string) []*rbacv1.RoleBinding { + if s == testUser.Groups[0] { + return []*rbacv1.RoleBinding{ + makeRB("testns", "testrb", testUser.Name, "testrole"), + } + } + return nil + }, + getCRBFunc: func(s string) []*rbacv1.ClusterRoleBinding { + if s == testUser.Groups[1] { + return []*rbacv1.ClusterRoleBinding{ + makeCRB("testcrb", testUser.Name, "testclusterrole"), + } + } + return nil + }, + }, + roles: roleRevisionsMock(func(ns, name string) string { + return fmt.Sprintf("%s%srev", ns, name) + }), + }, + verify: func(t *testing.T, store *AccessStore, res string) { + // swap order of groups + testUserAlt := &user.DefaultInfo{Name: testUser.Name, Groups: slices.Clone(testUser.Groups)} + slices.Reverse(testUserAlt.Groups) + + if store.CacheKey(testUserAlt) != res { + t.Fatal("CacheKey varies depending on groups order") + } + }, + }, + { + name: "role changes produce a different value", + store: &AccessStore{ + usersPolicyRules: &policyRulesMock{ + getRBFunc: func(s string) []*rbacv1.RoleBinding { + return []*rbacv1.RoleBinding{ + makeRB("testns", "testrb", testUser.Name, "testrole"), + } + }, + }, + groupsPolicyRules: &policyRulesMock{}, + roles: roleRevisionsMock(func(ns, name string) string { + return "rev1" + }), + }, + verify: func(t *testing.T, store *AccessStore, res string) { + store.roles = roleRevisionsMock(func(ns, name string) string { + return "rev2" + + }) + if store.CacheKey(testUser) == res { + t.Fatal("CacheKey did not change when on role change") + } + }, + }, + { + name: "new groups produce a different value", + store: &AccessStore{ + usersPolicyRules: &policyRulesMock{}, + groupsPolicyRules: &policyRulesMock{ + getRBFunc: func(s string) []*rbacv1.RoleBinding { + return []*rbacv1.RoleBinding{ + makeRB("testns", "testrb", testUser.Name, "testrole"), + } + }, + getCRBFunc: func(s string) []*rbacv1.ClusterRoleBinding { + if s == "newgroup" { + return []*rbacv1.ClusterRoleBinding{ + makeCRB("testcrb", testUser.Name, "testclusterrole"), + } + } + return nil + }, + }, + roles: roleRevisionsMock(func(ns, name string) string { + return fmt.Sprintf("%s%srev", ns, name) + }), + }, + verify: func(t *testing.T, store *AccessStore, res string) { + testUserAlt := &user.DefaultInfo{ + Name: testUser.Name, + Groups: append(slices.Clone(testUser.Groups), "newgroup"), + } + if store.CacheKey(testUserAlt) == res { + t.Fatal("CacheKey did not change when new group was added") + } + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.verify(t, tt.store, tt.store.CacheKey(testUser)) + }) + } +} + +func TestAccessStore_AccessFor(t *testing.T) { + testUser := &user.DefaultInfo{ + Name: "test-user", + Groups: []string{"users", "mygroup"}, + } + asCache := cache.NewLRUExpireCache(10) + store := &AccessStore{ + usersPolicyRules: &policyRulesMock{ + getRBFunc: func(s string) []*rbacv1.RoleBinding { + return []*rbacv1.RoleBinding{ + makeRB("testns", "testrb", testUser.Name, "testrole"), + } + }, + getFunc: func(_ string) *AccessSet { + return &AccessSet{ + set: map[key]resourceAccessSet{ + {"get", corev1.Resource("ConfigMap")}: map[Access]bool{ + {Namespace: All, ResourceName: All}: true, + }, + }, + } + }, + }, + groupsPolicyRules: &policyRulesMock{ + getCRBFunc: func(s string) []*rbacv1.ClusterRoleBinding { + if s == "mygroup" { + return []*rbacv1.ClusterRoleBinding{ + makeCRB("testcrb", testUser.Name, "testclusterrole"), + } + } + return nil + }, + getFunc: func(_ string) *AccessSet { + return &AccessSet{ + set: map[key]resourceAccessSet{ + {"list", appsv1.Resource("Deployment")}: map[Access]bool{ + {Namespace: "testns", ResourceName: All}: true, + }, + }, + } + }, + }, + roles: roleRevisionsMock(func(ns, name string) string { + return fmt.Sprintf("%s%srev", ns, name) + }), + cache: asCache, + } + + validateAccessSet := func(as *AccessSet) { + if as == nil { + t.Fatal("AccessFor() returned nil") + } + if as.ID == "" { + t.Fatal("AccessSet has empty ID") + } + if !as.Grants("get", corev1.Resource("ConfigMap"), "default", "cm") || + !as.Grants("list", appsv1.Resource("Deployment"), "testns", "deploy") { + t.Error("AccessSet does not grant desired permissions") + } + // wrong verbs + if as.Grants("delete", corev1.Resource("ConfigMap"), "default", "cm") || + as.Grants("get", appsv1.Resource("Deployment"), "testns", "deploy") || + // wrong resource + as.Grants("get", corev1.Resource("Secret"), "testns", "s") { + t.Error("AccessSet grants undesired permissions") + } + } + + as := store.AccessFor(testUser) + validateAccessSet(as) + if got, want := len(asCache.Keys()), 1; got != want { + t.Errorf("Unexpected number of cache keys: got %d, want %d", got, want) + } + + as = store.AccessFor(testUser) + validateAccessSet(as) + if got, want := len(asCache.Keys()), 1; got != want { + t.Errorf("Unexpected increase in cache size, got %d, want %d", got, want) + } + + store.PurgeUserData(as.ID) + if got, want := len(asCache.Keys()), 0; got != want { + t.Errorf("Cache was not purged, got len %d, want %d", got, want) + } +} + +func makeRB(ns, name, user, role string) *rbacv1.RoleBinding { + return &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{Namespace: ns, Name: name}, + Subjects: []rbacv1.Subject{ + {Name: user}, + }, + RoleRef: rbacv1.RoleRef{Name: role}, + } +} + +func makeCRB(name, user, role string) *rbacv1.ClusterRoleBinding { + return &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Subjects: []rbacv1.Subject{ + {Name: user}, + }, + RoleRef: rbacv1.RoleRef{Name: role}, + } +} + +type policyRulesMock struct { + getFunc func(string) *AccessSet + getRBFunc func(string) []*rbacv1.RoleBinding + getCRBFunc func(string) []*rbacv1.ClusterRoleBinding +} + +func (p policyRulesMock) get(s string) *AccessSet { + if p.getFunc == nil { + return nil + } + return p.getFunc(s) +} + +func (p policyRulesMock) getRoleBindings(s string) []*rbacv1.RoleBinding { + if p.getRBFunc == nil { + return nil + } + return p.getRBFunc(s) +} + +func (p policyRulesMock) getClusterRoleBindings(s string) []*rbacv1.ClusterRoleBinding { + if p.getCRBFunc == nil { + return nil + } + return p.getCRBFunc(s) +} + +type roleRevisionsMock func(ns, name string) string + +func (fn roleRevisionsMock) roleRevision(ns, name string) string { + return fn(ns, name) +} diff --git a/pkg/accesscontrol/policy_rule_index.go b/pkg/accesscontrol/policy_rule_index.go index 42395088..4f8d9ab9 100644 --- a/pkg/accesscontrol/policy_rule_index.go +++ b/pkg/accesscontrol/policy_rule_index.go @@ -2,7 +2,6 @@ package accesscontrol import ( "fmt" - "hash" "sort" v1 "github.com/rancher/wrangler/v3/pkg/generated/controllers/rbac/v1" @@ -20,13 +19,12 @@ type policyRuleIndex struct { rCache v1.RoleCache crbCache v1.ClusterRoleBindingCache rbCache v1.RoleBindingCache - revisions *roleRevisionIndex kind string roleIndexKey string clusterRoleIndexKey string } -func newPolicyRuleIndex(user bool, revisions *roleRevisionIndex, rbac v1.Interface) *policyRuleIndex { +func newPolicyRuleIndex(user bool, rbac v1.Interface) *policyRuleIndex { key := "Group" if user { key = "User" @@ -39,7 +37,6 @@ func newPolicyRuleIndex(user bool, revisions *roleRevisionIndex, rbac v1.Interfa rbCache: rbac.RoleBinding().Cache(), clusterRoleIndexKey: "crb" + key, roleIndexKey: "rb" + key, - revisions: revisions, } pi.crbCache.AddIndexer(pi.clusterRoleIndexKey, pi.clusterRoleBindingBySubjectIndexer) @@ -72,30 +69,6 @@ func (p *policyRuleIndex) roleBindingBySubject(rb *rbacv1.RoleBinding) (result [ return } -var null = []byte{'\x00'} - -func (p *policyRuleIndex) addRolesToHash(digest hash.Hash, subjectName string) { - for _, crb := range p.getClusterRoleBindings(subjectName) { - digest.Write([]byte(crb.RoleRef.Name)) - digest.Write([]byte(p.revisions.roleRevision("", crb.RoleRef.Name))) - digest.Write(null) - } - - for _, rb := range p.getRoleBindings(subjectName) { - switch rb.RoleRef.Kind { - case "Role": - digest.Write([]byte(rb.RoleRef.Name)) - digest.Write([]byte(rb.Namespace)) - digest.Write([]byte(p.revisions.roleRevision(rb.Namespace, rb.RoleRef.Name))) - digest.Write(null) - case "ClusterRole": - digest.Write([]byte(rb.RoleRef.Name)) - digest.Write([]byte(p.revisions.roleRevision("", rb.RoleRef.Name))) - digest.Write(null) - } - } -} - func (p *policyRuleIndex) get(subjectName string) *AccessSet { result := &AccessSet{}