From 66b437ca338b1e3d9d7f906adff9c5f975bc73cb Mon Sep 17 00:00:00 2001
From: James Phillips <slackpad@gmail.com>
Date: Wed, 7 Dec 2016 17:58:23 -0800
Subject: [PATCH] Removes the exception for the "consul" service in the
 catalog.

---
 command/agent/agent.go          |  4 ++++
 consul/catalog_endpoint.go      | 22 +++++++++++++---------
 consul/catalog_endpoint_test.go | 18 +++++++++++++++++-
 consul/config.go                |  4 ++++
 4 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/command/agent/agent.go b/command/agent/agent.go
index eef2992d8a..23c7f4d557 100644
--- a/command/agent/agent.go
+++ b/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
 	}
diff --git a/consul/catalog_endpoint.go b/consul/catalog_endpoint.go
index 06f81993b9..7b85aeec94 100644
--- a/consul/catalog_endpoint.go
+++ b/consul/catalog_endpoint.go
@@ -26,6 +26,12 @@ func (c *Catalog) Register(args *structs.RegisterRequest, reply *struct{}) error
 		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
+	}
+
 	if args.Service != nil {
 		// If no service id, but service name, use default
 		if args.Service.ID == "" && args.Service.Service != "" {
@@ -37,14 +43,12 @@ func (c *Catalog) Register(args *structs.RegisterRequest, reply *struct{}) error
 			return fmt.Errorf("Must provide service name with ID")
 		}
 
-		// Apply the ACL policy if any
-		// The 'consul' service is excluded since it is managed
-		// automatically internally.
-		if args.Service.Service != ConsulServiceName {
-			acl, err := c.srv.resolveToken(args.Token)
-			if err != nil {
-				return err
-			} else if acl != nil && !acl.ServiceWrite(args.Service.Service) {
+		// Apply the ACL policy if any. The 'consul' service is excluded
+		// since it is managed automatically internally (that behavior
+		// is going away after version 0.8).
+		if c.srv.config.ACLEnforceVersion8 ||
+			(args.Service.Service != ConsulServiceName) {
+			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
@@ -65,7 +69,7 @@ func (c *Catalog) Register(args *structs.RegisterRequest, reply *struct{}) error
 		}
 	}
 
-	_, err := c.srv.raftApply(structs.RegisterRequestType, args)
+	_, err = c.srv.raftApply(structs.RegisterRequestType, args)
 	if err != nil {
 		c.srv.logger.Printf("[ERR] consul.catalog: Register failed: %v", err)
 		return err
diff --git a/consul/catalog_endpoint_test.go b/consul/catalog_endpoint_test.go
index 8171551121..4282eea031 100644
--- a/consul/catalog_endpoint_test.go
+++ b/consul/catalog_endpoint_test.go
@@ -46,11 +46,12 @@ func TestCatalogRegister(t *testing.T) {
 	})
 }
 
-func TestCatalogRegister_ACLDeny(t *testing.T) {
+func TestCatalogRegister_ACLDeny_Service(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()
@@ -99,6 +100,21 @@ func TestCatalogRegister_ACLDeny(t *testing.T) {
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
+
+	// Try the special case for the "consul" service.
+	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)
+	}
 }
 
 func TestCatalogRegister_ForwardLeader(t *testing.T) {
diff --git a/consul/config.go b/consul/config.go
index 0e094f305b..0259b3d31d 100644
--- a/consul/config.go
+++ b/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