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.

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

Permissions problem on Docker 1.8 when reading from stdin #15785

Closed
riklaunim opened this issue Aug 24, 2015 · 49 comments · Fixed by #19099
Closed

Permissions problem on Docker 1.8 when reading from stdin #15785

riklaunim opened this issue Aug 24, 2015 · 49 comments · Fixed by #19099
Assignees
Labels
area/security/trust kind/bug Bugs are bugs. The cause may or may not be known at triage time so debugging may be needed.
Milestone

Comments

@riklaunim
Copy link

In my build scripts I have a line like so:

    tar c --mtime=2014-01-01 --exclude=__pycache__ --exclude=Dockerfile.template --mode="g=u,o=rX" --owner=root --group=root . | docker build --tag=$IMAGE_NAME:$BUILD_NUMBER -

It does work on Docker < 1.8 but on 1.8 it now throws:

unable to prepare context: unable to extract stdin to temporary context direcotry: lchown /tmp/docker-build-context-183505935/run_entrypoints.sh: operation not permitted

I can change it to use sudo:

        tar c --mtime=2014-01-01 --exclude=__pycache__ --exclude=*.pyc --exclude=Dockerfile.template --mode="g=u,o=rX" . | sudo docker build --tag=$IMAGE_NAME:$BUILD_NUMBER -

and then it works, but I wonder if it's a bug or some change that affects permissions/users.

@riklaunim
Copy link
Author

docker version: 1.8.1, build d12ea79
docker info:

 Containers: 31
Images: 1038
Storage Driver: aufs
 Root Dir: /home/piotrm/docker/aufs
 Backing Filesystem: extfs
 Dirs: 1100
 Dirperm1 Supported: true
Execution Driver: native-0.2
Logging Driver: json-file
Kernel Version: 3.19.0-21-generic
Operating System: Ubuntu 15.04
CPUs: 4
Total Memory: 15.67 GiB

uname -a:

 Linux 3.19.0-21-generic #21-Ubuntu SMP Sun Jun 14 18:31:11 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux

@cpuguy83
Copy link
Member

Yes there was a change in logic. The client is now extracting the tar to a tmp dir. Not sure why you don't have write perms on the temp dir... but in any case I'm not sure why we are extracting to disk here.
Ping @jlhawn who did this refactor.

@jlhawn
Copy link
Contributor

jlhawn commented Aug 24, 2015

docker build in the 1.8 client now always prepares context in a local directory so that it can parse and rewrite the Dockerfile with resolved image digest references in FROM commands (it was part of the Docker Content Trust work).

@riklaunim
Copy link
Author

What could be the cause of those permission problems?

@colemickens
Copy link

I'm hitting this as well, despite executing chmod -R ugo+rwx [directoryname] on the files before they're tar'd and piped into the docker build command.

This sort of breaks the "tar and output to standard out and take that as a new context" way of achieving nested builds. I'm not sure what else to do right now to workaround this.

@tomwys
Copy link

tomwys commented Aug 26, 2015

It may be important that we run docker client as normal user added to docker group. This problem probably occurs only in docker group + tar combo. Adding sudo before docker command works as workaround.

@cpuguy83
Copy link
Member

This is 100% because you don't have write access to /tmp.

@riklaunim
Copy link
Author

I have full access to local /tmp

@colemickens
Copy link

I also have full access to /tmp, and as described, already regranted permissions to user, group and others to the contents of the tar'd context.

@jlhawn
Copy link
Contributor

jlhawn commented Aug 26, 2015

@cpuguy83

This is 100% because you don't have write access to /tmp.

It's not that, it's that the user doesn't have permission to chown and/or chmod things to preserve the permissions and ownership of files from the archive.

@loa
Copy link
Contributor

loa commented Aug 27, 2015

Seem to have same issue but on OSX with docker-machine

docker run builder | docker build -t test -
unable to prepare context: unable to extract stdin to temporary context direcotry: lchown /var/folders/0s/qt2_tt396pj5lm1cqbf82_y00000gn/T/docker-build-context-220503351/hello: operation not permitted

