vault_token is written to /secrets world-readable

[grembo]

Update 2022-05-31: As things evolved and to clarify the mission: The best solution and my goal is to store the token in a separate directory that is not accessible from inside the container. #11905 accomplishes this by writing the vault token to a private directory with strict file permissions (also protecting the token from other unprivileged processes on the host running nomad). It then adds a parameter called file which allows writing a copy of vault token to secrets like before

Nomad writes a job's vault token to /secrets/vault_token using world-readable permissions, as default perms (0666) are used (and then umask is applied):

nomad/client/allocrunner/taskrunner/vault_hook.go

Lines 344 to 351 in eee5d90

	// writeToken writes the given token to disk
	func (h *vaultHook) writeToken(token string) error {
	if err := ioutil.WriteFile(h.tokenPath, []byte(token), 0666); err != nil {
	return fmt.Errorf("failed to write vault token: %v", err)
	}
	return nil
	}

In practice (using a default umask 0022) this means /secrets/vault_token is world readable and writeable only by root (0644).

As changing this might break existing setups, an alternative would be to add a perms parameter to the vault stanza like a job's template stanza already supports.

If there's a chance for adoption, I'm happy to implement/open a pull request for the feature.

Update: Misread how perms work in writefile, so I updated the issue title accordingly. This makes writing a patch for this more complicated (I already prepared one), as - in theory - existing users could have altered umask. It also means that the issue could be worked around by altering umask for the nomad process, even though this might have unexpected side-effects.

[jrasell]

Hi @grembo and thanks for raising this along with the PR. I'll raise this internally for discussion and hope to come back with feedback on the PR in a timely manner.

[schmichael]

The implementation of #11905 looks good, but I'm not sure it adds significant protections.

To be clear the Vault token is protected from arbitrary readers by two other layers:

1. It is protected from reading by processes outside Nomad by the permissions on alloc_dir being only readable by the Nomad agent's user (preferably root).
2. It is protected from reading by other allocations and tasks by the chroot/filesystem-isolation of the task driver used. (This doesn't apply to raw_exec, but raw_exec runs as root so there's no hiding anything from it!).

It's not clear to me what dropping permissions on the token file improves: even if it's only user-readable that doesn't add any additional protections than 1 or 2 above. Non-root system process still cannot read it and chroots still protect it from other tasks. Even without chroots changing the user permissions alone are insufficient to protect the token file from being read by other tasks as all tasks in Nomad can run as the same user.

That being said I'd love to lock down access to the vault token as much as possible, so I'd love to be convinced file_perms helps or maybe there's something else we could do? Perhaps a file bool to allow skipping the file altogether like we provide with env for the environment variable.

[grembo]

Hi @schmichael,

Thank you for the detailed response.

We have multiple applications for vault tokens in nomad:

• Use it for issuing various client certificates (e.g., database access): In this application, writing the file outside of one of the mounted directories would be best (I think it still should be written somewhere in the allocation to survive nomad restarts using a strict file mask). So the file bool you suggested would be useful in this scenario.
• Use it to retrieve secrets before a process is started inside the chroot - what we do there is, take the token, retrieve what is needed and then start the daemon in questions with dropped privileges (e.g., user www). In this scenario, protecting the token is useful, as removing it/changing its permissions inside the chroot won't stick in case a token is re-issued (this could be partially addressed by the file bool you suggested).
• Use it inside the actual service process (e.g., to access vault to sign data using the transit secrets engine). In this case the current token is always needed in case it gets re-issued, so the environment variable can't be used. Even in this scenario having some protection on the secrets dir/files inside of it would be nice, as a background process in the chroot could be used to make it available to the consumer (we already do this for other secrets by watching the secrets dir for a controlled/complex reload that goes beyond just sending a signal).

Now, in an ideal world, we could issue two vault tokens through the job specification, so anything concerning issuing client certificates would happen outside of the chroot, while the vault token to access the vault transit engine is shared with the chroot. Basically one vault token used in the template stanza and one to be published inside the chroot. As this isn't possible (yet), it would be good to have strict file permissions, so there is some control of which processes can access this information inside of the chroot.

Update: I wonder if it would be possible to create a vault token that has permissions to issue other vault tokens and use that in a template stanza to issue vault tokens to access the transit engine with strict permissions. Combined with having the option to disable writing the vault_token using the file bool this would actually address all our use cases.

Beyond our specific use case, I think that having files containing secrets with world readable permissions never looks good - so simply from a "I have to explain this to somebody auditing the system" perspective it would be good to be able to simply set strict permissions to avoid having to deal with those questions/make it easier to audit a setup overall (also internally).

Originally I though of using file_perms = 000 to do what the suggested file bool would do, but figured that this would be way too implicit and left it for a separate PR. Also, I would need to know where to write vault_token to, so it can be reloaded on nomad restarts (maybe alloc/<task>/vault_token?).

It is protected from reading by processes outside Nomad by the permissions on alloc_dir being only readable by the Nomad agent's user (preferably root).

On my nomad clients this isn't the case:

nomad/alloc - root:wheel - 0755
nomad/alloc/<task> - root:wheel - 0755
nomad/alloc/<task>/alloc - nobody:nobody - 0777
nomad/alloc/<task>/<jobname> - nobody:nobody - 0777

As far as I can tell, alloc_dir ("alloc") is created implicitly as part of creating a task's allocation directory here:

nomad/client/allocdir/alloc_dir.go

Line 319 in 3423118

if err := os.MkdirAll(d.AllocDir, 0755); err != nil {

It looks like it got intentionally changed to be that way in 8e54601. If my assertion is correct, this would be a good example of why setting strict permissions on the file containing the secret itself is more robust than relying on directory permissions further up the tree for protecting the content of the file.

[grembo]

I altered the PR to include the suggested file parameter. Details are in the commit messages/the PR's description.

[grembo]

@schmichael Just to quickly sum up my lengthy statement from above:

• Tokens can be read by all system users, protection 1 doesn't hold (alloc_dir is world-readable since 8e54601).
• I added a file bool to the PR, which adds <task_dir>/private to store vault tokens agent-user readable. If file is set to true (the default), the token is also written to secrets/vault_token applying file_perms. This is so that all tokens are always written to the same authoritative place from which they are read back in on nomad restart instead of reading from secrets/vault_token, which could have been altered from within the chroot.

[grembo]

@lgfa29 Is there anything else I could do at this point to aid the discussion?

[grembo]

@lgfa29 @schmichael Would it help to split the PR?

[lgfa29]

Hi @grembo, sorry for the delay here. We have this marked for review but no updates yet. We will let you know once we get to that PR.

[grembo]

@schmichael @lgfa29 I updated the description above to clarify what the solution we use looks like/what we consider the most important part of addressing the issue. As discussed with @tgross, I would like to reduce the scope of this so it becomes actionable, focussing on solving these two core issues:

1. vault token shouldn't be accessible from inside the task/container at all.
2. vault token shouldn't be world-readable on the host system.

We could to reduce the patch so it only changes where the vault token is written to using a restrictive file mask (task's new private dir, which is not part of the container). This would be a fairly minimal change and reduce the size of #11905 dramatically.

It's then up to you which additional functionality you would like to see there (e.g., actually having a file parameter to control the behavior).

See also: #11905 (comment)

As the scope of this issue moved quite a lot, I would suggest that I open a new PR just focusing on the above.

What do you think, would this be a reasonable path forward?

[lgfa29]

Hi @grembo 👋

We're still discussing this internally. We had to step back for a while to get 1.3.0 out, but I've raise it again with the team. I will keep you posted of any updates.

[lgfa29]

Hi @grembo,

After discussing with the team we reached a few points:

• Allowing tasks to opt-out from receiving the Vault token at all is a good idea and worth continuing the work. Whenever possible (meaning on Linux and running as root at this time), the privatedirectory should be in a tmpfs.
• The new vault.file configuration should default to true to maintain backwards compatibility.
• The file permissions should remain as they are currently unfortunately, more details below.

I think the work you currently have in the PR already addresses most of this, but the file permission changes would have to be reverted.

Another thing that would be great to have in the PR are more tests. It would be valuable to test some scenarios like making sure that the private directory is not accessible from within the task, and that the secrets/vault_token file is updated when a new token is derived. We would also need to document this new directory in this page.

Going deeper into the token Vault file permission discussion, the current implementation doesn't leave the Vault token readable to any user in the host. The file is set with permission 0666 (more on this later), but that doesn't mean any user can access it because the Nomad client data directory is set to 0700, so anything within (which includes the Vault token files) is not accessible to anyone other than the Nomad user (which for clients is usually root). If you did find a way to access the Vault token as a different user, please file a security report.

The reason why the Vault token file needs to be set to 0666 is that Nomad can't really tell what's the user running a task. For example, in Docker you can map a container user to a specific host user,(or even a random user), so to make sure the token file is accessible in all different scenarios, Nomad needs to set it to 0666.

Allowing users to change this value has a high chance of creating a lot of confusion, as it may not be clear what the user ID and group ID their task is using from the host perspective, and as mentioned before, changing the permission would not result in any real benefit.

This approach could also be used for other secrets, like Consul and Nomad tokens, so implementing it in a way that could support that as well will be great.

Lastly, we're investigating more identity-aware approaches to managing secrets within Nomad that will make things a lot better in the future 🙂

[grembo]

Hi @lgfa29,

Thank you for taking the time to discuss the issue and for writing a detailed response.

I'll only address/clarify a few topics, I'll ask more detailed questions later (if needed) when I find the time to update the PR.

• Allowing tasks to opt-out from receiving the Vault token at all is a good idea and worth continuing the work. Whenever possible (meaning on Linux and running as root at this time), the privatedirectory should be in a tmpfs.

Makes sense. I assume I can re-use what exists on for /secrets. I might also explore using tmpfs on FreeBSD at some point (but that would be a separate PR).

• The new vault.file configuration should default to true to maintain backwards compatibility.

Agreed.

• The file permissions should remain as they are currently unfortunately, more details below.

This only applies to the file written inside the container (to /secrets) (which can be opted out with file), and not the files in /private though, correct?

Another thing that would be great to have in the PR are more tests. It would be valuable to test some scenarios like making sure that the private directory is not accessible from within the task, and that the secrets/vault_token file is updated when a new token is derived. We would also need to document this new directory in this page.

Documentation I can tackle, I might need some help with tests (but I'll try first).

Going deeper into the token Vault file permission discussion, the current implementation doesn't leave the Vault token readable to any user in the host. The file is set with permission 0666 (more on this later), but that doesn't mean any user can access it because the Nomad client data directory is set to 0700, so anything within (which includes the Vault token files) is not accessible to anyone other than the Nomad user (which for clients is usually root). If you did find a way to access the Vault token as a different user, please file a security report.

I (mis)understood a previous comment in this thread in a way that this was about the allocation directory, data_dir makes a lot more sense. Making sure that data_dir has strict permissions is up to the admin/operator/author of the startup script/OS package though[0]. Given that this is crucial for securing the installation, it might be beneficial if nomad checked the permissions of data_dir on start (just like openssh checks the access permissions of your private key). Material for another PR, I assume.

The reason why the Vault token file needs to be set to 0666 is that Nomad can't really tell what's the user running a task. > For example, in Docker you can map a container user to a specific host user,(or even a random user), so to make sure the token file is accessible in all different scenarios, Nomad needs to set it to 0666.

I have some ideas about this, but as it stands, I'm happy if we can exclude this aspect from the PR and solely focus on vault.file.

Thanks
Michael

[0] which made me realize that I need to open a PR for the FreeBSD port, which creates data_dir using the default umask.

[lgfa29]

I assume I can re-use what exists on for /secrets.

Yup, I think that would work 👍

This only applies to the file written inside the container (to /secrets) (which can be opted out with file), and not the files in /private though, correct?

I think it may be safer (as in backwards compatible safe) to keep both files with the same permissions for now, just in case somewhere in the implementation (either for this PR or some future change) they get copied from /private to /secret. The file will still be protected at the host by the data_dir permission.

Documentation I can tackle, I might need some help with tests (but I'll try first).

Sure, feel free to reach out whenever you need help 👍

Given that this is crucial for securing the installation, it might be beneficial if nomad checked the permissions of data_dir on start (just like openssh checks the access permissions of your private key).

Good point. Would you mind opening a separate issue for this?

[grembo]

I assume I can re-use what exists on for /secrets.

Yup, I think that would work 👍

This only applies to the file written inside the container (to /secrets) (which can be opted out with file), and not the files in /private though, correct?

I think it may be safer (as in backwards compatible safe) to keep both files with the same permissions for now, just in case somewhere in the implementation (either for this PR or some future change) they get copied from /private to /secret. The file will still be protected at the host by the data_dir permission.

I think any copy operation from within go will require specifying explicit permissions anyway, so I don’t think it has to be world readable within /private - it will be in /secrets though (which is the part where compatibility comes will be relevant). Copy operation on the OS level apply umask by default, so they’re also non critical.

Documentation I can tackle, I might need some help with tests (but I'll try first).

Sure, feel free to reach out whenever you need help 👍

Thanks!

Given that this is crucial for securing the installation, it might be beneficial if nomad checked the permissions of data_dir on start (just like openssh checks the access permissions of your private key).

Good point. Would you mind opening a separate issue for this?

Will do

[lgfa29]

I think any copy operation from within go will require specifying explicit permissions anyway

Cool, so we're probably OK. But definitely something to keep in mind during implementation 🙂

[grembo]

@lgfa29 See #13343 for a first version of the change.

[grembo]

Please note that the pull request passed all tests besides the one that requires manual approval:

[lgfa29]

Closed by #13343