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.

Sign up for GitHub

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

Jump to bottom

proposal for file permission handling in projected service account volume #1598

Merged
merged 1 commit into from
Apr 15, 2020
Merged

proposal for file permission handling in projected service account volume #1598

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 54 additions & 4 deletions keps/sig-storage/20180515-svcacct-token-volumes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ authors:
- "@smarterclayton"
- "@liggitt"
- "@mikedanese"
- "@zshihang"
owning-sig: sig-storage
participating-sigs:
- sig-auth
Expand All @@ -14,7 +15,7 @@ approvers:
- "@saad-ali"
editor: "@zshihang"
creation-date: 2018-05-15
last-updated: 2020-03-09
last-updated: 2020-03-27
status: implemented
see-also:
- "https://github.com/kubernetes/community/blob/e3a6b8172fec9506b15520549e52be683e30cbfb/contributors/design-proposals/storage/svcacct-token-volume-source.md"
Expand All @@ -29,6 +30,9 @@ see-also:
- [Motivation](#motivation)
- [Proposal](#proposal)
- [Token Volume Projection](#token-volume-projection)
- [File Permission](#file-permission)
- [Proposed heuristics](#proposed-heuristics)
- [Alternatives considered](#alternatives-considered)
- [Alternatives](#alternatives)
- [Graduation Criteria](#graduation-criteria)
<!-- /toc -->
Expand Down Expand Up @@ -65,7 +69,6 @@ With this feature, we hope to provide a backwards compatible replacement for
service account tokens that strengthens the security and improves the
scalability of the platform.


## Proposal

Kubernetes should implement a ServiceAccountToken volume projection that
Expand Down Expand Up @@ -142,7 +145,6 @@ sources:
fieldRef: metadata.namespace
```


This fixes one scalability issue with the current service account token
deployment model where secret GETs are a large portion of overall apiserver
traffic.
Expand All @@ -159,6 +161,55 @@ sources:
audience: ca.istio.io
```

### File Permission
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know how/if this interacts with SELinux?

@gnufied @jsafrane

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SELinux labels on files are orthogonal to UID/GIDs and are set by the container runtime when pods start.
#1621 tries to change it, still, it's separate from file owners and permissions.


The secret projections are currently written with world readable (0644,
effectively 444) file permissions. Given that file permissions are one of the
oldest and most hardened isolation mechanisms on unix, this is not ideal.
We would like to opportunistically restrict permissions for projected service
account tokens as long we can show that they won’t break users if we are to
migrate away from secrets to distribute service account credentials.

#### Proposed heuristics

+ *Case 1*: The pod has an fsGroup set. We can set the file permission on the
token file to 0600 and let the fsGroup mechanism work as designed. It will
zshihang marked this conversation as resolved.
Show resolved Hide resolved
set the permissions to 0640, chown the token file to the fsGroup and start
the containers with a supplemental group that grants them access to the
token file. This works today.
zshihang marked this conversation as resolved.
Show resolved Hide resolved
+ *Case 2*: The pod’s containers declare the same runAsUser for all containers
zshihang marked this conversation as resolved.
Show resolved Hide resolved
(ephemeral containers are excluded) in the pod. We chown the token file to
zshihang marked this conversation as resolved.
Show resolved Hide resolved
the pod’s runAsUser to grant the containers access to the token. All
containers must have UID either specified in container security context or
inherited from pod security context. Preferred UIDs in container images are
zshihang marked this conversation as resolved.
Show resolved Hide resolved
ignored.
Comment on lines +175 to +185
Copy link
Member

@liggitt liggitt Mar 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fsGroup and runAsUser fields can be modified by the PodSecurityPolicy admission plugin, and by mutating webhook admission plugins.

The service account admission plugin that sets up the token volume currently runs before PodSecurityPolicy. If we want it to be informed by fsGroup/runAsUser settings applied by PSP, it would need to be moved after the PodSecurityPolicy admission plugin. That would mean when PSP sees the pod, it would not yet have an auto-injected secret or token volume mount, which could change which pod security policy is selected/applied to the pod.

If a mutating webhook modifies the pod, then the ServiceAccount admission plugin Admit() method will be reinvoked. Because the webhook could have injected additional sidecar containers with different (or missing) runAsUser settings, that could affect the calculated token volume permissions.

edit: I missed that the implementation was planned on the kubelet side, not in the admission plugin. That's good, since it simplifies this greatly.

+ *Fallback*: We set the file permissions to world readable (0644) to match
the behavior of secrets.

This gives users that run as non-root greater isolation between users without
breaking existing applications. We also may consider adding more cases in the
future as long as we can ensure that they won’t break users.

#### Alternatives considered

+ We can create a volume for each UserID and set the owner to be that UserID
with mode 0400. If user doesn't specify runAsUser, fetching UserID in image
requires a re-design of kubelet regarding volume mounts and image pulling.
This has significant implementation complexity because:
+ We would have to reorder container creation to introspect images (that
might declare USER or GROUP directives) to pass this information to the
projected volume mounter.
+ Further, images are mutable so these directives may change over the
lifetime of the pod.
+ Volumes are shared between all pods that mount them today. Mapping a
single logical volume in a pod spec to distinct mount points is likely a
significant architectural change.
+ We pick a random group and set fsGroup on all pods in the service account
admission controller. It’s unclear how we would do this without conflicting
with usage of groups and potentially compromising security.
+ We set token files to be world readable always. Problems with this are
discussed above.

### Alternatives

1. Instead of implementing a service account token volume projection, we could
Expand All @@ -173,7 +224,6 @@ sources:
users needing to install this extension.
1. Complicates installation for the majority of users.


## Graduation Criteria

TBD