recursive permissions check before renaming/copying directories

dataprovider/user.go

@@ -259,7 +259,7 @@ func (u *User) IsVirtualFolder(sftpPath string) bool {
 	return false
 }
 
-// HasVirtualFoldersInside return true if there are virtual folders inside the
+// HasVirtualFoldersInside returns true if there are virtual folders inside the
 // specified SFTP path. We assume that path are cleaned
 func (u *User) HasVirtualFoldersInside(sftpPath string) bool {
 	if sftpPath == "/" && len(u.VirtualFolders) > 0 {
@@ -275,6 +275,21 @@ func (u *User) HasVirtualFoldersInside(sftpPath string) bool {
 	return false
 }
 
+// HasPermissionsInside returns true if the specified sftpPath has no permissions itself and
+// no subdirs with defined permissions
+func (u *User) HasPermissionsInside(sftpPath string) bool {
+	for dir := range u.Permissions {
+		if dir == sftpPath {
+			return true
+		} else if len(dir) > len(sftpPath) {
+			if strings.HasPrefix(dir, sftpPath+"/") {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 // HasOverlappedMappedPaths returns true if this user has virtual folders with overlapped mapped paths
 func (u *User) HasOverlappedMappedPaths() bool {
 	if len(u.VirtualFolders) <= 1 {

docs/account.md

@@ -20,7 +20,7 @@ For each account, the following properties can be configured:
   - `upload` upload files is allowed
   - `overwrite` overwrite an existing file, while uploading, is allowed. `upload` permission is required to allow file overwrite
   - `delete` delete files or directories is allowed
-  - `rename` rename a file or a directory is allowed if this permission is granted on target path. You can enable rename in a more controlled way granting `delete` permission on source directory and `upload`/`create_dirs` permissions on target directory. Please be aware that no subdir permission is checked for the directories rename case
+  - `rename` rename a file or a directory is allowed if this permission is granted on source and target path. You can enable rename in a more controlled way granting `delete` permission on source directory and `upload`/`create_dirs`/`create_symlinks` permissions on target directory
   - `create_dirs` create directories is allowed
   - `create_symlinks` create symbolic links is allowed
   - `chmod` changing file or directory permissions is allowed. On Windows, only the 0200 bit (owner writable) of mode is used; it controls whether the file's read-only attribute is set or cleared. The other bits are currently unused. Use mode 0400 for a read-only file and 0600 for a readable+writable file.

docs/build-from-source.md

@@ -1,11 +1,13 @@
 # Build SFTPGo from source
 
-Install the package to your [\$GOPATH](https://github.com/golang/go/wiki/GOPATH "GOPATH") with the [go tool](https://golang.org/cmd/go/ "go command") from shell:
+You can install the package to your [\$GOPATH](https://github.com/golang/go/wiki/GOPATH "GOPATH") with the [go tool](https://golang.org/cmd/go/ "go command") from shell:
 
 ```bash
 go get -u github.com/drakkan/sftpgo
 ```
 
+Or you can download the sources and use `go build`.
+
 Make sure [Git](https://git-scm.com/downloads) is installed on your machine and in your system's `PATH`.
 
 The following build tags are available:

docs/full-configuration.md

@@ -32,7 +32,7 @@ The `serve` command supports the following flags:
 - `--log-max-size` int. Maximum size in megabytes of the log file before it gets rotated. Default 10 or the value of `SFTPGO_LOG_MAX_SIZE` environment variable. It is unused if `log-file-path` is empty.
 - `--log-verbose` boolean. Enable verbose logs. Default `true` or the value of `SFTPGO_LOG_VERBOSE` environment variable (1 or `true`, 0 or `false`).
 
-Log file can be rotated on demand sending a `SIGUSR1` signal on Unix based systems and using `sftpgo service rotatelogs` on Windows.
+Log file can be rotated on demand sending a `SIGUSR1` signal on Unix based systems and using the command `sftpgo service rotatelogs` on Windows.
 
 If you don't configure any private host key, the daemon will use `id_rsa` and `id_ecdsa` in the configuration directory. If these files don't exist, the daemon will attempt to autogenerate them (if the user that executes SFTPGo has write access to the `config-dir`). The server supports any private key format supported by [`crypto/ssh`](https://github.com/golang/crypto/blob/master/ssh/keys.go#L33).
 

docs/portable-mode.md

@@ -15,7 +15,7 @@ Usage:
 
 Flags:
   -C, --advertise-credentials            If the SFTP service is advertised via multicast DNS, this flag allows to put username/password inside the advertised TXT record
-  -S, --advertise-service                Advertise SFTP service using multicast DNS (default true)
+  -S, --advertise-service                Advertise SFTP service using multicast DNS
       --allowed-extensions stringArray   Allowed file extensions case insensitive. The format is /dir::ext1,ext2. For example: "/somedir::.jpg,.png"
       --denied-extensions stringArray    Denied file extensions case insensitive. The format is /dir::ext1,ext2. For example: "/somedir::.jpg,.png"
   -d, --directory string                 Path to the directory to serve. This can be an absolute path or a path relative to the current directory (default ".")

docs/ssh-commands.md

@@ -21,7 +21,7 @@ SFTPGo support the following built-in SSH commands:
 - `scp`, SFTPGo implements the SCP protocol so we can support it for cloud filesystems too and we can avoid the other system commands limitations. SCP between two remote hosts is supported using the `-3` scp option.
 - `md5sum`, `sha1sum`, `sha256sum`, `sha384sum`, `sha512sum`. Useful to check message digests for uploaded files.
 - `cd`, `pwd`. Some SFTP clients do not support the SFTP SSH_FXP_REALPATH packet type, so they use `cd` and `pwd` SSH commands to get the initial directory. Currently `cd` does nothing and `pwd` always returns the `/` path.
-- `sftpgo-copy`. This is a built-in copy implementation. It allows server side copy for files and directories. The first argument is the source file/directory and the second one is the destination file/directory, for example `sftpgo-copy <src> <dst>`. The command will fail if the destination directory exists. Copy for directories spanning virtual folders is not supported. Only local filesystem is supported: recursive copy for Cloud Storage filesystems requires a new request for every file in any case, so a server side copy is not possibile. Please be aware that only the `list` permission for the source path and the `upload` and `create_dirs` (for directories) permissions for the destination path are checked.
+- `sftpgo-copy`. This is a built-in copy implementation. It allows server side copy for files and directories. The first argument is the source file/directory and the second one is the destination file/directory, for example `sftpgo-copy <src> <dst>`. The command will fail if the destination exists. Copy for directories spanning virtual folders is not supported. Only local filesystem is supported: recursive copy for Cloud Storage filesystems requires a new request for every file in any case, so a real server side copy is not possibile.
 - `sftpgo-remove`. This is a built-in remove implementation. It allows to remove single files and to recursively remove directories. The first argument is the file/directory to remove, for example `sftpgo-remove <dst>`. Only local filesystem is supported: recursive remove for Cloud Storage filesystems requires a new request for every file in any case, so a server side remove is not possibile.
 
 The following SSH commands are enabled by default:

httpd/httpd_test.go

@@ -107,6 +107,7 @@ func TestMain(m *testing.M) {
 	providerConf.CredentialsPath = credentialsPath
 	providerDriverName = providerConf.Driver
 	os.RemoveAll(credentialsPath) //nolint:errcheck
+	logger.InfoToConsole("Starting HTTPD tests, provider: %v", providerConf.Driver)
 
 	err = dataprovider.Initialize(providerConf, configDir)
 	if err != nil {

service/service_portable.go

@@ -32,7 +32,10 @@ func (s *Service) StartPortableMode(sftpdPort int, enabledSSHCommands []string,
 	if len(s.PortableUser.Username) == 0 {
 		s.PortableUser.Username = "user"
 	}
-	printablePassword := "[redacted]"
+	printablePassword := ""
+	if len(s.PortableUser.Password) > 0 {
+		printablePassword = "[redacted]"
+	}
 	if len(s.PortableUser.PublicKeys) == 0 && len(s.PortableUser.Password) == 0 {
 		var b strings.Builder
 		for i := 0; i < 8; i++ {

sftpd/handler.go

@@ -5,6 +5,7 @@ import (
 	"net"
 	"os"
 	"path"
+	"strings"
 	"sync"
 	"time"
 
@@ -301,9 +302,6 @@ func (c Connection) handleSFTPSetstat(filePath string, request *sftp.Request) er
 }
 
 func (c Connection) handleSFTPRename(sourcePath, targetPath string, request *sftp.Request) error {
-	if !c.isRenamePermitted(sourcePath, request) {
-		return sftp.ErrSSHFxPermissionDenied
-	}
 	if c.User.IsMappedPath(sourcePath) {
 		c.Log(logger.LevelWarn, logSender, "renaming a directory mapped as virtual folder is not allowed: %#v", sourcePath)
 		return sftp.ErrSSHFxPermissionDenied
@@ -312,24 +310,22 @@ func (c Connection) handleSFTPRename(sourcePath, targetPath string, request *sft
 		c.Log(logger.LevelWarn, logSender, "renaming to a directory mapped as virtual folder is not allowed: %#v", targetPath)
 		return sftp.ErrSSHFxPermissionDenied
 	}
-	if c.User.HasVirtualFoldersInside(request.Filepath) {
-		if fi, err := c.fs.Stat(sourcePath); err == nil {
-			if fi.IsDir() {
-				c.Log(logger.LevelDebug, logSender, "renaming the folder %#v is not supported: it has virtual folders inside it",
-					request.Filepath)
-				return sftp.ErrSSHFxOpUnsupported
-			}
-		}
+	srcInfo, err := c.fs.Lstat(sourcePath)
+	if err != nil {
+		return vfs.GetSFTPError(c.fs, err)
+	}
+	if !c.isRenamePermitted(sourcePath, request.Filepath, request.Target, srcInfo) {
+		return sftp.ErrSSHFxPermissionDenied
 	}
 	initialSize := int64(-1)
-	if fi, err := c.fs.Lstat(targetPath); err == nil {
-		if fi.IsDir() {
+	if dstInfo, err := c.fs.Lstat(targetPath); err == nil {
+		if dstInfo.IsDir() {
 			c.Log(logger.LevelWarn, logSender, "attempted to rename %#v overwriting an existing directory %#v", sourcePath, targetPath)
 			return sftp.ErrSSHFxOpUnsupported
 		}
 		// we are overwriting an existing file/symlink
-		if fi.Mode().IsRegular() {
-			initialSize = fi.Size()
+		if dstInfo.Mode().IsRegular() {
+			initialSize = dstInfo.Size()
 		}
 		if !c.User.HasPerm(dataprovider.PermOverwrite, path.Dir(request.Target)) {
 			c.Log(logger.LevelDebug, logSender, "renaming is not allowed, source: %#v target: %#v. "+
@@ -337,6 +333,17 @@ func (c Connection) handleSFTPRename(sourcePath, targetPath string, request *sft
 			return sftp.ErrSSHFxPermissionDenied
 		}
 	}
+	if srcInfo.IsDir() {
+		if c.User.HasVirtualFoldersInside(request.Filepath) {
+			c.Log(logger.LevelDebug, logSender, "renaming the folder %#v is not supported: it has virtual folders inside it",
+				request.Filepath)
+			return sftp.ErrSSHFxOpUnsupported
+		}
+		if err = c.checkRecursiveRenameDirPermissions(sourcePath, targetPath); err != nil {
+			c.Log(logger.LevelDebug, logSender, "error checking recursive permissions before renaming %#v: %+v", sourcePath, err)
+			return vfs.GetSFTPError(c.fs, err)
+		}
+	}
 	if !c.hasSpaceForRename(request, initialSize, sourcePath) {
 		c.Log(logger.LevelInfo, logSender, "denying cross rename due to space limit")
 		return sftp.ErrSSHFxFailure
@@ -798,34 +805,77 @@ func (c Connection) isCrossFoldersRequest(request *sftp.Request) bool {
 	return true
 }
 
-func (c Connection) isRenamePermitted(sourcePath string, request *sftp.Request) bool {
-	if c.fs.GetRelativePath(sourcePath) == "/" {
+func (c Connection) isRenamePermitted(fsSrcPath, sftpSrcPath, sftpDstPath string, fi os.FileInfo) bool {
+	if c.fs.GetRelativePath(fsSrcPath) == "/" {
 		c.Log(logger.LevelWarn, logSender, "renaming root dir is not allowed")
 		return false
 	}
-	if c.User.IsVirtualFolder(request.Filepath) || c.User.IsVirtualFolder(request.Target) {
+	if c.User.IsVirtualFolder(sftpSrcPath) || c.User.IsVirtualFolder(sftpDstPath) {
 		c.Log(logger.LevelWarn, logSender, "renaming a virtual folder is not allowed")
 		return false
 	}
-	if !c.User.IsFileAllowed(request.Filepath) || !c.User.IsFileAllowed(request.Target) {
-		if fi, err := c.fs.Lstat(sourcePath); err == nil && fi.Mode().IsRegular() {
-			c.Log(logger.LevelDebug, logSender, "renaming file is not allowed, source: %#v target: %#v", request.Filepath,
-				request.Target)
+	if !c.User.IsFileAllowed(sftpSrcPath) || !c.User.IsFileAllowed(sftpDstPath) {
+		if fi != nil && fi.Mode().IsRegular() {
+			c.Log(logger.LevelDebug, logSender, "renaming file is not allowed, source: %#v target: %#v", sftpSrcPath,
+				sftpDstPath)
 			return false
 		}
 	}
-	if !c.User.HasPerm(dataprovider.PermRename, path.Dir(request.Target)) &&
-		(!c.User.HasPerm(dataprovider.PermDelete, path.Dir(request.Filepath)) ||
-			!c.User.HasPerm(dataprovider.PermUpload, path.Dir(request.Target))) {
+	if c.User.HasPerm(dataprovider.PermRename, path.Dir(sftpSrcPath)) &&
+		c.User.HasPerm(dataprovider.PermRename, path.Dir(sftpDstPath)) {
+		return true
+	}
+	if !c.User.HasPerm(dataprovider.PermDelete, path.Dir(sftpSrcPath)) {
 		return false
 	}
-	if !c.User.HasPerm(dataprovider.PermRename, path.Dir(request.Target)) &&
-		!c.User.HasPerm(dataprovider.PermCreateDirs, path.Dir(request.Target)) {
-		if fi, err := c.fs.Lstat(sourcePath); err == nil && fi.IsDir() {
-			return false
+	if fi != nil {
+		if fi.IsDir() {
+			return c.User.HasPerm(dataprovider.PermCreateDirs, path.Dir(sftpDstPath))
+		} else if fi.Mode()&os.ModeSymlink == os.ModeSymlink {
+			return c.User.HasPerm(dataprovider.PermCreateSymlinks, path.Dir(sftpDstPath))
 		}
 	}
-	return true
+	return c.User.HasPerm(dataprovider.PermUpload, path.Dir(sftpDstPath))
+}
+
+func (c Connection) checkRecursiveRenameDirPermissions(sourcePath, targetPath string) error {
+	dstPerms := []string{
+		dataprovider.PermCreateDirs,
+		dataprovider.PermUpload,
+		dataprovider.PermCreateSymlinks,
+	}
+
+	err := c.fs.Walk(sourcePath, func(walkedPath string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		dstPath := strings.Replace(walkedPath, sourcePath, targetPath, 1)
+		sftpSrcPath := c.fs.GetRelativePath(walkedPath)
+		sftpDstPath := c.fs.GetRelativePath(dstPath)
+		// walk scans the directory tree in order, checking the parent dirctory permissions we are sure that all contents
+		// inside the parent path was checked. If the current dir has no subdirs with defined permissions inside it
+		// and it has all the possible permissions we can stop scanning
+		if !c.User.HasPermissionsInside(path.Dir(sftpSrcPath)) && !c.User.HasPermissionsInside(path.Dir(sftpDstPath)) {
+			if c.User.HasPerm(dataprovider.PermRename, path.Dir(sftpSrcPath)) &&
+				c.User.HasPerm(dataprovider.PermRename, path.Dir(sftpDstPath)) {
+				return errSkipPermissionsCheck
+			}
+			if c.User.HasPerm(dataprovider.PermDelete, path.Dir(sftpSrcPath)) &&
+				c.User.HasPerms(dstPerms, path.Dir(sftpDstPath)) {
+				return errSkipPermissionsCheck
+			}
+		}
+		if !c.isRenamePermitted(walkedPath, sftpSrcPath, sftpDstPath, info) {
+			c.Log(logger.LevelInfo, logSender, "rename %#v -> %#v is not allowed, sftp destination path: %#v",
+				walkedPath, dstPath, sftpDstPath)
+			return os.ErrPermission
+		}
+		return nil
+	})
+	if err == errSkipPermissionsCheck {
+		err = nil
+	}
+	return err
 }
 
 func (c Connection) updateQuotaMoveBetweenVFolders(sourceFolder, dstFolder vfs.VirtualFolder, initialSize, filesSize int64, numFiles int) {

sftpd/internal_test.go

@@ -470,6 +470,11 @@ func TestMockFsErrors(t *testing.T) {
 	err = c.handleSFTPRemove(testfile, request)
 	assert.EqualError(t, err, sftp.ErrSSHFxFailure.Error())
 
+	request = sftp.NewRequest("Rename", filepath.Base(testfile))
+	request.Target = filepath.Base(testfile) + "1"
+	err = c.handleSFTPRename(testfile, testfile+"1", request)
+	assert.EqualError(t, err, sftp.ErrSSHFxFailure.Error())
+
 	err = os.Remove(testfile)
 	assert.NoError(t, err)
 }
@@ -782,10 +787,10 @@ func TestSSHCommandErrors(t *testing.T) {
 	}
 	cmd.connection.User.Permissions = make(map[string][]string)
 	cmd.connection.User.Permissions["/"] = []string{dataprovider.PermDownload}
-	_, _, err = cmd.getCopyPaths()
-	if assert.Error(t, err) {
-		assert.EqualError(t, err, errPermissionDenied.Error())
-	}
+	src, dst, err := cmd.getCopyPaths()
+	assert.NoError(t, err)
+	assert.False(t, cmd.hasCopyPermissions(src, dst, nil))
+
 	cmd.connection.User.Permissions = make(map[string][]string)
 	cmd.connection.User.Permissions["/"] = []string{dataprovider.PermAny}
 	if runtime.GOOS != osWindows {
@@ -2043,6 +2048,9 @@ func TestRenamePermission(t *testing.T) {
 	permissions["/dir2"] = []string{dataprovider.PermUpload}
 	permissions["/dir3"] = []string{dataprovider.PermDelete}
 	permissions["/dir4"] = []string{dataprovider.PermListItems}
+	permissions["/dir5"] = []string{dataprovider.PermCreateDirs, dataprovider.PermUpload}
+	permissions["/dir6"] = []string{dataprovider.PermCreateDirs, dataprovider.PermUpload,
+		dataprovider.PermListItems, dataprovider.PermCreateSymlinks}
 
 	user := dataprovider.User{
 		Permissions: permissions,
@@ -2055,34 +2063,103 @@ func TestRenamePermission(t *testing.T) {
 		fs:   fs,
 	}
 	request := sftp.NewRequest("Rename", "/testfile")
+	request.Target = "/dir1/testfile"
+	// rename is granted on Source and Target
+	assert.True(t, conn.isRenamePermitted("", request.Filepath, request.Target, nil))
 	request.Target = "/dir4/testfile"
 	// rename is not granted on Target
-	assert.False(t, conn.isRenamePermitted("", request))
+	assert.False(t, conn.isRenamePermitted("", request.Filepath, request.Target, nil))
+	request = sftp.NewRequest("Rename", "/dir1/testfile")
+	request.Target = "/dir2/testfile" //nolint:goconst
+	// rename is granted on Source but not on Target
+	assert.False(t, conn.isRenamePermitted("", request.Filepath, request.Target, nil))
 	request = sftp.NewRequest("Rename", "/dir4/testfile")
 	request.Target = "/dir1/testfile"
-	// rename is granted on Target, this is enough
-	assert.True(t, conn.isRenamePermitted("", request))
+	// rename is granted on Target but not on Source
+	assert.False(t, conn.isRenamePermitted("", request.Filepath, request.Target, nil))
 	request = sftp.NewRequest("Rename", "/dir4/testfile")
 	request.Target = "/testfile"
-	// rename is granted on Target, this is enough
-	assert.True(t, conn.isRenamePermitted("", request))
+	// rename is granted on Target but not on Source
+	assert.False(t, conn.isRenamePermitted("", request.Filepath, request.Target, nil))
 	request = sftp.NewRequest("Rename", "/dir3/testfile")
 	request.Target = "/dir2/testfile"
 	// delete is granted on Source and Upload on Target, the target is a file this is enough
-	assert.True(t, conn.isRenamePermitted("", request))
+	assert.True(t, conn.isRenamePermitted("", request.Filepath, request.Target, nil))
 	request = sftp.NewRequest("Rename", "/dir2/testfile")
 	request.Target = "/dir3/testfile"
-	assert.False(t, conn.isRenamePermitted("", request))
+	assert.False(t, conn.isRenamePermitted("", request.Filepath, request.Target, nil))
 	tmpDir := filepath.Join(os.TempDir(), "dir")
+	tmpDirLink := filepath.Join(os.TempDir(), "link")
 	err = os.Mkdir(tmpDir, os.ModePerm)
 	assert.NoError(t, err)
+	err = os.Symlink(tmpDir, tmpDirLink)
+	assert.NoError(t, err)
 	request.Filepath = "/dir"
 	request.Target = "/dir2/dir"
 	// the source is a dir and the target has no createDirs perm
-	assert.False(t, conn.isRenamePermitted(tmpDir, request))
-	conn.User.Permissions["/dir2"] = []string{dataprovider.PermUpload, dataprovider.PermCreateDirs}
-	// the source is a dir and the target has createDirs perm
-	assert.True(t, conn.isRenamePermitted(tmpDir, request))
+	info, err := os.Lstat(tmpDir)
+	if assert.NoError(t, err) {
+		assert.False(t, conn.isRenamePermitted(tmpDir, request.Filepath, request.Target, info))
+		conn.User.Permissions["/dir2"] = []string{dataprovider.PermUpload, dataprovider.PermCreateDirs}
+		// the source is a dir and the target has createDirs perm
+		assert.True(t, conn.isRenamePermitted(tmpDir, request.Filepath, request.Target, info))
+
+		request = sftp.NewRequest("Rename", "/testfile")
+		request.Target = "/dir5/testfile"
+		// the source is a dir and the target has createDirs and upload perm
+		assert.True(t, conn.isRenamePermitted(tmpDir, request.Filepath, request.Target, info))
+	}
+	info, err = os.Lstat(tmpDirLink)
+	if assert.NoError(t, err) {
+		assert.True(t, info.Mode()&os.ModeSymlink == os.ModeSymlink)
+		// the source is a symlink and the target has createDirs and upload perm
+		assert.False(t, conn.isRenamePermitted(tmpDir, request.Filepath, request.Target, info))
+	}
 	err = os.RemoveAll(tmpDir)
 	assert.NoError(t, err)
+	err = os.Remove(tmpDirLink)
+	assert.NoError(t, err)
+	conn.User.VirtualFolders = append(conn.User.VirtualFolders, vfs.VirtualFolder{
+		BaseVirtualFolder: vfs.BaseVirtualFolder{
+			MappedPath: os.TempDir(),
+		},
+		VirtualPath: "/dir1",
+	})
+	request = sftp.NewRequest("Rename", "/dir1")
+	request.Target = "/dir2/testfile"
+	// renaming a virtual folder is not allowed
+	assert.False(t, conn.isRenamePermitted("", request.Filepath, request.Target, nil))
+	err = conn.checkRecursiveRenameDirPermissions("invalid", "invalid")
+	assert.Error(t, err)
+	dir3 := filepath.Join(conn.User.HomeDir, "dir3")
+	dir6 := filepath.Join(conn.User.HomeDir, "dir6")
+	err = os.MkdirAll(filepath.Join(dir3, "subdir"), os.ModePerm)
+	assert.NoError(t, err)
+	err = ioutil.WriteFile(filepath.Join(dir3, "subdir", "testfile"), []byte("test"), os.ModePerm)
+	assert.NoError(t, err)
+	err = conn.checkRecursiveRenameDirPermissions(dir3, dir6)
+	assert.NoError(t, err)
+}
+
+func TestRecursiveCopyErrors(t *testing.T) {
+	permissions := make(map[string][]string)
+	permissions["/"] = []string{dataprovider.PermAny}
+	user := dataprovider.User{
+		Permissions: permissions,
+		HomeDir:     os.TempDir(),
+	}
+	fs, err := user.GetFilesystem("123")
+	assert.NoError(t, err)
+	conn := Connection{
+		User: user,
+		fs:   fs,
+	}
+	sshCmd := sshCommand{
+		command:    "sftpgo-copy",
+		connection: conn,
+		args:       []string{"adir", "another"},
+	}
+	// try to copy a missing directory
+	err = sshCmd.checkRecursiveCopyPermissions("adir", "another", "/another")
+	assert.Error(t, err)
 }

sftpd/sftpd_test.go

@@ -142,6 +142,7 @@ func TestMain(m *testing.M) {
 		os.Exit(1)
 	}
 	providerConf := config.GetProviderConf()
+	logger.InfoToConsole("Starting SFTPD tests, provider: %v", providerConf.Driver)
 
 	err = dataprovider.Initialize(providerConf, configDir)
 	if err != nil {
@@ -2158,12 +2159,19 @@ func TestVirtualFolders(t *testing.T) {
 	u := getTestUser(usePubKey)
 	mappedPath := filepath.Join(os.TempDir(), "vdir")
 	vdirPath := "/vdir/subdir"
+	testDir := "/userDir"
+	testDir1 := "/userDir1"
 	u.VirtualFolders = append(u.VirtualFolders, vfs.VirtualFolder{
 		BaseVirtualFolder: vfs.BaseVirtualFolder{
 			MappedPath: mappedPath,
 		},
 		VirtualPath: vdirPath,
 	})
+	u.Permissions[testDir] = []string{dataprovider.PermCreateDirs}
+	u.Permissions[testDir1] = []string{dataprovider.PermCreateDirs, dataprovider.PermUpload, dataprovider.PermDelete}
+	u.Permissions[path.Join(testDir1, "subdir")] = []string{dataprovider.PermCreateSymlinks, dataprovider.PermUpload,
+		dataprovider.PermDelete}
+
 	err := os.MkdirAll(mappedPath, os.ModePerm)
 	assert.NoError(t, err)
 	user, _, err := httpd.AddUser(u, http.StatusOK)
@@ -2195,8 +2203,48 @@ func TestVirtualFolders(t *testing.T) {
 		assert.Error(t, err, "removing a directory with a virtual folder inside must fail")
 		err = client.Mkdir("vdir1")
 		assert.NoError(t, err)
+		// rename empty dir /vdir1, we have permission on /
 		err = client.Rename("vdir1", "vdir2")
 		assert.NoError(t, err)
+		err = sftpUploadFile(testFilePath, path.Join("vdir2", testFileName), testFileSize, client)
+		assert.NoError(t, err)
+		// we don't have upload permission on testDir, we can only create dirs
+		err = client.Rename("vdir2", testDir)
+		assert.Error(t, err)
+		// on testDir1 only symlink aren't allowed
+		err = client.Rename("vdir2", testDir1)
+		assert.NoError(t, err)
+		err = client.Rename(testDir1, "vdir2")
+		assert.NoError(t, err)
+		err = client.MkdirAll(path.Join("vdir2", "subdir"))
+		assert.NoError(t, err)
+		err = sftpUploadFile(testFilePath, path.Join("vdir2", "subdir", testFileName), testFileSize, client)
+		assert.NoError(t, err)
+		err = client.Rename("vdir2", testDir1)
+		assert.NoError(t, err)
+		err = client.Rename(testDir1, "vdir2")
+		assert.NoError(t, err)
+		err = client.MkdirAll(path.Join("vdir2", "subdir", "subdir"))
+		assert.NoError(t, err)
+		err = sftpUploadFile(testFilePath, path.Join("vdir2", "subdir", "subdir", testFileName), testFileSize, client)
+		assert.NoError(t, err)
+		// we cannot create dirs inside /userDir1/subdir
+		err = client.Rename("vdir2", testDir1)
+		assert.Error(t, err)
+		err = client.Rename("vdir2", "vdir3")
+		assert.NoError(t, err)
+		err = client.Remove(path.Join("vdir3", "subdir", "subdir", testFileName))
+		assert.NoError(t, err)
+		err = client.RemoveDirectory(path.Join("vdir3", "subdir", "subdir"))
+		assert.NoError(t, err)
+		err = client.Rename("vdir3", testDir1)
+		assert.NoError(t, err)
+		err = client.Rename(testDir1, "vdir2")
+		assert.NoError(t, err)
+		err = client.Symlink(path.Join("vdir2", "subdir", testFileName), path.Join("vdir2", "subdir", "alink"))
+		assert.NoError(t, err)
+		err = client.Rename("vdir2", testDir1)
+		assert.NoError(t, err)
 		err = os.Remove(testFilePath)
 		assert.NoError(t, err)
 		err = os.Remove(localDownloadPath)
@@ -5499,11 +5547,16 @@ func TestSSHCopy(t *testing.T) {
 		_, err = runSSHCommand(fmt.Sprintf("sftpgo-copy %v %v", path.Join(testDir, testFileName), testFileName+".denied"), user, usePubKey)
 		assert.Error(t, err)
 		if runtime.GOOS != osWindows {
-			err = os.Chmod(filepath.Join(mappedPath1, testDir1), 0001)
+			subPath := filepath.Join(mappedPath1, testDir1, "asubdir", "anothersub", "another")
+			err = os.MkdirAll(subPath, os.ModePerm)
+			assert.NoError(t, err)
+			err = os.Chmod(subPath, 0001)
 			assert.NoError(t, err)
-			_, err = runSSHCommand(fmt.Sprintf("sftpgo-copy %v %v", path.Join(vdirPath1), "newdir"), user, usePubKey)
+			// c.connection.fs.GetDirSize(fsSourcePath) will fail scanning subdirs
+			// checkRecursiveCopyPermissions will work since it will skip subdirs with no permissions
+			_, err = runSSHCommand(fmt.Sprintf("sftpgo-copy %v %v", vdirPath1, "newdir"), user, usePubKey)
 			assert.Error(t, err)
-			err = os.Chmod(filepath.Join(mappedPath1, testDir1), os.ModePerm)
+			err = os.Chmod(subPath, os.ModePerm)
 			assert.NoError(t, err)
 			err = os.Chmod(filepath.Join(user.GetHomeDir(), testDir1), 0555)
 			assert.NoError(t, err)
@@ -5528,18 +5581,6 @@ func TestSSHCopy(t *testing.T) {
 		err = os.Remove(testFilePath1)
 		assert.NoError(t, err)
 	}
-	// test copy dir with no create dirs perm
-	user.Permissions["/"] = []string{dataprovider.PermUpload, dataprovider.PermDownload, dataprovider.PermListItems}
-	_, _, err = httpd.UpdateUser(user, http.StatusOK)
-	assert.NoError(t, err)
-	client, err = getSftpClient(user, usePubKey)
-	if assert.NoError(t, err) {
-		defer client.Close()
-		_, err = runSSHCommand(fmt.Sprintf("sftpgo-copy %v %v", path.Join(vdirPath1, testDir1), testDir1+"copy1"),
-			user, usePubKey)
-		assert.Error(t, err)
-	}
-
 	_, err = httpd.RemoveUser(user, http.StatusOK)
 	assert.NoError(t, err)
 	_, err = httpd.RemoveFolder(vfs.BaseVirtualFolder{MappedPath: mappedPath1}, http.StatusOK)
@@ -5554,8 +5595,71 @@ func TestSSHCopy(t *testing.T) {
 	assert.NoError(t, err)
 }
 
-func TestSSHCopyQuotaLimits(t *testing.T) {
+func TestSSHCopyPermissions(t *testing.T) {
 	usePubKey := false
+	u := getTestUser(usePubKey)
+	u.Permissions["/dir1"] = []string{dataprovider.PermUpload, dataprovider.PermDownload, dataprovider.PermListItems}
+	u.Permissions["/dir2"] = []string{dataprovider.PermCreateDirs, dataprovider.PermUpload, dataprovider.PermDownload,
+		dataprovider.PermListItems}
+	u.Permissions["/dir3"] = []string{dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermDownload,
+		dataprovider.PermListItems}
+	user, _, err := httpd.AddUser(u, http.StatusOK)
+	assert.NoError(t, err)
+	client, err := getSftpClient(user, usePubKey)
+	if assert.NoError(t, err) {
+		defer client.Close()
+		testDir := "tDir"
+		testFileSize := int64(131072)
+		testFileName := "test_file.dat"
+		testFilePath := filepath.Join(homeBasePath, testFileName)
+		err = createTestFile(testFilePath, testFileSize)
+		assert.NoError(t, err)
+		err = client.Mkdir(testDir)
+		assert.NoError(t, err)
+		err = sftpUploadFile(testFilePath, path.Join("/", testDir, testFileName), testFileSize, client)
+		assert.NoError(t, err)
+		// test copy file with no permission
+		_, err = runSSHCommand(fmt.Sprintf("sftpgo-copy %v %v", path.Join("/", testDir, testFileName), path.Join("/dir3", testFileName)),
+			user, usePubKey)
+		assert.Error(t, err)
+		// test copy dir with no create dirs perm
+		_, err = runSSHCommand(fmt.Sprintf("sftpgo-copy %v %v", path.Join("/", testDir), "/dir1/"), user, usePubKey)
+		assert.Error(t, err)
+		// dir2 has the needed permissions
+		_, err = runSSHCommand(fmt.Sprintf("sftpgo-copy %v %v", path.Join("/", testDir), "/dir2/"), user, usePubKey)
+		assert.NoError(t, err)
+		info, err := client.Stat(path.Join("/dir2", testDir))
+		if assert.NoError(t, err) {
+			assert.True(t, info.IsDir())
+		}
+		info, err = client.Stat(path.Join("/dir2", testDir, testFileName))
+		if assert.NoError(t, err) {
+			assert.True(t, info.Mode().IsRegular())
+		}
+		// now create a symlink, dir2 has no create symlink permission
+		err = client.Symlink(path.Join("/", testDir, testFileName), path.Join("/", testDir, testFileName+".link"))
+		assert.NoError(t, err)
+		_, err = runSSHCommand(fmt.Sprintf("sftpgo-copy %v %v", path.Join("/", testDir), "/dir2/sub"), user, usePubKey)
+		assert.Error(t, err)
+		_, err = runSSHCommand(fmt.Sprintf("sftpgo-copy %v %v", path.Join("/", testDir), "/newdir"), user, usePubKey)
+		assert.NoError(t, err)
+		// now delete the file and copy inside /dir3
+		err = client.Remove(path.Join("/", testDir, testFileName))
+		assert.NoError(t, err)
+		_, err = runSSHCommand(fmt.Sprintf("sftpgo-copy %v %v", path.Join("/", testDir), "/dir3"), user, usePubKey)
+		assert.NoError(t, err)
+
+		err = os.Remove(testFilePath)
+		assert.NoError(t, err)
+	}
+	_, err = httpd.RemoveUser(user, http.StatusOK)
+	assert.NoError(t, err)
+	err = os.RemoveAll(user.GetHomeDir())
+	assert.NoError(t, err)
+}
+
+func TestSSHCopyQuotaLimits(t *testing.T) {
+	usePubKey := true
 	testFileSize := int64(131072)
 	testFileSize1 := int64(65536)
 	testFileSize2 := int64(32768)

sftpd/ssh_cmd.go

@@ -30,9 +30,10 @@ import (
 const scpCmdName = "scp"
 
 var (
-	errQuotaExceeded     = errors.New("denying write due to space limit")
-	errPermissionDenied  = errors.New("Permission denied. You don't have the permissions to execute this command")
-	errUnsupportedConfig = errors.New("command unsupported for this configuration")
+	errQuotaExceeded        = errors.New("denying write due to space limit")
+	errPermissionDenied     = errors.New("Permission denied. You don't have the permissions to execute this command")
+	errUnsupportedConfig    = errors.New("command unsupported for this configuration")
+	errSkipPermissionsCheck = errors.New("permission check skipped")
 )
 
 type sshCommand struct {
@@ -134,12 +135,12 @@ func (c *sshCommand) handeSFTPGoCopy() error {
 	if err != nil {
 		return c.sendErrorResponse(err)
 	}
+	if err := c.checkCopyPermissions(fsSourcePath, fsDestPath, sshSourcePath, sshDestPath, fi); err != nil {
+		return c.sendErrorResponse(err)
+	}
 	filesNum := 0
 	filesSize := int64(0)
 	if fi.IsDir() {
-		if !c.connection.User.HasPerm(dataprovider.PermCreateDirs, path.Dir(sshDestPath)) {
-			return c.sendErrorResponse(errPermissionDenied)
-		}
 		filesNum, filesSize, err = c.connection.fs.GetDirSize(fsSourcePath)
 		if err != nil {
 			return c.sendErrorResponse(err)
@@ -593,13 +594,69 @@ func (c *sshCommand) getCopyPaths() (string, string, error) {
 		err := errors.New("usage sftpgo-copy <source dir path> <destination dir path>")
 		return "", "", err
 	}
-	if !c.connection.User.HasPerm(dataprovider.PermListItems, path.Dir(sshSourcePath)) ||
-		!c.connection.User.HasPerm(dataprovider.PermUpload, path.Dir(sshDestPath)) {
-		return "", "", errPermissionDenied
-	}
 	return sshSourcePath, sshDestPath, nil
 }
 
+func (c *sshCommand) hasCopyPermissions(sshSourcePath, sshDestPath string, srcInfo os.FileInfo) bool {
+	if !c.connection.User.HasPerm(dataprovider.PermListItems, path.Dir(sshSourcePath)) {
+		return false
+	}
+	if srcInfo.IsDir() {
+		return c.connection.User.HasPerm(dataprovider.PermCreateDirs, path.Dir(sshDestPath))
+	} else if srcInfo.Mode()&os.ModeSymlink == os.ModeSymlink {
+		return c.connection.User.HasPerm(dataprovider.PermCreateSymlinks, path.Dir(sshDestPath))
+	}
+	return c.connection.User.HasPerm(dataprovider.PermUpload, path.Dir(sshDestPath))
+}
+
+// fsSourcePath must be a directory
+func (c *sshCommand) checkRecursiveCopyPermissions(fsSourcePath, fsDestPath, sshDestPath string) error {
+	if !c.connection.User.HasPerm(dataprovider.PermCreateDirs, path.Dir(sshDestPath)) {
+		return errPermissionDenied
+	}
+	dstPerms := []string{
+		dataprovider.PermCreateDirs,
+		dataprovider.PermCreateSymlinks,
+		dataprovider.PermUpload,
+	}
+
+	err := c.connection.fs.Walk(fsSourcePath, func(walkedPath string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		fsDstSubPath := strings.Replace(walkedPath, fsSourcePath, fsDestPath, 1)
+		sshSrcSubPath := c.connection.fs.GetRelativePath(walkedPath)
+		sshDstSubPath := c.connection.fs.GetRelativePath(fsDstSubPath)
+		// If the current dir has no subdirs with defined permissions inside it
+		// and it has all the possible permissions we can stop scanning
+		if !c.connection.User.HasPermissionsInside(path.Dir(sshSrcSubPath)) &&
+			!c.connection.User.HasPermissionsInside(path.Dir(sshDstSubPath)) {
+			if c.connection.User.HasPerm(dataprovider.PermListItems, path.Dir(sshSrcSubPath)) &&
+				c.connection.User.HasPerms(dstPerms, path.Dir(sshDstSubPath)) {
+				return errSkipPermissionsCheck
+			}
+		}
+		if !c.hasCopyPermissions(sshSrcSubPath, sshDstSubPath, info) {
+			return errPermissionDenied
+		}
+		return nil
+	})
+	if err == errSkipPermissionsCheck {
+		err = nil
+	}
+	return err
+}
+
+func (c *sshCommand) checkCopyPermissions(fsSourcePath, fsDestPath, sshSourcePath, sshDestPath string, info os.FileInfo) error {
+	if info.IsDir() {
+		return c.checkRecursiveCopyPermissions(fsSourcePath, fsDestPath, sshDestPath)
+	}
+	if !c.hasCopyPermissions(sshSourcePath, sshDestPath, info) {
+		return errPermissionDenied
+	}
+	return nil
+}
+
 func (c *sshCommand) getRemovePath() (string, error) {
 	sshDestPath := c.getDestPath()
 	if len(sshDestPath) == 0 || len(c.args) != 1 {

utils/utils.go

@@ -266,7 +266,7 @@ func GetDirsForSFTPPath(p string) []string {
 	return dirsForPath
 }
 
-// CleanSFTPPath returns a clean sftp path
+// CleanSFTPPath returns a cleaned SFTP path
 func CleanSFTPPath(p string) string {
 	sftpPath := filepath.ToSlash(p)
 	if !path.IsAbs(p) {

vfs/gcsfs.go

@@ -10,6 +10,7 @@ import (
 	"net/http"
 	"os"
 	"path"
+	"path/filepath"
 	"strings"
 	"time"
 
@@ -420,7 +421,7 @@ func (fs GCSFs) ScanRootDirContents() (int, int64, error) {
 // GetDirSize returns the number of files and the size for a folder
 // including any subfolders
 func (GCSFs) GetDirSize(dirname string) (int, int64, error) {
-	return 0, 0, errors.New("Not implemented")
+	return 0, 0, errUnsupported
 }
 
 // GetAtomicUploadPath returns the path to use for an atomic upload.
@@ -448,6 +449,12 @@ func (fs GCSFs) GetRelativePath(name string) string {
 	return rel
 }
 
+// Walk walks the file tree rooted at root, calling walkFn for each file or
+// directory in the tree, including root
+func (GCSFs) Walk(root string, walkFn filepath.WalkFunc) error {
+	return errUnsupported
+}
+
 // Join joins any number of path elements into a single path
 func (GCSFs) Join(elem ...string) string {
 	return strings.TrimPrefix(path.Join(elem...), "/")

vfs/osfs.go

@@ -221,6 +221,12 @@ func (fs OsFs) GetRelativePath(name string) string {
 	return path.Join(virtualPath, filepath.ToSlash(rel))
 }
 
+// Walk walks the file tree rooted at root, calling walkFn for each file or
+// directory in the tree, including root
+func (OsFs) Walk(root string, walkFn filepath.WalkFunc) error {
+	return filepath.Walk(root, walkFn)
+}
+
 // Join joins any number of path elements into a single path
 func (OsFs) Join(elem ...string) string {
 	return filepath.Join(elem...)

vfs/s3fs.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"os"
 	"path"
+	"path/filepath"
 	"strings"
 	"time"
 
@@ -441,7 +442,7 @@ func (fs S3Fs) ScanRootDirContents() (int, int64, error) {
 // GetDirSize returns the number of files and the size for a folder
 // including any subfolders
 func (S3Fs) GetDirSize(dirname string) (int, int64, error) {
-	return 0, 0, errors.New("Not implemented")
+	return 0, 0, errUnsupported
 }
 
 // GetAtomicUploadPath returns the path to use for an atomic upload.
@@ -469,6 +470,12 @@ func (fs S3Fs) GetRelativePath(name string) string {
 	return rel
 }
 
+// Walk walks the file tree rooted at root, calling walkFn for each file or
+// directory in the tree, including root
+func (S3Fs) Walk(root string, walkFn filepath.WalkFunc) error {
+	return errUnsupported
+}
+
 // Join joins any number of path elements into a single path
 func (S3Fs) Join(elem ...string) string {
 	return path.Join(elem...)

vfs/vfs.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"os"
 	"path"
+	"path/filepath"
 	"runtime"
 	"strings"
 	"time"
@@ -42,9 +43,12 @@ type Fs interface {
 	GetDirSize(dirname string) (int, int64, error)
 	GetAtomicUploadPath(name string) string
 	GetRelativePath(name string) string
+	Walk(root string, walkFn filepath.WalkFunc) error
 	Join(elem ...string) string
 }
 
+var errUnsupported = errors.New("Not supported")
+
 // QuotaCheckResult defines the result for a quota check
 type QuotaCheckResult struct {
 	HasSpace     bool