From da80e15e33c5a0765c046b6584047632163be4e1 Mon Sep 17 00:00:00 2001 From: Daisuke Taniwaki Date: Thu, 14 Mar 2019 23:29:25 +0900 Subject: [PATCH 1/2] Fix file mode --- pkg/util/fs_util.go | 47 ++++++++++++++++++++++------------------ pkg/util/fs_util_test.go | 24 ++++++++++++++++++++ 2 files changed, 50 insertions(+), 21 deletions(-) diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 38b25bc518..5ef48845fc 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -216,27 +216,16 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error { if err != nil { return err } - // manually set permissions on file, since the default umask (022) will interfere - if err = os.Chmod(path, mode); err != nil { - return err - } if _, err = io.Copy(currFile, tr); err != nil { return err } - if err = currFile.Chown(uid, gid); err != nil { + if err = setFilePermissions(path, mode, uid, gid); err != nil { return err } currFile.Close() case tar.TypeDir: logrus.Debugf("creating dir %s", path) - if err := os.MkdirAll(path, mode); err != nil { - return err - } - // In some cases, MkdirAll doesn't change the permissions, so run Chmod - if err := os.Chmod(path, mode); err != nil { - return err - } - if err := os.Chown(path, uid, gid); err != nil { + if err := mkdirAllWithPermissions(path, mode, uid, gid); err != nil { return err } @@ -429,10 +418,7 @@ func CreateFile(path string, reader io.Reader, perm os.FileMode, uid uint32, gid if _, err := io.Copy(dest, reader); err != nil { return err } - if err := dest.Chmod(perm); err != nil { - return err - } - return dest.Chown(int(uid), int(gid)) + return setFilePermissions(path, perm, int(uid), int(gid)) } // AddVolumePath adds the given path to the volume whitelist. @@ -492,13 +478,11 @@ func CopyDir(src, dest, buildcontext string) ([]string, error) { if fi.IsDir() { logrus.Debugf("Creating directory %s", destPath) + mode := fi.Mode() uid := int(fi.Sys().(*syscall.Stat_t).Uid) gid := int(fi.Sys().(*syscall.Stat_t).Gid) - if err := os.MkdirAll(destPath, fi.Mode()); err != nil { - return nil, err - } - if err := os.Chown(destPath, uid, gid); err != nil { + if err := mkdirAllWithPermissions(destPath, mode, uid, gid); err != nil { return nil, err } } else if fi.Mode()&os.ModeSymlink != 0 { @@ -614,3 +598,24 @@ func HasFilepathPrefix(path, prefix string, prefixMatchOnly bool) bool { func Volumes() []string { return volumes } + +func mkdirAllWithPermissions(path string, mode os.FileMode, uid, gid int) error { + if err := os.MkdirAll(path, mode); err != nil { + return err + } + if err := os.Chown(path, uid, gid); err != nil { + return err + } + // In some cases, MkdirAll doesn't change the permissions, so run Chmod + // Must chmod after chown because chown resets the file mode. + return os.Chmod(path, mode) +} + +func setFilePermissions(path string, mode os.FileMode, uid, gid int) error { + if err := os.Chown(path, uid, gid); err != nil { + return err + } + // manually set permissions on file, since the default umask (022) will interfere + // Must chmod after chown because chown resets the file mode. + return os.Chmod(path, mode) +} diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index 4f91343f8d..eff39d5d95 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -503,6 +503,30 @@ func TestExtractFile(t *testing.T) { filesAreHardlinks("/bin/uncompress", "/bin/gzip"), }, }, + { + name: "file with setuid bit", + contents: []byte("helloworld"), + hdrs: []*tar.Header{fileHeader("./bar", "helloworld", 04644)}, + checkers: []checker{ + fileExists("/bar"), + fileMatches("/bar", []byte("helloworld")), + permissionsMatch("/bar", 0644|os.ModeSetuid), + }, + }, + { + name: "dir with sticky bit", + contents: []byte("helloworld"), + hdrs: []*tar.Header{ + dirHeader("./foo", 01755), + fileHeader("./foo/bar", "helloworld", 0644), + }, + checkers: []checker{ + fileExists("/foo/bar"), + fileMatches("/foo/bar", []byte("helloworld")), + permissionsMatch("/foo/bar", 0644), + permissionsMatch("/foo", 0755|os.ModeDir|os.ModeSticky), + }, + }, } for _, tc := range tcs { From 063b8aa36e635f875968db12ebd4fe16abf14e90 Mon Sep 17 00:00:00 2001 From: Daisuke Taniwaki Date: Sat, 16 Mar 2019 01:03:06 +0900 Subject: [PATCH 2/2] Add Dockerfile for special file permission test --- integration/dockerfiles/Dockerfile_test_cache_perm | 8 ++++++++ integration/images.go | 1 + 2 files changed, 9 insertions(+) create mode 100644 integration/dockerfiles/Dockerfile_test_cache_perm diff --git a/integration/dockerfiles/Dockerfile_test_cache_perm b/integration/dockerfiles/Dockerfile_test_cache_perm new file mode 100644 index 0000000000..06c311fe6a --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_cache_perm @@ -0,0 +1,8 @@ +# Test to make sure the cache works with special file permissions properly. +# If the image is built twice, directory foo should have the sticky bit, +# and file bar should have the setuid and setgid bits. + +FROM busybox + +RUN mkdir foo && chmod +t foo +RUN touch bar && chmod u+s,g+s bar diff --git a/integration/images.go b/integration/images.go index 27d54501cb..2eb00be60a 100644 --- a/integration/images.go +++ b/integration/images.go @@ -134,6 +134,7 @@ func NewDockerFileBuilder(dockerfiles []string) *DockerFileBuilder { d.TestCacheDockerfiles = map[string]struct{}{ "Dockerfile_test_cache": {}, "Dockerfile_test_cache_install": {}, + "Dockerfile_test_cache_perm": {}, } return &d }