From 209badf10c00b659bfc7cb3223cca53a6e01ad37 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Fri, 18 Sep 2020 19:13:09 +0200 Subject: [PATCH] scp: return better error messages Fixes #171 --- sftpd/scp.go | 45 +++++++++++++++++++++++++++------------------ vfs/gcsfs.go | 10 ++++++++++ vfs/osfs.go | 10 ++++++++++ vfs/s3fs.go | 10 ++++++++++ vfs/vfs.go | 2 ++ 5 files changed, 59 insertions(+), 18 deletions(-) diff --git a/sftpd/scp.go b/sftpd/scp.go index 1a8c3407..fdda0658 100644 --- 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,8 +281,9 @@ 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() { @@ -286,7 +291,7 @@ func (c *scpCommand) handleUpload(uploadFilePath string, sizeToRead int64) error 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,22 +449,23 @@ 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 @@ -467,19 +473,22 @@ func (c *scpCommand) handleDownload(filePath string) error { 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 index 59103cb6..ac0a605c 100644 --- 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 index d3558561..b60b5719 100644 --- 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 index 95bbba43..837fbd1d 100644 --- 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 index fbcf7108..1015515c 100644 --- 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