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

Fix file mode bug #618

Merged
merged 2 commits into from
Mar 18, 2019
Merged

Conversation

dtaniwaki
Copy link
Contributor

@dtaniwaki dtaniwaki commented Mar 14, 2019

I had an issue that I cannot sudo on a Docker image built with kaniko. I got the following error.

sudo: /usr/bin/sudo must be owned by uid 0 and have the setuid bit set

After a long investigation, I found it happened when I built an image over kaniko cache and the setuid bit can be removed by chown. You can check this behavior by the following steps.

$ touch foo
$ ls -l foo
-rw-rw-r-- 1 dtaniwaki dtaniwaki 0 Mar 14 22:56 foo
$ chmod u+s foo
$ ls -l foo
-rwSrw-r-- 1 dtaniwaki dtaniwaki 0 Mar 14 22:56 foo
$ chown dtaniwaki foo
$ ls -l foo
-rw-rw-r-- 1 dtaniwaki dtaniwaki 0 Mar 14 22:56 foo

It seems the syscall behavior, but at least the fix of this PR works.

You can see this behavior in kaniko by a Dockerfile like below.

FROM ubuntu:16.04

RUN mkdir aaa && chmod +t aaa
RUN touch bbb && chmod u+s bbb
# Change the echo string below to make a layer over the cache above.
RUN echo 1
RUN ls -la

@dtaniwaki dtaniwaki force-pushed the fix-file-mode branch 2 times, most recently from b3996c7 to d613038 Compare March 14, 2019 14:29
@dtaniwaki dtaniwaki changed the title Fix file mode Fix file mode bug Mar 15, 2019
@dtaniwaki
Copy link
Contributor Author

I updated the code to fix file mode for sticky bit on directories too.

@dlorenc
Copy link
Collaborator

dlorenc commented Mar 15, 2019

Would you mind adding a Dockerfile that reproduces this to the integration/dockerfiles directory as well? That way we can check that it keeps working in a real-world example.

@dtaniwaki
Copy link
Contributor Author

@dlorenc I added the Dockerfile 👍

@dtaniwaki
Copy link
Contributor Author

Oops, the Dockerfile I added has one odd line... Let me remove it and update the PR.

@dtaniwaki
Copy link
Contributor Author

dtaniwaki commented Mar 18, 2019

I now realized chown doesn’t reset sticky bit while it resets setuid and setgid bits although I thought it did when I tested a few days ago, but it might’ve been my mistake. At any rate, I think we can keep this change as is because using the same calling order of chown and chmod for both directories and files is easier to read.

@dlorenc dlorenc merged commit 28bfb75 into GoogleContainerTools:master Mar 18, 2019
@dtaniwaki dtaniwaki deleted the fix-file-mode branch May 7, 2019 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants