diff --git a/acl/acl.go b/acl/acl.go index 344afc5c341f..33f5e2337259 100644 --- a/acl/acl.go +++ b/acl/acl.go @@ -58,6 +58,14 @@ type ACL interface { // EventWrite determines if a specific event may be fired. EventWrite(string) bool + // PrepardQueryRead determines if a specific prepared query can be read + // to show its contents (this is not used for execution). + PreparedQueryRead(string) bool + + // PreparedQueryWrite determines if a specific prepared query can be + // created, modified, or deleted. + PreparedQueryWrite(string) bool + // KeyringRead determines if the encryption keyring used in // the gossip layer can be read. KeyringRead() bool @@ -70,12 +78,6 @@ type ACL interface { // ACLModify checks for permission to manipulate ACLs ACLModify() bool - - // QueryList checks for permission to list all the prepared queries. - QueryList() bool - - // QueryModify checks for permission to modify any prepared query. - QueryModify() bool } // StaticACL is used to implement a base ACL policy. It either @@ -114,27 +116,27 @@ func (s *StaticACL) EventWrite(string) bool { return s.defaultAllow } -func (s *StaticACL) KeyringRead() bool { +func (s *StaticACL) PreparedQueryRead(string) bool { return s.defaultAllow } -func (s *StaticACL) KeyringWrite() bool { +func (s *StaticACL) PreparedQueryWrite(string) bool { return s.defaultAllow } -func (s *StaticACL) ACLList() bool { - return s.allowManage +func (s *StaticACL) KeyringRead() bool { + return s.defaultAllow } -func (s *StaticACL) ACLModify() bool { - return s.allowManage +func (s *StaticACL) KeyringWrite() bool { + return s.defaultAllow } -func (s *StaticACL) QueryList() bool { +func (s *StaticACL) ACLList() bool { return s.allowManage } -func (s *StaticACL) QueryModify() bool { +func (s *StaticACL) ACLModify() bool { return s.allowManage } @@ -183,6 +185,9 @@ type PolicyACL struct { // eventRules contains the user event policies eventRules *radix.Tree + // preparedQueryRules contains the prepared query policies + preparedQueryRules *radix.Tree + // keyringRules contains the keyring policies. The keyring has // a very simple yes/no without prefix matching, so here we // don't need to use a radix tree. @@ -193,10 +198,11 @@ type PolicyACL struct { // and a parent policy to resolve missing cases. func New(parent ACL, policy *Policy) (*PolicyACL, error) { p := &PolicyACL{ - parent: parent, - keyRules: radix.New(), - serviceRules: radix.New(), - eventRules: radix.New(), + parent: parent, + keyRules: radix.New(), + serviceRules: radix.New(), + eventRules: radix.New(), + preparedQueryRules: radix.New(), } // Load the key policy @@ -214,6 +220,11 @@ func New(parent ACL, policy *Policy) (*PolicyACL, error) { p.eventRules.Insert(ep.Event, ep.Policy) } + // Load the prepared query policy + for _, pq := range policy.PreparedQueries { + p.preparedQueryRules.Insert(pq.Prefix, pq.Policy) + } + // Load the keyring policy p.keyringRule = policy.Keyring @@ -226,9 +237,7 @@ func (p *PolicyACL) KeyRead(key string) bool { _, rule, ok := p.keyRules.LongestPrefix(key) if ok { switch rule.(string) { - case KeyPolicyRead: - return true - case KeyPolicyWrite: + case PolicyRead, PolicyWrite: return true default: return false @@ -245,7 +254,7 @@ func (p *PolicyACL) KeyWrite(key string) bool { _, rule, ok := p.keyRules.LongestPrefix(key) if ok { switch rule.(string) { - case KeyPolicyWrite: + case PolicyWrite: return true default: return false @@ -260,7 +269,7 @@ func (p *PolicyACL) KeyWrite(key string) bool { func (p *PolicyACL) KeyWritePrefix(prefix string) bool { // Look for a matching rule that denies _, rule, ok := p.keyRules.LongestPrefix(prefix) - if ok && rule.(string) != KeyPolicyWrite { + if ok && rule.(string) != PolicyWrite { return false } @@ -268,7 +277,7 @@ func (p *PolicyACL) KeyWritePrefix(prefix string) bool { deny := false p.keyRules.WalkPrefix(prefix, func(path string, rule interface{}) bool { // We have a rule to prevent a write in a sub-directory! - if rule.(string) != KeyPolicyWrite { + if rule.(string) != PolicyWrite { deny = true return true } @@ -296,9 +305,7 @@ func (p *PolicyACL) ServiceRead(name string) bool { if ok { switch rule { - case ServicePolicyWrite: - return true - case ServicePolicyRead: + case PolicyRead, PolicyWrite: return true default: return false @@ -316,7 +323,7 @@ func (p *PolicyACL) ServiceWrite(name string) bool { if ok { switch rule { - case ServicePolicyWrite: + case PolicyWrite: return true default: return false @@ -333,9 +340,7 @@ func (p *PolicyACL) EventRead(name string) bool { // Longest-prefix match on event names if _, rule, ok := p.eventRules.LongestPrefix(name); ok { switch rule { - case EventPolicyRead: - return true - case EventPolicyWrite: + case PolicyRead, PolicyWrite: return true default: return false @@ -351,20 +356,58 @@ func (p *PolicyACL) EventRead(name string) bool { func (p *PolicyACL) EventWrite(name string) bool { // Longest-prefix match event names if _, rule, ok := p.eventRules.LongestPrefix(name); ok { - return rule == EventPolicyWrite + return rule == PolicyWrite } // No match, use parent return p.parent.EventWrite(name) } +// PreparedQueryRead checks if reading (listing) of a prepared query is +// allowed - this isn't execution, just listing its contents. +func (p *PolicyACL) PreparedQueryRead(prefix string) bool { + // Check for an exact rule or catch-all + _, rule, ok := p.preparedQueryRules.LongestPrefix(prefix) + + if ok { + switch rule { + case PolicyRead, PolicyWrite: + return true + default: + return false + } + } + + // No matching rule, use the parent. + return p.parent.PreparedQueryRead(prefix) +} + +// PreparedQueryWrite checks if writing (creating, updating, or deleting) of a +// prepared query is allowed. +func (p *PolicyACL) PreparedQueryWrite(prefix string) bool { + // Check for an exact rule or catch-all + _, rule, ok := p.preparedQueryRules.LongestPrefix(prefix) + + if ok { + switch rule { + case PolicyWrite: + return true + default: + return false + } + } + + // No matching rule, use the parent. + return p.parent.PreparedQueryWrite(prefix) +} + // KeyringRead is used to determine if the keyring can be // read by the current ACL token. func (p *PolicyACL) KeyringRead() bool { switch p.keyringRule { - case KeyringPolicyRead, KeyringPolicyWrite: + case PolicyRead, PolicyWrite: return true - case KeyringPolicyDeny: + case PolicyDeny: return false default: return p.parent.KeyringRead() @@ -373,7 +416,7 @@ func (p *PolicyACL) KeyringRead() bool { // KeyringWrite determines if the keyring can be manipulated. func (p *PolicyACL) KeyringWrite() bool { - if p.keyringRule == KeyringPolicyWrite { + if p.keyringRule == PolicyWrite { return true } return p.parent.KeyringWrite() @@ -388,13 +431,3 @@ func (p *PolicyACL) ACLList() bool { func (p *PolicyACL) ACLModify() bool { return p.parent.ACLModify() } - -// QueryList checks if listing of all prepared queries is allowed. -func (p *PolicyACL) QueryList() bool { - return p.parent.QueryList() -} - -// QueryModify checks if modifying of any prepared query is allowed. -func (p *PolicyACL) QueryModify() bool { - return p.parent.QueryModify() -} diff --git a/acl/acl_test.go b/acl/acl_test.go index 06cdfb755747..69c4f0a1bf86 100644 --- a/acl/acl_test.go +++ b/acl/acl_test.go @@ -53,6 +53,12 @@ func TestStaticACL(t *testing.T) { if !all.EventWrite("foobar") { t.Fatalf("should allow") } + if !all.PreparedQueryRead("foobar") { + t.Fatalf("should allow") + } + if !all.PreparedQueryWrite("foobar") { + t.Fatalf("should allow") + } if !all.KeyringRead() { t.Fatalf("should allow") } @@ -65,12 +71,6 @@ func TestStaticACL(t *testing.T) { if all.ACLModify() { t.Fatalf("should not allow") } - if all.QueryList() { - t.Fatalf("should not allow") - } - if all.QueryModify() { - t.Fatalf("should not allow") - } if none.KeyRead("foobar") { t.Fatalf("should not allow") @@ -96,6 +96,12 @@ func TestStaticACL(t *testing.T) { if none.EventWrite("") { t.Fatalf("should not allow") } + if none.PreparedQueryRead("foobar") { + t.Fatalf("should not allow") + } + if none.PreparedQueryWrite("foobar") { + t.Fatalf("should not allow") + } if none.KeyringRead() { t.Fatalf("should now allow") } @@ -108,12 +114,6 @@ func TestStaticACL(t *testing.T) { if none.ACLModify() { t.Fatalf("should not allow") } - if none.QueryList() { - t.Fatalf("should not allow") - } - if none.QueryModify() { - t.Fatalf("should not allow") - } if !manage.KeyRead("foobar") { t.Fatalf("should allow") @@ -133,22 +133,22 @@ func TestStaticACL(t *testing.T) { if !manage.EventWrite("foobar") { t.Fatalf("should allow") } - if !manage.KeyringRead() { + if !manage.PreparedQueryRead("foobar") { t.Fatalf("should allow") } - if !manage.KeyringWrite() { + if !manage.PreparedQueryWrite("foobar") { t.Fatalf("should allow") } - if !manage.ACLList() { + if !manage.KeyringRead() { t.Fatalf("should allow") } - if !manage.ACLModify() { + if !manage.KeyringWrite() { t.Fatalf("should allow") } - if !manage.QueryList() { + if !manage.ACLList() { t.Fatalf("should allow") } - if !manage.QueryModify() { + if !manage.ACLModify() { t.Fatalf("should allow") } } @@ -159,51 +159,69 @@ func TestPolicyACL(t *testing.T) { Keys: []*KeyPolicy{ &KeyPolicy{ Prefix: "foo/", - Policy: KeyPolicyWrite, + Policy: PolicyWrite, }, &KeyPolicy{ Prefix: "foo/priv/", - Policy: KeyPolicyDeny, + Policy: PolicyDeny, }, &KeyPolicy{ Prefix: "bar/", - Policy: KeyPolicyDeny, + Policy: PolicyDeny, }, &KeyPolicy{ Prefix: "zip/", - Policy: KeyPolicyRead, + Policy: PolicyRead, }, }, Services: []*ServicePolicy{ &ServicePolicy{ Name: "", - Policy: ServicePolicyWrite, + Policy: PolicyWrite, }, &ServicePolicy{ Name: "foo", - Policy: ServicePolicyRead, + Policy: PolicyRead, }, &ServicePolicy{ Name: "bar", - Policy: ServicePolicyDeny, + Policy: PolicyDeny, }, &ServicePolicy{ Name: "barfoo", - Policy: ServicePolicyWrite, + Policy: PolicyWrite, }, }, Events: []*EventPolicy{ &EventPolicy{ Event: "", - Policy: EventPolicyRead, + Policy: PolicyRead, }, &EventPolicy{ Event: "foo", - Policy: EventPolicyWrite, + Policy: PolicyWrite, }, &EventPolicy{ Event: "bar", - Policy: EventPolicyDeny, + Policy: PolicyDeny, + }, + }, + PreparedQueries: []*PreparedQueryPolicy{ + &PreparedQueryPolicy{ + Prefix: "", + Policy: PolicyRead, + }, + &PreparedQueryPolicy{ + Prefix: "foo", + Policy: PolicyWrite, + }, + &PreparedQueryPolicy{ + Prefix: "bar", + Policy: PolicyDeny, + }, + &PreparedQueryPolicy{ + Prefix: "zoo", + Policy: PolicyWrite, }, }, } @@ -284,6 +302,31 @@ func TestPolicyACL(t *testing.T) { t.Fatalf("Event fail: %#v", c) } } + + // Test prepared queries + type querycase struct { + inp string + read bool + write bool + } + querycases := []querycase{ + {"foo", true, true}, + {"foobar", true, true}, + {"bar", false, false}, + {"barbaz", false, false}, + {"baz", true, false}, + {"nope", true, false}, + {"zoo", true, true}, + {"zookeeper", true, true}, + } + for _, c := range querycases { + if c.read != acl.PreparedQueryRead(c.inp) { + t.Fatalf("Prepared query fail: %#v", c) + } + if c.write != acl.PreparedQueryWrite(c.inp) { + t.Fatalf("Prepared query fail: %#v", c) + } + } } func TestPolicyACL_Parent(t *testing.T) { @@ -292,21 +335,31 @@ func TestPolicyACL_Parent(t *testing.T) { Keys: []*KeyPolicy{ &KeyPolicy{ Prefix: "foo/", - Policy: KeyPolicyWrite, + Policy: PolicyWrite, }, &KeyPolicy{ Prefix: "bar/", - Policy: KeyPolicyRead, + Policy: PolicyRead, }, }, Services: []*ServicePolicy{ &ServicePolicy{ Name: "other", - Policy: ServicePolicyWrite, + Policy: PolicyWrite, }, &ServicePolicy{ Name: "foo", - Policy: ServicePolicyRead, + Policy: PolicyRead, + }, + }, + PreparedQueries: []*PreparedQueryPolicy{ + &PreparedQueryPolicy{ + Prefix: "other", + Policy: PolicyWrite, + }, + &PreparedQueryPolicy{ + Prefix: "foo", + Policy: PolicyRead, }, }, } @@ -319,21 +372,27 @@ func TestPolicyACL_Parent(t *testing.T) { Keys: []*KeyPolicy{ &KeyPolicy{ Prefix: "foo/priv/", - Policy: KeyPolicyRead, + Policy: PolicyRead, }, &KeyPolicy{ Prefix: "bar/", - Policy: KeyPolicyDeny, + Policy: PolicyDeny, }, &KeyPolicy{ Prefix: "zip/", - Policy: KeyPolicyRead, + Policy: PolicyRead, }, }, Services: []*ServicePolicy{ &ServicePolicy{ Name: "bar", - Policy: ServicePolicyDeny, + Policy: PolicyDeny, + }, + }, + PreparedQueries: []*PreparedQueryPolicy{ + &PreparedQueryPolicy{ + Prefix: "bar", + Policy: PolicyDeny, }, }, } @@ -388,6 +447,29 @@ func TestPolicyACL_Parent(t *testing.T) { } } + // Test prepared queries + type querycase struct { + inp string + read bool + write bool + } + querycases := []querycase{ + {"foo", true, false}, + {"foobar", true, false}, + {"bar", false, false}, + {"barbaz", false, false}, + {"baz", false, false}, + {"nope", false, false}, + } + for _, c := range querycases { + if c.read != acl.PreparedQueryRead(c.inp) { + t.Fatalf("Prepared query fail: %#v", c) + } + if c.write != acl.PreparedQueryWrite(c.inp) { + t.Fatalf("Prepared query fail: %#v", c) + } + } + // Check some management functions that chain up if acl.ACLList() { t.Fatalf("should not allow") @@ -395,12 +477,6 @@ func TestPolicyACL_Parent(t *testing.T) { if acl.ACLModify() { t.Fatalf("should not allow") } - if acl.QueryList() { - t.Fatalf("should not allow") - } - if acl.QueryModify() { - t.Fatalf("should not allow") - } } func TestPolicyACL_Keyring(t *testing.T) { @@ -412,9 +488,9 @@ func TestPolicyACL_Keyring(t *testing.T) { } keyringcases := []keyringcase{ {"", false, false}, - {KeyringPolicyRead, true, false}, - {KeyringPolicyWrite, true, true}, - {KeyringPolicyDeny, false, false}, + {PolicyRead, true, false}, + {PolicyWrite, true, true}, + {PolicyDeny, false, false}, } for _, c := range keyringcases { acl, err := New(DenyAll(), &Policy{Keyring: c.inp}) diff --git a/acl/policy.go b/acl/policy.go index 9009ee76b526..a0e56da425da 100644 --- a/acl/policy.go +++ b/acl/policy.go @@ -7,28 +7,20 @@ import ( ) const ( - KeyPolicyDeny = "deny" - KeyPolicyRead = "read" - KeyPolicyWrite = "write" - ServicePolicyDeny = "deny" - ServicePolicyRead = "read" - ServicePolicyWrite = "write" - EventPolicyRead = "read" - EventPolicyWrite = "write" - EventPolicyDeny = "deny" - KeyringPolicyWrite = "write" - KeyringPolicyRead = "read" - KeyringPolicyDeny = "deny" + PolicyDeny = "deny" + PolicyRead = "read" + PolicyWrite = "write" ) // Policy is used to represent the policy specified by // an ACL configuration. type Policy struct { - ID string `hcl:"-"` - Keys []*KeyPolicy `hcl:"key,expand"` - Services []*ServicePolicy `hcl:"service,expand"` - Events []*EventPolicy `hcl:"event,expand"` - Keyring string `hcl:"keyring"` + ID string `hcl:"-"` + Keys []*KeyPolicy `hcl:"key,expand"` + Services []*ServicePolicy `hcl:"service,expand"` + Events []*EventPolicy `hcl:"event,expand"` + PreparedQueries []*PreparedQueryPolicy `hcl:"query,expand"` + Keyring string `hcl:"keyring"` } // KeyPolicy represents a policy for a key @@ -61,6 +53,30 @@ func (e *EventPolicy) GoString() string { return fmt.Sprintf("%#v", *e) } +// PreparedQueryPolicy represents a prepared query policy. +type PreparedQueryPolicy struct { + Prefix string `hcl:",key"` + Policy string +} + +func (e *PreparedQueryPolicy) GoString() string { + return fmt.Sprintf("%#v", *e) +} + +// isPolicyValid makes sure the given string matches one of the valid policies. +func isPolicyValid(policy string) bool { + switch policy { + case PolicyDeny: + return true + case PolicyRead: + return true + case PolicyWrite: + return true + default: + return false + } +} + // Parse is used to parse the specified ACL rules into an // intermediary set of policies, before being compiled into // the ACL @@ -78,44 +94,34 @@ func Parse(rules string) (*Policy, error) { // Validate the key policy for _, kp := range p.Keys { - switch kp.Policy { - case KeyPolicyDeny: - case KeyPolicyRead: - case KeyPolicyWrite: - default: + if !isPolicyValid(kp.Policy) { return nil, fmt.Errorf("Invalid key policy: %#v", kp) } } // Validate the service policy for _, sp := range p.Services { - switch sp.Policy { - case ServicePolicyDeny: - case ServicePolicyRead: - case ServicePolicyWrite: - default: + if !isPolicyValid(sp.Policy) { return nil, fmt.Errorf("Invalid service policy: %#v", sp) } } // Validate the user event policies for _, ep := range p.Events { - switch ep.Policy { - case EventPolicyRead: - case EventPolicyWrite: - case EventPolicyDeny: - default: + if !isPolicyValid(ep.Policy) { return nil, fmt.Errorf("Invalid event policy: %#v", ep) } } - // Validate the keyring policy - switch p.Keyring { - case KeyringPolicyRead: - case KeyringPolicyWrite: - case KeyringPolicyDeny: - case "": // Special case to allow omitting the keyring policy - default: + // Validate the prepared query policies + for _, pq := range p.PreparedQueries { + if !isPolicyValid(pq.Policy) { + return nil, fmt.Errorf("Invalid query policy: %#v", pq) + } + } + + // Validate the keyring policy - this one is allowed to be empty + if p.Keyring != "" && !isPolicyValid(p.Keyring) { return nil, fmt.Errorf("Invalid keyring policy: %#v", p.Keyring) } diff --git a/acl/policy_test.go b/acl/policy_test.go index eb0528ffc93e..c59a4e014683 100644 --- a/acl/policy_test.go +++ b/acl/policy_test.go @@ -6,7 +6,7 @@ import ( "testing" ) -func TestParse(t *testing.T) { +func TestACLPolicy_Parse_HCL(t *testing.T) { inp := ` key "" { policy = "read" @@ -35,52 +35,75 @@ event "foo" { event "bar" { policy = "deny" } +query "" { + policy = "read" +} +query "foo" { + policy = "write" +} +query "bar" { + policy = "deny" +} keyring = "deny" ` exp := &Policy{ Keys: []*KeyPolicy{ &KeyPolicy{ Prefix: "", - Policy: KeyPolicyRead, + Policy: PolicyRead, }, &KeyPolicy{ Prefix: "foo/", - Policy: KeyPolicyWrite, + Policy: PolicyWrite, }, &KeyPolicy{ Prefix: "foo/bar/", - Policy: KeyPolicyRead, + Policy: PolicyRead, }, &KeyPolicy{ Prefix: "foo/bar/baz", - Policy: KeyPolicyDeny, + Policy: PolicyDeny, }, }, Services: []*ServicePolicy{ &ServicePolicy{ Name: "", - Policy: ServicePolicyWrite, + Policy: PolicyWrite, }, &ServicePolicy{ Name: "foo", - Policy: ServicePolicyRead, + Policy: PolicyRead, }, }, Events: []*EventPolicy{ &EventPolicy{ Event: "", - Policy: EventPolicyRead, + Policy: PolicyRead, }, &EventPolicy{ Event: "foo", - Policy: EventPolicyWrite, + Policy: PolicyWrite, }, &EventPolicy{ Event: "bar", - Policy: EventPolicyDeny, + Policy: PolicyDeny, + }, + }, + PreparedQueries: []*PreparedQueryPolicy{ + &PreparedQueryPolicy{ + Prefix: "", + Policy: PolicyRead, + }, + &PreparedQueryPolicy{ + Prefix: "foo", + Policy: PolicyWrite, + }, + &PreparedQueryPolicy{ + Prefix: "bar", + Policy: PolicyDeny, }, }, - Keyring: KeyringPolicyDeny, + Keyring: PolicyDeny, } out, err := Parse(inp) @@ -93,7 +116,7 @@ keyring = "deny" } } -func TestParse_JSON(t *testing.T) { +func TestACLPolicy_Parse_JSON(t *testing.T) { inp := `{ "key": { "": { @@ -128,52 +151,95 @@ func TestParse_JSON(t *testing.T) { "policy": "deny" } }, + "query": { + "": { + "policy": "read" + }, + "foo": { + "policy": "write" + }, + "bar": { + "policy": "deny" + } + }, "keyring": "deny" }` exp := &Policy{ Keys: []*KeyPolicy{ &KeyPolicy{ Prefix: "", - Policy: KeyPolicyRead, + Policy: PolicyRead, }, &KeyPolicy{ Prefix: "foo/", - Policy: KeyPolicyWrite, + Policy: PolicyWrite, }, &KeyPolicy{ Prefix: "foo/bar/", - Policy: KeyPolicyRead, + Policy: PolicyRead, }, &KeyPolicy{ Prefix: "foo/bar/baz", - Policy: KeyPolicyDeny, + Policy: PolicyDeny, }, }, Services: []*ServicePolicy{ &ServicePolicy{ Name: "", - Policy: ServicePolicyWrite, + Policy: PolicyWrite, }, &ServicePolicy{ Name: "foo", - Policy: ServicePolicyRead, + Policy: PolicyRead, }, }, Events: []*EventPolicy{ &EventPolicy{ Event: "", - Policy: EventPolicyRead, + Policy: PolicyRead, }, &EventPolicy{ Event: "foo", - Policy: EventPolicyWrite, + Policy: PolicyWrite, }, &EventPolicy{ Event: "bar", - Policy: EventPolicyDeny, + Policy: PolicyDeny, }, }, - Keyring: KeyringPolicyDeny, + PreparedQueries: []*PreparedQueryPolicy{ + &PreparedQueryPolicy{ + Prefix: "", + Policy: PolicyRead, + }, + &PreparedQueryPolicy{ + Prefix: "foo", + Policy: PolicyWrite, + }, + &PreparedQueryPolicy{ + Prefix: "bar", + Policy: PolicyDeny, + }, + }, + Keyring: PolicyDeny, + } + + out, err := Parse(inp) + if err != nil { + t.Fatalf("err: %v", err) + } + + if !reflect.DeepEqual(out, exp) { + t.Fatalf("bad: %#v %#v", out, exp) + } +} + +func TestACLPolicy_Keyring_Empty(t *testing.T) { + inp := ` +keyring = "" + ` + exp := &Policy{ + Keyring: "", } out, err := Parse(inp) @@ -186,11 +252,12 @@ func TestParse_JSON(t *testing.T) { } } -func TestACLPolicy_badPolicy(t *testing.T) { +func TestACLPolicy_Bad_Policy(t *testing.T) { cases := []string{ `key "" { policy = "nope" }`, `service "" { policy = "nope" }`, `event "" { policy = "nope" }`, + `query "" { policy = "nope" }`, `keyring = "nope"`, } for _, c := range cases { diff --git a/consul/acl.go b/consul/acl.go index 9d2c1d94b076..de305d04f5da 100644 --- a/consul/acl.go +++ b/consul/acl.go @@ -32,6 +32,10 @@ const ( // is no token ID provided anonymousToken = "anonymous" + // redactedToken is shown in structures with embedded tokens when they + // are not allowed to be displayed + redactedToken = "" + // Maximum number of cached ACL entries aclCacheSize = 256 ) @@ -366,6 +370,52 @@ func (f *aclFilter) filterNodeDump(dump *structs.NodeDump) { *dump = nd } +// filterPreparedQueries is used to filter prepared queries based on ACL rules. +// We prune entries the user doesn't have access to, and we redact any tokens +// unless the client has a management token. This eases the transition to +// delegated authority over prepared queries, since it was easy to capture +// management tokens in Consul 0.6.3 and earlier, and we don't want to +// willy-nilly show those. This does have the limitation of preventing delegated +// non-management users from seeing captured tokens, but they can at least see +// that they are set. +func (f *aclFilter) filterPreparedQueries(queries *structs.PreparedQueries) { + // Management tokens can see everything with no filtering. + if f.acl.ACLList() { + return + } + + // Otherwise, we need to see what the token has access to. + ret := make(structs.PreparedQueries, 0, len(*queries)) + for _, query := range *queries { + // If no prefix ACL applies to this query then filter it, since + // we know at this point the user doesn't have a management + // token, otherwise see what the policy says. + prefix, ok := query.GetACLPrefix() + if !ok || !f.acl.PreparedQueryRead(prefix) { + f.logger.Printf("[DEBUG] consul: dropping prepared query %q from result due to ACLs", query.ID) + continue + } + + // Let the user see if there's a blank token, otherwise we need + // to redact it, since we know they don't have a management + // token. + if query.Token == "" { + ret = append(ret, query) + } else { + // Redact the token, using a copy of the query structure + // since we could be pointed at a live instance from the + // state store so it's not safe to modify it. Note that + // this clone will still point to things like underlying + // arrays in the original, but for modifying just the + // token it will be safe to use. + clone := *query + clone.Token = redactedToken + ret = append(ret, &clone) + } + } + *queries = ret +} + // filterACL is used to filter results from our service catalog based on the // rules configured for the provided token. The subject is scrubbed and // modified in-place, leaving only resources the token can access. @@ -402,9 +452,15 @@ func (s *Server) filterACL(token string, subj interface{}) error { case *structs.IndexedCheckServiceNodes: filt.filterCheckServiceNodes(&v.Nodes) + case *structs.CheckServiceNodes: + filt.filterCheckServiceNodes(v) + case *structs.IndexedNodeDump: filt.filterNodeDump(&v.Dump) + case *structs.IndexedPreparedQueries: + filt.filterPreparedQueries(&v.Queries) + default: panic(fmt.Errorf("Unhandled type passed to ACL filter: %#v", subj)) } diff --git a/consul/acl_test.go b/consul/acl_test.go index 702be3f95d53..1e0d489d7598 100644 --- a/consul/acl_test.go +++ b/consul/acl_test.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "os" + "reflect" "testing" "github.com/hashicorp/consul/acl" @@ -861,6 +862,73 @@ func TestACL_filterNodeDump(t *testing.T) { } } +func TestACL_filterPreparedQueries(t *testing.T) { + queries := structs.PreparedQueries{ + &structs.PreparedQuery{ + ID: "f004177f-2c28-83b7-4229-eacc25fe55d1", + }, + &structs.PreparedQuery{ + ID: "f004177f-2c28-83b7-4229-eacc25fe55d2", + Name: "query-with-no-token", + }, + &structs.PreparedQuery{ + ID: "f004177f-2c28-83b7-4229-eacc25fe55d3", + Name: "query-with-a-token", + Token: "root", + }, + } + + expected := structs.PreparedQueries{ + &structs.PreparedQuery{ + ID: "f004177f-2c28-83b7-4229-eacc25fe55d1", + }, + &structs.PreparedQuery{ + ID: "f004177f-2c28-83b7-4229-eacc25fe55d2", + Name: "query-with-no-token", + }, + &structs.PreparedQuery{ + ID: "f004177f-2c28-83b7-4229-eacc25fe55d3", + Name: "query-with-a-token", + Token: "root", + }, + } + + // Try permissive filtering with a management token. This will allow the + // embedded token to be seen. + filt := newAclFilter(acl.ManageAll(), nil) + filt.filterPreparedQueries(&queries) + if !reflect.DeepEqual(queries, expected) { + t.Fatalf("bad: %#v", queries) + } + + // Hang on to the entry with a token, which needs to survive the next + // operation. + original := queries[2] + + // Now try permissive filtering with a client token, which should cause + // the embedded token to get redacted, and the query with no name to get + // filtered out. + filt = newAclFilter(acl.AllowAll(), nil) + filt.filterPreparedQueries(&queries) + expected[2].Token = redactedToken + expected = append(structs.PreparedQueries{}, expected[1], expected[2]) + if !reflect.DeepEqual(queries, expected) { + t.Fatalf("bad: %#v", queries) + } + + // Make sure that the original object didn't lose its token. + if original.Token != "root" { + t.Fatalf("bad token: %s", original.Token) + } + + // Now try restrictive filtering. + filt = newAclFilter(acl.DenyAll(), nil) + filt.filterPreparedQueries(&queries) + if len(queries) != 0 { + t.Fatalf("bad: %#v", queries) + } +} + func TestACL_unhandledFilterType(t *testing.T) { defer func(t *testing.T) { if recover() == nil { diff --git a/consul/prepared_query_endpoint.go b/consul/prepared_query_endpoint.go index 1227f78831b9..c3f027136e49 100644 --- a/consul/prepared_query_endpoint.go +++ b/consul/prepared_query_endpoint.go @@ -56,14 +56,25 @@ func (p *PreparedQuery) Apply(args *structs.PreparedQueryRequest, reply *string) } *reply = args.Query.ID - // Grab the ACL because we need it in several places below. + // Get the ACL token for the request for the checks below. acl, err := p.srv.resolveToken(args.Token) if err != nil { return err } - // Enforce that any modify operation has the same token used when the - // query was created, or a management token with sufficient rights. + // If prefix ACLs apply to the incoming query, then do an ACL check. We + // need to make sure they have write access for whatever they are + // proposing. + if prefix, ok := args.Query.GetACLPrefix(); ok { + if acl != nil && !acl.PreparedQueryWrite(prefix) { + p.srv.logger.Printf("[WARN] consul.prepared_query: Operation on prepared query '%s' denied due to ACLs", args.Query.ID) + return permissionDeniedErr + } + } + + // This is the second part of the check above. If they are referencing + // an existing query then make sure it exists and that they have write + // access to whatever they are changing, if prefix ACLs apply to it. if args.Op != structs.PreparedQueryCreate { state := p.srv.fsm.State() _, query, err := state.PreparedQueryGet(args.Query.ID) @@ -73,9 +84,12 @@ func (p *PreparedQuery) Apply(args *structs.PreparedQueryRequest, reply *string) if query == nil { return fmt.Errorf("Cannot modify non-existent prepared query: '%s'", args.Query.ID) } - if (query.Token != args.Token) && (acl != nil && !acl.QueryModify()) { - p.srv.logger.Printf("[WARN] consul.prepared_query: Operation on prepared query '%s' denied because ACL didn't match ACL used to create the query, and a management token wasn't supplied", args.Query.ID) - return permissionDeniedErr + + if prefix, ok := query.GetACLPrefix(); ok { + if acl != nil && !acl.PreparedQueryWrite(prefix) { + p.srv.logger.Printf("[WARN] consul.prepared_query: Operation on prepared query '%s' denied due to ACLs", args.Query.ID) + return permissionDeniedErr + } } } @@ -86,11 +100,6 @@ func (p *PreparedQuery) Apply(args *structs.PreparedQueryRequest, reply *string) return fmt.Errorf("Invalid prepared query: %v", err) } - if acl != nil && !acl.ServiceRead(args.Query.Service.Service) { - p.srv.logger.Printf("[WARN] consul.prepared_query: Operation on prepared query for service '%s' denied due to ACLs", args.Query.Service.Service) - return permissionDeniedErr - } - case structs.PreparedQueryDelete: // Nothing else to verify here, just do the delete (we only look // at the ID field for this op). @@ -99,10 +108,7 @@ func (p *PreparedQuery) Apply(args *structs.PreparedQueryRequest, reply *string) return fmt.Errorf("Unknown prepared query operation: %s", args.Op) } - // At this point the token has been vetted, so make sure the token that - // is stored in the state store matches what was supplied. - args.Query.Token = args.Token - + // Commit the query to the state store. resp, err := p.srv.raftApply(structs.PreparedQueryRequestType, args) if err != nil { p.srv.logger.Printf("[ERR] consul.prepared_query: Apply failed %v", err) @@ -128,7 +134,13 @@ func parseQuery(query *structs.PreparedQuery) error { // transaction. Otherwise, people could "steal" queries that they don't // have proper ACL rights to change. // - Session is optional and checked for integrity during the transaction. - // - Token is checked outside this fn. + + // Token is checked when the query is executed, but we do make sure the + // user hasn't accidentally pasted-in the special redacted token name, + // which if we allowed in would be super hard to debug and understand. + if query.Token == redactedToken { + return fmt.Errorf("Bad Token '%s', it looks like a query definition with a redacted token was submitted", query.Token) + } // Parse the service query sub-structure. if err := parseService(&query.Service); err != nil { @@ -148,7 +160,7 @@ func parseQuery(query *structs.PreparedQuery) error { // checked, as noted in the comments below. This also updates all the parsed // fields of the query. func parseService(svc *structs.ServiceQuery) error { - // Service is required. We check integrity during the transaction. + // Service is required. if svc.Service == "" { return fmt.Errorf("Must provide a service name to query") } @@ -191,13 +203,6 @@ func (p *PreparedQuery) Get(args *structs.PreparedQuerySpecificRequest, return err } - // We will use this in the loop to see if the caller is allowed to see - // the query. - acl, err := p.srv.resolveToken(args.Token) - if err != nil { - return err - } - // Get the requested query. state := p.srv.fsm.State() return p.srv.blockingRPC( @@ -213,13 +218,27 @@ func (p *PreparedQuery) Get(args *structs.PreparedQuerySpecificRequest, return ErrQueryNotFound } - if (query.Token != args.Token) && (acl != nil && !acl.QueryList()) { - p.srv.logger.Printf("[WARN] consul.prepared_query: Request to get prepared query '%s' denied because ACL didn't match ACL used to create the query, and a management token wasn't supplied", args.QueryID) + // If no prefix ACL applies to this query, then they are + // always allowed to see it if they have the ID. + reply.Index = index + reply.Queries = structs.PreparedQueries{query} + if _, ok := query.GetACLPrefix(); !ok { + return nil + } + + // Otherwise, attempt to filter it the usual way. + if err := p.srv.filterACL(args.Token, reply); err != nil { + return err + } + + // Since this is a GET of a specific query, if ACLs have + // prevented us from returning something that exists, + // then alert the user with a permission denied error. + if len(reply.Queries) == 0 { + p.srv.logger.Printf("[WARN] consul.prepared_query: Request to get prepared query '%s' denied due to ACLs", args.QueryID) return permissionDeniedErr } - reply.Index = index - reply.Queries = structs.PreparedQueries{query} return nil }) } @@ -230,16 +249,6 @@ func (p *PreparedQuery) List(args *structs.DCSpecificRequest, reply *structs.Ind return err } - // This always requires a management token. - acl, err := p.srv.resolveToken(args.Token) - if err != nil { - return err - } - if acl != nil && !acl.QueryList() { - p.srv.logger.Printf("[WARN] consul.prepared_query: Request to list prepared queries denied due to ACLs") - return permissionDeniedErr - } - // Get the list of queries. state := p.srv.fsm.State() return p.srv.blockingRPC( @@ -253,7 +262,7 @@ func (p *PreparedQuery) List(args *structs.DCSpecificRequest, reply *structs.Ind } reply.Index, reply.Queries = index, queries - return nil + return p.srv.filterACL(args.Token, reply) }) } @@ -290,6 +299,21 @@ func (p *PreparedQuery) Execute(args *structs.PreparedQueryExecuteRequest, return err } + // If they supplied a token with the query, use that, otherwise use the + // token passed in with the request. + token := args.QueryOptions.Token + if query.Token != "" { + token = query.Token + } + if err := p.srv.filterACL(token, &reply.Nodes); err != nil { + return err + } + + // TODO (slackpad) We could add a special case here that will avoid the + // fail over if we filtered everything due to ACLs. This seems like it + // might not be worth the code complexity and behavior differences, + // though, since this is essentially a misconfiguration. + // Shuffle the results in case coordinates are not available if they // requested an RTT sort. reply.Nodes.Shuffle() @@ -340,6 +364,16 @@ func (p *PreparedQuery) ExecuteRemote(args *structs.PreparedQueryExecuteRemoteRe return err } + // If they supplied a token with the query, use that, otherwise use the + // token passed in with the request. + token := args.QueryOptions.Token + if args.Query.Token != "" { + token = args.Query.Token + } + if err := p.srv.filterACL(token, &reply.Nodes); err != nil { + return err + } + // We don't bother trying to do an RTT sort here since we are by // definition in another DC. We just shuffle to make sure that we // balance the load across the results. @@ -354,7 +388,7 @@ func (p *PreparedQuery) ExecuteRemote(args *structs.PreparedQueryExecuteRemoteRe } // execute runs a prepared query in the local DC without any failover. We don't -// apply any sorting options at this level - it should be done up above. +// apply any sorting options or ACL checks at this level - it should be done up above. func (p *PreparedQuery) execute(query *structs.PreparedQuery, reply *structs.PreparedQueryExecuteResponse) error { state := p.srv.fsm.State() @@ -363,20 +397,6 @@ func (p *PreparedQuery) execute(query *structs.PreparedQuery, return err } - // This is kind of a paranoia ACL check, in case something changed with - // the token from the time the query was registered. Note that we use - // the token stored with the query, NOT the passed-in one, which is - // critical to how queries work (the query becomes a proxy for a lookup - // using the ACL it was created with). - acl, err := p.srv.resolveToken(query.Token) - if err != nil { - return err - } - if acl != nil && !acl.ServiceRead(query.Service.Service) { - p.srv.logger.Printf("[WARN] consul.prepared_query: Execute of prepared query for service '%s' denied due to ACLs", query.Service.Service) - return permissionDeniedErr - } - // Filter out any unhealthy nodes. nodes = nodes.Filter(query.Service.OnlyPassing) @@ -555,8 +575,8 @@ func queryFailover(q queryServer, query *structs.PreparedQuery, // Note that we pass along the limit since it can be applied // remotely to save bandwidth. We also pass along the consistency - // mode information we were given, so that applies to the remote - // query as well. + // mode information and token we were given, so that applies to + // the remote query as well. remote := &structs.PreparedQueryExecuteRemoteRequest{ Datacenter: dc, Query: *query, diff --git a/consul/prepared_query_endpoint_test.go b/consul/prepared_query_endpoint_test.go index 59d2297ea458..a6c37ce8800f 100644 --- a/consul/prepared_query_endpoint_test.go +++ b/consul/prepared_query_endpoint_test.go @@ -27,25 +27,6 @@ func TestPreparedQuery_Apply(t *testing.T) { testutil.WaitForLeader(t, s1.RPC, "dc1") - // Set up a node and service in the catalog. - { - req := structs.RegisterRequest{ - Datacenter: "dc1", - Node: "foo", - Address: "127.0.0.1", - Service: &structs.NodeService{ - Service: "redis", - Tags: []string{"master"}, - Port: 8000, - }, - } - var reply struct{} - err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &req, &reply) - if err != nil { - t.Fatalf("err: %v", err) - } - } - // Set up a bare bones query. query := structs.PreparedQueryRequest{ Datacenter: "dc1", @@ -75,7 +56,8 @@ func TestPreparedQuery_Apply(t *testing.T) { // Fix up the ID but invalidate the query itself. This proves we call // parseQuery for a create, but that function is checked in detail as - // part of another test. + // part of another test so we don't have to exercise all the checks + // here. query.Op = structs.PreparedQueryCreate query.Query.ID = "" query.Query.Service.Failover.NearestN = -1 @@ -86,14 +68,14 @@ func TestPreparedQuery_Apply(t *testing.T) { // Fix that and make sure it propagates an error from the Raft apply. query.Query.Service.Failover.NearestN = 0 - query.Query.Service.Service = "nope" + query.Query.Session = "nope" err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply) - if err == nil || !strings.Contains(err.Error(), "invalid service") { + if err == nil || !strings.Contains(err.Error(), "failed session lookup") { t.Fatalf("bad: %v", err) } // Fix that and make sure the apply goes through. - query.Query.Service.Service = "redis" + query.Query.Session = "" if err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply); err != nil { t.Fatalf("err: %v", err) } @@ -208,12 +190,12 @@ func TestPreparedQuery_Apply_ACLDeny(t *testing.T) { testutil.WaitForLeader(t, s1.RPC, "dc1") - // Create two ACLs with read permission to the service. - var token1, token2 string + // Create an ACL with write permissions for redis queries. + var token string { var rules = ` - service "redis" { - policy = "read" + query "redis" { + policy = "write" } ` @@ -227,35 +209,7 @@ func TestPreparedQuery_Apply_ACLDeny(t *testing.T) { }, WriteRequest: structs.WriteRequest{Token: "root"}, } - var reply string - - if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &req, &reply); err != nil { - t.Fatalf("err: %v", err) - } - token1 = reply - - if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &req, &reply); err != nil { - t.Fatalf("err: %v", err) - } - token2 = reply - } - - // Set up a node and service in the catalog. - { - req := structs.RegisterRequest{ - Datacenter: "dc1", - Node: "foo", - Address: "127.0.0.1", - Service: &structs.NodeService{ - Service: "redis", - Tags: []string{"master"}, - Port: 8000, - }, - WriteRequest: structs.WriteRequest{Token: "root"}, - } - var reply struct{} - err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &req, &reply) - if err != nil { + if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &req, &token); err != nil { t.Fatalf("err: %v", err) } } @@ -265,8 +219,9 @@ func TestPreparedQuery_Apply_ACLDeny(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "redis-master", Service: structs.ServiceQuery{ - Service: "redis", + Service: "the-redis", }, }, } @@ -280,14 +235,16 @@ func TestPreparedQuery_Apply_ACLDeny(t *testing.T) { } // Now add the token and try again. - query.WriteRequest.Token = token1 + query.WriteRequest.Token = token if err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply); err != nil { t.Fatalf("err: %v", err) } // Capture the ID and set the token, then read back the query to verify. + // Note that unlike previous versions of Consul, we DO NOT capture the + // token. We will set that here just to be explicit about it. query.Query.ID = reply - query.Query.Token = token1 + query.Query.Token = "" { req := &structs.PreparedQuerySpecificRequest{ Datacenter: "dc1", @@ -312,38 +269,22 @@ func TestPreparedQuery_Apply_ACLDeny(t *testing.T) { } } - // Try to do an update with a different token that does have access to - // the service, but isn't the one that was used to create the query. + // Try to do an update without a token; this should get rejected. query.Op = structs.PreparedQueryUpdate - query.WriteRequest.Token = token2 - err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply) - if err == nil || !strings.Contains(err.Error(), permissionDenied) { - t.Fatalf("bad: %v", err) - } - - // Try again with no token. query.WriteRequest.Token = "" err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply) if err == nil || !strings.Contains(err.Error(), permissionDenied) { t.Fatalf("bad: %v", err) } - // Try again with the original token. This should go through. - query.WriteRequest.Token = token1 + // Try again with the original token; this should go through. + query.WriteRequest.Token = token if err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply); err != nil { t.Fatalf("err: %v", err) } - // Try to do a delete with a different token that does have access to - // the service, but isn't the one that was used to create the query. + // Try to do a delete with no token; this should get rejected. query.Op = structs.PreparedQueryDelete - query.WriteRequest.Token = token2 - err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply) - if err == nil || !strings.Contains(err.Error(), permissionDenied) { - t.Fatalf("bad: %v", err) - } - - // Try again with no token. query.WriteRequest.Token = "" err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply) if err == nil || !strings.Contains(err.Error(), permissionDenied) { @@ -351,7 +292,7 @@ func TestPreparedQuery_Apply_ACLDeny(t *testing.T) { } // Try again with the original token. This should go through. - query.WriteRequest.Token = token1 + query.WriteRequest.Token = token if err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply); err != nil { t.Fatalf("err: %v", err) } @@ -378,15 +319,15 @@ func TestPreparedQuery_Apply_ACLDeny(t *testing.T) { // Make the query again. query.Op = structs.PreparedQueryCreate query.Query.ID = "" - query.Query.Token = "" - query.WriteRequest.Token = token1 + query.WriteRequest.Token = token if err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply); err != nil { t.Fatalf("err: %v", err) } - // Check that it's there. + // Check that it's there, and again make sure that the token did not get + // captured. query.Query.ID = reply - query.Query.Token = token1 + query.Query.Token = "" { req := &structs.PreparedQuerySpecificRequest{ Datacenter: "dc1", @@ -418,8 +359,8 @@ func TestPreparedQuery_Apply_ACLDeny(t *testing.T) { t.Fatalf("err: %v", err) } - // That last update should have changed the token to the management one. - query.Query.Token = "root" + // That last update should not have captured a token. + query.Query.Token = "" { req := &structs.PreparedQuerySpecificRequest{ Datacenter: "dc1", @@ -444,18 +385,14 @@ func TestPreparedQuery_Apply_ACLDeny(t *testing.T) { } } - // Make another query. - query.Op = structs.PreparedQueryCreate - query.Query.ID = "" - query.Query.Token = "" - query.WriteRequest.Token = token1 + // A management token should be able to delete the query no matter what. + query.Op = structs.PreparedQueryDelete + query.WriteRequest.Token = "root" if err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply); err != nil { t.Fatalf("err: %v", err) } - // Check that it's there. - query.Query.ID = reply - query.Query.Token = token1 + // Make sure the query got deleted. { req := &structs.PreparedQuerySpecificRequest{ Datacenter: "dc1", @@ -464,30 +401,28 @@ func TestPreparedQuery_Apply_ACLDeny(t *testing.T) { } var resp structs.IndexedPreparedQueries if err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Get", req, &resp); err != nil { - t.Fatalf("err: %v", err) + if err.Error() != ErrQueryNotFound.Error() { + t.Fatalf("err: %v", err) + } } - if len(resp.Queries) != 1 { + if len(resp.Queries) != 0 { t.Fatalf("bad: %v", resp) } - actual := resp.Queries[0] - if resp.Index != actual.ModifyIndex { - t.Fatalf("bad index: %d", resp.Index) - } - actual.CreateIndex, actual.ModifyIndex = 0, 0 - if !reflect.DeepEqual(actual, query.Query) { - t.Fatalf("bad: %v", actual) - } } - // A management token should be able to delete the query no matter what. - query.Op = structs.PreparedQueryDelete + // Use the root token to make a query under a different name. + query.Op = structs.PreparedQueryCreate + query.Query.ID = "" + query.Query.Name = "cassandra" query.WriteRequest.Token = "root" if err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply); err != nil { t.Fatalf("err: %v", err) } - // Make sure the query got deleted. + // Check that it's there and that the token did not get captured. + query.Query.ID = reply + query.Query.Token = "" { req := &structs.PreparedQuerySpecificRequest{ Datacenter: "dc1", @@ -496,14 +431,30 @@ func TestPreparedQuery_Apply_ACLDeny(t *testing.T) { } var resp structs.IndexedPreparedQueries if err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Get", req, &resp); err != nil { - if err.Error() != ErrQueryNotFound.Error() { - t.Fatalf("err: %v", err) - } + t.Fatalf("err: %v", err) } - if len(resp.Queries) != 0 { + if len(resp.Queries) != 1 { t.Fatalf("bad: %v", resp) } + actual := resp.Queries[0] + if resp.Index != actual.ModifyIndex { + t.Fatalf("bad index: %d", resp.Index) + } + actual.CreateIndex, actual.ModifyIndex = 0, 0 + if !reflect.DeepEqual(actual, query.Query) { + t.Fatalf("bad: %v", actual) + } + } + + // Now try to change that to redis with the valid redis token. This will + // fail because that token can't change cassandra queries. + query.Op = structs.PreparedQueryUpdate + query.Query.Name = "redis" + query.WriteRequest.Token = token + err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("bad: %v", err) } } @@ -588,6 +539,17 @@ func TestPreparedQuery_parseQuery(t *testing.T) { t.Fatalf("err: %v", err) } + query.Token = redactedToken + err = parseQuery(query) + if err == nil || !strings.Contains(err.Error(), "Bad Token") { + t.Fatalf("bad: %v", err) + } + + query.Token = "adf4238a-882b-9ddc-4a9d-5b6758e4159e" + if err := parseQuery(query); err != nil { + t.Fatalf("err: %v", err) + } + query.Service.Failover.NearestN = -1 err = parseQuery(query) if err == nil || !strings.Contains(err.Error(), "Bad NearestN") { @@ -630,12 +592,12 @@ func TestPreparedQuery_Get(t *testing.T) { testutil.WaitForLeader(t, s1.RPC, "dc1") - // Create two ACLs with read permission to the service. - var token1, token2 string + // Create an ACL with write permissions for redis queries. + var token string { var rules = ` - service "redis" { - policy = "read" + query "redis" { + policy = "write" } ` @@ -649,35 +611,7 @@ func TestPreparedQuery_Get(t *testing.T) { }, WriteRequest: structs.WriteRequest{Token: "root"}, } - var reply string - - if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &req, &reply); err != nil { - t.Fatalf("err: %v", err) - } - token1 = reply - - if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &req, &reply); err != nil { - t.Fatalf("err: %v", err) - } - token2 = reply - } - - // Set up a node and service in the catalog. - { - req := structs.RegisterRequest{ - Datacenter: "dc1", - Node: "foo", - Address: "127.0.0.1", - Service: &structs.NodeService{ - Service: "redis", - Tags: []string{"master"}, - Port: 8000, - }, - WriteRequest: structs.WriteRequest{Token: "root"}, - } - var reply struct{} - err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &req, &reply) - if err != nil { + if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &req, &token); err != nil { t.Fatalf("err: %v", err) } } @@ -687,26 +621,25 @@ func TestPreparedQuery_Get(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ - Name: "my-query", + Name: "redis-master", Service: structs.ServiceQuery{ - Service: "redis", + Service: "the-redis", }, }, - WriteRequest: structs.WriteRequest{Token: token1}, + WriteRequest: structs.WriteRequest{Token: token}, } var reply string if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply); err != nil { t.Fatalf("err: %v", err) } - // Capture the ID and set the token, then read back the query to verify. + // Capture the ID, then read back the query to verify. query.Query.ID = reply - query.Query.Token = token1 { req := &structs.PreparedQuerySpecificRequest{ Datacenter: "dc1", QueryID: query.Query.ID, - QueryOptions: structs.QueryOptions{Token: token1}, + QueryOptions: structs.QueryOptions{Token: token}, } var resp structs.IndexedPreparedQueries if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.Get", req, &resp); err != nil { @@ -726,14 +659,12 @@ func TestPreparedQuery_Get(t *testing.T) { } } - // Now try to read it with a token that has read access to the - // service but isn't the token used to create the query. This should - // be denied. + // Try again with no token, which should return an error. { req := &structs.PreparedQuerySpecificRequest{ Datacenter: "dc1", QueryID: query.Query.ID, - QueryOptions: structs.QueryOptions{Token: token2}, + QueryOptions: structs.QueryOptions{Token: ""}, } var resp structs.IndexedPreparedQueries err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.Get", req, &resp) @@ -746,30 +677,46 @@ func TestPreparedQuery_Get(t *testing.T) { } } - // Try again with no token, which should also be denied. + // A management token should be able to read no matter what. { req := &structs.PreparedQuerySpecificRequest{ Datacenter: "dc1", QueryID: query.Query.ID, - QueryOptions: structs.QueryOptions{Token: ""}, + QueryOptions: structs.QueryOptions{Token: "root"}, } var resp structs.IndexedPreparedQueries - err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.Get", req, &resp) - if err == nil || !strings.Contains(err.Error(), permissionDenied) { - t.Fatalf("bad: %v", err) + if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.Get", req, &resp); err != nil { + t.Fatalf("err: %v", err) } - if len(resp.Queries) != 0 { + if len(resp.Queries) != 1 { t.Fatalf("bad: %v", resp) } + actual := resp.Queries[0] + if resp.Index != actual.ModifyIndex { + t.Fatalf("bad index: %d", resp.Index) + } + actual.CreateIndex, actual.ModifyIndex = 0, 0 + if !reflect.DeepEqual(actual, query.Query) { + t.Fatalf("bad: %v", actual) + } } - // A management token should be able to read no matter what. + // Now update the query to take away its name. + query.Op = structs.PreparedQueryUpdate + query.Query.Name = "" + if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply); err != nil { + t.Fatalf("err: %v", err) + } + + // Try again with no token, this should work since this query is only + // managed by an ID (no name) so no ACLs apply to it. + query.Query.ID = reply { req := &structs.PreparedQuerySpecificRequest{ Datacenter: "dc1", QueryID: query.Query.ID, - QueryOptions: structs.QueryOptions{Token: "root"}, + QueryOptions: structs.QueryOptions{Token: ""}, } var resp structs.IndexedPreparedQueries if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.Get", req, &resp); err != nil { @@ -794,7 +741,7 @@ func TestPreparedQuery_Get(t *testing.T) { req := &structs.PreparedQuerySpecificRequest{ Datacenter: "dc1", QueryID: generateUUID(), - QueryOptions: structs.QueryOptions{Token: token1}, + QueryOptions: structs.QueryOptions{Token: token}, } var resp structs.IndexedPreparedQueries if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.Get", req, &resp); err != nil { @@ -822,12 +769,12 @@ func TestPreparedQuery_List(t *testing.T) { testutil.WaitForLeader(t, s1.RPC, "dc1") - // Create an ACL with read permission to the service. + // Create an ACL with write permissions for redis queries. var token string { var rules = ` - service "redis" { - policy = "read" + query "redis" { + policy = "write" } ` @@ -846,31 +793,11 @@ func TestPreparedQuery_List(t *testing.T) { } } - // Set up a node and service in the catalog. - { - req := structs.RegisterRequest{ - Datacenter: "dc1", - Node: "foo", - Address: "127.0.0.1", - Service: &structs.NodeService{ - Service: "redis", - Tags: []string{"master"}, - Port: 8000, - }, - WriteRequest: structs.WriteRequest{Token: "root"}, - } - var reply struct{} - err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &req, &reply) - if err != nil { - t.Fatalf("err: %v", err) - } - } - - // Query with a legit management token but no queries. + // Query with a legit token but no queries. { req := &structs.DCSpecificRequest{ Datacenter: "dc1", - QueryOptions: structs.QueryOptions{Token: "root"}, + QueryOptions: structs.QueryOptions{Token: token}, } var resp structs.IndexedPreparedQueries if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.List", req, &resp); err != nil { @@ -887,9 +814,9 @@ func TestPreparedQuery_List(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ - Name: "my-query", + Name: "redis-master", Service: structs.ServiceQuery{ - Service: "redis", + Service: "the-redis", }, }, WriteRequest: structs.WriteRequest{Token: token}, @@ -899,36 +826,89 @@ func TestPreparedQuery_List(t *testing.T) { t.Fatalf("err: %v", err) } - // Capture the ID and set the token, then try to list all the queries. - // A management token is required so this should be denied. + // Capture the ID and read back the query to verify. query.Query.ID = reply - query.Query.Token = token { req := &structs.DCSpecificRequest{ Datacenter: "dc1", QueryOptions: structs.QueryOptions{Token: token}, } var resp structs.IndexedPreparedQueries - err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.List", req, &resp) - if err == nil || !strings.Contains(err.Error(), permissionDenied) { - t.Fatalf("bad: %v", err) + if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.List", req, &resp); err != nil { + t.Fatalf("err: %v", err) } - if len(resp.Queries) != 0 { + if len(resp.Queries) != 1 { t.Fatalf("bad: %v", resp) } + actual := resp.Queries[0] + if resp.Index != actual.ModifyIndex { + t.Fatalf("bad index: %d", resp.Index) + } + actual.CreateIndex, actual.ModifyIndex = 0, 0 + if !reflect.DeepEqual(actual, query.Query) { + t.Fatalf("bad: %v", actual) + } } - // An empty token should fail in a similar way. + // An empty token should result in an empty list because of ACL + // filtering. { req := &structs.DCSpecificRequest{ Datacenter: "dc1", QueryOptions: structs.QueryOptions{Token: ""}, } var resp structs.IndexedPreparedQueries - err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.List", req, &resp) - if err == nil || !strings.Contains(err.Error(), permissionDenied) { - t.Fatalf("bad: %v", err) + if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.List", req, &resp); err != nil { + t.Fatalf("err: %v", err) + } + + if len(resp.Queries) != 0 { + t.Fatalf("bad: %v", resp) + } + } + + // But a management token should work. + { + req := &structs.DCSpecificRequest{ + Datacenter: "dc1", + QueryOptions: structs.QueryOptions{Token: "root"}, + } + var resp structs.IndexedPreparedQueries + if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.List", req, &resp); err != nil { + t.Fatalf("err: %v", err) + } + + if len(resp.Queries) != 1 { + t.Fatalf("bad: %v", resp) + } + actual := resp.Queries[0] + if resp.Index != actual.ModifyIndex { + t.Fatalf("bad index: %d", resp.Index) + } + actual.CreateIndex, actual.ModifyIndex = 0, 0 + if !reflect.DeepEqual(actual, query.Query) { + t.Fatalf("bad: %v", actual) + } + } + + // Now take away the query name. + query.Op = structs.PreparedQueryUpdate + query.Query.Name = "" + if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply); err != nil { + t.Fatalf("err: %v", err) + } + + // A query with the redis token shouldn't show anything since it doesn't + // match any un-named queries. + { + req := &structs.DCSpecificRequest{ + Datacenter: "dc1", + QueryOptions: structs.QueryOptions{Token: token}, + } + var resp structs.IndexedPreparedQueries + if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.List", req, &resp); err != nil { + t.Fatalf("err: %v", err) } if len(resp.Queries) != 0 { @@ -936,7 +916,7 @@ func TestPreparedQuery_List(t *testing.T) { } } - // Now try a legit management token. + // But a management token should work. { req := &structs.DCSpecificRequest{ Datacenter: "dc1", @@ -978,8 +958,6 @@ func TestPreparedQuery_Execute(t *testing.T) { dir2, s2 := testServerWithConfig(t, func(c *Config) { c.Datacenter = "dc2" c.ACLDatacenter = "dc1" - c.ACLMasterToken = "root" - c.ACLDefaultPolicy = "deny" }) defer os.RemoveAll(dir2) defer s2.Shutdown() @@ -1004,7 +982,7 @@ func TestPreparedQuery_Execute(t *testing.T) { }) // Create an ACL with read permission to the service. - var token string + var execToken string { var rules = ` service "foo" { @@ -1022,7 +1000,7 @@ func TestPreparedQuery_Execute(t *testing.T) { }, WriteRequest: structs.WriteRequest{Token: "root"}, } - if err := msgpackrpc.CallWithCodec(codec1, "ACL.Apply", &req, &token); err != nil { + if err := msgpackrpc.CallWithCodec(codec1, "ACL.Apply", &req, &execToken); err != nil { t.Fatalf("err: %v", err) } } @@ -1070,7 +1048,7 @@ func TestPreparedQuery_Execute(t *testing.T) { TTL: "10s", }, }, - WriteRequest: structs.WriteRequest{Token: token}, + WriteRequest: structs.WriteRequest{Token: "root"}, } if err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Apply", &query, &query.Query.ID); err != nil { t.Fatalf("err: %v", err) @@ -1099,6 +1077,7 @@ func TestPreparedQuery_Execute(t *testing.T) { req := structs.PreparedQueryExecuteRequest{ Datacenter: "dc1", QueryIDOrName: query.Query.ID, + QueryOptions: structs.QueryOptions{Token: execToken}, } var reply structs.PreparedQueryExecuteResponse @@ -1121,6 +1100,7 @@ func TestPreparedQuery_Execute(t *testing.T) { Datacenter: "dc1", QueryIDOrName: query.Query.ID, Limit: 3, + QueryOptions: structs.QueryOptions{Token: execToken}, } var reply structs.PreparedQueryExecuteResponse @@ -1163,6 +1143,7 @@ func TestPreparedQuery_Execute(t *testing.T) { Datacenter: "dc1", Node: "node3", }, + QueryOptions: structs.QueryOptions{Token: execToken}, } var reply structs.PreparedQueryExecuteResponse @@ -1188,6 +1169,7 @@ func TestPreparedQuery_Execute(t *testing.T) { req := structs.PreparedQueryExecuteRequest{ Datacenter: "dc1", QueryIDOrName: query.Query.ID, + QueryOptions: structs.QueryOptions{Token: execToken}, } var reply structs.PreparedQueryExecuteResponse @@ -1247,6 +1229,7 @@ func TestPreparedQuery_Execute(t *testing.T) { req := structs.PreparedQueryExecuteRequest{ Datacenter: "dc1", QueryIDOrName: query.Query.ID, + QueryOptions: structs.QueryOptions{Token: execToken}, } var reply structs.PreparedQueryExecuteResponse @@ -1274,6 +1257,7 @@ func TestPreparedQuery_Execute(t *testing.T) { req := structs.PreparedQueryExecuteRequest{ Datacenter: "dc1", QueryIDOrName: query.Query.ID, + QueryOptions: structs.QueryOptions{Token: execToken}, } var reply structs.PreparedQueryExecuteResponse @@ -1302,6 +1286,7 @@ func TestPreparedQuery_Execute(t *testing.T) { req := structs.PreparedQueryExecuteRequest{ Datacenter: "dc1", QueryIDOrName: query.Query.ID, + QueryOptions: structs.QueryOptions{Token: execToken}, } var reply structs.PreparedQueryExecuteResponse @@ -1337,6 +1322,7 @@ func TestPreparedQuery_Execute(t *testing.T) { req := structs.PreparedQueryExecuteRequest{ Datacenter: "dc1", QueryIDOrName: query.Query.ID, + QueryOptions: structs.QueryOptions{Token: execToken}, } var reply structs.PreparedQueryExecuteResponse @@ -1358,6 +1344,113 @@ func TestPreparedQuery_Execute(t *testing.T) { } } + // Make a new exec token that can't read the service. + var denyToken string + { + var rules = ` + service "foo" { + policy = "deny" + } + ` + + req := structs.ACLRequest{ + Datacenter: "dc1", + Op: structs.ACLSet, + ACL: structs.ACL{ + Name: "User token", + Type: structs.ACLTypeClient, + Rules: rules, + }, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + if err := msgpackrpc.CallWithCodec(codec1, "ACL.Apply", &req, &denyToken); err != nil { + t.Fatalf("err: %v", err) + } + } + + // Make sure the query gets denied with this token. + { + req := structs.PreparedQueryExecuteRequest{ + Datacenter: "dc1", + QueryIDOrName: query.Query.ID, + QueryOptions: structs.QueryOptions{Token: denyToken}, + } + + var reply structs.PreparedQueryExecuteResponse + if err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Execute", &req, &reply); err != nil { + t.Fatalf("err: %v", err) + } + + if len(reply.Nodes) != 0 || + reply.Datacenter != "dc1" || reply.Failovers != 0 || + reply.Service != query.Query.Service.Service || + !reflect.DeepEqual(reply.DNS, query.Query.DNS) || + !reply.QueryMeta.KnownLeader { + t.Fatalf("bad: %v", reply) + } + } + + // Bake the exec token into the query. + query.Query.Token = execToken + if err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Apply", &query, &query.Query.ID); err != nil { + t.Fatalf("err: %v", err) + } + + // Now even querying with the deny token should work. + { + req := structs.PreparedQueryExecuteRequest{ + Datacenter: "dc1", + QueryIDOrName: query.Query.ID, + QueryOptions: structs.QueryOptions{Token: denyToken}, + } + + var reply structs.PreparedQueryExecuteResponse + if err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Execute", &req, &reply); err != nil { + t.Fatalf("err: %v", err) + } + + if len(reply.Nodes) != 8 || + reply.Datacenter != "dc1" || reply.Failovers != 0 || + reply.Service != query.Query.Service.Service || + !reflect.DeepEqual(reply.DNS, query.Query.DNS) || + !reply.QueryMeta.KnownLeader { + t.Fatalf("bad: %v", reply) + } + for _, node := range reply.Nodes { + if node.Node.Node == "node1" || node.Node.Node == "node3" { + t.Fatalf("bad: %v", node) + } + } + } + + // Un-bake the token. + query.Query.Token = "" + if err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Apply", &query, &query.Query.ID); err != nil { + t.Fatalf("err: %v", err) + } + + // Make sure the query gets denied again with the deny token. + { + req := structs.PreparedQueryExecuteRequest{ + Datacenter: "dc1", + QueryIDOrName: query.Query.ID, + QueryOptions: structs.QueryOptions{Token: denyToken}, + } + + var reply structs.PreparedQueryExecuteResponse + if err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Execute", &req, &reply); err != nil { + t.Fatalf("err: %v", err) + } + + if len(reply.Nodes) != 0 || + reply.Datacenter != "dc1" || reply.Failovers != 0 || + reply.Service != query.Query.Service.Service || + !reflect.DeepEqual(reply.DNS, query.Query.DNS) || + !reply.QueryMeta.KnownLeader { + t.Fatalf("bad: %v", reply) + } + } + // Now fail everything in dc1 and we should get an empty list back. for i := 0; i < 10; i++ { setHealth(fmt.Sprintf("node%d", i+1), structs.HealthCritical) @@ -1366,6 +1459,7 @@ func TestPreparedQuery_Execute(t *testing.T) { req := structs.PreparedQueryExecuteRequest{ Datacenter: "dc1", QueryIDOrName: query.Query.ID, + QueryOptions: structs.QueryOptions{Token: execToken}, } var reply structs.PreparedQueryExecuteResponse @@ -1393,6 +1487,7 @@ func TestPreparedQuery_Execute(t *testing.T) { req := structs.PreparedQueryExecuteRequest{ Datacenter: "dc1", QueryIDOrName: query.Query.ID, + QueryOptions: structs.QueryOptions{Token: execToken}, } var reply structs.PreparedQueryExecuteResponse @@ -1420,7 +1515,10 @@ func TestPreparedQuery_Execute(t *testing.T) { Datacenter: "dc1", QueryIDOrName: query.Query.ID, Limit: 3, - QueryOptions: structs.QueryOptions{RequireConsistent: true}, + QueryOptions: structs.QueryOptions{ + Token: execToken, + RequireConsistent: true, + }, } var reply structs.PreparedQueryExecuteResponse @@ -1448,6 +1546,7 @@ func TestPreparedQuery_Execute(t *testing.T) { req := structs.PreparedQueryExecuteRequest{ Datacenter: "dc1", QueryIDOrName: query.Query.ID, + QueryOptions: structs.QueryOptions{Token: execToken}, } var reply structs.PreparedQueryExecuteResponse @@ -1477,46 +1576,59 @@ func TestPreparedQuery_Execute(t *testing.T) { t.Fatalf("unique shuffle ratio too low: %d/100", len(uniques)) } - // Finally, take away the token's ability to read the service. + // Make sure the query response from dc2 gets denied with the deny token. { - var rules = ` - service "foo" { - policy = "deny" - } - ` - - req := structs.ACLRequest{ - Datacenter: "dc1", - Op: structs.ACLSet, - ACL: structs.ACL{ - ID: token, - Name: "User token", - Type: structs.ACLTypeClient, - Rules: rules, - }, - WriteRequest: structs.WriteRequest{Token: "root"}, + req := structs.PreparedQueryExecuteRequest{ + Datacenter: "dc1", + QueryIDOrName: query.Query.ID, + QueryOptions: structs.QueryOptions{Token: denyToken}, } - if err := msgpackrpc.CallWithCodec(codec1, "ACL.Apply", &req, &token); err != nil { + + var reply structs.PreparedQueryExecuteResponse + if err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Execute", &req, &reply); err != nil { t.Fatalf("err: %v", err) } + + if len(reply.Nodes) != 0 || + reply.Datacenter != "dc2" || reply.Failovers != 1 || + reply.Service != query.Query.Service.Service || + !reflect.DeepEqual(reply.DNS, query.Query.DNS) || + !reply.QueryMeta.KnownLeader { + t.Fatalf("bad: %v", reply) + } } - // Now the query should be denied. + // Bake the exec token into the query. + query.Query.Token = execToken + if err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Apply", &query, &query.Query.ID); err != nil { + t.Fatalf("err: %v", err) + } + + // Now even querying with the deny token should work. { req := structs.PreparedQueryExecuteRequest{ Datacenter: "dc1", QueryIDOrName: query.Query.ID, + QueryOptions: structs.QueryOptions{Token: denyToken}, } var reply structs.PreparedQueryExecuteResponse - err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Execute", &req, &reply) - if err == nil || !strings.Contains(err.Error(), permissionDenied) { - t.Fatalf("bad: %v", err) + if err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Execute", &req, &reply); err != nil { + t.Fatalf("err: %v", err) } - if len(reply.Nodes) != 0 { + if len(reply.Nodes) != 9 || + reply.Datacenter != "dc2" || reply.Failovers != 1 || + reply.Service != query.Query.Service.Service || + !reflect.DeepEqual(reply.DNS, query.Query.DNS) || + !reply.QueryMeta.KnownLeader { t.Fatalf("bad: %v", reply) } + for _, node := range reply.Nodes { + if node.Node.Node == "node3" { + t.Fatalf("bad: %v", node) + } + } } } diff --git a/consul/state/prepared_query.go b/consul/state/prepared_query.go index f2fc1a8b059f..dad07b89621c 100644 --- a/consul/state/prepared_query.go +++ b/consul/state/prepared_query.go @@ -119,14 +119,9 @@ func (s *StateStore) preparedQuerySetTxn(tx *memdb.Txn, idx uint64, query *struc } } - // Verify that the service exists. - service, err := tx.First("services", "service", query.Service.Service) - if err != nil { - return fmt.Errorf("failed service lookup: %s", err) - } - if service == nil { - return fmt.Errorf("invalid service %#v", query.Service.Service) - } + // We do not verify the service here, nor the token, if any. These are + // checked at execute time and not doing integrity checking on them + // helps avoid bootstrapping chicken and egg problems. // Insert the query. if err := tx.Insert("prepared-queries", query); err != nil { diff --git a/consul/state/prepared_query_test.go b/consul/state/prepared_query_test.go index e8261b8c42c0..83c3625c3bb7 100644 --- a/consul/state/prepared_query_test.go +++ b/consul/state/prepared_query_test.go @@ -54,15 +54,16 @@ func TestStateStore_PreparedQuerySet_PreparedQueryGet(t *testing.T) { // Build a legit-looking query with the most basic options. query := &structs.PreparedQuery{ - ID: testUUID(), + ID: testUUID(), + Session: "nope", Service: structs.ServiceQuery{ Service: "redis", }, } - // The set will still fail because the service isn't registered yet. + // The set will still fail because the session is bogus. err = s.PreparedQuerySet(1, query) - if err == nil || !strings.Contains(err.Error(), "invalid service") { + if err == nil || !strings.Contains(err.Error(), "failed session lookup") { t.Fatalf("bad: %v", err) } @@ -71,9 +72,10 @@ func TestStateStore_PreparedQuerySet_PreparedQueryGet(t *testing.T) { t.Fatalf("bad index: %d", idx) } - // Now register the service. + // Now register the service and remove the bogus session. testRegisterNode(t, s, 1, "foo") testRegisterService(t, s, 2, "foo", "redis") + query.Session = "" // This should go through. if err := s.PreparedQuerySet(3, query); err != nil { diff --git a/consul/structs/prepared_query.go b/consul/structs/prepared_query.go index af360f7d248a..6058ca50f1a8 100644 --- a/consul/structs/prepared_query.go +++ b/consul/structs/prepared_query.go @@ -72,6 +72,17 @@ type PreparedQuery struct { RaftIndex } +// GetACLPrefix returns the prefix to look up the prepared_query ACL policy for +// this query, and whether the prefix applies to this query. You always need to +// check the ok value before using the prefix. +func (pq *PreparedQuery) GetACLPrefix() (string, bool) { + if pq.Name != "" { + return pq.Name, true + } + + return "", false +} + type PreparedQueries []*PreparedQuery type IndexedPreparedQueries struct { diff --git a/consul/structs/prepared_query_test.go b/consul/structs/prepared_query_test.go new file mode 100644 index 000000000000..b80fff897626 --- /dev/null +++ b/consul/structs/prepared_query_test.go @@ -0,0 +1,17 @@ +package structs + +import ( + "testing" +) + +func TestStructs_PreparedQuery_GetACLPrefix(t *testing.T) { + ephemeral := &PreparedQuery{} + if prefix, ok := ephemeral.GetACLPrefix(); ok { + t.Fatalf("bad: %s", prefix) + } + + named := &PreparedQuery{Name: "hello"} + if prefix, ok := named.GetACLPrefix(); !ok || prefix != "hello" { + t.Fatalf("bad: %#v", prefix) + } +} diff --git a/website/source/docs/agent/http/query.html.markdown b/website/source/docs/agent/http/query.html.markdown index a16e08bbdd3f..e3cb32602fd7 100644 --- a/website/source/docs/agent/http/query.html.markdown +++ b/website/source/docs/agent/http/query.html.markdown @@ -15,7 +15,7 @@ Prepared queries allow you to register a complex service query and then execute it later via its ID or name to get a set of healthy nodes that provide a given service. This is particularly useful in combination with Consul's [DNS Interface](/docs/agent/dns.html) as it allows for much richer queries than -would be possible given the limited interface DNS provides. +would be possible given the limited entry points exposed by DNS. The following endpoints are supported: @@ -29,8 +29,12 @@ The following endpoints are supported: Not all endpoints support blocking queries and all consistency modes, see details in the sections below. -The query endpoints support the use of ACL tokens. Prepared queries have some -special handling of ACL tokens that are highlighted in the sections below. +The query endpoints support the use of ACL Tokens. Prepared queries have some +special handling of ACL Tokens that are called out where applicable with the +details of each endpoint. + +See the [Prepared Query ACLs](/docs/internals/acl.html#prepared_query_acls) +internals guide for more details about how prepared query policies work. ### /v1/query @@ -41,9 +45,12 @@ The general query endpoint supports the `POST` and `GET` methods. When using the `POST` method, Consul will create a new prepared query and return its ID if it is created successfully. -By default, the datacenter of the agent is queried; however, the dc can be +By default, the datacenter of the agent is queried; however, the `dc` can be provided using the "?dc=" query parameter. +If ACLs are enabled, the client will need to supply an ACL Token with `query` +write privileges for the `Name` of the query being created. + The create operation expects a JSON request body that defines the prepared query, like this example: @@ -51,6 +58,7 @@ like this example: { "Name": "my-query", "Session": "adf4238a-882b-9ddc-4a9d-5b6758e4159e", + "Token": "", "Service": { "Service": "redis", "Failover": { @@ -76,6 +84,25 @@ of using its ID. given session is invalidated. This is optional, and if not given the prepared query must be manually removed when no longer needed. + +`Token`, if specified, is a captured ACL Token that is reused as the ACL Token +every time the query is executed. This allows queries to be executed by clients +with lesser or even no ACL Token, so this should be used with care. The token +itself can only be seen by clients with a management token. If the `Token` +field is left blank or omitted, the client's ACL Token will be used to determine +if they have access to the service being queried. If the client does not supply +an ACL Token, the anonymous token will be used. + +Note that Consul version 0.6.3 and earlier would automatically capture the ACL +Token for use in the future when prepared queries were executed and would +execute with the same privileges as the definer of the prepared query. Older +queries wishing to obtain the new behavior will need to be updated to remove +their captured `Token` field. Capturing ACL Tokens is analogous to +[PostgreSQL’s SECURITY DEFINER](http://www.postgresql.org/docs/current/static/sql-createfunction.html) +attribute which can be set on functions. This change in effect moves Consul +from using `SECURITY DEFINER` by default to `SECURITY INVOKER` by default for +new Prepared Queries. + The set of fields inside the `Service` structure define the query's behavior. `Service` is the name of the service to query. This is required. @@ -130,28 +157,18 @@ a JSON body: } ``` -If ACLs are enabled, then the provided token will be used to check access to -the service being queried, and it will be saved along with the query for use -when the query is executed. This is key to allowing prepared queries to work -via the DNS interface, and it's important to note that prepared query IDs and -names become a read-only proxy for the token used to create the query. - -The query IDs that Consul generates are done in the same manner as ACL tokens, -so provide equal strength, but names may be more guessable and should be used -carefully with ACLs. Also, the token used to create the prepared query (or a -management token) is required to read the query back, so the ability to execute -a prepared query is not enough to get access to the actual token. - #### GET Method When using the GET method, Consul will provide a listing of all prepared queries. -By default, the datacenter of the agent is queried; however, the dc can be +By default, the datacenter of the agent is queried; however, the `dc` can be provided using the "?dc=" query parameter. This endpoint supports blocking queries and all consistency modes. -Since this listing includes sensitive ACL tokens, this is a privileged endpoint -and always requires a management token to be supplied if ACLs are enabled. +If ACLs are enabled, then the client will only see prepared queries for which their +token has `query` read privileges. A management token will be able to see all +prepared queries. Tokens will be redacted and displayed as `` unless a +management token is used. This returns a JSON list of prepared queries, which looks like: @@ -161,7 +178,7 @@ This returns a JSON list of prepared queries, which looks like: "ID": "8f246b77-f3e1-ff88-5b48-8ec93abf3e05", "Name": "my-query", "Session": "adf4238a-882b-9ddc-4a9d-5b6758e4159e", - "Token": "", + "Token": "", "Service": { "Service": "redis", "Failover": { @@ -191,11 +208,11 @@ The query-specific endpoint supports the `GET`, `PUT`, and `DELETE` methods. The The `PUT` method allows an existing prepared query to be updated. -By default, the datacenter of the agent is queried; however, the dc can be +By default, the datacenter of the agent is queried; however, the `dc` can be provided using the "?dc=" query parameter. -If ACLs are enabled, then the same token used to create the query (or a -management token) must be supplied. +If ACLs are enabled, the client will need to supply an ACL Token with `query` +write privileges for the `Name` of the query being updated. The body is the same as is used to create a prepared query, as described above. @@ -205,7 +222,7 @@ If the API call succeeds, a 200 status code is returned. The `GET` method allows an existing prepared query to be fetched. -By default, the datacenter of the agent is queried; however, the dc can be +By default, the datacenter of the agent is queried; however, the `dc` can be provided using the "?dc=" query parameter. This endpoint supports blocking queries and all consistency modes. @@ -213,18 +230,20 @@ The returned response is the same as the list of prepared queries above, only with a single item present. If the query does not exist then a 404 status code will be returned. -If ACLs are enabled, then the same token used to create the query (or a -management token) must be supplied. +If ACLs are enabled, then the client will only see prepared queries for which their +token has `query` read privileges. A management token will be able to see all +prepared queries. Tokens will be redacted and displayed as `` unless a +management token is used. #### DELETE Method The `DELETE` method is used to delete a prepared query. -By default, the datacenter of the agent is queried; however, the dc can be +By default, the datacenter of the agent is queried; however, the `dc` can be provided using the "?dc=" query parameter. -If ACLs are enabled, then the same token used to create the query (or a -management token) must be supplied. +If ACLs are enabled, the client will need to supply an ACL Token with `query` +write privileges for the `Name` of the query being deleted. No body is required as part of this request. @@ -236,7 +255,7 @@ The query execute endpoint supports only the `GET` method and is used to execute a prepared query. The \ argument is the ID or name of an existing prepared query. -By default, the datacenter of the agent is queried; however, the dc can be +By default, the datacenter of the agent is queried; however, the `dc` can be provided using the "?dc=" query parameter. This endpoint does not support blocking queries, but it does support all consistency modes. @@ -249,8 +268,9 @@ order each time the query is executed. An optional "?limit=" parameter can be used to limit the size of the list to the given number of nodes. This is applied after any sorting or shuffling. -The ACL token supplied when the prepared query was created will be used to -execute the request, so no ACL token needs to be supplied (it will be ignored). +If an ACL Token was bound to the query when it was defined then it will be used +when executing the request. Otherwise, the client's supplied ACL Token will be +used. No body is required as part of this request. diff --git a/website/source/docs/internals/acl.html.markdown b/website/source/docs/internals/acl.html.markdown index c73b7fe7714b..6d9888684a06 100644 --- a/website/source/docs/internals/acl.html.markdown +++ b/website/source/docs/internals/acl.html.markdown @@ -147,6 +147,11 @@ event "" { As always, the more secure way to handle user events is to explicitly grant access to each API token based on the events they should be able to fire. +### Blacklist mode and Prepared Queries + +After Consul 0.6.3, significant changes were made to ACLs for prepared queries, +incuding a new `query` ACL policy. See [Prepared Query ACLs](#prepared_query_acls) below for more details. + ### Blacklist mode and Keyring Operations Consul 0.6 and later supports securing the encryption keyring operations using @@ -209,6 +214,10 @@ applied to any user event without a matching policy, is provided by an empty string. An event policy is one of "read", "write", or "deny". Currently, only the "write" level is enforced during event firing. Events can always be read. +Prepared query policies control access to create, update, and delete prepared +queries. Service policies are used when executing prepared queries. See +[below](#prepared_query_acls) for more details. + We make use of the [HashiCorp Configuration Language (HCL)](https://github.com/hashicorp/hcl/) to specify policy. This language is human readable and interoperable @@ -251,6 +260,11 @@ event "destroy-" { policy = "deny" } +# Default prepared queries to read-only. +query "" { + policy = "read" +} + # Read-only mode for the encryption keyring by default (list only) keyring = "read" ``` @@ -286,6 +300,11 @@ This is equivalent to the following JSON input: "policy": "deny" } }, + "query": { + "": { + "policy": "read" + } + }, "keyring": "read" } ``` @@ -325,3 +344,112 @@ making it appear as though the restricted services do not exist. Consul's DNS interface is also affected by restrictions to service registrations. If the token used by the agent does not have access to a given service, then the DNS interface will return no records when queried for it. + + +## Prepared Query ACLs + +As we've gotten feedback from Consul users, we've evolved how prepared queries +use ACLs. In this section we first cover the current implementation, and then we +follow that with details about what's changed between specific versions of Consul. + +#### Managing Prepared Queries + +Managing prepared queries includes creating, reading, updating, and deleting +queries. There are a few variations, each of which uses ACLs in one of two +ways: open, protected by unguessable IDs or closed, managed by ACL policies. +These variations are covered here, with examples: + +* Static queries with no `Name` defined are not controlled by any ACL policies. + These types of queries are meant to be ephemeral and not shared to untrusted + clients, and they are only reachable if the prepared query ID is known. Since + these IDs are generated using the same random ID scheme as ACL Tokens, it is + infeasible to guess them. When listing all prepared queries, only a management + token will be able to see these types, though clients can read instances for + which they have an ID. An example use for this type is a query built by a + startup script, tied to a session, and written to a configuration file for a + process to use via DNS. + +* Static queries with a `Name` defined are controlled by the + [`query`](/docs/internals/acl.html#prepared_query_acls) ACL policy. + Clients are required to have an ACL token with a prefix sufficient to cover + the name they are trying to manage, with a longest prefix match providing a + way to define more specific policies. Clients can list or read queries for + which they have "read" access based on their prefix, and similar they can + update any queries for which they have "write" access. An example use for + this type is a query with a well-known name (eg. `prod-master-customer-db`) + that is used and known by many clients to provide geo-failover behavior for + a database. + +#### Executing Pepared Queries + +When prepared queries are executed via DNS lookups or HTTP requests, the ACL +checks are run against the service being queried, similar to how ACLs work with +other service lookups. There are several ways the ACL token is selected for this +check: + +* If an ACL Token was captured when the prepared query was defined, it will be + used to perform the service lookup. This allows queries to be executed by + clients with lesser or even no ACL Token, so this should be used with care. + +* If no ACL Token was captured, then the client's ACL Token will be used to + perform the service lookup. + +* If no ACL Token was captured and the client has no ACL Token, then the + anonymous token will be used to perform the service lookup. + +In the common case, the ACL Token of the invoker is used +to test the ability to look up a service. If a `Token` was specified when the +prepared query was created, the behavior changes and now the captured +ACL Token set by the definer of the query is used when lookup up a service. + +Capturing ACL Tokens is analogous to +[PostgreSQL’s](http://www.postgresql.org/docs/current/static/sql-createfunction.html) +`SECURITY DEFINER` attribute which can be set on functions, and using the client's ACL +Token is similar to the complementary `SECURITY INVOKER` attribute. + + +#### ACL Implementation Changes + +Prepared queries were originally introduced in Consul 0.6.0, and ACL behavior remained +unchanged through version 0.6.3, but was then changed to allow better management of the +prepared query namespace. + +These differences are outlined in the table below: + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
OperationVersion <= 0.6.3 Version > 0.6.3
Create static query without `Name`The ACL Token used to create the prepared query is checked to make sure it can access the service being queried. This token is captured as the `Token` to use when executing the prepared query.No ACL policies are used as long as no `Name` is defined. No `Token` is captured by default unless specifically supplied by the client when creating the query.
Create static query with `Name`The ACL Token used to create the prepared query is checked to make sure it can access the service being queried. This token is captured as the `Token` to use when executing the prepared query.The client token's `query` ACL policy is used to determine if the client is allowed to register a query for the given `Name`. No `Token` is captured by default unless specifically supplied by the client when creating the query.
Manage static query without `Name`The ACL Token used to create the query, or a management token must be supplied in order to perform these operations.Any client with the ID of the query can perform these operations.
Manage static query with a `Name`The ACL token used to create the query, or a management token must be supplied in order to perform these operations.Similar to create, the client token's `query` ACL policy is used to determine if these operations are allowed.
List queriesA management token is required to list any queries.The client token's `query` ACL policy is used to determine which queries they can see. Only management tokens can see prepared queries without `Name`.
Execute querySince a `Token` is always captured when a query is created, that is used to check access to the service being queried. Any token supplied by the client is ignored.The captured token, client's token, or anonymous token is used to filter the results, as described above.
diff --git a/website/source/docs/upgrade-specific.html.markdown b/website/source/docs/upgrade-specific.html.markdown index 14b27e190e65..51088f9c2002 100644 --- a/website/source/docs/upgrade-specific.html.markdown +++ b/website/source/docs/upgrade-specific.html.markdown @@ -14,6 +14,20 @@ details provided for their upgrades as a result of new features or changed behavior. This page is used to document those details separately from the standard upgrade flow. +## Consul 0.6.4 + +Consul 0.6.4 made some substantial changes to how ACLs work with prepared +queries. Existing queries will execute with no changes, but there are important +differences to understand about how prepared queries are managed before you +upgrade. In particular, prepared queries with no `Name` defined will no longer +require any ACL to manage them, and prepared queries with a `Name` defined are +now governed by a new `prepared_query` ACL policy that will need to be configured +after the upgrade. + +See the [Prepared Query ACLs](/docs/internals/acl.html#prepared_query_acls) +internals guide for more details about the new behavior and how it compares to +previous versions of Consul. + ## Consul 0.6 Consul version 0.6 is a very large release with many enhancements and