Refactor IntentionDecision

This enables it to be called for many upstreams or downstreams of a
service while only querying intentions once.

Additionally, decisions are now optionally denied due to L7 permissions
being present. This enables the function to be used to filter for
potential upstreams/downstreams of a service.

agent/consul/intention_endpoint.go

@@ -8,7 +8,6 @@ import (
 	"github.com/armon/go-metrics"
 	"github.com/armon/go-metrics/prometheus"
 	"github.com/hashicorp/consul/acl"
-	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/consul/state"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/lib"
@@ -684,19 +683,6 @@ func (s *Intention) Check(args *structs.IntentionQueryRequest, reply *structs.In
 		return fmt.Errorf("Invalid destination namespace %q: %v", query.DestinationNS, err)
 	}
 
-	// Build the URI
-	var uri connect.CertURI
-	switch query.SourceType {
-	case structs.IntentionSourceConsul:
-		uri = &connect.SpiffeIDService{
-			Namespace: query.SourceNS,
-			Service:   query.SourceName,
-		}
-
-	default:
-		return fmt.Errorf("unsupported SourceType: %q", query.SourceType)
-	}
-
 	// Perform the ACL check. For Check we only require ServiceRead and
 	// NOT IntentionRead because the Check API only returns pass/fail and
 	// returns no other information about the intentions used. We could check
@@ -732,7 +718,17 @@ func (s *Intention) Check(args *structs.IntentionQueryRequest, reply *structs.In
 	}
 
 	state := s.srv.fsm.State()
-	decision, err := state.IntentionDecision(uri, query.DestinationName, query.DestinationNS, defaultDecision)
+
+	entry := structs.IntentionMatchEntry{
+		Namespace: query.SourceNS,
+		Name:      query.SourceName,
+	}
+	_, intentions, err := state.IntentionMatchOne(nil, entry, structs.IntentionMatchSource)
+	if err != nil {
+		return fmt.Errorf("failed to query intentions for %s/%s", query.SourceNS, query.SourceName)
+	}
+
+	decision, err := state.IntentionDecision(query.DestinationName, query.DestinationNS, intentions, structs.IntentionMatchDestination, defaultDecision, false)
 	if err != nil {
 		return fmt.Errorf("failed to get intention decision from (%s/%s) to (%s/%s): %v",
 			query.SourceNS, query.SourceName, query.DestinationNS, query.DestinationName, err)

agent/consul/state/catalog.go

@@ -11,7 +11,6 @@ import (
 	"github.com/mitchellh/copystructure"
 
 	"github.com/hashicorp/consul/acl"
-	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/lib"
@@ -2859,16 +2858,20 @@ func (s *Store) ServiceTopology(
 
 	upstreamDecisions := make(map[string]structs.IntentionDecisionSummary)
 
-	// The given service is the source relative to upstreams
+	matchEntry := structs.IntentionMatchEntry{
-	sourceURI := connect.SpiffeIDService{
 		Namespace: entMeta.NamespaceOrDefault(),
-		Service:   service,
+		Name:      service,
+	}
+	// The given service is a source relative to its upstreams
+	_, intentions, err := s.IntentionMatchOne(ws, matchEntry, structs.IntentionMatchSource)
+	if err != nil {
+		return 0, nil, fmt.Errorf("failed to query intentions for %s", sn.String())
 	}
 	for _, un := range upstreamNames {
-		decision, err := s.IntentionDecision(&sourceURI, un.Name, un.NamespaceOrDefault(), defaultAllow)
+		decision, err := s.IntentionDecision(un.Name, un.NamespaceOrDefault(), intentions, structs.IntentionMatchDestination, defaultAllow, false)
 		if err != nil {
-			return 0, nil, fmt.Errorf("failed to get intention decision from (%s/%s) to (%s/%s): %v",
+			return 0, nil, fmt.Errorf("failed to get intention decision from (%s) to (%s): %v",
-				sourceURI.Namespace, sourceURI.Service, un.Name, un.NamespaceOrDefault(), err)
+				sn.String(), un.String(), err)
 		}
 		upstreamDecisions[un.String()] = decision
 	}
@@ -2888,17 +2891,17 @@ func (s *Store) ServiceTopology(
 		maxIdx = idx
 	}
 
+	// The given service is a destination relative to its downstreams
+	_, intentions, err = s.IntentionMatchOne(ws, matchEntry, structs.IntentionMatchDestination)
+	if err != nil {
+		return 0, nil, fmt.Errorf("failed to query intentions for %s", sn.String())
+	}
 	downstreamDecisions := make(map[string]structs.IntentionDecisionSummary)
 	for _, dn := range downstreamNames {
-		// Downstreams are the source relative to the given service
+		decision, err := s.IntentionDecision(dn.Name, dn.NamespaceOrDefault(), intentions, structs.IntentionMatchSource, defaultAllow, false)
-		sourceURI := connect.SpiffeIDService{
-			Namespace: dn.NamespaceOrDefault(),
-			Service:   dn.Name,
-		}
-		decision, err := s.IntentionDecision(&sourceURI, service, entMeta.NamespaceOrDefault(), defaultAllow)
 		if err != nil {
-			return 0, nil, fmt.Errorf("failed to get intention decision from (%s/%s) to (%s/%s): %v",
+			return 0, nil, fmt.Errorf("failed to get intention decision from (%s) to (%s): %v",
-				sourceURI.Namespace, sourceURI.Service, service, dn.NamespaceOrDefault(), err)
+				dn.String(), sn.String(), err)
 		}
 		downstreamDecisions[dn.String()] = decision
 	}

agent/consul/state/intention.go

@@ -8,7 +8,6 @@ import (
 	"github.com/hashicorp/go-memdb"
 
 	"github.com/hashicorp/consul/acl"
-	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/structs"
 )
 
@@ -732,35 +731,19 @@ func (s *Store) LegacyIntentionDeleteAll(idx uint64) error {
 	return tx.Commit()
 }
 
-// IntentionDecision returns whether a connection should be allowed from a source URI to some destination
+// IntentionDecision returns whether a connection should be allowed to a source or destination given a set of intentions.
-// It returns true or false for the enforcement, and also a boolean for whether
+//
+// allowPermissions determines whether the presence of L7 permissions leads to a DENY decision.
+// This should be false when evaluating a connection between a source and destination, but not the request that will be sent.
 func (s *Store) IntentionDecision(
-	srcURI connect.CertURI, dstName, dstNS string, defaultDecision acl.EnforcementDecision,
+	target, targetNS string, intentions structs.Intentions, matchType structs.IntentionMatchType,
+	defaultDecision acl.EnforcementDecision, allowPermissions bool,
 ) (structs.IntentionDecisionSummary, error) {
 
-	_, matches, err := s.IntentionMatch(nil, &structs.IntentionQueryMatch{
-		Type: structs.IntentionMatchDestination,
-		Entries: []structs.IntentionMatchEntry{
-			{
-				Namespace: dstNS,
-				Name:      dstName,
-			},
-		},
-	})
-	if err != nil {
-		return structs.IntentionDecisionSummary{}, err
-	}
-	if len(matches) != 1 {
-		// This should never happen since the documented behavior of the
-		// Match call is that it'll always return exactly the number of results
-		// as entries passed in. But we guard against misbehavior.
-		return structs.IntentionDecisionSummary{}, errors.New("internal error loading matches")
-	}
-
 	// Figure out which source matches this request.
 	var ixnMatch *structs.Intention
-	for _, ixn := range matches[0] {
+	for _, ixn := range intentions {
-		if _, ok := srcURI.Authorize(ixn); ok {
+		if _, ok := AuthorizeIntentionTarget(target, targetNS, ixn, matchType); ok {
 			ixnMatch = ixn
 			break
 		}
@@ -778,7 +761,7 @@ func (s *Store) IntentionDecision(
 	if len(ixnMatch.Permissions) > 0 {
 		// If there are L7 permissions, DENY.
 		// We are only evaluating source and destination, not the request that will be sent.
-		resp.Allowed = false
+		resp.Allowed = allowPermissions
 		resp.HasPermissions = true
 	}
 	resp.ExternalSource = ixnMatch.Meta[structs.MetaExternalSource]
@@ -792,6 +775,37 @@ func (s *Store) IntentionDecision(
 	return resp, nil
 }
 
+// AuthorizeIntentionTarget determines whether the destination is covered by the given intention
+// and whether the intention action allows a connection.
+func AuthorizeIntentionTarget(target, targetNS string, ixn *structs.Intention, matchType structs.IntentionMatchType) (bool, bool) {
+	switch matchType {
+	case structs.IntentionMatchDestination:
+		if ixn.DestinationNS != structs.WildcardSpecifier && ixn.DestinationNS != targetNS {
+			// Non-matching namespace
+			return false, false
+		}
+
+		if ixn.DestinationName != structs.WildcardSpecifier && ixn.DestinationName != target {
+			// Non-matching name
+			return false, false
+		}
+
+	case structs.IntentionMatchSource:
+		if ixn.SourceNS != structs.WildcardSpecifier && ixn.SourceNS != targetNS {
+			// Non-matching namespace
+			return false, false
+		}
+
+		if ixn.SourceName != structs.WildcardSpecifier && ixn.SourceName != target {
+			// Non-matching name
+			return false, false
+		}
+	}
+
+	// The name and namespace match, so the destination is covered
+	return ixn.Action == structs.IntentionActionAllow, true
+}
+
 // IntentionMatch returns the list of intentions that match the namespace and
 // name for either a source or destination. This applies the resolution rules
 // so wildcards will match any value.

agent/consul/state/intention_test.go

@@ -9,7 +9,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/hashicorp/consul/acl"
-	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/sdk/testutil"
 )
@@ -1760,16 +1759,19 @@ func TestStore_IntentionDecision(t *testing.T) {
 	}
 
 	tt := []struct {
-		name            string
+		name             string
-		src             string
+		src              string
-		dst             string
+		dst              string
-		defaultDecision acl.EnforcementDecision
+		matchType        structs.IntentionMatchType
-		expect          structs.IntentionDecisionSummary
+		defaultDecision  acl.EnforcementDecision
+		allowPermissions bool
+		expect           structs.IntentionDecisionSummary
 	}{
 		{
 			name:            "no matching intention and default deny",
 			src:             "does-not-exist",
 			dst:             "ditto",
+			matchType:       structs.IntentionMatchDestination,
 			defaultDecision: acl.Deny,
 			expect:          structs.IntentionDecisionSummary{Allowed: false},
 		},
@@ -1777,13 +1779,15 @@ func TestStore_IntentionDecision(t *testing.T) {
 			name:            "no matching intention and default allow",
 			src:             "does-not-exist",
 			dst:             "ditto",
+			matchType:       structs.IntentionMatchDestination,
 			defaultDecision: acl.Allow,
 			expect:          structs.IntentionDecisionSummary{Allowed: true},
 		},
 		{
-			name: "denied with permissions",
+			name:      "denied with permissions",
-			src:  "web",
+			src:       "web",
-			dst:  "redis",
+			dst:       "redis",
+			matchType: structs.IntentionMatchDestination,
 			expect: structs.IntentionDecisionSummary{
 				Allowed:        false,
 				HasPermissions: true,
@@ -1791,9 +1795,22 @@ func TestStore_IntentionDecision(t *testing.T) {
 			},
 		},
 		{
-			name: "denied without permissions",
+			name:             "allowed with permissions",
-			src:  "api",
+			src:              "web",
-			dst:  "redis",
+			dst:              "redis",
+			allowPermissions: true,
+			matchType:        structs.IntentionMatchDestination,
+			expect: structs.IntentionDecisionSummary{
+				Allowed:        true,
+				HasPermissions: true,
+				HasExact:       true,
+			},
+		},
+		{
+			name:      "denied without permissions",
+			src:       "api",
+			dst:       "redis",
+			matchType: structs.IntentionMatchDestination,
 			expect: structs.IntentionDecisionSummary{
 				Allowed:        false,
 				HasPermissions: false,
@@ -1801,9 +1818,10 @@ func TestStore_IntentionDecision(t *testing.T) {
 			},
 		},
 		{
-			name: "allowed from external source",
+			name:      "allowed from external source",
-			src:  "api",
+			src:       "api",
-			dst:  "web",
+			dst:       "web",
+			matchType: structs.IntentionMatchDestination,
 			expect: structs.IntentionDecisionSummary{
 				Allowed:        true,
 				HasPermissions: false,
@@ -1812,9 +1830,21 @@ func TestStore_IntentionDecision(t *testing.T) {
 			},
 		},
 		{
-			name: "allowed by source wildcard not exact",
+			name:      "allowed by source wildcard not exact",
-			src:  "anything",
+			src:       "anything",
-			dst:  "mysql",
+			dst:       "mysql",
+			matchType: structs.IntentionMatchDestination,
+			expect: structs.IntentionDecisionSummary{
+				Allowed:        true,
+				HasPermissions: false,
+				HasExact:       false,
+			},
+		},
+		{
+			name:      "allowed by matching on source",
+			src:       "web",
+			dst:       "api",
+			matchType: structs.IntentionMatchSource,
 			expect: structs.IntentionDecisionSummary{
 				Allowed:        true,
 				HasPermissions: false,
@@ -1824,17 +1854,197 @@ func TestStore_IntentionDecision(t *testing.T) {
 	}
 	for _, tc := range tt {
 		t.Run(tc.name, func(t *testing.T) {
-			uri := connect.SpiffeIDService{
+			entry := structs.IntentionMatchEntry{
-				Service:   tc.src,
 				Namespace: structs.IntentionDefaultNamespace,
+				Name:      tc.src,
 			}
-			decision, err := s.IntentionDecision(&uri, tc.dst, structs.IntentionDefaultNamespace, tc.defaultDecision)
+			_, intentions, err := s.IntentionMatchOne(nil, entry, structs.IntentionMatchSource)
+			if err != nil {
+				require.NoError(t, err)
+			}
+			decision, err := s.IntentionDecision(tc.dst, structs.IntentionDefaultNamespace, intentions, tc.matchType, tc.defaultDecision, tc.allowPermissions)
 			require.NoError(t, err)
 			require.Equal(t, tc.expect, decision)
 		})
 	}
 }
 
+func TestAuthorizeIntentionTarget(t *testing.T) {
+	cases := []struct {
+		name      string
+		target    string
+		targetNS  string
+		ixn       *structs.Intention
+		matchType structs.IntentionMatchType
+		auth      bool
+		match     bool
+	}{
+		// Source match type
+		{
+			name:     "match exact source, not matching namespace",
+			target:   "web",
+			targetNS: structs.IntentionDefaultNamespace,
+			ixn: &structs.Intention{
+				SourceName: "db",
+				SourceNS:   "different",
+			},
+			matchType: structs.IntentionMatchSource,
+			auth:      false,
+			match:     false,
+		},
+		{
+			name:     "match exact source, not matching name",
+			target:   "web",
+			targetNS: structs.IntentionDefaultNamespace,
+			ixn: &structs.Intention{
+				SourceName: "db",
+				SourceNS:   structs.IntentionDefaultNamespace,
+			},
+			matchType: structs.IntentionMatchSource,
+			auth:      false,
+			match:     false,
+		},
+		{
+			name:     "match exact source, allow",
+			target:   "web",
+			targetNS: structs.IntentionDefaultNamespace,
+			ixn: &structs.Intention{
+				SourceName: "web",
+				SourceNS:   structs.IntentionDefaultNamespace,
+				Action:     structs.IntentionActionAllow,
+			},
+			matchType: structs.IntentionMatchSource,
+			auth:      true,
+			match:     true,
+		},
+		{
+			name:     "match exact source, deny",
+			target:   "web",
+			targetNS: structs.IntentionDefaultNamespace,
+			ixn: &structs.Intention{
+				SourceName: "web",
+				SourceNS:   structs.IntentionDefaultNamespace,
+				Action:     structs.IntentionActionDeny,
+			},
+			matchType: structs.IntentionMatchSource,
+			auth:      false,
+			match:     true,
+		},
+		{
+			name:     "match exact sourceNS for wildcard service, deny",
+			target:   "web",
+			targetNS: structs.IntentionDefaultNamespace,
+			ixn: &structs.Intention{
+				SourceName: structs.WildcardSpecifier,
+				SourceNS:   structs.IntentionDefaultNamespace,
+				Action:     structs.IntentionActionDeny,
+			},
+			matchType: structs.IntentionMatchSource,
+			auth:      false,
+			match:     true,
+		},
+		{
+			name:     "match exact sourceNS for wildcard service, allow",
+			target:   "web",
+			targetNS: structs.IntentionDefaultNamespace,
+			ixn: &structs.Intention{
+				SourceName: structs.WildcardSpecifier,
+				SourceNS:   structs.IntentionDefaultNamespace,
+				Action:     structs.IntentionActionAllow,
+			},
+			matchType: structs.IntentionMatchSource,
+			auth:      true,
+			match:     true,
+		},
+
+		// Destination match type
+		{
+			name:     "match exact destination, not matching namespace",
+			target:   "web",
+			targetNS: structs.IntentionDefaultNamespace,
+			ixn: &structs.Intention{
+				DestinationName: "db",
+				DestinationNS:   "different",
+			},
+			matchType: structs.IntentionMatchDestination,
+			auth:      false,
+			match:     false,
+		},
+		{
+			name:     "match exact destination, not matching name",
+			target:   "web",
+			targetNS: structs.IntentionDefaultNamespace,
+			ixn: &structs.Intention{
+				DestinationName: "db",
+				DestinationNS:   structs.IntentionDefaultNamespace,
+			},
+			matchType: structs.IntentionMatchDestination,
+			auth:      false,
+			match:     false,
+		},
+		{
+			name:     "match exact destination, allow",
+			target:   "web",
+			targetNS: structs.IntentionDefaultNamespace,
+			ixn: &structs.Intention{
+				DestinationName: "web",
+				DestinationNS:   structs.IntentionDefaultNamespace,
+				Action:          structs.IntentionActionAllow,
+			},
+			matchType: structs.IntentionMatchDestination,
+			auth:      true,
+			match:     true,
+		},
+		{
+			name:     "match exact destination, deny",
+			target:   "web",
+			targetNS: structs.IntentionDefaultNamespace,
+			ixn: &structs.Intention{
+				DestinationName: "web",
+				DestinationNS:   structs.IntentionDefaultNamespace,
+				Action:          structs.IntentionActionDeny,
+			},
+			matchType: structs.IntentionMatchDestination,
+			auth:      false,
+			match:     true,
+		},
+		{
+			name:     "match exact destinationNS for wildcard service, deny",
+			target:   "web",
+			targetNS: structs.IntentionDefaultNamespace,
+			ixn: &structs.Intention{
+				DestinationName: structs.WildcardSpecifier,
+				DestinationNS:   structs.IntentionDefaultNamespace,
+				Action:          structs.IntentionActionDeny,
+			},
+			matchType: structs.IntentionMatchDestination,
+			auth:      false,
+			match:     true,
+		},
+		{
+			name:     "match exact destinationNS for wildcard service, allow",
+			target:   "web",
+			targetNS: structs.IntentionDefaultNamespace,
+			ixn: &structs.Intention{
+				DestinationName: structs.WildcardSpecifier,
+				DestinationNS:   structs.IntentionDefaultNamespace,
+				Action:          structs.IntentionActionAllow,
+			},
+			matchType: structs.IntentionMatchDestination,
+			auth:      true,
+			match:     true,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			auth, match := AuthorizeIntentionTarget(tc.target, tc.targetNS, tc.ixn, tc.matchType)
+			assert.Equal(t, tc.auth, auth)
+			assert.Equal(t, tc.match, match)
+		})
+	}
+}
+
 func disableLegacyIntentions(s *Store) error {
 	return s.SystemMetadataSet(1, &structs.SystemMetadataEntry{
 		Key:   structs.SystemMetadataIntentionFormatKey,