scp: return better error messages

Fixes drakkan#171
devopsmakers · Sep 18, 2020 · 209badf · 209badf
1 parent 242dde4
commit 209badf
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 18 deletions.
diff --git a/sftpd/scp.go b/sftpd/scp.go
@@ -124,8 +124,9 @@ func (c *scpCommand) handleCreateDir(dirPath string) error {
 	}
 	if !c.connection.User.HasPerm(dataprovider.PermCreateDirs, path.Dir(dirPath)) {
 		c.connection.Log(logger.LevelWarn, "error creating dir: %#v, permission denied", dirPath)
-		c.sendErrorMessage(common.ErrPermissionDenied)
-		return common.ErrPermissionDenied
+		err := c.connection.Fs.GetPermissionError()
+		c.sendErrorMessage(err)
+		return err
 	}
 
 	err = c.createDir(p)
@@ -239,13 +240,15 @@ func (c *scpCommand) handleUpload(uploadFilePath string, sizeToRead int64) error
 
 	if !c.connection.User.IsFileAllowed(uploadFilePath) {
 		c.connection.Log(logger.LevelWarn, "writing file %#v is not allowed", uploadFilePath)
-		c.sendErrorMessage(common.ErrPermissionDenied)
+		err := c.connection.Fs.GetPermissionError()
+		c.sendErrorMessage(err)
+		return err
 	}
 
 	p, err := c.connection.Fs.ResolvePath(uploadFilePath)
 	if err != nil {
 		c.connection.Log(logger.LevelWarn, "error uploading file: %#v, err: %v", uploadFilePath, err)
-		c.sendErrorMessage(c.connection.GetFsError(err))
+		c.sendErrorMessage(err)
 		return err
 	}
 	filePath := p
@@ -256,8 +259,9 @@ func (c *scpCommand) handleUpload(uploadFilePath string, sizeToRead int64) error
 	if (statErr == nil && stat.Mode()&os.ModeSymlink == os.ModeSymlink) || c.connection.Fs.IsNotExist(statErr) {
 		if !c.connection.User.HasPerm(dataprovider.PermUpload, path.Dir(uploadFilePath)) {
 			c.connection.Log(logger.LevelWarn, "cannot upload file: %#v, permission denied", uploadFilePath)
-			c.sendErrorMessage(common.ErrPermissionDenied)
-			return common.ErrPermissionDenied
+			err := c.connection.Fs.GetPermissionError()
+			c.sendErrorMessage(err)
+			return err
 		}
 		return c.handleUploadFile(p, filePath, sizeToRead, true, 0, uploadFilePath)
 	}
@@ -277,16 +281,17 @@ func (c *scpCommand) handleUpload(uploadFilePath string, sizeToRead int64) error
 
 	if !c.connection.User.HasPerm(dataprovider.PermOverwrite, uploadFilePath) {
 		c.connection.Log(logger.LevelWarn, "cannot overwrite file: %#v, permission denied", uploadFilePath)
-		c.sendErrorMessage(common.ErrPermissionDenied)
-		return common.ErrPermissionDenied
+		err := c.connection.Fs.GetPermissionError()
+		c.sendErrorMessage(err)
+		return err
 	}
 
 	if common.Config.IsAtomicUploadEnabled() && c.connection.Fs.IsAtomicUploadSupported() {
 		err = c.connection.Fs.Rename(p, filePath)
 		if err != nil {
 			c.connection.Log(logger.LevelError, "error renaming existing file for atomic upload, source: %#v, dest: %#v, err: %v",
 				p, filePath, err)
-			c.sendErrorMessage(c.connection.GetFsError(err))
+			c.sendErrorMessage(err)
 			return err
 		}
 	}
@@ -444,42 +449,46 @@ func (c *scpCommand) handleDownload(filePath string) error {
 	if err != nil {
 		err := fmt.Errorf("Invalid file path")
 		c.connection.Log(logger.LevelWarn, "error downloading file: %#v, invalid file path", filePath)
-		c.sendErrorMessage(c.connection.GetFsError(err))
+		c.sendErrorMessage(err)
 		return err
 	}
 
 	var stat os.FileInfo
 	if stat, err = c.connection.Fs.Stat(p); err != nil {
 		c.connection.Log(logger.LevelWarn, "error downloading file: %#v->%#v, err: %v", filePath, p, err)
-		c.sendErrorMessage(c.connection.GetFsError(err))
+		c.sendErrorMessage(err)
 		return err
 	}
 
 	if stat.IsDir() {
 		if !c.connection.User.HasPerm(dataprovider.PermDownload, filePath) {
 			c.connection.Log(logger.LevelWarn, "error downloading dir: %#v, permission denied", filePath)
-			c.sendErrorMessage(common.ErrPermissionDenied)
-			return common.ErrPermissionDenied
+			err := c.connection.Fs.GetPermissionError()
+			c.sendErrorMessage(err)
+			return err
 		}
 		err = c.handleRecursiveDownload(p, stat)
 		return err
 	}
 
 	if !c.connection.User.HasPerm(dataprovider.PermDownload, path.Dir(filePath)) {
 		c.connection.Log(logger.LevelWarn, "error downloading dir: %#v, permission denied", filePath)
-		c.sendErrorMessage(common.ErrPermissionDenied)
-		return common.ErrPermissionDenied
+		err := c.connection.Fs.GetPermissionError()
+		c.sendErrorMessage(err)
+		return err
 	}
 
 	if !c.connection.User.IsFileAllowed(filePath) {
 		c.connection.Log(logger.LevelWarn, "reading file %#v is not allowed", filePath)
-		c.sendErrorMessage(common.ErrPermissionDenied)
+		err := c.connection.Fs.GetPermissionError()
+		c.sendErrorMessage(err)
+		return err
 	}
 
 	file, r, cancelFn, err := c.connection.Fs.Open(p, 0)
 	if err != nil {
 		c.connection.Log(logger.LevelError, "could not open file %#v for reading: %v", p, err)
-		c.sendErrorMessage(c.connection.GetFsError(err))
+		c.sendErrorMessage(err)
 		return err
 	}
 
@@ -625,7 +634,7 @@ func (c *scpCommand) createDir(dirPath string) error {
 	}
 	if err = c.connection.Fs.Mkdir(dirPath); err != nil {
 		c.connection.Log(logger.LevelError, "error creating dir %#v: %v", dirPath, err)
-		c.sendErrorMessage(c.connection.GetFsError(err))
+		c.sendErrorMessage(err)
 		return err
 	}
 	vfs.SetPathPermissions(c.connection.Fs, dirPath, c.connection.User.GetUID(), c.connection.User.GetGID())

diff --git a/vfs/gcsfs.go b/vfs/gcsfs.go
@@ -402,6 +402,16 @@ func (GCSFs) IsPermission(err error) bool {
 	return strings.Contains(err.Error(), "403")
 }
 
+// GetPermissionError returns a permission error for this FS
+func (GCSFs) GetPermissionError() error {
+	return errors.New("403 permission denied")
+}
+
+// GetNotExistError returns a not exist error for this FS
+func (GCSFs) GetNotExistError() error {
+	return errors.New("404 no such file or directory")
+}
+
 // CheckRootPath creates the specified local root directory if it does not exists
 func (fs GCSFs) CheckRootPath(username string, uid int, gid int) bool {
 	// we need a local directory for temporary files

diff --git a/vfs/osfs.go b/vfs/osfs.go
@@ -183,6 +183,16 @@ func (OsFs) IsPermission(err error) bool {
 	return os.IsPermission(err)
 }
 
+// GetPermissionError returns a permission error for this FS
+func (OsFs) GetPermissionError() error {
+	return os.ErrPermission
+}
+
+// GetNotExistError returns a not exist error for this FS
+func (OsFs) GetNotExistError() error {
+	return os.ErrNotExist
+}
+
 // CheckRootPath creates the root directory if it does not exists
 func (fs OsFs) CheckRootPath(username string, uid int, gid int) bool {
 	var err error

diff --git a/vfs/s3fs.go b/vfs/s3fs.go
@@ -427,6 +427,16 @@ func (S3Fs) IsPermission(err error) bool {
 	return strings.Contains(err.Error(), "403")
 }
 
+// GetPermissionError returns a permission error for this FS
+func (S3Fs) GetPermissionError() error {
+	return errors.New("403 permission denied")
+}
+
+// GetNotExistError returns a not exist error for this FS
+func (S3Fs) GetNotExistError() error {
+	return errors.New("404 no such file or directory")
+}
+
 // CheckRootPath creates the specified local root directory if it does not exists
 func (fs S3Fs) CheckRootPath(username string, uid int, gid int) bool {
 	// we need a local directory for temporary files

diff --git a/vfs/vfs.go b/vfs/vfs.go
@@ -40,6 +40,8 @@ type Fs interface {
 	ResolvePath(sftpPath string) (string, error)
 	IsNotExist(err error) bool
 	IsPermission(err error) bool
+	GetPermissionError() error
+	GetNotExistError() error
 	ScanRootDirContents() (int, int64, error)
 	GetDirSize(dirname string) (int, int64, error)
 	GetAtomicUploadPath(name string) string