luxolus
/
open-nomad
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

service: fix regression in task access to list/read endpoint (#16316) 

When native service discovery was added, we used the node secret as the auth
token. Once Workload Identity was added in Nomad 1.4.x we needed to use the
claim token for `template` blocks, and so we allowed valid claims to bypass the
ACL policy check to preserve the existing behavior. (Invalid claims are still
rejected, so this didn't widen any security boundary.)

In reworking authentication for 1.5.0, we unintentionally removed this
bypass. For WIs without a policy attached to their job, everything works as
expected because the resulting `acl.ACL` is nil. But once a policy is attached
to the job the `acl.ACL` is no longer nil and this causes permissions errors.

Fix the regression by adding back the bypass for valid claims. In future work,
we should strongly consider getting turning the implicit policies into real
`ACLPolicy` objects (even if not stored in state) so that we don't have these
kind of brittle exceptions to the auth code.
This commit is contained in:
Tim Gross 2023-03-03 11:41:19 -05:00 committed by GitHub
parent 62a69552c1
commit 8747059b86
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 24 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/16316.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
service: Fixed a bug where attaching a policy to a job would prevent workload identities for the job from reading the service registration API
```

6
nomad/service_registration_endpoint.go
View File

@ -217,7 +217,8 @@ func (s *ServiceRegistration) List(
if err != nil { if err != nil {
return structs.ErrPermissionDenied return structs.ErrPermissionDenied
} }
if !aclObj.AllowNsOp(args.RequestNamespace(), acl.NamespaceCapabilityReadJob) { if args.GetIdentity().Claims == nil &&
!aclObj.AllowNsOp(args.RequestNamespace(), acl.NamespaceCapabilityReadJob) {
return structs.ErrPermissionDenied return structs.ErrPermissionDenied
} }
@ -381,7 +382,8 @@ func (s *ServiceRegistration) GetService(
if err != nil { if err != nil {
return structs.ErrPermissionDenied return structs.ErrPermissionDenied
} }
if !aclObj.AllowNsOp(args.RequestNamespace(), acl.NamespaceCapabilityReadJob) { if args.GetIdentity().Claims == nil &&
!aclObj.AllowNsOp(args.RequestNamespace(), acl.NamespaceCapabilityReadJob) {
return structs.ErrPermissionDenied return structs.ErrPermissionDenied
} }

18
nomad/service_registration_endpoint_test.go
View File

@ -5,7 +5,7 @@ import (
"testing" "testing"
"github.com/hashicorp/go-memdb" "github.com/hashicorp/go-memdb"
"github.com/hashicorp/net-rpc-msgpackrpc" msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc"
"github.com/hashicorp/nomad/acl" "github.com/hashicorp/nomad/acl"
"github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/ci"
"github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/mock"
@ -882,12 +882,28 @@ func TestServiceRegistration_List(t *testing.T) {
// Generate an allocation with a signed identity // Generate an allocation with a signed identity
allocs := []*structs.Allocation{mock.Alloc()} allocs := []*structs.Allocation{mock.Alloc()}
job := allocs[0].Job job := allocs[0].Job
job.Namespace = "platform"
allocs[0].Namespace = "platform"
require.NoError(t, s.State().UpsertJob(structs.MsgTypeTestSetup, 10, job)) require.NoError(t, s.State().UpsertJob(structs.MsgTypeTestSetup, 10, job))
s.signAllocIdentities(job, allocs) s.signAllocIdentities(job, allocs)
require.NoError(t, s.State().UpsertAllocs(structs.MsgTypeTestSetup, 15, allocs)) require.NoError(t, s.State().UpsertAllocs(structs.MsgTypeTestSetup, 15, allocs))
signedToken := allocs[0].SignedIdentities["web"] signedToken := allocs[0].SignedIdentities["web"]
// Associate an unrelated policy with the identity's job to
// ensure it doesn't conflict.
policy := &structs.ACLPolicy{
Name: "policy-for-identity",
Rules: mock.NodePolicy("read"),
JobACL: &structs.JobACL{
Namespace: "platform",
JobID: job.ID,
},
}
policy.SetHash()
must.NoError(t, s.State().UpsertACLPolicies(structs.MsgTypeTestSetup, 16,
[]*structs.ACLPolicy{policy}))
// Generate and upsert some service registrations. // Generate and upsert some service registrations.
services := mock.ServiceRegistrations() services := mock.ServiceRegistrations()
require.NoError(t, s.State().UpsertServiceRegistrations( require.NoError(t, s.State().UpsertServiceRegistrations(