Backport of Fixup authz for data imported from peers into release/1.1…

…4.x (#15355)

This pull request was automerged via backport-assistant

acl/authorizer_oss.go

@@ -3,8 +3,19 @@
 
 package acl
 
-// AuthorizerContext stub
-type AuthorizerContext struct{}
+// AuthorizerContext contains extra information that can be
+// used in the determination of an ACL enforcement decision.
+type AuthorizerContext struct {
+	// Peer is the name of the peer that the resource was imported from.
+	Peer string
+}
+
+func (c *AuthorizerContext) PeerOrEmpty() string {
+	if c == nil {
+		return ""
+	}
+	return c.Peer
+}
 
 // enterpriseAuthorizer stub interface
 type enterpriseAuthorizer interface{}

acl/policy_authorizer.go

@@ -741,7 +741,18 @@ func (p *policyAuthorizer) OperatorWrite(*AuthorizerContext) EnforcementDecision
 }
 
 // NodeRead checks if reading (discovery) of a node is allowed
-func (p *policyAuthorizer) NodeRead(name string, _ *AuthorizerContext) EnforcementDecision {
+func (p *policyAuthorizer) NodeRead(name string, ctx *AuthorizerContext) EnforcementDecision {
+	// When reading a node imported from a peer we consider it to be allowed when:
+	//  - The request comes from a locally authenticated service, meaning that it
+	//    has service:write permissions on some name.
+	//  - The requester has permissions to read all nodes in its local cluster,
+	//    therefore it can also read imported nodes.
+	if ctx.PeerOrEmpty() != "" {
+		if p.ServiceWriteAny(nil) == Allow {
+			return Allow
+		}
+		return p.NodeReadAll(nil)
+	}
 	if rule, ok := getPolicy(name, p.nodeRules); ok {
 		return enforce(rule.access, AccessRead)
 	}
@@ -779,7 +790,18 @@ func (p *policyAuthorizer) PreparedQueryWrite(prefix string, _ *AuthorizerContex
 }
 
 // ServiceRead checks if reading (discovery) of a service is allowed
-func (p *policyAuthorizer) ServiceRead(name string, _ *AuthorizerContext) EnforcementDecision {
+func (p *policyAuthorizer) ServiceRead(name string, ctx *AuthorizerContext) EnforcementDecision {
+	// When reading a service imported from a peer we consider it to be allowed when:
+	//  - The request comes from a locally authenticated service, meaning that it
+	//    has service:write permissions on some name.
+	//  - The requester has permissions to read all services in its local cluster,
+	//    therefore it can also read imported services.
+	if ctx.PeerOrEmpty() != "" {
+		if p.ServiceWriteAny(nil) == Allow {
+			return Allow
+		}
+		return p.ServiceReadAll(nil)
+	}
 	if rule, ok := getPolicy(name, p.serviceRules); ok {
 		return enforce(rule.access, AccessRead)
 	}

acl/policy_authorizer_test.go

@@ -20,8 +20,9 @@ func TestPolicyAuthorizer(t *testing.T) {
 	}
 
 	type aclTest struct {
-		policy *Policy
-		checks []aclCheck
+		policy       *Policy
+		authzContext *AuthorizerContext
+		checks       []aclCheck
 	}
 
 	cases := map[string]aclTest{
@@ -64,6 +65,101 @@ func TestPolicyAuthorizer(t *testing.T) {
 				{name: "DefaultSnapshot", prefix: "foo", check: checkDefaultSnapshot},
 			},
 		},
+		"Defaults - from peer": {
+			policy:       &Policy{},
+			authzContext: &AuthorizerContext{Peer: "some-peer"},
+			checks: []aclCheck{
+				{name: "DefaultNodeRead", prefix: "foo", check: checkDefaultNodeRead},
+				{name: "DefaultServiceRead", prefix: "foo", check: checkDefaultServiceRead},
+			},
+		},
+		"Peering - ServiceRead allowed with service:write": {
+			policy: &Policy{PolicyRules: PolicyRules{
+				Services: []*ServiceRule{
+					{
+						Name:       "foo",
+						Policy:     PolicyWrite,
+						Intentions: PolicyWrite,
+					},
+				},
+			}},
+			authzContext: &AuthorizerContext{Peer: "some-peer"},
+			checks: []aclCheck{
+				{name: "ServiceWriteAny", prefix: "imported-svc", check: checkAllowServiceRead},
+			},
+		},
+		"Peering - ServiceRead allowed with service:read on all": {
+			policy: &Policy{PolicyRules: PolicyRules{
+				ServicePrefixes: []*ServiceRule{
+					{
+						Name:       "",
+						Policy:     PolicyRead,
+						Intentions: PolicyRead,
+					},
+				},
+			}},
+			authzContext: &AuthorizerContext{Peer: "some-peer"},
+			checks: []aclCheck{
+				{name: "ServiceReadAll", prefix: "imported-svc", check: checkAllowServiceRead},
+			},
+		},
+		"Peering - ServiceRead not allowed with service:read on single service": {
+			policy: &Policy{PolicyRules: PolicyRules{
+				Services: []*ServiceRule{
+					{
+						Name:       "same-name-as-imported",
+						Policy:     PolicyRead,
+						Intentions: PolicyRead,
+					},
+				},
+			}},
+			authzContext: &AuthorizerContext{Peer: "some-peer"},
+			checks: []aclCheck{
+				{name: "ServiceReadAll", prefix: "same-name-as-imported", check: checkDefaultServiceRead},
+			},
+		},
+		"Peering - NodeRead allowed with service:write": {
+			policy: &Policy{PolicyRules: PolicyRules{
+				Services: []*ServiceRule{
+					{
+						Name:   "foo",
+						Policy: PolicyWrite,
+					},
+				},
+			}},
+			authzContext: &AuthorizerContext{Peer: "some-peer"},
+			checks: []aclCheck{
+				{name: "ServiceWriteAny", prefix: "imported-svc", check: checkAllowNodeRead},
+			},
+		},
+		"Peering - NodeRead allowed with node:read on all": {
+			policy: &Policy{PolicyRules: PolicyRules{
+				NodePrefixes: []*NodeRule{
+					{
+						Name:   "",
+						Policy: PolicyRead,
+					},
+				},
+			}},
+			authzContext: &AuthorizerContext{Peer: "some-peer"},
+			checks: []aclCheck{
+				{name: "NodeReadAll", prefix: "imported-svc", check: checkAllowNodeRead},
+			},
+		},
+		"Peering - NodeRead not allowed with node:read on single service": {
+			policy: &Policy{PolicyRules: PolicyRules{
+				Nodes: []*NodeRule{
+					{
+						Name:   "same-name-as-imported",
+						Policy: PolicyRead,
+					},
+				},
+			}},
+			authzContext: &AuthorizerContext{Peer: "some-peer"},
+			checks: []aclCheck{
+				{name: "NodeReadAll", prefix: "same-name-as-imported", check: checkDefaultNodeRead},
+			},
+		},
 		"Prefer Exact Matches": {
 			policy: &Policy{PolicyRules: PolicyRules{
 				Agents: []*AgentRule{
@@ -461,7 +557,7 @@ func TestPolicyAuthorizer(t *testing.T) {
 				t.Run(checkName, func(t *testing.T) {
 					check := check
 
-					check.check(t, authz, check.prefix, nil)
+					check.check(t, authz, check.prefix, tcase.authzContext)
 				})
 			}
 		})

agent/consul/acl.go

@@ -1068,20 +1068,10 @@ func (r *ACLResolver) ACLsEnabled() bool {
 	return true
 }
 
-// TODO(peering): fix all calls to use the new signature and rename it back
 func (r *ACLResolver) ResolveTokenAndDefaultMeta(
 	token string,
 	entMeta *acl.EnterpriseMeta,
 	authzContext *acl.AuthorizerContext,
-) (resolver.Result, error) {
-	return r.ResolveTokenAndDefaultMetaWithPeerName(token, entMeta, structs.DefaultPeerKeyword, authzContext)
-}
-
-func (r *ACLResolver) ResolveTokenAndDefaultMetaWithPeerName(
-	token string,
-	entMeta *acl.EnterpriseMeta,
-	peerName string,
-	authzContext *acl.AuthorizerContext,
 ) (resolver.Result, error) {
 	result, err := r.ResolveToken(token)
 	if err != nil {
@@ -1095,8 +1085,9 @@ func (r *ACLResolver) ResolveTokenAndDefaultMetaWithPeerName(
 	// Default the EnterpriseMeta based on the Tokens meta or actual defaults
 	// in the case of unknown identity
 	switch {
-	case peerName == "" && result.ACLIdentity != nil:
+	case authzContext.PeerOrEmpty() == "" && result.ACLIdentity != nil:
 		entMeta.Merge(result.ACLIdentity.EnterpriseMetadata())
+
 	case result.ACLIdentity != nil:
 		// We _do not_ normalize the enterprise meta from the token when a peer
 		// name was specified because namespaces across clusters are not

agent/consul/catalog_endpoint.go

@@ -1,6 +1,7 @@
 package consul
 
 import (
+	"errors"
 	"fmt"
 	"sort"
 	"strings"
@@ -557,6 +558,14 @@ func (c *Catalog) ListServices(args *structs.DCSpecificRequest, reply *structs.I
 		return err
 	}
 
+	// Supporting querying by PeerName in this API would require modifying the return type or the ACL
+	// filtering logic so that it can be made aware that the data queried is coming from a peer.
+	// Currently the ACL filter will receive plain name strings with no awareness of the peer name,
+	// which means that authz will be done as if these were local service names.
+	if args.PeerName != structs.DefaultPeerKeyword {
+		return errors.New("listing service names imported from a peer is not supported")
+	}
+
 	authz, err := c.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, nil)
 	if err != nil {
 		return err
@@ -705,7 +714,9 @@ func (c *Catalog) ServiceNodes(args *structs.ServiceSpecificRequest, reply *stru
 		}
 	}
 
-	var authzContext acl.AuthorizerContext
+	authzContext := acl.AuthorizerContext{
+		Peer: args.PeerName,
+	}
 	authz, err := c.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext)
 	if err != nil {
 		return err
@@ -1087,7 +1098,9 @@ func (c *Catalog) VirtualIPForService(args *structs.ServiceSpecificRequest, repl
 		return err
 	}
 
-	var authzContext acl.AuthorizerContext
+	authzContext := acl.AuthorizerContext{
+		Peer: args.PeerName,
+	}
 	authz, err := c.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext)
 	if err != nil {
 		return err

agent/consul/filter.go

@@ -46,14 +46,17 @@ func (t *txnResultsFilter) Filter(i int) bool {
 	case result.KV != nil:
 		result.KV.EnterpriseMeta.FillAuthzContext(&authzContext)
 		return t.authorizer.KeyRead(result.KV.Key, &authzContext) != acl.Allow
+
 	case result.Node != nil:
 		(*structs.Node)(result.Node).FillAuthzContext(&authzContext)
 		return t.authorizer.NodeRead(result.Node.Node, &authzContext) != acl.Allow
+
 	case result.Service != nil:
-		result.Service.EnterpriseMeta.FillAuthzContext(&authzContext)
+		(*structs.NodeService)(result.Service).FillAuthzContext(&authzContext)
 		return t.authorizer.ServiceRead(result.Service.Service, &authzContext) != acl.Allow
+
 	case result.Check != nil:
-		result.Check.EnterpriseMeta.FillAuthzContext(&authzContext)
+		(*structs.HealthCheck)(result.Check).FillAuthzContext(&authzContext)
 		if result.Check.ServiceName != "" {
 			return t.authorizer.ServiceRead(result.Check.ServiceName, &authzContext) != acl.Allow
 		}

agent/consul/health_endpoint.go

@@ -211,7 +211,9 @@ func (h *Health) ServiceNodes(args *structs.ServiceSpecificRequest, reply *struc
 		f = h.serviceNodesDefault
 	}
 
-	var authzContext acl.AuthorizerContext
+	authzContext := acl.AuthorizerContext{
+		Peer: args.PeerName,
+	}
 	authz, err := h.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext)
 	if err != nil {
 		return err

agent/consul/internal_endpoint.go

@@ -619,8 +619,7 @@ func (m *Internal) ExportedPeeredServices(args *structs.DCSpecificRequest, reply
 		return err
 	}
 
-	var authzCtx acl.AuthorizerContext
-	authz, err := m.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzCtx)
+	authz, err := m.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, nil)
 	if err != nil {
 		return err
 	}

agent/consul/state/catalog_events.go

@@ -44,7 +44,6 @@ type EventPayloadCheckServiceNode struct {
 }
 
 func (e EventPayloadCheckServiceNode) HasReadPermission(authz acl.Authorizer) bool {
-	// TODO(peering): figure out how authz works for peered data
 	return e.Value.CanRead(authz) == acl.Allow
 }
 

agent/proxycfg-glue/exported_peered_services.go

@@ -6,7 +6,6 @@ import (
 	"github.com/hashicorp/consul/agent/structs/aclfilter"
 	"github.com/hashicorp/go-memdb"
 
-	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/cache"
 	cachetype "github.com/hashicorp/consul/agent/cache-types"
 	"github.com/hashicorp/consul/agent/consul/watch"
@@ -34,8 +33,7 @@ type serverExportedPeeredServices struct {
 func (s *serverExportedPeeredServices) Notify(ctx context.Context, req *structs.DCSpecificRequest, correlationID string, ch chan<- proxycfg.UpdateEvent) error {
 	return watch.ServerLocalNotify(ctx, correlationID, s.deps.GetStore,
 		func(ws memdb.WatchSet, store Store) (uint64, *structs.IndexedExportedServiceList, error) {
-			var authzCtx acl.AuthorizerContext
-			authz, err := s.deps.ACLResolver.ResolveTokenAndDefaultMeta(req.Token, &req.EnterpriseMeta, &authzCtx)
+			authz, err := s.deps.ACLResolver.ResolveTokenAndDefaultMeta(req.Token, &req.EnterpriseMeta, nil)
 			if err != nil {
 				return 0, nil, err
 			}

agent/proxycfg-glue/internal_service_dump.go

@@ -73,7 +73,7 @@ func (s *serverInternalServiceDump) Notify(ctx context.Context, req *structs.Ser
 				return 0, nil, err
 			}
 
-			idx, nodes, err := store.ServiceDump(ws, req.ServiceKind, req.UseServiceKind, &req.EnterpriseMeta, structs.DefaultPeerKeyword)
+			idx, nodes, err := store.ServiceDump(ws, req.ServiceKind, req.UseServiceKind, &req.EnterpriseMeta, req.PeerName)
 			if err != nil {
 				return 0, nil, err
 			}