edit:
Reverted local docker install to 1.7.1. This is a big issue though for our CI system that will have to stay on 1.7.1 until this is fixed.

@cpuguy83
Copy link
Member

Oh right, I see lchown in the error now.

@colemickens
Copy link

Any suggestions for workarounds besides downgrading docker?

@marcosnils
Copy link

@colemickens the description on the issue gives a workaround by adding sudo to the docker build command.

@rednil
Copy link

rednil commented Sep 1, 2015

could it be this aufs specific permission bug (long read!): #783 (comment)
solution would be to use the btrfs storage driver instead of aufs or (with kernel 3.19) the dirperm1 option to AUFS

@colemickens
Copy link

I'm using devicemapper and hitting this issue.

@GordonTheTurtle
Copy link

USER POLL

The best way to get notified when there are changes in this discussion is by clicking the Subscribe button in the top right.

The people listed below have appreciated your meaningfull discussion with a random +1:

@kern3l

@JanChec
Copy link

JanChec commented Sep 2, 2015

Awesome ;] My usecase is the same as Riklaunim's.

@kamkudla
Copy link

What's the latest version that doesn't have this issue?

@riklaunim
Copy link
Author

1.7.X

@thaJeztah
Copy link
Member

@jlhawn @cpuguy83 what should be done here? Is this a documentation issue (i.e., change the ownership of the tmp dir), should docker create its own tmp dir?

@riklaunim
Copy link
Author

Either there should be a documented backward incompatible change with a new solution or the backward compatibility retained. (sudo is not the perfect solution)

@tomwys
Copy link

tomwys commented Oct 3, 2015

This is bug, not documentation issue. docker client is user-space program and shouldn’t require root permissions to run. Feature of passing tar archive to docker build has been broken since 1.7, because it requires root access to be run in cases described above (no other feature of docker client requires root access).

/tmp/ directory is not the problem and creating custom /tmp/ dir will not solve this case. The problem is that docker client tries to extract archive preserving file ownership. This fails because non-root user can't change files ownership to arbitrary users.

The solution would be to:

  • do not extract archive at all (for example you can work with archive in memory or extract only part of files),
  • extract archive without file ownership preservation, and handle file ownership by other means.

@thaJeztah
Copy link
Member

/tmp/ directory is not the problem and creating custom /tmp/ dir will not solve this case. The problem is that docker client tries to extract archive preserving file ownership. This fails because non-root user can't change files ownership to arbitrary users.

Ah, you're right.

docker build in the 1.8 client now always prepares context in a local directory so that it can parse and rewrite the Dockerfile with resolved image digest references in FROM commands (it was part of the Docker Content Trust work).

@jlhawn so, IIUC, all it needs is the Dockerfile, and being able to send the patched Dockerfile to the builder. Would it be an option to only extract the Dockerfile, and send that separately from the build-context?

@jlhawn
Copy link
Contributor

jlhawn commented Oct 5, 2015

@thaJeztah

Would it be an option to only extract the Dockerfile, and send that separately from the build-context?

The Dockerfile has to be in the context, which means included in the tar archive which is uploaded. I think the best solution may be to special-case the situation where build context is read from stdin and not extract and then repackage it but instead modify the archive entry for the Dockerfile mid-flight to the daemon. It's kind of a hack that other maintainers are uncomfortable with but it would resolve the issue in this case. Unfortunately, a lot of work went into the build client to get this to work the way it does now and there's even more ongoing work to make builds completely client-driven (no context uploads at all).

@tiborvass, as you are currently working on builder-related things, what is your opinion on this issue and how to best resolve it?

@tomwys
Copy link

tomwys commented Oct 5, 2015

I don't know what tool/lib you use to work on archives, but can't you just replace single file rather that extracting and repacking whole archive?

For example standard GNU tar support such operations: https://www.gnu.org/software/tar/manual/html_section/tar_32.html

@colemickens
Copy link

So, I take it that this isn't being fixed for 1.9.0 then?

