pkg/archive: Unpack() use 0755 permissions for missing directories#42016
Merged
cpuguy83 merged 1 commit intomoby:masterfrom Feb 17, 2021
Merged
pkg/archive: Unpack() use 0755 permissions for missing directories#42016cpuguy83 merged 1 commit intomoby:masterfrom
cpuguy83 merged 1 commit intomoby:masterfrom
Conversation
thaJeztah
added
status/2-code-review
impact/changelog
process/cherry-pick
kind/bugfix
Member
Author
|
@tonistiigi @cpuguy83 PTAL |
This was referenced Feb 12, 2021
thaJeztah
added
area/builder
Contributor
|
LGTM |
Member
|
Could we have a test for this? Preferably one that also passes pre-19.03.15 and 20.10.2 . |
thaJeztah
force-pushed
the
archive_permissions
branch
2 times, most recently
from
b20974d to
6474e38
Compare
Member
Author
|
@tonistiigi added a test;
|
Commit edb62a3 fixed a bug in MkdirAllAndChown() that caused the specified permissions to not be applied correctly. As a result of that bug, the configured umask would be applied. When extracting archives, Unpack() used 0777 permissions when creating missing parent directories for files that were extracted. Before edb62a3, this resulted in actual permissions of those directories to be 0755 on most configurations (using a default 022 umask). Creating these directories should not depend on the host's umask configuration. This patch changes the permissions to 0755 to match the previous behavior, and to reflect the original intent of using 0755 as default. Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
archive_permissions
branch
from
6474e38 to
25ada76
Compare
Member
Author
|
s390x failure is unrelated |
tonistiigi
approved these changes
Feb 17, 2021
|
This change is making /tmp directory be created without write permission for other users than root. So, it's making some images that runs as non-root user to fail due to EACCES on /tmp directory. Is that possible to configure the permission to be used instead of assuming 0755 ? or at least for /tmp directory? |
Member
Author
|
@gknuppe this is on images you created, or are you seeing this on images you pulled from (e.g.) Docker Hub? do you have an example image where this is happening in? I guess it depends on wether or not the rootfs has a directory defined for for i in alpine ubuntu debian; do docker run --rm -u 123:456 $i ls -la /tmp; done
total 8
drwxrwxrwt 2 root root 4096 Jan 30 2019 .
drwxr-xr-x 1 root root 4096 Jul 8 14:45 ..
total 8
drwxrwxrwt 2 root root 4096 Mar 7 2019 .
drwxr-xr-x 1 root root 4096 Jul 8 14:45 ..
total 8
drwxrwxrwt 2 root root 4096 Feb 28 2019 .
drwxr-xr-x 1 root root 4096 Jul 8 14:45 .. |
|
Thanks for the follow up.
So, I'm not sure if this is the change that caused that issue, but downloading such image with newer version of docker-ce and containerd.io, /tmp directory does not have sufficient permissions to allow running services as non-root user. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
together with #41984, this fixes #41978 If I create a compressed tar ball with etc/password in it
replaces/closes #41990
Commit edb62a3 / #35541 fixed a bug in
MkdirAllAndChown()that caused the specified permissions to not be applied correctly. As a result of that bug, the configured umask would be applied.When extracting archives,
Unpack()used0777permissions when creating missing parent directories for files that were extracted. Before edb62a3, this resulted in actual permissions of the directories to be0755on most configurations (using a default022umask).Creating these directories should not depend on the host's umask configuration. This patch changes the permissions to
0755to match the previous behavior, and to reflect the original intent of using0755as default.- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)