Mirroristas/etcd
2
0
Fork 0
mirror of https://github.com/etcd-io/etcd.git synced 2024-09-27 06:25:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #15619 from mitake/3.5-backport-15294

Browse Source 
[3.5] backport 15294
This commit is contained in:
Marek Siarkowicz 2023-04-07 17:47:23 +02:00 committed by GitHub
commit 4501fd88c7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 309 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
server/auth/range_perm_cache.go
View File

@ -76,8 +76,10 @@ func checkKeyInterval(
cachedPerms *unifiedRangePermissions,
key, rangeEnd []byte,
permtyp authpb.Permission_Type) bool {
if len(rangeEnd) == 1 && rangeEnd[0] == 0 {
if isOpenEnded(rangeEnd) {
rangeEnd = nil
// nil rangeEnd will be converetd to []byte{}, the largest element of BytesAffineComparable,
// in NewBytesAffineInterval().
}
ivl := adt.NewBytesAffineInterval(key, rangeEnd)
@ -153,3 +155,51 @@ type unifiedRangePermissions struct {
readPerms adt.IntervalTree
writePerms adt.IntervalTree
}
// Constraints related to key range
// Assumptions:
// a1. key must be non-nil
// a2. []byte{} (in the case of string, "") is not a valid key of etcd
// For representing an open-ended range, BytesAffineComparable uses []byte{} as the largest element.
// a3. []byte{0x00} is the minimum valid etcd key
//
// Based on the above assumptions, key and rangeEnd must follow below rules:
// b1. for representing a single key point, rangeEnd should be nil or zero length byte array (in the case of string, "")
// Rule a2 guarantees that (X, []byte{}) for any X is not a valid range. So such ranges can be used for representing
// a single key permission.
//
// b2. key range with upper limit, like (X, Y), larger or equal to X and smaller than Y
//
// b3. key range with open-ended, like (X, <open ended>), is represented like (X, []byte{0x00})
// Because of rule a3, if we have (X, []byte{0x00}), such a range represents an empty range and makes no sense to have
// such a permission. So we use []byte{0x00} for representing an open-ended permission.
// Note that rangeEnd with []byte{0x00} will be converted into []byte{} before inserted into the interval tree
// (rule a2 ensures that this is the largest element).
// Special range like key = []byte{0x00} and rangeEnd = []byte{0x00} is treated as a range which matches with all keys.
//
// Treating a range whose rangeEnd with []byte{0x00} as an open-ended comes from the rules of Range() and Watch() API.
func isOpenEnded(rangeEnd []byte) bool { // check rule b3
return len(rangeEnd) == 1 && rangeEnd[0] == 0
}
func isValidPermissionRange(key, rangeEnd []byte) bool {
if len(key) == 0 {
return false
}
if rangeEnd == nil || len(rangeEnd) == 0 { // ensure rule b1
return true
}
begin := adt.BytesAffineComparable(key)
end := adt.BytesAffineComparable(rangeEnd)
if begin.Compare(end) == -1 { // rule b2
return true
}
if isOpenEnded(rangeEnd) {
return true
}
return false
}

115
server/auth/range_perm_cache_test.go
View File

@ -45,6 +45,26 @@ func TestRangePermission(t *testing.T) {
[]byte("a"), []byte("f"),
true,
},
{
[]adt.Interval{adt.NewBytesAffineInterval([]byte("a"), []byte("d")), adt.NewBytesAffineInterval([]byte("a"), []byte("b")), adt.NewBytesAffineInterval([]byte("c"), []byte("f"))},
[]byte("a"), []byte{},
false,
},
{
[]adt.Interval{adt.NewBytesAffineInterval([]byte("a"), []byte{})},
[]byte("a"), []byte{},
true,
},
{
[]adt.Interval{adt.NewBytesAffineInterval([]byte{0x00}, []byte{})},
[]byte("a"), []byte{},
true,
},
{
[]adt.Interval{adt.NewBytesAffineInterval([]byte{0x00}, []byte{})},
[]byte{0x00}, []byte{},
true,
},
}
for i, tt := range tests {
@ -86,6 +106,16 @@ func TestKeyPermission(t *testing.T) {
[]byte("f"),
false,
},
{
[]adt.Interval{adt.NewBytesAffineInterval([]byte("a"), []byte("d")), adt.NewBytesAffineInterval([]byte("a"), []byte("b")), adt.NewBytesAffineInterval([]byte("c"), []byte{})},
[]byte("f"),
true,
},
{
[]adt.Interval{adt.NewBytesAffineInterval([]byte("a"), []byte("d")), adt.NewBytesAffineInterval([]byte("a"), []byte("b")), adt.NewBytesAffineInterval([]byte{0x00}, []byte{})},
[]byte("f"),
true,
},
}
for i, tt := range tests {
@ -100,3 +130,88 @@ func TestKeyPermission(t *testing.T) {
}
}
}
func TestRangeCheck(t *testing.T) {
tests := []struct {
name string
key []byte
rangeEnd []byte
want bool
}{
{
name: "valid single key",
key: []byte("a"),
rangeEnd: []byte(""),
want: true,
},
{
name: "valid single key",
key: []byte("a"),
rangeEnd: nil,
want: true,
},
{
name: "valid key range, key < rangeEnd",
key: []byte("a"),
rangeEnd: []byte("b"),
want: true,
},
{
name: "invalid empty key range, key == rangeEnd",
key: []byte("a"),
rangeEnd: []byte("a"),
want: false,
},
{
name: "invalid empty key range, key > rangeEnd",
key: []byte("b"),
rangeEnd: []byte("a"),
want: false,
},
{
name: "invalid key, key must not be \"\"",
key: []byte(""),
rangeEnd: []byte("a"),
want: false,
},
{
name: "invalid key range, key must not be \"\"",
key: []byte(""),
rangeEnd: []byte(""),
want: false,
},
{
name: "invalid key range, key must not be \"\"",
key: []byte(""),
rangeEnd: []byte("\x00"),
want: false,
},
{
name: "valid single key (not useful in practice)",
key: []byte("\x00"),
rangeEnd: []byte(""),
want: true,
},
{
name: "valid key range, larger or equals to \"a\"",
key: []byte("a"),
rangeEnd: []byte("\x00"),
want: true,
},
{
name: "valid key range, which includes all keys",
key: []byte("\x00"),
rangeEnd: []byte("\x00"),
want: true,
},
}
for i, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := isValidPermissionRange(tt.key, tt.rangeEnd)
if result != tt.want {
t.Errorf("#%d: result=%t, want=%t", i, result, tt.want)
}
})
}
}

