Merge pull request #506 from hashicorp/f-service-acl

Service ACL support

acl/acl.go

@@ -46,6 +46,12 @@ 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
+
 	// ACLList checks for permission to list all the ACLs
 	ACLList() bool
 
@@ -73,6 +79,14 @@ 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) ACLList() bool {
 	return s.allowManage
 }
@@ -119,20 +133,29 @@ type PolicyACL struct {
 
 	// keyRules contains the key policies
 	keyRules *radix.Tree
+
+	// serviceRules contains the service policies
+	serviceRules map[string]string
 }
 
 // New is used to construct a policy based ACL from a set of policies
 // and a parent policy to resolve missing cases.
 func New(parent ACL, policy *Policy) (*PolicyACL, error) {
 	p := &PolicyACL{
-		parent:   parent,
+		parent:       parent,
-		keyRules: radix.New(),
+		keyRules:     radix.New(),
+		serviceRules: make(map[string]string, len(policy.Services)),
 	}
 
 	// Load the key policy
 	for _, kp := range policy.Keys {
 		p.keyRules.Insert(kp.Prefix, kp.Policy)
 	}
+
+	// Load the service policy
+	for _, sp := range policy.Services {
+		p.serviceRules[sp.Name] = sp.Policy
+	}
 	return p, nil
 }
 
@@ -205,6 +228,48 @@ func (p *PolicyACL) KeyWritePrefix(prefix string) bool {
 	return p.parent.KeyWritePrefix(prefix)
 }
 
