diff --git a/auth/range_perm_cache.go b/auth/range_perm_cache.go index 45a46daa4..613deecbf 100644 --- a/auth/range_perm_cache.go +++ b/auth/range_perm_cache.go @@ -15,8 +15,8 @@ package auth import ( + "bytes" "sort" - "strings" "github.com/coreos/etcd/auth/authpb" "github.com/coreos/etcd/mvcc/backend" @@ -24,7 +24,7 @@ import ( func isSubset(a, b *rangePerm) bool { // return true if a is a subset of b - return 0 <= strings.Compare(a.begin, b.begin) && strings.Compare(a.end, b.end) <= 0 + 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. @@ -61,7 +61,7 @@ func mergeRangePerms(perms []*rangePerm) []*rangePerm { i := 0 for i < len(perms) { begin, next := i, i - for next+1 < len(perms) && perms[next].end >= perms[next+1].begin { + for next+1 < len(perms) && bytes.Compare(perms[next].end, perms[next+1].begin) != -1 { next++ } @@ -92,7 +92,7 @@ func getMergedPerms(tx backend.BatchTx, userName string) *unifiedRangePermission if len(perm.RangeEnd) == 0 { continue } - rp := &rangePerm{begin: string(perm.Key), end: string(perm.RangeEnd)} + rp := &rangePerm{begin: perm.Key, end: perm.RangeEnd} switch perm.PermType { case authpb.READWRITE: @@ -114,7 +114,7 @@ func getMergedPerms(tx backend.BatchTx, userName string) *unifiedRangePermission } } -func checkKeyPerm(cachedPerms *unifiedRangePermissions, key, rangeEnd string, permtyp authpb.Permission_Type) bool { +func checkKeyPerm(cachedPerms *unifiedRangePermissions, key, rangeEnd []byte, permtyp authpb.Permission_Type) bool { var tocheck []*rangePerm switch permtyp { @@ -129,12 +129,12 @@ func checkKeyPerm(cachedPerms *unifiedRangePermissions, key, rangeEnd string, pe for _, perm := range tocheck { // check permission of a single key if len(rangeEnd) == 0 { - if strings.Compare(perm.begin, key) <= 0 && strings.Compare(rangeEnd, perm.end) <= 0 { + if bytes.Compare(perm.begin, key) <= 0 && bytes.Compare(rangeEnd, perm.end) <= 0 { return true } } - if strings.Compare(perm.begin, key) <= 0 && strings.Compare(perm.end, key) >= 0 { + if bytes.Compare(perm.begin, key) <= 0 && bytes.Compare(perm.end, key) >= 0 { return true } } @@ -142,7 +142,7 @@ func checkKeyPerm(cachedPerms *unifiedRangePermissions, key, rangeEnd string, pe return false } -func (as *authStore) isRangeOpPermitted(tx backend.BatchTx, userName string, key, rangeEnd string, permtyp authpb.Permission_Type) bool { +func (as *authStore) isRangeOpPermitted(tx backend.BatchTx, userName string, key, rangeEnd []byte, permtyp authpb.Permission_Type) bool { // assumption: tx is Lock()ed _, ok := as.rangePermCache[userName] if !ok { @@ -173,7 +173,7 @@ type unifiedRangePermissions struct { } type rangePerm struct { - begin, end string + begin, end []byte } type RangePermSliceByBegin []*rangePerm @@ -183,10 +183,16 @@ func (slice RangePermSliceByBegin) Len() int { } func (slice RangePermSliceByBegin) Less(i, j int) bool { - if slice[i].begin == slice[j].begin { - return slice[i].end < slice[j].end + switch bytes.Compare(slice[i].begin, slice[j].begin) { + case 0: // begin(i) == begin(j) + return bytes.Compare(slice[i].end, slice[j].end) == -1 + + case -1: // begin(i) < begin(j) + return true + + default: + return false } - return slice[i].begin < slice[j].begin } func (slice RangePermSliceByBegin) Swap(i, j int) { diff --git a/auth/range_perm_cache_test.go b/auth/range_perm_cache_test.go index 8c33532ae..2df3e6848 100644 --- a/auth/range_perm_cache_test.go +++ b/auth/range_perm_cache_test.go @@ -15,6 +15,7 @@ package auth import ( + "bytes" "testing" ) @@ -28,7 +29,7 @@ func isPermsEqual(a, b []*rangePerm) bool { return false } - if a[i].begin != b[i].begin || a[i].end != b[i].end { + if !bytes.Equal(a[i].begin, b[i].begin) || !bytes.Equal(a[i].end, b[i].end) { return false } } @@ -42,48 +43,48 @@ func TestUnifyParams(t *testing.T) { want []*rangePerm }{ { - []*rangePerm{{"a", "b"}}, - []*rangePerm{{"a", "b"}}, + []*rangePerm{{[]byte("a"), []byte("b")}}, + []*rangePerm{{[]byte("a"), []byte("b")}}, }, { - []*rangePerm{{"a", "b"}, {"b", "c"}}, - []*rangePerm{{"a", "c"}}, + []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("b"), []byte("c")}}, + []*rangePerm{{[]byte("a"), []byte("c")}}, }, { - []*rangePerm{{"a", "c"}, {"b", "d"}}, - []*rangePerm{{"a", "d"}}, + []*rangePerm{{[]byte("a"), []byte("c")}, {[]byte("b"), []byte("d")}}, + []*rangePerm{{[]byte("a"), []byte("d")}}, }, { - []*rangePerm{{"a", "b"}, {"b", "c"}, {"d", "e"}}, - []*rangePerm{{"a", "c"}, {"d", "e"}}, + []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("b"), []byte("c")}, {[]byte("d"), []byte("e")}}, + []*rangePerm{{[]byte("a"), []byte("c")}, {[]byte("d"), []byte("e")}}, }, { - []*rangePerm{{"a", "b"}, {"c", "d"}, {"e", "f"}}, - []*rangePerm{{"a", "b"}, {"c", "d"}, {"e", "f"}}, + []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("e"), []byte("f")}}, + []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("e"), []byte("f")}}, }, { - []*rangePerm{{"e", "f"}, {"c", "d"}, {"a", "b"}}, - []*rangePerm{{"a", "b"}, {"c", "d"}, {"e", "f"}}, + []*rangePerm{{[]byte("e"), []byte("f")}, {[]byte("c"), []byte("d")}, {[]byte("a"), []byte("b")}}, + []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("e"), []byte("f")}}, }, { - []*rangePerm{{"a", "b"}, {"c", "d"}, {"a", "z"}}, - []*rangePerm{{"a", "z"}}, + []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("a"), []byte("z")}}, + []*rangePerm{{[]byte("a"), []byte("z")}}, }, { - []*rangePerm{{"a", "b"}, {"c", "d"}, {"a", "z"}, {"1", "9"}}, - []*rangePerm{{"1", "9"}, {"a", "z"}}, + []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("a"), []byte("z")}, {[]byte("1"), []byte("9")}}, + []*rangePerm{{[]byte("1"), []byte("9")}, {[]byte("a"), []byte("z")}}, }, { - []*rangePerm{{"a", "b"}, {"c", "d"}, {"a", "z"}, {"1", "a"}}, - []*rangePerm{{"1", "z"}}, + []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("a"), []byte("z")}, {[]byte("1"), []byte("a")}}, + []*rangePerm{{[]byte("1"), []byte("z")}}, }, { - []*rangePerm{{"a", "b"}, {"a", "z"}, {"5", "6"}, {"1", "9"}}, - []*rangePerm{{"1", "9"}, {"a", "z"}}, + []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("a"), []byte("z")}, {[]byte("5"), []byte("6")}, {[]byte("1"), []byte("9")}}, + []*rangePerm{{[]byte("1"), []byte("9")}, {[]byte("a"), []byte("z")}}, }, { - []*rangePerm{{"a", "b"}, {"b", "c"}, {"c", "d"}, {"d", "f"}, {"1", "9"}}, - []*rangePerm{{"1", "9"}, {"a", "f"}}, + []*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")}}, }, } diff --git a/auth/store.go b/auth/store.go index 3148cef24..653fd78b7 100644 --- a/auth/store.go +++ b/auth/store.go @@ -108,10 +108,10 @@ type AuthStore interface { UsernameFromToken(token string) (string, bool) // IsPutPermitted checks put permission of the user - IsPutPermitted(header *pb.RequestHeader, key string) bool + IsPutPermitted(header *pb.RequestHeader, key []byte) bool // IsRangePermitted checks range permission of the user - IsRangePermitted(header *pb.RequestHeader, key, rangeEnd string) bool + IsRangePermitted(header *pb.RequestHeader, key, rangeEnd []byte) bool // IsAdminPermitted checks admin permission of the user IsAdminPermitted(username string) bool @@ -544,7 +544,7 @@ func (as *authStore) RoleGrantPermission(r *pb.AuthRoleGrantPermissionRequest) ( return &pb.AuthRoleGrantPermissionResponse{}, nil } -func (as *authStore) isOpPermitted(userName string, key, rangeEnd string, permTyp authpb.Permission_Type) bool { +func (as *authStore) isOpPermitted(userName string, key, rangeEnd []byte, permTyp authpb.Permission_Type) bool { // TODO(mitake): this function would be costly so we need a caching mechanism if !as.isAuthEnabled() { return true @@ -560,7 +560,7 @@ func (as *authStore) isOpPermitted(userName string, key, rangeEnd string, permTy return false } - if strings.Compare(rangeEnd, "") == 0 { + if len(rangeEnd) == 0 { for _, roleName := range user.Roles { role := getRole(tx, roleName) if role == nil { @@ -568,7 +568,7 @@ func (as *authStore) isOpPermitted(userName string, key, rangeEnd string, permTy } for _, perm := range role.KeyPermission { - if !bytes.Equal(perm.Key, []byte(key)) { + if !bytes.Equal(perm.Key, key) { continue } @@ -590,11 +590,11 @@ func (as *authStore) isOpPermitted(userName string, key, rangeEnd string, permTy return false } -func (as *authStore) IsPutPermitted(header *pb.RequestHeader, key string) bool { - return as.isOpPermitted(header.Username, key, "", authpb.WRITE) +func (as *authStore) IsPutPermitted(header *pb.RequestHeader, key []byte) bool { + return as.isOpPermitted(header.Username, key, nil, authpb.WRITE) } -func (as *authStore) IsRangePermitted(header *pb.RequestHeader, key, rangeEnd string) bool { +func (as *authStore) IsRangePermitted(header *pb.RequestHeader, key, rangeEnd []byte) bool { return as.isOpPermitted(header.Username, key, rangeEnd, authpb.READ) }