Merge pull request #5656 from gyuho/auth_bytes

make auth key, rangeEnd typed like mvcc ([]byte)

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) {

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")}},
 		},
 	}
 

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)
 }
 

etcdserver/apply.go

@@ -92,13 +92,13 @@ func (s *EtcdServer) applyV3Request(r *pb.InternalRaftRequest) *applyResult {
 
 	switch {
 	case r.Range != nil:
-		if s.AuthStore().IsRangePermitted(r.Header, string(r.Range.Key), string(r.Range.RangeEnd)) {
+		if s.AuthStore().IsRangePermitted(r.Header, r.Range.Key, r.Range.RangeEnd) {
 			ar.resp, ar.err = s.applyV3.Range(noTxn, r.Range)
 		} else {
 			ar.err = auth.ErrPermissionDenied
 		}
 	case r.Put != nil:
-		if s.AuthStore().IsPutPermitted(r.Header, string(r.Put.Key)) {
+		if s.AuthStore().IsPutPermitted(r.Header, r.Put.Key) {
 			ar.resp, ar.err = s.applyV3.Put(noTxn, r.Put)
 		} else {
 			ar.err = auth.ErrPermissionDenied

etcdserver/v3_server.go

@@ -82,7 +82,7 @@ func (s *EtcdServer) Range(ctx context.Context, r *pb.RangeRequest) (*pb.RangeRe
 			return nil, err
 		}
 		hdr := &pb.RequestHeader{Username: user}
-		if !s.AuthStore().IsRangePermitted(hdr, string(r.Key), string(r.RangeEnd)) {
+		if !s.AuthStore().IsRangePermitted(hdr, r.Key, r.RangeEnd) {
 			return nil, auth.ErrPermissionDenied
 		}
 		return s.applyV3.Range(noTxn, r)