From 9d6567ff083a279d6a4f77b7e14d5b4e9507f7cb Mon Sep 17 00:00:00 2001
From: James Phillips <slackpad@gmail.com>
Date: Fri, 9 Dec 2016 21:04:00 -0800
Subject: [PATCH] Adds complete ACL coverage for /v1/catalog/node/<node>.

---
 consul/catalog_endpoint.go      | 15 +++++++++
 consul/catalog_endpoint_test.go | 60 ++++++++++++++++++++++++++++++++-
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/consul/catalog_endpoint.go b/consul/catalog_endpoint.go
index ce61547b3a..a943aa91cd 100644
--- a/consul/catalog_endpoint.go
+++ b/consul/catalog_endpoint.go
@@ -267,6 +267,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)
 		})
diff --git a/consul/catalog_endpoint_test.go b/consul/catalog_endpoint_test.go
index bc01084d3a..afb41a34b5 100644
--- a/consul/catalog_endpoint_test.go
+++ b/consul/catalog_endpoint_test.go
@@ -1191,7 +1191,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()
@@ -1424,6 +1424,64 @@ func TestCatalog_ServiceNodes_FilterACL(t *testing.T) {
 	}
 }
 
+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)
+	}
+
+	// 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 = "write"
+}
+`, 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)
+	}
+}
+
 func TestCatalog_NodeServices_FilterACL(t *testing.T) {
 	dir, token, srv, codec := testACLFilterServer(t)
 	defer os.RemoveAll(dir)