Merge pull request #2590 from hashicorp/acl-complete-catalog

Adds new node ACL policy and applies it to catalog endpoints.

acl/acl.go

@@ -35,6 +35,18 @@ func init() {
 
 // ACL is the interface for policy enforcement.
 type ACL interface {
+	// ACLList checks for permission to list all the ACLs
+	ACLList() bool
+
+	// ACLModify checks for permission to manipulate ACLs
+	ACLModify() bool
+
+	// EventRead determines if a specific event can be queried.
+	EventRead(string) bool
+
+	// EventWrite determines if a specific event may be fired.
+	EventWrite(string) bool
+
 	// KeyRead checks for permission to read a given key
 	KeyRead(string) bool
 
@@ -46,26 +58,6 @@ type ACL interface {
 	// that deny a write.
 	KeyWritePrefix(string) bool
 
-	// ServiceWrite checks for permission to read a given service
-	ServiceWrite(string) bool
-
-	// ServiceRead checks for permission to read a given service
-	ServiceRead(string) bool
-
-	// EventRead determines if a specific event can be queried.
-	EventRead(string) bool
-
-	// EventWrite determines if a specific event may be fired.
-	EventWrite(string) bool
-
-	// PrepardQueryRead determines if a specific prepared query can be read
-	// to show its contents (this is not used for execution).
-	PreparedQueryRead(string) bool
-
-	// PreparedQueryWrite determines if a specific prepared query can be
-	// created, modified, or deleted.
-	PreparedQueryWrite(string) bool
-
 	// KeyringRead determines if the encryption keyring used in
 	// the gossip layer can be read.
 	KeyringRead() bool
@@ -73,6 +65,13 @@ type ACL interface {
 	// KeyringWrite determines if the keyring can be manipulated
 	KeyringWrite() bool
 
+	// NodeRead checks for permission to read (discover) a given node.
+	NodeRead(string) bool
+
+	// NodeWrite checks for permission to create or update (register) a
+	// given node.
+	NodeWrite(string) bool
+
 	// OperatorRead determines if the read-only Consul operator functions
 	// can be used.
 	OperatorRead() bool
@@ -81,11 +80,20 @@ type ACL interface {
 	// functions can be used.
 	OperatorWrite() bool
 
-	// ACLList checks for permission to list all the ACLs
+	// PrepardQueryRead determines if a specific prepared query can be read
-	ACLList() bool
+	// to show its contents (this is not used for execution).
+	PreparedQueryRead(string) bool
 
-	// ACLModify checks for permission to manipulate ACLs
+	// PreparedQueryWrite determines if a specific prepared query can be
-	ACLModify() bool
+	// created, modified, or deleted.
+	PreparedQueryWrite(string) bool
+
+	// ServiceRead checks for permission to read a given service
+	ServiceRead(string) bool
+
+	// ServiceWrite checks for permission to create or update a given
+	// service
+	ServiceWrite(string) bool
 
 	// Snapshot checks for permission to take and restore snapshots.
 	Snapshot() bool
@@ -99,6 +107,22 @@ type StaticACL struct {
 	defaultAllow bool
 }
 
+func (s *StaticACL) ACLList() bool {
+	return s.allowManage
+}
+
+func (s *StaticACL) ACLModify() bool {
+	return s.allowManage
+}
+
+func (s *StaticACL) EventRead(string) bool {
+	return s.defaultAllow
+}
+
+func (s *StaticACL) EventWrite(string) bool {
+	return s.defaultAllow
+}
+
 func (s *StaticACL) KeyRead(string) bool {
 	return s.defaultAllow
 }
@@ -111,30 +135,6 @@ func (s *StaticACL) KeyWritePrefix(string) bool {
 	return s.defaultAllow
 }
 
-func (s *StaticACL) ServiceRead(string) bool {
-	return s.defaultAllow
-}
-
-func (s *StaticACL) ServiceWrite(string) bool {
-	return s.defaultAllow
-}
-
-func (s *StaticACL) EventRead(string) bool {
-	return s.defaultAllow
-}
-
-func (s *StaticACL) EventWrite(string) bool {
-	return s.defaultAllow
-}
-
-func (s *StaticACL) PreparedQueryRead(string) bool {
-	return s.defaultAllow
-}
-
-func (s *StaticACL) PreparedQueryWrite(string) bool {
-	return s.defaultAllow
-}
-
 func (s *StaticACL) KeyringRead() bool {
 	return s.defaultAllow
 }
@@ -143,6 +143,14 @@ func (s *StaticACL) KeyringWrite() bool {
 	return s.defaultAllow
 }
 
+func (s *StaticACL) NodeRead(string) bool {
+	return s.defaultAllow
+}
+
+func (s *StaticACL) NodeWrite(string) bool {
+	return s.defaultAllow
+}
+
 func (s *StaticACL) OperatorRead() bool {
 	return s.defaultAllow
 }
@@ -151,12 +159,20 @@ func (s *StaticACL) OperatorWrite() bool {
 	return s.defaultAllow
 }
 
-func (s *StaticACL) ACLList() bool {
+func (s *StaticACL) PreparedQueryRead(string) bool {
-	return s.allowManage
+	return s.defaultAllow
 }
 
-func (s *StaticACL) ACLModify() bool {
+func (s *StaticACL) PreparedQueryWrite(string) bool {
-	return s.allowManage
+	return s.defaultAllow
+}
+
+func (s *StaticACL) ServiceRead(string) bool {
+	return s.defaultAllow
+}
+
+func (s *StaticACL) ServiceWrite(string) bool {
+	return s.defaultAllow
 }
 
 func (s *StaticACL) Snapshot() bool {
@@ -202,6 +218,9 @@ type PolicyACL struct {
 	// keyRules contains the key policies
 	keyRules *radix.Tree
 
+	// nodeRules contains the node policies
+	nodeRules *radix.Tree
+
 	// serviceRules contains the service policies
 	serviceRules *radix.Tree
 
@@ -226,6 +245,7 @@ func New(parent ACL, policy *Policy) (*PolicyACL, error) {
 	p := &PolicyACL{
 		parent:             parent,
 		keyRules:           radix.New(),
+		nodeRules:          radix.New(),
 		serviceRules:       radix.New(),
 		eventRules:         radix.New(),
 		preparedQueryRules: radix.New(),
@@ -236,6 +256,11 @@ func New(parent ACL, policy *Policy) (*PolicyACL, error) {
 		p.keyRules.Insert(kp.Prefix, kp.Policy)
 	}
 
+	// Load the node policy
+	for _, np := range policy.Nodes {
+		p.nodeRules.Insert(np.Name, np.Policy)
+	}
+
 	// Load the service policy
 	for _, sp := range policy.Services {
 		p.serviceRules.Insert(sp.Name, sp.Policy)
@@ -260,6 +285,50 @@ func New(parent ACL, policy *Policy) (*PolicyACL, error) {
 	return p, nil
 }
 
+// ACLList checks if listing of ACLs is allowed
+func (p *PolicyACL) ACLList() bool {
+	return p.parent.ACLList()
+}
+
+// ACLModify checks if modification of ACLs is allowed
+func (p *PolicyACL) ACLModify() bool {
+	return p.parent.ACLModify()
+}
+
+// Snapshot checks if taking and restoring snapshots is allowed.
+func (p *PolicyACL) Snapshot() bool {
+	return p.parent.Snapshot()
+}
+
+// EventRead is used to determine if the policy allows for a
+// specific user event to be read.
+func (p *PolicyACL) EventRead(name string) bool {
+	// Longest-prefix match on event names
+	if _, rule, ok := p.eventRules.LongestPrefix(name); ok {
+		switch rule {
+		case PolicyRead, PolicyWrite:
+			return true
+		default:
+			return false
+		}
+	}
+
+	// Nothing matched, use parent
+	return p.parent.EventRead(name)
+}
+
+// EventWrite is used to determine if new events can be created
+// (fired) by the policy.
+func (p *PolicyACL) EventWrite(name string) bool {
+	// Longest-prefix match event names
+	if _, rule, ok := p.eventRules.LongestPrefix(name); ok {
+		return rule == PolicyWrite
+	}
+
+	// No match, use parent
+	return p.parent.EventWrite(name)
+}
+
 // KeyRead returns if a key is allowed to be read
 func (p *PolicyACL) KeyRead(key string) bool {
 	// Look for a matching rule
@@ -327,10 +396,43 @@ func (p *PolicyACL) KeyWritePrefix(prefix string) bool {
 	return p.parent.KeyWritePrefix(prefix)
 }
 
-// ServiceRead checks if reading (discovery) of a service is allowed
+// KeyringRead is used to determine if the keyring can be
-func (p *PolicyACL) ServiceRead(name string) bool {
+// read by the current ACL token.
+func (p *PolicyACL) KeyringRead() bool {
+	switch p.keyringRule {
+	case PolicyRead, PolicyWrite:
+		return true
+	case PolicyDeny:
+		return false
+	default:
+		return p.parent.KeyringRead()
+	}
+}
+
+// KeyringWrite determines if the keyring can be manipulated.
+func (p *PolicyACL) KeyringWrite() bool {
+	if p.keyringRule == PolicyWrite {
+		return true
+	}
+	return p.parent.KeyringWrite()
+}
+
+// OperatorRead determines if the read-only operator functions are allowed.
+func (p *PolicyACL) OperatorRead() bool {
+	switch p.operatorRule {
+	case PolicyRead, PolicyWrite:
+		return true
+	case PolicyDeny:
+		return false
+	default:
+		return p.parent.OperatorRead()
+	}
+}
+
+// NodeRead checks if reading (discovery) of a node is allowed
+func (p *PolicyACL) NodeRead(name string) bool {
 	// Check for an exact rule or catch-all
-	_, rule, ok := p.serviceRules.LongestPrefix(name)
+	_, rule, ok := p.nodeRules.LongestPrefix(name)
 
 	if ok {
 		switch rule {
@@ -342,13 +444,13 @@ func (p *PolicyACL) ServiceRead(name string) bool {
 	}
 
 	// No matching rule, use the parent.
-	return p.parent.ServiceRead(name)
+	return p.parent.NodeRead(name)
 }
 
-// ServiceWrite checks if writing (registering) a service is allowed
+// NodeWrite checks if writing (registering) a node is allowed
-func (p *PolicyACL) ServiceWrite(name string) bool {
+func (p *PolicyACL) NodeWrite(name string) bool {
 	// Check for an exact rule or catch-all
-	_, rule, ok := p.serviceRules.LongestPrefix(name)
+	_, rule, ok := p.nodeRules.LongestPrefix(name)
 
 	if ok {
 		switch rule {
@@ -360,36 +462,16 @@ func (p *PolicyACL) ServiceWrite(name string) bool {
 	}
 
 	// No matching rule, use the parent.
-	return p.parent.ServiceWrite(name)
+	return p.parent.NodeWrite(name)
 }
 
-// EventRead is used to determine if the policy allows for a
+// OperatorWrite determines if the state-changing operator functions are
-// specific user event to be read.
+// allowed.
-func (p *PolicyACL) EventRead(name string) bool {
+func (p *PolicyACL) OperatorWrite() bool {
-	// Longest-prefix match on event names
+	if p.operatorRule == PolicyWrite {
-	if _, rule, ok := p.eventRules.LongestPrefix(name); ok {
+		return true
-		switch rule {
-		case PolicyRead, PolicyWrite:
-			return true
-		default:
-			return false
-		}
 	}
-
+	return p.parent.OperatorWrite()
-	// Nothing matched, use parent
-	return p.parent.EventRead(name)
-}
-
-// EventWrite is used to determine if new events can be created
-// (fired) by the policy.
-func (p *PolicyACL) EventWrite(name string) bool {
-	// Longest-prefix match event names
-	if _, rule, ok := p.eventRules.LongestPrefix(name); ok {
-		return rule == PolicyWrite
-	}
-
-	// No match, use parent
-	return p.parent.EventWrite(name)
 }
 
 // PreparedQueryRead checks if reading (listing) of a prepared query is
@@ -430,59 +512,38 @@ func (p *PolicyACL) PreparedQueryWrite(prefix string) bool {
 	return p.parent.PreparedQueryWrite(prefix)
 }
 
-// KeyringRead is used to determine if the keyring can be
+// ServiceRead checks if reading (discovery) of a service is allowed
-// read by the current ACL token.
+func (p *PolicyACL) ServiceRead(name string) bool {
-func (p *PolicyACL) KeyringRead() bool {
+	// Check for an exact rule or catch-all
-	switch p.keyringRule {
+	_, rule, ok := p.serviceRules.LongestPrefix(name)
-	case PolicyRead, PolicyWrite:
+
-		return true
+	if ok {
-	case PolicyDeny:
+		switch rule {
-		return false
+		case PolicyRead, PolicyWrite:
-	default:
+			return true
-		return p.parent.KeyringRead()
+		default:
+			return false
+		}
 	}
+
+	// No matching rule, use the parent.
+	return p.parent.ServiceRead(name)
 }
 
-// KeyringWrite determines if the keyring can be manipulated.
+// ServiceWrite checks if writing (registering) a service is allowed
-func (p *PolicyACL) KeyringWrite() bool {
+func (p *PolicyACL) ServiceWrite(name string) bool {
-	if p.keyringRule == PolicyWrite {
+	// Check for an exact rule or catch-all
-		return true
+	_, rule, ok := p.serviceRules.LongestPrefix(name)
+
+	if ok {
+		switch rule {
+		case PolicyWrite:
+			return true
+		default:
+			return false
+		}
 	}
-	return p.parent.KeyringWrite()
-}
 
-// OperatorRead determines if the read-only operator functions are allowed.
+	// No matching rule, use the parent.
-func (p *PolicyACL) OperatorRead() bool {
+	return p.parent.ServiceWrite(name)
-	switch p.operatorRule {
-	case PolicyRead, PolicyWrite:
-		return true
-	case PolicyDeny:
-		return false
-	default:
-		return p.parent.OperatorRead()
-	}
-}
-
-// OperatorWrite determines if the state-changing operator functions are
-// allowed.
-func (p *PolicyACL) OperatorWrite() bool {
-	if p.operatorRule == PolicyWrite {
-		return true
-	}
-	return p.parent.OperatorWrite()
-}
-
-// ACLList checks if listing of ACLs is allowed
-func (p *PolicyACL) ACLList() bool {
-	return p.parent.ACLList()
-}
-
-// ACLModify checks if modification of ACLs is allowed
-func (p *PolicyACL) ACLModify() bool {
-	return p.parent.ACLModify()
-}
-
-// Snapshot checks if taking and restoring snapshots is allowed.
-func (p *PolicyACL) Snapshot() bool {
-	return p.parent.Snapshot()
 }

acl/acl_test.go

@@ -35,17 +35,11 @@ func TestStaticACL(t *testing.T) {
 		t.Fatalf("expected static")
 	}
 
-	if !all.KeyRead("foobar") {
+	if all.ACLList() {
-		t.Fatalf("should allow")
+		t.Fatalf("should not allow")
 	}
-	if !all.KeyWrite("foobar") {
+	if all.ACLModify() {
-		t.Fatalf("should allow")
+		t.Fatalf("should not allow")
-	}
-	if !all.ServiceRead("foobar") {
-		t.Fatalf("should allow")
-	}
-	if !all.ServiceWrite("foobar") {
-		t.Fatalf("should allow")
 	}
 	if !all.EventRead("foobar") {
 		t.Fatalf("should allow")
@@ -53,10 +47,10 @@ func TestStaticACL(t *testing.T) {
 	if !all.EventWrite("foobar") {
 		t.Fatalf("should allow")
 	}
-	if !all.PreparedQueryRead("foobar") {
+	if !all.KeyRead("foobar") {
 		t.Fatalf("should allow")
 	}
-	if !all.PreparedQueryWrite("foobar") {
+	if !all.KeyWrite("foobar") {
 		t.Fatalf("should allow")
 	}
 	if !all.KeyringRead() {
@@ -65,32 +59,38 @@ func TestStaticACL(t *testing.T) {
 	if !all.KeyringWrite() {
 		t.Fatalf("should allow")
 	}
+	if !all.NodeRead("foobar") {
+		t.Fatalf("should allow")
+	}
+	if !all.NodeWrite("foobar") {
+		t.Fatalf("should allow")
+	}
 	if !all.OperatorRead() {
 		t.Fatalf("should allow")
 	}
 	if !all.OperatorWrite() {
 		t.Fatalf("should allow")
 	}
-	if all.ACLList() {
+	if !all.PreparedQueryRead("foobar") {
-		t.Fatalf("should not allow")
+		t.Fatalf("should allow")
 	}
-	if all.ACLModify() {
+	if !all.PreparedQueryWrite("foobar") {
-		t.Fatalf("should not allow")
+		t.Fatalf("should allow")
+	}
+	if !all.ServiceRead("foobar") {
+		t.Fatalf("should allow")
+	}
+	if !all.ServiceWrite("foobar") {
+		t.Fatalf("should allow")
 	}
 	if all.Snapshot() {
 		t.Fatalf("should not allow")
 	}
 
-	if none.KeyRead("foobar") {
+	if none.ACLList() {
 		t.Fatalf("should not allow")
 	}
-	if none.KeyWrite("foobar") {
+	if none.ACLModify() {
-		t.Fatalf("should not allow")
-	}
-	if none.ServiceRead("foobar") {
-		t.Fatalf("should not allow")
-	}
-	if none.ServiceWrite("foobar") {
 		t.Fatalf("should not allow")
 	}
 	if none.EventRead("foobar") {
@@ -105,10 +105,10 @@ func TestStaticACL(t *testing.T) {
 	if none.EventWrite("") {
 		t.Fatalf("should not allow")
 	}
-	if none.PreparedQueryRead("foobar") {
+	if none.KeyRead("foobar") {
 		t.Fatalf("should not allow")
 	}
-	if none.PreparedQueryWrite("foobar") {
+	if none.KeyWrite("foobar") {
 		t.Fatalf("should not allow")
 	}
 	if none.KeyringRead() {
@@ -117,32 +117,38 @@ func TestStaticACL(t *testing.T) {
 	if none.KeyringWrite() {
 		t.Fatalf("should not allow")
 	}
+	if none.NodeRead("foobar") {
+		t.Fatalf("should not allow")
+	}
+	if none.NodeWrite("foobar") {
+		t.Fatalf("should not allow")
+	}
 	if none.OperatorRead() {
 		t.Fatalf("should now allow")
 	}
 	if none.OperatorWrite() {
 		t.Fatalf("should not allow")
 	}
-	if none.ACLList() {
+	if none.PreparedQueryRead("foobar") {
 		t.Fatalf("should not allow")
 	}
-	if none.ACLModify() {
+	if none.PreparedQueryWrite("foobar") {
+		t.Fatalf("should not allow")
+	}
+	if none.ServiceRead("foobar") {
+		t.Fatalf("should not allow")
+	}
+	if none.ServiceWrite("foobar") {
 		t.Fatalf("should not allow")
 	}
 	if none.Snapshot() {
 		t.Fatalf("should not allow")
 	}
 
-	if !manage.KeyRead("foobar") {
+	if !manage.ACLList() {
 		t.Fatalf("should allow")
 	}
-	if !manage.KeyWrite("foobar") {
+	if !manage.ACLModify() {
-		t.Fatalf("should allow")
-	}
-	if !manage.ServiceRead("foobar") {
-		t.Fatalf("should allow")
-	}
-	if !manage.ServiceWrite("foobar") {
 		t.Fatalf("should allow")
 	}
 	if !manage.EventRead("foobar") {
@@ -151,10 +157,10 @@ func TestStaticACL(t *testing.T) {
 	if !manage.EventWrite("foobar") {
 		t.Fatalf("should allow")
 	}
-	if !manage.PreparedQueryRead("foobar") {
+	if !manage.KeyRead("foobar") {
 		t.Fatalf("should allow")
 	}
-	if !manage.PreparedQueryWrite("foobar") {
+	if !manage.KeyWrite("foobar") {
 		t.Fatalf("should allow")
 	}
 	if !manage.KeyringRead() {
@@ -163,16 +169,28 @@ func TestStaticACL(t *testing.T) {
 	if !manage.KeyringWrite() {
 		t.Fatalf("should allow")
 	}
+	if !manage.NodeRead("foobar") {
+		t.Fatalf("should allow")
+	}
+	if !manage.NodeWrite("foobar") {
+		t.Fatalf("should allow")
+	}
 	if !manage.OperatorRead() {
 		t.Fatalf("should allow")
 	}
 	if !manage.OperatorWrite() {
 		t.Fatalf("should allow")
 	}
-	if !manage.ACLList() {
+	if !manage.PreparedQueryRead("foobar") {
 		t.Fatalf("should allow")
 	}
-	if !manage.ACLModify() {
+	if !manage.PreparedQueryWrite("foobar") {
+		t.Fatalf("should allow")
+	}
+	if !manage.ServiceRead("foobar") {
+		t.Fatalf("should allow")
+	}
+	if !manage.ServiceWrite("foobar") {
 		t.Fatalf("should allow")
 	}
 	if !manage.Snapshot() {
@@ -183,6 +201,20 @@ func TestStaticACL(t *testing.T) {
 func TestPolicyACL(t *testing.T) {
 	all := AllowAll()
 	policy := &Policy{
+		Events: []*EventPolicy{
+			&EventPolicy{
+				Event:  "",
+				Policy: PolicyRead,
+			},
+			&EventPolicy{
+				Event:  "foo",
+				Policy: PolicyWrite,
+			},
+			&EventPolicy{
+				Event:  "bar",
+				Policy: PolicyDeny,
+			},
+		},
 		Keys: []*KeyPolicy{
 			&KeyPolicy{
 				Prefix: "foo/",
@@ -201,38 +233,6 @@ func TestPolicyACL(t *testing.T) {
 				Policy: PolicyRead,
 			},
 		},
-		Services: []*ServicePolicy{
-			&ServicePolicy{
-				Name:   "",
-				Policy: PolicyWrite,
-			},
-			&ServicePolicy{
-				Name:   "foo",
-				Policy: PolicyRead,
-			},
-			&ServicePolicy{
-				Name:   "bar",
-				Policy: PolicyDeny,
-			},
-			&ServicePolicy{
-				Name:   "barfoo",
-				Policy: PolicyWrite,
-			},
-		},
-		Events: []*EventPolicy{
-			&EventPolicy{
-				Event:  "",
-				Policy: PolicyRead,
-			},
-			&EventPolicy{
-				Event:  "foo",
-				Policy: PolicyWrite,
-			},
-			&EventPolicy{
-				Event:  "bar",
-				Policy: PolicyDeny,
-			},
-		},
 		PreparedQueries: []*PreparedQueryPolicy{
 			&PreparedQueryPolicy{
 				Prefix: "",
@@ -251,6 +251,24 @@ func TestPolicyACL(t *testing.T) {
 				Policy: PolicyWrite,
 			},
 		},
+		Services: []*ServicePolicy{
+			&ServicePolicy{
+				Name:   "",
+				Policy: PolicyWrite,
+			},
+			&ServicePolicy{
+				Name:   "foo",
+				Policy: PolicyRead,
+			},
+			&ServicePolicy{
+				Name:   "bar",
+				Policy: PolicyDeny,
+			},
+			&ServicePolicy{
+				Name:   "barfoo",
+				Policy: PolicyWrite,
+			},
+		},
 	}
 	acl, err := New(all, policy)
 	if err != nil {
@@ -369,16 +387,6 @@ func TestPolicyACL_Parent(t *testing.T) {
 				Policy: PolicyRead,
 			},
 		},
-		Services: []*ServicePolicy{
-			&ServicePolicy{
-				Name:   "other",
-				Policy: PolicyWrite,
-			},
-			&ServicePolicy{
-				Name:   "foo",
-				Policy: PolicyRead,
-			},
-		},
 		PreparedQueries: []*PreparedQueryPolicy{
 			&PreparedQueryPolicy{
 				Prefix: "other",
@@ -389,6 +397,16 @@ func TestPolicyACL_Parent(t *testing.T) {
 				Policy: PolicyRead,
 			},
 		},
+		Services: []*ServicePolicy{
+			&ServicePolicy{
+				Name:   "other",
+				Policy: PolicyWrite,
+			},
+			&ServicePolicy{
+				Name:   "foo",
+				Policy: PolicyRead,
+			},
+		},
 	}
 	root, err := New(deny, policyRoot)
 	if err != nil {
@@ -410,18 +428,18 @@ func TestPolicyACL_Parent(t *testing.T) {
 				Policy: PolicyRead,
 			},
 		},
-		Services: []*ServicePolicy{
-			&ServicePolicy{
-				Name:   "bar",
-				Policy: PolicyDeny,
-			},
-		},
 		PreparedQueries: []*PreparedQueryPolicy{
 			&PreparedQueryPolicy{
 				Prefix: "bar",
 				Policy: PolicyDeny,
 			},
 		},
+		Services: []*ServicePolicy{
+			&ServicePolicy{
+				Name:   "bar",
+				Policy: PolicyDeny,
+			},
+		},
 	}
 	acl, err := New(root, policy)
 	if err != nil {
@@ -560,3 +578,86 @@ func TestPolicyACL_Operator(t *testing.T) {
 		}
 	}
 }
+
+func TestPolicyACL_Node(t *testing.T) {
+	deny := DenyAll()
+	policyRoot := &Policy{
+		Nodes: []*NodePolicy{
+			&NodePolicy{
+				Name:   "root-nope",
+				Policy: PolicyDeny,
+			},
+			&NodePolicy{
+				Name:   "root-ro",
+				Policy: PolicyRead,
+			},
+			&NodePolicy{
+				Name:   "root-rw",
+				Policy: PolicyWrite,
+			},
+			&NodePolicy{
+				Name:   "override",
+				Policy: PolicyDeny,
+			},
+		},
+	}
+	root, err := New(deny, policyRoot)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	policy := &Policy{
+		Nodes: []*NodePolicy{
+			&NodePolicy{
+				Name:   "child-nope",
+				Policy: PolicyDeny,
+			},
+			&NodePolicy{
+				Name:   "child-ro",
+				Policy: PolicyRead,
+			},
+			&NodePolicy{
+				Name:   "child-rw",
+				Policy: PolicyWrite,
+			},
+			&NodePolicy{
+				Name:   "override",
+				Policy: PolicyWrite,
+			},
+		},
+	}
+	acl, err := New(root, policy)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	type nodecase struct {
+		inp   string
+		read  bool
+		write bool
+	}
+	cases := []nodecase{
+		{"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.NodeRead(c.inp) {
+			t.Fatalf("Read fail: %#v", c)
+		}
+		if c.write != acl.NodeWrite(c.inp) {
+			t.Fatalf("Write fail: %#v", c)
+		}
+	}
+}

acl/policy.go

@@ -17,6 +17,7 @@ const (
 type Policy struct {
 	ID              string                 `hcl:"-"`
 	Keys            []*KeyPolicy           `hcl:"key,expand"`
+	Nodes           []*NodePolicy          `hcl:"node,expand"`
 	Services        []*ServicePolicy       `hcl:"service,expand"`
 	Events          []*EventPolicy         `hcl:"event,expand"`
 	PreparedQueries []*PreparedQueryPolicy `hcl:"query,expand"`
@@ -34,14 +35,24 @@ func (k *KeyPolicy) GoString() string {
 	return fmt.Sprintf("%#v", *k)
 }
 
+// NodePolicy represents a policy for a node
+type NodePolicy struct {
+	Name   string `hcl:",key"`
+	Policy string
+}
+
+func (n *NodePolicy) GoString() string {
+	return fmt.Sprintf("%#v", *n)
+}
+
 // ServicePolicy represents a policy for a service
 type ServicePolicy struct {
 	Name   string `hcl:",key"`
 	Policy string
 }
 
-func (k *ServicePolicy) GoString() string {
+func (s *ServicePolicy) GoString() string {
-	return fmt.Sprintf("%#v", *k)
+	return fmt.Sprintf("%#v", *s)
 }
 
 // EventPolicy represents a user event policy.
@@ -60,8 +71,8 @@ type PreparedQueryPolicy struct {
 	Policy string
 }
 
-func (e *PreparedQueryPolicy) GoString() string {
+func (p *PreparedQueryPolicy) GoString() string {
-	return fmt.Sprintf("%#v", *e)
+	return fmt.Sprintf("%#v", *p)
 }
 
 // isPolicyValid makes sure the given string matches one of the valid policies.
@@ -100,7 +111,14 @@ func Parse(rules string) (*Policy, error) {
 		}
 	}
 
-	// Validate the service policy
+	// Validate the node policies
+	for _, np := range p.Nodes {
+		if !isPolicyValid(np.Policy) {
+			return nil, fmt.Errorf("Invalid node policy: %#v", np)
+		}
+	}
+
+	// Validate the service policies
 	for _, sp := range p.Services {
 		if !isPolicyValid(sp.Policy) {
 			return nil, fmt.Errorf("Invalid service policy: %#v", sp)

acl/policy_test.go

@@ -8,6 +8,15 @@ import (
 
 func TestACLPolicy_Parse_HCL(t *testing.T) {
 	inp := `
+event "" {
+	policy = "read"
+}
+event "foo" {
+	policy = "write"
+}
+event "bar" {
+	policy = "deny"
+}
 key "" {
 	policy = "read"
 }
@@ -20,21 +29,23 @@ key "foo/bar/" {
 key "foo/bar/baz" {
 	policy = "deny"
 }
+keyring = "deny"
+node "" {
+	policy = "read"
+}
+node "foo" {
+	policy = "write"
+}
+node "bar" {
+	policy = "deny"
+}
+operator = "deny"
 service "" {
 	policy = "write"
 }
 service "foo" {
 	policy = "read"
 }
-event "" {
-	policy = "read"
-}
-event "foo" {
-	policy = "write"
-}
-event "bar" {
-	policy = "deny"
-}
 query "" {
 	policy = "read"
 }
@@ -44,10 +55,23 @@ query "foo" {
 query "bar" {
 	policy = "deny"
 }
-keyring = "deny"
-operator = "deny"
 	`
 	exp := &Policy{
+		Events: []*EventPolicy{
+			&EventPolicy{
+				Event:  "",
+				Policy: PolicyRead,
+			},
+			&EventPolicy{
+				Event:  "foo",
+				Policy: PolicyWrite,
+			},
+			&EventPolicy{
+				Event:  "bar",
+				Policy: PolicyDeny,
+			},
+		},
+		Keyring: PolicyDeny,
 		Keys: []*KeyPolicy{
 			&KeyPolicy{
 				Prefix: "",
@@ -66,30 +90,21 @@ operator = "deny"
 				Policy: PolicyDeny,
 			},
 		},
-		Services: []*ServicePolicy{
+		Nodes: []*NodePolicy{
-			&ServicePolicy{
+			&NodePolicy{
 				Name:   "",
-				Policy: PolicyWrite,
+				Policy: PolicyRead,
 			},
-			&ServicePolicy{
+			&NodePolicy{
 				Name:   "foo",
-				Policy: PolicyRead,
-			},
-		},
-		Events: []*EventPolicy{
-			&EventPolicy{
-				Event:  "",
-				Policy: PolicyRead,
-			},
-			&EventPolicy{
-				Event:  "foo",
 				Policy: PolicyWrite,
 			},
-			&EventPolicy{
+			&NodePolicy{
-				Event:  "bar",
+				Name:   "bar",
 				Policy: PolicyDeny,
 			},
 		},
+		Operator: PolicyDeny,
 		PreparedQueries: []*PreparedQueryPolicy{
 			&PreparedQueryPolicy{
 				Prefix: "",
@@ -104,8 +119,16 @@ operator = "deny"
 				Policy: PolicyDeny,
 			},
 		},
-		Keyring:  PolicyDeny,
+		Services: []*ServicePolicy{
-		Operator: PolicyDeny,
+			&ServicePolicy{
+				Name:   "",
+				Policy: PolicyWrite,
+			},
+			&ServicePolicy{
+				Name:   "foo",
+				Policy: PolicyRead,
+			},
+		},
 	}
 
 	out, err := Parse(inp)
@@ -120,6 +143,17 @@ operator = "deny"
 
 func TestACLPolicy_Parse_JSON(t *testing.T) {
 	inp := `{
+	"event": {
+		"": {
+			"policy": "read"
+		},
+		"foo": {
+			"policy": "write"
+		},
+		"bar": {
+			"policy": "deny"
+		}
+	},
 	"key": {
 		"": {
 			"policy": "read"
@@ -134,15 +168,8 @@ func TestACLPolicy_Parse_JSON(t *testing.T) {
 			"policy": "deny"
 		}
 	},
-	"service": {
+	"keyring": "deny",
-		"": {
+	"node": {
-			"policy": "write"
-		},
-		"foo": {
-			"policy": "read"
-		}
-	},
-	"event": {
 		"": {
 			"policy": "read"
 		},
@@ -153,6 +180,7 @@ func TestACLPolicy_Parse_JSON(t *testing.T) {
 			"policy": "deny"
 		}
 	},
+	"operator": "deny",
 	"query": {
 		"": {
 			"policy": "read"
@@ -164,10 +192,31 @@ func TestACLPolicy_Parse_JSON(t *testing.T) {
 			"policy": "deny"
 		}
 	},
-	"keyring": "deny",
+	"service": {
-	"operator": "deny"
+		"": {
+			"policy": "write"
+		},
+		"foo": {
+			"policy": "read"
+		}
+	}
 }`
 	exp := &Policy{
+		Events: []*EventPolicy{
+			&EventPolicy{
+				Event:  "",
+				Policy: PolicyRead,
+			},
+			&EventPolicy{
+				Event:  "foo",
+				Policy: PolicyWrite,
+			},
+			&EventPolicy{
+				Event:  "bar",
+				Policy: PolicyDeny,
+			},
+		},
+		Keyring: PolicyDeny,
 		Keys: []*KeyPolicy{
 			&KeyPolicy{
 				Prefix: "",
@@ -186,30 +235,21 @@ func TestACLPolicy_Parse_JSON(t *testing.T) {
 				Policy: PolicyDeny,
 			},
 		},
-		Services: []*ServicePolicy{
+		Nodes: []*NodePolicy{
-			&ServicePolicy{
+			&NodePolicy{
 				Name:   "",
-				Policy: PolicyWrite,
+				Policy: PolicyRead,
 			},
-			&ServicePolicy{
+			&NodePolicy{
 				Name:   "foo",
-				Policy: PolicyRead,
-			},
-		},
-		Events: []*EventPolicy{
-			&EventPolicy{
-				Event:  "",
-				Policy: PolicyRead,
-			},
-			&EventPolicy{
-				Event:  "foo",
 				Policy: PolicyWrite,
 			},
-			&EventPolicy{
+			&NodePolicy{
-				Event:  "bar",
+				Name:   "bar",
 				Policy: PolicyDeny,
 			},
 		},
+		Operator: PolicyDeny,
 		PreparedQueries: []*PreparedQueryPolicy{
 			&PreparedQueryPolicy{
 				Prefix: "",
@@ -224,8 +264,16 @@ func TestACLPolicy_Parse_JSON(t *testing.T) {
 				Policy: PolicyDeny,
 			},
 		},
-		Keyring:  PolicyDeny,
+		Services: []*ServicePolicy{
-		Operator: PolicyDeny,
+			&ServicePolicy{
+				Name:   "",
+				Policy: PolicyWrite,
+			},
+			&ServicePolicy{
+				Name:   "foo",
+				Policy: PolicyRead,
+			},
+		},
 	}
 
 	out, err := Parse(inp)
@@ -276,12 +324,13 @@ operator = ""
 
 func TestACLPolicy_Bad_Policy(t *testing.T) {
 	cases := []string{
-		`key "" { policy = "nope" }`,
-		`service "" { policy = "nope" }`,
 		`event "" { policy = "nope" }`,
-		`query "" { policy = "nope" }`,
+		`key "" { policy = "nope" }`,
 		`keyring = "nope"`,
+		`node "" { policy = "nope" }`,
 		`operator = "nope"`,
+		`query "" { policy = "nope" }`,
+		`service "" { policy = "nope" }`,
 	}
 	for _, c := range cases {
 		_, err := Parse(c)

command/agent/agent.go

@@ -227,6 +227,7 @@ func Create(config *Config, logOutput io.Writer, logWriter *logger.LogWriter,
 			Port:    agent.config.Ports.Server,
 			Tags:    []string{},
 		}
+		// TODO (slackpad) - Plumb the "acl_agent_token" into here.
 		agent.state.AddService(&consulService, "")
 	} else {
 		err = agent.setupClient()
@@ -381,6 +382,9 @@ func (a *Agent) consulConfig() *consul.Config {
 	if a.config.ACLReplicationToken != "" {
 		base.ACLReplicationToken = a.config.ACLReplicationToken
 	}
+	if a.config.ACLEnforceVersion8 != nil {
+		base.ACLEnforceVersion8 = *a.config.ACLEnforceVersion8
+	}
 	if a.config.SessionTTLMinRaw != "" {
 		base.SessionTTLMin = a.config.SessionTTLMin
 	}

command/agent/config.go

@@ -525,6 +525,10 @@ type Config struct {
 	// other than the ACLDatacenter.
 	ACLReplicationToken string `mapstructure:"acl_replication_token" json:"-"`
 
+	// ACLEnforceVersion8 is used to gate a set of ACL policy features that
+	// are opt-in prior to Consul 0.8 and opt-out in Consul 0.8 and later.
+	ACLEnforceVersion8 *bool `mapstructure:"acl_enforce_version_8"`
+
 	// Watches are used to monitor various endpoints and to invoke a
 	// handler to act appropriately. These are managed entirely in the
 	// agent layer using the standard APIs.
@@ -705,11 +709,12 @@ func DefaultConfig() *Config {
 		SyncCoordinateRateTarget:  64.0, // updates / second
 		SyncCoordinateIntervalMin: 15 * time.Second,
 
-		ACLTTL:           30 * time.Second,
+		ACLTTL:             30 * time.Second,
-		ACLDownPolicy:    "extend-cache",
+		ACLDownPolicy:      "extend-cache",
-		ACLDefaultPolicy: "allow",
+		ACLDefaultPolicy:   "allow",
-		RetryInterval:    30 * time.Second,
+		ACLEnforceVersion8: Bool(false),
-		RetryIntervalWan: 30 * time.Second,
+		RetryInterval:      30 * time.Second,
+		RetryIntervalWan:   30 * time.Second,
 	}
 }
 
@@ -1480,6 +1485,9 @@ func MergeConfig(a, b *Config) *Config {
 	if b.ACLReplicationToken != "" {
 		result.ACLReplicationToken = b.ACLReplicationToken
 	}
+	if b.ACLEnforceVersion8 != nil {
+		result.ACLEnforceVersion8 = b.ACLEnforceVersion8
+	}
 	if len(b.Watches) != 0 {
 		result.Watches = append(result.Watches, b.Watches...)
 	}

command/agent/config_test.go

@@ -674,6 +674,22 @@ func TestDecodeConfig(t *testing.T) {
 		t.Fatalf("bad: %#v", config)
 	}
 
+	// ACL flag for Consul version 0.8 features (broken out since we will
+	// eventually remove this). We first verify this is opt-out.
+	config = DefaultConfig()
+	if *config.ACLEnforceVersion8 != false {
+		t.Fatalf("bad: %#v", config)
+	}
+
+	input = `{"acl_enforce_version_8": true}`
+	config, err = DecodeConfig(bytes.NewReader([]byte(input)))
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if *config.ACLEnforceVersion8 != true {
+		t.Fatalf("bad: %#v", config)
+	}
+
 	// Watches
 	input = `{"watches": [{"type":"keyprefix", "prefix":"foo/", "handler":"foobar"}]}`
 	config, err = DecodeConfig(bytes.NewReader([]byte(input)))
@@ -1552,6 +1568,7 @@ func TestMergeConfig(t *testing.T) {
 		ACLDownPolicy:          "deny",
 		ACLDefaultPolicy:       "deny",
 		ACLReplicationToken:    "8765309",
+		ACLEnforceVersion8:     Bool(true),
 		Watches: []map[string]interface{}{
 			map[string]interface{}{
 				"type":    "keyprefix",

consul/acl.go

@@ -311,20 +311,33 @@ func (c *aclCache) useACLPolicy(id, authDC string, cached *aclCacheEntry, p *str
 // aclFilter is used to filter results from our state store based on ACL rules
 // configured for the provided token.
 type aclFilter struct {
-	acl    acl.ACL
+	acl             acl.ACL
-	logger *log.Logger
+	logger          *log.Logger
+	enforceVersion8 bool
 }
 
 // newAclFilter constructs a new aclFilter.
-func newAclFilter(acl acl.ACL, logger *log.Logger) *aclFilter {
+func newAclFilter(acl acl.ACL, logger *log.Logger, enforceVersion8 bool) *aclFilter {
 	if logger == nil {
 		logger = log.New(os.Stdout, "", log.LstdFlags)
 	}
-	return &aclFilter{acl, logger}
+	return &aclFilter{
+		acl:             acl,
+		logger:          logger,
+		enforceVersion8: enforceVersion8,
+	}
 }
 
-// filterService is used to determine if a service is accessible for an ACL.
+// allowNode is used to determine if a node is accessible for an ACL.
-func (f *aclFilter) filterService(service string) bool {
+func (f *aclFilter) allowNode(node string) bool {
+	if !f.enforceVersion8 {
+		return true
+	}
+	return f.acl.NodeRead(node)
+}
+
+// allowService is used to determine if a service is accessible for an ACL.
+func (f *aclFilter) allowService(service string) bool {
 	if service == "" || service == ConsulServiceID {
 		return true
 	}
@@ -337,7 +350,7 @@ func (f *aclFilter) filterHealthChecks(checks *structs.HealthChecks) {
 	hc := *checks
 	for i := 0; i < len(hc); i++ {
 		check := hc[i]
-		if f.filterService(check.ServiceName) {
+		if f.allowService(check.ServiceName) {
 			continue
 		}
 		f.logger.Printf("[DEBUG] consul: dropping check %q from result due to ACLs", check.CheckID)
@@ -350,7 +363,7 @@ func (f *aclFilter) filterHealthChecks(checks *structs.HealthChecks) {
 // filterServices is used to filter a set of services based on ACLs.
 func (f *aclFilter) filterServices(services structs.Services) {
 	for svc, _ := range services {
-		if f.filterService(svc) {
+		if f.allowService(svc) {
 			continue
 		}
 		f.logger.Printf("[DEBUG] consul: dropping service %q from result due to ACLs", svc)
@@ -364,7 +377,7 @@ func (f *aclFilter) filterServiceNodes(nodes *structs.ServiceNodes) {
 	sn := *nodes
 	for i := 0; i < len(sn); i++ {
 		node := sn[i]
-		if f.filterService(node.ServiceName) {
+		if f.allowNode(node.Node) && f.allowService(node.ServiceName) {
 			continue
 		}
 		f.logger.Printf("[DEBUG] consul: dropping node %q from result due to ACLs", node.Node)
@@ -377,7 +390,7 @@ func (f *aclFilter) filterServiceNodes(nodes *structs.ServiceNodes) {
 // filterNodeServices is used to filter services on a given node base on ACLs.
 func (f *aclFilter) filterNodeServices(services *structs.NodeServices) {
 	for svc, _ := range services.Services {
-		if f.filterService(svc) {
+		if f.allowService(svc) {
 			continue
 		}
 		f.logger.Printf("[DEBUG] consul: dropping service %q from result due to ACLs", svc)
@@ -390,7 +403,7 @@ func (f *aclFilter) filterCheckServiceNodes(nodes *structs.CheckServiceNodes) {
 	csn := *nodes
 	for i := 0; i < len(csn); i++ {
 		node := csn[i]
-		if f.filterService(node.Service.Service) {
+		if f.allowService(node.Service.Service) {
 			continue
 		}
 		f.logger.Printf("[DEBUG] consul: dropping node %q from result due to ACLs", node.Node.Node)
@@ -410,7 +423,7 @@ func (f *aclFilter) filterNodeDump(dump *structs.NodeDump) {
 		// Filter services
 		for i := 0; i < len(info.Services); i++ {
 			svc := info.Services[i].Service
-			if f.filterService(svc) {
+			if f.allowService(svc) {
 				continue
 			}
 			f.logger.Printf("[DEBUG] consul: dropping service %q from result due to ACLs", svc)
@@ -421,7 +434,7 @@ func (f *aclFilter) filterNodeDump(dump *structs.NodeDump) {
 		// Filter checks
 		for i := 0; i < len(info.Checks); i++ {
 			chk := info.Checks[i]
-			if f.filterService(chk.ServiceName) {
+			if f.allowService(chk.ServiceName) {
 				continue
 			}
 			f.logger.Printf("[DEBUG] consul: dropping check %q from result due to ACLs", chk.CheckID)
@@ -432,6 +445,26 @@ func (f *aclFilter) filterNodeDump(dump *structs.NodeDump) {
 	*dump = nd
 }
 
+// filterNodes is used to filter through all parts of a node list and remove
+// elements the provided ACL token cannot access.
+func (f *aclFilter) filterNodes(nodes *structs.Nodes) {
+	if !f.enforceVersion8 {
+		return
+	}
+
+	n := *nodes
+	for i := 0; i < len(n); i++ {
+		node := n[i].Node
+		if f.acl.NodeRead(node) {
+			continue
+		}
+		f.logger.Printf("[DEBUG] consul: dropping node %q from result due to ACLs", node)
+		n = append(n[:i], n[i+1:]...)
+		i--
+	}
+	*nodes = n
+}
+
 // redactPreparedQueryTokens will redact any tokens unless the client has a
 // management token. This eases the transition to delegated authority over
 // prepared queries, since it was easy to capture management tokens in Consul
@@ -506,31 +539,34 @@ func (s *Server) filterACL(token string, subj interface{}) error {
 	}
 
 	// Create the filter
-	filt := newAclFilter(acl, s.logger)
+	filt := newAclFilter(acl, s.logger, s.config.ACLEnforceVersion8)
 
 	switch v := subj.(type) {
+	case *structs.CheckServiceNodes:
+		filt.filterCheckServiceNodes(v)
+
+	case *structs.IndexedCheckServiceNodes:
+		filt.filterCheckServiceNodes(&v.Nodes)
+
 	case *structs.IndexedHealthChecks:
 		filt.filterHealthChecks(&v.HealthChecks)
 
-	case *structs.IndexedServices:
+	case *structs.IndexedNodeDump:
-		filt.filterServices(v.Services)
+		filt.filterNodeDump(&v.Dump)
 
-	case *structs.IndexedServiceNodes:
+	case *structs.IndexedNodes:
-		filt.filterServiceNodes(&v.ServiceNodes)
+		filt.filterNodes(&v.Nodes)
 
 	case *structs.IndexedNodeServices:
 		if v.NodeServices != nil {
 			filt.filterNodeServices(v.NodeServices)
 		}
 
-	case *structs.IndexedCheckServiceNodes:
+	case *structs.IndexedServiceNodes:
-		filt.filterCheckServiceNodes(&v.Nodes)
+		filt.filterServiceNodes(&v.ServiceNodes)
 
-	case *structs.CheckServiceNodes:
+	case *structs.IndexedServices:
-		filt.filterCheckServiceNodes(v)
+		filt.filterServices(v.Services)
-
-	case *structs.IndexedNodeDump:
-		filt.filterNodeDump(&v.Dump)
 
 	case *structs.IndexedPreparedQueries:
 		filt.filterPreparedQueries(&v.Queries)
@@ -544,3 +580,149 @@ func (s *Server) filterACL(token string, subj interface{}) error {
 
 	return nil
 }
+
+// vetRegisterWithACL applies the given ACL's policy to the catalog update and
+// determines if it is allowed. Since the catalog register request is so
+// dynamic, this is a pretty complex algorithm and was worth breaking out of the
+// endpoint. The NodeServices record for the node must be supplied, and can be
+// nil.
+//
+// This is a bit racy because we have to check the state store outside of a
+// transaction. It's the best we can do because we don't want to flow ACL
+// checking down there. The node information doesn't change in practice, so this
+// will be fine. If we expose ways to change node addresses in a later version,
+// then we should split the catalog API at the node and service level so we can
+// address this race better (even then it would be super rare, and would at
+// worst let a service update revert a recent node update, so it doesn't open up
+// too much abuse).
+func vetRegisterWithACL(acl acl.ACL, subj *structs.RegisterRequest,
+	ns *structs.NodeServices) error {
+	// Fast path if ACLs are not enabled.
+	if acl == nil {
+		return nil
+	}
+
+	// Vet the node info. This allows service updates to re-post the required
+	// node info for each request without having to have node "write"
+	// privileges.
+	needsNode := ns == nil || subj.ChangesNode(ns.Node)
+	if needsNode && !acl.NodeWrite(subj.Node) {
+		return permissionDeniedErr
+	}
+
+	// Vet the service change. This includes making sure they can register
+	// the given service, and that we can write to any existing service that
+	// is being modified by id (if any).
+	if subj.Service != nil {
+		if !acl.ServiceWrite(subj.Service.Service) {
+			return permissionDeniedErr
+		}
+
+		if ns != nil {
+			other, ok := ns.Services[subj.Service.ID]
+			if ok && !acl.ServiceWrite(other.Service) {
+				return permissionDeniedErr
+			}
+		}
+	}
+
+	// Make sure that the member was flattened before we got there. This
+	// keeps us from having to verify this check as well.
+	if subj.Check != nil {
+		return fmt.Errorf("check member must be nil")
+	}
+
+	// Vet the checks. Node-level checks require node write, and
+	// service-level checks require service write.
+	for _, check := range subj.Checks {
+		// Make sure that the node matches - we don't allow you to mix
+		// checks from other nodes because we'd have to pull a bunch
+		// more state store data to check this. If ACLs are enabled then
+		// we simply require them to match in a given request. There's a
+		// note in state_store.go to ban this down there in Consul 0.8,
+		// but it's good to leave this here because it's required for
+		// correctness wrt. ACLs.
+		if check.Node != subj.Node {
+			return fmt.Errorf("Node '%s' for check '%s' doesn't match register request node '%s'",
+				check.Node, check.CheckID, subj.Node)
+		}
+
+		// Node-level check.
+		if check.ServiceID == "" {
+			if !acl.NodeWrite(subj.Node) {
+				return permissionDeniedErr
+			}
+			continue
+		}
+
+		// Service-level check, check the common case where it
+		// matches the service part of this request, which has
+		// already been vetted above, and might be being registered
+		// along with its checks.
+		if subj.Service != nil && subj.Service.ID == check.ServiceID {
+			continue
+		}
+
+		// Service-level check for some other service. Make sure they've
+		// got write permissions for that service.
+		if ns == nil {
+			return fmt.Errorf("Unknown service '%s' for check '%s'",
+				check.ServiceID, check.CheckID)
+		} else {
+			other, ok := ns.Services[check.ServiceID]
+			if !ok {
+				return fmt.Errorf("Unknown service '%s' for check '%s'",
+					check.ServiceID, check.CheckID)
+			}
+			if !acl.ServiceWrite(other.Service) {
+				return permissionDeniedErr
+			}
+		}
+	}
+
+	return nil
+}
+
+// vetDeregisterWithACL applies the given ACL's policy to the catalog update and
+// determines if it is allowed. Since the catalog deregister request is so
+// dynamic, this is a pretty complex algorithm and was worth breaking out of the
+// endpoint. The NodeService for the referenced service must be supplied, and can
+// be nil; similar for the HealthCheck for the referenced health check.
+func vetDeregisterWithACL(acl acl.ACL, subj *structs.DeregisterRequest,
+	ns *structs.NodeService, nc *structs.HealthCheck) error {
+	// Fast path if ACLs are not enabled.
+	if acl == nil {
+		return nil
+	}
+
+	// This order must match the code in applyRegister() in fsm.go since it
+	// also evaluates things in this order, and will ignore fields based on
+	// this precedence. This lets us also ignore them from an ACL perspective.
+	if subj.ServiceID != "" {
+		if ns == nil {
+			return fmt.Errorf("Unknown service '%s'", subj.ServiceID)
+		}
+		if !acl.ServiceWrite(ns.Service) {
+			return permissionDeniedErr
+		}
+	} else if subj.CheckID != "" {
+		if nc == nil {
+			return fmt.Errorf("Unknown check '%s'", subj.CheckID)
+		}
+		if nc.ServiceID != "" {
+			if !acl.ServiceWrite(nc.ServiceName) {
+				return permissionDeniedErr
+			}
+		} else {
+			if !acl.NodeWrite(subj.Node) {
+				return permissionDeniedErr
+			}
+		}
+	} else {
+		if !acl.NodeWrite(subj.Node) {
+			return permissionDeniedErr
+		}
+	}
+
+	return nil
+}

consul/acl_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/hashicorp/consul/acl"
@@ -12,6 +13,15 @@ import (
 	"github.com/hashicorp/consul/testutil"
 )
 
+var testACLPolicy = `
+key "" {
+	policy = "deny"
+}
+key "foo/" {
+	policy = "write"
+}
+`
+
 func TestACL_Disabled(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
@@ -808,14 +818,14 @@ func TestACL_filterHealthChecks(t *testing.T) {
 	}
 
 	// Try permissive filtering
-	filt := newAclFilter(acl.AllowAll(), nil)
+	filt := newAclFilter(acl.AllowAll(), nil, false)
 	filt.filterHealthChecks(&hc)
 	if len(hc) != 1 {
 		t.Fatalf("bad: %#v", hc)
 	}
 
 	// Try restrictive filtering
-	filt = newAclFilter(acl.DenyAll(), nil)
+	filt = newAclFilter(acl.DenyAll(), nil, false)
 	filt.filterHealthChecks(&hc)
 	if len(hc) != 0 {
 		t.Fatalf("bad: %#v", hc)
@@ -830,14 +840,14 @@ func TestACL_filterServices(t *testing.T) {
 	}
 
 	// Try permissive filtering
-	filt := newAclFilter(acl.AllowAll(), nil)
+	filt := newAclFilter(acl.AllowAll(), nil, false)
 	filt.filterServices(services)
 	if len(services) != 2 {
 		t.Fatalf("bad: %#v", services)
 	}
 
 	// Try restrictive filtering
-	filt = newAclFilter(acl.DenyAll(), nil)
+	filt = newAclFilter(acl.DenyAll(), nil, false)
 	filt.filterServices(services)
 	if len(services) != 0 {
 		t.Fatalf("bad: %#v", services)
@@ -845,26 +855,92 @@ func TestACL_filterServices(t *testing.T) {
 }
 
 func TestACL_filterServiceNodes(t *testing.T) {
-	// Create some service nodes
+	// Create some service nodes.
-	nodes := structs.ServiceNodes{
+	fill := func() structs.ServiceNodes {
-		&structs.ServiceNode{
+		return structs.ServiceNodes{
-			Node:        "node1",
+			&structs.ServiceNode{
-			ServiceName: "foo",
+				Node:        "node1",
-		},
+				ServiceName: "foo",
+			},
+		}
 	}
 
-	// Try permissive filtering
+	// Try permissive filtering.
-	filt := newAclFilter(acl.AllowAll(), nil)
+	{
-	filt.filterServiceNodes(&nodes)
+		nodes := fill()
-	if len(nodes) != 1 {
+		filt := newAclFilter(acl.AllowAll(), nil, false)
-		t.Fatalf("bad: %#v", nodes)
+		filt.filterServiceNodes(&nodes)
+		if len(nodes) != 1 {
+			t.Fatalf("bad: %#v", nodes)
+		}
 	}
 
-	// Try restrictive filtering
+	// Try restrictive filtering.
-	filt = newAclFilter(acl.DenyAll(), nil)
+	{
-	filt.filterServiceNodes(&nodes)
+		nodes := fill()
-	if len(nodes) != 0 {
+		filt := newAclFilter(acl.DenyAll(), nil, false)
-		t.Fatalf("bad: %#v", nodes)
+		filt.filterServiceNodes(&nodes)
+		if len(nodes) != 0 {
+			t.Fatalf("bad: %#v", nodes)
+		}
+	}
+
+	// Allowed to see the service but not the node.
+	policy, err := acl.Parse(`
+service "foo" {
+  policy = "read"
+}
+`)
+	if err != nil {
+		t.Fatalf("err %v", err)
+	}
+	perms, err := acl.New(acl.DenyAll(), policy)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	/// This will work because version 8 ACLs aren't being enforced.
+	{
+		nodes := fill()
+		filt := newAclFilter(perms, nil, false)
+		filt.filterServiceNodes(&nodes)
+		if len(nodes) != 1 {
+			t.Fatalf("bad: %#v", nodes)
+		}
+	}
+
+	// But with version 8 the node will block it.
+	{
+		nodes := fill()
+		filt := newAclFilter(perms, nil, true)
+		filt.filterServiceNodes(&nodes)
+		if len(nodes) != 0 {
+			t.Fatalf("bad: %#v", nodes)
+		}
+	}
+
+	// Chain on access to the node.
+	policy, err = acl.Parse(`
+node "node1" {
+  policy = "read"
+}
+`)
+	if err != nil {
+		t.Fatalf("err %v", err)
+	}
+	perms, err = acl.New(perms, policy)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Now it should go through.
+	{
+		nodes := fill()
+		filt := newAclFilter(perms, nil, true)
+		filt.filterServiceNodes(&nodes)
+		if len(nodes) != 1 {
+			t.Fatalf("bad: %#v", nodes)
+		}
 	}
 }
 
@@ -883,14 +959,14 @@ func TestACL_filterNodeServices(t *testing.T) {
 	}
 
 	// Try permissive filtering
-	filt := newAclFilter(acl.AllowAll(), nil)
+	filt := newAclFilter(acl.AllowAll(), nil, false)
 	filt.filterNodeServices(&services)
 	if len(services.Services) != 1 {
 		t.Fatalf("bad: %#v", services.Services)
 	}
 
 	// Try restrictive filtering
-	filt = newAclFilter(acl.DenyAll(), nil)
+	filt = newAclFilter(acl.DenyAll(), nil, false)
 	filt.filterNodeServices(&services)
 	if len(services.Services) != 0 {
 		t.Fatalf("bad: %#v", services.Services)
@@ -919,7 +995,7 @@ func TestACL_filterCheckServiceNodes(t *testing.T) {
 	}
 
 	// Try permissive filtering
-	filt := newAclFilter(acl.AllowAll(), nil)
+	filt := newAclFilter(acl.AllowAll(), nil, false)
 	filt.filterCheckServiceNodes(&nodes)
 	if len(nodes) != 1 {
 		t.Fatalf("bad: %#v", nodes)
@@ -929,7 +1005,7 @@ func TestACL_filterCheckServiceNodes(t *testing.T) {
 	}
 
 	// Try restrictive filtering
-	filt = newAclFilter(acl.DenyAll(), nil)
+	filt = newAclFilter(acl.DenyAll(), nil, false)
 	filt.filterCheckServiceNodes(&nodes)
 	if len(nodes) != 0 {
 		t.Fatalf("bad: %#v", nodes)
@@ -958,7 +1034,7 @@ func TestACL_filterNodeDump(t *testing.T) {
 	}
 
 	// Try permissive filtering
-	filt := newAclFilter(acl.AllowAll(), nil)
+	filt := newAclFilter(acl.AllowAll(), nil, false)
 	filt.filterNodeDump(&dump)
 	if len(dump) != 1 {
 		t.Fatalf("bad: %#v", dump)
@@ -971,7 +1047,7 @@ func TestACL_filterNodeDump(t *testing.T) {
 	}
 
 	// Try restrictive filtering
-	filt = newAclFilter(acl.DenyAll(), nil)
+	filt = newAclFilter(acl.DenyAll(), nil, false)
 	filt.filterNodeDump(&dump)
 	if len(dump) != 1 {
 		t.Fatalf("bad: %#v", dump)
@@ -984,6 +1060,39 @@ func TestACL_filterNodeDump(t *testing.T) {
 	}
 }
 
+func TestACL_filterNodes(t *testing.T) {
+	// Create a nodes list.
+	nodes := structs.Nodes{
+		&structs.Node{
+			Node: "foo",
+		},
+		&structs.Node{
+			Node: "bar",
+		},
+	}
+
+	// Try permissive filtering.
+	filt := newAclFilter(acl.AllowAll(), nil, true)
+	filt.filterNodes(&nodes)
+	if len(nodes) != 2 {
+		t.Fatalf("bad: %#v", nodes)
+	}
+
+	// Try restrictive filtering but with version 8 enforcement turned off.
+	filt = newAclFilter(acl.DenyAll(), nil, false)
+	filt.filterNodes(&nodes)
+	if len(nodes) != 2 {
+		t.Fatalf("bad: %#v", nodes)
+	}
+
+	// Try restrictive filtering with version 8 enforcement turned on.
+	filt = newAclFilter(acl.DenyAll(), nil, true)
+	filt.filterNodes(&nodes)
+	if len(nodes) != 0 {
+		t.Fatalf("bad: %#v", nodes)
+	}
+}
+
 func TestACL_redactPreparedQueryTokens(t *testing.T) {
 	query := &structs.PreparedQuery{
 		ID:    "f004177f-2c28-83b7-4229-eacc25fe55d1",
@@ -997,7 +1106,7 @@ func TestACL_redactPreparedQueryTokens(t *testing.T) {
 
 	// Try permissive filtering with a management token. This will allow the
 	// embedded token to be seen.
-	filt := newAclFilter(acl.ManageAll(), nil)
+	filt := newAclFilter(acl.ManageAll(), nil, false)
 	filt.redactPreparedQueryTokens(&query)
 	if !reflect.DeepEqual(query, expected) {
 		t.Fatalf("bad: %#v", &query)
@@ -1009,7 +1118,7 @@ func TestACL_redactPreparedQueryTokens(t *testing.T) {
 
 	// Now try permissive filtering with a client token, which should cause
 	// the embedded token to get redacted.
-	filt = newAclFilter(acl.AllowAll(), nil)
+	filt = newAclFilter(acl.AllowAll(), nil, false)
 	filt.redactPreparedQueryTokens(&query)
 	expected.Token = redactedToken
 	if !reflect.DeepEqual(query, expected) {
@@ -1055,7 +1164,7 @@ func TestACL_filterPreparedQueries(t *testing.T) {
 
 	// Try permissive filtering with a management token. This will allow the
 	// embedded token to be seen.
-	filt := newAclFilter(acl.ManageAll(), nil)
+	filt := newAclFilter(acl.ManageAll(), nil, false)
 	filt.filterPreparedQueries(&queries)
 	if !reflect.DeepEqual(queries, expected) {
 		t.Fatalf("bad: %#v", queries)
@@ -1068,7 +1177,7 @@ func TestACL_filterPreparedQueries(t *testing.T) {
 	// Now try permissive filtering with a client token, which should cause
 	// the embedded token to get redacted, and the query with no name to get
 	// filtered out.
-	filt = newAclFilter(acl.AllowAll(), nil)
+	filt = newAclFilter(acl.AllowAll(), nil, false)
 	filt.filterPreparedQueries(&queries)
 	expected[2].Token = redactedToken
 	expected = append(structs.PreparedQueries{}, expected[1], expected[2])
@@ -1082,7 +1191,7 @@ func TestACL_filterPreparedQueries(t *testing.T) {
 	}
 
 	// Now try restrictive filtering.
-	filt = newAclFilter(acl.DenyAll(), nil)
+	filt = newAclFilter(acl.DenyAll(), nil, false)
 	filt.filterPreparedQueries(&queries)
 	if len(queries) != 0 {
 		t.Fatalf("bad: %#v", queries)
@@ -1106,11 +1215,333 @@ func TestACL_unhandledFilterType(t *testing.T) {
 	srv.filterACL(token, &structs.HealthCheck{})
 }
 
-var testACLPolicy = `
+func TestACL_vetRegisterWithACL(t *testing.T) {
-key "" {
+	args := &structs.RegisterRequest{
-	policy = "deny"
+		Node:    "nope",
+		Address: "127.0.0.1",
+	}
+
+	// With a nil ACL, the update should be allowed.
+	if err := vetRegisterWithACL(nil, args, nil); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Create a basic node policy.
+	policy, err := acl.Parse(`
+node "node" {
+  policy = "write"
 }
-key "foo/" {
+`)
-	policy = "write"
+	if err != nil {
+		t.Fatalf("err %v", err)
+	}
+	perms, err := acl.New(acl.DenyAll(), policy)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// With that policy, the update should now be blocked for node reasons.
+	err = vetRegisterWithACL(perms, args, nil)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("bad: %v", err)
+	}
+
+	// Now use a permitted node name.
+	args.Node = "node"
+	if err := vetRegisterWithACL(perms, args, nil); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Build some node info that matches what we have now.
+	ns := &structs.NodeServices{
+		Node: &structs.Node{
+			Node:    "node",
+			Address: "127.0.0.1",
+		},
+		Services: make(map[string]*structs.NodeService),
+	}
+
+	// Try to register a service, which should be blocked.
+	args.Service = &structs.NodeService{
+		Service: "service",
+		ID:      "my-id",
+	}
+	err = vetRegisterWithACL(perms, args, ns)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("bad: %v", err)
+	}
+
+	// Chain on a basic service policy.
+	policy, err = acl.Parse(`
+service "service" {
+  policy = "write"
+}
+`)
+	if err != nil {
+		t.Fatalf("err %v", err)
+	}
+	perms, err = acl.New(perms, policy)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// With the service ACL, the update should go through.
+	if err := vetRegisterWithACL(perms, args, ns); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Add an existing service that they are clobbering and aren't allowed
+	// to write to.
+	ns.Services["my-id"] = &structs.NodeService{
+		Service: "other",
+		ID:      "my-id",
+	}
+	err = vetRegisterWithACL(perms, args, ns)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("bad: %v", err)
+	}
+
+	// Chain on a policy that allows them to write to the other service.
+	policy, err = acl.Parse(`
+service "other" {
+  policy = "write"
+}
+`)
+	if err != nil {
+		t.Fatalf("err %v", err)
+	}
+	perms, err = acl.New(perms, policy)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Now it should go through.
+	if err := vetRegisterWithACL(perms, args, ns); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Try creating the node and the service at once by having no existing
+	// node record. This should be ok since we have node and service
+	// permissions.
+	if err := vetRegisterWithACL(perms, args, nil); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Add a node-level check to the member, which should be rejected.
+	args.Check = &structs.HealthCheck{
+		Node: "node",
+	}
+	err = vetRegisterWithACL(perms, args, ns)
+	if err == nil || !strings.Contains(err.Error(), "check member must be nil") {
+		t.Fatalf("bad: %v", err)
+	}
+
+	// Move the check into the slice, but give a bad node name.
+	args.Check.Node = "nope"
+	args.Checks = append(args.Checks, args.Check)
+	args.Check = nil
+	err = vetRegisterWithACL(perms, args, ns)
+	if err == nil || !strings.Contains(err.Error(), "doesn't match register request node") {
+		t.Fatalf("bad: %v", err)
+	}
+
+	// Fix the node name, which should now go through.
+	args.Checks[0].Node = "node"
+	if err := vetRegisterWithACL(perms, args, ns); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Add a service-level check.
+	args.Checks = append(args.Checks, &structs.HealthCheck{
+		Node:      "node",
+		ServiceID: "my-id",
+	})
+	if err := vetRegisterWithACL(perms, args, ns); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Try creating everything at once. This should be ok since we have all
+	// the permissions we need. It also makes sure that we can register a
+	// new node, service, and associated checks.
+	if err := vetRegisterWithACL(perms, args, nil); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Nil out the service registration, which'll skip the special case
+	// and force us to look at the ns data (it will look like we are
+	// writing to the "other" service which also has "my-id").
+	args.Service = nil
+	if err := vetRegisterWithACL(perms, args, ns); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Chain on a policy that forbids them to write to the other service.
+	policy, err = acl.Parse(`
+service "other" {
+  policy = "deny"
+}
+`)
+	if err != nil {
+		t.Fatalf("err %v", err)
+	}
+	perms, err = acl.New(perms, policy)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// This should get rejected.
+	err = vetRegisterWithACL(perms, args, ns)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("bad: %v", err)
+	}
+
+	// Change the existing service data to point to a service name they
+	// car write to. This should go through.
+	ns.Services["my-id"] = &structs.NodeService{
+		Service: "service",
+		ID:      "my-id",
+	}
+	if err := vetRegisterWithACL(perms, args, ns); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Chain on a policy that forbids them to write to the node.
+	policy, err = acl.Parse(`
+node "node" {
+  policy = "deny"
+}
+`)
+	if err != nil {
+		t.Fatalf("err %v", err)
+	}
+	perms, err = acl.New(perms, policy)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// This should get rejected because there's a node-level check in here.
+	err = vetRegisterWithACL(perms, args, ns)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("bad: %v", err)
+	}
+
+	// Change the node-level check into a service check, and then it should
+	// go through.
+	args.Checks[0].ServiceID = "my-id"
+	if err := vetRegisterWithACL(perms, args, ns); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Finally, attempt to update the node part of the data and make sure
+	// that gets rejected since they no longer have permissions.
+	args.Address = "127.0.0.2"
+	err = vetRegisterWithACL(perms, args, ns)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("bad: %v", err)
+	}
+}
+
+func TestACL_vetDeregisterWithACL(t *testing.T) {
+	args := &structs.DeregisterRequest{
+		Node: "nope",
+	}
+
+	// With a nil ACL, the update should be allowed.
+	if err := vetDeregisterWithACL(nil, args, nil, nil); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Create a basic node policy.
+	policy, err := acl.Parse(`
+node "node" {
+  policy = "write"
+}
+service "service" {
+  policy = "write"
+}
+`)
+	if err != nil {
+		t.Fatalf("err %v", err)
+	}
+	perms, err := acl.New(acl.DenyAll(), policy)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// With that policy, the update should now be blocked for node reasons.
+	err = vetDeregisterWithACL(perms, args, nil, nil)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("bad: %v", err)
+	}
+
+	// Now use a permitted node name.
+	args.Node = "node"
+	if err := vetDeregisterWithACL(perms, args, nil, nil); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Try an unknown check.
+	args.CheckID = "check-id"
+	err = vetDeregisterWithACL(perms, args, nil, nil)
+	if err == nil || !strings.Contains(err.Error(), "Unknown check") {
+		t.Fatalf("bad: %v", err)
+	}
+
+	// Now pass in a check that should be blocked.
+	nc := &structs.HealthCheck{
+		Node:        "node",
+		CheckID:     "check-id",
+		ServiceID:   "service-id",
+		ServiceName: "nope",
+	}
+	err = vetDeregisterWithACL(perms, args, nil, nc)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("bad: %v", err)
+	}
+
+	// Change it to an allowed service, which should go through.
+	nc.ServiceName = "service"
+	if err := vetDeregisterWithACL(perms, args, nil, nc); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Switch to a node check that should be blocked.
+	args.Node = "nope"
+	nc.Node = "nope"
+	nc.ServiceID = ""
+	nc.ServiceName = ""
+	err = vetDeregisterWithACL(perms, args, nil, nc)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("bad: %v", err)
+	}
+
+	// Switch to an allowed node check, which should go through.
+	args.Node = "node"
+	nc.Node = "node"
+	if err := vetDeregisterWithACL(perms, args, nil, nc); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Try an unknown service.
+	args.ServiceID = "service-id"
+	err = vetDeregisterWithACL(perms, args, nil, nil)
+	if err == nil || !strings.Contains(err.Error(), "Unknown service") {
+		t.Fatalf("bad: %v", err)
+	}
+
+	// Now pass in a service that should be blocked.
+	ns := &structs.NodeService{
+		ID:      "service-id",
+		Service: "nope",
+	}
+	err = vetDeregisterWithACL(perms, args, ns, nil)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("bad: %v", err)
+	}
+
+	// Change it to an allowed service, which should go through.
+	ns.Service = "service"
+	if err := vetDeregisterWithACL(perms, args, ns, nil); err != nil {
+		t.Fatalf("err: %v", err)
+	}
 }
-`

consul/catalog_endpoint.go

@@ -21,37 +21,42 @@ func (c *Catalog) Register(args *structs.RegisterRequest, reply *struct{}) error
 	}
 	defer metrics.MeasureSince([]string{"consul", "catalog", "register"}, time.Now())
 
-	// Verify the args
+	// Verify the args.
 	if args.Node == "" || args.Address == "" {
 		return fmt.Errorf("Must provide node and address")
 	}
 
+	// Fetch the ACL token, if any.
+	acl, err := c.srv.resolveToken(args.Token)
+	if err != nil {
+		return err
+	}
+
+	// Handle a service registration.
 	if args.Service != nil {
 		// If no service id, but service name, use default
 		if args.Service.ID == "" && args.Service.Service != "" {
 			args.Service.ID = args.Service.Service
 		}
 
-		// Verify ServiceName provided if ID
+		// Verify ServiceName provided if ID.
 		if args.Service.ID != "" && args.Service.Service == "" {
 			return fmt.Errorf("Must provide service name with ID")
 		}
 
-		// Apply the ACL policy if any
+		// Apply the ACL policy if any. The 'consul' service is excluded
-		// The 'consul' service is excluded since it is managed
+		// since it is managed automatically internally (that behavior
-		// automatically internally.
+		// is going away after version 0.8). We check this same policy
+		// later if version 0.8 is enabled, so we can eventually just
+		// delete this and do all the ACL checks down there.
 		if args.Service.Service != ConsulServiceName {
-			acl, err := c.srv.resolveToken(args.Token)
+			if acl != nil && !acl.ServiceWrite(args.Service.Service) {
-			if err != nil {
-				return err
-			} else if acl != nil && !acl.ServiceWrite(args.Service.Service) {
-				c.srv.logger.Printf("[WARN] consul.catalog: Register of service '%s' on '%s' denied due to ACLs",
-					args.Service.Service, args.Node)
 				return permissionDeniedErr
 			}
 		}
 	}
 
+	// Move the old format single check into the slice, and fixup IDs.
 	if args.Check != nil {
 		args.Checks = append(args.Checks, args.Check)
 		args.Check = nil
@@ -65,9 +70,20 @@ func (c *Catalog) Register(args *structs.RegisterRequest, reply *struct{}) error
 		}
 	}
 
-	_, err := c.srv.raftApply(structs.RegisterRequestType, args)
+	// Check the complete register request against the given ACL policy.
+	if acl != nil && c.srv.config.ACLEnforceVersion8 {
+		state := c.srv.fsm.State()
+		_, ns, err := state.NodeServices(args.Node)
+		if err != nil {
+			return fmt.Errorf("Node lookup failed: %v", err)
+		}
+		if err := vetRegisterWithACL(acl, args, ns); err != nil {
+			return err
+		}
+	}
+
+	_, err = c.srv.raftApply(structs.RegisterRequestType, args)
 	if err != nil {
-		c.srv.logger.Printf("[ERR] consul.catalog: Register failed: %v", err)
 		return err
 	}
 
@@ -86,9 +102,38 @@ func (c *Catalog) Deregister(args *structs.DeregisterRequest, reply *struct{}) e
 		return fmt.Errorf("Must provide node")
 	}
 
-	_, err := c.srv.raftApply(structs.DeregisterRequestType, args)
+	// Fetch the ACL token, if any.
+	acl, err := c.srv.resolveToken(args.Token)
 	if err != nil {
-		c.srv.logger.Printf("[ERR] consul.catalog: Deregister failed: %v", err)
+		return err
+	}
+
+	// Check the complete deregister request against the given ACL policy.
+	if acl != nil && c.srv.config.ACLEnforceVersion8 {
+		state := c.srv.fsm.State()
+
+		var ns *structs.NodeService
+		if args.ServiceID != "" {
+			_, ns, err = state.NodeService(args.Node, args.ServiceID)
+			if err != nil {
+				return fmt.Errorf("Service lookup failed: %v", err)
+			}
+		}
+
+		var nc *structs.HealthCheck
+		if args.CheckID != "" {
+			_, nc, err = state.NodeCheck(args.Node, args.CheckID)
+			if err != nil {
+				return fmt.Errorf("Check lookup failed: %v", err)
+			}
+		}
+
+		if err := vetDeregisterWithACL(acl, args, ns, nc); err != nil {
+			return err
+		}
+	}
+
+	if _, err := c.srv.raftApply(structs.DeregisterRequestType, args); err != nil {
 		return err
 	}
 	return nil
@@ -124,6 +169,9 @@ func (c *Catalog) ListNodes(args *structs.DCSpecificRequest, reply *structs.Inde
 			}
 
 			reply.Index, reply.Nodes = index, nodes
+			if err := c.srv.filterACL(args.Token, reply); err != nil {
+				return err
+			}
 			return c.srv.sortNodesByDistanceFrom(args.Source, reply.Nodes)
 		})
 }
@@ -222,6 +270,21 @@ func (c *Catalog) NodeServices(args *structs.NodeSpecificRequest, reply *structs
 			if err != nil {
 				return err
 			}
+
+			// Node read access is required with version 8 ACLs. We
+			// just return the same response as if the node doesn't
+			// exist, which is consistent with how the rest of the
+			// catalog filtering works and doesn't disclose the node.
+			acl, err := c.srv.resolveToken(args.Token)
+			if err != nil {
+				return err
+			}
+			if acl != nil && c.srv.config.ACLEnforceVersion8 {
+				if !acl.NodeRead(args.Node) {
+					return permissionDeniedErr
+				}
+			}
+
 			reply.Index, reply.NodeServices = index, services
 			return c.srv.filterACL(args.Token, reply)
 		})

consul/catalog_endpoint_test.go

@@ -14,7 +14,7 @@ import (
 	"github.com/hashicorp/net-rpc-msgpackrpc"
 )
 
-func TestCatalogRegister(t *testing.T) {
+func TestCatalog_Register(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -30,6 +30,9 @@ func TestCatalogRegister(t *testing.T) {
 			Tags:    []string{"master"},
 			Port:    8000,
 		},
+		Check: &structs.HealthCheck{
+			ServiceID: "db",
+		},
 	}
 	var out struct{}
 
@@ -46,11 +49,12 @@ func TestCatalogRegister(t *testing.T) {
 	})
 }
 
-func TestCatalogRegister_ACLDeny(t *testing.T) {
+func TestCatalog_Register_ACLDeny(t *testing.T) {
 	dir1, s1 := testServerWithConfig(t, func(c *Config) {
 		c.ACLDatacenter = "dc1"
 		c.ACLMasterToken = "root"
 		c.ACLDefaultPolicy = "deny"
+		c.ACLEnforceVersion8 = false
 	})
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -59,22 +63,25 @@ func TestCatalogRegister_ACLDeny(t *testing.T) {
 
 	testutil.WaitForLeader(t, s1.RPC, "dc1")
 
-	// Create the ACL
+	// Create the ACL.
 	arg := structs.ACLRequest{
 		Datacenter: "dc1",
 		Op:         structs.ACLSet,
 		ACL: structs.ACL{
-			Name:  "User token",
+			Name: "User token",
-			Type:  structs.ACLTypeClient,
+			Type: structs.ACLTypeClient,
-			Rules: testRegisterRules,
+			Rules: `
+service "foo" {
+	policy = "write"
+}
+`,
 		},
 		WriteRequest: structs.WriteRequest{Token: "root"},
 	}
-	var out string
+	var id string
-	if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &out); err != nil {
+	if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &id); err != nil {
 		t.Fatalf("err: %v", err)
 	}
-	id := out
 
 	argR := structs.RegisterRequest{
 		Datacenter: "dc1",
@@ -89,19 +96,61 @@ func TestCatalogRegister_ACLDeny(t *testing.T) {
 	}
 	var outR struct{}
 
+	// This should fail since we are writing to the "db" service, which isn't
+	// allowed.
 	err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &argR, &outR)
 	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
 		t.Fatalf("err: %v", err)
 	}
 
+	// The "foo" service should work, though.
 	argR.Service.Service = "foo"
 	err = msgpackrpc.CallWithCodec(codec, "Catalog.Register", &argR, &outR)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
+
+	// Try the special case for the "consul" service that allows it no matter
+	// what with pre-version 8 ACL enforcement.
+	argR.Service.Service = "consul"
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Register", &argR, &outR)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Make sure the exception goes away when we turn on version 8 ACL
+	// enforcement.
+	s1.config.ACLEnforceVersion8 = true
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Register", &argR, &outR)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Register a db service using the root token.
+	argR.Service.Service = "db"
+	argR.Service.ID = "my-id"
+	argR.Token = "root"
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Register", &argR, &outR)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Prove that we are properly looking up the node services and passing
+	// that to the ACL helper. We can vet the helper independently in its
+	// own unit test after this. This is trying to register over the db
+	// service we created above, which is a check that depends on looking
+	// at the existing registration data with that service ID. This is a new
+	// check for version 8.
+	argR.Service.Service = "foo"
+	argR.Service.ID = "my-id"
+	argR.Token = id
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Register", &argR, &outR)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("err: %v", err)
+	}
 }
 
-func TestCatalogRegister_ForwardLeader(t *testing.T) {
+func TestCatalog_Register_ForwardLeader(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -148,7 +197,7 @@ func TestCatalogRegister_ForwardLeader(t *testing.T) {
 	}
 }
 
-func TestCatalogRegister_ForwardDC(t *testing.T) {
+func TestCatalog_Register_ForwardDC(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -184,7 +233,7 @@ func TestCatalogRegister_ForwardDC(t *testing.T) {
 	}
 }
 
-func TestCatalogDeregister(t *testing.T) {
+func TestCatalog_Deregister(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -209,7 +258,218 @@ func TestCatalogDeregister(t *testing.T) {
 	}
 }
 
-func TestCatalogListDatacenters(t *testing.T) {
+func TestCatalog_Deregister_ACLDeny(t *testing.T) {
+	dir1, s1 := testServerWithConfig(t, func(c *Config) {
+		c.ACLDatacenter = "dc1"
+		c.ACLMasterToken = "root"
+		c.ACLDefaultPolicy = "deny"
+		c.ACLEnforceVersion8 = false
+	})
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	defer codec.Close()
+
+	testutil.WaitForLeader(t, s1.RPC, "dc1")
+
+	// Create the ACL.
+	arg := structs.ACLRequest{
+		Datacenter: "dc1",
+		Op:         structs.ACLSet,
+		ACL: structs.ACL{
+			Name: "User token",
+			Type: structs.ACLTypeClient,
+			Rules: `
+node "node" {
+	policy = "write"
+}
+
+service "service" {
+	policy = "write"
+}
+`,
+		},
+		WriteRequest: structs.WriteRequest{Token: "root"},
+	}
+	var id string
+	if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &id); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Register a node, node check, service, and service check.
+	argR := structs.RegisterRequest{
+		Datacenter: "dc1",
+		Node:       "node",
+		Address:    "127.0.0.1",
+		Service: &structs.NodeService{
+			Service: "service",
+			Port:    8000,
+		},
+		Checks: structs.HealthChecks{
+			&structs.HealthCheck{
+				Node:    "node",
+				CheckID: "node-check",
+			},
+			&structs.HealthCheck{
+				Node:      "node",
+				CheckID:   "service-check",
+				ServiceID: "service",
+			},
+		},
+		WriteRequest: structs.WriteRequest{Token: id},
+	}
+	var outR struct{}
+	if err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &argR, &outR); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// First pass with version 8 ACL enforcement disabled, we should be able
+	// to deregister everything even without a token.
+	var err error
+	var out struct{}
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Deregister",
+		&structs.DeregisterRequest{
+			Datacenter: "dc1",
+			Node:       "node",
+			CheckID:    "service-check"}, &out)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Deregister",
+		&structs.DeregisterRequest{
+			Datacenter: "dc1",
+			Node:       "node",
+			CheckID:    "node-check"}, &out)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Deregister",
+		&structs.DeregisterRequest{
+			Datacenter: "dc1",
+			Node:       "node",
+			ServiceID:  "service"}, &out)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Deregister",
+		&structs.DeregisterRequest{
+			Datacenter: "dc1",
+			Node:       "node"}, &out)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Turn on version 8 ACL enforcement and put the catalog entry back.
+	s1.config.ACLEnforceVersion8 = true
+	if err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &argR, &outR); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Second pass with version 8 ACL enforcement enabled, these should all
+	// get rejected.
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Deregister",
+		&structs.DeregisterRequest{
+			Datacenter: "dc1",
+			Node:       "node",
+			CheckID:    "service-check"}, &out)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("err: %v", err)
+	}
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Deregister",
+		&structs.DeregisterRequest{
+			Datacenter: "dc1",
+			Node:       "node",
+			CheckID:    "node-check"}, &out)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("err: %v", err)
+	}
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Deregister",
+		&structs.DeregisterRequest{
+			Datacenter: "dc1",
+			Node:       "node",
+			ServiceID:  "service"}, &out)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("err: %v", err)
+	}
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Deregister",
+		&structs.DeregisterRequest{
+			Datacenter: "dc1",
+			Node:       "node"}, &out)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Third pass these should all go through with the token set.
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Deregister",
+		&structs.DeregisterRequest{
+			Datacenter: "dc1",
+			Node:       "node",
+			CheckID:    "service-check",
+			WriteRequest: structs.WriteRequest{
+				Token: id,
+			}}, &out)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Deregister",
+		&structs.DeregisterRequest{
+			Datacenter: "dc1",
+			Node:       "node",
+			CheckID:    "node-check",
+			WriteRequest: structs.WriteRequest{
+				Token: id,
+			}}, &out)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Deregister",
+		&structs.DeregisterRequest{
+			Datacenter: "dc1",
+			Node:       "node",
+			ServiceID:  "service",
+			WriteRequest: structs.WriteRequest{
+				Token: id,
+			}}, &out)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Deregister",
+		&structs.DeregisterRequest{
+			Datacenter: "dc1",
+			Node:       "node",
+			WriteRequest: structs.WriteRequest{
+				Token: id,
+			}}, &out)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Try a few error cases.
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Deregister",
+		&structs.DeregisterRequest{
+			Datacenter: "dc1",
+			Node:       "node",
+			ServiceID:  "nope",
+			WriteRequest: structs.WriteRequest{
+				Token: id,
+			}}, &out)
+	if err == nil || !strings.Contains(err.Error(), "Unknown service") {
+		t.Fatalf("err: %v", err)
+	}
+	err = msgpackrpc.CallWithCodec(codec, "Catalog.Deregister",
+		&structs.DeregisterRequest{
+			Datacenter: "dc1",
+			Node:       "node",
+			CheckID:    "nope",
+			WriteRequest: structs.WriteRequest{
+				Token: id,
+			}}, &out)
+	if err == nil || !strings.Contains(err.Error(), "Unknown check") {
+		t.Fatalf("err: %v", err)
+	}
+}
+
+func TestCatalog_ListDatacenters(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -246,7 +506,7 @@ func TestCatalogListDatacenters(t *testing.T) {
 	}
 }
 
-func TestCatalogListDatacenters_DistanceSort(t *testing.T) {
+func TestCatalog_ListDatacenters_DistanceSort(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -290,7 +550,7 @@ func TestCatalogListDatacenters_DistanceSort(t *testing.T) {
 	}
 }
 
-func TestCatalogListNodes(t *testing.T) {
+func TestCatalog_ListNodes(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -332,7 +592,7 @@ func TestCatalogListNodes(t *testing.T) {
 	}
 }
 
-func TestCatalogListNodes_StaleRaad(t *testing.T) {
+func TestCatalog_ListNodes_StaleRaad(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -400,7 +660,7 @@ func TestCatalogListNodes_StaleRaad(t *testing.T) {
 	}
 }
 
-func TestCatalogListNodes_ConsistentRead_Fail(t *testing.T) {
+func TestCatalog_ListNodes_ConsistentRead_Fail(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -450,7 +710,7 @@ func TestCatalogListNodes_ConsistentRead_Fail(t *testing.T) {
 	}
 }
 
-func TestCatalogListNodes_ConsistentRead(t *testing.T) {
+func TestCatalog_ListNodes_ConsistentRead(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -498,7 +758,7 @@ func TestCatalogListNodes_ConsistentRead(t *testing.T) {
 	}
 }
 
-func TestCatalogListNodes_DistanceSort(t *testing.T) {
+func TestCatalog_ListNodes_DistanceSort(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -586,7 +846,84 @@ func TestCatalogListNodes_DistanceSort(t *testing.T) {
 	}
 }
 
-func BenchmarkCatalogListNodes(t *testing.B) {
+func TestCatalog_ListNodes_ACLFilter(t *testing.T) {
+	dir1, s1 := testServerWithConfig(t, func(c *Config) {
+		c.ACLDatacenter = "dc1"
+		c.ACLMasterToken = "root"
+		c.ACLDefaultPolicy = "deny"
+		c.ACLEnforceVersion8 = false
+	})
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	defer codec.Close()
+
+	testutil.WaitForLeader(t, s1.RPC, "dc1")
+
+	// We scope the reply in each of these since msgpack won't clear out an
+	// existing slice if the incoming one is nil, so it's best to start
+	// clean each time.
+
+	// Prior to version 8, the node policy should be ignored.
+	args := structs.DCSpecificRequest{
+		Datacenter: "dc1",
+	}
+	{
+		reply := structs.IndexedNodes{}
+		if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &reply); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		if len(reply.Nodes) != 1 {
+			t.Fatalf("bad: %v", reply.Nodes)
+		}
+	}
+
+	// Now turn on version 8 enforcement and try again.
+	s1.config.ACLEnforceVersion8 = true
+	{
+		reply := structs.IndexedNodes{}
+		if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &reply); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		if len(reply.Nodes) != 0 {
+			t.Fatalf("bad: %v", reply.Nodes)
+		}
+	}
+
+	// Create an ACL that can read the node.
+	arg := structs.ACLRequest{
+		Datacenter: "dc1",
+		Op:         structs.ACLSet,
+		ACL: structs.ACL{
+			Name: "User token",
+			Type: structs.ACLTypeClient,
+			Rules: fmt.Sprintf(`
+node "%s" {
+	policy = "read"
+}
+`, s1.config.NodeName),
+		},
+		WriteRequest: structs.WriteRequest{Token: "root"},
+	}
+	var id string
+	if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &id); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Now try with the token and it will go through.
+	args.Token = id
+	{
+		reply := structs.IndexedNodes{}
+		if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &reply); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		if len(reply.Nodes) != 1 {
+			t.Fatalf("bad: %v", reply.Nodes)
+		}
+	}
+}
+
+func Benchmark_Catalog_ListNodes(t *testing.B) {
 	dir1, s1 := testServer(nil)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -609,7 +946,7 @@ func BenchmarkCatalogListNodes(t *testing.B) {
 	}
 }
 
-func TestCatalogListServices(t *testing.T) {
+func TestCatalog_ListServices(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -659,7 +996,7 @@ func TestCatalogListServices(t *testing.T) {
 	}
 }
 
-func TestCatalogListServices_Blocking(t *testing.T) {
+func TestCatalog_ListServices_Blocking(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -717,7 +1054,7 @@ func TestCatalogListServices_Blocking(t *testing.T) {
 	}
 }
 
-func TestCatalogListServices_Timeout(t *testing.T) {
+func TestCatalog_ListServices_Timeout(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -758,7 +1095,7 @@ func TestCatalogListServices_Timeout(t *testing.T) {
 	}
 }
 
-func TestCatalogListServices_Stale(t *testing.T) {
+func TestCatalog_ListServices_Stale(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -795,7 +1132,7 @@ func TestCatalogListServices_Stale(t *testing.T) {
 	}
 }
 
-func TestCatalogListServiceNodes(t *testing.T) {
+func TestCatalog_ListServiceNodes(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -844,7 +1181,7 @@ func TestCatalogListServiceNodes(t *testing.T) {
 	}
 }
 
-func TestCatalogListServiceNodes_DistanceSort(t *testing.T) {
+func TestCatalog_ListServiceNodes_DistanceSort(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -931,7 +1268,7 @@ func TestCatalogListServiceNodes_DistanceSort(t *testing.T) {
 	}
 }
 
-func TestCatalogNodeServices(t *testing.T) {
+func TestCatalog_NodeServices(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -981,7 +1318,7 @@ func TestCatalogNodeServices(t *testing.T) {
 }
 
 // Used to check for a regression against a known bug
-func TestCatalogRegister_FailedCase1(t *testing.T) {
+func TestCatalog_Register_FailedCase1(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -1032,6 +1369,7 @@ func testACLFilterServer(t *testing.T) (dir, token string, srv *Server, codec rp
 		c.ACLDatacenter = "dc1"
 		c.ACLMasterToken = "root"
 		c.ACLDefaultPolicy = "deny"
+		c.ACLEnforceVersion8 = false
 	})
 
 	codec = rpcClient(t, srv)
@@ -1042,9 +1380,13 @@ func testACLFilterServer(t *testing.T) (dir, token string, srv *Server, codec rp
 		Datacenter: "dc1",
 		Op:         structs.ACLSet,
 		ACL: structs.ACL{
-			Name:  "User token",
+			Name: "User token",
-			Type:  structs.ACLTypeClient,
+			Type: structs.ACLTypeClient,
-			Rules: testRegisterRules,
+			Rules: `
+service "foo" {
+	policy = "write"
+}
+`,
 		},
 		WriteRequest: structs.WriteRequest{Token: "root"},
 	}
@@ -1158,6 +1500,76 @@ func TestCatalog_ServiceNodes_FilterACL(t *testing.T) {
 			t.Fatalf("bad: %#v", reply.ServiceNodes)
 		}
 	}
+
+	// We've already proven that we call the ACL filtering function so we
+	// test node filtering down in acl.go for node cases. This also proves
+	// that we respect the version 8 ACL flag, since the test server sets
+	// that to false (the regression value of *not* changing this is better
+	// for now until we change the sense of the version 8 ACL flag).
+}
+
+func TestCatalog_NodeServices_ACLDeny(t *testing.T) {
+	dir1, s1 := testServerWithConfig(t, func(c *Config) {
+		c.ACLDatacenter = "dc1"
+		c.ACLMasterToken = "root"
+		c.ACLDefaultPolicy = "deny"
+		c.ACLEnforceVersion8 = false
+	})
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	defer codec.Close()
+
+	testutil.WaitForLeader(t, s1.RPC, "dc1")
+
+	// Prior to version 8, the node policy should be ignored.
+	args := structs.NodeSpecificRequest{
+		Datacenter: "dc1",
+		Node:       s1.config.NodeName,
+	}
+	reply := structs.IndexedNodeServices{}
+	if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &args, &reply); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if reply.NodeServices == nil {
+		t.Fatalf("should not be nil")
+	}
+
+	// Now turn on version 8 enforcement and try again.
+	s1.config.ACLEnforceVersion8 = true
+	err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &args, &reply)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Create an ACL that can read the node.
+	arg := structs.ACLRequest{
+		Datacenter: "dc1",
+		Op:         structs.ACLSet,
+		ACL: structs.ACL{
+			Name: "User token",
+			Type: structs.ACLTypeClient,
+			Rules: fmt.Sprintf(`
+node "%s" {
+	policy = "read"
+}
+`, s1.config.NodeName),
+		},
+		WriteRequest: structs.WriteRequest{Token: "root"},
+	}
+	var id string
+	if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &id); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Now try with the token and it will go through.
+	args.Token = id
+	if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &args, &reply); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if reply.NodeServices == nil {
+		t.Fatalf("should not be nil")
+	}
 }
 
 func TestCatalog_NodeServices_FilterACL(t *testing.T) {
@@ -1189,9 +1601,3 @@ func TestCatalog_NodeServices_FilterACL(t *testing.T) {
 		t.Fatalf("bad: %#v", reply.NodeServices)
 	}
 }
-
-var testRegisterRules = `
-service "foo" {
-	policy = "write"
-}
-`

consul/config.go

@@ -200,6 +200,10 @@ type Config struct {
 	// used to limit the amount of Raft bandwidth used for replication.
 	ACLReplicationApplyLimit int
 
+	// ACLEnforceVersion8 is used to gate a set of ACL policy features that
+	// are opt-in prior to Consul 0.8 and opt-out in Consul 0.8 and later.
+	ACLEnforceVersion8 bool
+
 	// TombstoneTTL is used to control how long KV tombstones are retained.
 	// This provides a window of time where the X-Consul-Index is monotonic.
 	// Outside this window, the index may not be monotonic. This is a result

consul/fsm.go

@@ -127,7 +127,9 @@ func (c *consulFSM) applyDeregister(buf []byte, index uint64) interface{} {
 		panic(fmt.Errorf("failed to decode request: %v", err))
 	}
 
-	// Either remove the service entry or the whole node
+	// Either remove the service entry or the whole node. The precedence
+	// here is also baked into vetDeregisterWithACL() in acl.go, so if you
+	// make changes here, be sure to also adjust the code over there.
 	if req.ServiceID != "" {
 		if err := c.state.DeleteService(index, req.Node, req.ServiceID); err != nil {
 			c.logger.Printf("[INFO] consul.fsm: DeleteNodeService failed: %v", err)

consul/state/state_store.go

@@ -349,9 +349,9 @@ func (s *StateStore) getWatchTables(method string) []string {
 		return []string{"nodes"}
 	case "Services":
 		return []string{"services"}
-	case "ServiceNodes", "NodeServices":
+	case "NodeService", "NodeServices", "ServiceNodes":
 		return []string{"nodes", "services"}
-	case "NodeChecks", "ServiceChecks", "ChecksInState":
+	case "NodeCheck", "NodeChecks", "ServiceChecks", "ChecksInState":
 		return []string{"checks"}
 	case "CheckServiceNodes", "NodeInfo", "NodeDump":
 		return []string{"nodes", "services", "checks"}
@@ -438,6 +438,12 @@ func (s *StateStore) ensureRegistrationTxn(tx *memdb.Txn, idx uint64, watches *D
 		}
 	}
 
+	// TODO (slackpad) In Consul 0.8 ban checks that don't have the same
+	// node as the top-level registration. This is just weird to be able to
+	// update unrelated nodes' checks from in here. In 0.7.2 we banned this
+	// up in the ACL check since that's guarded behind an opt-in flag until
+	// Consul 0.8.
+
 	// Add the checks, if any.
 	if req.Check != nil {
 		if err := s.ensureCheckTxn(tx, idx, watches, req.Check); err != nil {
@@ -854,6 +860,28 @@ func (s *StateStore) parseServiceNodes(tx *memdb.Txn, services structs.ServiceNo
 	return results, nil
 }
 
+// NodeService is used to retrieve a specific service associated with the given
+// node.
+func (s *StateStore) NodeService(nodeID string, serviceID string) (uint64, *structs.NodeService, error) {
+	tx := s.db.Txn(false)
+	defer tx.Abort()
+
+	// Get the table index.
+	idx := maxIndexTxn(tx, s.getWatchTables("NodeService")...)
+
+	// Query the service
+	service, err := tx.First("services", "id", nodeID, serviceID)
+	if err != nil {
+		return 0, nil, fmt.Errorf("failed querying service for node %q: %s", nodeID, err)
+	}
+
+	if service != nil {
+		return idx, service.(*structs.ServiceNode).ToNodeService(), nil
+	} else {
+		return idx, nil, nil
+	}
+}
+
 // NodeServices is used to query service registrations by node ID.
 func (s *StateStore) NodeServices(nodeID string) (uint64, *structs.NodeServices, error) {
 	tx := s.db.Txn(false)
@@ -1056,6 +1084,27 @@ func (s *StateStore) ensureCheckTxn(tx *memdb.Txn, idx uint64, watches *DumbWatc
 	return nil
 }
 
+// NodeCheck is used to retrieve a specific check associated with the given
+// node.
+func (s *StateStore) NodeCheck(nodeID string, checkID types.CheckID) (uint64, *structs.HealthCheck, error) {
+	tx := s.db.Txn(false)
+	defer tx.Abort()
+
+	// Get the table index.
+	idx := maxIndexTxn(tx, s.getWatchTables("NodeCheck")...)
+
+	// Return the check.
+	check, err := tx.First("checks", "id", nodeID, string(checkID))
+	if err != nil {
+		return 0, nil, fmt.Errorf("failed check lookup: %s", err)
+	}
+	if check != nil {
+		return idx, check.(*structs.HealthCheck), nil
+	} else {
+		return idx, nil, nil
+	}
+}
+
 // NodeChecks is used to retrieve checks associated with the
 // given node from the state store.
 func (s *StateStore) NodeChecks(nodeID string) (uint64, structs.HealthChecks, error) {

consul/state/state_store_test.go

@@ -284,11 +284,24 @@ func TestStateStore_EnsureRegistration(t *testing.T) {
 		if len(out.Services) != 1 {
 			t.Fatalf("bad: %#v", out.Services)
 		}
-		s := out.Services["redis1"]
+		r := out.Services["redis1"]
-		if s.ID != "redis1" || s.Service != "redis" ||
+		if r == nil || r.ID != "redis1" || r.Service != "redis" ||
-			s.Address != "1.1.1.1" || s.Port != 8080 ||
+			r.Address != "1.1.1.1" || r.Port != 8080 ||
-			s.CreateIndex != created || s.ModifyIndex != modified {
+			r.CreateIndex != created || r.ModifyIndex != modified {
-			t.Fatalf("bad service returned: %#v", s)
+			t.Fatalf("bad service returned: %#v", r)
+		}
+
+		idx, r, err = s.NodeService("node1", "redis1")
+		if err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		if idx != modified {
+			t.Fatalf("bad index: %d", idx)
+		}
+		if r == nil || r.ID != "redis1" || r.Service != "redis" ||
+			r.Address != "1.1.1.1" || r.Port != 8080 ||
+			r.CreateIndex != created || r.ModifyIndex != modified {
+			t.Fatalf("bad service returned: %#v", r)
 		}
 	}
 	verifyNode(1, 2)
@@ -321,6 +334,18 @@ func TestStateStore_EnsureRegistration(t *testing.T) {
 			c.CreateIndex != created || c.ModifyIndex != modified {
 			t.Fatalf("bad check returned: %#v", c)
 		}
+
+		idx, c, err = s.NodeCheck("node1", "check1")
+		if err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		if idx != modified {
+			t.Fatalf("bad index: %d", idx)
+		}
+		if c.Node != "node1" || c.CheckID != "check1" || c.Name != "check" ||
+			c.CreateIndex != created || c.ModifyIndex != modified {
+			t.Fatalf("bad check returned: %#v", c)
+		}
 	}
 	verifyNode(1, 3)
 	verifyService(2, 3)

consul/structs/structs.go

@@ -183,6 +183,25 @@ func (r *RegisterRequest) RequestDatacenter() string {
 	return r.Datacenter
 }
 
+// ChangesNode returns true if the given register request changes the given
+// node, which can be nil. This only looks for changes to the node record itself,
+// not any of the health checks.
+func (r *RegisterRequest) ChangesNode(node *Node) bool {
+	// This means it's creating the node.
+	if node == nil {
+		return true
+	}
+
+	// Check if any of the node-level fields are being changed.
+	if r.Node != node.Node ||
+		r.Address != node.Address ||
+		!reflect.DeepEqual(r.TaggedAddresses, node.TaggedAddresses) {
+		return true
+	}
+
+	return false
+}
+
 // DeregisterRequest is used for the Catalog.Deregister endpoint
 // to deregister a node as providing a service. If no service is
 // provided the entire node is deregistered.

consul/structs/structs_test.go

@@ -105,6 +105,44 @@ func TestStructs_ACL_IsSame(t *testing.T) {
 	check(func() { other.Rules = "" }, func() { other.Rules = "service \"\" { policy = \"read\" }" })
 }
 
+func TestStructs_RegisterRequest_ChangesNode(t *testing.T) {
+	req := &RegisterRequest{
+		Node:            "test",
+		Address:         "127.0.0.1",
+		TaggedAddresses: make(map[string]string),
+	}
+
+	node := &Node{
+		Node:            "test",
+		Address:         "127.0.0.1",
+		TaggedAddresses: make(map[string]string),
+	}
+
+	check := func(twiddle, restore func()) {
+		if req.ChangesNode(node) {
+			t.Fatalf("should not change")
+		}
+
+		twiddle()
+		if !req.ChangesNode(node) {
+			t.Fatalf("should change")
+		}
+
+		restore()
+		if req.ChangesNode(node) {
+			t.Fatalf("should not change")
+		}
+	}
+
+	check(func() { req.Node = "nope" }, func() { req.Node = "test" })
+	check(func() { req.Address = "127.0.0.2" }, func() { req.Address = "127.0.0.1" })
+	check(func() { req.TaggedAddresses["wan"] = "nope" }, func() { delete(req.TaggedAddresses, "wan") })
+
+	if !req.ChangesNode(nil) {
+		t.Fatalf("should change")
+	}
+}
+
 // testServiceNode gives a fully filled out ServiceNode instance.
 func testServiceNode() *ServiceNode {
 	return &ServiceNode{

website/source/docs/agent/options.html.markdown

@@ -377,6 +377,13 @@ 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".
 
+* <a name="acl_enforce_version_8"></a><a href="#acl_enforce_version_8">`acl_enforce_version_8`</a> -
+  Used for clients and servers to determine if enforcement should occur for new ACL policies being
+  previewed before Consul 0.8. Added in Consul 0.7.2, this will default to false in versions of
+  Consul prior to 0.8, and will default to true in Consul 0.8 and later. This helps ease the
+  transition to the new ACL features by allowing policies to be in place before enforcement begins.
+  Please see the [ACL internals guide](/docs/internals/acl.htmlXS) for more details.
+
 * <a name="acl_master_token"></a><a href="#acl_master_token">`acl_master_token`</a> - Only used
   for servers in the [`acl_datacenter`](#acl_datacenter). This token will be created with management-level
   permissions if it does not exist. It allows operators to bootstrap the ACL system