Dockerfile: ADD does not honor USER: files always owned by root #6119

Closed
discordianfish opened this Issue May 30, 2014 · 58 comments

Projects

None yet
@discordianfish
Contributor

Hi,

consider this Dockerfile:

FROM ubuntu
RUN adduser foo
USER foo
ADD . /foo

/foo in the container will be owned by root, not by 'foo' as one might expect. There are easy workaround but it's still a inconsistency which should be fixed at some point.

@LK4D4
Contributor
LK4D4 commented May 30, 2014

Cool, I'll do it on weekend :)

@crosbymichael
Member

FYI, I'm not sure we want this behavior, we will need to discuss first

@abevoelker

I'm relatively new to Docker and this had confused me at first as well. Every time I have to add files that should belong to a non-root user, I have to do RUN chown after the ADD (keeping it all above the USER line since chown needs root permission).

I had just assumed it was expected behavior and there was a good reason for it. The documentation for ADD does make it sound like expected behavior:

All new files and directories are created with mode 0755, uid and gid 0.

@LK4D4
Contributor
LK4D4 commented May 30, 2014

Also it was in original issue about chowning, that have been done by @creack. But I was too lazy to do it and @creack also didn't do it by some reasons.
#2684 - original issue

@unclejack
Contributor

I don't think making ADD set file ownership to whatever user the USER instruction has set above is a good idea. USER was initally added to specify the user which the commands executed by RUN or CMD should be run under.

@discordianfish
Contributor

@unclejack Why do you think this isn't a good idea? I think it's what the user expects: All following operations will be executed as the given user.

@divoxx
divoxx commented Jun 3, 2014

I just got bitten by this and gotta say it's a surprise that ADD does not respect the preceding USER declaration. If someone wants to add as root, simply run ADD before the USER is set to something else, as one would do w/ RUN.

+1 for this

@discordianfish
Contributor

Keep in mind we're recommend that people run their services as a non-privileged user. Inconveniences like this behavior will prevent people from following that.

@crosbymichael
Member

Are there any issues running a chown after you add things?

Ref: https://github.com/crosbymichael/influxdb-docker/blob/master/Dockerfile#L9

@tianon
Member
tianon commented Jun 20, 2014

Doesn't AUFS have issues with tracking ownership/mode changes?

@crosbymichael
Member

@tianon yes

@LK4D4
Contributor
LK4D4 commented Jun 20, 2014

@crosbymichael yeah, this is very inconvenient. for example I have postgres image, there is already USER postgres in it, so I need to do:

USER root
RUN chown
USER postgres

Also I must know about USER postgres in base image.

@pikeas
pikeas commented Aug 11, 2014

Proposal: a second form of the ADD command. I'd love to be able to do:

ADD ["filename", "owner", "group", "755"]
@cyphar
Contributor
cyphar commented Aug 20, 2014

I open a proposal a few days ago that addresses this issue: #7537. Essentially, after talking on IRC, it was decided that only COPY should have this particular behaviour (and ADD will be backward compatible until it is deprecated and removed).

@ericchaves

Hi guys, any update on this issue?

@david-mccullars

Being forced to ADD and then RUN chown has a very nasty side-effect of artificially bloating docker images. For example:

FROM busybox:latest

RUN adduser -D bob
ADD 30MB_file /30MB_file
RUN chown bob /30MB_file

I ran build on this once without the chown and once with:

βž” docker images | grep bob
test    with_chown          a27913861eca        6 days ago          65.35 MB
test    without_chown       7ab2bbe29b50        6 days ago          33.89 MB

And we can see what is happening via docker history:

 βž” docker history test:with_chown
IMAGE         CREATED        CREATED BY                                      SIZE
a27913861eca  6 days ago     /bin/sh -c chown bob /30MB_file                 31.46 MB
7ab2bbe29b50  6 days ago     /bin/sh -c #(nop) ADD file:9474e987e88bd93248   31.46 MB
e9413ccecaa1  6 days ago     /bin/sh -c adduser -D bob                       3.955 kB
4986bf8c1536  2 weeks ago    /bin/sh -c #(nop) CMD [/bin/sh]                 0 B
ea13149945cb  2 weeks ago    /bin/sh -c #(nop) ADD file:8cf517d90fe79547c4   2.433 MB
df7546f9f060  3 months ago   /bin/sh -c #(nop) MAINTAINER JΓ©rΓ΄me Petazzo     0 B
511136ea3c5a  19 months ago                                                  0 B

