auth: key, range in []byte type

Fix https://github.com/coreos/etcd/issues/5655.
This commit is contained in:
Gyu-Ho Lee 2016-06-13 14:17:10 -07:00
parent 70a2add2b0
commit e9d2eb2b54
3 changed files with 50 additions and 43 deletions

View File

@ -15,8 +15,8 @@
package auth package auth
import ( import (
"bytes"
"sort" "sort"
"strings"
"github.com/coreos/etcd/auth/authpb" "github.com/coreos/etcd/auth/authpb"
"github.com/coreos/etcd/mvcc/backend" "github.com/coreos/etcd/mvcc/backend"
@ -24,7 +24,7 @@ import (
func isSubset(a, b *rangePerm) bool { func isSubset(a, b *rangePerm) bool {
// return true if a is a subset of b // 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. // removeSubsetRangePerms removes any rangePerms that are subsets of other rangePerms.
@ -61,7 +61,7 @@ func mergeRangePerms(perms []*rangePerm) []*rangePerm {
i := 0 i := 0
for i < len(perms) { for i < len(perms) {
begin, next := i, i 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++ next++
} }
@ -92,7 +92,7 @@ func getMergedPerms(tx backend.BatchTx, userName string) *unifiedRangePermission
if len(perm.RangeEnd) == 0 { if len(perm.RangeEnd) == 0 {
continue continue
} }
rp := &rangePerm{begin: string(perm.Key), end: string(perm.RangeEnd)} rp := &rangePerm{begin: perm.Key, end: perm.RangeEnd}
switch perm.PermType { switch perm.PermType {
case authpb.READWRITE: 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 var tocheck []*rangePerm
switch permtyp { switch permtyp {
@ -129,12 +129,12 @@ func checkKeyPerm(cachedPerms *unifiedRangePermissions, key, rangeEnd string, pe
for _, perm := range tocheck { for _, perm := range tocheck {
// check permission of a single key // check permission of a single key
if len(rangeEnd) == 0 { 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 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 return true
} }
} }
@ -142,7 +142,7 @@ func checkKeyPerm(cachedPerms *unifiedRangePermissions, key, rangeEnd string, pe
return false 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 // assumption: tx is Lock()ed
_, ok := as.rangePermCache[userName] _, ok := as.rangePermCache[userName]
if !ok { if !ok {
@ -173,7 +173,7 @@ type unifiedRangePermissions struct {
} }
type rangePerm struct { type rangePerm struct {
begin, end string begin, end []byte
} }
type RangePermSliceByBegin []*rangePerm type RangePermSliceByBegin []*rangePerm
@ -183,10 +183,16 @@ func (slice RangePermSliceByBegin) Len() int {
} }
func (slice RangePermSliceByBegin) Less(i, j int) bool { func (slice RangePermSliceByBegin) Less(i, j int) bool {
if slice[i].begin == slice[j].begin { switch bytes.Compare(slice[i].begin, slice[j].begin) {
return slice[i].end < slice[j].end 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) { func (slice RangePermSliceByBegin) Swap(i, j int) {

View File

@ -15,6 +15,7 @@
package auth package auth
import ( import (
"bytes"
"testing" "testing"
) )
@ -28,7 +29,7 @@ func isPermsEqual(a, b []*rangePerm) bool {
return false 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 return false
} }
} }
@ -42,48 +43,48 @@ func TestUnifyParams(t *testing.T) {
want []*rangePerm want []*rangePerm
}{ }{
{ {
[]*rangePerm{{"a", "b"}}, []*rangePerm{{[]byte("a"), []byte("b")}},
[]*rangePerm{{"a", "b"}}, []*rangePerm{{[]byte("a"), []byte("b")}},
}, },
{ {
[]*rangePerm{{"a", "b"}, {"b", "c"}}, []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("b"), []byte("c")}},
[]*rangePerm{{"a", "c"}}, []*rangePerm{{[]byte("a"), []byte("c")}},
}, },
{ {
[]*rangePerm{{"a", "c"}, {"b", "d"}}, []*rangePerm{{[]byte("a"), []byte("c")}, {[]byte("b"), []byte("d")}},
[]*rangePerm{{"a", "d"}}, []*rangePerm{{[]byte("a"), []byte("d")}},
}, },
{ {
[]*rangePerm{{"a", "b"}, {"b", "c"}, {"d", "e"}}, []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("b"), []byte("c")}, {[]byte("d"), []byte("e")}},
[]*rangePerm{{"a", "c"}, {"d", "e"}}, []*rangePerm{{[]byte("a"), []byte("c")}, {[]byte("d"), []byte("e")}},
}, },
{ {
[]*rangePerm{{"a", "b"}, {"c", "d"}, {"e", "f"}}, []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("e"), []byte("f")}},
[]*rangePerm{{"a", "b"}, {"c", "d"}, {"e", "f"}}, []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("e"), []byte("f")}},
}, },
{ {
[]*rangePerm{{"e", "f"}, {"c", "d"}, {"a", "b"}}, []*rangePerm{{[]byte("e"), []byte("f")}, {[]byte("c"), []byte("d")}, {[]byte("a"), []byte("b")}},
[]*rangePerm{{"a", "b"}, {"c", "d"}, {"e", "f"}}, []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("e"), []byte("f")}},
}, },
{ {
[]*rangePerm{{"a", "b"}, {"c", "d"}, {"a", "z"}}, []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("a"), []byte("z")}},
[]*rangePerm{{"a", "z"}}, []*rangePerm{{[]byte("a"), []byte("z")}},
}, },
{ {
[]*rangePerm{{"a", "b"}, {"c", "d"}, {"a", "z"}, {"1", "9"}}, []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("a"), []byte("z")}, {[]byte("1"), []byte("9")}},
[]*rangePerm{{"1", "9"}, {"a", "z"}}, []*rangePerm{{[]byte("1"), []byte("9")}, {[]byte("a"), []byte("z")}},
}, },
{ {
[]*rangePerm{{"a", "b"}, {"c", "d"}, {"a", "z"}, {"1", "a"}}, []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("a"), []byte("z")}, {[]byte("1"), []byte("a")}},
[]*rangePerm{{"1", "z"}}, []*rangePerm{{[]byte("1"), []byte("z")}},
}, },
{ {
[]*rangePerm{{"a", "b"}, {"a", "z"}, {"5", "6"}, {"1", "9"}}, []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("a"), []byte("z")}, {[]byte("5"), []byte("6")}, {[]byte("1"), []byte("9")}},
[]*rangePerm{{"1", "9"}, {"a", "z"}}, []*rangePerm{{[]byte("1"), []byte("9")}, {[]byte("a"), []byte("z")}},
}, },
{ {
[]*rangePerm{{"a", "b"}, {"b", "c"}, {"c", "d"}, {"d", "f"}, {"1", "9"}}, []*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("b"), []byte("c")}, {[]byte("c"), []byte("d")}, {[]byte("d"), []byte("f")}, {[]byte("1"), []byte("9")}},
[]*rangePerm{{"1", "9"}, {"a", "f"}}, []*rangePerm{{[]byte("1"), []byte("9")}, {[]byte("a"), []byte("f")}},
}, },
} }

