From e55164c42d9fb1c613c123c5cd3bc46c55f71ae9 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Fri, 16 Mar 2018 16:58:47 +0100 Subject: [PATCH 1/2] Fix creation of subpath with SUID/SGID directories. SafeMakeDir() should apply SUID/SGID/sticky bits to the directory it creates. --- pkg/util/mount/mount_linux.go | 14 +++++++- pkg/util/mount/mount_linux_test.go | 56 +++++++++++++++++++++++++++--- 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/pkg/util/mount/mount_linux.go b/pkg/util/mount/mount_linux.go index d4512307620..0758b0df8ca 100644 --- a/pkg/util/mount/mount_linux.go +++ b/pkg/util/mount/mount_linux.go @@ -1032,7 +1032,19 @@ func doSafeMakeDir(pathname string, base string, perm os.FileMode) error { // so user can read/write it. This is the behavior of previous code. // TODO: chmod all created directories, not just the last one. // parentFD is the last created directory. - if err = syscall.Fchmod(parentFD, uint32(perm)&uint32(os.ModePerm)); err != nil { + + // Translate perm (os.FileMode) to uint32 that fchmod() expects + kernelPerm := uint32(perm & os.ModePerm) + if perm&os.ModeSetgid > 0 { + kernelPerm |= syscall.S_ISGID + } + if perm&os.ModeSetuid > 0 { + kernelPerm |= syscall.S_ISUID + } + if perm&os.ModeSticky > 0 { + kernelPerm |= syscall.S_ISVTX + } + if err = syscall.Fchmod(parentFD, kernelPerm); err != nil { return fmt.Errorf("chmod %q failed: %s", currentPath, err) } return nil diff --git a/pkg/util/mount/mount_linux_test.go b/pkg/util/mount/mount_linux_test.go index 14e0c9af49e..cbbf0cacfb7 100644 --- a/pkg/util/mount/mount_linux_test.go +++ b/pkg/util/mount/mount_linux_test.go @@ -418,7 +418,7 @@ func TestPathWithinBase(t *testing.T) { } func TestSafeMakeDir(t *testing.T) { - defaultPerm := os.FileMode(0750) + defaultPerm := os.FileMode(0750) + os.ModeDir tests := []struct { name string // Function that prepares directory structure for the test under given @@ -426,6 +426,7 @@ func TestSafeMakeDir(t *testing.T) { prepare func(base string) error path string checkPath string + perm os.FileMode expectError bool }{ { @@ -435,6 +436,37 @@ func TestSafeMakeDir(t *testing.T) { }, "test/directory", "test/directory", + defaultPerm, + false, + }, + { + "directory-with-sgid", + func(base string) error { + return nil + }, + "test/directory", + "test/directory", + os.FileMode(0777) + os.ModeDir + os.ModeSetgid, + false, + }, + { + "directory-with-suid", + func(base string) error { + return nil + }, + "test/directory", + "test/directory", + os.FileMode(0777) + os.ModeDir + os.ModeSetuid, + false, + }, + { + "directory-with-sticky-bit", + func(base string) error { + return nil + }, + "test/directory", + "test/directory", + os.FileMode(0777) + os.ModeDir + os.ModeSticky, false, }, { @@ -444,6 +476,7 @@ func TestSafeMakeDir(t *testing.T) { }, "test/directory", "test/directory", + defaultPerm, false, }, { @@ -453,6 +486,7 @@ func TestSafeMakeDir(t *testing.T) { }, "", "", + defaultPerm, false, }, { @@ -462,6 +496,7 @@ func TestSafeMakeDir(t *testing.T) { }, "..", "", + defaultPerm, true, }, { @@ -471,6 +506,7 @@ func TestSafeMakeDir(t *testing.T) { }, "test/../../..", "", + defaultPerm, true, }, { @@ -483,6 +519,7 @@ func TestSafeMakeDir(t *testing.T) { }, "test/directory", "destination/directory", + defaultPerm, false, }, { @@ -492,6 +529,7 @@ func TestSafeMakeDir(t *testing.T) { }, "test/directory", "", + defaultPerm, true, }, { @@ -514,6 +552,7 @@ func TestSafeMakeDir(t *testing.T) { }, "test1/dir/dir/dir/dir/dir/dir/dir/foo", "test2/foo", + defaultPerm, false, }, { @@ -523,6 +562,7 @@ func TestSafeMakeDir(t *testing.T) { }, "test/directory", "", + defaultPerm, true, }, { @@ -532,6 +572,7 @@ func TestSafeMakeDir(t *testing.T) { }, "test/directory", "", + defaultPerm, true, }, { @@ -541,6 +582,7 @@ func TestSafeMakeDir(t *testing.T) { }, "test", "", + defaultPerm, true, }, { @@ -556,6 +598,7 @@ func TestSafeMakeDir(t *testing.T) { }, "dir/test", "", + defaultPerm, false, }, { @@ -568,6 +611,7 @@ func TestSafeMakeDir(t *testing.T) { }, "dir/test", "", + defaultPerm, true, }, { @@ -577,6 +621,7 @@ func TestSafeMakeDir(t *testing.T) { }, "test/directory", "", + defaultPerm, true, }, } @@ -589,7 +634,7 @@ func TestSafeMakeDir(t *testing.T) { } test.prepare(base) pathToCreate := filepath.Join(base, test.path) - err = doSafeMakeDir(pathToCreate, base, defaultPerm) + err = doSafeMakeDir(pathToCreate, base, test.perm) if err != nil && !test.expectError { t.Errorf("test %q: %s", test.name, err) } @@ -601,14 +646,17 @@ func TestSafeMakeDir(t *testing.T) { } if test.checkPath != "" { - if _, err := os.Stat(filepath.Join(base, test.checkPath)); err != nil { + st, err := os.Stat(filepath.Join(base, test.checkPath)) + if err != nil { t.Errorf("test %q: cannot read path %s", test.name, test.checkPath) } + if st.Mode() != test.perm { + t.Errorf("test %q: expected permissions %o, got %o", test.name, test.perm, st.Mode()) + } } os.RemoveAll(base) } - } func validateDirEmpty(dir string) error { From 0600f7ee22a29977863f602cc5d6913da41a3b7c Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Fri, 16 Mar 2018 15:14:42 -0400 Subject: [PATCH 2/2] Fix e2e tests for emptydir --- test/e2e/common/empty_dir.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/e2e/common/empty_dir.go b/test/e2e/common/empty_dir.go index bc6ad4b6cc2..613a88ef42e 100644 --- a/test/e2e/common/empty_dir.go +++ b/test/e2e/common/empty_dir.go @@ -41,7 +41,7 @@ var ( var _ = Describe("[sig-storage] EmptyDir volumes", func() { f := framework.NewDefaultFramework("emptydir") - Context("when FSGroup is specified [Feature:FSGroup]", func() { + Context("when FSGroup is specified", func() { It("new files should be created with FSGroup ownership when container is root", func() { doTestSetgidFSGroup(f, testImageRootUid, v1.StorageMediumMemory) }) @@ -252,6 +252,7 @@ func doTestSubPathFSGroup(f *framework.Framework, image string, medium v1.Storag fmt.Sprintf("--fs_type=%v", volumePath), fmt.Sprintf("--file_perm=%v", volumePath), fmt.Sprintf("--file_owner=%v", volumePath), + fmt.Sprintf("--file_mode=%v", volumePath), } pod.Spec.Containers[0].VolumeMounts[0].SubPath = subPath @@ -264,6 +265,7 @@ func doTestSubPathFSGroup(f *framework.Framework, image string, medium v1.Storag "perms of file \"/test-volume\": -rwxrwxrwx", "owner UID of \"/test-volume\": 0", "owner GID of \"/test-volume\": 123", + "mode of file \"/test-volume\": dgtrwxrwxrwx", } if medium == v1.StorageMediumMemory { out = append(out, "mount type of \"/test-volume\": tmpfs")