diff --git a/README.md b/README.md index fefc872f..44d3970b 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ Full featured and highly configurable SFTP server - Quota support: accounts can have individual quota expressed as max total size and/or max number of files. - Bandwidth throttling is supported, with distinct settings for upload and download. - Per user maximum concurrent sessions. -- Per user permissions: list directories content, upload, overwrite, download, delete, rename, create directories, create symlinks can be enabled or disabled. +- Per user permissions: list directories content, upload, overwrite, download, delete, rename, create directories, create symlinks, changing owner/group and mode can be enabled or disabled. - Per user files/folders ownership: you can map all the users to the system account that runs SFTPGo (all platforms are supported) or you can run SFTPGo as root user and map each user or group of users to a different system account (*NIX only). - Configurable custom commands and/or HTTP notifications on files upload, download, delete, rename and on users add, update and delete. - Automatically terminating idle connections. @@ -150,6 +150,7 @@ The `sftpgo` configuration file contains the following sections: - `ciphers`, list of strings. Allowed ciphers. Leave empty to use default values. The supported values can be found here: [`crypto/ssh`](https://github.com/golang/crypto/blob/master/ssh/common.go#L28 "Supported ciphers") - `macs`, list of strings. available MAC (message authentication code) algorithms in preference order. Leave empty to use default values. The supported values can be found here: [`crypto/ssh`](https://github.com/golang/crypto/blob/master/ssh/common.go#L84 "Supported MACs") - `login_banner_file`, path to the login banner file. The contents of the specified file, if any, are sent to the remote user before authentication is allowed. It can be a path relative to the config dir or an absolute one. Leave empty to send no login banner + - `setstat_mode`, integer. 0 means "normal mode": requests for changing permissions and owner/group are executed. 1 means "ignore mode": requests for changing permissions and owner/group are silently ignored. - **"data_provider"**, the configuration for the data provider - `driver`, string. Supported drivers are `sqlite`, `mysql`, `postgresql`, `bolt`, `memory` - `name`, string. Database name. For driver `sqlite` this can be the database name relative to the config dir or the absolute path to the SQLite database. @@ -207,7 +208,8 @@ Here is a full example showing the default config in JSON format: "kex_algorithms": [], "ciphers": [], "macs": [], - "login_banner_file": "" + "login_banner_file": "", + "setstat_mode": 0 }, "data_provider": { "driver": "sqlite", @@ -363,6 +365,8 @@ For each account the following properties can be configured: - `rename` rename files or directories is allowed - `create_dirs` create directories is allowed - `create_symlinks` create symbolic links is allowed + - `chmod` changing file or directory permissions is allowed + - `chown` changing file or directory owner and group is allowed - `upload_bandwidth` maximum upload bandwidth as KB/s, 0 means unlimited. - `download_bandwidth` maximum download bandwidth as KB/s, 0 means unlimited. @@ -452,11 +456,14 @@ The logs can be divided into the following categories: - `connection_id` string. Unique connection identifier - `protocol` string. `SFTP` or `SCP` - **"command logs"**, SFTP/SCP command logs: - - `sender` string. `Rename`, `Rmdir`, `Mkdir`, `Symlink`, `Remove` + - `sender` string. `Rename`, `Rmdir`, `Mkdir`, `Symlink`, `Remove`, `Chmod`, `Chown` - `level` string - `username`, string - `file_path` string - `target_path` string + - `filemode` string. Valid for sender `Chmod` otherwise empty + - `uid` integer. Valid for sender `Chown` otherwise -1 + - `gid` integer. Valid for sender `Chown` otherwise -1 - `connection_id` string. Unique connection identifier - `protocol` string. `SFTP` or `SCP` - **"http logs"**, REST API logs: diff --git a/dataprovider/dataprovider.go b/dataprovider/dataprovider.go index 6b078f7f..c41b49e1 100644 --- a/dataprovider/dataprovider.go +++ b/dataprovider/dataprovider.go @@ -65,7 +65,7 @@ var ( BoltDataProviderName, MemoryDataProviderName} // ValidPerms list that contains all the valid permissions for an user ValidPerms = []string{PermAny, PermListItems, PermDownload, PermUpload, PermOverwrite, PermRename, PermDelete, - PermCreateDirs, PermCreateSymlinks} + PermCreateDirs, PermCreateSymlinks, PermChmod, PermChown} config Config provider Provider sqlPlaceholders []string diff --git a/dataprovider/user.go b/dataprovider/user.go index 6cf364e9..87ce1b3b 100644 --- a/dataprovider/user.go +++ b/dataprovider/user.go @@ -30,6 +30,10 @@ const ( PermCreateDirs = "create_dirs" // create symbolic links is allowed PermCreateSymlinks = "create_symlinks" + // changing file or directory permissions is allowed + PermChmod = "chmod" + // changing file or directory owner is allowed + PermChown = "chown" ) // User defines an SFTP user diff --git a/httpd/schema/openapi.yaml b/httpd/schema/openapi.yaml index c7dfb1f6..7a34f6f6 100644 --- a/httpd/schema/openapi.yaml +++ b/httpd/schema/openapi.yaml @@ -543,6 +543,8 @@ components: - rename - create_dirs - create_symlinks + - chmod + - chown description: > Permissions: * `*` - all permissions are granted @@ -554,6 +556,8 @@ components: * `rename` - rename files or directories is allowed * `create_dirs` - create directories is allowed * `create_symlinks` - create links is allowed + * `chmod` changing file or directory permissions is allowed + * `chown` changing file or directory owner and group is allowed User: type: object properties: diff --git a/logger/logger.go b/logger/logger.go index 40c2531d..0eebebf7 100644 --- a/logger/logger.go +++ b/logger/logger.go @@ -150,13 +150,16 @@ func TransferLog(operation string, path string, elapsed int64, size int64, user } // CommandLog logs an SFTP/SCP command -func CommandLog(command string, path string, target string, user string, connectionID string, protocol string) { +func CommandLog(command, path, target, user, fileMode, connectionID, protocol string, uid, gid int) { logger.Info(). Timestamp(). Str("sender", command). Str("username", user). Str("file_path", path). Str("target_path", target). + Str("filemode", fileMode). + Int("uid", uid). + Int("gid", gid). Str("connection_id", connectionID). Str("protocol", protocol). Msg("") diff --git a/scripts/sftpgo_api_cli.py b/scripts/sftpgo_api_cli.py index e0c5b6c6..9043003e 100755 --- a/scripts/sftpgo_api_cli.py +++ b/scripts/sftpgo_api_cli.py @@ -1,8 +1,8 @@ #!/usr/bin/env python -import argparse from datetime import datetime -import json +import argparse +import json import requests try: @@ -160,7 +160,7 @@ def addCommonUserArguments(parser): parser.add_argument('-F', '--quota-files', type=int, default=0, help="default: %(default)s") parser.add_argument('-G', '--permissions', type=str, nargs='+', default=[], choices=['*', 'list', 'download', 'upload', 'overwrite', 'delete', 'rename', 'create_dirs', - 'create_symlinks'], help='Default: %(default)s') + 'create_symlinks', 'chmod', 'chown'], help='Default: %(default)s') parser.add_argument('-U', '--upload-bandwidth', type=int, default=0, help='Maximum upload bandwidth as KB/s, 0 means unlimited. Default: %(default)s') parser.add_argument('-D', '--download-bandwidth', type=int, default=0, diff --git a/sftpd/handler.go b/sftpd/handler.go index 7ea40130..f380b094 100644 --- a/sftpd/handler.go +++ b/sftpd/handler.go @@ -56,20 +56,20 @@ func (c Connection) Fileread(request *sftp.Request) (io.ReaderAt, error) { p, err := c.buildPath(request.Filepath) if err != nil { - return nil, sftp.ErrSSHFxNoSuchFile + return nil, getSFTPErrorFromOSError(err) } c.lock.Lock() defer c.lock.Unlock() - if _, err := os.Stat(p); os.IsNotExist(err) { - return nil, sftp.ErrSSHFxNoSuchFile + if _, err := os.Stat(p); err != nil { + return nil, getSFTPErrorFromOSError(err) } file, err := os.Open(p) if err != nil { - c.Log(logger.LevelError, logSender, "could not open file %#v for reading: %v", p, err) - return nil, sftp.ErrSSHFxFailure + c.Log(logger.LevelWarn, logSender, "could not open file %#v for reading: %v", p, err) + return nil, getSFTPErrorFromOSError(err) } c.Log(logger.LevelDebug, logSender, "fileread requested for path: %#v", p) @@ -103,7 +103,7 @@ func (c Connection) Filewrite(request *sftp.Request) (io.WriterAt, error) { p, err := c.buildPath(request.Filepath) if err != nil { - return nil, sftp.ErrSSHFxNoSuchFile + return nil, getSFTPErrorFromOSError(err) } filePath := p @@ -123,7 +123,7 @@ func (c Connection) Filewrite(request *sftp.Request) (io.WriterAt, error) { if statErr != nil { c.Log(logger.LevelError, logSender, "error performing file stat %#v: %v", p, statErr) - return nil, sftp.ErrSSHFxFailure + return nil, getSFTPErrorFromOSError(err) } // This happen if we upload a file that has the same name of an existing directory @@ -146,12 +146,12 @@ func (c Connection) Filecmd(request *sftp.Request) error { p, err := c.buildPath(request.Filepath) if err != nil { - return sftp.ErrSSHFxNoSuchFile + return getSFTPErrorFromOSError(err) } target, err := c.getSFTPCmdTargetPath(request.Target) if err != nil { - return sftp.ErrSSHFxOpUnsupported + return err } c.Log(logger.LevelDebug, logSender, "new cmd, method: %v, sourcePath: %#v, targetPath: %#v", request.Method, @@ -159,10 +159,9 @@ func (c Connection) Filecmd(request *sftp.Request) error { switch request.Method { case "Setstat": - return nil + return c.handleSFTPSetstat(p, request) case "Rename": - err = c.handleSFTPRename(p, target) - if err != nil { + if err = c.handleSFTPRename(p, target); err != nil { return err } @@ -178,8 +177,7 @@ func (c Connection) Filecmd(request *sftp.Request) error { break case "Symlink": - err = c.handleSFTPSymlink(p, target) - if err != nil { + if err = c.handleSFTPSymlink(p, target); err != nil { return err } @@ -208,7 +206,7 @@ func (c Connection) Filelist(request *sftp.Request) (sftp.ListerAt, error) { updateConnectionActivity(c.ID) p, err := c.buildPath(request.Filepath) if err != nil { - return nil, sftp.ErrSSHFxNoSuchFile + return nil, getSFTPErrorFromOSError(err) } switch request.Method { @@ -220,11 +218,9 @@ func (c Connection) Filelist(request *sftp.Request) (sftp.ListerAt, error) { c.Log(logger.LevelDebug, logSender, "requested list file for dir: %#v", p) files, err := ioutil.ReadDir(p) - if os.IsNotExist(err) { - return nil, sftp.ErrSSHFxNoSuchFile - } else if err != nil { - c.Log(logger.LevelError, logSender, "error listing directory: %#v", err) - return nil, sftp.ErrSSHFxFailure + if err != nil { + c.Log(logger.LevelWarn, logSender, "error listing directory: %#v", err) + return nil, getSFTPErrorFromOSError(err) } return listerAt(files), nil @@ -233,13 +229,11 @@ func (c Connection) Filelist(request *sftp.Request) (sftp.ListerAt, error) { return nil, sftp.ErrSSHFxPermissionDenied } - c.Log(logger.LevelDebug, logSender, "requested stat for file: %#v", p) + c.Log(logger.LevelDebug, logSender, "requested Stat for file: %#v", p) s, err := os.Stat(p) - if os.IsNotExist(err) { - return nil, sftp.ErrSSHFxNoSuchFile - } else if err != nil { - c.Log(logger.LevelError, logSender, "error running STAT on file: %#v", err) - return nil, sftp.ErrSSHFxFailure + if err != nil { + c.Log(logger.LevelWarn, logSender, "error running Stat on file: %#v", err) + return nil, getSFTPErrorFromOSError(err) } return listerAt([]os.FileInfo{s}), nil @@ -251,27 +245,58 @@ func (c Connection) Filelist(request *sftp.Request) (sftp.ListerAt, error) { func (c Connection) getSFTPCmdTargetPath(requestTarget string) (string, error) { var target string // If a target is provided in this request validate that it is going to the correct - // location for the server. If it is not, return an operation unsupported error. This - // is maybe not the best error response, but its not wrong either. - if requestTarget != "" { + // location for the server. If it is not, return an error + if len(requestTarget) > 0 { var err error target, err = c.buildPath(requestTarget) if err != nil { - return target, sftp.ErrSSHFxOpUnsupported + return target, getSFTPErrorFromOSError(err) } } return target, nil } +func (c Connection) handleSFTPSetstat(path string, request *sftp.Request) error { + if setstatMode == 1 { + return nil + } + attrFlags := request.AttrFlags() + if attrFlags.Permissions { + if !c.User.HasPerm(dataprovider.PermChmod) { + return sftp.ErrSSHFxPermissionDenied + } + fileMode := request.Attributes().FileMode() + if err := os.Chmod(path, fileMode); err != nil { + c.Log(logger.LevelWarn, logSender, "failed to chmod path %#v, mode: %v, err: %v", path, fileMode.String(), err) + return getSFTPErrorFromOSError(err) + } + logger.CommandLog(chmodLogSender, path, "", c.User.Username, fileMode.String(), c.ID, c.protocol, -1, -1) + return nil + } else if attrFlags.UidGid { + if !c.User.HasPerm(dataprovider.PermChown) { + return sftp.ErrSSHFxPermissionDenied + } + uid := int(request.Attributes().UID) + gid := int(request.Attributes().GID) + if err := os.Chown(path, uid, gid); err != nil { + c.Log(logger.LevelWarn, logSender, "failed to chown path %#v, uid: %v, gid: %v, err: %v", path, uid, gid, err) + return getSFTPErrorFromOSError(err) + } + logger.CommandLog(chownLogSender, path, "", c.User.Username, "", c.ID, c.protocol, uid, gid) + return nil + } + return nil +} + func (c Connection) handleSFTPRename(sourcePath string, targetPath string) error { if !c.User.HasPerm(dataprovider.PermRename) { return sftp.ErrSSHFxPermissionDenied } if err := os.Rename(sourcePath, targetPath); err != nil { - c.Log(logger.LevelError, logSender, "failed to rename file, source: %#v target: %#v: %v", sourcePath, targetPath, err) - return sftp.ErrSSHFxFailure + c.Log(logger.LevelWarn, logSender, "failed to rename file, source: %#v target: %#v: %v", sourcePath, targetPath, err) + return getSFTPErrorFromOSError(err) } - logger.CommandLog(renameLogSender, sourcePath, targetPath, c.User.Username, c.ID, c.protocol) + logger.CommandLog(renameLogSender, sourcePath, targetPath, c.User.Username, "", c.ID, c.protocol, -1, -1) go executeAction(operationRename, c.User.Username, sourcePath, targetPath) return nil } @@ -284,8 +309,8 @@ func (c Connection) handleSFTPRmdir(path string) error { 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 + c.Log(logger.LevelWarn, logSender, "failed to remove a dir %#v: stat error: %v", path, err) + return getSFTPErrorFromOSError(err) } if !fi.IsDir() || fi.Mode()&os.ModeSymlink == os.ModeSymlink { c.Log(logger.LevelDebug, logSender, "cannot remove %#v is not a directory", path) @@ -293,11 +318,11 @@ func (c Connection) handleSFTPRmdir(path string) error { } if err = os.Remove(path); err != nil { - c.Log(logger.LevelError, logSender, "failed to remove directory %#v: %v", path, err) - return sftp.ErrSSHFxFailure + c.Log(logger.LevelWarn, logSender, "failed to remove directory %#v: %v", path, err) + return getSFTPErrorFromOSError(err) } - logger.CommandLog(rmdirLogSender, path, "", c.User.Username, c.ID, c.protocol) + logger.CommandLog(rmdirLogSender, path, "", c.User.Username, "", c.ID, c.protocol, -1, -1) return sftp.ErrSSHFxOk } @@ -307,10 +332,10 @@ func (c Connection) handleSFTPSymlink(sourcePath string, targetPath string) erro } if err := os.Symlink(sourcePath, targetPath); err != nil { c.Log(logger.LevelWarn, logSender, "failed to create symlink %#v -> %#v: %v", sourcePath, targetPath, err) - return sftp.ErrSSHFxFailure + return getSFTPErrorFromOSError(err) } - logger.CommandLog(symlinkLogSender, sourcePath, targetPath, c.User.Username, c.ID, c.protocol) + logger.CommandLog(symlinkLogSender, sourcePath, targetPath, c.User.Username, "", c.ID, c.protocol, -1, -1) return nil } @@ -319,12 +344,12 @@ func (c Connection) handleSFTPMkdir(path string) error { return sftp.ErrSSHFxPermissionDenied } 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 + c.Log(logger.LevelWarn, logSender, "error creating missing dir: %#v error: %v", path, err) + return getSFTPErrorFromOSError(err) } utils.SetPathPermissions(path, c.User.GetUID(), c.User.GetGID()) - logger.CommandLog(mkdirLogSender, path, "", c.User.Username, c.ID, c.protocol) + logger.CommandLog(mkdirLogSender, path, "", c.User.Username, "", c.ID, c.protocol, -1, -1) return nil } @@ -337,8 +362,8 @@ func (c Connection) handleSFTPRemove(path string) error { var fi os.FileInfo var err error if fi, err = os.Lstat(path); err != nil { - c.Log(logger.LevelError, logSender, "failed to remove a file %#v: stat error: %v", path, err) - return sftp.ErrSSHFxFailure + c.Log(logger.LevelWarn, logSender, "failed to remove a file %#v: stat error: %v", path, err) + return getSFTPErrorFromOSError(err) } if fi.IsDir() && fi.Mode()&os.ModeSymlink != os.ModeSymlink { c.Log(logger.LevelDebug, logSender, "cannot remove %#v is not a file/symlink", path) @@ -346,11 +371,11 @@ func (c Connection) handleSFTPRemove(path string) error { } 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) - return sftp.ErrSSHFxFailure + c.Log(logger.LevelWarn, logSender, "failed to remove a file/symlink %#v: %v", path, err) + return getSFTPErrorFromOSError(err) } - logger.CommandLog(removeLogSender, path, "", c.User.Username, c.ID, c.protocol) + logger.CommandLog(removeLogSender, path, "", c.User.Username, "", c.ID, c.protocol, -1, -1) if fi.Mode()&os.ModeSymlink != os.ModeSymlink { dataprovider.UpdateUserQuota(dataProvider, c.User, -1, -size, false) } @@ -367,8 +392,8 @@ func (c Connection) handleSFTPUploadToNewFile(requestPath, filePath string) (io. file, err := os.Create(filePath) if err != nil { - c.Log(logger.LevelError, logSender, "error creating file %#v: %v", requestPath, err) - return nil, sftp.ErrSSHFxFailure + c.Log(logger.LevelWarn, logSender, "error creating file %#v: %v", requestPath, err) + return nil, getSFTPErrorFromOSError(err) } utils.SetPathPermissions(filePath, c.User.GetUID(), c.User.GetGID()) @@ -407,16 +432,16 @@ func (c Connection) handleSFTPUploadToExistingFile(pflags sftp.FileOpenFlags, re if isAtomicUploadEnabled() { err = os.Rename(requestPath, filePath) if err != nil { - c.Log(logger.LevelError, logSender, "error renaming existing file for atomic upload, source: %#v, dest: %#v, err: %v", + c.Log(logger.LevelWarn, logSender, "error renaming existing file for atomic upload, source: %#v, dest: %#v, err: %v", requestPath, filePath, err) - return nil, sftp.ErrSSHFxFailure + return nil, getSFTPErrorFromOSError(err) } } // we use 0666 so the umask is applied file, err := os.OpenFile(filePath, osFlags, 0666) if err != nil { - c.Log(logger.LevelError, logSender, "error opening existing file, flags: %v, source: %#v, err: %v", pflags, filePath, err) - return nil, sftp.ErrSSHFxFailure + c.Log(logger.LevelWarn, logSender, "error opening existing file, flags: %v, source: %#v, err: %v", pflags, filePath, err) + return nil, getSFTPErrorFromOSError(err) } if pflags.Append && osFlags&os.O_TRUNC == 0 { @@ -602,3 +627,14 @@ func getUploadTempFilePath(path string) string { guid := xid.New().String() return filepath.Join(dir, ".sftpgo-upload."+guid+"."+filepath.Base(path)) } + +func getSFTPErrorFromOSError(err error) error { + if os.IsNotExist(err) { + return sftp.ErrSSHFxNoSuchFile + } else if os.IsPermission(err) { + return sftp.ErrSSHFxPermissionDenied + } else if err != nil { + return sftp.ErrSSHFxFailure + } + return nil +} diff --git a/sftpd/internal_test.go b/sftpd/internal_test.go index fea3677c..d7c435c7 100644 --- a/sftpd/internal_test.go +++ b/sftpd/internal_test.go @@ -191,11 +191,39 @@ func TestSFTPCmdTargetPath(t *testing.T) { User: u, } _, err := connection.getSFTPCmdTargetPath("invalid_path") - if err != sftp.ErrSSHFxOpUnsupported { + if err != sftp.ErrSSHFxNoSuchFile { t.Errorf("getSFTPCmdTargetPath must fal with the expected error: %v", err) } } +func TestGetSFTPErrorFromOSError(t *testing.T) { + err := os.ErrNotExist + err = getSFTPErrorFromOSError(err) + if err != sftp.ErrSSHFxNoSuchFile { + t.Errorf("unexpected error: %v", err) + } + err = os.ErrPermission + err = getSFTPErrorFromOSError(err) + if err != sftp.ErrSSHFxPermissionDenied { + t.Errorf("unexpected error: %v", err) + } + err = getSFTPErrorFromOSError(nil) + if err != nil { + t.Errorf("unexpected error: %v", err) + } +} + +func TestSetstatModeIgnore(t *testing.T) { + originalMode := setstatMode + setstatMode = 1 + connection := Connection{} + err := connection.handleSFTPSetstat("invalid", nil) + if err != nil { + t.Errorf("unexpected error: %v setstat should be silently ignore in mode 1", err) + } + setstatMode = originalMode +} + func TestSFTPGetUsedQuota(t *testing.T) { u := dataprovider.User{} u.HomeDir = "home_rel_path" diff --git a/sftpd/scp.go b/sftpd/scp.go index a3190384..f00401a5 100644 --- a/sftpd/scp.go +++ b/sftpd/scp.go @@ -601,14 +601,14 @@ func (c *scpCommand) sendProtocolMessage(message string) error { func (c *scpCommand) sendExitStatus(err error) { status := uint32(0) if err != nil { - status = 1 + status = uint32(1) } - ex := exitStatusMsg{ + exitStatus := sshSubsystemExitStatus{ Status: status, } c.connection.Log(logger.LevelDebug, logSenderSCP, "send exit status for command with args: %v user: %v err: %v", c.args, c.connection.User.Username, err) - c.connection.channel.SendRequest("exit-status", false, ssh.Marshal(&ex)) + c.connection.channel.SendRequest("exit-status", false, ssh.Marshal(&exitStatus)) c.connection.channel.Close() } diff --git a/sftpd/server.go b/sftpd/server.go index bbb236ed..12c77c6a 100644 --- a/sftpd/server.go +++ b/sftpd/server.go @@ -30,10 +30,6 @@ const defaultPrivateKeyName = "id_rsa" var sftpExtensions = []string{"posix-rename@openssh.com"} -type exitStatusMsg struct { - Status uint32 -} - // Configuration for the SFTP server type Configuration struct { // Identification string used by the server @@ -84,6 +80,9 @@ type Configuration struct { // LoginBannerFile the contents of the specified file, if any, are sent to // the remote user before authentication is allowed. LoginBannerFile string `json:"login_banner_file" mapstructure:"login_banner_file"` + // SetstatMode 0 means "normal mode": requests for changing permissions and owner/group are executed. + // 1 means "ignore mode": requests for changing permissions and owner/group are silently ignored. + SetstatMode int `json:"setstat_mode" mapstructure:"setstat_mode"` } // Key contains information about host keys @@ -169,6 +168,7 @@ func (c Configuration) Initialize(configDir string) error { actions = c.Actions uploadMode = c.UploadMode + setstatMode = c.SetstatMode logger.Info(logSender, "", "server listener registered address: %v", listener.Addr().String()) if c.IdleTimeout > 0 { startIdleTimer(time.Duration(c.IdleTimeout) * time.Minute) @@ -333,12 +333,10 @@ func (c Configuration) handleSftpConnection(channel ssh.Channel, connection Conn server := sftp.NewRequestServer(channel, handler) if err := server.Serve(); err == io.EOF { - connection.Log(logger.LevelDebug, logSender, "connection closed, sending exit-status") - ex := exitStatusMsg{ - Status: 0, - } - _, err = channel.SendRequest("exit-status", false, ssh.Marshal(&ex)) - connection.Log(logger.LevelDebug, logSender, "send exit status error: %v", err) + connection.Log(logger.LevelDebug, logSender, "connection closed, sending exit status") + exitStatus := sshSubsystemExitStatus{Status: uint32(0)} + _, err = channel.SendRequest("exit-status", false, ssh.Marshal(&exitStatus)) + connection.Log(logger.LevelDebug, logSender, "sent exit status %+v error: %v", exitStatus, err) server.Close() } else if err != nil { connection.Log(logger.LevelWarn, logSender, "connection closed with error: %v", err) @@ -357,13 +355,13 @@ func (c Configuration) createHandler(connection Connection) sftp.Handlers { func loginUser(user dataprovider.User, loginType string) (*ssh.Permissions, error) { if !filepath.IsAbs(user.HomeDir) { - logger.Warn(logSender, "", "user %v has invalid home dir: %#v. Home dir must be an absolute path, login not allowed", + logger.Warn(logSender, "", "user %#v has an invalid home dir: %#v. Home dir must be an absolute path, login not allowed", user.Username, user.HomeDir) - return nil, fmt.Errorf("cannot login user with invalid home dir: %v", user.HomeDir) + return nil, fmt.Errorf("cannot login user with invalid home dir: %#v", user.HomeDir) } if _, err := os.Stat(user.HomeDir); os.IsNotExist(err) { err := os.MkdirAll(user.HomeDir, 0777) - logger.Debug(logSender, "", "home directory %#v for user %v does not exist, try to create, mkdir error: %v", + logger.Debug(logSender, "", "home directory %#v for user %#v does not exist, try to create, mkdir error: %v", user.HomeDir, user.Username, err) if err == nil { utils.SetPathPermissions(user.HomeDir, user.GetUID(), user.GetGID()) @@ -373,7 +371,7 @@ func loginUser(user dataprovider.User, loginType string) (*ssh.Permissions, erro if user.MaxSessions > 0 { activeSessions := getActiveSessions(user.Username) if activeSessions >= user.MaxSessions { - logger.Debug(logSender, "", "authentication refused for user: %v, too many open sessions: %v/%v", user.Username, + logger.Debug(logSender, "", "authentication refused for user: %#v, too many open sessions: %v/%v", user.Username, activeSessions, user.MaxSessions) return nil, fmt.Errorf("too many open sessions: %v", activeSessions) } diff --git a/sftpd/sftpd.go b/sftpd/sftpd.go index 2fab74d4..91e57da1 100644 --- a/sftpd/sftpd.go +++ b/sftpd/sftpd.go @@ -29,6 +29,8 @@ const ( mkdirLogSender = "Mkdir" symlinkLogSender = "Symlink" removeLogSender = "Remove" + chownLogSender = "Chown" + chmodLogSender = "Chmod" operationDownload = "download" operationUpload = "upload" operationDelete = "delete" @@ -54,6 +56,7 @@ var ( dataProvider dataprovider.Provider actions Actions uploadMode int + setstatMode int ) type connectionTransfer struct { @@ -103,6 +106,10 @@ type ConnectionStatus struct { Transfers []connectionTransfer `json:"active_transfers"` } +type sshSubsystemExitStatus struct { + Status uint32 +} + func init() { openConnections = make(map[string]Connection) idleConnectionTicker = time.NewTicker(5 * time.Minute) diff --git a/sftpd/sftpd_test.go b/sftpd/sftpd_test.go index ad758ad5..28711821 100644 --- a/sftpd/sftpd_test.go +++ b/sftpd/sftpd_test.go @@ -14,6 +14,7 @@ import ( "path" "path/filepath" "runtime" + "strings" "testing" "time" @@ -415,7 +416,7 @@ func TestRemove(t *testing.T) { } err = client.RemoveDirectory(path.Join("/test", testFileName)) if err == nil { - t.Errorf("remove directory as file must fail") + t.Errorf("remove a file with rmdir must fail") } err = client.Remove(path.Join("/test", testFileName)) if err != nil { @@ -498,9 +499,6 @@ func TestLink(t *testing.T) { func TestStat(t *testing.T) { usePubKey := false 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) @@ -517,15 +515,16 @@ func TestStat(t *testing.T) { if err != nil { t.Errorf("file upload error: %v", err) } - fi, err := client.Lstat(testFileName) + _, err := client.Lstat(testFileName) if err != nil { t.Errorf("stat error: %v", err) } - err = client.Chown(testFileName, 1000, 1000) + err = client.Chown(testFileName, os.Getuid(), os.Getgid()) if err != nil { t.Errorf("chown error: %v", err) } - err = client.Chmod(testFileName, 0600) + newPerm := os.FileMode(0600) + err = client.Chmod(testFileName, newPerm) if err != nil { t.Errorf("chmod error: %v", err) } @@ -533,8 +532,8 @@ func TestStat(t *testing.T) { if err != nil { t.Errorf("stat error: %v", err) } - if fi.Mode().Perm() != newFi.Mode().Perm() { - t.Errorf("stat must remain unchanged") + if newPerm != newFi.Mode().Perm() { + t.Errorf("chown failed expected: %v, actual: %v", newPerm, newFi.Mode().Perm()) } _, err = client.ReadLink(testFileName) if err == nil { @@ -544,11 +543,21 @@ func TestStat(t *testing.T) { if err != nil { t.Errorf("error removing uploaded file: %v", err) } + // l'errore viene riconvertito da sftp.ErrSSHFxNoSuchFile in os.ErrNotExist + err = client.Chmod(testFileName, newPerm) + if err != os.ErrNotExist { + t.Errorf("unexpected chmod error: %v expected: %v", err, os.ErrNotExist) + } + err = client.Chown(testFileName, os.Getuid(), os.Getgid()) + if err != os.ErrNotExist { + t.Errorf("unexpected chown error: %v expected: %v", err, os.ErrNotExist) + } + err = client.Chtimes(testFileName, time.Now(), time.Now()) + if err != nil { + t.Errorf("chtime must be silently ignored: %v", err) + } } _, err = httpd.RemoveUser(user, http.StatusOK) - if err != nil { - t.Errorf("unable to remove user: %v", err) - } os.RemoveAll(user.GetHomeDir()) } @@ -1274,12 +1283,22 @@ func TestOpenError(t *testing.T) { if err == nil { t.Errorf("upload must fail if we have no filesystem write permissions") } + err = client.Mkdir("test") + if err != nil { + t.Errorf("error making dir: %v", err) + } os.Chmod(user.GetHomeDir(), 0000) _, err = client.Lstat(testFileName) if err == nil { t.Errorf("file stat must fail if we have no filesystem read permissions") } os.Chmod(user.GetHomeDir(), 0755) + os.Chmod(filepath.Join(user.GetHomeDir(), "test"), 0000) + err = client.Rename(testFileName, path.Join("test", testFileName)) + if err == nil || !strings.Contains(err.Error(), sftp.ErrSSHFxPermissionDenied.Error()) { + t.Errorf("unexpected error: %v expected: %v", err, sftp.ErrSSHFxPermissionDenied) + } + os.Chmod(filepath.Join(user.GetHomeDir(), "test"), 0755) } _, err = httpd.RemoveUser(user, http.StatusOK) if err != nil { @@ -1509,7 +1528,7 @@ func TestPermList(t *testing.T) { usePubKey := true u := getTestUser(usePubKey) u.Permissions = []string{dataprovider.PermDownload, dataprovider.PermUpload, dataprovider.PermDelete, dataprovider.PermRename, - dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite} + dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite, dataprovider.PermChmod, dataprovider.PermChown} user, _, err := httpd.AddUser(u, http.StatusOK) if err != nil { t.Errorf("unable to add user: %v", err) @@ -1539,7 +1558,7 @@ func TestPermDownload(t *testing.T) { usePubKey := true u := getTestUser(usePubKey) u.Permissions = []string{dataprovider.PermListItems, dataprovider.PermUpload, dataprovider.PermDelete, dataprovider.PermRename, - dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite} + dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite, dataprovider.PermChmod, dataprovider.PermChown} user, _, err := httpd.AddUser(u, http.StatusOK) if err != nil { t.Errorf("unable to add user: %v", err) @@ -1581,7 +1600,7 @@ func TestPermUpload(t *testing.T) { usePubKey := false u := getTestUser(usePubKey) u.Permissions = []string{dataprovider.PermListItems, dataprovider.PermDownload, dataprovider.PermDelete, dataprovider.PermRename, - dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite} + dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite, dataprovider.PermChmod, dataprovider.PermChown} user, _, err := httpd.AddUser(u, http.StatusOK) if err != nil { t.Errorf("unable to add user: %v", err) @@ -1614,7 +1633,7 @@ func TestPermOverwrite(t *testing.T) { usePubKey := false u := getTestUser(usePubKey) u.Permissions = []string{dataprovider.PermListItems, dataprovider.PermDownload, dataprovider.PermUpload, dataprovider.PermDelete, - dataprovider.PermRename, dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks} + dataprovider.PermRename, dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermChmod, dataprovider.PermChown} user, _, err := httpd.AddUser(u, http.StatusOK) if err != nil { t.Errorf("unable to add user: %v", err) @@ -1651,7 +1670,7 @@ func TestPermDelete(t *testing.T) { usePubKey := false u := getTestUser(usePubKey) u.Permissions = []string{dataprovider.PermListItems, dataprovider.PermDownload, dataprovider.PermUpload, dataprovider.PermRename, - dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite} + dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite, dataprovider.PermChmod, dataprovider.PermChown} user, _, err := httpd.AddUser(u, http.StatusOK) if err != nil { t.Errorf("unable to add user: %v", err) @@ -1688,7 +1707,7 @@ func TestPermRename(t *testing.T) { usePubKey := false u := getTestUser(usePubKey) u.Permissions = []string{dataprovider.PermListItems, dataprovider.PermDownload, dataprovider.PermUpload, dataprovider.PermDelete, - dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite} + dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite, dataprovider.PermChmod, dataprovider.PermChown} user, _, err := httpd.AddUser(u, http.StatusOK) if err != nil { t.Errorf("unable to add user: %v", err) @@ -1729,7 +1748,7 @@ func TestPermCreateDirs(t *testing.T) { usePubKey := false u := getTestUser(usePubKey) u.Permissions = []string{dataprovider.PermListItems, dataprovider.PermDownload, dataprovider.PermUpload, dataprovider.PermDelete, - dataprovider.PermRename, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite} + dataprovider.PermRename, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite, dataprovider.PermChmod, dataprovider.PermChown} user, _, err := httpd.AddUser(u, http.StatusOK) if err != nil { t.Errorf("unable to add user: %v", err) @@ -1755,7 +1774,7 @@ func TestPermSymlink(t *testing.T) { usePubKey := false u := getTestUser(usePubKey) u.Permissions = []string{dataprovider.PermListItems, dataprovider.PermDownload, dataprovider.PermUpload, dataprovider.PermDelete, - dataprovider.PermRename, dataprovider.PermCreateDirs, dataprovider.PermOverwrite} + dataprovider.PermRename, dataprovider.PermCreateDirs, dataprovider.PermOverwrite, dataprovider.PermChmod, dataprovider.PermChown} user, _, err := httpd.AddUser(u, http.StatusOK) if err != nil { t.Errorf("unable to add user: %v", err) @@ -1792,6 +1811,89 @@ func TestPermSymlink(t *testing.T) { os.RemoveAll(user.GetHomeDir()) } +func TestPermChmod(t *testing.T) { + usePubKey := false + u := getTestUser(usePubKey) + u.Permissions = []string{dataprovider.PermListItems, dataprovider.PermDownload, dataprovider.PermUpload, dataprovider.PermDelete, + dataprovider.PermRename, dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite, + dataprovider.PermChown} + user, _, err := httpd.AddUser(u, 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() + 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, testFileName, testFileSize, client) + if err != nil { + t.Errorf("file upload error: %v", err) + } + err = client.Chmod(testFileName, 0666) + if err == nil { + t.Errorf("chmod without permission should not succeed") + } + err = client.Remove(testFileName) + if err != nil { + t.Errorf("error removing uploaded file: %v", err) + } + } + _, err = httpd.RemoveUser(user, http.StatusOK) + if err != nil { + t.Errorf("unable to remove user: %v", err) + } + os.RemoveAll(user.GetHomeDir()) +} + +func TestPermChown(t *testing.T) { + usePubKey := false + u := getTestUser(usePubKey) + u.Permissions = []string{dataprovider.PermListItems, dataprovider.PermDownload, dataprovider.PermUpload, dataprovider.PermDelete, + dataprovider.PermRename, dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite, + dataprovider.PermChmod} + user, _, err := httpd.AddUser(u, 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() + 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, testFileName, testFileSize, client) + if err != nil { + t.Errorf("file upload error: %v", err) + } + err = client.Chown(testFileName, 1000, 1000) + if err == nil { + t.Errorf("chown without permission should not succeed") + } + err = client.Remove(testFileName) + if err != nil { + t.Errorf("error removing uploaded file: %v", err) + } + } + _, err = httpd.RemoveUser(user, http.StatusOK) + if err != nil { + t.Errorf("unable to remove user: %v", err) + } + os.RemoveAll(user.GetHomeDir()) +} func TestSSHConnection(t *testing.T) { usePubKey := false user, _, err := httpd.AddUser(getTestUser(usePubKey), http.StatusOK) diff --git a/sftpgo.json b/sftpgo.json index 428e9f73..d6486c39 100644 --- a/sftpgo.json +++ b/sftpgo.json @@ -17,7 +17,8 @@ "kex_algorithms": [], "ciphers": [], "macs": [], - "login_banner_file": "" + "login_banner_file": "", + "setstat_mode": 0 }, "data_provider": { "driver": "sqlite",