Verify trust domain on /authorize calls

agent/agent_endpoint.go

@@ -1159,8 +1159,30 @@ func (s *HTTPServer) AgentConnectAuthorize(resp http.ResponseWriter, req *http.R
 		return nil, acl.ErrPermissionDenied
 	}
 
-	// TODO(mitchellh): we need to verify more things here, such as the
-	// trust domain, blacklist lookup of the serial, etc.
+	// Validate the trust domain matches ours. Later we will support explicit
+	// external federation but not built yet.
+	rootArgs := &structs.DCSpecificRequest{Datacenter: s.agent.config.Datacenter}
+	raw, err := s.agent.cache.Get(cachetype.ConnectCARootName, rootArgs)
+	if err != nil {
+		return nil, err
+	}
+
+	roots, ok := raw.(*structs.IndexedCARoots)
+	if !ok {
+		return nil, fmt.Errorf("internal error: roots response type not correct")
+	}
+	if roots.TrustDomain == "" {
+		return nil, fmt.Errorf("connect CA not bootstrapped yet")
+	}
+	if roots.TrustDomain != strings.ToLower(uriService.Host) {
+		return &connectAuthorizeResp{
+			Authorized: false,
+			Reason: fmt.Sprintf("Identity from an external trust domain: %s",
+				uriService.Host),
+		}, nil
+	}
+
+	// TODO(banks): Implement revocation list checking here.
 
 	// Get the intentions for this target service.
 	args := &structs.IntentionQueryRequest{
@@ -1177,7 +1199,7 @@ func (s *HTTPServer) AgentConnectAuthorize(resp http.ResponseWriter, req *http.R
 	}
 	args.Token = token
 
-	raw, err := s.agent.cache.Get(cachetype.IntentionMatchName, args)
+	raw, err = s.agent.cache.Get(cachetype.IntentionMatchName, args)
 	if err != nil {
 		return nil, err
 	}

agent/agent_endpoint_test.go

@@ -3286,6 +3286,17 @@ func TestAgentConnectAuthorize_idNotService(t *testing.T) {
 	assert.Contains(obj.Reason, "must be a valid")
 }
 
+func testFetchTrustDomain(t *testing.T, a *TestAgent) string {
+	req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/roots", nil)
+	resp := httptest.NewRecorder()
+	obj, err := a.srv.AgentConnectCARoots(resp, req)
+	require.NoError(t, err)
+
+	value := obj.(structs.IndexedCARoots)
+	require.NotEmpty(t, value.TrustDomain)
+	return value.TrustDomain
+}
+
 // Test when there is an intention allowing the connection
 func TestAgentConnectAuthorize_allow(t *testing.T) {
 	t.Parallel()
@@ -3296,6 +3307,8 @@ func TestAgentConnectAuthorize_allow(t *testing.T) {
 
 	target := "db"
 
+	trustDomain := testFetchTrustDomain(t, a)
+
 	// Create some intentions
 	var ixnId string
 	{
@@ -3318,7 +3331,8 @@ func TestAgentConnectAuthorize_allow(t *testing.T) {
 
 	args := &structs.ConnectAuthorizeRequest{
 		Target: target,
-		ClientCertURI: connect.TestSpiffeIDService(t, "web").URI().String(),
+		ClientCertURI: connect.TestSpiffeIDServiceWithHost(t, "web", trustDomain).
+			URI().String(),
 	}
 	req, _ := http.NewRequest("POST", "/v1/agent/connect/authorize", jsonReader(args))
 	resp := httptest.NewRecorder()
@@ -3330,8 +3344,13 @@ func TestAgentConnectAuthorize_allow(t *testing.T) {
 	require.True(obj.Authorized)
 	require.Contains(obj.Reason, "Matched")
 
-	// That should've been a cache miss, so not hit change
-	require.Equal(cacheHits, a.cache.Hits())
+	// That should've been a cache miss, so no hit change, however since
+	// testFetchTrustDomain already called Roots and caused it to be in cache, the
+	// authorize call above will also call it and see a cache hit for the Roots
+	// RPC. In other words, there are 2 cached calls in /authorize and we always
+	// expect one of them to be a hit. So asserting only 1 happened is as close as
+	// we can get to verifying that the intention match RPC was a hit.
+	require.Equal(cacheHits+1, a.cache.Hits())
 
 	// Make the request again
 	{
@@ -3346,9 +3365,10 @@ func TestAgentConnectAuthorize_allow(t *testing.T) {
 		require.Contains(obj.Reason, "Matched")
 	}
 
-	// That should've been a cache hit
-	require.Equal(cacheHits+1, a.cache.Hits())
-	cacheHits++
+	// That should've been a cache hit. We add the one hit from Roots from first
+	// call as well as the 2 from this call (Roots + Intentions).
+	require.Equal(cacheHits+1+2, a.cache.Hits())
+	cacheHits = a.cache.Hits()
 
 	// Change the intention
 	{
@@ -3384,9 +3404,9 @@ func TestAgentConnectAuthorize_allow(t *testing.T) {
 	}
 
 	// That should've been a cache hit, too, since it updated in the
-	// background.
-	require.Equal(cacheHits+1, a.cache.Hits())
-	cacheHits++
+	// background. (again 2 hits for Roots + Intentions)
+	require.Equal(cacheHits+2, a.cache.Hits())
+	cacheHits += 2
 }
 
 // Test when there is an intention denying the connection
@@ -3399,6 +3419,8 @@ func TestAgentConnectAuthorize_deny(t *testing.T) {
 
 	target := "db"
 
+	trustDomain := testFetchTrustDomain(t, a)
+
 	// Create some intentions
 	{
 		req := structs.IntentionRequest{
@@ -3418,7 +3440,8 @@ func TestAgentConnectAuthorize_deny(t *testing.T) {
 
 	args := &structs.ConnectAuthorizeRequest{
 		Target: target,
-		ClientCertURI: connect.TestSpiffeIDService(t, "web").URI().String(),
+		ClientCertURI: connect.TestSpiffeIDServiceWithHost(t, "web", trustDomain).
+			URI().String(),
 	}
 	req, _ := http.NewRequest("POST", "/v1/agent/connect/authorize", jsonReader(args))
 	resp := httptest.NewRecorder()
@@ -3431,6 +3454,53 @@ func TestAgentConnectAuthorize_deny(t *testing.T) {
 	assert.Contains(obj.Reason, "Matched")
 }
 
+// Test when there is an intention allowing service but for a different trust
+// domain.
+func TestAgentConnectAuthorize_denyTrustDomain(t *testing.T) {
+	t.Parallel()
+
+	assert := assert.New(t)
+	a := NewTestAgent(t.Name(), "")
+	defer a.Shutdown()
+
+	target := "db"
+
+	// Create some intentions
+	{
+		req := structs.IntentionRequest{
+			Datacenter: "dc1",
+			Op:         structs.IntentionOpCreate,
+			Intention:  structs.TestIntention(t),
+		}
+		req.Intention.SourceNS = structs.IntentionDefaultNamespace
+		req.Intention.SourceName = "web"
+		req.Intention.DestinationNS = structs.IntentionDefaultNamespace
+		req.Intention.DestinationName = target
+		req.Intention.Action = structs.IntentionActionAllow
+
+		var reply string
+		assert.Nil(a.RPC("Intention.Apply", &req, &reply))
+	}
+
+	{
+		args := &structs.ConnectAuthorizeRequest{
+			Target: target,
+			// Rely on the test trust domain this will choose to not match the random
+			// one picked on agent startup.
+			ClientCertURI: connect.TestSpiffeIDService(t, "web").URI().String(),
+		}
+		req, _ := http.NewRequest("POST", "/v1/agent/connect/authorize", jsonReader(args))
+		resp := httptest.NewRecorder()
+		respRaw, err := a.srv.AgentConnectAuthorize(resp, req)
+		assert.Nil(err)
+		assert.Equal(200, resp.Code)
+
+		obj := respRaw.(*connectAuthorizeResp)
+		assert.False(obj.Authorized)
+		assert.Contains(obj.Reason, "Identity from an external trust domain")
+	}
+}
+
 func TestAgentConnectAuthorize_denyWildcard(t *testing.T) {
 	t.Parallel()
 
@@ -3440,6 +3510,8 @@ func TestAgentConnectAuthorize_denyWildcard(t *testing.T) {
 
 	target := "db"
 
+	trustDomain := testFetchTrustDomain(t, a)
+
 	// Create some intentions
 	{
 		// Deny wildcard to DB
@@ -3478,7 +3550,8 @@ func TestAgentConnectAuthorize_denyWildcard(t *testing.T) {
 	{
 		args := &structs.ConnectAuthorizeRequest{
 			Target: target,
-			ClientCertURI: connect.TestSpiffeIDService(t, "web").URI().String(),
+			ClientCertURI: connect.TestSpiffeIDServiceWithHost(t, "web", trustDomain).
+				URI().String(),
 		}
 		req, _ := http.NewRequest("POST", "/v1/agent/connect/authorize", jsonReader(args))
 		resp := httptest.NewRecorder()
@@ -3495,7 +3568,8 @@ func TestAgentConnectAuthorize_denyWildcard(t *testing.T) {
 	{
 		args := &structs.ConnectAuthorizeRequest{
 			Target: target,
-			ClientCertURI: connect.TestSpiffeIDService(t, "api").URI().String(),
+			ClientCertURI: connect.TestSpiffeIDServiceWithHost(t, "api", trustDomain).
+				URI().String(),
 		}
 		req, _ := http.NewRequest("POST", "/v1/agent/connect/authorize", jsonReader(args))
 		resp := httptest.NewRecorder()

agent/cache/cache.go

@@ -72,7 +72,7 @@ type Cache struct {
 	// of "<DC>/<ACL token>/<Request key>" in order to properly partition
 	// requests to different datacenters and ACL tokens. This format has some
 	// big drawbacks: we can't evict by datacenter, ACL token, etc. For an
-	// initial implementaiton this works and the tests are agnostic to the
+	// initial implementation this works and the tests are agnostic to the
 	// internal storage format so changing this should be possible safely.
 	entriesLock       sync.RWMutex
 	entries           map[string]cacheEntry

agent/connect/testing_spiffe.go

@@ -6,8 +6,14 @@ import (
 
 // TestSpiffeIDService returns a SPIFFE ID representing a service.
 func TestSpiffeIDService(t testing.T, service string) *SpiffeIDService {
+	return TestSpiffeIDServiceWithHost(t, service, testClusterID+".consul")
+}
+
+// TestSpiffeIDServiceWithHost returns a SPIFFE ID representing a service with
+// the specified trust domain.
+func TestSpiffeIDServiceWithHost(t testing.T, service, host string) *SpiffeIDService {
 	return &SpiffeIDService{
-		Host:       testClusterID + ".consul",
+		Host:       host,
 		Namespace:  "default",
 		Datacenter: "dc1",
 		Service:    service,