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
This commit is contained in:
Seth Hoenig 2023-01-27 18:15:51 -06:00 committed by GitHub
parent d2fc65764e
commit 074b76e3bf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 29 additions and 4 deletions

3
.changelog/15928.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
consul: Fixed a bug where acceptable service identity on Consul token was not accepted
```

View File

@ -128,6 +128,7 @@ const (
ExampleOperatorTokenID3 = "6177d1b9-c0f6-4118-b891-d818a3cb80b1" ExampleOperatorTokenID3 = "6177d1b9-c0f6-4118-b891-d818a3cb80b1"
ExampleOperatorTokenID4 = "754ae26c-f3cc-e088-d486-9c0d20f5eaea" ExampleOperatorTokenID4 = "754ae26c-f3cc-e088-d486-9c0d20f5eaea"
ExampleOperatorTokenID5 = "097cbb45-506b-c79c-ec38-82eb0dc0794a" 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 // Example Consul ACL tokens for use in tests that match the policies as the
@ -214,6 +215,16 @@ var (
Namespace: "", 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" // In Consul namespace "banana"
ExampleOperatorToken10 = &api.ACLToken{ ExampleOperatorToken10 = &api.ACLToken{

View File

@ -160,6 +160,14 @@ func (c *consulACLsAPI) canWriteService(namespace, service string, token *api.AC
// treat that like an exact match to preserve backwards compatibility // treat that like an exact match to preserve backwards compatibility
matches := (namespace == token.Namespace) || (namespace == "" && token.Namespace == "default") 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 // check each policy directly attached to the token
for _, policyRef := range token.Policies { for _, policyRef := range token.Policies {
if allowable, err := c.policyAllowsServiceWrite(matches, namespace, service, policyRef.ID); err != nil { if allowable, err := c.policyAllowsServiceWrite(matches, namespace, service, policyRef.ID); err != nil {

View File

@ -1,5 +1,4 @@
//go:build !ent //go:build !ent
// +build !ent
package nomad package nomad
@ -10,7 +9,7 @@ import (
"github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/ci"
"github.com/hashicorp/nomad/command/agent/consul" "github.com/hashicorp/nomad/command/agent/consul"
"github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/helper/testlog"
"github.com/stretchr/testify/require" "github.com/shoenig/test/must"
) )
func TestConsulACLsAPI_hasSufficientPolicy_oss(t *testing.T) { func TestConsulACLsAPI_hasSufficientPolicy_oss(t *testing.T) {
@ -23,8 +22,8 @@ func TestConsulACLsAPI_hasSufficientPolicy_oss(t *testing.T) {
logger: logger, logger: logger,
} }
result, err := cAPI.canWriteService(namespace, task, token) result, err := cAPI.canWriteService(namespace, task, token)
require.NoError(t, err) must.NoError(t, err)
require.Equal(t, exp, result) must.Eq(t, exp, result)
} }
// In Nomad OSS, group consul namespace will always be empty string. // 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) { t.Run("working role only", func(t *testing.T) {
try(t, "", "service1", consul.ExampleOperatorToken4, true) try(t, "", "service1", consul.ExampleOperatorToken4, true)
}) })
t.Run("working service identity only", func(t *testing.T) {
try(t, "", "service1", consul.ExampleOperatorToken6, true)
})
}) })
} }