From d21b52dee25a18c30b6b731e851e4ceb19306313 Mon Sep 17 00:00:00 2001
From: Luke Kysow <1034429+lkysow@users.noreply.github.com>
Date: Mon, 12 Aug 2019 14:06:46 +0200
Subject: [PATCH] New intention precedence for external intentions.

Support external intentions by making source type part of the precedence
decision. The goal is that an external-uri intention, e.g.
spiffe://trust.domain/path, is ranked higher than an
external-trust-domain intention, e.g. spiffe://trust.domain, because it
is a more exact match.

The maximum precedence value is increased, however the relative ordering
of Consul intentions has not changed.
---
 agent/consul/state/txn_test.go                |  9 +-
 agent/structs/intention.go                    | 39 ++++++--
 agent/structs/intention_test.go               | 96 ++++++++++++-------
 api/connect_intention_test.go                 |  2 +-
 .../source/docs/connect/intentions.html.md    | 16 ++--
 5 files changed, 107 insertions(+), 55 deletions(-)

diff --git a/agent/consul/state/txn_test.go b/agent/consul/state/txn_test.go
index c21e3a0ea9..5d5ee6a48d 100644
--- a/agent/consul/state/txn_test.go
+++ b/agent/consul/state/txn_test.go
@@ -20,6 +20,7 @@ func TestStateStore_Txn_Intention(t *testing.T) {
 	// Create some intentions.
 	ixn1 := &structs.Intention{
 		ID:              testUUID(),
+		SourceType:      structs.IntentionSourceConsul,
 		SourceNS:        "default",
 		SourceName:      "web",
 		DestinationNS:   "default",
@@ -28,6 +29,7 @@ func TestStateStore_Txn_Intention(t *testing.T) {
 	}
 	ixn2 := &structs.Intention{
 		ID:              testUUID(),
+		SourceType:      structs.IntentionSourceConsul,
 		SourceNS:        "default",
 		SourceName:      "db",
 		DestinationNS:   "default",
@@ -37,6 +39,7 @@ func TestStateStore_Txn_Intention(t *testing.T) {
 	}
 	ixn3 := &structs.Intention{
 		ID:              testUUID(),
+		SourceType:      structs.IntentionSourceConsul,
 		SourceNS:        "default",
 		SourceName:      "foo",
 		DestinationNS:   "default",
@@ -90,12 +93,13 @@ func TestStateStore_Txn_Intention(t *testing.T) {
 	intentions := structs.Intentions{
 		&structs.Intention{
 			ID:              ixn1.ID,
+			SourceType:      structs.IntentionSourceConsul,
 			SourceNS:        "default",
 			SourceName:      "web",
 			DestinationNS:   "default",
 			DestinationName: "db",
 			Meta:            map[string]string{},
-			Precedence:      9,
+			Precedence:      15,
 			RaftIndex: structs.RaftIndex{
 				CreateIndex: 1,
 				ModifyIndex: 3,
@@ -103,12 +107,13 @@ func TestStateStore_Txn_Intention(t *testing.T) {
 		},
 		&structs.Intention{
 			ID:              ixn3.ID,
+			SourceType:      structs.IntentionSourceConsul,
 			SourceNS:        "default",
 			SourceName:      "foo",
 			DestinationNS:   "default",
 			DestinationName: "*",
 			Meta:            map[string]string{},
-			Precedence:      6,
+			Precedence:      10,
 			RaftIndex: structs.RaftIndex{
 				CreateIndex: 3,
 				ModifyIndex: 3,
diff --git a/agent/structs/intention.go b/agent/structs/intention.go
index 914b6985e7..0a3f30bee7 100644
--- a/agent/structs/intention.go
+++ b/agent/structs/intention.go
@@ -179,11 +179,11 @@ func (x *Intention) UpdatePrecedence() {
 	var max int
 	switch x.countExact(x.DestinationNS, x.DestinationName) {
 	case 2:
-		max = 9
+		max = 15
 	case 1:
-		max = 6
+		max = 10
 	case 0:
-		max = 3
+		max = 5
 	default:
 		// This shouldn't be possible, just set it to zero
 		x.Precedence = 0
@@ -191,9 +191,9 @@ func (x *Intention) UpdatePrecedence() {
 	}
 
 	// Given the maximum, the exact value is determined based on the
-	// number of source exact values.
-	countSrc := x.countExact(x.SourceNS, x.SourceName)
-	x.Precedence = max - (2 - countSrc)
+	// number of source exact values in combination with the src type.
+	countSrc := x.srcPrecedence(x.SourceType, x.SourceNS, x.SourceName)
+	x.Precedence = max - (4 - countSrc)
 }
 
 // countExact counts the number of exact values (not wildcards) in
@@ -214,6 +214,33 @@ func (x *Intention) countExact(ns, n string) int {
 	return 2
 }
 
+// srcPrecedence returns a precedence value based on the srcType, namespace
+// and name.
+func (x *Intention) srcPrecedence(srcType IntentionSourceType, ns string, n string) int {
+	// ExternalTrustDomain and ExternalURI don't support wildcards so they are
+	// given higher precedence than anything using a wildcard.
+	// Non-wildcard Consul sources are given highest precedence as an arbitrary
+	// decision. It's arbitrary because an intention with an external source
+	// won't also match a Consul-source intention.
+	// ExternalURI is given higher precedence than ExternalTrustDomain because
+	// it's a more exact match.
+	switch {
+	case srcType == IntentionSourceConsul && ns == IntentionWildcard:
+		return 0
+	case srcType == IntentionSourceConsul && n == IntentionWildcard:
+		return 1
+	case srcType == IntentionSourceExternalTrustDomain:
+		return 2
+	case srcType == IntentionSourceExternalURI:
+		return 3
+	case srcType == IntentionSourceConsul:
+		return 4
+	default:
+		// Shouldn't be possible.
+		return 0
+	}
+}
+
 // GetACLPrefix returns the prefix to look up the ACL policy for this
 // intention, and a boolean noting whether the prefix is valid to check
 // or not. You must check the ok value before using the prefix.
diff --git a/agent/structs/intention_test.go b/agent/structs/intention_test.go
index 994897ce95..2cadf2497e 100644
--- a/agent/structs/intention_test.go
+++ b/agent/structs/intention_test.go
@@ -176,56 +176,76 @@ func TestIntentionValidate(t *testing.T) {
 func TestIntentionPrecedenceSorter(t *testing.T) {
 	cases := []struct {
 		Name     string
-		Input    [][]string // SrcNS, SrcN, DstNS, DstN
+		Input    [][]string // SrcType, SrcNS, SrcN, DstNS, DstN
 		Expected [][]string // Same structure as Input
 	}{
 		{
 			"exhaustive list",
 			[][]string{
-				{"*", "*", "exact", "*"},
-				{"*", "*", "*", "*"},
-				{"exact", "*", "exact", "exact"},
-				{"*", "*", "exact", "exact"},
-				{"exact", "exact", "*", "*"},
-				{"exact", "exact", "exact", "exact"},
-				{"exact", "exact", "exact", "*"},
-				{"exact", "*", "exact", "*"},
-				{"exact", "*", "*", "*"},
+				{"consul", "*", "*", "exact", "*"},
+				{"consul", "*", "*", "*", "*"},
+				{"consul", "exact", "*", "exact", "exact"},
+				{"consul", "*", "*", "exact", "exact"},
+				{"consul", "exact", "exact", "*", "*"},
+				{"consul", "exact", "exact", "exact", "exact"},
+				{"consul", "exact", "exact", "exact", "*"},
+				{"consul", "exact", "*", "exact", "*"},
+				{"consul", "exact", "*", "*", "*"},
+				{"external-trust-domain", "exact", "spiffe://trust.domain", "*", "*"},
+				{"external-trust-domain", "exact", "spiffe://trust.domain", "exact", "exact"},
+				{"external-trust-domain", "exact", "spiffe://trust.domain", "exact", "*"},
+				{"external-uri", "exact", "spiffe://trust.domain/path", "*", "*"},
+				{"external-uri", "exact", "spiffe://trust.domain/path", "exact", "exact"},
+				{"external-uri", "exact", "spiffe://trust.domain/path", "exact", "*"},
 			},
 			[][]string{
-				{"exact", "exact", "exact", "exact"},
-				{"exact", "*", "exact", "exact"},
-				{"*", "*", "exact", "exact"},
-				{"exact", "exact", "exact", "*"},
-				{"exact", "*", "exact", "*"},
-				{"*", "*", "exact", "*"},
-				{"exact", "exact", "*", "*"},
-				{"exact", "*", "*", "*"},
-				{"*", "*", "*", "*"},
+				{"consul", "exact", "exact", "exact", "exact"},
+				{"external-uri", "exact", "spiffe://trust.domain/path", "exact", "exact"},
+				{"external-trust-domain", "exact", "spiffe://trust.domain", "exact", "exact"},
+				{"consul", "exact", "*", "exact", "exact"},
+				{"consul", "*", "*", "exact", "exact"},
+				{"consul", "exact", "exact", "exact", "*"},
+				{"external-uri", "exact", "spiffe://trust.domain/path", "exact", "*"},
+				{"external-trust-domain", "exact", "spiffe://trust.domain", "exact", "*"},
+				{"consul", "exact", "*", "exact", "*"},
+				{"consul", "*", "*", "exact", "*"},
+				{"consul", "exact", "exact", "*", "*"},
+				{"external-uri", "exact", "spiffe://trust.domain/path", "*", "*"},
+				{"external-trust-domain", "exact", "spiffe://trust.domain", "*", "*"},
+				{"consul", "exact", "*", "*", "*"},
+				{"consul", "*", "*", "*", "*"},
 			},
 		},
 		{
 			"tiebreak deterministically",
 			[][]string{
-				{"a", "*", "a", "b"},
-				{"a", "*", "a", "a"},
-				{"b", "a", "a", "a"},
-				{"a", "b", "a", "a"},
-				{"a", "a", "b", "a"},
-				{"a", "a", "a", "b"},
-				{"a", "a", "a", "a"},
+				{"consul", "a", "*", "a", "b"},
+				{"consul", "a", "*", "a", "a"},
+				{"consul", "b", "a", "a", "a"},
+				{"consul", "a", "b", "a", "a"},
+				{"consul", "a", "a", "b", "a"},
+				{"consul", "a", "a", "a", "b"},
+				{"consul", "a", "a", "a", "a"},
+				{"external-trust-domain", "a", "spiffe://a", "a", "a"},
+				{"external-trust-domain", "a", "spiffe://b", "a", "a"},
+				{"external-uri", "a", "spiffe://a/a", "a", "a"},
+				{"external-uri", "a", "spiffe://a/b", "a", "a"},
 			},
 			[][]string{
 				// Exact matches first in lexicographical order (arbitrary but
 				// deterministic)
-				{"a", "a", "a", "a"},
-				{"a", "a", "a", "b"},
-				{"a", "a", "b", "a"},
-				{"a", "b", "a", "a"},
-				{"b", "a", "a", "a"},
+				{"consul", "a", "a", "a", "a"},
+				{"consul", "a", "a", "a", "b"},
+				{"consul", "a", "a", "b", "a"},
+				{"consul", "a", "b", "a", "a"},
+				{"consul", "b", "a", "a", "a"},
+				{"external-uri", "a", "spiffe://a/a", "a", "a"},
+				{"external-uri", "a", "spiffe://a/b", "a", "a"},
+				{"external-trust-domain", "a", "spiffe://a", "a", "a"},
+				{"external-trust-domain", "a", "spiffe://b", "a", "a"},
 				// Wildcards next, lexicographical
-				{"a", "*", "a", "a"},
-				{"a", "*", "a", "b"},
+				{"consul", "a", "*", "a", "a"},
+				{"consul", "a", "*", "a", "b"},
 			},
 		},
 	}
@@ -237,10 +257,11 @@ func TestIntentionPrecedenceSorter(t *testing.T) {
 			var input Intentions
 			for _, v := range tc.Input {
 				input = append(input, &Intention{
-					SourceNS:        v[0],
-					SourceName:      v[1],
-					DestinationNS:   v[2],
-					DestinationName: v[3],
+					SourceType:      IntentionSourceType(v[0]),
+					SourceNS:        v[1],
+					SourceName:      v[2],
+					DestinationNS:   v[3],
+					DestinationName: v[4],
 				})
 			}
 
@@ -256,6 +277,7 @@ func TestIntentionPrecedenceSorter(t *testing.T) {
 			var actual [][]string
 			for _, v := range input {
 				actual = append(actual, []string{
+					string(v.SourceType),
 					v.SourceNS,
 					v.SourceName,
 					v.DestinationNS,
diff --git a/api/connect_intention_test.go b/api/connect_intention_test.go
index 0ace2afdaa..513e2f12fb 100644
--- a/api/connect_intention_test.go
+++ b/api/connect_intention_test.go
@@ -179,7 +179,7 @@ func testIntention() *Intention {
 		SourceName:      "api",
 		DestinationNS:   "eng",
 		DestinationName: "db",
-		Precedence:      9,
+		Precedence:      15,
 		Action:          IntentionActionAllow,
 		SourceType:      IntentionSourceConsul,
 		Meta:            map[string]string{},
diff --git a/website/source/docs/connect/intentions.html.md b/website/source/docs/connect/intentions.html.md
index 46a3f5df96..95ed8a5d5a 100644
--- a/website/source/docs/connect/intentions.html.md
+++ b/website/source/docs/connect/intentions.html.md
@@ -93,10 +93,10 @@ top to bottom, with larger numbers being evaluated first.
 
 | Source Name | Destination Name | Precedence |
 | ----------- | ---------------- | ---------- |
-| Exact       | Exact            | 9          |
-| `*`         | Exact            | 8          |
-| Exact       | `*`              | 6          |
-| `*`         | `*`              | 5          |
+| Exact       | Exact            | 15         |
+| `*`         | Exact            | 12         |
+| Exact       | `*`              | 10         |
+| `*`         | `*`              | 7          |
 
 The precedence value can be read from the [API](/api/connect/intentions.html)
 after an intention is created.
@@ -109,11 +109,9 @@ source name. In practice, this is a moot point since authorizing a connection
 has an exact source and destination value so its impossible for two
 valid non-wildcard intentions to match.
 
-The numbers in the table above are not stable. Their ordering will remain
-fixed but the actual number values may change in the future.
-The numbers are non-contiguous because there are
-some unused values in the middle in preparation for a future version of
-Consul supporting namespaces.
+~> **NOTE:** The numbers in the table above are not stable. Their ordering will remain
+  fixed but the actual number values may change in the future.
+  The numbers are non-contiguous to make way for future features.
 
 ## Intention Management Permissions
 
-- 
GitLab