View File

@ -108,10 +108,10 @@ type AuthStore interface {
UsernameFromToken(token string) (string, bool) UsernameFromToken(token string) (string, bool)
// IsPutPermitted checks put permission of the user // 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 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 checks admin permission of the user
IsAdminPermitted(username string) bool IsAdminPermitted(username string) bool
@ -544,7 +544,7 @@ func (as *authStore) RoleGrantPermission(r *pb.AuthRoleGrantPermissionRequest) (
return &pb.AuthRoleGrantPermissionResponse{}, nil 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 // TODO(mitake): this function would be costly so we need a caching mechanism
if !as.isAuthEnabled() { if !as.isAuthEnabled() {
return true return true
@ -560,7 +560,7 @@ func (as *authStore) isOpPermitted(userName string, key, rangeEnd string, permTy
return false return false
} }
if strings.Compare(rangeEnd, "") == 0 { if len(rangeEnd) == 0 {
for _, roleName := range user.Roles { for _, roleName := range user.Roles {
role := getRole(tx, roleName) role := getRole(tx, roleName)
if role == nil { if role == nil {
@ -568,7 +568,7 @@ func (as *authStore) isOpPermitted(userName string, key, rangeEnd string, permTy
} }
for _, perm := range role.KeyPermission { for _, perm := range role.KeyPermission {
if !bytes.Equal(perm.Key, []byte(key)) { if !bytes.Equal(perm.Key, key) {
continue continue
} }
@ -590,11 +590,11 @@ func (as *authStore) isOpPermitted(userName string, key, rangeEnd string, permTy
return false return false
} }
func (as *authStore) IsPutPermitted(header *pb.RequestHeader, key string) bool { func (as *authStore) IsPutPermitted(header *pb.RequestHeader, key []byte) bool {
return as.isOpPermitted(header.Username, key, "", authpb.WRITE) 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) return as.isOpPermitted(header.Username, key, rangeEnd, authpb.READ)
} }