diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b47c1f2dfb..bdba568e0d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ BUG FIXES: * core: Fix search endpoint forwarding for multi-region clusters [[GH-3680](https://github.com/hashicorp/nomad/issues/3680)] * core: Fix an issue in which batch jobs with queued placements and lost allocations could result in improper placement counts [[GH-3717](https://github.com/hashicorp/nomad/issues/3717)] + * client: Migrated ephemeral_disk's maintain directory permissions [[GH-3723](https://github.com/hashicorp/nomad/issues/3723)] * config: Revert minimum CPU limit back to 20 from 100. * ui: Fix requests using client-side certificates in Firefox. [[GH-3728](https://github.com/hashicorp/nomad/pull/3728)] diff --git a/client/alloc_watcher.go b/client/alloc_watcher.go index e54e7184d13..ecac7190f48 100644 --- a/client/alloc_watcher.go +++ b/client/alloc_watcher.go @@ -499,7 +499,15 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser // If the header is for a directory we create the directory if hdr.Typeflag == tar.TypeDir { - os.MkdirAll(filepath.Join(dest, hdr.Name), os.FileMode(hdr.Mode)) + name := filepath.Join(dest, hdr.Name) + os.MkdirAll(name, os.FileMode(hdr.Mode)) + + // Can't change owner if not root or on Windows. + if euid == 0 { + if err := os.Chown(name, hdr.Uid, hdr.Gid); err != nil { + return fmt.Errorf("error chowning directory %v", err) + } + } continue } // If the header is for a symlink we create the symlink @@ -522,8 +530,7 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser return fmt.Errorf("error chmoding file %v", err) } - // Can't change owner if not root. Returns false on - // Windows as Chown always errors there. + // Can't change owner if not root or on Windows. if euid == 0 { if err := f.Chown(hdr.Uid, hdr.Gid); err != nil { f.Close() diff --git a/client/alloc_watcher_test.go b/client/alloc_watcher_test.go index 0ecb4686510..45972e7593f 100644 --- a/client/alloc_watcher_test.go +++ b/client/alloc_watcher_test.go @@ -10,11 +10,13 @@ import ( "os" "path/filepath" "strings" + "syscall" "testing" "time" "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/config" + "github.com/hashicorp/nomad/client/testutil" "github.com/hashicorp/nomad/nomad/mock" ) @@ -73,6 +75,7 @@ func TestPrevAlloc_LocalPrevAlloc(t *testing.T) { // TestPrevAlloc_StreamAllocDir_Ok asserts that streaming a tar to an alloc dir // works. func TestPrevAlloc_StreamAllocDir_Ok(t *testing.T) { + testutil.RequireRoot(t) t.Parallel() dir, err := ioutil.TempDir("", "") if err != nil { @@ -80,18 +83,29 @@ func TestPrevAlloc_StreamAllocDir_Ok(t *testing.T) { } defer os.RemoveAll(dir) - if err := os.Mkdir(filepath.Join(dir, "foo"), 0777); err != nil { + // Create foo/ + fooDir := filepath.Join(dir, "foo") + if err := os.Mkdir(fooDir, 0777); err != nil { t.Fatalf("err: %v", err) } - dirInfo, err := os.Stat(filepath.Join(dir, "foo")) + + // Change ownership of foo/ to test #3702 (any non-root user is fine) + const uid, gid = 1, 1 + if err := os.Chown(fooDir, uid, gid); err != nil { + t.Fatalf("err : %v", err) + } + + dirInfo, err := os.Stat(fooDir) if err != nil { t.Fatalf("err: %v", err) } - f, err := os.Create(filepath.Join(dir, "foo", "bar")) + + // Create foo/bar + f, err := os.Create(filepath.Join(fooDir, "bar")) if err != nil { t.Fatalf("err: %v", err) } - if _, err := f.WriteString("foo"); err != nil { + if _, err := f.WriteString("123"); err != nil { t.Fatalf("err: %v", err) } if err := f.Chmod(0644); err != nil { @@ -102,6 +116,8 @@ func TestPrevAlloc_StreamAllocDir_Ok(t *testing.T) { t.Fatalf("err: %v", err) } f.Close() + + // Create foo/baz -> bar symlink if err := os.Symlink("bar", filepath.Join(dir, "foo", "baz")); err != nil { t.Fatalf("err: %v", err) } @@ -181,6 +197,11 @@ func TestPrevAlloc_StreamAllocDir_Ok(t *testing.T) { if fi.Mode() != dirInfo.Mode() { t.Fatalf("mode: %v", fi.Mode()) } + stat := fi.Sys().(*syscall.Stat_t) + if stat.Uid != uid || stat.Gid != gid { + t.Fatalf("foo/ has incorrect ownership: expected %d:%d found %d:%d", + uid, gid, stat.Uid, stat.Gid) + } fi1, err := os.Stat(filepath.Join(dir1, "bar")) if err != nil {