From e4fdccdc91ceb61b3e53f313108fcf8ba65822e1 Mon Sep 17 00:00:00 2001
From: Seth Hoenig <shoenig@duck.com>
Date: Fri, 27 Jan 2023 13:35:51 -0600
Subject: [PATCH] consul: check for acceptable service identity on consul
 tokens

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 00000000000..8d334b18463
--- /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 01146a8b25b..9ead7358105 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 71b4e6b6d48..d99f320f954 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 9936c4fe932..fcd149ad26c 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)
+		})
 	})
 }