From cafba938694327decf3ad91a7c30de668f2de25e Mon Sep 17 00:00:00 2001
From: Armon Dadgar <armon.dadgar@gmail.com>
Date: Sun, 30 Nov 2014 21:05:15 -0700
Subject: [PATCH] consul: Enforce service registration ACLs

---
 consul/catalog_endpoint.go                    | 19 ++++++-
 consul/catalog_endpoint_test.go               | 56 +++++++++++++++++++
 consul/leader.go                              |  8 +++
 .../source/docs/internals/acl.html.markdown   |  3 +-
 4 files changed, 83 insertions(+), 3 deletions(-)

diff --git a/consul/catalog_endpoint.go b/consul/catalog_endpoint.go
index 9fb982e145..24995fdeb4 100644
--- a/consul/catalog_endpoint.go
+++ b/consul/catalog_endpoint.go
@@ -2,10 +2,11 @@ package consul
 
 import (
 	"fmt"
-	"github.com/armon/go-metrics"
-	"github.com/hashicorp/consul/consul/structs"
 	"sort"
 	"time"
+
+	"github.com/armon/go-metrics"
+	"github.com/hashicorp/consul/consul/structs"
 )
 
 // Catalog endpoint is used to manipulate the service catalog
@@ -35,6 +36,20 @@ func (c *Catalog) Register(args *structs.RegisterRequest, reply *struct{}) error
 		if args.Service.ID != "" && args.Service.Service == "" {
 			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) {
+				c.srv.logger.Printf("[WARN] consul.catalog: Register of service '%s' on '%s' denied due to ACLs",
+					args.Service.Service, args.Node)
+				return permissionDeniedErr
+			}
+		}
 	}
 
 	if args.Check != nil {
diff --git a/consul/catalog_endpoint_test.go b/consul/catalog_endpoint_test.go
index 4ba76ce014..9c4e86ea9a 100644
--- a/consul/catalog_endpoint_test.go
+++ b/consul/catalog_endpoint_test.go
@@ -45,6 +45,56 @@ func TestCatalogRegister(t *testing.T) {
 	})
 }
 
+func TestCatalogRegister_ACLDeny(t *testing.T) {
+	dir1, s1 := testServerWithConfig(t, func(c *Config) {
+		c.ACLDatacenter = "dc1"
+		c.ACLMasterToken = "root"
+		c.ACLDefaultPolicy = "deny"
+		c.ACLToken = "root"
+	})
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	client := rpcClient(t, s1)
+	defer client.Close()
+
+	testutil.WaitForLeader(t, client.Call, "dc1")
+
+	// Create the ACL
+	arg := structs.ACLRequest{
+		Datacenter: "dc1",
+		Op:         structs.ACLSet,
+		ACL: structs.ACL{
+			Name:  "User token",
+			Type:  structs.ACLTypeClient,
+			Rules: testRegisterRules,
+		},
+		WriteRequest: structs.WriteRequest{Token: "root"},
+	}
+	var out string
+	if err := client.Call("ACL.Apply", &arg, &out); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	id := out
+
+	argR := structs.RegisterRequest{
+		Datacenter: "dc1",
+		Node:       "foo",
+		Address:    "127.0.0.1",
+		Service: &structs.NodeService{
+			Service: "db",
+			Tags:    []string{"master"},
+			Port:    8000,
+		},
+		WriteRequest: structs.WriteRequest{Token: id},
+	}
+	var outR struct{}
+
+	err := client.Call("Catalog.Register", &argR, &outR)
+	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
+		t.Fatalf("err: %v", err)
+	}
+}
+
 func TestCatalogRegister_ForwardLeader(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
@@ -722,3 +772,9 @@ func TestCatalogRegister_FailedCase1(t *testing.T) {
 		t.Fatalf("Bad: %v", out2)
 	}
 }
+
+var testRegisterRules = `
+service "foo" {
+	policy = "read"
+}
+`
diff --git a/consul/leader.go b/consul/leader.go
index b41f612cbc..7f4f378a64 100644
--- a/consul/leader.go
+++ b/consul/leader.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/armon/go-metrics"
@@ -265,6 +266,11 @@ func (s *Server) reconcileMember(member serf.Member) error {
 	if err != nil {
 		s.logger.Printf("[ERR] consul: failed to reconcile member: %v: %v",
 			member, err)
+
+		// Permission denied should not bubble up
+		if strings.Contains(err.Error(), permissionDenied) {
+			return nil
+		}
 		return err
 	}
 	return nil
@@ -344,6 +350,7 @@ AFTER_CHECK:
 			Status:  structs.HealthPassing,
 			Output:  SerfCheckAliveOutput,
 		},
+		WriteRequest: structs.WriteRequest{Token: s.config.ACLToken},
 	}
 	var out struct{}
 	return s.endpoints.Catalog.Register(&req, &out)
@@ -379,6 +386,7 @@ func (s *Server) handleFailedMember(member serf.Member) error {
 			Status:  structs.HealthCritical,
 			Output:  SerfCheckFailedOutput,
 		},
+		WriteRequest: structs.WriteRequest{Token: s.config.ACLToken},
 	}
 	var out struct{}
 	return s.endpoints.Catalog.Register(&req, &out)
diff --git a/website/source/docs/internals/acl.html.markdown b/website/source/docs/internals/acl.html.markdown
index 33994e3acb..b847e17da5 100644
--- a/website/source/docs/internals/acl.html.markdown
+++ b/website/source/docs/internals/acl.html.markdown
@@ -163,5 +163,6 @@ enforced using an exact match policy. The default rule is provided using
 the empty string. The policy is either "read", "write", or "deny". A "write"
 policy implies "read", and there is no way to specify write-only. If there
 is no applicable rule, the `acl_default_policy` is applied. Currently, only
-the "write" level is enforced for registration of services.
+the "write" level is enforced for registration of services. The policy for
+the "consul" service is always "write" as it is managed internally.