+// ServiceRead checks if reading (discovery) of a service is allowed
+func (p *PolicyACL) ServiceRead(name string) bool {
+	// Check for an exact rule or catch-all
+	rule, ok := p.serviceRules[name]
+	if !ok {
+		rule, ok = p.serviceRules[""]
+	}
+	if ok {
+		switch rule {
+		case ServicePolicyWrite:
+			return true
+		case ServicePolicyRead:
+			return true
+		default:
+			return false
+		}
+	}
+
+	// No matching rule, use the parent.
+	return p.parent.ServiceRead(name)
+}
+
+// ServiceWrite checks if writing (registering) a service is allowed
+func (p *PolicyACL) ServiceWrite(name string) bool {
+	// Check for an exact rule or catch-all
+	rule, ok := p.serviceRules[name]
+	if !ok {
+		rule, ok = p.serviceRules[""]
+	}
+	if ok {
+		switch rule {
+		case ServicePolicyWrite:
+			return true
+		default:
+			return false
+		}
+	}
+
+	// No matching rule, use the parent.
+	return p.parent.ServiceWrite(name)
+}
+
 // ACLList checks if listing of ACLs is allowed
 func (p *PolicyACL) ACLList() bool {
 	return p.parent.ACLList()

acl/acl_test.go

@@ -41,6 +41,12 @@ func TestStaticACL(t *testing.T) {
 	if !all.KeyWrite("foobar") {
 		t.Fatalf("should allow")
 	}
+	if !all.ServiceRead("foobar") {
+		t.Fatalf("should allow")
+	}
+	if !all.ServiceWrite("foobar") {
+		t.Fatalf("should allow")
+	}
 	if all.ACLList() {
 		t.Fatalf("should not allow")
 	}
@@ -54,6 +60,12 @@ func TestStaticACL(t *testing.T) {
 	if none.KeyWrite("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.ACLList() {
 		t.Fatalf("should not noneow")
 	}
@@ -67,6 +79,12 @@ func TestStaticACL(t *testing.T) {
 	if !manage.KeyWrite("foobar") {
 		t.Fatalf("should allow")
 	}
+	if !manage.ServiceRead("foobar") {
+		t.Fatalf("should allow")
+	}
+	if !manage.ServiceWrite("foobar") {
+		t.Fatalf("should allow")
+	}
 	if !manage.ACLList() {
 		t.Fatalf("should allow")
 	}
@@ -96,19 +114,33 @@ func TestPolicyACL(t *testing.T) {
 				Policy: KeyPolicyRead,
 			},
 		},
+		Services: []*ServicePolicy{
+			&ServicePolicy{
+				Name:   "",
+				Policy: ServicePolicyWrite,
+			},
+			&ServicePolicy{
+				Name:   "foo",
+				Policy: ServicePolicyRead,
+			},
+			&ServicePolicy{
+				Name:   "bar",
+				Policy: ServicePolicyDeny,
+			},
+		},
 	}
 	acl, err := New(all, policy)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
 
-	type tcase struct {
+	type keycase struct {
 		inp         string
 		read        bool
 		write       bool
 		writePrefix bool
 	}
-	cases := []tcase{
+	cases := []keycase{
 		{"other", true, true, true},
 		{"foo/test", true, true, true},
 		{"foo/priv/test", false, false, false},
@@ -128,6 +160,26 @@ func TestPolicyACL(t *testing.T) {
 			t.Fatalf("Write prefix fail: %#v", c)
 		}
 	}
+
+	// Test the services
+	type servicecase struct {
+		inp   string
+		read  bool
+		write bool
+	}
+	scases := []servicecase{
+		{"other", true, true},
+		{"foo", true, false},
+		{"bar", false, false},
+	}
+	for _, c := range scases {
+		if c.read != acl.ServiceRead(c.inp) {
+			t.Fatalf("Read fail: %#v", c)
+		}
+		if c.write != acl.ServiceWrite(c.inp) {
+			t.Fatalf("Write fail: %#v", c)
+		}
+	}
 }
 
 func TestPolicyACL_Parent(t *testing.T) {
@@ -143,6 +195,16 @@ func TestPolicyACL_Parent(t *testing.T) {
 				Policy: KeyPolicyRead,
 			},
 		},
+		Services: []*ServicePolicy{
+			&ServicePolicy{
+				Name:   "other",
+				Policy: ServicePolicyWrite,
+			},
+			&ServicePolicy{
+				Name:   "foo",
+				Policy: ServicePolicyRead,
+			},
+		},
 	}
 	root, err := New(deny, policyRoot)
 	if err != nil {
@@ -164,19 +226,25 @@ func TestPolicyACL_Parent(t *testing.T) {
 				Policy: KeyPolicyRead,
 			},
 		},
+		Services: []*ServicePolicy{
+			&ServicePolicy{
+				Name:   "bar",
+				Policy: ServicePolicyDeny,
+			},
+		},
 	}
 	acl, err := New(root, policy)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
 
-	type tcase struct {
+	type keycase struct {
 		inp         string
 		read        bool
 		write       bool
 		writePrefix bool
 	}
-	cases := []tcase{
+	cases := []keycase{
 		{"other", false, false, false},
 		{"foo/test", true, true, true},
 		{"foo/priv/test", true, false, false},
@@ -194,4 +262,25 @@ func TestPolicyACL_Parent(t *testing.T) {
 			t.Fatalf("Write prefix fail: %#v", c)
 		}
 	}
+
+	// Test the services
+	type servicecase struct {
+		inp   string
+		read  bool
+		write bool
+	}
+	scases := []servicecase{
+		{"fail", false, false},
+		{"other", true, true},
+		{"foo", true, false},
+		{"bar", false, false},
+	}
+	for _, c := range scases {
+		if c.read != acl.ServiceRead(c.inp) {
+			t.Fatalf("Read fail: %#v", c)
+		}
+		if c.write != acl.ServiceWrite(c.inp) {
+			t.Fatalf("Write fail: %#v", c)
+		}
+	}
 }

acl/policy.go

@@ -2,20 +2,25 @@ package acl
 
 import (
 	"fmt"
+
 	"github.com/hashicorp/hcl"
 )
 
 const (
-	KeyPolicyDeny  = "deny"
+	KeyPolicyDeny      = "deny"
-	KeyPolicyRead  = "read"
+	KeyPolicyRead      = "read"
-	KeyPolicyWrite = "write"
+	KeyPolicyWrite     = "write"
+	ServicePolicyDeny  = "deny"
+	ServicePolicyRead  = "read"
+	ServicePolicyWrite = "write"
 )
 
 // Policy is used to represent the policy specified by
 // an ACL configuration.
 type Policy struct {
-	ID   string       `hcl:"-"`
+	ID       string           `hcl:"-"`
-	Keys []*KeyPolicy `hcl:"key,expand"`
+	Keys     []*KeyPolicy     `hcl:"key,expand"`
+	Services []*ServicePolicy `hcl:"service,expand"`
 }
 
 // KeyPolicy represents a policy for a key
@@ -28,6 +33,16 @@ func (k *KeyPolicy) GoString() string {
 	return fmt.Sprintf("%#v", *k)
 }
 
+// ServicePolicy represents a policy for a service
+type ServicePolicy struct {
+	Name   string `hcl:",key"`
+	Policy string
+}
+
+func (k *ServicePolicy) GoString() string {
+	return fmt.Sprintf("%#v", *k)
+}
+
 // Parse is used to parse the specified ACL rules into an
 // intermediary set of policies, before being compiled into
 // the ACL
@@ -53,5 +68,17 @@ func Parse(rules string) (*Policy, error) {
 			return nil, fmt.Errorf("Invalid key policy: %#v", kp)
 		}
 	}
+
+	// Validate the service policy
+	for _, sp := range p.Services {
+		switch sp.Policy {
+		case ServicePolicyDeny:
+		case ServicePolicyRead:
+		case ServicePolicyWrite:
+		default:
+			return nil, fmt.Errorf("Invalid service policy: %#v", sp)
+		}
+	}
+
 	return p, nil
 }

