sftpd: make file/dir removal and creation more standard

- remove a non empty directory. Before: the directory contents were
removed recursively. Now: removing a non empty directory fails.

- make a directory in a non existent path: Before: any necessary parents
were created. Now: it fails.

- remove a file. Before: files, directories and symlinks were removed.
Now: only files and symlink are removed, removing a directory using "Remove"
instead of "Rmdir" fails.

Upload a file in a non existent directory. Before: any necessary parents
were created. Now: it fails.

Now SFTPGo behaves as OpenSSH.
This commit is contained in:
Nicola Murino 2019-10-16 07:48:22 +02:00
parent f98a29a1e0
commit 8682ae4a54
2 changed files with 92 additions and 70 deletions

View file

@ -279,21 +279,23 @@ func (c Connection) handleSFTPRmdir(path string) error {
return sftp.ErrSSHFxPermissionDenied
}
numFiles, size, fileList, err := utils.ScanDirContents(path)
if err != nil {
c.Log(logger.LevelError, logSender, "failed to remove directory %#v, scanning error: %v", path, err)
var fi os.FileInfo
var err error
if fi, err = os.Lstat(path); err != nil {
c.Log(logger.LevelError, logSender, "failed to remove a dir %#v: stat error: %v", path, err)
return sftp.ErrSSHFxFailure
}
if err := os.RemoveAll(path); err != nil {
if !fi.IsDir() || fi.Mode()&os.ModeSymlink == os.ModeSymlink {
c.Log(logger.LevelDebug, logSender, "cannot remove %#v is not a directory", path)
return sftp.ErrSSHFxFailure
}
if err = os.Remove(path); err != nil {
c.Log(logger.LevelError, logSender, "failed to remove directory %#v: %v", path, err)
return sftp.ErrSSHFxFailure
}
logger.CommandLog(rmdirLogSender, path, "", c.User.Username, c.ID, c.protocol)
dataprovider.UpdateUserQuota(dataProvider, c.User, -numFiles, -size, false)
for _, p := range fileList {
executeAction(operationDelete, c.User.Username, p, "")
}
return sftp.ErrSSHFxOk
}
@ -314,11 +316,12 @@ func (c Connection) handleSFTPMkdir(path string) error {
if !c.User.HasPerm(dataprovider.PermCreateDirs) {
return sftp.ErrSSHFxPermissionDenied
}
if err := c.createMissingDirs(filepath.Join(path, "testfile")); err != nil {
c.Log(logger.LevelError, logSender, "error making missing dir for path %#v: %v", path, err)
if err := os.Mkdir(path, 0777); err != nil {
c.Log(logger.LevelError, logSender, "error creating missing dir: %#v error: %v", path, err)
return sftp.ErrSSHFxFailure
}
utils.SetPathPermissions(path, c.User.GetUID(), c.User.GetGID())
logger.CommandLog(mkdirLogSender, path, "", c.User.Username, c.ID, c.protocol)
return nil
}
@ -335,6 +338,10 @@ func (c Connection) handleSFTPRemove(path string) error {
c.Log(logger.LevelError, logSender, "failed to remove a file %#v: stat error: %v", path, err)
return sftp.ErrSSHFxFailure
}
if fi.IsDir() && fi.Mode()&os.ModeSymlink != os.ModeSymlink {
c.Log(logger.LevelDebug, logSender, "cannot remove %#v is not a file/symlink", path)
return sftp.ErrSSHFxFailure
}
size = fi.Size()
if err := os.Remove(path); err != nil {
c.Log(logger.LevelError, logSender, "failed to remove a file/symlink %#v: %v", path, err)
@ -356,18 +363,6 @@ func (c Connection) handleSFTPUploadToNewFile(requestPath, filePath string) (io.
return nil, sftp.ErrSSHFxFailure
}
if _, err := os.Stat(filepath.Dir(requestPath)); os.IsNotExist(err) {
if !c.User.HasPerm(dataprovider.PermCreateDirs) {
return nil, sftp.ErrSSHFxPermissionDenied
}
}
err := c.createMissingDirs(requestPath)
if err != nil {
c.Log(logger.LevelError, logSender, "error making missing dir for path %#v: %v", requestPath, err)
return nil, sftp.ErrSSHFxFailure
}
file, err := os.Create(filePath)
if err != nil {
c.Log(logger.LevelError, logSender, "error creating file %#v: %v", requestPath, err)
@ -569,23 +564,6 @@ func (c Connection) isSubDir(sub string) error {
return nil
}
func (c Connection) createMissingDirs(filePath string) error {
dirsToCreate, err := c.findNonexistentDirs(filePath)
if err != nil {
return err
}
last := len(dirsToCreate) - 1
for i := range dirsToCreate {
d := dirsToCreate[last-i]
if err := os.Mkdir(d, 0777); err != nil {
c.Log(logger.LevelError, logSender, "error creating missing dir: %#v", d)
return err
}
utils.SetPathPermissions(d, c.User.GetUID(), c.User.GetGID())
}
return nil
}
func (c Connection) close() error {
if c.channel != nil {
err := c.channel.Close()

View file

@ -201,8 +201,9 @@ func TestBasicSFTPHandling(t *testing.T) {
expectedQuotaSize := user.UsedQuotaSize + testFileSize
expectedQuotaFiles := user.UsedQuotaFiles + 1
err = createTestFile(testFilePath, testFileSize)
if err != nil {
t.Errorf("unable to create test file: %v", err)
err = sftpUploadFile(testFilePath, path.Join("/missing_dir", testFileName), testFileSize, client)
if err == nil {
t.Errorf("upload a file to a missing dir must fail")
}
err = sftpUploadFile(testFilePath, testFileName, testFileSize, client)
if err != nil {
@ -317,10 +318,7 @@ func TestDirCommands(t *testing.T) {
t.Errorf("unable to add user: %v", err)
}
// remove the home dir to test auto creation
_, err = os.Stat(user.HomeDir)
if err == nil {
os.RemoveAll(user.HomeDir)
}
os.RemoveAll(user.HomeDir)
client, err := getSftpClient(user, usePubKey)
if err != nil {
t.Errorf("unable to create sftp client: %v", err)
@ -339,13 +337,61 @@ func TestDirCommands(t *testing.T) {
t.Errorf("error rmdir: %v", err)
}
err = client.Mkdir("/test/test1")
if err == nil {
t.Errorf("recursive mkdir must fail")
}
client.Mkdir("/test")
err = client.Mkdir("/test/test1")
if err != nil {
t.Errorf("error mkdir all: %v", err)
t.Errorf("mkdir dir error: %v", err)
}
_, err = client.ReadDir("/this/dir/does/not/exist")
if err == nil {
t.Errorf("reading a missing dir must fail")
}
err = client.RemoveDirectory("/test/test1")
if err != nil {
t.Errorf("remove dir error: %v", err)
}
err = client.RemoveDirectory("/test")
if err != nil {
t.Errorf("remove dir error: %v", err)
}
_, err = client.Lstat("/test")
if err == nil {
t.Errorf("stat for deleted dir must not succeed")
}
err = client.RemoveDirectory("/test")
if err == nil {
t.Errorf("remove missing path must fail")
}
}
_, err = httpd.RemoveUser(user, http.StatusOK)
if err != nil {
t.Errorf("unable to remove user: %v", err)
}
os.RemoveAll(user.GetHomeDir())
}
func TestRemove(t *testing.T) {
usePubKey := true
user, _, err := httpd.AddUser(getTestUser(usePubKey), http.StatusOK)
if err != nil {
t.Errorf("unable to add user: %v", err)
}
client, err := getSftpClient(user, usePubKey)
if err != nil {
t.Errorf("unable to create sftp client: %v", err)
} else {
defer client.Close()
err = client.Mkdir("test")
if err != nil {
t.Errorf("error mkdir: %v", err)
}
err = client.Mkdir("/test/test1")
if err != nil {
t.Errorf("error mkdir subdir: %v", err)
}
testFileName := "/test_file.dat"
testFilePath := filepath.Join(homeBasePath, testFileName)
testFileSize := int64(65535)
@ -357,20 +403,29 @@ func TestDirCommands(t *testing.T) {
if err != nil {
t.Errorf("file upload error: %v", err)
}
// internally client.Remove will call RemoveDirectory on failure
// the first remove will fail since test directory is not empty
// the RemoveDirectory called internally by client.Remove will succeed
err = client.Remove("/test")
if err == nil {
t.Errorf("remove non empty dir must fail")
}
err = client.RemoveDirectory(filepath.Join("/test", testFileName))
if err == nil {
t.Errorf("remove directory as file must fail")
}
err = client.Remove(filepath.Join("/test", testFileName))
if err != nil {
t.Errorf("remove file error: %v", err)
}
err = client.Remove(filepath.Join("/test", testFileName))
if err == nil {
t.Errorf("remove missing file must fail")
}
err = client.Remove("/test/test1")
if err != nil {
t.Errorf("remove dir error: %v", err)
}
err = client.Remove("/test")
if err != nil {
t.Errorf("error rmdir all: %v", err)
}
_, err = client.Lstat("/test")
if err == nil {
t.Errorf("stat for deleted dir must not succeed")
}
err = client.Remove("/test")
if err == nil {
t.Errorf("remove missing path must fail")
t.Errorf("remove dir error: %v", err)
}
}
_, err = httpd.RemoveUser(user, http.StatusOK)
@ -1523,17 +1578,6 @@ func TestPermCreateDirs(t *testing.T) {
if err == nil {
t.Errorf("mkdir without permission should not succeed")
}
testFileName := "test_file.dat"
testFilePath := filepath.Join(homeBasePath, testFileName)
testFileSize := int64(65535)
err = createTestFile(testFilePath, testFileSize)
if err != nil {
t.Errorf("unable to create test file: %v", err)
}
err = sftpUploadFile(testFilePath, "/dir/subdir/test_file.dat", testFileSize, client)
if err == nil {
t.Errorf("mkdir without permission should not succeed")
}
}
_, err = httpd.RemoveUser(user, http.StatusOK)
if err != nil {