When we consider that a large application might have several hundred megabytes or more, this chown becomes very expensive.

@cyphar
Contributor
cyphar commented Jan 20, 2015

#9934 has been opened to fix this issue (my patches were rejected).

@rafabene rafabene added a commit to rafabene/docker_tutorial that referenced this issue Feb 6, 2015
@rafabene rafabene Update Dockerfile commands
I had permission issues with the centos image running on MacOS/Boot2Docker 1.4.1. It cause the javaee6angularjs.war to be copied using root ownership and 740 permission. That caused WildFly to throw a FileNotFoundException. 

The proposed changes fix that issue by placing the file under wildfly ownership. There's a docker issue opened about that issue: docker/docker#6119
04f2747
@rafabene rafabene referenced this issue in burrsutter/docker_tutorial Feb 6, 2015
Closed

Update Dockerfile commands #2

@jessfraz jessfraz added the feature label Feb 27, 2015
@goldmann
Contributor

This behavior really, really needs to be changed to make sure ADD uses current USER. Current way of implementing it with chown:

  1. Created 3 (sic!) unnecessary layers
  2. Makes Dockerfile ugly
  3. Is totally not intuitive.
@sandric
sandric commented Apr 4, 2015

@goldmann agree on all points!

@thaJeztah
Member

This topic was discussed by the maintainers and, while they agreed that having USER specify the owner of files added by ADD / COPY would have been a better choice, changing this behavior now would be a breaking change, which would be too much of a risk.

For this reason, #9934 was created to come with alternatives. An implementation of this is created in #10775, which is currently under review.

For those interested, please follow the review/discussion on #9934 to check progress.

@duglin
Contributor
duglin commented May 6, 2015

@kostickm is working on a proposal for this.

@kiwenlau

I think fix this problem will help to decrease images size. Specify users when ADD files is a great idea!

For example, if I ADD a binary like Hadoop, then chown it to hadoop user, the image size will increase 100+MB!

That's why I decided to install hadoop under root user:(

@rhuss rhuss referenced this issue in fabric8io/docker-maven-plugin May 22, 2015
Closed

Specify the user for assembly.xml #175

@gfyrag
gfyrag commented May 28, 2015

+1
I have an issue with permissions.
I build images for arm architecture (Raspberry) with buildroot (to generate my rootfs) and i can't use chown command since the command is compiled for arm.
ADD keywork honoring USER would be great.
Also like the proposal : ADD ["filename", "owner", "group", "755"]
Or something similar. In many case, being able to build image without the RUN command would be great for many cases, extending possibilities to multi architecture for Dockerfile.

@dnozay
Contributor
dnozay commented Jun 22, 2015

how about a ADDUSER and COPYUSER commands which would do an implicit chown?

@pothibo
pothibo commented Jun 28, 2015

Hi!

I just got bitten by this too. Is there still discussion about this? What are the pros/cons?

@thaJeztah
Member

There's currently a proposal for implementing this using command flags. The current status is best described in this comment; #13600 (comment)

@jessfraz
Contributor

Hello!
We are no longer accepting patches to the Dockerfile syntax as you can read about here: https://github.com/docker/docker/blob/master/ROADMAP.md#22-dockerfile-syntax

Mainly:

Allowing the Builder to be implemented as a separate utility consuming the Engine's API will open the door for many possibilities, such as offering alternate syntaxes or DSL for existing languages without cluttering the Engine's codebase

Then from there, patches/features like this can be re-thought. Hope you can understand.

@jessfraz jessfraz closed this Jul 10, 2015
@stefanleh

Am i doing something complete the wrong way?

RUN useradd -d /home/hybris -u 1000 -m -s /bin/bash hybris
VOLUME /home/hybris
ADD hybris /home/hybris
RUN chown -R hybris /home/hybris

Still all files belong to root.

@lukebarton

@jfrazelle This issue is resulting in a doubling of the size cost of every chown'd file in an image.

Given myfile of 1gb size:

...
ADD myfile /tmp
RUN chown bob:group /tmp/myfile

Result:
ADD: 1 layer @ 1gb
RUN: 1 layer @ 1gb

Total: 2gb

I beg you to reconsider just closing this issue off as 'no longer accepting patches'. It's begun causing a real headache as our project has grown.

@thaJeztah
Member

@stefanleh @lukebarton yes, it's a PITA, but please, also read the linked issues/PR for some background.

There's active work in progress to move the builder out from the daemon and once that's arrived (it's looks to be on target for 1.10), discussions around changes in the Dockerfile syntax/behavior, or alternatives can be reopened.

@easternbloc easternbloc added a commit to UKHomeOffice/brp_app that referenced this issue Nov 18, 2015
@easternbloc easternbloc Docker COPY copies everything as root.
Lols and fun can be found here: docker/docker#6119

For now we're just going to run transpile as root.
cc3a9e5
@typekpb
typekpb commented Feb 28, 2016

@lukebarton the growing image size is a real pain here
@thaJeztah any updates here (as 1.10 is out)?

@phuihock

@typekpb Same here, and I was expecting 1.10 to have a fix for this issue.

I currently work around the problem with RUN in conjunction with SimpleHTTPServer or git daemon, like this:
On the host: python -m SimpleHTTPServer
Inside Dockerfile: RUN wget http://${host:-172.17.0.1}/somefile.tar.gz && tar xzf somefile.tar.gz
or
On the host: git daemon --base-path=$(pwd) $(pwd)/repo
In Dockerfile: RUN git clone --single-branch myrepo

@cyphar
Contributor
cyphar commented Feb 28, 2016

@thaJeztah I'm happy to revive my 2-year-old PR that fixes this issue, as long as I get some guarantee that it won't be blocked for "backwards compatibility issues".

@thaJeztah
Member

@cyphar sorry, can't give you guarantees (you know I was +1 for it a the time)

@cyphar
Contributor
cyphar commented Feb 28, 2016

Heh, yeah. It's just that there's a very strong split personality regarding backwards compatibility issues. For example, my PR fixing this issue (which everyone agrees is better than the current setup) was rejected for not being "backwards compatible" -- an argument I do not understand. On the other hand, even with API versioning, clients aren't backwards compatible with old servers (something that has actually bit us at SUSE).

I wish we'd all agree on what does and doesn't make sense to complain about.

@duglin
Contributor
duglin commented Feb 29, 2016

There was, once, a suggestion to allow people to specify the user via a flag on the ADD/COPY command but that was rejected at all - despite it being backwards compatible (e.g. ADD --user=foo src tgt).

@dnozay
Contributor
dnozay commented Mar 2, 2016

about backwards compatibility, just update the major version and call it a day.

@skolodyazhnyy

Is there any solution? It seems this topic was discussed in many issues and lots of proposals were described, but I can't really figure out if anything better than ADD + RUN chown exists. I don't really care how, but would really love to remove this extra layer which created after ADD and completely replaced by layer after RUN chown

@cyphar
Contributor
cyphar commented Mar 9, 2016

/me will work on a PR for this this week. Hopefully it'll get merged this time.

@mespinosaz

That would be great, thanks!

@typekpb
typekpb commented Mar 9, 2016

Well, not sure if doable together, but I would love to see this solved for
COPY as well.
On Mar 9, 2016 09:51, "Marc" notifications@github.com wrote:

That would be great, thanks!

β€”
Reply to this email directly or view it on GitHub
#6119 (comment).

@cyphar
Contributor
cyphar commented Mar 9, 2016

@typekpb It's likely that it'll only be trivially solvable for COPY (ADD has several misfeatures that make this unreasonably hard -- namely the magical .tar unpacking).

EDIT: I spoke too soon, we can solve both at the same time (although decompressing archives might have some odd consequences).

@cyphar
Contributor
cyphar commented Mar 11, 2016

I've created a PR that implements this (again): #21088.

@devlinrcg

+1 to this guy. :)

@sruffell

:) agreed

@helmingstay

Shot down again. Boo.

@ripun
ripun commented May 19, 2016

Hi Team,
Kindly advise what is the status of this issue , because it does not make sense having this unexpected behavior of ADD

Current behavior:

ADD source_1gb.file destination_directory

RUN chown -R 775 destination_directory

Expected: 1gb
Actual:2gb

suggested solution:

ADD source_1gb.file destination_directory --USER=non_root

ADD source_1gb.file destination_directory --REQ_PERMISSION=755

just the way we have RUN command which follows below
USER non_root
RUN echo "hello i am runninga s non root"

or give us the option to run command "ADD" like "RUN" in 2 modes "root and non root "

i am planning to open new issue to have flexibility to run "ADD" command in 2 modes "root and non root "

Please fix the issue of ADD , its really expensive because its increasing size of image by adding extra layer

also we understand we can avoid this issue by maintaining proper permission of file or directory to be added , but that's extra overhead and not user friendly

Thanks
Ripunjay

@MaxNoe
MaxNoe commented Jun 4, 2016

+1

@brettvitaz

I too would like to see this issue resolved. The current behavior really does not make sense.

@paulp
paulp commented Aug 7, 2016

I thought I had to be misunderstanding something. Is there some story that's not being told here? There's no logic in having a USER command which continues to act as root!

@cfontes
cfontes commented Aug 30, 2016

Read it all and still can't figure out if any of those PRs were merged or accepted.

Any news?

Cheers

@etotheipi

I too am intensely curious whether any sane solution was implemented. I can't afford doubling the size of the stuff we copy in.

Out of curiosity, why was the idea of simply adding a COPYUSER and ADDUSER command rejected? Simply duplicates COPY and ADD but respects previous USER directives. No backwards compatibility issues, and should be fairly straightforward to implement and document. It doesn't have the same flexibility as adding --user and --group flags, but it would cover 98% of use cases and be much safer to implement.

@Sodki
Sodki commented Aug 30, 2016 edited

As an alternative I've been looking at Rocker, which implements this feature, and I think it fits the bill perfectly. Rocker is a tool that builds Docker containers, extending the spec of Dockerfiles. Using it in a CI environment should be absolutely straight forward.

@rhuss rhuss referenced this issue in fabric8io/docker-maven-plugin Sep 1, 2016
Open

Avoiding chown to reduce the image size #544

@soulrebel

@Sodki beat me to suggesting Rocker.

Probably many problem with permissions and image bloat stem from two antipatterns (IMHO):

  1. the same container is used to build and to run the code. Split the two step and you can keep things clean. That's where rocker comes handy.
  2. apps that write in the same directories where their code is. The unix way is that program files are owned by root and processes run as nobody or another less privileged user.
    Rocker can help here as well to preserve user ownership, but it probably is still bad for security and at least error prone (user ids must be made to coincide).
@borntorock

Hey, after reading all the comments. I'm not sure whether there any of the solution has been merged. Still facing the same issue in docker version 1.12.3.

Anyone able to help here? The only problem is of the big image size in executing the chown command.

@MaRuifeng
MaRuifeng commented Dec 22, 2016 edited

@kiwenlau @ripun @david-mccullars This is to comment on the additional layer size introduced by the chown command. I tested a more drastic situation on my Ubuntu system (in Oracle VirtualBox) where more than 1 chown instructions are placed in the Dockerfile, and indeed the image size gets larger and larger with additional chown instructions in the Dockerfile. But strangely such issue does not occur on our Jenkins server, which is a RHEL.

Eventually I found out that this is because the storage driver used by the RHEL server is devicemapper, whereas in my Ubuntu system it's aufs by default. I forcibly changed the storage driver in my Ubuntu system to devicemapper and the additional chown command does not introduce the new layer disk usage any more.

docker history image_size_chown

With devicemapper the output is
IMAGE CREATED CREATED_BY SIZE
a87cb09fe91f About an hour ago /bin/sh -c #(nop) CMD ["/bin/sh" "-c" "/bin/ 0 B
e1f41b263f7c About an hour ago /bin/sh -c chown -R martian /home/martian/ 0 B
f3169ab0f1f5 About an hour ago /bin/sh -c chown -R martian /home/martian 0 B
e68d44c49296 About an hour ago /bin/sh -c mv /home/martian/test_images/som 59.77 MB

With aufs the output is
IMAGE CREATED CREATED_BY SIZE
a87cb09fe91f About an hour ago /bin/sh -c #(nop) CMD ["/bin/sh" "-c" "/bin/ 0 B
e1f41b263f7c About an hour ago /bin/sh -c chown -R martian /home/martian/ 117.5 MB
f3169ab0f1f5 About an hour ago /bin/sh -c chown -R martian /home/martian 117.5 MB
e68d44c49296 About an hour ago /bin/sh -c mv /home/martian/test_images/som 59.77 MB

And the second image is exactly larger by 235 MB in size, introduced by the two chown commands.

Docker has documented on storage driver selection here.

The default storage driver selection can be checked via sudo docker info.
To configure docker daemon to use a different storage driver, add below line to /etc/default/docker and restart docker service. Note that images created under the original storage driver will NOT be available under the new one. They can be seen again when the storage driver option is restored.
DOCKER_OPTS="--storage-driver=devicemapper"

@robhaswell

I am still seeing this behaviour in Docker version 1.13.1, build 092cba3, and it was very surprising to me.

@thaJeztah
Member

@robhaswell correct, there has been a lot of discussion around this, but changing it would be a breaking change. see #30110 for the current follow-up discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment