Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Projected serviceAccountToken has mode of mounted file hardcoded to 0600 #82573

Closed
alfredkrohmer opened this issue Sep 11, 2019 · 14 comments
Closed
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. sig/auth Categorizes an issue or PR as relevant to SIG Auth.

Comments

@alfredkrohmer
Copy link

What happened:

Mounting projected serviceAccountTokens into a container always sets the mode of the mounted token files to 0600. This is hardcoded in the source code:

What you expected to happen:

The file mode should equal the defaultMode given in the projected map.

How to reproduce it (as minimally and precisely as possible):

Create a pod with a projected serviceAccountToken with any defaultMode set. Observe that the file mode of the file is always 0600.

Environment:

  • Kubernetes version (use kubectl version): latest master
@alfredkrohmer alfredkrohmer added the kind/bug Categorizes issue or PR as related to a bug. label Sep 11, 2019
@k8s-ci-robot k8s-ci-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Sep 11, 2019
@alfredkrohmer
Copy link
Author

@kubernetes/sig-node-bugs

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Sep 11, 2019
@k8s-ci-robot
Copy link
Contributor

@devkid: Reiterating the mentions to trigger a notification:
@kubernetes/sig-node-bugs

In response to this:

@kubernetes/sig-node-bugs

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@liggitt
Copy link
Member

liggitt commented Sep 11, 2019

duplicate of #74565

/cc @mikedanese
/close

@k8s-ci-robot
Copy link
Contributor

@liggitt: Closing this issue.

In response to this:

duplicate of #74565

/cc @mikedanese
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@liggitt
Copy link
Member

liggitt commented Sep 11, 2019

actually, leaving this open to track resolution

/remove-sig node
/sig auth
/assign @mikedanese
/reopen

@k8s-ci-robot
Copy link
Contributor

@liggitt: Reopened this issue.

In response to this:

actually, leaving this open to track resolution

/remove-sig node
/sig auth
/assign @mikedanese
/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot reopened this Sep 11, 2019
@k8s-ci-robot k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed sig/node Categorizes an issue or PR as relevant to SIG Node. labels Sep 11, 2019
@mikedanese
Copy link
Member

This might be a docs issue, or the webhook could work around this by adding supplemental groups. I'm not sure if that's particularly desirable.

cc @micahhausler

@micahhausler
Copy link
Member

Yea, adding supplemental groups from the webhook is probably not the route we want to go, since it's hard to determine which GID is correct. We'll update our docs on that workaround, but having the mode be set by defaultMode is the desired behavior

@mikedanese
Copy link
Member

deafultMode defaults to world readable. The current behavior was intentional. I would really like if the world bit was never set on these tokens. This is something that I was hoping to fix with the second iteration of tokens. Coincidentally, the security audit pointed this issue out: #81116

@liggitt
Copy link
Member

liggitt commented Sep 18, 2019

Would you agree that requiring changes to pod specs in order to replace the secret-based token with the projected one by default doesn't seem tenable? If we can't require pod spec changes, and the pod spec doesn't contain enough info to confidently use gid / fsGroup / supplementalGroups, I don't see great options other than preserving the secret-based behavior.

What are the actual risks of injecting a token that is group or world-readable into a pod?

@mikedanese
Copy link
Member

I'd like to pursue the avenue of setting better file ownership of TokenVolumeProjections which would be preferable as it offers better security to users that are running as non-root transparently.

@willthames
Copy link

willthames commented Oct 24, 2019

One point to note here is that setting the fsGroup to the gid of the pod is a commonly documented workaround for this issue. However, this gid is typically 0 by default, which doesn't seem ideal - but if you then set the gid of the pod with RunAsGroup, this solution no longer works (fsGroup seems to be dropped if RunAsGroup is set to the same value (presumably as it's no longer a supplemental group) and the token is then only owner-readable again)

Edit: fsGroup is only available in the pod security context, and I was trying to set it in the container security context.

@tallclair tallclair added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Oct 30, 2019
@liggitt liggitt added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Nov 28, 2019
@mikedanese
Copy link
Member

Proposal to fix in kubernetes/enhancements#1598

@mikedanese
Copy link
Member

Fixed by @zshihang in #89193

modax added a commit to inventi/helm that referenced this issue Feb 2, 2021
rfranzke added a commit to rfranzke/gardener that referenced this issue Nov 30, 2021
krgostev pushed a commit to krgostev/gardener that referenced this issue Apr 21, 2022
krgostev pushed a commit to krgostev/gardener that referenced this issue Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. sig/auth Categorizes an issue or PR as relevant to SIG Auth.
Projects
None yet
Development

No branches or pull requests

7 participants