Introduces new 'list' permission that applies to KV store recursive reads, and enforced only when opted in.

This commit is contained in:
Preetha Appan 2017-10-02 17:10:21 -05:00
parent 7c7927ff3d
commit d5acfc3982
11 changed files with 299 additions and 34 deletions

View File

@ -60,6 +60,9 @@ type ACL interface {
// EventWrite determines if a specific event may be fired.
EventWrite(string) bool
// KeyList checks for permission to list keys under a prefix
KeyList(string) bool
// KeyRead checks for permission to read a given key
KeyRead(string) bool
@ -155,6 +158,10 @@ func (s *StaticACL) KeyRead(string) bool {
return s.defaultAllow
}
func (s *StaticACL) KeyList(string) bool {
return s.defaultAllow
}
func (s *StaticACL) KeyWrite(string, sentinel.ScopeFn) bool {
return s.defaultAllow
}
@ -455,7 +462,7 @@ func (p *PolicyACL) KeyRead(key string) bool {
if ok {
pr := rule.(PolicyRule)
switch pr.aclPolicy {
case PolicyRead, PolicyWrite:
case PolicyRead, PolicyWrite, PolicyList:
return true
default:
return false
@ -466,6 +473,24 @@ func (p *PolicyACL) KeyRead(key string) bool {
return p.parent.KeyRead(key)
}
// KeyList returns if a key is allowed to be listed
func (p *PolicyACL) KeyList(key string) bool {
// Look for a matching rule
_, rule, ok := p.keyRules.LongestPrefix(key)
if ok {
pr := rule.(PolicyRule)
switch pr.aclPolicy {
case PolicyList, PolicyWrite:
return true
default:
return false
}
}
// No matching rule, use the parent.
return p.parent.KeyList(key)
}
// KeyWrite returns if a key is allowed to be written
func (p *PolicyACL) KeyWrite(key string, scope sentinel.ScopeFn) bool {
// Look for a matching rule

View File

@ -268,6 +268,10 @@ func TestPolicyACL(t *testing.T) {
Prefix: "zip/",
Policy: PolicyRead,
},
&KeyPolicy{
Prefix: "zap/",
Policy: PolicyList,
},
},
PreparedQueries: []*PreparedQueryPolicy{
&PreparedQueryPolicy{
@ -316,15 +320,17 @@ func TestPolicyACL(t *testing.T) {
read bool
write bool
writePrefix bool
list bool
}
cases := []keycase{
{"other", true, true, true},
{"foo/test", true, true, true},
{"foo/priv/test", false, false, false},
{"bar/any", false, false, false},
{"zip/test", true, false, false},
{"foo/", true, true, false},
{"", true, true, false},
{"other", true, true, true, true},
{"foo/test", true, true, true, true},
{"foo/priv/test", false, false, false, false},
{"bar/any", false, false, false, false},
{"zip/test", true, false, false, false},
{"foo/", true, true, false, true},
{"", true, true, false, true},
{"zap/test", true, false, false, true},
}
for _, c := range cases {
if c.read != acl.KeyRead(c.inp) {

View File

@ -11,6 +11,7 @@ const (
PolicyDeny = "deny"
PolicyRead = "read"
PolicyWrite = "write"
PolicyList = "list"
)
// Policy is used to represent the policy specified by
@ -118,6 +119,8 @@ func isPolicyValid(policy string) bool {
return true
case PolicyWrite:
return true
case PolicyList:
return true
default:
return false
}

View File

@ -676,6 +676,9 @@ func (a *Agent) consulConfig() (*consul.Config, error) {
if a.config.ACLEnforceVersion8 {
base.ACLEnforceVersion8 = a.config.ACLEnforceVersion8
}
if a.config.ACLEnableKeyListPolicy {
base.ACLEnableKeyListPolicy = a.config.ACLEnableKeyListPolicy
}
if a.config.SessionTTLMin != 0 {
base.SessionTTLMin = a.config.SessionTTLMin
}

View File

@ -487,6 +487,7 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
ACLDefaultPolicy: b.stringVal(c.ACLDefaultPolicy),
ACLDownPolicy: b.stringVal(c.ACLDownPolicy),
ACLEnforceVersion8: b.boolVal(c.ACLEnforceVersion8),
ACLEnableKeyListPolicy: b.boolVal(c.ACLEnableKeyListPolicy),
ACLMasterToken: b.stringVal(c.ACLMasterToken),
ACLReplicationToken: b.stringVal(c.ACLReplicationToken),
ACLTTL: b.durationVal("acl_ttl", c.ACLTTL),

View File

@ -128,6 +128,7 @@ type Config struct {
ACLDatacenter *string `json:"acl_datacenter,omitempty" hcl:"acl_datacenter" mapstructure:"acl_datacenter"`
ACLDefaultPolicy *string `json:"acl_default_policy,omitempty" hcl:"acl_default_policy" mapstructure:"acl_default_policy"`
ACLDownPolicy *string `json:"acl_down_policy,omitempty" hcl:"acl_down_policy" mapstructure:"acl_down_policy"`
ACLEnableKeyListPolicy *bool `json:"acl_enable_key_list_policy,omitempty" hcl:"acl_enable_key_list_policy" mapstructure:"acl_enable_key_list_policy"`
ACLEnforceVersion8 *bool `json:"acl_enforce_version_8,omitempty" hcl:"acl_enforce_version_8" mapstructure:"acl_enforce_version_8"`
ACLMasterToken *string `json:"acl_master_token,omitempty" hcl:"acl_master_token" mapstructure:"acl_master_token"`
ACLReplicationToken *string `json:"acl_replication_token,omitempty" hcl:"acl_replication_token" mapstructure:"acl_replication_token"`

View File

@ -54,6 +54,7 @@ type RuntimeConfig struct {
ACLDefaultPolicy string
ACLDownPolicy string
ACLEnforceVersion8 bool
ACLEnableKeyListPolicy bool
ACLMasterToken string
ACLReplicationToken string
ACLTTL time.Duration

View File

@ -1932,6 +1932,7 @@ func TestFullConfig(t *testing.T) {
"acl_default_policy": "ArK3WIfE",
"acl_down_policy": "vZXMfMP0",
"acl_enforce_version_8": true,
"acl_enable_key_list_policy": true,
"acl_master_token": "C1Q1oIwh",
"acl_replication_token": "LMmgy5dO",
"acl_token": "O1El0wan",
@ -2347,6 +2348,7 @@ func TestFullConfig(t *testing.T) {
acl_default_policy = "ArK3WIfE"
acl_down_policy = "vZXMfMP0"
acl_enforce_version_8 = true
acl_enable_key_list_policy = true
acl_master_token = "C1Q1oIwh"
acl_replication_token = "LMmgy5dO"
acl_token = "O1El0wan"
@ -2905,6 +2907,7 @@ func TestFullConfig(t *testing.T) {
ACLDefaultPolicy: "ArK3WIfE",
ACLDownPolicy: "vZXMfMP0",
ACLEnforceVersion8: true,
ACLEnableKeyListPolicy: true,
ACLMasterToken: "C1Q1oIwh",
ACLReplicationToken: "LMmgy5dO",
ACLTTL: 18060 * time.Second,
@ -3592,6 +3595,7 @@ func TestSanitize(t *testing.T) {
"ACLDefaultPolicy": "",
"ACLDisabledTTL": "0s",
"ACLDownPolicy": "",
"ACLEnableKeyListPolicy": false,
"ACLEnforceVersion8": false,
"ACLMasterToken": "hidden",
"ACLReplicationToken": "hidden",

View File

@ -257,6 +257,11 @@ type Config struct {
// are opt-in prior to Consul 0.8 and opt-out in Consul 0.8 and later.
ACLEnforceVersion8 bool
// ACLEnableKeyListPolicy is used to gate enforcement of the new "list" policy that
// protects listing keys by prefix. This behavior is opt-in
// by default in Consul 1.0 and later.
ACLEnableKeyListPolicy bool
// TombstoneTTL is used to control how long KV tombstones are retained.
// This provides a window of time where the X-Consul-Index is monotonic.
// Outside this window, the index may not be monotonic. This is a result

View File

@ -133,7 +133,7 @@ func (k *KVS) Get(args *structs.KeyRequest, reply *structs.IndexedDirEntries) er
return err
}
if aclRule != nil && !aclRule.KeyRead(args.Key) {
ent = nil
return acl.ErrPermissionDenied
}
if ent == nil {
@ -159,11 +159,15 @@ func (k *KVS) List(args *structs.KeyRequest, reply *structs.IndexedDirEntries) e
return err
}
acl, err := k.srv.resolveToken(args.Token)
aclToken, err := k.srv.resolveToken(args.Token)
if err != nil {
return err
}
if aclToken != nil && k.srv.config.ACLEnableKeyListPolicy && !aclToken.KeyList(args.Key) {
return acl.ErrPermissionDenied
}
return k.srv.blockingQuery(
&args.QueryOptions,
&reply.QueryMeta,
@ -172,8 +176,8 @@ func (k *KVS) List(args *structs.KeyRequest, reply *structs.IndexedDirEntries) e
if err != nil {
return err
}
if acl != nil {
ent = FilterDirEnt(acl, ent)
if aclToken != nil {
ent = FilterDirEnt(aclToken, ent)
}
if len(ent) == 0 {
@ -199,11 +203,15 @@ func (k *KVS) ListKeys(args *structs.KeyListRequest, reply *structs.IndexedKeyLi
return err
}
acl, err := k.srv.resolveToken(args.Token)
aclToken, err := k.srv.resolveToken(args.Token)
if err != nil {
return err
}
if aclToken != nil && k.srv.config.ACLEnableKeyListPolicy && !aclToken.KeyList(args.Prefix) {
return acl.ErrPermissionDenied
}
return k.srv.blockingQuery(
&args.QueryOptions,
&reply.QueryMeta,
@ -221,8 +229,8 @@ func (k *KVS) ListKeys(args *structs.KeyListRequest, reply *structs.IndexedKeyLi
reply.Index = index
}
if acl != nil {
keys = FilterKeys(acl, keys)
if aclToken != nil {
keys = FilterKeys(aclToken, keys)
}
reply.Keys = keys
return nil

View File

@ -214,16 +214,10 @@ func TestKVS_Get_ACLDeny(t *testing.T) {
Key: "zip",
}
var dirent structs.IndexedDirEntries
if err := msgpackrpc.CallWithCodec(codec, "KVS.Get", &getR, &dirent); err != nil {
t.Fatalf("err: %v", err)
if err := msgpackrpc.CallWithCodec(codec, "KVS.Get", &getR, &dirent); !acl.IsErrPermissionDenied(err) {
t.Fatalf("Expected %v, got err: %v", acl.ErrPermissionDenied, err)
}
if dirent.Index == 0 {
t.Fatalf("Bad: %v", dirent)
}
if len(dirent.Entries) != 0 {
t.Fatalf("Bad: %v", dirent)
}
}
func TestKVSEndpoint_List(t *testing.T) {
@ -479,6 +473,116 @@ func TestKVSEndpoint_List_ACLDeny(t *testing.T) {
}
}
func TestKVSEndpoint_List_ACLEnableKeyListPolicy(t *testing.T) {
t.Parallel()
dir1, s1 := testServerWithConfig(t, func(c *Config) {
c.ACLDatacenter = "dc1"
c.ACLMasterToken = "root"
c.ACLDefaultPolicy = "deny"
c.ACLEnableKeyListPolicy = true
})
defer os.RemoveAll(dir1)
defer s1.Shutdown()
codec := rpcClient(t, s1)
defer codec.Close()
testrpc.WaitForLeader(t, s1.RPC, "dc1")
keys := []string{
"abe",
"bar/bar1",
"bar/bar2",
"zip",
}
for _, key := range keys {
arg := structs.KVSRequest{
Datacenter: "dc1",
Op: api.KVSet,
DirEnt: structs.DirEntry{
Key: key,
Flags: 1,
},
WriteRequest: structs.WriteRequest{Token: "root"},
}
var out bool
if err := msgpackrpc.CallWithCodec(codec, "KVS.Apply", &arg, &out); err != nil {
t.Fatalf("err: %v", err)
}
}
//write acl policy that denies recursive reads on ""
var testListRules1 = `
key "" {
policy = "deny"
}
key "bar" {
policy = "list"
}
key "zip" {
policy = "read"
}
`
arg := structs.ACLRequest{
Datacenter: "dc1",
Op: structs.ACLSet,
ACL: structs.ACL{
Name: "User token",
Type: structs.ACLTypeClient,
Rules: testListRules1,
},
WriteRequest: structs.WriteRequest{Token: "root"},
}
var out string
if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &out); err != nil {
t.Fatalf("err: %v", err)
}
id := out
//recursive read on empty prefix should fail
getR := structs.KeyRequest{
Datacenter: "dc1",
Key: "",
QueryOptions: structs.QueryOptions{Token: id},
}
var dirent structs.IndexedDirEntries
if err := msgpackrpc.CallWithCodec(codec, "KVS.List", &getR, &dirent); !acl.IsErrPermissionDenied(err) {
t.Fatalf("expected %v but got err: %v", acl.ErrPermissionDenied, err)
}
// recursive read with a prefix that has list permissions should succeed
getR2 := structs.KeyRequest{
Datacenter: "dc1",
Key: "bar",
QueryOptions: structs.QueryOptions{Token: id},
}
if err := msgpackrpc.CallWithCodec(codec, "KVS.List", &getR2, &dirent); err != nil {
t.Fatalf("err: %v", err)
}
if dirent.Index == 0 {
t.Fatalf("Bad: %v", dirent)
}
if len(dirent.Entries) != 2 {
t.Fatalf("Bad: %v", dirent.Entries)
}
for i := 0; i < len(dirent.Entries); i++ {
d := dirent.Entries[i]
switch i {
case 0:
if d.Key != "bar/bar1" {
t.Fatalf("bad key %v", d.Key)
}
case 1:
if d.Key != "bar/bar2" {
t.Fatalf("bad key %v", d.Key)
}
}
}
}
func TestKVSEndpoint_ListKeys(t *testing.T) {
t.Parallel()
dir1, s1 := testServer(t)
@ -628,6 +732,110 @@ func TestKVSEndpoint_ListKeys_ACLDeny(t *testing.T) {
}
}
func TestKVSEndpoint_ListKeys_ACLEnableKeyListPolicy(t *testing.T) {
t.Parallel()
dir1, s1 := testServerWithConfig(t, func(c *Config) {
c.ACLDatacenter = "dc1"
c.ACLMasterToken = "root"
c.ACLDefaultPolicy = "deny"
c.ACLEnableKeyListPolicy = true
})
defer os.RemoveAll(dir1)
defer s1.Shutdown()
codec := rpcClient(t, s1)
defer codec.Close()
testrpc.WaitForLeader(t, s1.RPC, "dc1")
keys := []string{
"abe",
"bar/bar1",
"bar/bar2",
"zip",
}
for _, key := range keys {
arg := structs.KVSRequest{
Datacenter: "dc1",
Op: api.KVSet,
DirEnt: structs.DirEntry{
Key: key,
Flags: 1,
},
WriteRequest: structs.WriteRequest{Token: "root"},
}
var out bool
if err := msgpackrpc.CallWithCodec(codec, "KVS.Apply", &arg, &out); err != nil {
t.Fatalf("err: %v", err)
}
}
//write acl policy that denies recursive reads on ""
var testListRules1 = `
key "" {
policy = "deny"
}
key "bar" {
policy = "list"
}
key "zip" {
policy = "read"
}
`
arg := structs.ACLRequest{
Datacenter: "dc1",
Op: structs.ACLSet,
ACL: structs.ACL{
Name: "User token",
Type: structs.ACLTypeClient,
Rules: testListRules1,
},
WriteRequest: structs.WriteRequest{Token: "root"},
}
var out string
if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &out); err != nil {
t.Fatalf("err: %v", err)
}
id := out
//recursive read on empty prefix should fail
getR := structs.KeyListRequest{
Datacenter: "dc1",
Prefix: "",
Seperator: "/",
QueryOptions: structs.QueryOptions{Token: id},
}
var dirent structs.IndexedKeyList
if err := msgpackrpc.CallWithCodec(codec, "KVS.ListKeys", &getR, &dirent); !acl.IsErrPermissionDenied(err) {
t.Fatalf("expected %v but got err: %v", acl.ErrPermissionDenied, err)
}
// recursive read with a prefix that has list permissions should succeed
getR2 := structs.KeyListRequest{
Datacenter: "dc1",
Prefix: "bar",
QueryOptions: structs.QueryOptions{Token: id},
}
if err := msgpackrpc.CallWithCodec(codec, "KVS.ListKeys", &getR2, &dirent); err != nil {
t.Fatalf("err: %v", err)
}
if dirent.Index == 0 {
t.Fatalf("Bad: %v", dirent)
}
if len(dirent.Keys) != 2 {
t.Fatalf("Bad: %v", dirent.Keys)
}
if dirent.Keys[0] != "bar/bar1" {
t.Fatalf("Bad: %v", dirent.Keys)
}
if dirent.Keys[1] != "bar/bar2" {
t.Fatalf("Bad: %v", dirent.Keys)
}
}
func TestKVS_Apply_LockDelay(t *testing.T) {
t.Parallel()
dir1, s1 := testServer(t)