Add option to expose workload token to task

[angrycub]

This PR creates a configuration flag that, when set, outputs the Nomad task identity token to the task secrets folder and loads it into the environment. The goal being to provide users credentials that they could use to perform actions based on the Nomad Workload Identity token.

[tgross]

In the upcoming WI epic @mikenomitch wrote up #15614 it seems like we want to always expose the WI token to the task. Is there a reason we want to do this as a configuration flag?

[angrycub]

Making it user selectable creates parity with the ability to emit or not emit vault tokens to the tasks, since not all tasks might be trusted, but might require workload identity for variables to be used in templates.

[tgross]

Making it user selectable creates parity with the ability to emit or not emit vault tokens to the tasks, since not all tasks might be trusted, but might require workload identity for variables to be used in templates.

Arguably there's a difference between the Nomad WI and Vault tokens, so it's not obvious to me there needs to be 1:1 parity. Also, we have to support jobspec flags basically forever once they're created. I'm open to this idea but, I'd rather have a more holistic discussion about how we want WI to be exposed to tasks than a one-off PR for a config flag.

[mikenomitch]

Oooh, I'm excited for this :)

I think since the token's purpose is to declare "I am this task", it probably makes sense to have it available by default and the flag isn't worth it. Would remove one step from configuring workflows that use it too.

since not all tasks might be trusted curious what you were thinking about with this comment @angrycub. Any specific workflow/use case?

[tgross]

I think since the token's purpose is to declare "I am this task", it probably makes sense to have it available by default and the flag isn't worth it.

I think you're on the right track here. Having the WI token just lets the workload claim an identity -- it doesn't expose any new capabilities (other than workload-scoped variables, which we already grant the workload via template). You still need to configure a policy for that identity for it to be at all interesting to the workload.

[schmichael]

I pushed cae00049787f6b66ec19101b32d3481d1346abea to split the single bool into a block similar to vault with env and file bools like:

identity {
  env  = true
  file = true
}

It defaults to disabled and each parameter must be enabled individually which differs from vault which defaults to env = true and always writes the token file.

To match Vault we could make file and env default to true if identity is set so that:

task "foo" {
  identity {}

  # ...
}

...gets both an env var and file by default.

Regardless of which direction we go I think we should get #13343 merged ASAP after this as it's nice to give users fine grained control.

[shoenig]

Suggested change

	if err := ioutil.WriteFile(h.tokenPath, []byte(token), 0666); err != nil {
	if err := ioutil.WriteFile(h.tokenPath, []byte(token), 0666); err != nil {

Does this file need to be world writable?

[schmichael]

Hm, this copied the Vault token file's behavior. My assumptions are:

1. This is necessary because we don't know what user the task will run as
2. This is safe because of directory permissions on the parent dir

If these aren't true, I'd love to fix them. If nothing else it's an awful smell.

[shoenig]

Hmm yeah, maybe there's something we could do if Task.User happens to be set?

But my real question is why does the file need to be writable? Would 0664 or even 0644 be sufficient?

[schmichael]

Hmm yeah, maybe there's something we could do if Task.User happens to be set?

Yeah, I'll look into this a bit. Perhaps the Vault token is constrained by backward compat in a way this new functionality shouldn't be.

But my real question is why does the file need to be writable? Would 0664 or even 0644 be sufficient?

Ah, that's so folks can delete it for workflows like:

1. Start trusted supervisor process
2. Supervisor process reads token, does its privileged thing, deletes file
3. Supervisor starts untrusted work (perhaps via fork/exec) that now has no access to the token

[schmichael]

Just realized my 1-3 workflow above is broken by the fact that alloc updates from the server will rewrite the token, so that use case is right out...

Maybe it shouldn't be world writable 🤔

[schmichael]

The good: Checkout what I came up with in helper/users.WriteFileFor. It appears to degrade gracefully when Task.User is unset (even without the explicit guard I added) as well as properly set the mode and owner when running as root on Linux. All of this without any OS specific implementations so it should even work on any Unix.

The bad: I don't think there's anything bad here, but it definitely increases risk. I think that's tolerable for a new off-by-default feature.

The ugly: Check out the Dockerfile for our example job: https://github.com/docker-library/redis/blob/master/Dockerfile.template#L3-L4 It creates a user within the container and runs Redis as that. Setting task.user = "nobody" does still work. Redis runs as nobody within the container, and nomad_secret gets properly owned by nobody. Without setting task.user the fallback is used, so Redis, running as redis, is still able to access nomad_secret! So... it's ugly but it works?

I haven't tested it with Docker and user namespaces.

[schmichael]

@shoenig mind peeking at this and resolving the comment thread if it looks good to you?

[shoenig]

I added this comment about Windows
https://github.com/hashicorp/nomad/pull/15755/files#r1092065794