@thaJeztah
Copy link
Member

@colemickens no, there wasn't time to look at this properly before the 1.9 code freeze.

@phemmer
Copy link
Contributor

phemmer commented Oct 16, 2015

git bisect shows #14546 is the cause of this issue.
Specifically 3021b7a

@phemmer phemmer mentioned this issue Oct 16, 2015
2 tasks
@karlgrz
Copy link
Contributor

karlgrz commented Nov 18, 2015

+1, still present in 1.9. I can't upgrade until that's fixed.

@posita
Copy link

posita commented Nov 21, 2015

FYI, this definitely kills Circle (they use 1.8+). 😠 I think Codeship is affected as well?

UPDATE: FYI, Circle's version can be manually overridden using this technique if required.

UPDATE: Circle has broken the ability to manually override Docker versions twice in the past two months. Their work-around can no longer be considered viable.

@lidel
Copy link

lidel commented Dec 11, 2015

A temporary workaround for Docker 1.8 / 1.9:
https://github.com/jamiemccrindle/dockerception#18-permissions-error

@icecrime icecrime added the priority/P2 Normal priority: default priority applied. label Dec 29, 2015
@icecrime
Copy link
Contributor

Ping @docker/security-team!

@posita
Copy link

posita commented Dec 31, 2015

Having to do the kinds of gymnastics @lidel describes in his #15785 (comment) for a regression that has been identified since August does not seem reasonable, especially where the source of the problem appears to have been identified (see this #14546 (comment)). Will a fix still be included in the next release? This appears to be frustrating docker build - builds on several CI platforms.

@calavera
Copy link
Contributor

calavera commented Jan 5, 2016

Given that there is not a formal proposal to completely fix this issue, I think we should remove the context switch by default until we have a better solution.

I opened #19099 that fixes this issue for all builds with content trust disabled. Content Trust is not on by default yet, so we should be okay with having some issues there.

If #19099 is merged, all builds will work as expected as long as you don't have Content Trust enabled. This issue still remains if you have Content Trust enabled.

@phemmer
Copy link
Contributor

phemmer commented Jan 6, 2016

hehe, might want to reopen. Github tried to be smart.

This PR does not fix #15785 but it mitigates its impact...

@thaJeztah thaJeztah reopened this Jan 6, 2016
@thaJeztah
Copy link
Member

Hah, yes, I saw that one coming :-)

@thaJeztah
Copy link
Member

Hm, only might be confusing on the milestone, so perhaps we should remove the priority label now :/

@thaJeztah thaJeztah removed the priority/P2 Normal priority: default priority applied. label Jan 6, 2016
@tonistiigi
Copy link
Member

I don't think #19099 changes this much because the problem doesn't seem to be the dockerfile rewriting but initial extraction from stdin or http to a temp directory. Extracting from URL isn't exactly safe as well so we should try to use the tar directly and modify the it on-the-fly(this is already done for dockerfile rewriting). The difficult thing about this seems to be handling .dockerignore as includes/excludes are currently built in features in the package we use to make tarballs. Either we need to reimplement all this logic with tar parsing as well or I think better alternative would be just not to support these features if user specifies the tar. I think trust path is the only modification we should support in the user specified tar as this is the only one user can't control.

@tonistiigi tonistiigi self-assigned this Jan 12, 2016
tonistiigi added a commit to tonistiigi/docker that referenced this issue Jan 13, 2016
Fixes moby#15785

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
aditirajagopal pushed a commit to aditirajagopal/docker that referenced this issue Feb 8, 2016
Fixes moby#15785

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
lidel added a commit to lidel/meowkov that referenced this issue Mar 6, 2016
tianon added a commit to infosiftr/stackbrew that referenced this issue Jun 6, 2016
tianon added a commit to docker-library/bashbrew that referenced this issue Jul 13, 2019
tianon added a commit to docker-library/bashbrew that referenced this issue Apr 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security/trust kind/bug Bugs are bugs. The cause may or may not be known at triage time so debugging may be needed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.