Clarification about commit 586c38d

[Josh Seba]

I was wondering if someone wouldn't mind sharing some details about the commit "Apply a image's file capabilities when creating a process from the image" (`586c38d`). This caused a failure in one of our services when we pulled it in, which I think is correct based on what was happening. The service was running as a non-root user and trying to read a file owned by another user with 0600 permissions. Before that commit, if the process was the one invoked by the gVisor boot loader, it would be allowed to read that file; after the change it gets permission denied. 

I definitely think the behavior was incorrect before, an unprivileged process should never be able to read files it doesn't have permissions for, but I'm just curious about the details that led to the change, as the commit message is pretty terse and there's no PR or issue to look at for any information.

Thanks!

  -- Josh Seba

[Jing Chen]

Hi,

I am the author of the change. Some details are not visible since it is an internal issue,  I will try to cover as much as i can here. 

Initially, we found out that runsc grants processes certain abilities and permissions outside of the user that they run as, the capabilities permit for unprivileged users to have abilities to change file owners, bypass file system permissions, and switch users entirely. These privileges could pass onto file system mounts, so files outside of the container located within the mount may have owners and permissions modified as well, potentially causing a denial of service for other containers sharing the mounted files.

Some speculations around that time lead to mishandling of Linux capabilities.

Then we realized that docker made https://github.com/moby/moby/pull/45511/commits/3aef732e61ec8ae0ea0bd8ad31116194e0fc21a6 that relies solely on how runc implements init process creation and changes the definition of the Capabilities field in the OCI spec. Capabilities are now used for the process that starts the container init process, and not for the init process itself

In order to avoid giving processes more capabilities than intended, we implement this temporary solution, where runsc reads file capabilities of the target process binary and calculate its effective and permitted sets. I am also working on the file capabilities support in gVisor, that will be consistent with the behavior you are seeing once merged.

Let me know if you have any question.

Thanks

Jing