diff --git a/auth/range_perm_cache.go b/auth/range_perm_cache.go index 613deecbf..a3b2ebea9 100644 --- a/auth/range_perm_cache.go +++ b/auth/range_perm_cache.go @@ -22,9 +22,20 @@ import ( "github.com/coreos/etcd/mvcc/backend" ) +// isSubset returns true if a is a subset of b func isSubset(a, b *rangePerm) bool { - // return true if a is a subset of b - return 0 <= bytes.Compare(a.begin, b.begin) && bytes.Compare(a.end, b.end) <= 0 + switch { + case len(a.end) == 0 && len(b.end) == 0: + // a, b are both keys + return bytes.Equal(a.begin, b.begin) + case len(b.end) == 0: + // b is a key, a is a range + return false + case len(a.end) == 0: + return 0 <= bytes.Compare(a.begin, b.begin) && bytes.Compare(a.begin, b.end) <= 0 + default: + return 0 <= bytes.Compare(a.begin, b.begin) && bytes.Compare(a.end, b.end) <= 0 + } } // removeSubsetRangePerms removes any rangePerms that are subsets of other rangePerms. @@ -89,9 +100,6 @@ func getMergedPerms(tx backend.BatchTx, userName string) *unifiedRangePermission } for _, perm := range role.KeyPermission { - if len(perm.RangeEnd) == 0 { - continue - } rp := &rangePerm{begin: perm.Key, end: perm.RangeEnd} switch perm.PermType { @@ -126,15 +134,10 @@ func checkKeyPerm(cachedPerms *unifiedRangePermissions, key, rangeEnd []byte, pe plog.Panicf("unknown auth type: %v", permtyp) } - for _, perm := range tocheck { - // check permission of a single key - if len(rangeEnd) == 0 { - if bytes.Compare(perm.begin, key) <= 0 && bytes.Compare(rangeEnd, perm.end) <= 0 { - return true - } - } + requiredPerm := &rangePerm{begin: key, end: rangeEnd} - if bytes.Compare(perm.begin, key) <= 0 && bytes.Compare(perm.end, key) >= 0 { + for _, perm := range tocheck { + if isSubset(requiredPerm, perm) { return true } } diff --git a/auth/range_perm_cache_test.go b/auth/range_perm_cache_test.go index 2df3e6848..7f7dff6e3 100644 --- a/auth/range_perm_cache_test.go +++ b/auth/range_perm_cache_test.go @@ -37,7 +37,7 @@ func isPermsEqual(a, b []*rangePerm) bool { return true } -func TestUnifyParams(t *testing.T) { +func TestGetMergedPerms(t *testing.T) { tests := []struct { params []*rangePerm want []*rangePerm @@ -86,6 +86,28 @@ func TestUnifyParams(t *testing.T) { []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("b"), []byte("c")}, {[]byte("c"), []byte("d")}, {[]byte("d"), []byte("f")}, {[]byte("1"), []byte("9")}}, []*rangePerm{{[]byte("1"), []byte("9")}, {[]byte("a"), []byte("f")}}, }, + // overlapping + { + []*rangePerm{{[]byte("a"), []byte("f")}, {[]byte("b"), []byte("g")}}, + []*rangePerm{{[]byte("a"), []byte("g")}}, + }, + // keys + { + []*rangePerm{{[]byte("a"), []byte("")}, {[]byte("b"), []byte("")}}, + []*rangePerm{{[]byte("a"), []byte("")}, {[]byte("b"), []byte("")}}, + }, + { + []*rangePerm{{[]byte("a"), []byte("")}, {[]byte("a"), []byte("c")}}, + []*rangePerm{{[]byte("a"), []byte("c")}}, + }, + { + []*rangePerm{{[]byte("a"), []byte("")}, {[]byte("a"), []byte("c")}, {[]byte("b"), []byte("")}}, + []*rangePerm{{[]byte("a"), []byte("c")}}, + }, + { + []*rangePerm{{[]byte("a"), []byte("")}, {[]byte("b"), []byte("c")}, {[]byte("b"), []byte("")}, {[]byte("c"), []byte("")}, {[]byte("d"), []byte("")}}, + []*rangePerm{{[]byte("a"), []byte("")}, {[]byte("b"), []byte("c")}, {[]byte("d"), []byte("")}}, + }, } for i, tt := range tests { diff --git a/auth/store.go b/auth/store.go index 653fd78b7..62feed053 100644 --- a/auth/store.go +++ b/auth/store.go @@ -560,29 +560,6 @@ func (as *authStore) isOpPermitted(userName string, key, rangeEnd []byte, permTy return false } - if len(rangeEnd) == 0 { - for _, roleName := range user.Roles { - role := getRole(tx, roleName) - if role == nil { - continue - } - - for _, perm := range role.KeyPermission { - if !bytes.Equal(perm.Key, key) { - continue - } - - if perm.PermType == authpb.READWRITE { - return true - } - - if permTyp == perm.PermType { - return true - } - } - } - } - if as.isRangeOpPermitted(tx, userName, key, rangeEnd, permTyp) { return true }