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.

Sign up for GitHub

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

Merged
merged 4 commits into from
May 22, 2022
Merged

Fix missing setuid flags on COPY --from=build operation #2089

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
13b56a6
Fix missing file permissions on multi-stage build
tonydelanuez May 15, 2022
2ab1d48
Update pkg/util/fs_util.go
tonydelanuez May 17, 2022
027ea96
Adding boilerplate to dockerfile
tonydelanuez May 20, 2022
c0c0ee6
Add bash check to bail with exit code 1 if setuid not present
tonydelanuez May 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions integration/dockerfiles-with-context/issue-2075/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Copyright 2022 Google, Inc. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

FROM docker.io/debian:bullseye-slim as base
FROM base as build
COPY ["top1", "/tmp/top1"]
RUN \
set -eu; \
cp /tmp/top1 /usr/local/bin/top1; \
chown root:root /usr/local/bin/top1; \
chmod u=rxs,go=rx /usr/local/bin/top1; \
ls -lh /usr/local/bin/top1
FROM base as final
COPY --from=build ["/usr/local/bin/top1", "/usr/local/bin/"]
LABEL \
description="Testing setuid behavior in Kaniko"
Copy link
Collaborator

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?

Copy link
Contributor Author

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

Binary file added integration/dockerfiles-with-context/issue-2075/top1
View file
Open in desktop
Binary file not shown.
15 changes: 10 additions & 5 deletions pkg/util/fs_util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,11 @@ func getSymlink(path string) error {
func CopyFileOrSymlink(src string, destDir string, root string) error {
destFile := filepath.Join(destDir, src)
src = filepath.Join(root, src)
if fi, _ := os.Lstat(src); IsSymlink(fi) {
fi, err := os.Lstat(src)
if err != nil {
return errors.Wrap(err, "getting file info")
}
if IsSymlink(fi) {
link, err := os.Readlink(src)
if err != nil {
return errors.Wrap(err, "copying file or symlink")
Expand All @@ -902,14 +906,15 @@ func CopyFileOrSymlink(src string, destDir string, root string) error {
}
return os.Symlink(link, destFile)
}
err := otiai10Cpy.Copy(src, destFile)
if err != nil {
if err := otiai10Cpy.Copy(src, destFile); err != nil {
return errors.Wrap(err, "copying file")
}
err = CopyOwnership(src, destDir, root)
if err != nil {
if err := CopyOwnership(src, destDir, root); err != nil {
return errors.Wrap(err, "copying ownership")
}
if err := os.Chmod(destFile, fi.Mode()); err != nil {
return errors.Wrap(err, "copying file mode")
}
return nil
}

Expand Down