3
server/auth/store.go
View File

@ -791,6 +791,9 @@ func (as *authStore) RoleGrantPermission(r *pb.AuthRoleGrantPermissionRequest) (
if r.Perm == nil {
return nil, ErrPermissionNotGiven
}
if !isValidPermissionRange(r.Perm.Key, r.Perm.RangeEnd) {
return nil, ErrInvalidAuthMgmt
}
tx := as.be.BatchTx()
tx.LockInsideApply()

139
server/auth/store_test.go
View File

@ -17,6 +17,7 @@ package auth
import (
"context"
"encoding/base64"
"errors"
"fmt"
"reflect"
"strings"
@ -541,6 +542,144 @@ func TestRoleGrantPermission(t *testing.T) {
}
}
func TestRoleGrantInvalidPermission(t *testing.T) {
as, tearDown := setupAuthStore(t)
defer tearDown(t)
_, err := as.RoleAdd(&pb.AuthRoleAddRequest{Name: "role-test-1"})
if err != nil {
t.Fatal(err)
}
tests := []struct {
name string
perm *authpb.Permission
want error
}{
{
name: "valid range",
perm: &authpb.Permission{
PermType: authpb.WRITE,
Key: []byte("Keys"),
RangeEnd: []byte("RangeEnd"),
},
want: nil,
},
{
name: "invalid range: nil key",
perm: &authpb.Permission{
PermType: authpb.WRITE,
Key: nil,
RangeEnd: []byte("RangeEnd"),
},
want: ErrInvalidAuthMgmt,
},
{
name: "valid range: single key",
perm: &authpb.Permission{
PermType: authpb.WRITE,
Key: []byte("Keys"),
RangeEnd: nil,
},
want: nil,
},
{
name: "valid range: single key",
perm: &authpb.Permission{
PermType: authpb.WRITE,
Key: []byte("Keys"),
RangeEnd: []byte{},
},
want: nil,
},
{
name: "invalid range: empty (Key == RangeEnd)",
perm: &authpb.Permission{
PermType: authpb.WRITE,
Key: []byte("a"),
RangeEnd: []byte("a"),
},
want: ErrInvalidAuthMgmt,
},
{
name: "invalid range: empty (Key > RangeEnd)",
perm: &authpb.Permission{
PermType: authpb.WRITE,
Key: []byte("b"),
RangeEnd: []byte("a"),
},
want: ErrInvalidAuthMgmt,
},
{
name: "invalid range: length of key is 0",
perm: &authpb.Permission{
PermType: authpb.WRITE,
Key: []byte(""),
RangeEnd: []byte("a"),
},
want: ErrInvalidAuthMgmt,
},
{
name: "invalid range: length of key is 0",
perm: &authpb.Permission{
PermType: authpb.WRITE,
Key: []byte(""),
RangeEnd: []byte(""),
},
want: ErrInvalidAuthMgmt,
},
{
name: "invalid range: length of key is 0",
perm: &authpb.Permission{
PermType: authpb.WRITE,
Key: []byte(""),
RangeEnd: []byte{0x00},
},
want: ErrInvalidAuthMgmt,
},
{
name: "valid range: single key permission for []byte{0x00}",
perm: &authpb.Permission{
PermType: authpb.WRITE,
Key: []byte{0x00},
RangeEnd: []byte(""),
},
want: nil,
},
{
name: "valid range: \"a\" or larger keys",
perm: &authpb.Permission{
PermType: authpb.WRITE,
Key: []byte("a"),
RangeEnd: []byte{0x00},
},
want: nil,
},
{
name: "valid range: the entire keys",
perm: &authpb.Permission{
PermType: authpb.WRITE,
Key: []byte{0x00},
RangeEnd: []byte{0x00},
},
want: nil,
},
}
for i, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err = as.RoleGrantPermission(&pb.AuthRoleGrantPermissionRequest{
Name: "role-test-1",
Perm: tt.perm,
})
if !errors.Is(err, tt.want) {
t.Errorf("#%d: result=%t, want=%t", i, err, tt.want)
}
})
}
}
func TestRoleRevokePermission(t *testing.T) {
as, tearDown := setupAuthStore(t)
defer tearDown(t)

2
tests/integration/v3_auth_test.go
View File

@ -406,7 +406,7 @@ func TestV3AuthOldRevConcurrent(t *testing.T) {
role, user := fmt.Sprintf("test-role-%d", i), fmt.Sprintf("test-user-%d", i)
_, err := c.RoleAdd(context.TODO(), role)
testutil.AssertNil(t, err)
_, err = c.RoleGrantPermission(context.TODO(), role, "", clientv3.GetPrefixRangeEnd(""), clientv3.PermissionType(clientv3.PermReadWrite))
_, err = c.RoleGrantPermission(context.TODO(), role, "\x00", clientv3.GetPrefixRangeEnd(""), clientv3.PermissionType(clientv3.PermReadWrite))
testutil.AssertNil(t, err)
_, err = c.UserAdd(context.TODO(), user, "123")
testutil.AssertNil(t, err)