workload identity: add support for extra claims config for Vault #23675
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@the-nando if you have a moment, take a look at the E2E test I've added here and see if that looks vaguely like what your team is looking to do. |
schmichael
reviewed
Jul 24, 2024
There was a problem hiding this comment.
We should also update docs: https://developer.hashicorp.com/nomad/docs/concepts/workload-identity
They're also lacking the standard JWT claims like aud and sub. It would be nice to list out all of the claims Nomad adds with some explanation as the difference between nomad_workload_id, nomad_job_id, and sub are pretty subtle and have security implications for federated identity users.
|
Moving to draft while the design gets further discussed in #23510 |
|
Going to rework this PR to what's been described in #23510 (comment) |
tgross
added a commit
that referenced
this pull request
Jul 31, 2024
Upcoming work to add extensibility to identity claims for Vault (ref #23510) will require exposing server configuration and more objects from state to the process of creating an `IdentityClaims` struct. Depending on how we inject these parameters into the constructor, we end up creating circular dependencies or a lot more logic in the setup in the plan applier and alloc endpoint. There are three contexts where we call `NewIdentityClaims`: the plan applier (where we only care about the default identity), signing task identities, and signing service identities. Each needs different parameters. So we'll refactor the constructor as a builder with methods that the caller can decide to use (or not) depending on context. I've pulled this work out of #23675 to make it easier to review separately. Ref: #23510 Ref: #23675 Ref: https://hashicorp.atlassian.net/browse/NET-10372 Ref: https://hashicorp.atlassian.net/browse/NET-10387
tgross
force-pushed
the
f-workload-identity-namespaced-job
branch
from
7d8e326 to
631d612
Compare
tgross
force-pushed
the
f-workload-identity-namespaced-job
branch
from
631d612 to
654d609
Compare
tgross
changed the title
tgross
force-pushed
the
f-workload-identity-namespaced-job
branch
from
654d609 to
113c3d3
Compare
tgross
force-pushed
the
f-workload-identity-namespaced-job
branch
from
113c3d3 to
febe1ef
Compare
schmichael
approved these changes
Jul 31, 2024
There was a problem hiding this comment.
Thanks for sticking with this everyone! I think this solution looks great. Enables a specific useful workflow and doesn't leak into places its not needed. Gluing federated identities and auth methods together gets a bit mind bending with all these parameters and claims, but from looking at other federated identity implementations that seems unavoidable.
|
Going to wait until #23708 is reviewed and merged to merge this. |
tgross
added a commit
that referenced
this pull request
Aug 5, 2024
Upcoming work to add extensibility to identity claims for Vault (ref #23510) will require exposing server configuration and more objects from state to the process of creating an `IdentityClaims` struct. Depending on how we inject these parameters into the constructor, we end up creating circular dependencies or a lot more logic in the setup in the plan applier and alloc endpoint. There are three contexts where we call `NewIdentityClaims`: the plan applier (where we only care about the default identity), signing task identities, and signing service identities. Each needs different parameters. So we'll refactor the constructor as a builder with methods that the caller can decide to use (or not) depending on context. I've pulled this work out of #23675 to make it easier to review separately. Ref: #23510 Ref: #23675 Ref: https://hashicorp.atlassian.net/browse/NET-10372 Ref: https://hashicorp.atlassian.net/browse/NET-10387
Although we encourage users to use Vault roles, sometimes they're going to want to assign policies based on entity and pre-create entities and aliases based on claims. This allows them to use single default role (or at least small number of them) that has a templated policy, but have an escape hatch from that. When defining Vault entities the `user_claim` must be unique. When writing Vault binding rules for use with Nomad workload identities the binding rule won't be able to create a 1:1 mapping because the selector language allows accessing only a single field. The `nomad_job_id` claim isn't sufficient to uniquely identify a job because of namespaces. It's possible to create a JWT auth role with `bound_claims` to avoid this becoming a security problem, but this doesn't allow for correct accounting of user claims. Add support for an `extra_claims` block on the server's `default_identity` blocks for Vault. This allows a cluster administrator to add a custom claim on all allocations. The values for these claims are interpolatable with a limited subset of fields, similar to how we interpolate the task environment. Fixes: #23510 Ref: https://hashicorp.atlassian.net/browse/NET-10372 Ref: https://hashicorp.atlassian.net/browse/NET-10387
tgross
force-pushed
the
f-workload-identity-namespaced-job
branch
from
0e9fd92 to
90d646a
Compare
|
Missing changelog added in #23817 |
tgross
added a commit
that referenced
this pull request
Aug 14, 2024
When we interpolate job fields for the `vault.default_identity.extra_claims` block, we forgot to use the parent job ID when that's available (as we do for all other claims). This changeset fixes the bug and adds a helper method that'll hopefully remind us to do this going forward. Also added a missing changelog entry for #23675 where we implemented the `extra_claims` block originally, which shipped in Nomad 1.8.3. Fixes: #23798
tgross
added a commit
that referenced
this pull request
Sep 3, 2024
When we interpolate job fields for the `vault.default_identity.extra_claims` block, we forgot to use the parent job ID when that's available (as we do for all other claims). This changeset fixes the bug and adds a helper method that'll hopefully remind us to do this going forward. Also added a missing changelog entry for #23675 where we implemented the `extra_claims` block originally, which shipped in Nomad 1.8.3. Fixes: #23798
tgross
added a commit
that referenced
this pull request
Sep 3, 2024
#23817) When we interpolate job fields for the `vault.default_identity.extra_claims` block, we forgot to use the parent job ID when that's available (as we do for all other claims). This changeset fixes the bug and adds a helper method that'll hopefully remind us to do this going forward. Also added a missing changelog entry for #23675 where we implemented the `extra_claims` block originally, which shipped in Nomad 1.8.3. Fixes: #23798
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Although we encourage users to use Vault roles, sometimes they're going to want to assign policies based on entity and pre-create entities and aliases based on claims. This allows them to use single default role (or at least small number of them) that has a templated policy, but have an escape hatch from that.
When defining Vault entities the
user_claimmust be unique. When writing Vault binding rules for use with Nomad workload identities the binding rule won't be able to create a 1:1 mapping because the selector language allows accessing only a single field. Thenomad_job_idclaim isn't sufficient to uniquely identify a job because of namespaces. It's possible to create a JWT auth role withbound_claimsto avoid this becoming a security problem, but this doesn't allow for correct accounting of user claims.Add support for an
extra_claimsblock on the server'sdefault_identityblocks for Vault. This allows a cluster administrator to add a custom claim on all allocations. The values for these claims are interpolatable with a limited subset of fields, similar to how we interpolate the task environment.Fixes: #23510
Ref: https://hashicorp.atlassian.net/browse/NET-10372
Ref: https://hashicorp.atlassian.net/browse/NET-10387
Note to reviewers: this PR targets the branch of #23708 so that it includes all those changes, but I expect the refactoring PR will merge first and then this PR will target
mainagain.