acl/policy_test.go

@@ -18,6 +18,12 @@ key "foo/bar/" {
 }
 key "foo/bar/baz" {
 	policy = "deny"
+}
+service "" {
+	policy = "write"
+}
+service "foo" {
+	policy = "read"
 }
 	`
 	exp := &Policy{
@@ -39,6 +45,16 @@ key "foo/bar/baz" {
 				Policy: KeyPolicyDeny,
 			},
 		},
+		Services: []*ServicePolicy{
+			&ServicePolicy{
+				Name:   "",
+				Policy: ServicePolicyWrite,
+			},
+			&ServicePolicy{
+				Name:   "foo",
+				Policy: ServicePolicyRead,
+			},
+		},
 	}
 
 	out, err := Parse(inp)
@@ -66,6 +82,14 @@ func TestParse_JSON(t *testing.T) {
 		"foo/bar/baz": {
 			"policy": "deny"
 		}
+	},
+	"service": {
+		"": {
+			"policy": "write"
+		},
+		"foo": {
+			"policy": "read"
+		}
 	}
 }`
 	exp := &Policy{
@@ -87,6 +111,16 @@ func TestParse_JSON(t *testing.T) {
 				Policy: KeyPolicyDeny,
 			},
 		},
+		Services: []*ServicePolicy{
+			&ServicePolicy{
+				Name:   "",
+				Policy: ServicePolicyWrite,
+			},
+			&ServicePolicy{
+				Name:   "foo",
+				Policy: ServicePolicyRead,
+			},
+		},
 	}
 
 	out, err := Parse(inp)

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 {

consul/catalog_endpoint_test.go

@@ -45,6 +45,61 @@ 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"
+	})
+	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)
+	}
+
+	argR.Service.Service = "foo"
+	err = client.Call("Catalog.Register", &argR, &outR)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+}
+
 func TestCatalogRegister_ForwardLeader(t *testing.T) {
 	dir1, s1 := testServer(t)
 	defer os.RemoveAll(dir1)
@@ -722,3 +777,9 @@ func TestCatalogRegister_FailedCase1(t *testing.T) {
 		t.Fatalf("Bad: %v", out2)
 	}
 }
+
+var testRegisterRules = `
+service "foo" {
+	policy = "write"
+}
+`

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)

website/source/docs/internals/acl.html.markdown

@@ -114,6 +114,16 @@ key "foo/private/" {
   # Deny access to the private dir
   policy = "deny"
 }
+
+# Default all services to allowing registration
+service "" {
+    policy = "write"
+}
+
+service "secure" {
+    # Deny registration access to secure service
+    policy = "read"
+}
 ```
 
 This is equivalent to the following JSON input:
@@ -122,14 +132,22 @@ This is equivalent to the following JSON input:
 {
   "key": {
     "": {
-      "policy": "read",
+      "policy": "read"
     },
     "foo/": {
-      "policy": "write",
+      "policy": "write"
     },
     "foo/private": {
-      "policy": "deny",
+      "policy": "deny"
     }
+  },
+  "service": {
+      "": {
+          "policy": "write"
+      },
+      "secure": {
+          "policy": "read"
+      }
   }
 }
 ```
@@ -139,3 +157,12 @@ using a longest-prefix match policy. This means we pick the most specific
 policy possible. 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.
+
+Services policies provide both a service name and a policy. The rules are
+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 policy for
+the "consul" service is always "write" as it is managed internally.
+