From 34da7ccd64d2cbc4eed9d0d6a47be1d6dc190c85 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Mon, 12 Dec 2016 22:09:35 -0800 Subject: [PATCH 01/10] Adds a unit test to make sure the status endpoint doesn't ever show anything with "token" in the name. --- command/agent/agent_endpoint_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/command/agent/agent_endpoint_test.go b/command/agent/agent_endpoint_test.go index 522a508d13..d9db96210e 100644 --- a/command/agent/agent_endpoint_test.go +++ b/command/agent/agent_endpoint_test.go @@ -1,6 +1,7 @@ package agent import ( + "bytes" "errors" "fmt" "io" @@ -117,6 +118,15 @@ func TestHTTPAgentSelf(t *testing.T) { if val.Coord != nil { t.Fatalf("should have been nil: %v", val.Coord) } + + // Make sure there's nothing called "token" that's leaked. + raw, err := srv.marshalJSON(req, obj) + if err != nil { + t.Fatalf("err: %v", err) + } + if bytes.Contains(bytes.ToLower(raw), []byte("token")) { + t.Fatalf("bad: %s", raw) + } } func TestHTTPAgentReload(t *testing.T) { From 022baeea13d5e531884aad5ad0a7dc18083d0173 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Mon, 12 Dec 2016 23:05:11 -0800 Subject: [PATCH 02/10] Adds support to the ACL package for agent policies. --- acl/acl.go | 63 ++++++++++++++++++++++++++++ acl/acl_test.go | 101 +++++++++++++++++++++++++++++++++++++++++++++ acl/policy.go | 19 +++++++++ acl/policy_test.go | 35 ++++++++++++++++ 4 files changed, 218 insertions(+) diff --git a/acl/acl.go b/acl/acl.go index 46288a9d7d..3ade9d4055 100644 --- a/acl/acl.go +++ b/acl/acl.go @@ -41,6 +41,14 @@ type ACL interface { // ACLModify checks for permission to manipulate ACLs ACLModify() bool + // AgentRead checks for permission to read from agent endpoints for a + // given node. + AgentRead(string) bool + + // AgentWrite checks for permission to make changes via agent endpoints + // for a given node. + AgentWrite(string) bool + // EventRead determines if a specific event can be queried. EventRead(string) bool @@ -122,6 +130,14 @@ func (s *StaticACL) ACLModify() bool { return s.allowManage } +func (s *StaticACL) AgentRead(string) bool { + return s.defaultAllow +} + +func (s *StaticACL) AgentWrite(string) bool { + return s.defaultAllow +} + func (s *StaticACL) EventRead(string) bool { return s.defaultAllow } @@ -230,6 +246,9 @@ type PolicyACL struct { // no matching rule. parent ACL + // agentRules contains the agent policies + agentRules *radix.Tree + // keyRules contains the key policies keyRules *radix.Tree @@ -262,6 +281,7 @@ type PolicyACL struct { func New(parent ACL, policy *Policy) (*PolicyACL, error) { p := &PolicyACL{ parent: parent, + agentRules: radix.New(), keyRules: radix.New(), nodeRules: radix.New(), serviceRules: radix.New(), @@ -270,6 +290,11 @@ func New(parent ACL, policy *Policy) (*PolicyACL, error) { preparedQueryRules: radix.New(), } + // Load the agent policy + for _, ap := range policy.Agents { + p.agentRules.Insert(ap.Node, ap.Policy) + } + // Load the key policy for _, kp := range policy.Keys { p.keyRules.Insert(kp.Prefix, kp.Policy) @@ -319,6 +344,44 @@ func (p *PolicyACL) ACLModify() bool { return p.parent.ACLModify() } +// AgentRead checks for permission to read from agent endpoints for a given +// node. +func (p *PolicyACL) AgentRead(node string) bool { + // Check for an exact rule or catch-all + _, rule, ok := p.agentRules.LongestPrefix(node) + + if ok { + switch rule { + case PolicyRead, PolicyWrite: + return true + default: + return false + } + } + + // No matching rule, use the parent. + return p.parent.AgentRead(node) +} + +// AgentWrite checks for permission to make changes via agent endpoints for a +// given node. +func (p *PolicyACL) AgentWrite(node string) bool { + // Check for an exact rule or catch-all + _, rule, ok := p.agentRules.LongestPrefix(node) + + if ok { + switch rule { + case PolicyWrite: + return true + default: + return false + } + } + + // No matching rule, use the parent. + return p.parent.AgentWrite(node) +} + // Snapshot checks if taking and restoring snapshots is allowed. func (p *PolicyACL) Snapshot() bool { return p.parent.Snapshot() diff --git a/acl/acl_test.go b/acl/acl_test.go index 197f9390cf..b008846fc7 100644 --- a/acl/acl_test.go +++ b/acl/acl_test.go @@ -41,6 +41,12 @@ func TestStaticACL(t *testing.T) { if all.ACLModify() { t.Fatalf("should not allow") } + if !all.AgentRead("foobar") { + t.Fatalf("should allow") + } + if !all.AgentWrite("foobar") { + t.Fatalf("should allow") + } if !all.EventRead("foobar") { t.Fatalf("should allow") } @@ -99,6 +105,12 @@ func TestStaticACL(t *testing.T) { if none.ACLModify() { t.Fatalf("should not allow") } + if none.AgentRead("foobar") { + t.Fatalf("should not allow") + } + if none.AgentWrite("foobar") { + t.Fatalf("should not allow") + } if none.EventRead("foobar") { t.Fatalf("should not allow") } @@ -163,6 +175,12 @@ func TestStaticACL(t *testing.T) { if !manage.ACLModify() { t.Fatalf("should allow") } + if !manage.AgentRead("foobar") { + t.Fatalf("should allow") + } + if !manage.AgentWrite("foobar") { + t.Fatalf("should allow") + } if !manage.EventRead("foobar") { t.Fatalf("should allow") } @@ -545,6 +563,89 @@ func TestPolicyACL_Parent(t *testing.T) { } } +func TestPolicyACL_Agent(t *testing.T) { + deny := DenyAll() + policyRoot := &Policy{ + Agents: []*AgentPolicy{ + &AgentPolicy{ + Node: "root-nope", + Policy: PolicyDeny, + }, + &AgentPolicy{ + Node: "root-ro", + Policy: PolicyRead, + }, + &AgentPolicy{ + Node: "root-rw", + Policy: PolicyWrite, + }, + &AgentPolicy{ + Node: "override", + Policy: PolicyDeny, + }, + }, + } + root, err := New(deny, policyRoot) + if err != nil { + t.Fatalf("err: %v", err) + } + + policy := &Policy{ + Agents: []*AgentPolicy{ + &AgentPolicy{ + Node: "child-nope", + Policy: PolicyDeny, + }, + &AgentPolicy{ + Node: "child-ro", + Policy: PolicyRead, + }, + &AgentPolicy{ + Node: "child-rw", + Policy: PolicyWrite, + }, + &AgentPolicy{ + Node: "override", + Policy: PolicyWrite, + }, + }, + } + acl, err := New(root, policy) + if err != nil { + t.Fatalf("err: %v", err) + } + + type agentcase struct { + inp string + read bool + write bool + } + cases := []agentcase{ + {"nope", false, false}, + {"root-nope", false, false}, + {"root-ro", true, false}, + {"root-rw", true, true}, + {"root-nope-prefix", false, false}, + {"root-ro-prefix", true, false}, + {"root-rw-prefix", true, true}, + {"child-nope", false, false}, + {"child-ro", true, false}, + {"child-rw", true, true}, + {"child-nope-prefix", false, false}, + {"child-ro-prefix", true, false}, + {"child-rw-prefix", true, true}, + {"override", true, true}, + } + for _, c := range cases { + if c.read != acl.AgentRead(c.inp) { + t.Fatalf("Read fail: %#v", c) + } + if c.write != acl.AgentWrite(c.inp) { + t.Fatalf("Write fail: %#v", c) + } + } +} + func TestPolicyACL_Keyring(t *testing.T) { type keyringcase struct { inp string diff --git a/acl/policy.go b/acl/policy.go index 13d0d99124..f7781b81e7 100644 --- a/acl/policy.go +++ b/acl/policy.go @@ -16,6 +16,7 @@ const ( // an ACL configuration. type Policy struct { ID string `hcl:"-"` + Agents []*AgentPolicy `hcl:"agent,expand"` Keys []*KeyPolicy `hcl:"key,expand"` Nodes []*NodePolicy `hcl:"node,expand"` Services []*ServicePolicy `hcl:"service,expand"` @@ -26,6 +27,17 @@ type Policy struct { Operator string `hcl:"operator"` } +// AgentPolicy represents a policy for working with agent endpoints on nodes +// with specific name prefixes. +type AgentPolicy struct { + Node string `hcl:",key"` + Policy string +} + +func (a *AgentPolicy) GoString() string { + return fmt.Sprintf("%#v", *a) +} + // KeyPolicy represents a policy for a key type KeyPolicy struct { Prefix string `hcl:",key"` @@ -116,6 +128,13 @@ func Parse(rules string) (*Policy, error) { return nil, fmt.Errorf("Failed to parse ACL rules: %v", err) } + // Validate the agent policy + for _, ap := range p.Agents { + if !isPolicyValid(ap.Policy) { + return nil, fmt.Errorf("Invalid agent policy: %#v", ap) + } + } + // Validate the key policy for _, kp := range p.Keys { if !isPolicyValid(kp.Policy) { diff --git a/acl/policy_test.go b/acl/policy_test.go index 9120049168..d7b8138e9f 100644 --- a/acl/policy_test.go +++ b/acl/policy_test.go @@ -8,6 +8,12 @@ import ( func TestACLPolicy_Parse_HCL(t *testing.T) { inp := ` +agent "foo" { + policy = "read" +} +agent "bar" { + policy = "write" +} event "" { policy = "read" } @@ -63,6 +69,16 @@ query "bar" { } ` exp := &Policy{ + Agents: []*AgentPolicy{ + &AgentPolicy{ + Node: "foo", + Policy: PolicyRead, + }, + &AgentPolicy{ + Node: "bar", + Policy: PolicyWrite, + }, + }, Events: []*EventPolicy{ &EventPolicy{ Event: "", @@ -159,6 +175,14 @@ query "bar" { func TestACLPolicy_Parse_JSON(t *testing.T) { inp := `{ + "agent": { + "foo": { + "policy": "write" + }, + "bar": { + "policy": "deny" + } + }, "event": { "": { "policy": "read" @@ -226,6 +250,16 @@ func TestACLPolicy_Parse_JSON(t *testing.T) { } }` exp := &Policy{ + Agents: []*AgentPolicy{ + &AgentPolicy{ + Node: "foo", + Policy: PolicyWrite, + }, + &AgentPolicy{ + Node: "bar", + Policy: PolicyDeny, + }, + }, Events: []*EventPolicy{ &EventPolicy{ Event: "", @@ -358,6 +392,7 @@ operator = "" func TestACLPolicy_Bad_Policy(t *testing.T) { cases := []string{ + `agent "" { policy = "nope" }`, `event "" { policy = "nope" }`, `key "" { policy = "nope" }`, `keyring = "nope"`, From ca7a243b70b76e22b295f2aabd70d66e49c422ba Mon Sep 17 00:00:00 2001 From: James Phillips Date: Tue, 13 Dec 2016 23:21:14 -0800 Subject: [PATCH 03/10] Adds ACL management support to the agent. --- command/agent/acl.go | 251 ++++++++++ command/agent/acl_endpoint.go | 4 +- command/agent/acl_test.go | 474 ++++++++++++++++++ command/agent/agent.go | 11 +- command/agent/agent_test.go | 1 + command/agent/config.go | 15 + command/agent/config_test.go | 17 +- command/agent/http.go | 14 +- command/agent/local.go | 3 - consul/acl.go | 21 +- .../source/docs/agent/options.html.markdown | 9 +- 11 files changed, 791 insertions(+), 29 deletions(-) create mode 100644 command/agent/acl.go create mode 100644 command/agent/acl_test.go diff --git a/command/agent/acl.go b/command/agent/acl.go new file mode 100644 index 0000000000..06d594667e --- /dev/null +++ b/command/agent/acl.go @@ -0,0 +1,251 @@ +package agent + +import ( + "errors" + "fmt" + "strings" + "sync" + "time" + + "github.com/armon/go-metrics" + "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/consul/structs" + "github.com/hashicorp/golang-lru" +) + +// There's enough behavior difference with client-side ACLs that we've +// intentionally kept this code separate from the server-side ACL code in +// consul/acl.go. We may refactor some of the caching logic in the future, +// but for now we are developing this separately to see how things shake out. + +// These must be kept in sync with the constants in consul/acl.go. +const ( + // aclNotFound indicates there is no matching ACL. + aclNotFound = "ACL not found" + + // rootDenied is returned when attempting to resolve a root ACL. + rootDenied = "Cannot resolve root ACL" + + // permissionDenied is returned when an ACL based rejection happens. + permissionDenied = "Permission denied" + + // aclDisabled is returned when ACL changes are not permitted since they + // are disabled. + aclDisabled = "ACL support disabled" + + // anonymousToken is the token ID we re-write to if there is no token ID + // provided. + anonymousToken = "anonymous" + + // Maximum number of cached ACL entries. + aclCacheSize = 10 * 1024 +) + +var ( + permissionDeniedErr = errors.New(permissionDenied) +) + +// aclCacheEntry is used to cache ACL tokens. +type aclCacheEntry struct { + // ACL is the cached ACL. + ACL acl.ACL + + // Expires is set based on the TTL for the ACL. + Expires time.Time + + // ETag is used as an optimization when fetching ACLs from servers to + // avoid transmitting data back when the agent has a good copy, which is + // usually the case when refreshing a TTL. + ETag string +} + +// aclManager is used by the agent to keep track of state related to ACLs, +// including caching tokens from the servers. This has some internal state that +// we don't want to dump into the agent itself. +type aclManager struct { + // acls is a cache mapping ACL tokens to compiled policies. + acls *lru.TwoQueueCache + + // master is the ACL to use when the agent master token is supplied. + // This may be nil if that option isn't set in the agent config. + master acl.ACL + + // down is the ACL to use when the servers are down. This may be nil + // which means to try and use the cached policy if there is one (or + // deny if there isn't a policy in the cache). + down acl.ACL + + // disabled is used to keep track of feedback from the servers that ACLs + // are disabled. If the manager discovers that ACLs are disabled, this + // will be set to the next time we should check to see if they have been + // enabled. This helps cut useless traffic, but allows us to turn on ACL + // support at the servers without having to restart the whole cluster. + disabled time.Time + disabledLock sync.RWMutex +} + +// newACLManager returns an ACL manager based on the given config. +func newACLManager(config *Config) (*aclManager, error) { + // Set up the cache from ID to ACL (we don't cache policies like the + // servers; only one level). + acls, err := lru.New2Q(aclCacheSize) + if err != nil { + return nil, err + } + + // If an agent master token is configured, build a policy and ACL for + // it, otherwise leave it nil. + var master acl.ACL + if len(config.ACLAgentMasterToken) > 0 { + policy := &acl.Policy{ + Agents: []*acl.AgentPolicy{ + &acl.AgentPolicy{ + Node: config.NodeName, + Policy: acl.PolicyWrite, + }, + }, + } + acl, err := acl.New(acl.DenyAll(), policy) + if err != nil { + return nil, err + } + master = acl + } + + var down acl.ACL + switch config.ACLDownPolicy { + case "allow": + down = acl.AllowAll() + case "deny": + down = acl.DenyAll() + case "extend-cache": + // Leave the down policy as nil to signal this. + default: + return nil, fmt.Errorf("invalid ACL down policy %q", config.ACLDownPolicy) + } + + // Give back a manager. + return &aclManager{ + acls: acls, + master: master, + down: down, + }, nil +} + +// isDisabled returns true if the manager has discovered that ACLs are disabled +// on the servers. +func (m *aclManager) isDisabled() bool { + m.disabledLock.RLock() + defer m.disabledLock.RUnlock() + return time.Now().Before(m.disabled) +} + +// lookupACL attempts to locate the compiled policy associated with the given +// token. The agent may be used to perform RPC calls to the servers to fetch +// policies that aren't in the cache. +func (m *aclManager) lookupACL(agent *Agent, id string) (acl.ACL, error) { + // Handle some special cases for the ID. + if len(id) == 0 { + id = anonymousToken + } else if acl.RootACL(id) != nil { + return nil, errors.New(rootDenied) + } else if m.master != nil && id == agent.config.ACLAgentMasterToken { + return m.master, nil + } + + // Try the cache first. + var cached *aclCacheEntry + if raw, ok := m.acls.Get(id); ok { + cached = raw.(*aclCacheEntry) + } + if cached != nil && time.Now().Before(cached.Expires) { + metrics.IncrCounter([]string{"consul", "acl", "cache_hit"}, 1) + return cached.ACL, nil + } else { + metrics.IncrCounter([]string{"consul", "acl", "cache_miss"}, 1) + } + + // At this point we might have a stale cached ACL, or none at all, so + // try to contact the servers. + args := structs.ACLPolicyRequest{ + Datacenter: agent.config.Datacenter, + ACL: id, + } + if cached != nil { + args.ETag = cached.ETag + } + var reply structs.ACLPolicy + err := agent.RPC(agent.getEndpoint("ACL")+".GetPolicy", &args, &reply) + if err != nil { + if strings.Contains(err.Error(), aclDisabled) { + agent.logger.Printf("[DEBUG] agent: ACLs disabled on servers, will check again after %s", agent.config.ACLDisabledTTL) + m.disabledLock.Lock() + m.disabled = time.Now().Add(agent.config.ACLDisabledTTL) + m.disabledLock.Unlock() + return nil, nil + } else if strings.Contains(err.Error(), aclNotFound) { + return nil, errors.New(aclNotFound) + } else { + agent.logger.Printf("[DEBUG] agent: Failed to get policy for ACL from servers: %v", err) + if m.down != nil { + return m.down, nil + } else if cached != nil { + return cached.ACL, nil + } else { + return acl.DenyAll(), nil + } + } + } + + // Use the old cached compiled ACL if we can, otherwise compile it and + // resolve any parents. + var compiled acl.ACL + if cached != nil && cached.ETag == reply.ETag { + compiled = cached.ACL + } else { + parent := acl.RootACL(reply.Parent) + if parent == nil { + parent, err = m.lookupACL(agent, reply.Parent) + if err != nil { + return nil, err + } + } + + acl, err := acl.New(parent, reply.Policy) + if err != nil { + return nil, err + } + compiled = acl + } + + // Update the cache. + cached = &aclCacheEntry{ + ACL: compiled, + ETag: reply.ETag, + } + if reply.TTL > 0 { + cached.Expires = time.Now().Add(reply.TTL) + } + m.acls.Add(id, cached) + return compiled, nil +} + +// resolveToken is the primary interface used by ACL-checkers in the agent +// endpoints, which is the one place where we do some ACL enforcement on +// clients. Some of the enforcement is normative (e.g. self and monitor) +// and some is informative (e.g. catalog and health). +func (a *Agent) resolveToken(id string) (acl.ACL, error) { + // Disable ACLs if version 8 enforcement isn't enabled. + if !(*a.config.ACLEnforceVersion8) { + return nil, nil + } + + // Bail if the ACL manager is disabled. This happens if it gets feedback + // from the servers that ACLs are disabled. + if a.acls.isDisabled() { + return nil, nil + } + + // This will look in the cache and fetch from the servers if necessary. + return a.acls.lookupACL(a, id) +} diff --git a/command/agent/acl_endpoint.go b/command/agent/acl_endpoint.go index b60502ce99..e97189e2c7 100644 --- a/command/agent/acl_endpoint.go +++ b/command/agent/acl_endpoint.go @@ -13,8 +13,8 @@ type aclCreateResponse struct { ID string } -// aclDisabled handles if ACL datacenter is not configured -func aclDisabled(resp http.ResponseWriter, req *http.Request) (interface{}, error) { +// ACLDisabled handles if ACL datacenter is not configured +func ACLDisabled(resp http.ResponseWriter, req *http.Request) (interface{}, error) { resp.WriteHeader(401) resp.Write([]byte("ACL support disabled")) return nil, nil diff --git a/command/agent/acl_test.go b/command/agent/acl_test.go new file mode 100644 index 0000000000..0f965edca6 --- /dev/null +++ b/command/agent/acl_test.go @@ -0,0 +1,474 @@ +package agent + +import ( + "errors" + "fmt" + "io/ioutil" + "os" + "strings" + "testing" + "time" + + rawacl "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/consul/structs" + "github.com/hashicorp/consul/testutil" +) + +func TestACL_Bad_Config(t *testing.T) { + config := nextConfig() + config.ACLDownPolicy = "nope" + + var err error + config.DataDir, err = ioutil.TempDir("", "agent") + if err != nil { + t.Fatalf("err: %v", err) + } + defer os.RemoveAll(config.DataDir) + + _, err = Create(config, nil, nil, nil) + if err == nil || !strings.Contains(err.Error(), "invalid ACL down policy") { + t.Fatalf("err: %v", err) + } +} + +type MockServer struct { + getPolicyFn func(*structs.ACLPolicyRequest, *structs.ACLPolicy) error +} + +func (m *MockServer) GetPolicy(args *structs.ACLPolicyRequest, reply *structs.ACLPolicy) error { + if m.getPolicyFn != nil { + return m.getPolicyFn(args, reply) + } else { + return fmt.Errorf("should not have called GetPolicy") + } +} + +func TestACL_Version8(t *testing.T) { + config := nextConfig() + config.ACLEnforceVersion8 = Bool(false) + + dir, agent := makeAgent(t, config) + defer os.RemoveAll(dir) + defer agent.Shutdown() + + testutil.WaitForLeader(t, agent.RPC, "dc1") + + m := MockServer{} + if err := agent.InjectEndpoint("ACL", &m); err != nil { + t.Fatalf("err: %v", err) + } + + // With version 8 enforcement off, this should not get called. + m.getPolicyFn = func(*structs.ACLPolicyRequest, *structs.ACLPolicy) error { + t.Fatalf("should not have called to server") + return nil + } + if token, err := agent.resolveToken("nope"); token != nil || err != nil { + t.Fatalf("bad: %v err: %v", token, err) + } +} + +func TestACL_Disabled(t *testing.T) { + config := nextConfig() + config.ACLDisabledTTL = 10 * time.Millisecond + config.ACLEnforceVersion8 = Bool(true) + + dir, agent := makeAgent(t, config) + defer os.RemoveAll(dir) + defer agent.Shutdown() + + testutil.WaitForLeader(t, agent.RPC, "dc1") + + m := MockServer{} + if err := agent.InjectEndpoint("ACL", &m); err != nil { + t.Fatalf("err: %v", err) + } + + // Fetch a token without ACLs enabled and make sure the manager sees it. + m.getPolicyFn = func(*structs.ACLPolicyRequest, *structs.ACLPolicy) error { + return errors.New(aclDisabled) + } + if agent.acls.isDisabled() { + t.Fatalf("should not be disabled yet") + } + if token, err := agent.resolveToken("nope"); token != nil || err != nil { + t.Fatalf("bad: %v err: %v", token, err) + } + if !agent.acls.isDisabled() { + t.Fatalf("should be disabled") + } + + // Now turn on ACLs and check right away, it should still think ACLs are + // disabled since we don't check again right away. + m.getPolicyFn = func(*structs.ACLPolicyRequest, *structs.ACLPolicy) error { + return errors.New(aclNotFound) + } + if token, err := agent.resolveToken("nope"); token != nil || err != nil { + t.Fatalf("bad: %v err: %v", token, err) + } + if !agent.acls.isDisabled() { + t.Fatalf("should be disabled") + } + + // Wait the waiting period and make sure it checks again. Do a few tries + // to make sure we don't think it's disabled. + time.Sleep(2 * config.ACLDisabledTTL) + for i := 0; i < 10; i++ { + _, err := agent.resolveToken("nope") + if err == nil || !strings.Contains(err.Error(), aclNotFound) { + t.Fatalf("err: %v", err) + } + if agent.acls.isDisabled() { + t.Fatalf("should not be disabled") + } + } +} + +func TestACL_Special_IDs(t *testing.T) { + config := nextConfig() + config.ACLEnforceVersion8 = Bool(true) + config.ACLAgentMasterToken = "towel" + + dir, agent := makeAgent(t, config) + defer os.RemoveAll(dir) + defer agent.Shutdown() + + testutil.WaitForLeader(t, agent.RPC, "dc1") + + m := MockServer{} + if err := agent.InjectEndpoint("ACL", &m); err != nil { + t.Fatalf("err: %v", err) + } + + // An empty ID should get mapped to the anonymous token. + m.getPolicyFn = func(req *structs.ACLPolicyRequest, reply *structs.ACLPolicy) error { + if req.ACL != "anonymous" { + t.Fatalf("bad: %#v", *req) + } + return errors.New(aclNotFound) + } + _, err := agent.resolveToken("") + if err == nil || !strings.Contains(err.Error(), aclNotFound) { + t.Fatalf("err: %v", err) + } + + // A root ACL request should get rejected and not call the server. + m.getPolicyFn = func(*structs.ACLPolicyRequest, *structs.ACLPolicy) error { + t.Fatalf("should not have called to server") + return nil + } + _, err = agent.resolveToken("deny") + if err == nil || !strings.Contains(err.Error(), rootDenied) { + t.Fatalf("err: %v", err) + } + + // The ACL master token should also not call the server, but should give + // us a working agent token. + acl, err := agent.resolveToken("towel") + if err != nil { + t.Fatalf("err: %v", err) + } + if acl == nil { + t.Fatalf("should not be nil") + } + if !acl.AgentRead(config.NodeName) { + t.Fatalf("should be able to read agent") + } + if !acl.AgentWrite(config.NodeName) { + t.Fatalf("should be able to write agent") + } +} + +func TestACL_Down_Deny(t *testing.T) { + config := nextConfig() + config.ACLDownPolicy = "deny" + config.ACLEnforceVersion8 = Bool(true) + + dir, agent := makeAgent(t, config) + defer os.RemoveAll(dir) + defer agent.Shutdown() + + testutil.WaitForLeader(t, agent.RPC, "dc1") + + m := MockServer{} + if err := agent.InjectEndpoint("ACL", &m); err != nil { + t.Fatalf("err: %v", err) + } + + // Resolve with ACLs down. + m.getPolicyFn = func(*structs.ACLPolicyRequest, *structs.ACLPolicy) error { + return fmt.Errorf("ACLs are broken") + } + acl, err := agent.resolveToken("nope") + if err != nil { + t.Fatalf("err: %v", err) + } + if acl == nil { + t.Fatalf("should not be nil") + } + if acl.AgentRead(config.NodeName) { + t.Fatalf("should deny") + } +} + +func TestACL_Down_Allow(t *testing.T) { + config := nextConfig() + config.ACLDownPolicy = "allow" + config.ACLEnforceVersion8 = Bool(true) + + dir, agent := makeAgent(t, config) + defer os.RemoveAll(dir) + defer agent.Shutdown() + + testutil.WaitForLeader(t, agent.RPC, "dc1") + + m := MockServer{} + if err := agent.InjectEndpoint("ACL", &m); err != nil { + t.Fatalf("err: %v", err) + } + + // Resolve with ACLs down. + m.getPolicyFn = func(*structs.ACLPolicyRequest, *structs.ACLPolicy) error { + return fmt.Errorf("ACLs are broken") + } + acl, err := agent.resolveToken("nope") + if err != nil { + t.Fatalf("err: %v", err) + } + if acl == nil { + t.Fatalf("should not be nil") + } + if !acl.AgentRead(config.NodeName) { + t.Fatalf("should allow") + } +} + +func TestACL_Down_Extend(t *testing.T) { + config := nextConfig() + config.ACLDownPolicy = "extend-cache" + config.ACLEnforceVersion8 = Bool(true) + + dir, agent := makeAgent(t, config) + defer os.RemoveAll(dir) + defer agent.Shutdown() + + testutil.WaitForLeader(t, agent.RPC, "dc1") + + m := MockServer{} + if err := agent.InjectEndpoint("ACL", &m); err != nil { + t.Fatalf("err: %v", err) + } + + // Populate the cache for one of the tokens. + m.getPolicyFn = func(req *structs.ACLPolicyRequest, reply *structs.ACLPolicy) error { + *reply = structs.ACLPolicy{ + Parent: "allow", + Policy: &rawacl.Policy{ + Agents: []*rawacl.AgentPolicy{ + &rawacl.AgentPolicy{ + Node: config.NodeName, + Policy: "read", + }, + }, + }, + } + return nil + } + acl, err := agent.resolveToken("yep") + if err != nil { + t.Fatalf("err: %v", err) + } + if acl == nil { + t.Fatalf("should not be nil") + } + if !acl.AgentRead(config.NodeName) { + t.Fatalf("should allow") + } + if acl.AgentWrite(config.NodeName) { + t.Fatalf("should deny") + } + + // Now take down ACLs and make sure a new token fails to resolve. + m.getPolicyFn = func(*structs.ACLPolicyRequest, *structs.ACLPolicy) error { + return fmt.Errorf("ACLs are broken") + } + acl, err = agent.resolveToken("nope") + if err != nil { + t.Fatalf("err: %v", err) + } + if acl == nil { + t.Fatalf("should not be nil") + } + if acl.AgentRead(config.NodeName) { + t.Fatalf("should deny") + } + if acl.AgentWrite(config.NodeName) { + t.Fatalf("should deny") + } + + // Read the token from the cache while ACLs are broken, which should + // extend. + acl, err = agent.resolveToken("yep") + if err != nil { + t.Fatalf("err: %v", err) + } + if acl == nil { + t.Fatalf("should not be nil") + } + if !acl.AgentRead(config.NodeName) { + t.Fatalf("should allow") + } + if acl.AgentWrite(config.NodeName) { + t.Fatalf("should deny") + } +} + +func TestACL_Cache(t *testing.T) { + config := nextConfig() + config.ACLEnforceVersion8 = Bool(true) + + dir, agent := makeAgent(t, config) + defer os.RemoveAll(dir) + defer agent.Shutdown() + + testutil.WaitForLeader(t, agent.RPC, "dc1") + + m := MockServer{} + if err := agent.InjectEndpoint("ACL", &m); err != nil { + t.Fatalf("err: %v", err) + } + + // Populate the cache for one of the tokens. + m.getPolicyFn = func(req *structs.ACLPolicyRequest, reply *structs.ACLPolicy) error { + *reply = structs.ACLPolicy{ + ETag: "hash1", + Parent: "deny", + Policy: &rawacl.Policy{ + Agents: []*rawacl.AgentPolicy{ + &rawacl.AgentPolicy{ + Node: config.NodeName, + Policy: "read", + }, + }, + }, + TTL: 10 * time.Millisecond, + } + return nil + } + acl, err := agent.resolveToken("yep") + if err != nil { + t.Fatalf("err: %v", err) + } + if acl == nil { + t.Fatalf("should not be nil") + } + if !acl.AgentRead(config.NodeName) { + t.Fatalf("should allow") + } + if acl.AgentWrite(config.NodeName) { + t.Fatalf("should deny") + } + if acl.NodeRead("nope") { + t.Fatalf("should deny") + } + + // Fetch right away and make sure it uses the cache. + m.getPolicyFn = func(*structs.ACLPolicyRequest, *structs.ACLPolicy) error { + t.Fatalf("should not have called to server") + return nil + } + acl, err = agent.resolveToken("yep") + if err != nil { + t.Fatalf("err: %v", err) + } + if acl == nil { + t.Fatalf("should not be nil") + } + if !acl.AgentRead(config.NodeName) { + t.Fatalf("should allow") + } + if acl.AgentWrite(config.NodeName) { + t.Fatalf("should deny") + } + if acl.NodeRead("nope") { + t.Fatalf("should deny") + } + + // Wait for the TTL to expire and try again. This time the token will be + // gone. + time.Sleep(20 * time.Millisecond) + m.getPolicyFn = func(req *structs.ACLPolicyRequest, reply *structs.ACLPolicy) error { + return errors.New(aclNotFound) + } + _, err = agent.resolveToken("yep") + if err == nil || !strings.Contains(err.Error(), aclNotFound) { + t.Fatalf("err: %v", err) + } + + // Page it back in with a new tag and different policy + m.getPolicyFn = func(req *structs.ACLPolicyRequest, reply *structs.ACLPolicy) error { + *reply = structs.ACLPolicy{ + ETag: "hash2", + Parent: "deny", + Policy: &rawacl.Policy{ + Agents: []*rawacl.AgentPolicy{ + &rawacl.AgentPolicy{ + Node: config.NodeName, + Policy: "write", + }, + }, + }, + TTL: 10 * time.Millisecond, + } + return nil + } + acl, err = agent.resolveToken("yep") + if err != nil { + t.Fatalf("err: %v", err) + } + if acl == nil { + t.Fatalf("should not be nil") + } + if !acl.AgentRead(config.NodeName) { + t.Fatalf("should allow") + } + if !acl.AgentWrite(config.NodeName) { + t.Fatalf("should allow") + } + if acl.NodeRead("nope") { + t.Fatalf("should deny") + } + + // Wait for the TTL to expire and try again. This will match the tag + // and not send the policy back, but we should have the old token + // behavior. + time.Sleep(20 * time.Millisecond) + var didRefresh bool + m.getPolicyFn = func(req *structs.ACLPolicyRequest, reply *structs.ACLPolicy) error { + *reply = structs.ACLPolicy{ + ETag: "hash2", + TTL: 10 * time.Millisecond, + } + didRefresh = true + return nil + } + acl, err = agent.resolveToken("yep") + if err != nil { + t.Fatalf("err: %v", err) + } + if acl == nil { + t.Fatalf("should not be nil") + } + if !acl.AgentRead(config.NodeName) { + t.Fatalf("should allow") + } + if !acl.AgentWrite(config.NodeName) { + t.Fatalf("should allow") + } + if acl.NodeRead("nope") { + t.Fatalf("should deny") + } + if !didRefresh { + t.Fatalf("should refresh") + } +} diff --git a/command/agent/agent.go b/command/agent/agent.go index 6b9bcaa50d..789768a5b7 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -74,6 +74,9 @@ type Agent struct { server *consul.Server client *consul.Client + // acls is an object that helps manage local ACL enforcement. + acls *aclManager + // state stores a local representation of the node, // services and checks. Used for anti-entropy. state localState @@ -211,11 +214,17 @@ func Create(config *Config, logOutput io.Writer, logWriter *logger.LogWriter, return nil, err } + // Initialize the ACL manager. + acls, err := newACLManager(config) + if err != nil { + return nil, err + } + agent.acls = acls + // Initialize the local state. agent.state.Init(config, agent.logger) // Setup either the client or the server. - var err error if config.Server { err = agent.setupServer() agent.state.SetIface(agent.server) diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go index 64d6d05e31..c84a5b0088 100644 --- a/command/agent/agent_test.go +++ b/command/agent/agent_test.go @@ -55,6 +55,7 @@ func nextConfig() *Config { conf.Ports.SerfWan = basePortNumber + idx + portOffsetSerfWan conf.Ports.Server = basePortNumber + idx + portOffsetServer conf.Server = true + conf.ACLEnforceVersion8 = Bool(false) conf.ACLDatacenter = "dc1" conf.ACLMasterToken = "root" diff --git a/command/agent/config.go b/command/agent/config.go index c4c4c06e29..4d95d62446 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -488,6 +488,11 @@ type Config struct { // token is not provided. If not configured the 'anonymous' token is used. ACLToken string `mapstructure:"acl_token" json:"-"` + // ACLAgentMasterToken is a special token that has full read and write + // privileges for this agent, and can be used to call agent endpoints + // when no servers are available. + ACLAgentMasterToken string `mapstructure:"acl_agent_master_token" json:"-"` + // ACLAgentToken is the default token used to make requests for the agent // itself, such as for registering itself with the catalog. If not // configured, the 'acl_token' will be used. @@ -514,9 +519,15 @@ type Config struct { // white-lists. ACLDefaultPolicy string `mapstructure:"acl_default_policy"` + // ACLDisabledTTL is used by clients to determine how long they will + // wait to check again with the servers if they discover ACLs are not + // enabled. + ACLDisabledTTL time.Duration `mapstructure:"-"` + // ACLDownPolicy is used to control the ACL interaction when we cannot // reach the ACLDatacenter and the token is not in the cache. // There are two modes: + // * allow - Allow all requests // * deny - Deny all requests // * extend-cache - Ignore the cache expiration, and allow cached // ACL's to be used to service requests. This @@ -717,6 +728,7 @@ func DefaultConfig() *Config { ACLTTL: 30 * time.Second, ACLDownPolicy: "extend-cache", ACLDefaultPolicy: "allow", + ACLDisabledTTL: 120 * time.Second, ACLEnforceVersion8: Bool(false), RetryInterval: 30 * time.Second, RetryIntervalWan: 30 * time.Second, @@ -1483,6 +1495,9 @@ func MergeConfig(a, b *Config) *Config { if b.ACLToken != "" { result.ACLToken = b.ACLToken } + if b.ACLAgentMasterToken != "" { + result.ACLAgentMasterToken = b.ACLAgentMasterToken + } if b.ACLAgentToken != "" { result.ACLAgentToken = b.ACLAgentToken } diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 58f97213b3..caef0ce6f6 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -643,7 +643,8 @@ func TestDecodeConfig(t *testing.T) { } // ACLs - input = `{"acl_token": "1234", "acl_agent_token": "5678", "acl_datacenter": "dc2", + input = `{"acl_token": "1111", "acl_agent_master_token": "2222", + "acl_agent_token": "3333", "acl_datacenter": "dc2", "acl_ttl": "60s", "acl_down_policy": "deny", "acl_default_policy": "deny", "acl_master_token": "2345", "acl_replication_token": "8675309"}` @@ -652,10 +653,13 @@ func TestDecodeConfig(t *testing.T) { t.Fatalf("err: %s", err) } - if config.ACLToken != "1234" { + if config.ACLToken != "1111" { t.Fatalf("bad: %#v", config) } - if config.ACLAgentToken != "5678" { + if config.ACLAgentMasterToken != "2222" { + t.Fatalf("bad: %#v", config) + } + if config.ACLAgentToken != "3333" { t.Fatalf("bad: %#v", config) } if config.ACLMasterToken != "2345" { @@ -1589,9 +1593,10 @@ func TestMergeConfig(t *testing.T) { ReconnectTimeoutWan: 36 * time.Hour, CheckUpdateInterval: 8 * time.Minute, CheckUpdateIntervalRaw: "8m", - ACLToken: "1234", - ACLAgentToken: "5678", - ACLMasterToken: "2345", + ACLToken: "1111", + ACLAgentMasterToken: "2222", + ACLAgentToken: "3333", + ACLMasterToken: "4444", ACLDatacenter: "dc2", ACLTTL: 15 * time.Second, ACLTTLRaw: "15s", diff --git a/command/agent/http.go b/command/agent/http.go index 8f1a1a1db0..7070706f11 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -241,13 +241,13 @@ func (s *HTTPServer) registerHandlers(enableDebug bool) { s.handleFuncMetrics("/v1/acl/list", s.wrap(s.ACLList)) s.handleFuncMetrics("/v1/acl/replication", s.wrap(s.ACLReplicationStatus)) } else { - s.handleFuncMetrics("/v1/acl/create", s.wrap(aclDisabled)) - s.handleFuncMetrics("/v1/acl/update", s.wrap(aclDisabled)) - s.handleFuncMetrics("/v1/acl/destroy/", s.wrap(aclDisabled)) - s.handleFuncMetrics("/v1/acl/info/", s.wrap(aclDisabled)) - s.handleFuncMetrics("/v1/acl/clone/", s.wrap(aclDisabled)) - s.handleFuncMetrics("/v1/acl/list", s.wrap(aclDisabled)) - s.handleFuncMetrics("/v1/acl/replication", s.wrap(aclDisabled)) + s.handleFuncMetrics("/v1/acl/create", s.wrap(ACLDisabled)) + s.handleFuncMetrics("/v1/acl/update", s.wrap(ACLDisabled)) + s.handleFuncMetrics("/v1/acl/destroy/", s.wrap(ACLDisabled)) + s.handleFuncMetrics("/v1/acl/info/", s.wrap(ACLDisabled)) + s.handleFuncMetrics("/v1/acl/clone/", s.wrap(ACLDisabled)) + s.handleFuncMetrics("/v1/acl/list", s.wrap(ACLDisabled)) + s.handleFuncMetrics("/v1/acl/replication", s.wrap(ACLDisabled)) } s.handleFuncMetrics("/v1/agent/self", s.wrap(s.AgentSelf)) s.handleFuncMetrics("/v1/agent/maintenance", s.wrap(s.AgentNodeMaintenance)) diff --git a/command/agent/local.go b/command/agent/local.go index c324a15361..fdd3d37889 100644 --- a/command/agent/local.go +++ b/command/agent/local.go @@ -18,9 +18,6 @@ import ( const ( syncStaggerIntv = 3 * time.Second syncRetryIntv = 15 * time.Second - - // permissionDenied is returned when an ACL based rejection happens - permissionDenied = "Permission denied" ) // syncStatus is used to represent the difference between diff --git a/consul/acl.go b/consul/acl.go index 36135d9ac6..50b7b675c1 100644 --- a/consul/acl.go +++ b/consul/acl.go @@ -14,29 +14,30 @@ import ( "github.com/hashicorp/golang-lru" ) +// These must be kept in sync with the constants in command/agent/acl.go. const ( - // aclNotFound indicates there is no matching ACL + // aclNotFound indicates there is no matching ACL. aclNotFound = "ACL not found" - // rootDenied is returned when attempting to resolve a root ACL + // rootDenied is returned when attempting to resolve a root ACL. rootDenied = "Cannot resolve root ACL" - // permissionDenied is returned when an ACL based rejection happens + // permissionDenied is returned when an ACL based rejection happens. permissionDenied = "Permission denied" - // aclDisabled is returned when ACL changes are not permitted - // since they are disabled. + // aclDisabled is returned when ACL changes are not permitted since they + // are disabled. aclDisabled = "ACL support disabled" - // anonymousToken is the token ID we re-write to if there - // is no token ID provided + // anonymousToken is the token ID we re-write to if there is no token ID + // provided. anonymousToken = "anonymous" // redactedToken is shown in structures with embedded tokens when they - // are not allowed to be displayed + // are not allowed to be displayed. redactedToken = "" - // Maximum number of cached ACL entries + // Maximum number of cached ACL entries. aclCacheSize = 10 * 1024 ) @@ -264,6 +265,8 @@ func (c *aclCache) useACLPolicy(id, authDC string, cached *aclCacheEntry, p *str // Check if we can used the cached policy if cached != nil && cached.ETag == p.ETag { if p.TTL > 0 { + // TODO (slackpad) - This seems like it's an unsafe + // write. cached.Expires = time.Now().Add(p.TTL) } return cached.ACL, nil diff --git a/website/source/docs/agent/options.html.markdown b/website/source/docs/agent/options.html.markdown index 59118d9401..357f4a9589 100644 --- a/website/source/docs/agent/options.html.markdown +++ b/website/source/docs/agent/options.html.markdown @@ -377,7 +377,14 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass all operations, and "extend-cache" allows any cached ACLs to be used, ignoring their TTL values. If a non-cached ACL is used, "extend-cache" acts like "deny". -* `acl_agent_token` - Used for clients +* `acl_agent_master_token` - + Used to access agent endpoints that require agent read + or write privileges even if Consul servers aren't present to validate any tokens. This should only + be used by operators during outages, regular ACL tokens should normally be used by applications. + This was added in Consul 0.7.2 and is only used when `acl_enforce_version_8` + is set to true. + +* `acl_agent_token` - Used for clients and servers to perform internal operations to the service catalog. If this isn't specified, then the `acl_token` will be used. This was added in Consul 0.7.2.

From 01b6766099963c55be8208340ca229be4fcf35ac Mon Sep 17 00:00:00 2001 From: James Phillips Date: Wed, 14 Dec 2016 09:33:57 -0800 Subject: [PATCH 04/10] Adds complete ACL support for agent utility endpoints. --- command/agent/agent_endpoint.go | 102 ++++++-- command/agent/agent_endpoint_test.go | 345 ++++++++++++++++++++++++--- command/agent/http_test.go | 11 + 3 files changed, 406 insertions(+), 52 deletions(-) diff --git a/command/agent/agent_endpoint.go b/command/agent/agent_endpoint.go index 0067d1b356..cbb89f3272 100644 --- a/command/agent/agent_endpoint.go +++ b/command/agent/agent_endpoint.go @@ -31,6 +31,17 @@ func (s *HTTPServer) AgentSelf(resp http.ResponseWriter, req *http.Request) (int } } + // Fetch the ACL token, if any, and enforce agent policy. + var token string + s.parseToken(req, &token) + acl, err := s.agent.resolveToken(token) + if err != nil { + return nil, err + } + if acl != nil && !acl.AgentRead(s.agent.config.NodeName) { + return nil, permissionDeniedErr + } + return AgentSelf{ Config: s.agent.config, Coord: c, @@ -45,9 +56,19 @@ func (s *HTTPServer) AgentReload(resp http.ResponseWriter, req *http.Request) (i return nil, nil } - errCh := make(chan error, 0) + // Fetch the ACL token, if any, and enforce agent policy. + var token string + s.parseToken(req, &token) + acl, err := s.agent.resolveToken(token) + if err != nil { + return nil, err + } + if acl != nil && !acl.AgentWrite(s.agent.config.NodeName) { + return nil, permissionDeniedErr + } // Trigger the reload + errCh := make(chan error, 0) select { case <-s.agent.ShutdownCh(): return nil, fmt.Errorf("Agent was shutdown before reload could be completed") @@ -87,6 +108,17 @@ func (s *HTTPServer) AgentMembers(resp http.ResponseWriter, req *http.Request) ( } func (s *HTTPServer) AgentJoin(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + // Fetch the ACL token, if any, and enforce agent policy. + var token string + s.parseToken(req, &token) + acl, err := s.agent.resolveToken(token) + if err != nil { + return nil, err + } + if acl != nil && !acl.AgentWrite(s.agent.config.NodeName) { + return nil, permissionDeniedErr + } + // Check if the WAN is being queried wan := false if other := req.URL.Query().Get("wan"); other != "" { @@ -110,6 +142,17 @@ func (s *HTTPServer) AgentLeave(resp http.ResponseWriter, req *http.Request) (in return nil, nil } + // Fetch the ACL token, if any, and enforce agent policy. + var token string + s.parseToken(req, &token) + acl, err := s.agent.resolveToken(token) + if err != nil { + return nil, err + } + if acl != nil && !acl.AgentWrite(s.agent.config.NodeName) { + return nil, permissionDeniedErr + } + if err := s.agent.Leave(); err != nil { return nil, err } @@ -117,10 +160,30 @@ func (s *HTTPServer) AgentLeave(resp http.ResponseWriter, req *http.Request) (in } func (s *HTTPServer) AgentForceLeave(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + // Fetch the ACL token, if any, and enforce agent policy. + var token string + s.parseToken(req, &token) + acl, err := s.agent.resolveToken(token) + if err != nil { + return nil, err + } + if acl != nil && !acl.AgentWrite(s.agent.config.NodeName) { + return nil, permissionDeniedErr + } + addr := strings.TrimPrefix(req.URL.Path, "/v1/agent/force-leave/") return nil, s.agent.ForceLeave(addr) } +// syncChanges is a helper function which wraps a blocking call to sync +// services and checks to the server. If the operation fails, we only +// only warn because the write did succeed and anti-entropy will sync later. +func (s *HTTPServer) syncChanges() { + if err := s.agent.state.syncChanges(); err != nil { + s.logger.Printf("[ERR] agent: failed to sync changes: %v", err) + } +} + const invalidCheckMessage = "Must provide TTL or Script/DockerContainerID/HTTP/TCP and Interval" func (s *HTTPServer) AgentRegisterCheck(resp http.ResponseWriter, req *http.Request) (interface{}, error) { @@ -433,31 +496,33 @@ func (s *HTTPServer) AgentNodeMaintenance(resp http.ResponseWriter, req *http.Re } func (s *HTTPServer) AgentMonitor(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - // Only GET supported + // Only GET supported. if req.Method != "GET" { resp.WriteHeader(405) return nil, nil } - var args structs.DCSpecificRequest - args.Datacenter = s.agent.config.Datacenter - s.parseToken(req, &args.Token) - // Validate that the given token has operator permissions - var reply structs.RaftConfigurationResponse - if err := s.agent.RPC("Operator.RaftGetConfiguration", &args, &reply); err != nil { + // Fetch the ACL token, if any, and enforce agent policy. + var token string + s.parseToken(req, &token) + acl, err := s.agent.resolveToken(token) + if err != nil { return nil, err } + if acl != nil && !acl.AgentRead(s.agent.config.NodeName) { + return nil, permissionDeniedErr + } - // Get the provided loglevel + // Get the provided loglevel. logLevel := req.URL.Query().Get("loglevel") if logLevel == "" { logLevel = "INFO" } - // Upper case the log level + // Upper case the level since that's required by the filter. logLevel = strings.ToUpper(logLevel) - // Create a level filter + // Create a level filter and flusher. filter := logger.LevelFilter() filter.MinLevel = logutils.LogLevel(logLevel) if !logger.ValidateLevelFilter(filter.MinLevel, filter) { @@ -465,13 +530,12 @@ func (s *HTTPServer) AgentMonitor(resp http.ResponseWriter, req *http.Request) ( resp.Write([]byte(fmt.Sprintf("Unknown log level: %s", filter.MinLevel))) return nil, nil } - flusher, ok := resp.(http.Flusher) if !ok { return nil, fmt.Errorf("Streaming not supported") } - // Set up a log handler + // Set up a log handler. handler := &httpLogHandler{ filter: filter, logCh: make(chan string, 512), @@ -479,10 +543,9 @@ func (s *HTTPServer) AgentMonitor(resp http.ResponseWriter, req *http.Request) ( } s.agent.logWriter.RegisterHandler(handler) defer s.agent.logWriter.DeregisterHandler(handler) - notify := resp.(http.CloseNotifier).CloseNotify() - // Stream logs until the connection is closed + // Stream logs until the connection is closed. for { select { case <-notify: @@ -500,15 +563,6 @@ func (s *HTTPServer) AgentMonitor(resp http.ResponseWriter, req *http.Request) ( return nil, nil } -// syncChanges is a helper function which wraps a blocking call to sync -// services and checks to the server. If the operation fails, we only -// only warn because the write did succeed and anti-entropy will sync later. -func (s *HTTPServer) syncChanges() { - if err := s.agent.state.syncChanges(); err != nil { - s.logger.Printf("[ERR] agent: failed to sync changes: %v", err) - } -} - type httpLogHandler struct { filter *logutils.LevelFilter logCh chan string diff --git a/command/agent/agent_endpoint_test.go b/command/agent/agent_endpoint_test.go index d9db96210e..4195a294d6 100644 --- a/command/agent/agent_endpoint_test.go +++ b/command/agent/agent_endpoint_test.go @@ -2,6 +2,7 @@ package agent import ( "bytes" + "encoding/json" "errors" "fmt" "io" @@ -22,7 +23,30 @@ import ( "github.com/mitchellh/cli" ) -func TestHTTPAgentServices(t *testing.T) { +func makeReadOnlyAgentACL(t *testing.T, srv *HTTPServer) string { + body := bytes.NewBuffer(nil) + enc := json.NewEncoder(body) + raw := map[string]interface{}{ + "Name": "User Token", + "Type": "client", + "Rules": fmt.Sprintf(`agent "" { policy = "read" }`), + } + enc.Encode(raw) + + req, err := http.NewRequest("PUT", "/v1/acl/create?token=root", body) + if err != nil { + t.Fatalf("err: %v", err) + } + resp := httptest.NewRecorder() + obj, err := srv.ACLCreate(resp, req) + if err != nil { + t.Fatalf("err: %v", err) + } + aclResp := obj.(aclCreateResponse) + return aclResp.ID +} + +func TestAgent_Services(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -49,7 +73,7 @@ func TestHTTPAgentServices(t *testing.T) { } } -func TestHTTPAgentChecks(t *testing.T) { +func TestAgent_Checks(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -76,7 +100,7 @@ func TestHTTPAgentChecks(t *testing.T) { } } -func TestHTTPAgentSelf(t *testing.T) { +func TestAgent_Self(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -129,7 +153,54 @@ func TestHTTPAgentSelf(t *testing.T) { } } -func TestHTTPAgentReload(t *testing.T) { +func TestAgent_Self_ACLDeny(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + // Try without a token. + { + req, err := http.NewRequest("GET", "/v1/agent/self", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + _, err = srv.AgentSelf(nil, req) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + } + + // Try the agent master token (resolved on the agent). + { + req, err := http.NewRequest("GET", "/v1/agent/self?token=towel", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + _, err = srv.AgentSelf(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } + } + + // Try a read only token (resolved on the servers). + ro := makeReadOnlyAgentACL(t, srv) + { + req, err := http.NewRequest("GET", fmt.Sprintf("/v1/agent/self?token=%s", ro), nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + _, err = srv.AgentSelf(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } + } +} + +func TestAgent_Reload(t *testing.T) { conf := nextConfig() tmpDir, err := ioutil.TempDir("", "consul") if err != nil { @@ -204,7 +275,46 @@ func TestHTTPAgentReload(t *testing.T) { } } -func TestHTTPAgentMembers(t *testing.T) { +func TestAgent_Reload_ACLDeny(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + // Try without a token. + { + req, err := http.NewRequest("PUT", "/v1/agent/reload", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + _, err = srv.AgentReload(nil, req) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + } + + // Try with a read only token (resolved on the servers). + ro := makeReadOnlyAgentACL(t, srv) + { + req, err := http.NewRequest("PUT", fmt.Sprintf("/v1/agent/reload?token=%s", ro), nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + _, err = srv.AgentReload(nil, req) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + } + + // This proves we call the ACL function, and we've got the other reload + // test to prove we do the reload, which should be sufficient. + // The reload logic is a little complex to set up so isn't worth + // repeating again here. +} + +func TestAgent_Members(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -229,7 +339,7 @@ func TestHTTPAgentMembers(t *testing.T) { } } -func TestHTTPAgentMembers_WAN(t *testing.T) { +func TestAgent_Members_WAN(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -254,7 +364,7 @@ func TestHTTPAgentMembers_WAN(t *testing.T) { } } -func TestHTTPAgentJoin(t *testing.T) { +func TestAgent_Join(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -289,7 +399,7 @@ func TestHTTPAgentJoin(t *testing.T) { }) } -func TestHTTPAgentJoin_WAN(t *testing.T) { +func TestAgent_Join_WAN(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -324,7 +434,59 @@ func TestHTTPAgentJoin_WAN(t *testing.T) { }) } -func TestHTTPAgentLeave(t *testing.T) { +func TestAgent_Join_ACLDeny(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + dir2, a2 := makeAgent(t, nextConfig()) + defer os.RemoveAll(dir2) + defer a2.Shutdown() + addr := fmt.Sprintf("127.0.0.1:%d", a2.config.Ports.SerfLan) + + // Try without a token. + { + req, err := http.NewRequest("GET", fmt.Sprintf("/v1/agent/join/%s", addr), nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + _, err = srv.AgentJoin(nil, req) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + } + + // Try the agent master token (resolved on the agent). + { + req, err := http.NewRequest("GET", fmt.Sprintf("/v1/agent/join/%s?token=towel", addr), nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + _, err = srv.AgentJoin(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } + } + + // Try with a read only token (resolved on the servers). + ro := makeReadOnlyAgentACL(t, srv) + { + req, err := http.NewRequest("GET", fmt.Sprintf("/v1/agent/join/%s?token=%s", addr, ro), nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + _, err = srv.AgentJoin(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } + } +} + +func TestAgent_Leave(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -367,7 +529,64 @@ func TestHTTPAgentLeave(t *testing.T) { }) } -func TestHTTPAgentForceLeave(t *testing.T) { +func TestAgent_Leave_ACLDeny(t *testing.T) { + // Try without a token. + func() { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + req, err := http.NewRequest("PUT", "/v1/agent/leave", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + _, err = srv.AgentLeave(nil, req) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + }() + + // Try the agent master token (resolved on the agent). + func() { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + req, err := http.NewRequest("PUT", "/v1/agent/leave?token=towel", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + _, err = srv.AgentLeave(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } + }() + + // Try with a read only token (resolved on the servers). + func() { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + ro := makeReadOnlyAgentACL(t, srv) + req, err := http.NewRequest("PUT", fmt.Sprintf("/v1/agent/leave?token=%s", ro), nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + _, err = srv.AgentLeave(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } + }() +} + +func TestAgent_ForceLeave(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -409,7 +628,54 @@ func TestHTTPAgentForceLeave(t *testing.T) { }) } -func TestHTTPAgentRegisterCheck(t *testing.T) { +func TestAgent_ForceLeave_ACLDeny(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + // Try without a token. + { + req, err := http.NewRequest("GET", "/v1/agent/force-leave/nope", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + _, err = srv.AgentForceLeave(nil, req) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + } + + // Try the agent master token (resolved on the agent). + { + req, err := http.NewRequest("GET", "/v1/agent/force-leave/nope?token=towel", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + _, err = srv.AgentForceLeave(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } + } + + // Try a read only token (resolved on the servers). + ro := makeReadOnlyAgentACL(t, srv) + { + req, err := http.NewRequest("GET", fmt.Sprintf("/v1/agent/force-leave/nope?token=%s", ro), nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + _, err = srv.AgentForceLeave(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } + } +} + +func TestAgent_RegisterCheck(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -458,7 +724,7 @@ func TestHTTPAgentRegisterCheck(t *testing.T) { } } -func TestHTTPAgentRegisterCheckPassing(t *testing.T) { +func TestAgent_RegisterCheckPassing(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -502,7 +768,7 @@ func TestHTTPAgentRegisterCheckPassing(t *testing.T) { } } -func TestHTTPAgentRegisterCheckBadStatus(t *testing.T) { +func TestAgent_RegisterCheckBadStatus(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -531,7 +797,7 @@ func TestHTTPAgentRegisterCheckBadStatus(t *testing.T) { } } -func TestHTTPAgentDeregisterCheck(t *testing.T) { +func TestAgent_DeregisterCheck(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -562,7 +828,7 @@ func TestHTTPAgentDeregisterCheck(t *testing.T) { } } -func TestHTTPAgentPassCheck(t *testing.T) { +func TestAgent_PassCheck(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -594,7 +860,7 @@ func TestHTTPAgentPassCheck(t *testing.T) { } } -func TestHTTPAgentWarnCheck(t *testing.T) { +func TestAgent_WarnCheck(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -626,7 +892,7 @@ func TestHTTPAgentWarnCheck(t *testing.T) { } } -func TestHTTPAgentFailCheck(t *testing.T) { +func TestAgent_FailCheck(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -658,7 +924,7 @@ func TestHTTPAgentFailCheck(t *testing.T) { } } -func TestHTTPAgentUpdateCheck(t *testing.T) { +func TestAgent_UpdateCheck(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -786,7 +1052,7 @@ func TestHTTPAgentUpdateCheck(t *testing.T) { } } -func TestHTTPAgentRegisterService(t *testing.T) { +func TestAgent_RegisterService(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -844,7 +1110,7 @@ func TestHTTPAgentRegisterService(t *testing.T) { } } -func TestHTTPAgentDeregisterService(t *testing.T) { +func TestAgent_DeregisterService(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -882,7 +1148,7 @@ func TestHTTPAgentDeregisterService(t *testing.T) { } } -func TestHTTPAgent_ServiceMaintenanceEndpoint_BadRequest(t *testing.T) { +func TestAgent_ServiceMaintenanceEndpoint_BadRequest(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -929,7 +1195,7 @@ func TestHTTPAgent_ServiceMaintenanceEndpoint_BadRequest(t *testing.T) { } } -func TestHTTPAgent_EnableServiceMaintenance(t *testing.T) { +func TestAgent_EnableServiceMaintenance(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -972,7 +1238,7 @@ func TestHTTPAgent_EnableServiceMaintenance(t *testing.T) { } } -func TestHTTPAgent_DisableServiceMaintenance(t *testing.T) { +func TestAgent_DisableServiceMaintenance(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -1009,7 +1275,7 @@ func TestHTTPAgent_DisableServiceMaintenance(t *testing.T) { } } -func TestHTTPAgent_NodeMaintenanceEndpoint_BadRequest(t *testing.T) { +func TestAgent_NodeMaintenanceEndpoint_BadRequest(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -1036,7 +1302,7 @@ func TestHTTPAgent_NodeMaintenanceEndpoint_BadRequest(t *testing.T) { } } -func TestHTTPAgent_EnableNodeMaintenance(t *testing.T) { +func TestAgent_EnableNodeMaintenance(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -1070,7 +1336,7 @@ func TestHTTPAgent_EnableNodeMaintenance(t *testing.T) { } } -func TestHTTPAgent_DisableNodeMaintenance(t *testing.T) { +func TestAgent_DisableNodeMaintenance(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -1095,7 +1361,7 @@ func TestHTTPAgent_DisableNodeMaintenance(t *testing.T) { } } -func TestHTTPAgentRegisterServiceCheck(t *testing.T) { +func TestAgent_RegisterServiceCheck(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -1152,7 +1418,7 @@ func TestHTTPAgentRegisterServiceCheck(t *testing.T) { } } -func TestHTTPAgent_Monitor(t *testing.T) { +func TestAgent_Monitor(t *testing.T) { logWriter := logger.NewLogWriter(512) logger := io.MultiWriter(os.Stdout, logWriter) @@ -1245,3 +1511,26 @@ func newClosableRecorder() *closableRecorder { func (r *closableRecorder) CloseNotify() <-chan bool { return r.closer } + +func TestAgent_Monitor_ACLDeny(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + // Try without a token. + req, err := http.NewRequest("GET", "/v1/agent/monitor", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + _, err = srv.AgentMonitor(nil, req) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // This proves we call the ACL function, and we've got the other monitor + // test to prove monitor works, which should be sufficient. The monitor + // logic is a little complex to set up so isn't worth repeating again + // here. +} diff --git a/command/agent/http_test.go b/command/agent/http_test.go index b02b659705..846778ea98 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -32,6 +32,17 @@ func makeHTTPServerWithConfig(t *testing.T, cb func(c *Config)) (string, *HTTPSe return makeHTTPServerWithConfigLog(t, cb, nil, nil) } +func makeHTTPServerWithACLs(t *testing.T) (string, *HTTPServer) { + return makeHTTPServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = c.Datacenter + c.ACLDefaultPolicy = "deny" + c.ACLMasterToken = "root" + c.ACLAgentToken = "root" + c.ACLAgentMasterToken = "towel" + c.ACLEnforceVersion8 = Bool(true) + }) +} + func makeHTTPServerWithConfigLog(t *testing.T, cb func(c *Config), l io.Writer, logWriter *logger.LogWriter) (string, *HTTPServer) { configTry := 0 RECONF: From 03f40116f41d163a4d65a8684616612cab11a42f Mon Sep 17 00:00:00 2001 From: James Phillips Date: Wed, 14 Dec 2016 14:16:46 -0800 Subject: [PATCH 05/10] Adds complete ACL coverage for non-utility agent endpoints. This is a checkpoint - we need to complete some unit tests for agent/acl.go. --- command/agent/acl.go | 201 +++++++++++ command/agent/agent_endpoint.go | 120 ++++++- command/agent/agent_endpoint_test.go | 518 ++++++++++++++++++++++++++- 3 files changed, 807 insertions(+), 32 deletions(-) diff --git a/command/agent/acl.go b/command/agent/acl.go index 06d594667e..ee1aeb620e 100644 --- a/command/agent/acl.go +++ b/command/agent/acl.go @@ -10,7 +10,9 @@ import ( "github.com/armon/go-metrics" "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/consul/structs" + "github.com/hashicorp/consul/types" "github.com/hashicorp/golang-lru" + "github.com/hashicorp/serf/serf" ) // There's enough behavior difference with client-side ACLs that we've @@ -249,3 +251,202 @@ func (a *Agent) resolveToken(id string) (acl.ACL, error) { // This will look in the cache and fetch from the servers if necessary. return a.acls.lookupACL(a, id) } + +// vetServiceRegister makes sure the service registration action is allowed by +// the given token. +func (a *Agent) vetServiceRegister(token string, service *structs.NodeService) error { + // Resolve the token and bail if ACLs aren't enabled. + acl, err := a.resolveToken(token) + if err != nil { + return err + } + if acl == nil { + return nil + } + + // Vet the service itself. + if !acl.ServiceWrite(service.Service) { + return permissionDeniedErr + } + + // Vet any service that might be getting overwritten. + services := a.state.Services() + if existing, ok := services[service.ID]; ok { + if !acl.ServiceWrite(existing.Service) { + return permissionDeniedErr + } + } + + return nil +} + +// vetServiceUpdate makes sure the service update action is allowed by the given +// token. +func (a *Agent) vetServiceUpdate(token string, serviceID string) error { + // Resolve the token and bail if ACLs aren't enabled. + acl, err := a.resolveToken(token) + if err != nil { + return err + } + if acl == nil { + return nil + } + + // Vet any changes based on the existing services's info. + services := a.state.Services() + if existing, ok := services[serviceID]; ok { + if !acl.ServiceWrite(existing.Service) { + return permissionDeniedErr + } + } else { + return fmt.Errorf("Unknown service %q", serviceID) + } + + return nil +} + +// vetCheckRegister makes sure the check registration action is allowed by the +// given token. +func (a *Agent) vetCheckRegister(token string, check *structs.HealthCheck) error { + // Resolve the token and bail if ACLs aren't enabled. + acl, err := a.resolveToken(token) + if err != nil { + return err + } + if acl == nil { + return nil + } + + // Vet the check itself. + if len(check.ServiceName) > 0 { + if !acl.ServiceWrite(check.ServiceName) { + return permissionDeniedErr + } + } else { + if !acl.NodeWrite(a.config.NodeName) { + return permissionDeniedErr + } + } + + // Vet any check that might be getting overwritten. + checks := a.state.Checks() + if existing, ok := checks[check.CheckID]; ok { + if len(existing.ServiceName) > 0 { + if !acl.ServiceWrite(existing.ServiceName) { + return permissionDeniedErr + } + } else { + if !acl.NodeWrite(a.config.NodeName) { + return permissionDeniedErr + } + } + } + + return nil +} + +// vetCheckUpdate makes sure that a check update is allowed by the given token. +func (a *Agent) vetCheckUpdate(token string, checkID types.CheckID) error { + // Resolve the token and bail if ACLs aren't enabled. + acl, err := a.resolveToken(token) + if err != nil { + return err + } + if acl == nil { + return nil + } + + // Vet any changes based on the existing check's info. + checks := a.state.Checks() + if existing, ok := checks[checkID]; ok { + if len(existing.ServiceName) > 0 { + if !acl.ServiceWrite(existing.ServiceName) { + return permissionDeniedErr + } + } else { + if !acl.NodeWrite(a.config.NodeName) { + return permissionDeniedErr + } + } + } else { + return fmt.Errorf("Unknown check %q", checkID) + } + + return nil +} + +// filterMembers redacts members that the token doesn't have access to. +func (a *Agent) filterMembers(token string, members *[]serf.Member) error { + // Resolve the token and bail if ACLs aren't enabled. + acl, err := a.resolveToken(token) + if err != nil { + return err + } + if acl == nil { + return nil + } + + // Filter out members based on the node policy. + m := *members + for i := 0; i < len(m); i++ { + node := m[i].Name + if acl.NodeRead(node) { + continue + } + a.logger.Printf("[DEBUG] agent: dropping node %q from result due to ACLs", node) + m = append(m[:i], m[i+1:]...) + i-- + } + *members = m + return nil +} + +// filterServices redacts services that the token doesn't have access to. +func (a *Agent) filterServices(token string, services *map[string]*structs.NodeService) error { + // Resolve the token and bail if ACLs aren't enabled. + acl, err := a.resolveToken(token) + if err != nil { + return err + } + if acl == nil { + return nil + } + + // Filter out services based on the service policy. + for id, service := range *services { + if acl.ServiceRead(service.Service) { + continue + } + a.logger.Printf("[DEBUG] agent: dropping service %q from result due to ACLs", id) + delete(*services, id) + } + return nil +} + +// filterChecks redacts checks that the token doesn't have access to. +func (a *Agent) filterChecks(token string, checks *map[types.CheckID]*structs.HealthCheck) error { + // Resolve the token and bail if ACLs aren't enabled. + acl, err := a.resolveToken(token) + if err != nil { + return err + } + if acl == nil { + return nil + } + + // Filter out checks based on the node or service policy. + for id, check := range *checks { + if len(check.ServiceName) > 0 { + if acl.ServiceRead(check.ServiceName) { + continue + } + } else { + if acl.NodeRead(a.config.NodeName) { + continue + } + } + a.logger.Printf("[DEBUG] agent: dropping check %q from result due to ACLs", id) + delete(*checks, id) + } + return nil +} diff --git a/command/agent/agent_endpoint.go b/command/agent/agent_endpoint.go index cbb89f3272..e96218d864 100644 --- a/command/agent/agent_endpoint.go +++ b/command/agent/agent_endpoint.go @@ -85,26 +85,50 @@ func (s *HTTPServer) AgentReload(resp http.ResponseWriter, req *http.Request) (i } func (s *HTTPServer) AgentServices(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + // Fetch the ACL token, if any. + var token string + s.parseToken(req, &token) + services := s.agent.state.Services() + if err := s.agent.filterServices(token, &services); err != nil { + return nil, err + } return services, nil } func (s *HTTPServer) AgentChecks(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + // Fetch the ACL token, if any. + var token string + s.parseToken(req, &token) + checks := s.agent.state.Checks() + if err := s.agent.filterChecks(token, &checks); err != nil { + return nil, err + } return checks, nil } func (s *HTTPServer) AgentMembers(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + // Fetch the ACL token, if any. + var token string + s.parseToken(req, &token) + // Check if the WAN is being queried wan := false if other := req.URL.Query().Get("wan"); other != "" { wan = true } + + var members []serf.Member if wan { - return s.agent.WANMembers(), nil + members = s.agent.WANMembers() } else { - return s.agent.LANMembers(), nil + members = s.agent.LANMembers() } + if err := s.agent.filterMembers(token, &members); err != nil { + return nil, err + } + return members, nil } func (s *HTTPServer) AgentJoin(resp http.ResponseWriter, req *http.Request) (interface{}, error) { @@ -188,7 +212,7 @@ const invalidCheckMessage = "Must provide TTL or Script/DockerContainerID/HTTP/T func (s *HTTPServer) AgentRegisterCheck(resp http.ResponseWriter, req *http.Request) (interface{}, error) { var args CheckDefinition - // Fixup the type decode of TTL or Interval + // Fixup the type decode of TTL or Interval. decodeCB := func(raw interface{}) error { return FixupCheckType(raw) } @@ -198,7 +222,7 @@ func (s *HTTPServer) AgentRegisterCheck(resp http.ResponseWriter, req *http.Requ return nil, nil } - // Verify the check has a name + // Verify the check has a name. if args.Name == "" { resp.WriteHeader(400) resp.Write([]byte("Missing check name")) @@ -211,10 +235,10 @@ func (s *HTTPServer) AgentRegisterCheck(resp http.ResponseWriter, req *http.Requ return nil, nil } - // Construct the health check + // Construct the health check. health := args.HealthCheck(s.agent.config.NodeName) - // Verify the check type + // Verify the check type. chkType := &args.CheckType if !chkType.Valid() { resp.WriteHeader(400) @@ -222,11 +246,14 @@ func (s *HTTPServer) AgentRegisterCheck(resp http.ResponseWriter, req *http.Requ return nil, nil } - // Get the provided token, if any + // Get the provided token, if any, and vet against any ACL policies. var token string s.parseToken(req, &token) + if err := s.agent.vetCheckRegister(token, health); err != nil { + return nil, err + } - // Add the check + // Add the check. if err := s.agent.AddCheck(health, chkType, true, token); err != nil { return nil, err } @@ -236,6 +263,14 @@ func (s *HTTPServer) AgentRegisterCheck(resp http.ResponseWriter, req *http.Requ func (s *HTTPServer) AgentDeregisterCheck(resp http.ResponseWriter, req *http.Request) (interface{}, error) { checkID := types.CheckID(strings.TrimPrefix(req.URL.Path, "/v1/agent/check/deregister/")) + + // Get the provided token, if any, and vet against any ACL policies. + var token string + s.parseToken(req, &token) + if err := s.agent.vetCheckUpdate(token, checkID); err != nil { + return nil, err + } + if err := s.agent.RemoveCheck(checkID, true); err != nil { return nil, err } @@ -246,6 +281,14 @@ func (s *HTTPServer) AgentDeregisterCheck(resp http.ResponseWriter, req *http.Re func (s *HTTPServer) AgentCheckPass(resp http.ResponseWriter, req *http.Request) (interface{}, error) { checkID := types.CheckID(strings.TrimPrefix(req.URL.Path, "/v1/agent/check/pass/")) note := req.URL.Query().Get("note") + + // Get the provided token, if any, and vet against any ACL policies. + var token string + s.parseToken(req, &token) + if err := s.agent.vetCheckUpdate(token, checkID); err != nil { + return nil, err + } + if err := s.agent.updateTTLCheck(checkID, structs.HealthPassing, note); err != nil { return nil, err } @@ -256,6 +299,14 @@ func (s *HTTPServer) AgentCheckPass(resp http.ResponseWriter, req *http.Request) func (s *HTTPServer) AgentCheckWarn(resp http.ResponseWriter, req *http.Request) (interface{}, error) { checkID := types.CheckID(strings.TrimPrefix(req.URL.Path, "/v1/agent/check/warn/")) note := req.URL.Query().Get("note") + + // Get the provided token, if any, and vet against any ACL policies. + var token string + s.parseToken(req, &token) + if err := s.agent.vetCheckUpdate(token, checkID); err != nil { + return nil, err + } + if err := s.agent.updateTTLCheck(checkID, structs.HealthWarning, note); err != nil { return nil, err } @@ -266,6 +317,14 @@ func (s *HTTPServer) AgentCheckWarn(resp http.ResponseWriter, req *http.Request) func (s *HTTPServer) AgentCheckFail(resp http.ResponseWriter, req *http.Request) (interface{}, error) { checkID := types.CheckID(strings.TrimPrefix(req.URL.Path, "/v1/agent/check/fail/")) note := req.URL.Query().Get("note") + + // Get the provided token, if any, and vet against any ACL policies. + var token string + s.parseToken(req, &token) + if err := s.agent.vetCheckUpdate(token, checkID); err != nil { + return nil, err + } + if err := s.agent.updateTTLCheck(checkID, structs.HealthCritical, note); err != nil { return nil, err } @@ -318,6 +377,14 @@ func (s *HTTPServer) AgentCheckUpdate(resp http.ResponseWriter, req *http.Reques } checkID := types.CheckID(strings.TrimPrefix(req.URL.Path, "/v1/agent/check/update/")) + + // Get the provided token, if any, and vet against any ACL policies. + var token string + s.parseToken(req, &token) + if err := s.agent.vetCheckUpdate(token, checkID); err != nil { + return nil, err + } + if err := s.agent.updateTTLCheck(checkID, update.Status, update.Output); err != nil { return nil, err } @@ -327,7 +394,7 @@ func (s *HTTPServer) AgentCheckUpdate(resp http.ResponseWriter, req *http.Reques func (s *HTTPServer) AgentRegisterService(resp http.ResponseWriter, req *http.Request) (interface{}, error) { var args ServiceDefinition - // Fixup the type decode of TTL or Interval if a check if provided + // Fixup the type decode of TTL or Interval if a check if provided. decodeCB := func(raw interface{}) error { rawMap, ok := raw.(map[string]interface{}) if !ok { @@ -360,17 +427,17 @@ func (s *HTTPServer) AgentRegisterService(resp http.ResponseWriter, req *http.Re return nil, nil } - // Verify the service has a name + // Verify the service has a name. if args.Name == "" { resp.WriteHeader(400) resp.Write([]byte("Missing service name")) return nil, nil } - // Get the node service + // Get the node service. ns := args.NodeService() - // Verify the check type + // Verify the check type. chkTypes := args.CheckTypes() for _, check := range chkTypes { if check.Status != "" && !structs.ValidStatus(check.Status) { @@ -385,11 +452,14 @@ func (s *HTTPServer) AgentRegisterService(resp http.ResponseWriter, req *http.Re } } - // Get the provided token, if any + // Get the provided token, if any, and vet against any ACL policies. var token string s.parseToken(req, &token) + if err := s.agent.vetServiceRegister(token, ns); err != nil { + return nil, err + } - // Add the check + // Add the service. if err := s.agent.AddService(ns, chkTypes, true, token); err != nil { return nil, err } @@ -399,6 +469,14 @@ func (s *HTTPServer) AgentRegisterService(resp http.ResponseWriter, req *http.Re func (s *HTTPServer) AgentDeregisterService(resp http.ResponseWriter, req *http.Request) (interface{}, error) { serviceID := strings.TrimPrefix(req.URL.Path, "/v1/agent/service/deregister/") + + // Get the provided token, if any, and vet against any ACL policies. + var token string + s.parseToken(req, &token) + if err := s.agent.vetServiceUpdate(token, serviceID); err != nil { + return nil, err + } + if err := s.agent.RemoveService(serviceID, true); err != nil { return nil, err } @@ -437,9 +515,12 @@ func (s *HTTPServer) AgentServiceMaintenance(resp http.ResponseWriter, req *http return nil, nil } - // Get the provided token, if any + // Get the provided token, if any, and vet against any ACL policies. var token string s.parseToken(req, &token) + if err := s.agent.vetServiceUpdate(token, serviceID); err != nil { + return nil, err + } if enable { reason := params.Get("reason") @@ -482,9 +563,16 @@ func (s *HTTPServer) AgentNodeMaintenance(resp http.ResponseWriter, req *http.Re return nil, nil } - // Get the provided token, if any + // Get the provided token, if any, and vet against any ACL policies. var token string s.parseToken(req, &token) + acl, err := s.agent.resolveToken(token) + if err != nil { + return nil, err + } + if acl != nil && !acl.NodeWrite(s.agent.config.NodeName) { + return nil, permissionDeniedErr + } if enable { s.agent.EnableNodeMaintenance(params.Get("reason"), token) diff --git a/command/agent/agent_endpoint_test.go b/command/agent/agent_endpoint_test.go index 4195a294d6..110a33795b 100644 --- a/command/agent/agent_endpoint_test.go +++ b/command/agent/agent_endpoint_test.go @@ -60,7 +60,12 @@ func TestAgent_Services(t *testing.T) { } srv.agent.state.AddService(srv1, "") - obj, err := srv.AgentServices(nil, nil) + req, err := http.NewRequest("GET", "/v1/agent/services", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + obj, err := srv.AgentServices(nil, req) if err != nil { t.Fatalf("Err: %v", err) } @@ -73,6 +78,47 @@ func TestAgent_Services(t *testing.T) { } } +func TestAgent_Services_ACLFilter(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + // Try no token. + { + req, err := http.NewRequest("GET", "/v1/agent/services", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + obj, err := srv.AgentServices(nil, req) + if err != nil { + t.Fatalf("Err: %v", err) + } + val := obj.(map[string]*structs.NodeService) + if len(val) != 0 { + t.Fatalf("bad: %v", obj) + } + } + + // Try the root token (we will get the implicit "consul" service). + { + req, err := http.NewRequest("GET", "/v1/agent/services?token=root", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + obj, err := srv.AgentServices(nil, req) + if err != nil { + t.Fatalf("Err: %v", err) + } + val := obj.(map[string]*structs.NodeService) + if len(val) != 1 { + t.Fatalf("bad: %v", obj) + } + } +} + func TestAgent_Checks(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) @@ -87,7 +133,12 @@ func TestAgent_Checks(t *testing.T) { } srv.agent.state.AddCheck(chk1, "") - obj, err := srv.AgentChecks(nil, nil) + req, err := http.NewRequest("GET", "/v1/agent/checks", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + obj, err := srv.AgentChecks(nil, req) if err != nil { t.Fatalf("Err: %v", err) } @@ -100,6 +151,55 @@ func TestAgent_Checks(t *testing.T) { } } +func TestAgent_Checks_ACLFilter(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + chk1 := &structs.HealthCheck{ + Node: srv.agent.config.NodeName, + CheckID: "mysql", + Name: "mysql", + Status: structs.HealthPassing, + } + srv.agent.state.AddCheck(chk1, "") + + // Try no token. + { + req, err := http.NewRequest("GET", "/v1/agent/checks", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + obj, err := srv.AgentChecks(nil, req) + if err != nil { + t.Fatalf("Err: %v", err) + } + val := obj.(map[types.CheckID]*structs.HealthCheck) + if len(val) != 0 { + t.Fatalf("bad checks: %v", obj) + } + } + + // Try the root token. + { + req, err := http.NewRequest("GET", "/v1/agent/checks?token=root", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + obj, err := srv.AgentChecks(nil, req) + if err != nil { + t.Fatalf("Err: %v", err) + } + val := obj.(map[types.CheckID]*structs.HealthCheck) + if len(val) != 1 { + t.Fatalf("bad checks: %v", obj) + } + } +} + func TestAgent_Self(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) @@ -364,6 +464,47 @@ func TestAgent_Members_WAN(t *testing.T) { } } +func TestAgent_Members_ACLFilter(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + // Try no token. + { + req, err := http.NewRequest("GET", "/v1/agent/members", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + obj, err := srv.AgentMembers(nil, req) + if err != nil { + t.Fatalf("Err: %v", err) + } + val := obj.([]serf.Member) + if len(val) != 0 { + t.Fatalf("bad members: %v", obj) + } + } + + // Try the root token. + { + req, err := http.NewRequest("GET", "/v1/agent/members?token=root", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + obj, err := srv.AgentMembers(nil, req) + if err != nil { + t.Fatalf("Err: %v", err) + } + val := obj.([]serf.Member) + if len(val) != 1 { + t.Fatalf("bad members: %v", obj) + } + } +} + func TestAgent_Join(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) @@ -480,7 +621,7 @@ func TestAgent_Join_ACLDeny(t *testing.T) { } _, err = srv.AgentJoin(nil, req) - if err != nil { + if err == nil || !strings.Contains(err.Error(), permissionDenied) { t.Fatalf("err: %v", err) } } @@ -580,7 +721,7 @@ func TestAgent_Leave_ACLDeny(t *testing.T) { } _, err = srv.AgentLeave(nil, req) - if err != nil { + if err == nil || !strings.Contains(err.Error(), permissionDenied) { t.Fatalf("err: %v", err) } }() @@ -669,7 +810,7 @@ func TestAgent_ForceLeave_ACLDeny(t *testing.T) { } _, err = srv.AgentForceLeave(nil, req) - if err != nil { + if err == nil || !strings.Contains(err.Error(), permissionDenied) { t.Fatalf("err: %v", err) } } @@ -724,7 +865,7 @@ func TestAgent_RegisterCheck(t *testing.T) { } } -func TestAgent_RegisterCheckPassing(t *testing.T) { +func TestAgent_RegisterCheck_Passing(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -768,7 +909,7 @@ func TestAgent_RegisterCheckPassing(t *testing.T) { } } -func TestAgent_RegisterCheckBadStatus(t *testing.T) { +func TestAgent_RegisterCheck_BadStatus(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -797,6 +938,41 @@ func TestAgent_RegisterCheckBadStatus(t *testing.T) { } } +func TestAgent_RegisterCheck_ACLDeny(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + // Try with no token. + req, err := http.NewRequest("GET", "/v1/agent/check/register", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + args := &CheckDefinition{ + Name: "test", + CheckType: CheckType{ + TTL: 15 * time.Second, + }, + } + req.Body = encodeReq(args) + _, err = srv.AgentRegisterCheck(nil, req) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Try the root token. + req, err = http.NewRequest("GET", "/v1/agent/check/register?token=root", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + req.Body = encodeReq(args) + _, err = srv.AgentRegisterCheck(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } +} + func TestAgent_DeregisterCheck(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) @@ -828,6 +1004,38 @@ func TestAgent_DeregisterCheck(t *testing.T) { } } +func TestAgent_DeregisterCheckACLDeny(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + chk := &structs.HealthCheck{Name: "test", CheckID: "test"} + if err := srv.agent.AddCheck(chk, nil, false, ""); err != nil { + t.Fatalf("err: %v", err) + } + + // Try with no token. + req, err := http.NewRequest("GET", "/v1/agent/check/deregister/test", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + _, err = srv.AgentDeregisterCheck(nil, req) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Try with the root token. + req, err = http.NewRequest("GET", "/v1/agent/check/deregister/test?token=root", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + _, err = srv.AgentDeregisterCheck(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } +} + func TestAgent_PassCheck(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) @@ -860,6 +1068,39 @@ func TestAgent_PassCheck(t *testing.T) { } } +func TestAgent_PassCheck_ACLDeny(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + chk := &structs.HealthCheck{Name: "test", CheckID: "test"} + chkType := &CheckType{TTL: 15 * time.Second} + if err := srv.agent.AddCheck(chk, chkType, false, ""); err != nil { + t.Fatalf("err: %v", err) + } + + // Try with no token. + req, err := http.NewRequest("GET", "/v1/agent/check/pass/test", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + _, err = srv.AgentCheckPass(nil, req) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Try with the root token. + req, err = http.NewRequest("GET", "/v1/agent/check/pass/test?token=root", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + _, err = srv.AgentCheckPass(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } +} + func TestAgent_WarnCheck(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) @@ -892,6 +1133,39 @@ func TestAgent_WarnCheck(t *testing.T) { } } +func TestAgent_WarnCheck_ACLDeny(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + chk := &structs.HealthCheck{Name: "test", CheckID: "test"} + chkType := &CheckType{TTL: 15 * time.Second} + if err := srv.agent.AddCheck(chk, chkType, false, ""); err != nil { + t.Fatalf("err: %v", err) + } + + // Try with no token. + req, err := http.NewRequest("GET", "/v1/agent/check/warn/test", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + _, err = srv.AgentCheckWarn(nil, req) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Try with the root token. + req, err = http.NewRequest("GET", "/v1/agent/check/warn/test?token=root", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + _, err = srv.AgentCheckWarn(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } +} + func TestAgent_FailCheck(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) @@ -924,6 +1198,39 @@ func TestAgent_FailCheck(t *testing.T) { } } +func TestAgent_FailCheck_ACLDeny(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + chk := &structs.HealthCheck{Name: "test", CheckID: "test"} + chkType := &CheckType{TTL: 15 * time.Second} + if err := srv.agent.AddCheck(chk, chkType, false, ""); err != nil { + t.Fatalf("err: %v", err) + } + + // Try with no token. + req, err := http.NewRequest("GET", "/v1/agent/check/fail/test", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + _, err = srv.AgentCheckFail(nil, req) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Try with the root token. + req, err = http.NewRequest("GET", "/v1/agent/check/fail/test?token=root", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + _, err = srv.AgentCheckFail(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } +} + func TestAgent_UpdateCheck(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) @@ -1052,13 +1359,47 @@ func TestAgent_UpdateCheck(t *testing.T) { } } +func TestAgent_UpdateCheck_ACLDeny(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + chk := &structs.HealthCheck{Name: "test", CheckID: "test"} + chkType := &CheckType{TTL: 15 * time.Second} + if err := srv.agent.AddCheck(chk, chkType, false, ""); err != nil { + t.Fatalf("err: %v", err) + } + + // Try with no token. + req, err := http.NewRequest("PUT", "/v1/agent/check/update/test", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + req.Body = encodeReq(checkUpdate{structs.HealthPassing, "hello-passing"}) + _, err = srv.AgentCheckUpdate(nil, req) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Try with the root token. + req, err = http.NewRequest("PUT", "/v1/agent/check/update/test?token=root", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + req.Body = encodeReq(checkUpdate{structs.HealthPassing, "hello-passing"}) + _, err = srv.AgentCheckUpdate(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } +} + func TestAgent_RegisterService(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() defer srv.agent.Shutdown() - // Register node req, err := http.NewRequest("GET", "/v1/agent/service/register?token=abc123", nil) if err != nil { t.Fatalf("err: %v", err) @@ -1110,6 +1451,52 @@ func TestAgent_RegisterService(t *testing.T) { } } +func TestAgent_RegisterService_ACLDeny(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + args := &ServiceDefinition{ + Name: "test", + Tags: []string{"master"}, + Port: 8000, + Check: CheckType{ + TTL: 15 * time.Second, + }, + Checks: CheckTypes{ + &CheckType{ + TTL: 20 * time.Second, + }, + &CheckType{ + TTL: 30 * time.Second, + }, + }, + } + + // Try with no token. + req, err := http.NewRequest("GET", "/v1/agent/service/register", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + req.Body = encodeReq(args) + _, err = srv.AgentRegisterService(nil, req) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Try with the root token. + req, err = http.NewRequest("GET", "/v1/agent/service/register?token=root", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + req.Body = encodeReq(args) + _, err = srv.AgentRegisterService(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } +} + func TestAgent_DeregisterService(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) @@ -1124,7 +1511,6 @@ func TestAgent_DeregisterService(t *testing.T) { t.Fatalf("err: %v", err) } - // Register node req, err := http.NewRequest("GET", "/v1/agent/service/deregister/test", nil) if err != nil { t.Fatalf("err: %v", err) @@ -1148,7 +1534,42 @@ func TestAgent_DeregisterService(t *testing.T) { } } -func TestAgent_ServiceMaintenanceEndpoint_BadRequest(t *testing.T) { +func TestAgent_DeregisterService_ACLDeny(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + service := &structs.NodeService{ + ID: "test", + Service: "test", + } + if err := srv.agent.AddService(service, nil, false, ""); err != nil { + t.Fatalf("err: %v", err) + } + + // Try without a token. + req, err := http.NewRequest("GET", "/v1/agent/service/deregister/test", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + _, err = srv.AgentDeregisterService(nil, req) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Try with the root. + req, err = http.NewRequest("GET", "/v1/agent/service/deregister/test?token=root", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + _, err = srv.AgentDeregisterService(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } +} + +func TestAgent_ServiceMaintenance_BadRequest(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -1195,7 +1616,7 @@ func TestAgent_ServiceMaintenanceEndpoint_BadRequest(t *testing.T) { } } -func TestAgent_EnableServiceMaintenance(t *testing.T) { +func TestAgent_ServiceMaintenance_Enable(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -1238,7 +1659,7 @@ func TestAgent_EnableServiceMaintenance(t *testing.T) { } } -func TestAgent_DisableServiceMaintenance(t *testing.T) { +func TestAgent_ServiceMaintenance_Disable(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -1275,7 +1696,43 @@ func TestAgent_DisableServiceMaintenance(t *testing.T) { } } -func TestAgent_NodeMaintenanceEndpoint_BadRequest(t *testing.T) { +func TestAgent_ServiceMaintenance_ACLDeny(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + // Register the service. + service := &structs.NodeService{ + ID: "test", + Service: "test", + } + if err := srv.agent.AddService(service, nil, false, ""); err != nil { + t.Fatalf("err: %v", err) + } + + // Try with no token. + req, err := http.NewRequest("PUT", "/v1/agent/service/maintenance/test?enable=true&reason=broken", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + _, err = srv.AgentServiceMaintenance(nil, req) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Try with the root token. + req, err = http.NewRequest("PUT", "/v1/agent/service/maintenance/test?enable=true&reason=broken&token=root", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + _, err = srv.AgentServiceMaintenance(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } +} + +func TestAgent_NodeMaintenance_BadRequest(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -1302,7 +1759,7 @@ func TestAgent_NodeMaintenanceEndpoint_BadRequest(t *testing.T) { } } -func TestAgent_EnableNodeMaintenance(t *testing.T) { +func TestAgent_NodeMaintenance_Enable(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -1336,7 +1793,7 @@ func TestAgent_EnableNodeMaintenance(t *testing.T) { } } -func TestAgent_DisableNodeMaintenance(t *testing.T) { +func TestAgent_NodeMaintenance_Disable(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -1361,7 +1818,36 @@ func TestAgent_DisableNodeMaintenance(t *testing.T) { } } -func TestAgent_RegisterServiceCheck(t *testing.T) { +func TestAgent_NodeMaintenance_ACLDeny(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + // Try with no token. + req, err := http.NewRequest( + "PUT", "/v1/agent/self/maintenance?enable=true&reason=broken", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + _, err = srv.AgentNodeMaintenance(nil, req) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Try with the root token. + req, err = http.NewRequest( + "PUT", "/v1/agent/self/maintenance?enable=true&reason=broken&token=root", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + _, err = srv.AgentNodeMaintenance(nil, req) + if err != nil { + t.Fatalf("err: %v", err) + } +} + +func TestAgent_RegisterCheck_Service(t *testing.T) { dir, srv := makeHTTPServer(t) defer os.RemoveAll(dir) defer srv.Shutdown() From 4ffd8245473bff221f225a9ebad1959bde6f3db0 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Wed, 14 Dec 2016 16:18:17 -0800 Subject: [PATCH 06/10] Adds a leader wait when testing with ACLs. --- command/agent/http_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 846778ea98..540d329175 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -33,7 +33,7 @@ func makeHTTPServerWithConfig(t *testing.T, cb func(c *Config)) (string, *HTTPSe } func makeHTTPServerWithACLs(t *testing.T) (string, *HTTPServer) { - return makeHTTPServerWithConfig(t, func(c *Config) { + dir, srv := makeHTTPServerWithConfig(t, func(c *Config) { c.ACLDatacenter = c.Datacenter c.ACLDefaultPolicy = "deny" c.ACLMasterToken = "root" @@ -41,6 +41,11 @@ func makeHTTPServerWithACLs(t *testing.T) (string, *HTTPServer) { c.ACLAgentMasterToken = "towel" c.ACLEnforceVersion8 = Bool(true) }) + + // Need a leader to look up ACLs, so wait here so we don't need to + // repeat this in each test. + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") + return dir, srv } func makeHTTPServerWithConfigLog(t *testing.T, cb func(c *Config), l io.Writer, logWriter *logger.LogWriter) (string, *HTTPServer) { From 171ec6e48754a8124b0fb687b1e0eeabb4a53dfd Mon Sep 17 00:00:00 2001 From: James Phillips Date: Wed, 14 Dec 2016 18:13:30 -0800 Subject: [PATCH 07/10] Fixes a race in the monitor endpoint test that would cause panics. --- command/agent/agent_endpoint_test.go | 70 +++++++++------------------- 1 file changed, 22 insertions(+), 48 deletions(-) diff --git a/command/agent/agent_endpoint_test.go b/command/agent/agent_endpoint_test.go index 110a33795b..b755f94d6d 100644 --- a/command/agent/agent_endpoint_test.go +++ b/command/agent/agent_endpoint_test.go @@ -1927,59 +1927,29 @@ func TestAgent_Monitor(t *testing.T) { t.Fatalf("bad: %s", body) } - // Begin streaming logs from the monitor endpoint - req, _ = http.NewRequest("GET", "/v1/agent/monitor?loglevel=debug", nil) - resp = newClosableRecorder() - go func() { - if _, err := srv.AgentMonitor(resp, req); err != nil { - t.Fatalf("err: %s", err) - } - }() - - // Write the incoming logs from http to a channel for comparison - logCh := make(chan string, 5) - - // Block until the first log entry from http + // Try to stream logs until we see the expected log line + expected := []byte("raft: Initial configuration (index=1)") testutil.WaitForResult(func() (bool, error) { - line, err := resp.Body.ReadString('\n') - if err != nil && err != io.EOF { - return false, fmt.Errorf("err: %v", err) - } - if line == "" { - return false, fmt.Errorf("blank line") - } - logCh <- line - return true, nil - }, func(err error) { - t.Fatal(err) - }) + req, _ = http.NewRequest("GET", "/v1/agent/monitor?loglevel=debug", nil) + resp = newClosableRecorder() + done := make(chan struct{}) + go func() { + if _, err := srv.AgentMonitor(resp, req); err != nil { + t.Fatalf("err: %s", err) + } + close(done) + }() - go func() { - for { - line, err := resp.Body.ReadString('\n') - if err != nil && err != io.EOF { - t.Fatalf("err: %v", err) - } - if line != "" { - logCh <- line - } - } - }() + resp.Close() + <-done - // Wait until we see the expected log line - expected := "raft: Initial configuration (index=1)" - testutil.WaitForResult(func() (bool, error) { - select { - case log := <-logCh: - if !strings.Contains(log, expected) { - return false, fmt.Errorf("Log message does not match expected") - } - case <-time.After(10 * time.Second): - return false, fmt.Errorf("failed to get log within timeout") + if bytes.Contains(resp.Body.Bytes(), expected) { + return true, nil + } else { + return false, fmt.Errorf("didn't see expected") } - return true, nil }, func(err error) { - t.Fatal(err) + t.Fatalf("err: %v", err) }) } @@ -1994,6 +1964,10 @@ func newClosableRecorder() *closableRecorder { return &closableRecorder{r, closer} } +func (r *closableRecorder) Close() { + close(r.closer) +} + func (r *closableRecorder) CloseNotify() <-chan bool { return r.closer } From babb0a12352dfc1a201d911afa72a6501c13594e Mon Sep 17 00:00:00 2001 From: James Phillips Date: Wed, 14 Dec 2016 19:28:09 -0800 Subject: [PATCH 08/10] Adds remaining unit tests for agent ACL vet and filter functions. --- command/agent/acl_test.go | 389 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 389 insertions(+) diff --git a/command/agent/acl_test.go b/command/agent/acl_test.go index 0f965edca6..2723cda002 100644 --- a/command/agent/acl_test.go +++ b/command/agent/acl_test.go @@ -12,6 +12,8 @@ import ( rawacl "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/consul/structs" "github.com/hashicorp/consul/testutil" + "github.com/hashicorp/consul/types" + "github.com/hashicorp/serf/serf" ) func TestACL_Bad_Config(t *testing.T) { @@ -472,3 +474,390 @@ func TestACL_Cache(t *testing.T) { t.Fatalf("should refresh") } } + +// catalogPolicy supplies some standard policies to help with testing the +// catalog-related vet and filter functions. +func catalogPolicy(req *structs.ACLPolicyRequest, reply *structs.ACLPolicy) error { + reply.Policy = &rawacl.Policy{} + + switch req.ACL { + + case "node-ro": + reply.Policy.Nodes = append(reply.Policy.Nodes, + &rawacl.NodePolicy{Name: "Node", Policy: "read"}) + + case "node-rw": + reply.Policy.Nodes = append(reply.Policy.Nodes, + &rawacl.NodePolicy{Name: "Node", Policy: "write"}) + + case "service-ro": + reply.Policy.Services = append(reply.Policy.Services, + &rawacl.ServicePolicy{Name: "service", Policy: "read"}) + + case "service-rw": + reply.Policy.Services = append(reply.Policy.Services, + &rawacl.ServicePolicy{Name: "service", Policy: "write"}) + + case "other-rw": + reply.Policy.Services = append(reply.Policy.Services, + &rawacl.ServicePolicy{Name: "other", Policy: "write"}) + + default: + return fmt.Errorf("unknown token %q", req.ACL) + } + + return nil +} + +func TestACL_vetServiceRegister(t *testing.T) { + config := nextConfig() + config.ACLEnforceVersion8 = Bool(true) + + dir, agent := makeAgent(t, config) + defer os.RemoveAll(dir) + defer agent.Shutdown() + + testutil.WaitForLeader(t, agent.RPC, "dc1") + + m := MockServer{catalogPolicy} + if err := agent.InjectEndpoint("ACL", &m); err != nil { + t.Fatalf("err: %v", err) + } + + // Register a new service, with permission. + err := agent.vetServiceRegister("service-rw", &structs.NodeService{ + ID: "my-service", + Service: "service", + }) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Register a new service without write privs. + err = agent.vetServiceRegister("service-ro", &structs.NodeService{ + ID: "my-service", + Service: "service", + }) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Try to register over a service without write privs to the existing + // service. + agent.state.AddService(&structs.NodeService{ + ID: "my-service", + Service: "other", + }, "") + err = agent.vetServiceRegister("service-rw", &structs.NodeService{ + ID: "my-service", + Service: "service", + }) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } +} + +func TestACL_vetServiceUpdate(t *testing.T) { + config := nextConfig() + config.ACLEnforceVersion8 = Bool(true) + + dir, agent := makeAgent(t, config) + defer os.RemoveAll(dir) + defer agent.Shutdown() + + testutil.WaitForLeader(t, agent.RPC, "dc1") + + m := MockServer{catalogPolicy} + if err := agent.InjectEndpoint("ACL", &m); err != nil { + t.Fatalf("err: %v", err) + } + + // Update a service that doesn't exist. + err := agent.vetServiceUpdate("service-rw", "my-service") + if err == nil || !strings.Contains(err.Error(), "Unknown service") { + t.Fatalf("err: %v", err) + } + + // Update with write privs. + agent.state.AddService(&structs.NodeService{ + ID: "my-service", + Service: "service", + }, "") + err = agent.vetServiceUpdate("service-rw", "my-service") + if err != nil { + t.Fatalf("err: %v", err) + } + + // Update without write privs. + err = agent.vetServiceUpdate("service-ro", "my-service") + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } +} + +func TestACL_vetCheckRegister(t *testing.T) { + config := nextConfig() + config.ACLEnforceVersion8 = Bool(true) + + dir, agent := makeAgent(t, config) + defer os.RemoveAll(dir) + defer agent.Shutdown() + + testutil.WaitForLeader(t, agent.RPC, "dc1") + + m := MockServer{catalogPolicy} + if err := agent.InjectEndpoint("ACL", &m); err != nil { + t.Fatalf("err: %v", err) + } + + // Register a new service check with write privs. + err := agent.vetCheckRegister("service-rw", &structs.HealthCheck{ + CheckID: types.CheckID("my-check"), + ServiceID: "my-service", + ServiceName: "service", + }) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Register a new service check without write privs. + err = agent.vetCheckRegister("service-ro", &structs.HealthCheck{ + CheckID: types.CheckID("my-check"), + ServiceID: "my-service", + ServiceName: "service", + }) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Register a new node check with write privs. + err = agent.vetCheckRegister("node-rw", &structs.HealthCheck{ + CheckID: types.CheckID("my-check"), + }) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Register a new node check without write privs. + err = agent.vetCheckRegister("node-ro", &structs.HealthCheck{ + CheckID: types.CheckID("my-check"), + }) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Try to register over a service check without write privs to the + // existing service. + agent.state.AddService(&structs.NodeService{ + ID: "my-service", + Service: "service", + }, "") + agent.state.AddCheck(&structs.HealthCheck{ + CheckID: types.CheckID("my-check"), + ServiceID: "my-service", + ServiceName: "other", + }, "") + err = agent.vetCheckRegister("service-rw", &structs.HealthCheck{ + CheckID: types.CheckID("my-check"), + ServiceID: "my-service", + ServiceName: "service", + }) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Try to register over a node check without write privs to the node. + agent.state.AddCheck(&structs.HealthCheck{ + CheckID: types.CheckID("my-node-check"), + }, "") + err = agent.vetCheckRegister("service-rw", &structs.HealthCheck{ + CheckID: types.CheckID("my-node-check"), + ServiceID: "my-service", + ServiceName: "service", + }) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } +} + +func TestACL_vetCheckUpdate(t *testing.T) { + config := nextConfig() + config.ACLEnforceVersion8 = Bool(true) + + dir, agent := makeAgent(t, config) + defer os.RemoveAll(dir) + defer agent.Shutdown() + + testutil.WaitForLeader(t, agent.RPC, "dc1") + + m := MockServer{catalogPolicy} + if err := agent.InjectEndpoint("ACL", &m); err != nil { + t.Fatalf("err: %v", err) + } + + // Update a check that doesn't exist. + err := agent.vetCheckUpdate("node-rw", "my-check") + if err == nil || !strings.Contains(err.Error(), "Unknown check") { + t.Fatalf("err: %v", err) + } + + // Update service check with write privs. + agent.state.AddService(&structs.NodeService{ + ID: "my-service", + Service: "service", + }, "") + agent.state.AddCheck(&structs.HealthCheck{ + CheckID: types.CheckID("my-service-check"), + ServiceID: "my-service", + ServiceName: "service", + }, "") + err = agent.vetCheckUpdate("service-rw", "my-service-check") + if err != nil { + t.Fatalf("err: %v", err) + } + + // Update service check without write privs. + err = agent.vetCheckUpdate("service-ro", "my-service-check") + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Update node check with write privs. + agent.state.AddCheck(&structs.HealthCheck{ + CheckID: types.CheckID("my-node-check"), + }, "") + err = agent.vetCheckUpdate("node-rw", "my-node-check") + if err != nil { + t.Fatalf("err: %v", err) + } + + // Update without write privs. + err = agent.vetCheckUpdate("node-ro", "my-node-check") + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } +} + +func TestACL_filterMembers(t *testing.T) { + config := nextConfig() + config.ACLEnforceVersion8 = Bool(true) + + dir, agent := makeAgent(t, config) + defer os.RemoveAll(dir) + defer agent.Shutdown() + + testutil.WaitForLeader(t, agent.RPC, "dc1") + + m := MockServer{catalogPolicy} + if err := agent.InjectEndpoint("ACL", &m); err != nil { + t.Fatalf("err: %v", err) + } + + var members []serf.Member + if err := agent.filterMembers("node-ro", &members); err != nil { + t.Fatalf("err: %v", err) + } + if len(members) != 0 { + t.Fatalf("bad: %#v", members) + } + + members = []serf.Member{ + serf.Member{Name: "Node 1"}, + serf.Member{Name: "Nope"}, + serf.Member{Name: "Node 2"}, + } + if err := agent.filterMembers("node-ro", &members); err != nil { + t.Fatalf("err: %v", err) + } + if len(members) != 2 || + members[0].Name != "Node 1" || + members[1].Name != "Node 2" { + t.Fatalf("bad: %#v", members) + } +} + +func TestACL_filterServices(t *testing.T) { + config := nextConfig() + config.ACLEnforceVersion8 = Bool(true) + + dir, agent := makeAgent(t, config) + defer os.RemoveAll(dir) + defer agent.Shutdown() + + testutil.WaitForLeader(t, agent.RPC, "dc1") + + m := MockServer{catalogPolicy} + if err := agent.InjectEndpoint("ACL", &m); err != nil { + t.Fatalf("err: %v", err) + } + + services := make(map[string]*structs.NodeService) + if err := agent.filterServices("node-ro", &services); err != nil { + t.Fatalf("err: %v", err) + } + + services["my-service"] = &structs.NodeService{ID: "my-service", Service: "service"} + services["my-other"] = &structs.NodeService{ID: "my-other", Service: "other"} + if err := agent.filterServices("service-ro", &services); err != nil { + t.Fatalf("err: %v", err) + } + if _, ok := services["my-service"]; !ok { + t.Fatalf("bad: %#v", services) + } + if _, ok := services["my-other"]; ok { + t.Fatalf("bad: %#v", services) + } +} + +func TestACL_filterChecks(t *testing.T) { + config := nextConfig() + config.ACLEnforceVersion8 = Bool(true) + + dir, agent := makeAgent(t, config) + defer os.RemoveAll(dir) + defer agent.Shutdown() + + testutil.WaitForLeader(t, agent.RPC, "dc1") + + m := MockServer{catalogPolicy} + if err := agent.InjectEndpoint("ACL", &m); err != nil { + t.Fatalf("err: %v", err) + } + + checks := make(map[types.CheckID]*structs.HealthCheck) + if err := agent.filterChecks("node-ro", &checks); err != nil { + t.Fatalf("err: %v", err) + } + + checks["my-node"] = &structs.HealthCheck{} + checks["my-service"] = &structs.HealthCheck{ServiceName: "service"} + checks["my-other"] = &structs.HealthCheck{ServiceName: "other"} + if err := agent.filterChecks("service-ro", &checks); err != nil { + t.Fatalf("err: %v", err) + } + if _, ok := checks["my-node"]; ok { + t.Fatalf("bad: %#v", checks) + } + if _, ok := checks["my-service"]; !ok { + t.Fatalf("bad: %#v", checks) + } + if _, ok := checks["my-other"]; ok { + t.Fatalf("bad: %#v", checks) + } + + checks["my-node"] = &structs.HealthCheck{} + checks["my-service"] = &structs.HealthCheck{ServiceName: "service"} + checks["my-other"] = &structs.HealthCheck{ServiceName: "other"} + if err := agent.filterChecks("node-ro", &checks); err != nil { + t.Fatalf("err: %v", err) + } + if _, ok := checks["my-node"]; !ok { + t.Fatalf("bad: %#v", checks) + } + if _, ok := checks["my-service"]; ok { + t.Fatalf("bad: %#v", checks) + } + if _, ok := checks["my-other"]; ok { + t.Fatalf("bad: %#v", checks) + } +} From ededf330bad0c916733f5fd6f92ca71cbe2e1b88 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Wed, 14 Dec 2016 19:42:37 -0800 Subject: [PATCH 09/10] Adds complete ACL support for listing events. --- command/agent/event_endpoint.go | 23 +++++++++- command/agent/event_endpoint_test.go | 66 ++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/command/agent/event_endpoint.go b/command/agent/event_endpoint.go index 94d35fc5ae..84d2550c52 100644 --- a/command/agent/event_endpoint.go +++ b/command/agent/event_endpoint.go @@ -83,6 +83,14 @@ func (s *HTTPServer) EventList(resp http.ResponseWriter, req *http.Request) (int return nil, nil } + // Fetch the ACL token, if any. + var token string + s.parseToken(req, &token) + acl, err := s.agent.resolveToken(token) + if err != nil { + return nil, err + } + // Look for a name filter var nameFilter string if filt := req.URL.Query().Get("name"); filt != "" { @@ -126,7 +134,20 @@ RUN_QUERY: // Get the recent events events := s.agent.UserEvents() - // Filter the events if necessary + // Filter the events using the ACL, if present + if acl != nil { + for i := 0; i < len(events); i++ { + name := events[i].Name + if acl.EventRead(name) { + continue + } + s.agent.logger.Printf("[DEBUG] agent: dropping event %q from result due to ACLs", name) + events = append(events[:i], events[i+1:]...) + i-- + } + } + + // Filter the events if requested if nameFilter != "" { for i := 0; i < len(events); i++ { if events[i].Name != nameFilter { diff --git a/command/agent/event_endpoint_test.go b/command/agent/event_endpoint_test.go index d90ddc485c..53927daebf 100644 --- a/command/agent/event_endpoint_test.go +++ b/command/agent/event_endpoint_test.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "os" "strings" "testing" "time" @@ -192,6 +193,71 @@ func TestEventList_Filter(t *testing.T) { }) } +func TestEventList_ACLFilter(t *testing.T) { + dir, srv := makeHTTPServerWithACLs(t) + defer os.RemoveAll(dir) + defer srv.Shutdown() + defer srv.agent.Shutdown() + + // Fire an event. + p := &UserEvent{Name: "foo"} + if err := srv.agent.UserEvent("dc1", "root", p); err != nil { + t.Fatalf("err: %v", err) + } + + // Try no token. + { + testutil.WaitForResult(func() (bool, error) { + req, err := http.NewRequest("GET", "/v1/event/list", nil) + if err != nil { + return false, err + } + resp := httptest.NewRecorder() + obj, err := srv.EventList(resp, req) + if err != nil { + return false, err + } + + list, ok := obj.([]*UserEvent) + if !ok { + return false, fmt.Errorf("bad: %#v", obj) + } + if len(list) != 0 { + return false, fmt.Errorf("bad: %#v", list) + } + return true, nil + }, func(err error) { + t.Fatalf("err: %v", err) + }) + } + + // Try the root token. + { + testutil.WaitForResult(func() (bool, error) { + req, err := http.NewRequest("GET", "/v1/event/list?token=root", nil) + if err != nil { + return false, err + } + resp := httptest.NewRecorder() + obj, err := srv.EventList(resp, req) + if err != nil { + return false, err + } + + list, ok := obj.([]*UserEvent) + if !ok { + return false, fmt.Errorf("bad: %#v", obj) + } + if len(list) != 1 || list[0].Name != "foo" { + return false, fmt.Errorf("bad: %#v", list) + } + return true, nil + }, func(err error) { + t.Fatalf("err: %v", err) + }) + } +} + func TestEventList_Blocking(t *testing.T) { httpTest(t, func(srv *HTTPServer) { p := &UserEvent{Name: "test"} From 3a44d4612672fc83c0beab10a90e4a995a7736c3 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Wed, 14 Dec 2016 20:32:44 -0800 Subject: [PATCH 10/10] Adds some basic documentation about the new ACL changes. --- .../source/docs/internals/acl.html.markdown | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/website/source/docs/internals/acl.html.markdown b/website/source/docs/internals/acl.html.markdown index b156a3f7cb..e113f9dfc5 100644 --- a/website/source/docs/internals/acl.html.markdown +++ b/website/source/docs/internals/acl.html.markdown @@ -531,3 +531,83 @@ These differences are outlined in the table below: The captured token, client's token, or anonymous token is used to filter the results, as described above. + + +## ACL Changes Coming in Consul 0.8 + +Consul 0.8 will feature complete ACL coverage for all of Consul. To ease the +transition to the new policies, a beta version of complete ACL support is +available starting in Consul 0.7.2. + +Here's a summary of the upcoming changes: + +* Agents now check `node` and `service` ACL policies for catalog-related operations + in `/v1/agent` endpoints, such as service and check registration and health check + updates. +* Agents enforce a new `agent` ACL policy for utility operations in `/v1/agent` + endpoints, such as joins and leaves. +* A new `node` ACL policy is enforced throughout Consul, providing a mechanism to + restrict registration and discovery of nodes by name. This also applies to + service discovery, so provides an additional dimension for controlling access to + services. +* A new `session` ACL policy controls the ability to create session objects by node + name. +* Anonymous prepared queries (non-templates without a `Name`) now require a valid + session, which ties their creation to the new `session` ACL policy. +* The existing `event` ACL policy has been applied to the `/v1/event/list` endpoint. + +#### New Configuration Options + +To enable beta support for complete ACL coverage, set the +[`acl_enforce_version_8`](/docs/agent/options.html#acl_enforce_version_8) configuration +option to `true` on Consul clients and servers. + +Two new configuration options are used once complete ACLs are enabled: + +* [`acl_agent_master_token`](/docs/agent/options.html#acl_agent_master_token) is used as + a special access token that has `agent` ACL policy `write` privileges on each agent where + it is configured. This token should only be used by operators during outages when Consul + servers aren't available to resolve ACL tokens. Applications should use regular ACL + tokens during normal operation. +* [`acl_agent_token`](/docs/agent/options.html#acl_agent_token) is used internally by + Consul agents to perform operations to the service catalog when registering themselves + or sending network coordinates to the servers. +
+
+ For clients, this token must at least have `node` ACL policy `write` access to the node + name it will register as. For servers, this must have `node` ACL policy `write` access to + all nodes that are expected to join the cluster, as well as `service` ACL policy `write` + access to the `consul` service, which will be registered automatically on its behalf. + +Since clients now resolve ACLs locally, the [`acl_down_policy`](/docs/agent/options.html#acl_down_policy) +now applies to Consul clients as well as Consul servers. This will determine what the +client will do in the event that the servers are down. + +Consul clients *do not* need to have the [`acl_master_token`](/docs/agent/options.html#acl_agent_master_token) +or the [`acl_datacenter`](/docs/agent/options.html#acl_datacenter) configured. They will +contact the Consul servers to determine if ACLs are enabled. If they detect that ACLs are +not enabled, they will check at most every 2 minutes to see if they have become enabled, and +will start enforcing ACLs automatically. + +#### New ACL Policies + +The new `agent` ACL policy looks like this: + +``` +agent "" { + policy = "" +} +``` + +This affects utility-related agent endpoints, such as `/v1/agent/self` and `/v1/agent/join`. + +The new `node` ACL policy looks like this: + +``` +node "" { + policy = "" +} +```` + +This affects node registration, node discovery, service discovery, and endpoints like +`/v1/agent/members`.