Proposal: Make USER change the uid and gid of COPY'd files

[cyphar]

The current USER directive changes the user which every RUN directive executes under. However, file copying instructions such as ADD and COPY are always copied with the owner being root:root. This was originally added in 026aebd, in order to fix #2684 and #3950. However, this results in an inconsistency in the Dockerfile interface, since USER can modify some directives, but not all of them.

I propose that the COPY directive should change the owner of any copied file to be the uid of the current user (as set by the USER directive and found by parsing /etc/shadow in the container -- or failing that, just setting it to nobody [65534]) and the gid should be the primary gid of the current user (as found by parsing the /etc/shadow file in the container -- or failing that, just setting it to nobody [65534]).

NOTE: After discussing this proposal on the IRC, it was decided that ADD should not be modified along the lines of this proposal (since it must be supported and backward compatible until it is deprecated and removed). To this end, I am only proposing that COPY should be modified, due to it's relative newness and since there is nowhere near as many backward compatibility concerns as with ADD.

---

EDIT: I propose a modification to the above proposal, such that:

• If the b.Config.User is set to an empty string, then the same method as used in libcontainer is applied -- syscall.Getuid() and syscall.Getgid() are used instead.
• If b.Config.User does not exist in /etc/passwd, then the builder emits an error.

Does anyone have any objections with the above modification to the proposal?

---

To assist in the tracking of patchsets and pull requests which are required to implement this proposal:

• Refactor and expose private functions within libcontainer/user. docker-archive/libcontainer#158: libcontainer patchset to expose internal functions for parsing and filtering /etc/{passwd,group} files (allowing arbitrary io.Readers and thus container /etc/{passwd,group} files to be parsed).
• Update vendor'd libcontainer and update to use new user interface. #9023: docker patch to use the above libcontainer patch.
• Implement USER setting owner of COPY'd files. #9046: actually implement the above proposal (and document it).

---

/ping @shykes and others.

[tianon]

+1; this would make it significantly easier to use USER effectively

[thaJeztah]

+1 For making it more consistent and align better with user expectations. Sad that ADD won't be included, but for obvious reasons.

[shykes]

I am OK with this.

In general I would like everyone's opinion on how to deal with a situation like this, where we have to choose between 1) full reverse compatibility, and 2) a significant improvement to the user experience.

In this case we got "lucky" and can make an acceptable compromise thanks to the timing of the introduction of COPY... But that doesn't give us a general solution for the next case.

[thaJeztah]

cough by adding versioning to the Dockerfile format? cough 😺

[shykes]

@thaJeztah are you volunteering to prepare a proposal?

[thaJeztah]

@shykes see the link (#4907) but I wrote it quite a while ago and may need to update it a bit as I'm sure it contains some non-sense as well :)

Additionally, as it contains some strategic/policy stuff (enforce / don't enforce versioning?) Will require input from the Docker core members as well.

I won't be able to assist in implementing it though. Coding in Docker would be really out of my league at this moment (I do track the issues and try to 'follow' changes, purely out of interest). Having said that, I'm always open to help thinking :)

[cyphar]

The proposal has been modified to deal with some edge cases (non-existent users) so that it is more consistent with the way libcontainer handles conversions from runconfig.User to a uid and gid.

[tianon]

I'm still +1 - anything less would be inconsistent with how USER works in practice. 😄

[pikeas]

+1. I proposed an extension to ADD at #6119, and this solution is almost as good. However, I recommend that COPY follow RUN, and accept either a simple string or an array of arguments: file, user, group, and permissions mask.

[cyphar]

@pikeas Your proposal (at least, how I understand it) is not similar to the the RUN directive's JSON form. The RUN directive's JSON form is used to explicitly state the process that should be execve'd (not to provide optional arguments that affect how the RUN directive is interpreted). If we were to use your proposal, I would think that some form like COPY file {"user": "<user>", "group": "<group>", "mode": "<mode>"} would make more sense (since they are options which are optional and are distinct from the file being copied) but would also (unfortunately) be uglier.

But personally, I don't really like that particular idea because it now means that there is even more inconsistencies in the interface (you can choose what user you can COPY as inline, but not what user you RUN as -- and the like) as well as the fact that now you have a whole new optional argument that adds more magic to COPY (which is kind of the reason why COPY was added -- to remove the magic from ADD). Also there aren't (AFAIK) any directives which have arguments that will affect a directive in this manner (all directives are essentially treated equally, whereby the arguments to a directive do not actually affect how a directive functions).

But feel free to open a proposal and ask on the IRC, I might be incorrect in my assertions.

[sruffell]

Just adding my +1 as well.

Came upon this thread while searching for a way to change the owner (and in my case permissions) on a large collection of legacy files. My problem with changing the permission / mode in a separate run command is that I double the size of the image since it appeared one layer contained the initial files, then the next layer contained copies of all the files with the permissions changed.

Something like this would allow me to work around since the files are added with rw permission on the group of the user.

[kdomanski]

+1
The storage cost of running a separate chown is very impractical in my deployments.

[sarath]

👍
New to docker. ::excited:: But this one bummed me out. Lost a couple of hours trying to figure this out, seems reasonable to expect ownership after USER.

[masom]

👍

[cyphar]

@crosbymichael has closed the implementation PR, because apparently some functionality is "in the works" that would allow for (among other things) this functionality.

[sarath]

@cyphar is there a issue id we can refer to/track progress?

[cyphar]

@sarath I am not sure, it was discussed on #docker-maintainers quite recently. If worse comes to worse and you need this functionality, you can use my patches (#9046) to patch your own version of Docker. N.B This is universally considered as a bad idea, because it will require you to update my patches with upstream changes.

EDIT: I missed @thaJeztah's proposal (#9934).

[thaJeztah]

@cyphar Oops sorry for that, I should probably have put pinged you there. @unclejack as well because he was assigned to carry it. 😄