Merge pull request #414 from greatroar/chmod-bits

Support os.Mode{Setuid,Setgid,Sticky} in Chmod

client.go

@@ -563,8 +563,12 @@ func (c *Client) Chown(path string, uid, gid int) error {
 }
 
 // Chmod changes the permissions of the named file.
+//
+// Chmod does not apply a umask, because even retrieving the umask is not
+// possible in a portable way without causing a race condition. Callers
+// should mask off umask bits, if desired.
 func (c *Client) Chmod(path string, mode os.FileMode) error {
-	return c.setstat(path, sshFileXferAttrPermissions, uint32(mode))
+	return c.setstat(path, sshFileXferAttrPermissions, toChmodPerm(mode))
 }
 
 // Truncate sets the size of the named file. Although it may be safely assumed
@@ -1714,6 +1718,8 @@ func (f *File) Chown(uid, gid int) error {
 }
 
 // Chmod changes the permissions of the current file.
+//
+// See Client.Chmod for details.
 func (f *File) Chmod(mode os.FileMode) error {
 	return f.c.Chmod(f.path, mode)
 }
@@ -1825,3 +1831,25 @@ func flags(f int) uint32 {
 	}
 	return out
 }
+
+// toChmodPerm converts Go permission bits to POSIX permission bits.
+//
+// This differs from fromFileMode in that we preserve the POSIX versions of
+// setuid, setgid and sticky in m, because we've historically supported those
+// bits, and we mask off any non-permission bits.
+func toChmodPerm(m os.FileMode) (perm uint32) {
+	const mask = os.ModePerm | s_ISUID | s_ISGID | s_ISVTX
+	perm = uint32(m & mask)
+
+	if m&os.ModeSetuid != 0 {
+		perm |= s_ISUID
+	}
+	if m&os.ModeSetgid != 0 {
+		perm |= s_ISGID
+	}
+	if m&os.ModeSticky != 0 {
+		perm |= s_ISVTX
+	}
+
+	return perm
+}

client_integration_test.go

@@ -873,6 +873,59 @@ func TestClientChmodReadonly(t *testing.T) {
 	}
 }
 
+func TestClientSetuid(t *testing.T) {
+	skipIfWindows(t) // No UNIX permissions.
+	if *testServerImpl {
+		t.Skipf("skipping with -testserver")
+	}
+
+	sftp, cmd := testClient(t, READWRITE, NODELAY)
+	defer cmd.Wait()
+	defer sftp.Close()
+
+	f, err := ioutil.TempFile("", "sftptest-setuid")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.Remove(f.Name())
+	f.Close()
+
+	const allPerm = os.ModePerm | os.ModeSetuid | os.ModeSetgid | os.ModeSticky |
+		s_ISUID | s_ISGID | s_ISVTX
+
+	for _, c := range []struct {
+		goPerm    os.FileMode
+		posixPerm uint32
+	}{
+		{os.ModeSetuid, s_ISUID},
+		{os.ModeSetgid, s_ISGID},
+		{os.ModeSticky, s_ISVTX},
+		{os.ModeSetuid | os.ModeSticky, s_ISUID | s_ISVTX},
+	} {
+		goPerm := 0700 | c.goPerm
+		posixPerm := 0700 | c.posixPerm
+
+		err = sftp.Chmod(f.Name(), goPerm)
+		require.NoError(t, err)
+
+		info, err := sftp.Stat(f.Name())
+		require.NoError(t, err)
+		require.Equal(t, goPerm, info.Mode()&allPerm)
+
+		err = sftp.Chmod(f.Name(), 0700) // Reset funny bits.
+		require.NoError(t, err)
+
+		// For historical reasons, we also support literal POSIX mode bits in
+		// Chmod. Stat should still translate these to Go os.FileMode bits.
+		err = sftp.Chmod(f.Name(), os.FileMode(posixPerm))
+		require.NoError(t, err)
+
+		info, err = sftp.Stat(f.Name())
+		require.NoError(t, err)
+		require.Equal(t, goPerm, info.Mode()&allPerm)
+	}
+}
+
 func TestClientChown(t *testing.T) {
 	skipIfWindows(t) // No UNIX permissions.
 	sftp, cmd := testClient(t, READWRITE, NODELAY)

stat_plan9.go

@@ -94,3 +94,11 @@ func fromFileMode(mode os.FileMode) uint32 {
 
 	return ret
 }
+
+// Plan 9 doesn't have setuid, setgid or sticky, but a Plan 9 client should
+// be able to send these bits to a POSIX server.
+const (
+	s_ISUID = 04000
+	s_ISGID = 02000
+	S_ISVTX = 01000
+)

stat_posix.go

@@ -114,3 +114,9 @@ func fromFileMode(mode os.FileMode) uint32 {
 
 	return ret
 }
+
+const (
+	s_ISUID = syscall.S_ISUID
+	s_ISGID = syscall.S_ISGID
+	s_ISVTX = syscall.S_ISVTX
+)