-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 missing setuid flags on COPY --from=build operation #2089
Fix missing setuid flags on COPY --from=build operation #2089
Conversation
Fixes GoogleContainerTools#2075 When a file with the setuid bit is copied from one stage to another, the permissions were not copied over properly after setting ownership on directory and the file itself.
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). For more information, open the CLA check for this pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for including an integration test! 👍
Co-authored-by: Jason Hall <[email protected]>
Judging by some other PRs with similar failures I think y'all had some broken integration tests, would you mind re-running builds for this PR? Thanks! |
FROM base as final | ||
COPY --from=build ["/usr/local/bin/top1", "/usr/local/bin/"] | ||
LABEL \ | ||
description="Testing setuid behavior in Kaniko" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to only test that the existence of chowned/chmoded files doesn't break this build, but AFAIK it doesn't do any checks to ensure the resulting file in the output image is actually chowned/chmoded as expected. Can you add some RUN
commands here that check that the permissions are set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure can; added a run command to test for setuid on the binary.
Bails as-expected on a binary that doesn't have setuid unmodified (/usr/bin/top in example below)
INFO[0005] Unpacking rootfs as cmd COPY --from=build ["/usr/local/bin/top1", "/usr/local/bin/"] requires it.
INFO[0006] COPY --from=build ["/usr/local/bin/top1", "/usr/local/bin/"]
INFO[0006] Taking snapshot of files...
INFO[0006] RUN [ -u /usr/bin/top ]
INFO[0006] Taking snapshot of full filesystem...
INFO[0006] cmd: /bin/sh
INFO[0006] args: [-c [ -u /usr/bin/top ]]
INFO[0006] Running: [/bin/sh -c [ -u /usr/bin/top ]]
error building image: error building stage: failed to execute command: waiting for process to exit: exit status 1
With the copied binary from the build:
INFO[0006] Taking snapshot of full filesystem...
INFO[0006] cmd: /bin/sh
INFO[0006] args: [-c [ -u /usr/local/bin/top1 ]]
INFO[0006] Running: [/bin/sh -c [ -u /usr/local/bin/top1 ]]
INFO[0006] Taking snapshot of full filesystem...
INFO[0007] No files were changed, appending empty layer to config. No layer added to image.
INFO[0007] LABEL description="Testing setuid behavior in Kaniko"
INFO[0007] Applying label description=Testing setuid behavior in Kaniko
INFO[0007] Pushing image to docker.io/tonydelanuez/suid-test
INFO[0008] Pushed index.docker.io/tonydelanuez/suid-test@sha256:237464e4442d995338c21eef9b2362a864c71d24922280cebe7640160945f1d1
Highlights - Installed binaries are missing from image [#2049](#2049) - proc: detect kubernetes runtime by mounts [#2054](#2054) - Fixes #2046: make target stage lookup case insensitive [#2047](#2047) - fix: Refactor LayersMap to correct old strange code behavior [#2066](#2066) - Fix missing setuid flags on COPY --from=build operation [#2089](#2089) - Fixes #2046: make target stage lookup case insensitive [#2047](#2047) - Add GitLab CI credentials helper [#2040]((#2040)) - and a number of dependency bumps
Highlights - Installed binaries are missing from image #2049 - proc: detect kubernetes runtime by mounts #2054 - Fixes #2046: make target stage lookup case insensitive #2047 - Fix: Refactor LayersMap to correct old strange code behavior #2066 - Fix missing setuid flags on COPY --from=build operation #2089 - Fixes #2046: make target stage lookup case insensitive #2047 - Add GitLab CI credentials helper #2040 - And a number of dependency bumps
Highlights - Installed binaries are missing from image #2049 - proc: detect kubernetes runtime by mounts #2054 - Fixes #2046: make target stage lookup case insensitive #2047 - Fix: Refactor LayersMap to correct old strange code behavior #2066 - Fix missing setuid flags on COPY --from=build operation #2089 - Fixes #2046: make target stage lookup case insensitive #2047 - Add GitLab CI credentials helper #2040 - And a number of dependency bumps
Highlights - Installed binaries are missing from image #2049 - proc: detect kubernetes runtime by mounts #2054 - Fixes #2046: make target stage lookup case insensitive #2047 - Fix: Refactor LayersMap to correct old strange code behavior #2066 - Fix missing setuid flags on COPY --from=build operation #2089 - Fixes #2046: make target stage lookup case insensitive #2047 - Add GitLab CI credentials helper #2040 - And a number of dependency bumps
Fixes #2075
When a file with the setuid bit is copied from one stage to another the permissions were not copied over properly after
setting ownership on directory and the file itself. This PR ensures that the file permissions are synced after ownership is set on the file/directory which fixes the regression in SUID bit behavior.
Images available:
Reviewer Notes
Release Notes