From 074b76e3bfe6313631f896c7da4e3043a5593baa Mon Sep 17 00:00:00 2001
From: Seth Hoenig <shoenig@duck.com>
Date: Fri, 27 Jan 2023 18:15:51 -0600
Subject: [PATCH] consul: check for acceptable service identity on consul
 tokens (#15928)

When registering a job with a service and 'consul.allow_unauthenticated=false',
we scan the given Consul token for an acceptable policy or role with an
acceptable policy, but did not scan for an acceptable service identity (which
is backed by an acceptable virtual policy). This PR updates our consul token
validation to also accept a matching service identity when registering a service
into Consul.

Fixes #15902
---
 .changelog/15928.txt                |  3 +++
 command/agent/consul/acl_testing.go | 11 +++++++++++
 nomad/consul_policy.go              |  8 ++++++++
 nomad/consul_policy_oss_test.go     | 11 +++++++----
 4 files changed, 29 insertions(+), 4 deletions(-)
 create mode 100644 .changelog/15928.txt

diff --git a/.changelog/15928.txt b/.changelog/15928.txt
new file mode 100644
index 000000000..8d334b184
--- /dev/null
+++ b/.changelog/15928.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+consul: Fixed a bug where acceptable service identity on Consul token was not accepted
+```
diff --git a/command/agent/consul/acl_testing.go b/command/agent/consul/acl_testing.go
index 01146a8b2..9ead73581 100644
--- a/command/agent/consul/acl_testing.go
+++ b/command/agent/consul/acl_testing.go
@@ -128,6 +128,7 @@ const (
 	ExampleOperatorTokenID3 = "6177d1b9-c0f6-4118-b891-d818a3cb80b1"
 	ExampleOperatorTokenID4 = "754ae26c-f3cc-e088-d486-9c0d20f5eaea"
 	ExampleOperatorTokenID5 = "097cbb45-506b-c79c-ec38-82eb0dc0794a"
+	ExampleOperatorTokenID6 = "6268bd42-6f72-4c90-9c83-90ed6336dcf9"
 )
 
 // Example Consul ACL tokens for use in tests that match the policies as the
@@ -214,6 +215,16 @@ var (
 		Namespace: "",
 	}
 
+	ExampleOperatorToken6 = &api.ACLToken{
+		SecretID:    ExampleOperatorTokenID6,
+		AccessorID:  "93786935-8856-6e17-0488-c5370a1f044e",
+		Description: "Operator Token 6",
+		ServiceIdentities: []*api.ACLServiceIdentity{
+			{ServiceName: "service1"},
+		},
+		Namespace: "",
+	}
+
 	// In Consul namespace "banana"
 
 	ExampleOperatorToken10 = &api.ACLToken{
diff --git a/nomad/consul_policy.go b/nomad/consul_policy.go
index 71b4e6b6d..d99f320f9 100644
--- a/nomad/consul_policy.go
+++ b/nomad/consul_policy.go
@@ -160,6 +160,14 @@ func (c *consulACLsAPI) canWriteService(namespace, service string, token *api.AC
 	// treat that like an exact match to preserve backwards compatibility
 	matches := (namespace == token.Namespace) || (namespace == "" && token.Namespace == "default")
 
+	// check each service identity attached to the token -
+	// the virtual policy for service identities enables service:write
+	for _, si := range token.ServiceIdentities {
+		if si.ServiceName == service {
+			return true, nil
+		}
+	}
+
 	// check each policy directly attached to the token
 	for _, policyRef := range token.Policies {
 		if allowable, err := c.policyAllowsServiceWrite(matches, namespace, service, policyRef.ID); err != nil {
diff --git a/nomad/consul_policy_oss_test.go b/nomad/consul_policy_oss_test.go
index 9936c4fe9..fcd149ad2 100644
--- a/nomad/consul_policy_oss_test.go
+++ b/nomad/consul_policy_oss_test.go
@@ -1,5 +1,4 @@
 //go:build !ent
-// +build !ent
 
 package nomad
 
@@ -10,7 +9,7 @@ import (
 	"github.com/hashicorp/nomad/ci"
 	"github.com/hashicorp/nomad/command/agent/consul"
 	"github.com/hashicorp/nomad/helper/testlog"
-	"github.com/stretchr/testify/require"
+	"github.com/shoenig/test/must"
 )
 
 func TestConsulACLsAPI_hasSufficientPolicy_oss(t *testing.T) {
@@ -23,8 +22,8 @@ func TestConsulACLsAPI_hasSufficientPolicy_oss(t *testing.T) {
 			logger:    logger,
 		}
 		result, err := cAPI.canWriteService(namespace, task, token)
-		require.NoError(t, err)
-		require.Equal(t, exp, result)
+		must.NoError(t, err)
+		must.Eq(t, exp, result)
 	}
 
 	// In Nomad OSS, group consul namespace will always be empty string.
@@ -41,5 +40,9 @@ func TestConsulACLsAPI_hasSufficientPolicy_oss(t *testing.T) {
 		t.Run("working role only", func(t *testing.T) {
 			try(t, "", "service1", consul.ExampleOperatorToken4, true)
 		})
+
+		t.Run("working service identity only", func(t *testing.T) {
+			try(t, "", "service1", consul.ExampleOperatorToken6, true)
+		})
 	})
 }