proposal for file permission handling in projected service account volume

[zshihang]

Motivation:

currently the mode of projected service account volume is 0600 effectively 0400, which may not work in pods with non-root containers. see kubernetes/kubernetes#74565.

Proposal:

• 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
  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.
• Case 2: The pod’s containers declare the same runAsUser for all containers
  in the pod. We chown the token file to the pod’s runAsUser to grant the
  containers access to the token.
• Fallback: We set the file permissions to world readable (0644) to match the
  behavior of secrets.

Ref: kubernetes/kubernetes#70679
Ref: #542

[k8s-ci-robot]

Hi @zshihang. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[zshihang]

/cc @mikedanese
/cc @liggitt

[zshihang]

/assign @saad-ali

[liggitt]

are all images pulled and present by the time we need to create the volume for mounting and choose a file mode (including volumes we would need to create for initContainers)?

also, can the preferred uid/gid for an image change over the life of a pod if a container image is changed in the pod spec or is restarted/repulled?

[zshihang]

no, all volumes are mounted before SyncPod is called in kuberuntime manager. this proposal will not work without alternation on the existing abstraction of kuberuntime manager and volume manager.

good point. the uid/gid will not change if a new image with different uid/gid is pulled. this will be an issue for uid but not for gid because the user in the new image will be added to the gid. to work around this changing uid issue, we have to change the ownership each pulling, which requires implementing a SetOwnership method without remounting. talked to @saad-ali about the implications of this method: 1. "the operation is recursive and can take many many minutes to complete for volumes with lots of files and directories." 2. some files might be used by containers while being updated with new permissions.

the complexity of solving both issues is high when introducing the uid in images. ideally in this case we have to push container runtime side to add a functionality to set ownership of volume mounts. see moby/moby#2259. then we can set ownership uid-wise.

[msau42]

I don't think the recursive SetOwnership is a concern for this particularly case. It's only problematic for very large volumes with lots of files. But changing permissions of files while they're in use is a concern.

[liggitt]

if we reuse the same token (to avoid a renewal loop per container), is the scale we'd hit at up to one volume per container problematic?

[mikedanese]

We have the data in cache in the kubelet so this is purely scalability of kubelet volume management. IMO the major downside of this approach is implementation complexity.

[zshihang]

yes, besides the implementation complexity of managing volumes for different uids, i'd also like the unix primitive idea of group.

[mikedanese]

@msau42 @gnufied any more comments on this?

[msau42]

I am fine limiting the scope to projected service account volumes for now, and revisiting other ephemeral volumes as a future enhancement.

[msau42]

Do we know how/if this interacts with SELinux?

@gnufied @jsafrane

[jsafrane]

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.

[mikedanese]

From sig-auth.

/lgtm

[msau42]

/lgtm
/approve
from sig-storage

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mikedanese, msau42, zshihang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• keps/sig-storage/OWNERS [msau42]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[hasheddan]

@zshihang @mikedanese has there been any documentation added to k/website for this behavior? I would be happy to add some if not :)

[zshihang]

feel free to do so.