Completes switch of prepared_query ACLs to govern query names.

acl/acl_test.go

@@ -352,6 +352,16 @@ func TestPolicyACL_Parent(t *testing.T) {
 				Policy: PolicyRead,
 			},
 		},
+		PreparedQueries: []*PreparedQueryPolicy{
+			&PreparedQueryPolicy{
+				Prefix: "other",
+				Policy: PolicyWrite,
+			},
+			&PreparedQueryPolicy{
+				Prefix: "foo",
+				Policy: PolicyRead,
+			},
+		},
 	}
 	root, err := New(deny, policyRoot)
 	if err != nil {
@@ -379,6 +389,12 @@ func TestPolicyACL_Parent(t *testing.T) {
 				Policy: PolicyDeny,
 			},
 		},
+		PreparedQueries: []*PreparedQueryPolicy{
+			&PreparedQueryPolicy{
+				Prefix: "bar",
+				Policy: PolicyDeny,
+			},
+		},
 	}
 	acl, err := New(root, policy)
 	if err != nil {
@@ -431,6 +447,29 @@ func TestPolicyACL_Parent(t *testing.T) {
 		}
 	}
 
+	// Test prepared queries
+	type querycase struct {
+		inp   string
+		read  bool
+		write bool
+	}
+	querycases := []querycase{
+		{"foo", true, false},
+		{"foobar", true, false},
+		{"bar", false, false},
+		{"barbaz", false, false},
+		{"baz", false, false},
+		{"nope", false, false},
+	}
+	for _, c := range querycases {
+		if c.read != acl.PreparedQueryRead(c.inp) {
+			t.Fatalf("Prepared query fail: %#v", c)
+		}
+		if c.write != acl.PreparedQueryWrite(c.inp) {
+			t.Fatalf("Prepared query fail: %#v", c)
+		}
+	}
+
 	// Check some management functions that chain up
 	if acl.ACLList() {
 		t.Fatalf("should not allow")

consul/acl.go

@@ -377,19 +377,29 @@ func (f *aclFilter) filterNodeDump(dump *structs.NodeDump) {
 // management tokens in Consul 0.6.3 and earlier, and we don't want to
 // willy-nilly show those. This does have the limitation of preventing delegated
 // non-management users from seeing captured tokens, but they can at least see
-// that they are set and we want to discourage using them going forward.
+// that they are set.
 func (f *aclFilter) filterPreparedQueries(queries *structs.PreparedQueries) {
-	isManagementToken := f.acl.ACLList()
+	// Management tokens can see everything with no filtering.
+	if f.acl.ACLList() {
+		return
+	}
+
+	// Otherwise, we need to see what the token has access to.
 	ret := make(structs.PreparedQueries, 0, len(*queries))
 	for _, query := range *queries {
-		if !f.acl.PreparedQueryRead(query.GetACLPrefix()) {
+		// If no prefix ACL applies to this query then filter it, since
+		// we know at this point the user doesn't have a management
+		// token.
+		prefix := query.GetACLPrefix()
+		if prefix == nil || !f.acl.PreparedQueryRead(*prefix) {
 			f.logger.Printf("[DEBUG] consul: dropping prepared query %q from result due to ACLs", query.ID)
 			continue
 		}
 
-		// Secure tokens unless there's a management token doing the
+		// Let the user see if there's a blank token, otherwise we need
-		// query.
+		// to redact it, since we know they don't have a management
-		if isManagementToken || query.Token == "" {
+		// token.
+		if query.Token == "" {
 			ret = append(ret, query)
 		} else {
 			// Redact the token, using a copy of the query structure

consul/acl_test.go

@@ -866,37 +866,35 @@ func TestACL_filterPreparedQueries(t *testing.T) {
 	queries := structs.PreparedQueries{
 		&structs.PreparedQuery{
 			ID: "f004177f-2c28-83b7-4229-eacc25fe55d1",
-			Service: structs.ServiceQuery{
-				Service: "foo",
-			},
 		},
 		&structs.PreparedQuery{
 			ID:   "f004177f-2c28-83b7-4229-eacc25fe55d2",
-			Token: "root",
+			Name: "query-with-no-token",
-			Service: structs.ServiceQuery{
-				Service: "bar",
 		},
+		&structs.PreparedQuery{
+			ID:    "f004177f-2c28-83b7-4229-eacc25fe55d3",
+			Name:  "query-with-a-token",
+			Token: "root",
 		},
 	}
 
 	expected := structs.PreparedQueries{
 		&structs.PreparedQuery{
 			ID: "f004177f-2c28-83b7-4229-eacc25fe55d1",
-			Service: structs.ServiceQuery{
-				Service: "foo",
-			},
 		},
 		&structs.PreparedQuery{
 			ID:   "f004177f-2c28-83b7-4229-eacc25fe55d2",
-			Token: "root",
+			Name: "query-with-no-token",
-			Service: structs.ServiceQuery{
-				Service: "bar",
 		},
+		&structs.PreparedQuery{
+			ID:    "f004177f-2c28-83b7-4229-eacc25fe55d3",
+			Name:  "query-with-a-token",
+			Token: "root",
 		},
 	}
 
 	// Try permissive filtering with a management token. This will allow the
-	// embedded tokens to be seen.
+	// embedded token to be seen.
 	filt := newAclFilter(acl.ManageAll(), nil)
 	filt.filterPreparedQueries(&queries)
 	if !reflect.DeepEqual(queries, expected) {
@@ -905,13 +903,15 @@ func TestACL_filterPreparedQueries(t *testing.T) {
 
 	// Hang on to the entry with a token, which needs to survive the next
 	// operation.
-	original := queries[1]
+	original := queries[2]
 
 	// Now try permissive filtering with a client token, which should cause
-	// the embedded tokens to get redacted.
+	// the embedded token to get redacted, and the query with no name to get
+	// filtered out.
 	filt = newAclFilter(acl.AllowAll(), nil)
 	filt.filterPreparedQueries(&queries)
-	expected[1].Token = redactedToken
+	expected[2].Token = redactedToken
+	expected = append(structs.PreparedQueries{}, expected[1], expected[2])
 	if !reflect.DeepEqual(queries, expected) {
 		t.Fatalf("bad: %#v", queries)
 	}

consul/prepared_query_endpoint.go

@@ -56,21 +56,25 @@ func (p *PreparedQuery) Apply(args *structs.PreparedQueryRequest, reply *string)
 	}
 	*reply = args.Query.ID
 
-	// Do an ACL check. We need to make sure they are allowed to write
+	// Get the ACL token for the request for the checks below.
-	// to whatever prefix is incoming, and any existing prefix if this
-	// isn't a create operation.
 	acl, err := p.srv.resolveToken(args.Token)
 	if err != nil {
 		return err
 	}
-	if acl != nil && !acl.PreparedQueryWrite(args.Query.GetACLPrefix()) {
+
+	// If prefix ACLs apply to the incoming query, then do an ACL check. We
+	// need to make sure they have write access for whatever they are
+	// proposing.
+	if prefix := args.Query.GetACLPrefix(); prefix != nil {
+		if acl != nil && !acl.PreparedQueryWrite(*prefix) {
 			p.srv.logger.Printf("[WARN] consul.prepared_query: Operation on prepared query '%s' denied due to ACLs", args.Query.ID)
 			return permissionDeniedErr
 		}
+	}
 
 	// This is the second part of the check above. If they are referencing
-	// an existing query then make sure it exists and that they have perms
+	// an existing query then make sure it exists and that they have write
-	// that let the modify that prefix as well.
+	// access to whatever they are changing, if prefix ACLs apply to it.
 	if args.Op != structs.PreparedQueryCreate {
 		state := p.srv.fsm.State()
 		_, query, err := state.PreparedQueryGet(args.Query.ID)
@@ -80,11 +84,14 @@ func (p *PreparedQuery) Apply(args *structs.PreparedQueryRequest, reply *string)
 		if query == nil {
 			return fmt.Errorf("Cannot modify non-existent prepared query: '%s'", args.Query.ID)
 		}
-		if acl != nil && !acl.PreparedQueryWrite(query.GetACLPrefix()) {
+
+		if prefix := query.GetACLPrefix(); prefix != nil {
+			if acl != nil && !acl.PreparedQueryWrite(*prefix) {
 				p.srv.logger.Printf("[WARN] consul.prepared_query: Operation on prepared query '%s' denied due to ACLs", args.Query.ID)
 				return permissionDeniedErr
 			}
 		}
+	}
 
 	// Parse the query and prep it for the state store.
 	switch args.Op {
@@ -205,17 +212,24 @@ func (p *PreparedQuery) Get(args *structs.PreparedQuerySpecificRequest,
 				return ErrQueryNotFound
 			}
 
+			// If no prefix ACL applies to this query, then they are
+			// always allowed to see it if they have the ID.
 			reply.Index = index
 			reply.Queries = structs.PreparedQueries{query}
+			if prefix := query.GetACLPrefix(); prefix == nil {
+				return nil
+			}
+
+			// Otherwise, attempt to filter it the usual way.
 			if err := p.srv.filterACL(args.Token, reply); err != nil {
 				return err
 			}
 
-			// If ACLs filtered out query, then let them know that
+			// Since this is a GET of a specific query, if ACLs have
-			// access to this is forbidden, since they are requesting
+			// prevented us from returning something that exists,
-			// a specific query.
+			// then alert the user with a permission denied error.
 			if len(reply.Queries) == 0 {
-				p.srv.logger.Printf("[DEBUG] consul.prepared_query: Request to get prepared query '%s' denied due to ACLs", args.QueryID)
+				p.srv.logger.Printf("[WARN] consul.prepared_query: Request to get prepared query '%s' denied due to ACLs", args.QueryID)
 				return permissionDeniedErr
 			}
 

consul/prepared_query_endpoint_test.go

@@ -27,25 +27,6 @@ func TestPreparedQuery_Apply(t *testing.T) {
 
 	testutil.WaitForLeader(t, s1.RPC, "dc1")
 
-	// Set up a node and service in the catalog.
-	{
-		req := structs.RegisterRequest{
-			Datacenter: "dc1",
-			Node:       "foo",
-			Address:    "127.0.0.1",
-			Service: &structs.NodeService{
-				Service: "redis",
-				Tags:    []string{"master"},
-				Port:    8000,
-			},
-		}
-		var reply struct{}
-		err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &req, &reply)
-		if err != nil {
-			t.Fatalf("err: %v", err)
-		}
-	}
-
 	// Set up a bare bones query.
 	query := structs.PreparedQueryRequest{
 		Datacenter: "dc1",
@@ -233,33 +214,14 @@ func TestPreparedQuery_Apply_ACLDeny(t *testing.T) {
 		}
 	}
 
-	// Set up a node and service in the catalog.
-	{
-		req := structs.RegisterRequest{
-			Datacenter: "dc1",
-			Node:       "foo",
-			Address:    "127.0.0.1",
-			Service: &structs.NodeService{
-				Service: "redis",
-				Tags:    []string{"master"},
-				Port:    8000,
-			},
-			WriteRequest: structs.WriteRequest{Token: "root"},
-		}
-		var reply struct{}
-		err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &req, &reply)
-		if err != nil {
-			t.Fatalf("err: %v", err)
-		}
-	}
-
 	// Set up a bare bones query.
 	query := structs.PreparedQueryRequest{
 		Datacenter: "dc1",
 		Op:         structs.PreparedQueryCreate,
 		Query: &structs.PreparedQuery{
+			Name: "redis-master",
 			Service: structs.ServiceQuery{
-				Service: "redis",
+				Service: "the-redis",
 			},
 		},
 	}
@@ -423,41 +385,6 @@ func TestPreparedQuery_Apply_ACLDeny(t *testing.T) {
 		}
 	}
 
-	// Make another query.
-	query.Op = structs.PreparedQueryCreate
-	query.Query.ID = ""
-	query.WriteRequest.Token = token
-	if err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply); err != nil {
-		t.Fatalf("err: %v", err)
-	}
-
-	// Check that it's there and that the token did not get captured.
-	query.Query.ID = reply
-	query.Query.Token = ""
-	{
-		req := &structs.PreparedQuerySpecificRequest{
-			Datacenter:   "dc1",
-			QueryID:      query.Query.ID,
-			QueryOptions: structs.QueryOptions{Token: "root"},
-		}
-		var resp structs.IndexedPreparedQueries
-		if err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Get", req, &resp); err != nil {
-			t.Fatalf("err: %v", err)
-		}
-
-		if len(resp.Queries) != 1 {
-			t.Fatalf("bad: %v", resp)
-		}
-		actual := resp.Queries[0]
-		if resp.Index != actual.ModifyIndex {
-			t.Fatalf("bad index: %d", resp.Index)
-		}
-		actual.CreateIndex, actual.ModifyIndex = 0, 0
-		if !reflect.DeepEqual(actual, query.Query) {
-			t.Fatalf("bad: %v", actual)
-		}
-	}
-
 	// A management token should be able to delete the query no matter what.
 	query.Op = structs.PreparedQueryDelete
 	query.WriteRequest.Token = "root"
@@ -484,10 +411,10 @@ func TestPreparedQuery_Apply_ACLDeny(t *testing.T) {
 		}
 	}
 
-	// Use the root token to make a query for a different service.
+	// Use the root token to make a query under a different name.
 	query.Op = structs.PreparedQueryCreate
 	query.Query.ID = ""
-	query.Query.Service.Service = "cassandra"
+	query.Query.Name = "cassandra"
 	query.WriteRequest.Token = "root"
 	if err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply); err != nil {
 		t.Fatalf("err: %v", err)
@@ -523,7 +450,7 @@ func TestPreparedQuery_Apply_ACLDeny(t *testing.T) {
 	// Now try to change that to redis with the valid redis token. This will
 	// fail because that token can't change cassandra queries.
 	query.Op = structs.PreparedQueryUpdate
-	query.Query.Service.Service = "redis"
+	query.Query.Name = "redis"
 	query.WriteRequest.Token = token
 	err = msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply)
 	if err == nil || !strings.Contains(err.Error(), permissionDenied) {
@@ -678,34 +605,14 @@ func TestPreparedQuery_Get(t *testing.T) {
 		}
 	}
 
-	// Set up a node and service in the catalog.
-	{
-		req := structs.RegisterRequest{
-			Datacenter: "dc1",
-			Node:       "foo",
-			Address:    "127.0.0.1",
-			Service: &structs.NodeService{
-				Service: "redis",
-				Tags:    []string{"master"},
-				Port:    8000,
-			},
-			WriteRequest: structs.WriteRequest{Token: "root"},
-		}
-		var reply struct{}
-		err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &req, &reply)
-		if err != nil {
-			t.Fatalf("err: %v", err)
-		}
-	}
-
 	// Set up a bare bones query.
 	query := structs.PreparedQueryRequest{
 		Datacenter: "dc1",
 		Op:         structs.PreparedQueryCreate,
 		Query: &structs.PreparedQuery{
-			Name: "my-query",
+			Name: "redis-master",
 			Service: structs.ServiceQuery{
-				Service: "redis",
+				Service: "the-redis",
 			},
 		},
 		WriteRequest: structs.WriteRequest{Token: token},
@@ -784,6 +691,40 @@ func TestPreparedQuery_Get(t *testing.T) {
 		}
 	}
 
+	// Now update the query to take away its name.
+	query.Op = structs.PreparedQueryUpdate
+	query.Query.Name = ""
+	if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Try again with no token, this should work since this query is only
+	// managed by an ID (no name) so no ACLs apply to it.
+	query.Query.ID = reply
+	{
+		req := &structs.PreparedQuerySpecificRequest{
+			Datacenter:   "dc1",
+			QueryID:      query.Query.ID,
+			QueryOptions: structs.QueryOptions{Token: ""},
+		}
+		var resp structs.IndexedPreparedQueries
+		if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.Get", req, &resp); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+
+		if len(resp.Queries) != 1 {
+			t.Fatalf("bad: %v", resp)
+		}
+		actual := resp.Queries[0]
+		if resp.Index != actual.ModifyIndex {
+			t.Fatalf("bad index: %d", resp.Index)
+		}
+		actual.CreateIndex, actual.ModifyIndex = 0, 0
+		if !reflect.DeepEqual(actual, query.Query) {
+			t.Fatalf("bad: %v", actual)
+		}
+	}
+
 	// Try to get an unknown ID.
 	{
 		req := &structs.PreparedQuerySpecificRequest{
@@ -841,26 +782,6 @@ func TestPreparedQuery_List(t *testing.T) {
 		}
 	}
 
-	// Set up a node and service in the catalog.
-	{
-		req := structs.RegisterRequest{
-			Datacenter: "dc1",
-			Node:       "foo",
-			Address:    "127.0.0.1",
-			Service: &structs.NodeService{
-				Service: "redis",
-				Tags:    []string{"master"},
-				Port:    8000,
-			},
-			WriteRequest: structs.WriteRequest{Token: "root"},
-		}
-		var reply struct{}
-		err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &req, &reply)
-		if err != nil {
-			t.Fatalf("err: %v", err)
-		}
-	}
-
 	// Query with a legit token but no queries.
 	{
 		req := &structs.DCSpecificRequest{
@@ -882,9 +803,9 @@ func TestPreparedQuery_List(t *testing.T) {
 		Datacenter: "dc1",
 		Op:         structs.PreparedQueryCreate,
 		Query: &structs.PreparedQuery{
-			Name: "my-query",
+			Name: "redis-master",
 			Service: structs.ServiceQuery{
-				Service: "redis",
+				Service: "the-redis",
 			},
 		},
 		WriteRequest: structs.WriteRequest{Token: token},
@@ -959,6 +880,54 @@ func TestPreparedQuery_List(t *testing.T) {
 			t.Fatalf("bad: %v", actual)
 		}
 	}
+
+	// Now take away the query name.
+	query.Op = structs.PreparedQueryUpdate
+	query.Query.Name = ""
+	if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// A query with the redis token shouldn't show anything since it doesn't
+	// match any un-named queries.
+	{
+		req := &structs.DCSpecificRequest{
+			Datacenter:   "dc1",
+			QueryOptions: structs.QueryOptions{Token: token},
+		}
+		var resp structs.IndexedPreparedQueries
+		if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.List", req, &resp); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+
+		if len(resp.Queries) != 0 {
+			t.Fatalf("bad: %v", resp)
+		}
+	}
+
+	// But a management token should work.
+	{
+		req := &structs.DCSpecificRequest{
+			Datacenter:   "dc1",
+			QueryOptions: structs.QueryOptions{Token: "root"},
+		}
+		var resp structs.IndexedPreparedQueries
+		if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.List", req, &resp); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+
+		if len(resp.Queries) != 1 {
+			t.Fatalf("bad: %v", resp)
+		}
+		actual := resp.Queries[0]
+		if resp.Index != actual.ModifyIndex {
+			t.Fatalf("bad index: %d", resp.Index)
+		}
+		actual.CreateIndex, actual.ModifyIndex = 0, 0
+		if !reflect.DeepEqual(actual, query.Query) {
+			t.Fatalf("bad: %v", actual)
+		}
+	}
 }
 
 // This is a beast of a test, but the setup is so extensive it makes sense to
@@ -1025,30 +994,6 @@ func TestPreparedQuery_Execute(t *testing.T) {
 		}
 	}
 
-	// Create an ACL with write permission to make queries for the service.
-	var queryCRUDToken string
-	{
-		var rules = `
-                    prepared_query "foo" {
-                        policy = "write"
-                    }
-                `
-
-		req := structs.ACLRequest{
-			Datacenter: "dc1",
-			Op:         structs.ACLSet,
-			ACL: structs.ACL{
-				Name:  "User token",
-				Type:  structs.ACLTypeClient,
-				Rules: rules,
-			},
-			WriteRequest: structs.WriteRequest{Token: "root"},
-		}
-		if err := msgpackrpc.CallWithCodec(codec1, "ACL.Apply", &req, &queryCRUDToken); err != nil {
-			t.Fatalf("err: %v", err)
-		}
-	}
-
 	// Set up some nodes in each DC that host the service.
 	{
 		for i := 0; i < 10; i++ {
@@ -1092,7 +1037,7 @@ func TestPreparedQuery_Execute(t *testing.T) {
 				TTL: "10s",
 			},
 		},
-		WriteRequest: structs.WriteRequest{Token: queryCRUDToken},
+		WriteRequest: structs.WriteRequest{Token: "root"},
 	}
 	if err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Apply", &query, &query.Query.ID); err != nil {
 		t.Fatalf("err: %v", err)

consul/structs/prepared_query.go

@@ -72,9 +72,14 @@ type PreparedQuery struct {
 	RaftIndex
 }
 
-// GetACLPrefix returns the prefix of this query for ACL purposes.
+// GetACLPrefix returns the prefix to look up the prepared_query ACL policy for
-func (pq *PreparedQuery) GetACLPrefix() string {
+// this query, or nil if such a policy doesn't apply.
-	return pq.Service.Service
+func (pq *PreparedQuery) GetACLPrefix() *string {
+	if pq.Name != "" {
+		return &pq.Name
+	}
+
+	return nil
 }
 
 type PreparedQueries []*PreparedQuery

consul/structs/prepared_query_test.go

@@ -4,13 +4,14 @@ import (
 	"testing"
 )
 
-func TestStructs_PreparedQuery_GetACLPrefix(t *testing.T) {
+func TestStructs_PreparedQuery_GetACLInfo(t *testing.T) {
-	query := &PreparedQuery{
+	ephemeral := &PreparedQuery{}
-		Service: ServiceQuery{
+	if prefix := ephemeral.GetACLPrefix(); prefix != nil {
-			Service: "foo",
+		t.Fatalf("bad: %#v", prefix)
-		},
 	}
-	if prefix := query.GetACLPrefix(); prefix != "foo" {
+
-		t.Fatalf("bad: %s", prefix)
+	named := &PreparedQuery{Name: "hello"}
+	if prefix := named.GetACLPrefix(); prefix == nil || *prefix != "hello" {
+		t.Fatalf("bad: %#v", prefix)
 	}
 }