connect: include optional partition prefixes in SPIFFE identifiers (#10507)

NOTE: this does not include any intentions enforcement changes yet

.changelog/10507.txt

@@ -0,0 +1,3 @@
+```release-note:feature
+connect: include optional partition prefixes in SPIFFE identifiers
+```

agent/auto-config/tls.go

@@ -216,6 +216,7 @@ func (ac *AutoConfig) generateCSR() (csr string, key string, err error) {
 		Host:       unknownTrustDomain,
 		Datacenter: ac.config.Datacenter,
 		Agent:      ac.config.NodeName,
+		// TODO(rb)(partitions): populate the partition field from the agent config
 	}
 
 	caConfig, err := ac.config.ConnectCAConfiguration()

agent/cache-types/connect_ca_leaf.go

@@ -524,6 +524,7 @@ func (c *ConnectCALeaf) generateNewLeaf(req *ConnectCALeafRequest,
 		id = &connect.SpiffeIDService{
 			Host:       roots.TrustDomain,
 			Datacenter: req.Datacenter,
+			Partition:  req.TargetPartition(),
 			Namespace:  req.TargetNamespace(),
 			Service:    req.Service,
 		}
@@ -532,6 +533,7 @@ func (c *ConnectCALeaf) generateNewLeaf(req *ConnectCALeafRequest,
 		id = &connect.SpiffeIDAgent{
 			Host:       roots.TrustDomain,
 			Datacenter: req.Datacenter,
+			Partition:  req.TargetPartition(),
 			Agent:      req.Agent,
 		}
 		dnsNames = append([]string{"localhost"}, req.DNSSAN...)
@@ -676,6 +678,10 @@ func (r *ConnectCALeafRequest) Key() string {
 	return ""
 }
 
+func (req *ConnectCALeafRequest) TargetPartition() string {
+	return req.PartitionOrDefault()
+}
+
 func (r *ConnectCALeafRequest) CacheInfo() cache.RequestInfo {
 	return cache.RequestInfo{
 		Token:      r.Token,

agent/connect/uri.go

@@ -21,9 +21,9 @@ type CertURI interface {
 
 var (
 	spiffeIDServiceRegexp = regexp.MustCompile(
-		`^/ns/([^/]+)/dc/([^/]+)/svc/([^/]+)$`)
+		`^(?:/ap/([^/]+))?/ns/([^/]+)/dc/([^/]+)/svc/([^/]+)$`)
 	spiffeIDAgentRegexp = regexp.MustCompile(
-		`^/agent/client/dc/([^/]+)/id/([^/]+)$`)
+		`^(?:/ap/([^/]+))?/agent/client/dc/([^/]+)/id/([^/]+)$`)
 )
 
 // ParseCertURIFromString attempts to parse a string representation of a
@@ -56,24 +56,29 @@ func ParseCertURI(input *url.URL) (CertURI, error) {
 		// Determine the values. We assume they're sane to save cycles,
 		// but if the raw path is not empty that means that something is
 		// URL encoded so we go to the slow path.
-		ns := v[1]
+		ap := v[1]
-		dc := v[2]
+		ns := v[2]
-		service := v[3]
+		dc := v[3]
+		service := v[4]
 		if input.RawPath != "" {
 			var err error
-			if ns, err = url.PathUnescape(v[1]); err != nil {
+			if ap, err = url.PathUnescape(v[1]); err != nil {
+				return nil, fmt.Errorf("Invalid admin partition: %s", err)
+			}
+			if ns, err = url.PathUnescape(v[2]); err != nil {
 				return nil, fmt.Errorf("Invalid namespace: %s", err)
 			}
-			if dc, err = url.PathUnescape(v[2]); err != nil {
+			if dc, err = url.PathUnescape(v[3]); err != nil {
 				return nil, fmt.Errorf("Invalid datacenter: %s", err)
 			}
-			if service, err = url.PathUnescape(v[3]); err != nil {
+			if service, err = url.PathUnescape(v[4]); err != nil {
 				return nil, fmt.Errorf("Invalid service: %s", err)
 			}
 		}
 
 		return &SpiffeIDService{
 			Host:       input.Host,
+			Partition:  ap,
 			Namespace:  ns,
 			Datacenter: dc,
 			Service:    service,
@@ -82,20 +87,25 @@ func ParseCertURI(input *url.URL) (CertURI, error) {
 		// Determine the values. We assume they're sane to save cycles,
 		// but if the raw path is not empty that means that something is
 		// URL encoded so we go to the slow path.
-		dc := v[1]
+		ap := v[1]
-		agent := v[2]
+		dc := v[2]
+		agent := v[3]
 		if input.RawPath != "" {
 			var err error
-			if dc, err = url.PathUnescape(v[1]); err != nil {
+			if ap, err = url.PathUnescape(v[1]); err != nil {
+				return nil, fmt.Errorf("Invalid admin partition: %s", err)
+			}
+			if dc, err = url.PathUnescape(v[2]); err != nil {
 				return nil, fmt.Errorf("Invalid datacenter: %s", err)
 			}
-			if agent, err = url.PathUnescape(v[2]); err != nil {
+			if agent, err = url.PathUnescape(v[3]); err != nil {
 				return nil, fmt.Errorf("Invalid node: %s", err)
 			}
 		}
 
 		return &SpiffeIDAgent{
 			Host:       input.Host,
+			Partition:  ap,
 			Datacenter: dc,
 			Agent:      agent,
 		}, nil

agent/connect/uri_agent.go

@@ -1,22 +1,28 @@
 package connect
 
 import (
-	"fmt"
 	"net/url"
+
+	"github.com/hashicorp/consul/agent/structs"
 )
 
 // SpiffeIDService is the structure to represent the SPIFFE ID for an agent.
 type SpiffeIDAgent struct {
 	Host       string
+	Partition  string
 	Datacenter string
 	Agent      string
 }
 
+func (id SpiffeIDAgent) PartitionOrDefault() string {
+	return structs.PartitionOrDefault(id.Partition)
+}
+
 // URI returns the *url.URL for this SPIFFE ID.
-func (id *SpiffeIDAgent) URI() *url.URL {
+func (id SpiffeIDAgent) URI() *url.URL {
 	var result url.URL
 	result.Scheme = "spiffe"
 	result.Host = id.Host
-	result.Path = fmt.Sprintf("/agent/client/dc/%s/id/%s", id.Datacenter, id.Agent)
+	result.Path = id.uriPath()
 	return &result
 }

agent/connect/uri_agent_oss.go

@@ -0,0 +1,9 @@
+// +build !consulent
+
+package connect
+
+import "fmt"
+
+func (id SpiffeIDAgent) uriPath() string {
+	return fmt.Sprintf("/agent/client/dc/%s/id/%s", id.Datacenter, id.Agent)
+}

agent/connect/uri_agent_oss_test.go

@@ -0,0 +1,32 @@
+// +build !consulent
+
+package connect
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestSpiffeIDAgentURI(t *testing.T) {
+	t.Run("default partition", func(t *testing.T) {
+		agent := &SpiffeIDAgent{
+			Host:       "1234.consul",
+			Datacenter: "dc1",
+			Agent:      "123",
+		}
+
+		require.Equal(t, "spiffe://1234.consul/agent/client/dc/dc1/id/123", agent.URI().String())
+	})
+
+	t.Run("partitions are ignored", func(t *testing.T) {
+		agent := &SpiffeIDAgent{
+			Host:       "1234.consul",
+			Partition:  "foobar",
+			Datacenter: "dc1",
+			Agent:      "123",
+		}
+
+		require.Equal(t, "spiffe://1234.consul/agent/client/dc/dc1/id/123", agent.URI().String())
+	})
+}

agent/connect/uri_agent_test.go

@@ -1,17 +0,0 @@
-package connect
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/require"
-)
-
-func TestSpiffeIDAgentURI(t *testing.T) {
-	agent := &SpiffeIDAgent{
-		Host:       "1234.consul",
-		Datacenter: "dc1",
-		Agent:      "123",
-	}
-
-	require.Equal(t, "spiffe://1234.consul/agent/client/dc/dc1/id/123", agent.URI().String())
-}

agent/connect/uri_service.go

@@ -1,24 +1,37 @@
 package connect
 
 import (
-	"fmt"
 	"net/url"
+
+	"github.com/hashicorp/consul/agent/structs"
 )
 
 // SpiffeIDService is the structure to represent the SPIFFE ID for a service.
 type SpiffeIDService struct {
 	Host       string
+	Partition  string
 	Namespace  string
 	Datacenter string
 	Service    string
 }
 
+func (id SpiffeIDService) NamespaceOrDefault() string {
+	return structs.NamespaceOrDefault(id.Namespace)
+}
+
+func (id SpiffeIDService) MatchesPartition(partition string) bool {
+	return id.PartitionOrDefault() == structs.PartitionOrDefault(partition)
+}
+
+func (id SpiffeIDService) PartitionOrDefault() string {
+	return structs.PartitionOrDefault(id.Partition)
+}
+
 // URI returns the *url.URL for this SPIFFE ID.
-func (id *SpiffeIDService) URI() *url.URL {
+func (id SpiffeIDService) URI() *url.URL {
 	var result url.URL
 	result.Scheme = "spiffe"
 	result.Host = id.Host
-	result.Path = fmt.Sprintf("/ns/%s/dc/%s/svc/%s",
+	result.Path = id.uriPath()
-		id.Namespace, id.Datacenter, id.Service)
 	return &result
 }

agent/connect/uri_service_oss.go

@@ -3,11 +3,21 @@
 package connect
 
 import (
+	"fmt"
+
 	"github.com/hashicorp/consul/agent/structs"
 )
 
 // GetEnterpriseMeta will synthesize an EnterpriseMeta struct from the SpiffeIDService.
 // in OSS this just returns an empty (but never nil) struct pointer
-func (id *SpiffeIDService) GetEnterpriseMeta() *structs.EnterpriseMeta {
+func (id SpiffeIDService) GetEnterpriseMeta() *structs.EnterpriseMeta {
 	return &structs.EnterpriseMeta{}
 }
+
+func (id SpiffeIDService) uriPath() string {
+	return fmt.Sprintf("/ns/%s/dc/%s/svc/%s",
+		id.NamespaceOrDefault(),
+		id.Datacenter,
+		id.Service,
+	)
+}

agent/connect/uri_service_oss_test.go

@@ -0,0 +1,40 @@
+// +build !consulent
+
+package connect
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestSpiffeIDServiceURI(t *testing.T) {
+	t.Run("default partition; default namespace", func(t *testing.T) {
+		svc := &SpiffeIDService{
+			Host:       "1234.consul",
+			Datacenter: "dc1",
+			Service:    "web",
+		}
+		require.Equal(t, "spiffe://1234.consul/ns/default/dc/dc1/svc/web", svc.URI().String())
+	})
+
+	t.Run("partitions are ignored", func(t *testing.T) {
+		svc := &SpiffeIDService{
+			Host:       "1234.consul",
+			Partition:  "other",
+			Datacenter: "dc1",
+			Service:    "web",
+		}
+		require.Equal(t, "spiffe://1234.consul/ns/default/dc/dc1/svc/web", svc.URI().String())
+	})
+
+	t.Run("namespaces are ignored", func(t *testing.T) {
+		svc := &SpiffeIDService{
+			Host:       "1234.consul",
+			Namespace:  "other",
+			Datacenter: "dc1",
+			Service:    "web",
+		}
+		require.Equal(t, "spiffe://1234.consul/ns/default/dc/dc1/svc/web", svc.URI().String())
+	})
+}

agent/connect/uri_signing.go

@@ -16,7 +16,7 @@ type SpiffeIDSigning struct {
 }
 
 // URI returns the *url.URL for this SPIFFE ID.
-func (id *SpiffeIDSigning) URI() *url.URL {
+func (id SpiffeIDSigning) URI() *url.URL {
 	var result url.URL
 	result.Scheme = "spiffe"
 	result.Host = id.Host()
@@ -24,7 +24,7 @@ func (id *SpiffeIDSigning) URI() *url.URL {
 }
 
 // Host is the canonical representation as a DNS-compatible hostname.
-func (id *SpiffeIDSigning) Host() string {
+func (id SpiffeIDSigning) Host() string {
 	return strings.ToLower(fmt.Sprintf("%s.%s", id.ClusterID, id.Domain))
 }
 
@@ -36,7 +36,7 @@ func (id *SpiffeIDSigning) Host() string {
 // method on CertURI interface since we don't intend this to be extensible
 // outside and it's easier to reason about the security properties when they are
 // all in one place with "allowlist" semantics.
-func (id *SpiffeIDSigning) CanSign(cu CertURI) bool {
+func (id SpiffeIDSigning) CanSign(cu CertURI) bool {
 	switch other := cu.(type) {
 	case *SpiffeIDSigning:
 		// We can only sign other CA certificates for the same trust domain. Note

agent/connect/uri_signing_test.go

@@ -79,25 +79,25 @@ func TestSpiffeIDSigning_CanSign(t *testing.T) {
 		{
 			name:  "service - good",
 			id:    testSigning,
-			input: &SpiffeIDService{TestClusterID + ".consul", "default", "dc1", "web"},
+			input: &SpiffeIDService{Host: TestClusterID + ".consul", Namespace: "default", Datacenter: "dc1", Service: "web"},
 			want:  true,
 		},
 		{
 			name:  "service - good midex case",
 			id:    testSigning,
-			input: &SpiffeIDService{strings.ToUpper(TestClusterID) + ".CONsuL", "defAUlt", "dc1", "WEB"},
+			input: &SpiffeIDService{Host: strings.ToUpper(TestClusterID) + ".CONsuL", Namespace: "defAUlt", Datacenter: "dc1", Service: "WEB"},
 			want:  true,
 		},
 		{
 			name:  "service - different cluster",
 			id:    testSigning,
-			input: &SpiffeIDService{"55555555-4444-3333-2222-111111111111.consul", "default", "dc1", "web"},
+			input: &SpiffeIDService{Host: "55555555-4444-3333-2222-111111111111.consul", Namespace: "default", Datacenter: "dc1", Service: "web"},
 			want:  false,
 		},
 		{
 			name:  "service - different TLD",
 			id:    testSigning,
-			input: &SpiffeIDService{TestClusterID + ".fake", "default", "dc1", "web"},
+			input: &SpiffeIDService{Host: TestClusterID + ".fake", Namespace: "default", Datacenter: "dc1", Service: "web"},
 			want:  false,
 		},
 	}

agent/connect/uri_test.go

@@ -3,86 +3,113 @@ package connect
 import (
 	"testing"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/hashicorp/consul/sdk/testutil"
 )
 
-// testCertURICases contains the test cases for parsing and encoding
-// the SPIFFE IDs. This is a global since it is used in multiple test functions.
-var testCertURICases = []struct {
-	Name       string
-	URI        string
-	Struct     interface{}
-	ParseError string
-}{
-	{
-		"invalid scheme",
-		"http://google.com/",
-		nil,
-		"scheme",
-	},
-
-	{
-		"basic service ID",
-		"spiffe://1234.consul/ns/default/dc/dc01/svc/web",
-		&SpiffeIDService{
-			Host:       "1234.consul",
-			Namespace:  "default",
-			Datacenter: "dc01",
-			Service:    "web",
-		},
-		"",
-	},
-
-	{
-		"basic agent ID",
-		"spiffe://1234.consul/agent/client/dc/dc1/id/uuid",
-		&SpiffeIDAgent{
-			Host:       "1234.consul",
-			Datacenter: "dc1",
-			Agent:      "uuid",
-		},
-		"",
-	},
-
-	{
-		"service with URL-encoded values",
-		"spiffe://1234.consul/ns/foo%2Fbar/dc/bar%2Fbaz/svc/baz%2Fqux",
-		&SpiffeIDService{
-			Host:       "1234.consul",
-			Namespace:  "foo/bar",
-			Datacenter: "bar/baz",
-			Service:    "baz/qux",
-		},
-		"",
-	},
-
-	{
-		"signing ID",
-		"spiffe://1234.consul",
-		&SpiffeIDSigning{
-			ClusterID: "1234",
-			Domain:    "consul",
-		},
-		"",
-	},
-}
-
 func TestParseCertURIFromString(t *testing.T) {
-	for _, tc := range testCertURICases {
+	var cases = []struct {
-		t.Run(tc.Name, func(t *testing.T) {
+		Name       string
-			assert := assert.New(t)
+		URI        string
+		Struct     interface{}
+		ParseError string
+	}{
+		{
+			"invalid scheme",
+			"http://google.com/",
+			nil,
+			"scheme",
+		},
+		{
+			"basic service ID",
+			"spiffe://1234.consul/ns/default/dc/dc01/svc/web",
+			&SpiffeIDService{
+				Host:       "1234.consul",
+				Namespace:  "default",
+				Datacenter: "dc01",
+				Service:    "web",
+			},
+			"",
+		},
+		{
+			"basic service ID with partition",
+			"spiffe://1234.consul/ap/bizdev/ns/default/dc/dc01/svc/web",
+			&SpiffeIDService{
+				Host:       "1234.consul",
+				Partition:  "bizdev",
+				Namespace:  "default",
+				Datacenter: "dc01",
+				Service:    "web",
+			},
+			"",
+		},
+		{
+			"basic agent ID",
+			"spiffe://1234.consul/agent/client/dc/dc1/id/uuid",
+			&SpiffeIDAgent{
+				Host:       "1234.consul",
+				Datacenter: "dc1",
+				Agent:      "uuid",
+			},
+			"",
+		},
+		{
+			"basic agent ID with partition",
+			"spiffe://1234.consul/ap/bizdev/agent/client/dc/dc1/id/uuid",
+			&SpiffeIDAgent{
+				Host:       "1234.consul",
+				Partition:  "bizdev",
+				Datacenter: "dc1",
+				Agent:      "uuid",
+			},
+			"",
+		},
+		{
+			"service with URL-encoded values",
+			"spiffe://1234.consul/ns/foo%2Fbar/dc/bar%2Fbaz/svc/baz%2Fqux",
+			&SpiffeIDService{
+				Host:       "1234.consul",
+				Namespace:  "foo/bar",
+				Datacenter: "bar/baz",
+				Service:    "baz/qux",
+			},
+			"",
+		},
+		{
+			"service with URL-encoded values with partition",
+			"spiffe://1234.consul/ap/biz%2Fdev/ns/foo%2Fbar/dc/bar%2Fbaz/svc/baz%2Fqux",
+			&SpiffeIDService{
+				Host:       "1234.consul",
+				Partition:  "biz/dev",
+				Namespace:  "foo/bar",
+				Datacenter: "bar/baz",
+				Service:    "baz/qux",
+			},
+			"",
+		},
+		{
+			"signing ID",
+			"spiffe://1234.consul",
+			&SpiffeIDSigning{
+				ClusterID: "1234",
+				Domain:    "consul",
+			},
+			"",
+		},
+	}
 
-			// Parse the ID and check the error/return value
+	for _, tc := range cases {
+		t.Run(tc.Name, func(t *testing.T) {
 			actual, err := ParseCertURIFromString(tc.URI)
-			if err != nil {
+			if tc.ParseError != "" {
-				t.Logf("parse error: %s", err.Error())
+				require.Error(t, err)
+				require.Contains(t, err.Error(), tc.ParseError)
+				testutil.RequireErrorContains(t, err, tc.ParseError)
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, tc.Struct, actual)
 			}
-			assert.Equal(tc.ParseError != "", err != nil, "error value")
-			if err != nil {
-				assert.Contains(err.Error(), tc.ParseError)
-				return
-			}
-			assert.Equal(tc.Struct, actual)
 		})
 	}
 }

agent/connect_auth.go

@@ -3,6 +3,7 @@ package agent
 import (
 	"context"
 	"fmt"
+
 	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/cache"
 	cachetype "github.com/hashicorp/consul/agent/cache-types"
@@ -68,6 +69,13 @@ func (a *Agent) ConnectAuthorize(token string,
 		return returnErr(acl.ErrPermissionDenied)
 	}
 
+	if !uriService.MatchesPartition(req.TargetPartition()) {
+		reason = fmt.Sprintf("Mismatched partitions: %q != %q",
+			uriService.PartitionOrDefault(),
+			structs.PartitionOrDefault(req.TargetPartition()))
+		return false, reason, nil, nil
+	}
+
 	// Note that we DON'T explicitly validate the trust-domain matches ours. See
 	// the PR for this change for details.
 

agent/structs/connect.go

@@ -18,3 +18,7 @@ type ConnectAuthorizeRequest struct {
 	ClientCertURI    string
 	ClientCertSerial string
 }
+
+func (req *ConnectAuthorizeRequest) TargetPartition() string {
+	return req.PartitionOrDefault()
+}

agent/structs/structs_oss.go

@@ -46,6 +46,10 @@ func (m *EnterpriseMeta) NamespaceOrDefault() string {
 	return IntentionDefaultNamespace
 }
 
+func NamespaceOrDefault(_ string) string {
+	return IntentionDefaultNamespace
+}
+
 func (m *EnterpriseMeta) NamespaceOrEmpty() string {
 	return ""
 }
@@ -54,6 +58,10 @@ func (m *EnterpriseMeta) PartitionOrDefault() string {
 	return ""
 }
 
+func PartitionOrDefault(_ string) string {
+	return ""
+}
+
 func (m *EnterpriseMeta) PartitionOrEmpty() string {
 	return ""
 }

connect/resolver.go

@@ -155,6 +155,7 @@ func (cr *ConsulResolver) resolveServiceEntry(entry *api.ServiceEntry) (string,
 		Namespace:  "default",
 		Datacenter: entry.Node.Datacenter,
 		Service:    service,
+		// NOTE: this only handles the default implicit partition currently.
 	}
 
 	return ipaddr.FormatAddressPort(addr, port), certURI, nil