From 51a04ec87d859d8cdf7bd93fd941f2b14016f0af Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Mon, 2 Oct 2017 17:10:21 -0500 Subject: [PATCH 1/4] Introduces new 'list' permission that applies to KV store recursive reads, and enforced only when opted in. --- acl/acl.go | 27 +++- acl/acl_test.go | 20 ++- acl/policy.go | 3 + agent/agent.go | 3 + agent/config/builder.go | 23 +-- agent/config/config.go | 1 + agent/config/runtime.go | 1 + agent/config/runtime_test.go | 4 + agent/consul/config.go | 5 + agent/consul/kvs_endpoint.go | 22 ++- agent/consul/kvs_endpoint_test.go | 224 ++++++++++++++++++++++++++++-- 11 files changed, 299 insertions(+), 34 deletions(-) diff --git a/acl/acl.go b/acl/acl.go index 3f867dbd9a..72a2e78f2f 100644 --- a/acl/acl.go +++ b/acl/acl.go @@ -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 diff --git a/acl/acl_test.go b/acl/acl_test.go index c8fbb3d777..02ae0efb46 100644 --- a/acl/acl_test.go +++ b/acl/acl_test.go @@ -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) { diff --git a/acl/policy.go b/acl/policy.go index 9df0020a37..4da26f45bf 100644 --- a/acl/policy.go +++ b/acl/policy.go @@ -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 } diff --git a/agent/agent.go b/agent/agent.go index 7ee7097323..c6d3a1a62b 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -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 } diff --git a/agent/config/builder.go b/agent/config/builder.go index 0bd383e29f..d8c33cef33 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -481,17 +481,18 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) { ConsulServerHealthInterval: b.durationVal("consul.server.health_interval", c.Consul.Server.HealthInterval), // ACL - ACLAgentMasterToken: b.stringVal(c.ACLAgentMasterToken), - ACLAgentToken: b.stringVal(c.ACLAgentToken), - ACLDatacenter: strings.ToLower(b.stringVal(c.ACLDatacenter)), - ACLDefaultPolicy: b.stringVal(c.ACLDefaultPolicy), - ACLDownPolicy: b.stringVal(c.ACLDownPolicy), - ACLEnforceVersion8: b.boolVal(c.ACLEnforceVersion8), - ACLMasterToken: b.stringVal(c.ACLMasterToken), - ACLReplicationToken: b.stringVal(c.ACLReplicationToken), - ACLTTL: b.durationVal("acl_ttl", c.ACLTTL), - ACLToken: b.stringVal(c.ACLToken), - EnableACLReplication: b.boolVal(c.EnableACLReplication), + ACLAgentMasterToken: b.stringVal(c.ACLAgentMasterToken), + ACLAgentToken: b.stringVal(c.ACLAgentToken), + ACLDatacenter: strings.ToLower(b.stringVal(c.ACLDatacenter)), + 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), + ACLToken: b.stringVal(c.ACLToken), + EnableACLReplication: b.boolVal(c.EnableACLReplication), // Autopilot AutopilotCleanupDeadServers: b.boolVal(c.Autopilot.CleanupDeadServers), diff --git a/agent/config/config.go b/agent/config/config.go index 1ace6779fc..20a291db97 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -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"` diff --git a/agent/config/runtime.go b/agent/config/runtime.go index fd8ad4744c..28cb13ac85 100644 --- a/agent/config/runtime.go +++ b/agent/config/runtime.go @@ -54,6 +54,7 @@ type RuntimeConfig struct { ACLDefaultPolicy string ACLDownPolicy string ACLEnforceVersion8 bool + ACLEnableKeyListPolicy bool ACLMasterToken string ACLReplicationToken string ACLTTL time.Duration diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index e77c511f49..b63ecc14a8 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -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", diff --git a/agent/consul/config.go b/agent/consul/config.go index 1b62af96c5..1b904593e1 100644 --- a/agent/consul/config.go +++ b/agent/consul/config.go @@ -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 diff --git a/agent/consul/kvs_endpoint.go b/agent/consul/kvs_endpoint.go index 78a631f88d..19591e8c63 100644 --- a/agent/consul/kvs_endpoint.go +++ b/agent/consul/kvs_endpoint.go @@ -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 diff --git a/agent/consul/kvs_endpoint_test.go b/agent/consul/kvs_endpoint_test.go index daaa237bcf..5a72bd0a22 100644 --- a/agent/consul/kvs_endpoint_test.go +++ b/agent/consul/kvs_endpoint_test.go @@ -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) From 26accb3b8a07e2cf9b1e74ab272d6a09beb8baba Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Tue, 3 Oct 2017 15:15:56 -0500 Subject: [PATCH 2/4] Only allow 'list' policies within 'key' policy definitions. Consolidated two similar tests into one and fixed alignment. --- acl/policy.go | 4 +- agent/config/runtime_test.go | 4 +- agent/consul/kvs_endpoint_test.go | 162 +++++++----------------------- 3 files changed, 42 insertions(+), 128 deletions(-) diff --git a/acl/policy.go b/acl/policy.go index 4da26f45bf..c8b5a93bda 100644 --- a/acl/policy.go +++ b/acl/policy.go @@ -119,8 +119,6 @@ func isPolicyValid(policy string) bool { return true case PolicyWrite: return true - case PolicyList: - return true default: return false } @@ -178,7 +176,7 @@ func Parse(rules string, sentinel sentinel.Evaluator) (*Policy, error) { // Validate the key policy for _, kp := range p.Keys { - if !isPolicyValid(kp.Policy) { + if kp.Policy != PolicyList && !isPolicyValid(kp.Policy) { return nil, fmt.Errorf("Invalid key policy: %#v", kp) } if err := isSentinelValid(sentinel, kp.Policy, kp.Sentinel); err != nil { diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index b63ecc14a8..9caf9a216a 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -1932,7 +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_enable_key_list_policy": true, "acl_master_token": "C1Q1oIwh", "acl_replication_token": "LMmgy5dO", "acl_token": "O1El0wan", @@ -2348,7 +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_enable_key_list_policy = true acl_master_token = "C1Q1oIwh" acl_replication_token = "LMmgy5dO" acl_token = "O1El0wan" diff --git a/agent/consul/kvs_endpoint_test.go b/agent/consul/kvs_endpoint_test.go index 5a72bd0a22..07228411ec 100644 --- a/agent/consul/kvs_endpoint_test.go +++ b/agent/consul/kvs_endpoint_test.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/testrpc" "github.com/hashicorp/net-rpc-msgpackrpc" + "github.com/pascaldekloe/goe/verify" ) func TestKVS_Apply(t *testing.T) { @@ -541,46 +542,64 @@ key "zip" { id := out //recursive read on empty prefix should fail - getR := structs.KeyRequest{ + getReq := 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) { + if err := msgpackrpc.CallWithCodec(codec, "KVS.List", &getReq, &dirent); !acl.IsErrPermissionDenied(err) { + t.Fatalf("expected %v but got err: %v", acl.ErrPermissionDenied, err) + } + + //listing keys on empty prefix should fail + getKeysReq := structs.KeyListRequest{ + Datacenter: "dc1", + Prefix: "", + Seperator: "/", + QueryOptions: structs.QueryOptions{Token: id}, + } + var keyList structs.IndexedKeyList + if err := msgpackrpc.CallWithCodec(codec, "KVS.ListKeys", &getKeysReq, &keyList); !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{ + getReq2 := structs.KeyRequest{ Datacenter: "dc1", Key: "bar", QueryOptions: structs.QueryOptions{Token: id}, } - if err := msgpackrpc.CallWithCodec(codec, "KVS.List", &getR2, &dirent); err != nil { + if err := msgpackrpc.CallWithCodec(codec, "KVS.List", &getReq2, &dirent); err != nil { t.Fatalf("err: %v", err) } - if dirent.Index == 0 { - t.Fatalf("Bad: %v", dirent) + expectedKeys := []string{"bar/bar1", "bar/bar2"} + var actualKeys []string + for _, entry := range dirent.Entries { + actualKeys = append(actualKeys, entry.Key) } - if len(dirent.Entries) != 2 { - t.Fatalf("Bad: %v", dirent.Entries) + + verify.Values(t, "", actualKeys, expectedKeys) + + // list keys with a prefix that has list permissions should succeed + getKeysReq2 := structs.KeyListRequest{ + Datacenter: "dc1", + Prefix: "bar", + QueryOptions: structs.QueryOptions{Token: id}, } - 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) - } - } + if err := msgpackrpc.CallWithCodec(codec, "KVS.ListKeys", &getKeysReq2, &keyList); err != nil { + t.Fatalf("err: %v", err) } + actualKeys = []string{} + + for _, key := range keyList.Keys { + actualKeys = append(actualKeys, key) + } + + verify.Values(t, "", actualKeys, expectedKeys) + } func TestKVSEndpoint_ListKeys(t *testing.T) { @@ -732,109 +751,6 @@ 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() From 8dcd7e700c890c39bfc3f6af5424c77ce1c6b9d9 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Tue, 3 Oct 2017 15:19:31 -0500 Subject: [PATCH 3/4] Remove extra newline --- agent/consul/kvs_endpoint_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/agent/consul/kvs_endpoint_test.go b/agent/consul/kvs_endpoint_test.go index 07228411ec..ee2a7ea2d8 100644 --- a/agent/consul/kvs_endpoint_test.go +++ b/agent/consul/kvs_endpoint_test.go @@ -751,7 +751,6 @@ func TestKVSEndpoint_ListKeys_ACLDeny(t *testing.T) { } } - func TestKVS_Apply_LockDelay(t *testing.T) { t.Parallel() dir1, s1 := testServer(t) From 41ec69f71a3ee5b9e75344edb12a9336e9b11100 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Wed, 4 Oct 2017 06:19:20 -0500 Subject: [PATCH 4/4] Update ACL guide to describe the new list policy for Keys --- website/source/docs/guides/acl.html.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/website/source/docs/guides/acl.html.md b/website/source/docs/guides/acl.html.md index be42222296..4bbf65aaae 100644 --- a/website/source/docs/guides/acl.html.md +++ b/website/source/docs/guides/acl.html.md @@ -671,6 +671,32 @@ the example above, the rules allow read-only access to any key name with the emp read-write access to any key name that starts with "foo", and deny all access to any key name that starts with "bar". +#### List Policy for Keys + +Consul 1.0 introduces a new `list` policy for keys that is only enforced when opted in via the boolean config param "acl_enable_key_list_policy". +`list` controls access to recursively list entries and keys, and enables more fine grained policies. With "acl_enable_key_list_policy", +recursive reads via [the KV API](/api/kv.html#recurse) with an invalid token result in a 403. Example: + +```text +key "" { + policy = "deny" +} + +key "bar" { + policy = "list" +} + +key "baz" { + policy = "read" +} +``` + +In the example above, the rules allow reading the key "baz", and only allow recursive reads on the prefix "bar". + +A token with `write` access on a prefix also has `list` access. A token with `list` access on a prefix also has `read` access on all its suffixes. + +#### Sentinel Integration + Consul Enterprise supports additional optional fields for key write policies for [Sentinel](https://docs.hashicorp.com/sentinel/app/consul/) integration. An example key rule with a Sentinel code policy looks like this: