scp, ssh commands: hide the real fs path on errors

The underlying filesystem errors for permissions and non-existing files
can contain the real storage path.
Map these errors to more generic ones to avoid to leak this info

Fixes #109
This commit is contained in:
Nicola Murino
2020-04-22 12:26:18 +02:00
parent 4f668bf558
commit 0a47412e8c
3 changed files with 47 additions and 34 deletions

View File

@@ -1195,6 +1195,7 @@ func TestSCPParseUploadMessage(t *testing.T) {
} }
connection := Connection{ connection := Connection{
channel: &mockSSHChannel, channel: &mockSSHChannel,
fs: vfs.NewOsFs("", os.TempDir(), nil),
} }
scpCommand := scpCommand{ scpCommand := scpCommand{
sshCommand: sshCommand{ sshCommand: sshCommand{
@@ -1632,6 +1633,7 @@ func TestSCPUploadFiledata(t *testing.T) {
}, },
protocol: protocolSCP, protocol: protocolSCP,
channel: &mockSSHChannel, channel: &mockSSHChannel,
fs: vfs.NewOsFs("", os.TempDir(), nil),
} }
scpCommand := scpCommand{ scpCommand := scpCommand{
sshCommand: sshCommand{ sshCommand: sshCommand{

View File

@@ -24,7 +24,7 @@ var (
warnMsg = []byte{0x01} // must be followed by an optional message and a newline warnMsg = []byte{0x01} // must be followed by an optional message and a newline
errMsg = []byte{0x02} // must be followed by an optional message and a newline errMsg = []byte{0x02} // must be followed by an optional message and a newline
newLine = []byte{0x0A} newLine = []byte{0x0A}
errPermission = errors.New("Permission denied") errPermission = errors.New("permission denied")
) )
type scpCommand struct { type scpCommand struct {
@@ -122,12 +122,12 @@ func (c *scpCommand) handleCreateDir(dirPath string) error {
p, err := c.connection.fs.ResolvePath(dirPath) p, err := c.connection.fs.ResolvePath(dirPath)
if err != nil { if err != nil {
c.connection.Log(logger.LevelWarn, logSenderSCP, "error creating dir: %#v, invalid file path, err: %v", dirPath, err) c.connection.Log(logger.LevelWarn, logSenderSCP, "error creating dir: %#v, invalid file path, err: %v", dirPath, err)
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return err return err
} }
if !c.connection.User.HasPerm(dataprovider.PermCreateDirs, path.Dir(dirPath)) { if !c.connection.User.HasPerm(dataprovider.PermCreateDirs, path.Dir(dirPath)) {
c.connection.Log(logger.LevelWarn, logSenderSCP, "error creating dir: %#v, permission denied", dirPath) c.connection.Log(logger.LevelWarn, logSenderSCP, "error creating dir: %#v, permission denied", dirPath)
c.sendErrorMessage(errPermission.Error()) c.sendErrorMessage(errPermission)
return errPermission return errPermission
} }
@@ -154,14 +154,14 @@ func (c *scpCommand) getUploadFileData(sizeToRead int64, transfer *Transfer) err
for { for {
n, err := c.connection.channel.Read(buf) n, err := c.connection.channel.Read(buf)
if err != nil { if err != nil {
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
transfer.TransferError(err) transfer.TransferError(err)
transfer.Close() transfer.Close()
return err return err
} }
_, err = transfer.WriteAt(buf[:n], sizeToRead-remaining) _, err = transfer.WriteAt(buf[:n], sizeToRead-remaining)
if err != nil { if err != nil {
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
transfer.Close() transfer.Close()
return err return err
} }
@@ -182,7 +182,7 @@ func (c *scpCommand) getUploadFileData(sizeToRead int64, transfer *Transfer) err
} }
err = transfer.Close() err = transfer.Close()
if err != nil { if err != nil {
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return err return err
} }
return c.sendConfirmationMessage() return c.sendConfirmationMessage()
@@ -192,7 +192,7 @@ func (c *scpCommand) handleUploadFile(requestPath, filePath string, sizeToRead i
if !c.connection.hasSpace(true) { if !c.connection.hasSpace(true) {
err := fmt.Errorf("denying file write due to space limit") err := fmt.Errorf("denying file write due to space limit")
c.connection.Log(logger.LevelWarn, logSenderSCP, "error uploading file: %#v, err: %v", filePath, err) c.connection.Log(logger.LevelWarn, logSenderSCP, "error uploading file: %#v, err: %v", filePath, err)
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return err return err
} }
@@ -207,7 +207,7 @@ func (c *scpCommand) handleUploadFile(requestPath, filePath string, sizeToRead i
file, w, cancelFn, err := c.connection.fs.Create(filePath, 0) file, w, cancelFn, err := c.connection.fs.Create(filePath, 0)
if err != nil { if err != nil {
c.connection.Log(logger.LevelError, logSenderSCP, "error creating file %#v: %v", requestPath, err) c.connection.Log(logger.LevelError, logSenderSCP, "error creating file %#v: %v", requestPath, err)
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return err return err
} }
@@ -246,13 +246,13 @@ func (c *scpCommand) handleUpload(uploadFilePath string, sizeToRead int64) error
if !c.connection.User.IsFileAllowed(uploadFilePath) { if !c.connection.User.IsFileAllowed(uploadFilePath) {
c.connection.Log(logger.LevelWarn, logSenderSCP, "writing file %#v is not allowed", uploadFilePath) c.connection.Log(logger.LevelWarn, logSenderSCP, "writing file %#v is not allowed", uploadFilePath)
c.sendErrorMessage(errPermission.Error()) c.sendErrorMessage(errPermission)
} }
p, err := c.connection.fs.ResolvePath(uploadFilePath) p, err := c.connection.fs.ResolvePath(uploadFilePath)
if err != nil { if err != nil {
c.connection.Log(logger.LevelWarn, logSenderSCP, "error uploading file: %#v, err: %v", uploadFilePath, err) c.connection.Log(logger.LevelWarn, logSenderSCP, "error uploading file: %#v, err: %v", uploadFilePath, err)
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return err return err
} }
filePath := p filePath := p
@@ -263,7 +263,7 @@ func (c *scpCommand) handleUpload(uploadFilePath string, sizeToRead int64) error
if c.connection.fs.IsNotExist(statErr) { if c.connection.fs.IsNotExist(statErr) {
if !c.connection.User.HasPerm(dataprovider.PermUpload, path.Dir(uploadFilePath)) { if !c.connection.User.HasPerm(dataprovider.PermUpload, path.Dir(uploadFilePath)) {
c.connection.Log(logger.LevelWarn, logSenderSCP, "cannot upload file: %#v, permission denied", uploadFilePath) c.connection.Log(logger.LevelWarn, logSenderSCP, "cannot upload file: %#v, permission denied", uploadFilePath)
c.sendErrorMessage(errPermission.Error()) c.sendErrorMessage(errPermission)
return errPermission return errPermission
} }
return c.handleUploadFile(p, filePath, sizeToRead, true, 0) return c.handleUploadFile(p, filePath, sizeToRead, true, 0)
@@ -271,20 +271,20 @@ func (c *scpCommand) handleUpload(uploadFilePath string, sizeToRead int64) error
if statErr != nil { if statErr != nil {
c.connection.Log(logger.LevelError, logSenderSCP, "error performing file stat %#v: %v", p, statErr) c.connection.Log(logger.LevelError, logSenderSCP, "error performing file stat %#v: %v", p, statErr)
c.sendErrorMessage(statErr.Error()) c.sendErrorMessage(statErr)
return statErr return statErr
} }
if stat.IsDir() { if stat.IsDir() {
c.connection.Log(logger.LevelWarn, logSenderSCP, "attempted to open a directory for writing to: %#v", p) c.connection.Log(logger.LevelWarn, logSenderSCP, "attempted to open a directory for writing to: %#v", p)
err = fmt.Errorf("Attempted to open a directory for writing: %#v", p) err = fmt.Errorf("Attempted to open a directory for writing: %#v", p)
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return err return err
} }
if !c.connection.User.HasPerm(dataprovider.PermOverwrite, uploadFilePath) { if !c.connection.User.HasPerm(dataprovider.PermOverwrite, uploadFilePath) {
c.connection.Log(logger.LevelWarn, logSenderSCP, "cannot overwrite file: %#v, permission denied", uploadFilePath) c.connection.Log(logger.LevelWarn, logSenderSCP, "cannot overwrite file: %#v, permission denied", uploadFilePath)
c.sendErrorMessage(errPermission.Error()) c.sendErrorMessage(errPermission)
return errPermission return errPermission
} }
@@ -293,7 +293,7 @@ func (c *scpCommand) handleUpload(uploadFilePath string, sizeToRead int64) error
if err != nil { if err != nil {
c.connection.Log(logger.LevelError, logSenderSCP, "error renaming existing file for atomic upload, source: %#v, dest: %#v, err: %v", c.connection.Log(logger.LevelError, logSenderSCP, "error renaming existing file for atomic upload, source: %#v, dest: %#v, err: %v",
p, filePath, err) p, filePath, err)
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return err return err
} }
} }
@@ -346,7 +346,7 @@ func (c *scpCommand) handleRecursiveDownload(dirPath string, stat os.FileInfo) e
files, err := c.connection.fs.ReadDir(dirPath) files, err := c.connection.fs.ReadDir(dirPath)
files = c.connection.User.AddVirtualDirs(files, c.connection.fs.GetRelativePath(dirPath)) files = c.connection.User.AddVirtualDirs(files, c.connection.fs.GetRelativePath(dirPath))
if err != nil { if err != nil {
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return err return err
} }
var dirs []string var dirs []string
@@ -362,7 +362,7 @@ func (c *scpCommand) handleRecursiveDownload(dirPath string, stat os.FileInfo) e
} }
} }
if err != nil { if err != nil {
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return err return err
} }
for _, dir := range dirs { for _, dir := range dirs {
@@ -372,7 +372,7 @@ func (c *scpCommand) handleRecursiveDownload(dirPath string, stat os.FileInfo) e
} }
} }
if err != nil { if err != nil {
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return err return err
} }
err = c.sendProtocolMessage("E\n") err = c.sendProtocolMessage("E\n")
@@ -386,7 +386,7 @@ func (c *scpCommand) handleRecursiveDownload(dirPath string, stat os.FileInfo) e
return err return err
} }
err = fmt.Errorf("Unable to send directory for non recursive copy") err = fmt.Errorf("Unable to send directory for non recursive copy")
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return err return err
} }
@@ -432,7 +432,7 @@ func (c *scpCommand) sendDownloadFileData(filePath string, stat os.FileInfo, tra
} }
} }
if err != io.EOF { if err != io.EOF {
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return err return err
} }
err = c.sendConfirmationMessage() err = c.sendConfirmationMessage()
@@ -452,21 +452,21 @@ func (c *scpCommand) handleDownload(filePath string) error {
if err != nil { if err != nil {
err := fmt.Errorf("Invalid file path") err := fmt.Errorf("Invalid file path")
c.connection.Log(logger.LevelWarn, logSenderSCP, "error downloading file: %#v, invalid file path", filePath) c.connection.Log(logger.LevelWarn, logSenderSCP, "error downloading file: %#v, invalid file path", filePath)
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return err return err
} }
var stat os.FileInfo var stat os.FileInfo
if stat, err = c.connection.fs.Stat(p); err != nil { if stat, err = c.connection.fs.Stat(p); err != nil {
c.connection.Log(logger.LevelWarn, logSenderSCP, "error downloading file: %#v, err: %v", p, err) c.connection.Log(logger.LevelWarn, logSenderSCP, "error downloading file: %#v, err: %v", p, err)
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return err return err
} }
if stat.IsDir() { if stat.IsDir() {
if !c.connection.User.HasPerm(dataprovider.PermDownload, filePath) { if !c.connection.User.HasPerm(dataprovider.PermDownload, filePath) {
c.connection.Log(logger.LevelWarn, logSenderSCP, "error downloading dir: %#v, permission denied", filePath) c.connection.Log(logger.LevelWarn, logSenderSCP, "error downloading dir: %#v, permission denied", filePath)
c.sendErrorMessage(errPermission.Error()) c.sendErrorMessage(errPermission)
return errPermission return errPermission
} }
err = c.handleRecursiveDownload(p, stat) err = c.handleRecursiveDownload(p, stat)
@@ -475,19 +475,19 @@ func (c *scpCommand) handleDownload(filePath string) error {
if !c.connection.User.HasPerm(dataprovider.PermDownload, path.Dir(filePath)) { if !c.connection.User.HasPerm(dataprovider.PermDownload, path.Dir(filePath)) {
c.connection.Log(logger.LevelWarn, logSenderSCP, "error downloading dir: %#v, permission denied", filePath) c.connection.Log(logger.LevelWarn, logSenderSCP, "error downloading dir: %#v, permission denied", filePath)
c.sendErrorMessage(errPermission.Error()) c.sendErrorMessage(errPermission)
return errPermission return errPermission
} }
if !c.connection.User.IsFileAllowed(filePath) { if !c.connection.User.IsFileAllowed(filePath) {
c.connection.Log(logger.LevelWarn, logSenderSCP, "reading file %#v is not allowed", filePath) c.connection.Log(logger.LevelWarn, logSenderSCP, "reading file %#v is not allowed", filePath)
c.sendErrorMessage(errPermission.Error()) c.sendErrorMessage(errPermission)
} }
file, r, cancelFn, err := c.connection.fs.Open(p) file, r, cancelFn, err := c.connection.fs.Open(p)
if err != nil { if err != nil {
c.connection.Log(logger.LevelError, logSenderSCP, "could not open file %#v for reading: %v", p, err) c.connection.Log(logger.LevelError, logSenderSCP, "could not open file %#v for reading: %v", p, err)
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return err return err
} }
@@ -593,9 +593,9 @@ func (c *scpCommand) readProtocolMessage() (string, error) {
} }
// send an error message and close the channel // send an error message and close the channel
func (c *scpCommand) sendErrorMessage(error string) { func (c *scpCommand) sendErrorMessage(err error) {
c.connection.channel.Write(errMsg) c.connection.channel.Write(errMsg)
c.connection.channel.Write([]byte(error)) c.connection.channel.Write([]byte(c.getMappedError(err).Error()))
c.connection.channel.Write(newLine) c.connection.channel.Write(newLine)
c.connection.channel.Close() c.connection.channel.Close()
} }
@@ -651,7 +651,7 @@ func (c *scpCommand) createDir(dirPath string) error {
} }
if err = c.connection.fs.Mkdir(dirPath); err != nil { if err = c.connection.fs.Mkdir(dirPath); err != nil {
c.connection.Log(logger.LevelError, logSenderSCP, "error creating dir %#v: %v", dirPath, err) c.connection.Log(logger.LevelError, logSenderSCP, "error creating dir %#v: %v", dirPath, err)
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return err return err
} }
vfs.SetPathPermissions(c.connection.fs, dirPath, c.connection.User.GetUID(), c.connection.User.GetGID()) vfs.SetPathPermissions(c.connection.fs, dirPath, c.connection.User.GetUID(), c.connection.User.GetGID())
@@ -671,7 +671,7 @@ func (c *scpCommand) parseUploadMessage(command string) (int64, string, error) {
err = fmt.Errorf("unknown or invalid upload message: %v args: %v user: %v", err = fmt.Errorf("unknown or invalid upload message: %v args: %v user: %v",
command, c.args, c.connection.User.Username) command, c.args, c.connection.User.Username)
c.connection.Log(logger.LevelWarn, logSenderSCP, "error: %v", err) c.connection.Log(logger.LevelWarn, logSenderSCP, "error: %v", err)
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return size, name, err return size, name, err
} }
parts := strings.SplitN(command, " ", 3) parts := strings.SplitN(command, " ", 3)
@@ -679,20 +679,20 @@ func (c *scpCommand) parseUploadMessage(command string) (int64, string, error) {
size, err = strconv.ParseInt(parts[1], 10, 64) size, err = strconv.ParseInt(parts[1], 10, 64)
if err != nil { if err != nil {
c.connection.Log(logger.LevelWarn, logSenderSCP, "error getting size from upload message: %v", err) c.connection.Log(logger.LevelWarn, logSenderSCP, "error getting size from upload message: %v", err)
c.sendErrorMessage(fmt.Sprintf("Error getting size: %v", err)) c.sendErrorMessage(err)
return size, name, err return size, name, err
} }
name = parts[2] name = parts[2]
if len(name) == 0 { if len(name) == 0 {
err = fmt.Errorf("error getting name from upload message, cannot be empty") err = fmt.Errorf("error getting name from upload message, cannot be empty")
c.connection.Log(logger.LevelWarn, logSenderSCP, "error: %v", err) c.connection.Log(logger.LevelWarn, logSenderSCP, "error: %v", err)
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return size, name, err return size, name, err
} }
} else { } else {
err = fmt.Errorf("Error splitting upload message: %#v", command) err = fmt.Errorf("Error splitting upload message: %#v", command)
c.connection.Log(logger.LevelWarn, logSenderSCP, "error: %v", err) c.connection.Log(logger.LevelWarn, logSenderSCP, "error: %v", err)
c.sendErrorMessage(err.Error()) c.sendErrorMessage(err)
return size, name, err return size, name, err
} }
return size, name, err return size, name, err

View File

@@ -422,8 +422,19 @@ func (c *sshCommand) getDestPath() string {
return result return result
} }
// we try to avoid to leak the real filesystem path here
func (c *sshCommand) getMappedError(err error) error {
if c.connection.fs.IsNotExist(err) {
return errors.New("no such file or directory")
}
if c.connection.fs.IsPermission(err) {
return errors.New("permission denied")
}
return err
}
func (c *sshCommand) sendErrorResponse(err error) error { func (c *sshCommand) sendErrorResponse(err error) error {
errorString := fmt.Sprintf("%v: %v %v\n", c.command, c.getDestPath(), err) errorString := fmt.Sprintf("%v: %v %v\n", c.command, c.getDestPath(), c.getMappedError(err))
c.connection.channel.Write([]byte(errorString)) c.connection.channel.Write([]byte(errorString))
c.sendExitStatus(err) c.sendExitStatus(err)
return err return err