From ed43ddd79d4a72866f22bebce0e3c77fe9facb58 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sun, 13 Dec 2020 15:11:55 +0100 Subject: [PATCH] enable hash commands for any supported backend --- docs/ssh-commands.md | 2 +- sftpd/internal_test.go | 2 -- sftpd/sftpd_test.go | 78 ++++++++++++++++++++++++------------------ sftpd/ssh_cmd.go | 21 +++++++----- 4 files changed, 58 insertions(+), 45 deletions(-) diff --git a/docs/ssh-commands.md b/docs/ssh-commands.md index e763bfb9..40658c79 100644 --- a/docs/ssh-commands.md +++ b/docs/ssh-commands.md @@ -34,7 +34,7 @@ SFTPGo supports the following built-in SSH commands: - `scp`, SFTPGo implements the SCP protocol so we can support it for cloud filesystems too and we can avoid the other system commands limitations. SCP between two remote hosts is supported using the `-3` scp option. - `md5sum`, `sha1sum`, `sha256sum`, `sha384sum`, `sha512sum`. Useful to check message digests for uploaded files. -- `cd`, `pwd`. Some SFTP clients do not support the SFTP SSH_FXP_REALPATH packet type, so they use `cd` and `pwd` SSH commands to get the initial directory. Currently `cd` does nothing and `pwd` always returns the `/` path. +- `cd`, `pwd`. Some SFTP clients do not support the SFTP SSH_FXP_REALPATH packet type, so they use `cd` and `pwd` SSH commands to get the initial directory. Currently `cd` does nothing and `pwd` always returns the `/` path. These commands will work with any storage backend but keep in mind that to calculate the hash we need to read the whole file, for remote backends this means downloading the file, for the encrypted backend this means decrypting the file. - `sftpgo-copy`. This is a built-in copy implementation. It allows server side copy for files and directories. The first argument is the source file/directory and the second one is the destination file/directory, for example `sftpgo-copy `. The command will fail if the destination exists. Copy for directories spanning virtual folders is not supported. Only local filesystem is supported: recursive copy for Cloud Storage filesystems requires a new request for every file in any case, so a real server side copy is not possible. - `sftpgo-remove`. This is a built-in remove implementation. It allows to remove single files and to recursively remove directories. The first argument is the file/directory to remove, for example `sftpgo-remove `. Only local filesystem is supported: recursive remove for Cloud Storage filesystems requires a new request for every file in any case, so a server side remove is not possible. diff --git a/sftpd/internal_test.go b/sftpd/internal_test.go index 6616661a..35448bc1 100644 --- a/sftpd/internal_test.go +++ b/sftpd/internal_test.go @@ -740,8 +740,6 @@ func TestSSHCommandsRemoteFs(t *testing.T) { connection: connection, args: []string{}, } - err = cmd.handleHashCommands() - assert.Error(t, err, "command must fail for a non local filesystem") command, err := cmd.getSystemCommand() assert.NoError(t, err) diff --git a/sftpd/sftpd_test.go b/sftpd/sftpd_test.go index ef95795f..dfb48595 100644 --- a/sftpd/sftpd_test.go +++ b/sftpd/sftpd_test.go @@ -6438,44 +6438,56 @@ func TestSSHCommands(t *testing.T) { func TestSSHFileHash(t *testing.T) { usePubKey := true - user, _, err := httpd.AddUser(getTestUser(usePubKey), http.StatusOK) + localUser, _, err := httpd.AddUser(getTestUser(usePubKey), http.StatusOK) assert.NoError(t, err) - client, err := getSftpClient(user, usePubKey) - if assert.NoError(t, err) { - defer client.Close() - testFilePath := filepath.Join(homeBasePath, testFileName) - testFileSize := int64(65535) - err = createTestFile(testFilePath, testFileSize) - assert.NoError(t, err) - err = sftpUploadFile(testFilePath, testFileName, testFileSize, client) - assert.NoError(t, err) - user.Permissions = make(map[string][]string) - user.Permissions["/"] = []string{dataprovider.PermUpload} - _, _, err = httpd.UpdateUser(user, http.StatusOK, "") - assert.NoError(t, err) - _, err = runSSHCommand("sha512sum "+testFileName, user, usePubKey) - assert.Error(t, err, "hash command with no list permission must fail") - - user.Permissions["/"] = []string{dataprovider.PermAny} - _, _, err = httpd.UpdateUser(user, http.StatusOK, "") - assert.NoError(t, err) - - initialHash, err := computeHashForFile(sha512.New(), testFilePath) - assert.NoError(t, err) - - out, err := runSSHCommand("sha512sum "+testFileName, user, usePubKey) + sftpUser, _, err := httpd.AddUser(getTestSFTPUser(usePubKey), http.StatusOK) + assert.NoError(t, err) + u := getTestUserWithCryptFs(usePubKey) + u.Username = u.Username + "_crypt" + cryptUser, _, err := httpd.AddUser(u, http.StatusOK) + assert.NoError(t, err) + for _, user := range []dataprovider.User{localUser, sftpUser, cryptUser} { + client, err := getSftpClient(user, usePubKey) if assert.NoError(t, err) { - assert.Contains(t, string(out), initialHash) - } - _, err = runSSHCommand("sha512sum invalid_path", user, usePubKey) - assert.Error(t, err, "hash for an invalid path must fail") + defer client.Close() + testFilePath := filepath.Join(homeBasePath, testFileName) + testFileSize := int64(65535) + err = createTestFile(testFilePath, testFileSize) + assert.NoError(t, err) + err = sftpUploadFile(testFilePath, testFileName, testFileSize, client) + assert.NoError(t, err) + user.Permissions = make(map[string][]string) + user.Permissions["/"] = []string{dataprovider.PermUpload} + _, _, err = httpd.UpdateUser(user, http.StatusOK, "") + assert.NoError(t, err) + _, err = runSSHCommand("sha512sum "+testFileName, user, usePubKey) + assert.Error(t, err, "hash command with no list permission must fail") - err = os.Remove(testFilePath) - assert.NoError(t, err) + user.Permissions["/"] = []string{dataprovider.PermAny} + _, _, err = httpd.UpdateUser(user, http.StatusOK, "") + assert.NoError(t, err) + + initialHash, err := computeHashForFile(sha512.New(), testFilePath) + assert.NoError(t, err) + + out, err := runSSHCommand("sha512sum "+testFileName, user, usePubKey) + if assert.NoError(t, err) { + assert.Contains(t, string(out), initialHash) + } + _, err = runSSHCommand("sha512sum invalid_path", user, usePubKey) + assert.Error(t, err, "hash for an invalid path must fail") + + err = os.Remove(testFilePath) + assert.NoError(t, err) + } } - _, err = httpd.RemoveUser(user, http.StatusOK) + _, err = httpd.RemoveUser(sftpUser, http.StatusOK) assert.NoError(t, err) - err = os.RemoveAll(user.GetHomeDir()) + _, err = httpd.RemoveUser(cryptUser, http.StatusOK) + assert.NoError(t, err) + _, err = httpd.RemoveUser(localUser, http.StatusOK) + assert.NoError(t, err) + err = os.RemoveAll(localUser.GetHomeDir()) assert.NoError(t, err) } diff --git a/sftpd/ssh_cmd.go b/sftpd/ssh_cmd.go index a14c9403..a008d9a2 100644 --- a/sftpd/ssh_cmd.go +++ b/sftpd/ssh_cmd.go @@ -3,7 +3,6 @@ package sftpd import ( "crypto/md5" "crypto/sha1" - "crypto/sha256" "crypto/sha512" "errors" "fmt" @@ -17,6 +16,7 @@ import ( "sync" "github.com/google/shlex" + "github.com/minio/sha256-simd" fscopy "github.com/otiai10/copy" "golang.org/x/crypto/ssh" @@ -258,9 +258,6 @@ func (c *sshCommand) updateQuota(sshDestPath string, filesNum int, filesSize int } func (c *sshCommand) handleHashCommands() error { - if !vfs.IsLocalOsFs(c.connection.Fs) { - return c.sendErrorResponse(errUnsupportedConfig) - } var h hash.Hash if c.command == "md5sum" { h = md5.New() @@ -296,7 +293,7 @@ func (c *sshCommand) handleHashCommands() error { if !c.connection.User.HasPerm(dataprovider.PermListItems, sshPath) { return c.sendErrorResponse(common.ErrPermissionDenied) } - hash, err := computeHashForFile(h, fsPath) + hash, err := c.computeHashForFile(h, fsPath) if err != nil { return c.sendErrorResponse(err) } @@ -736,14 +733,20 @@ func (c *sshCommand) sendExitStatus(err error) { } } -func computeHashForFile(hasher hash.Hash, path string) (string, error) { +func (c *sshCommand) computeHashForFile(hasher hash.Hash, path string) (string, error) { hash := "" - f, err := os.Open(path) + f, r, _, err := c.connection.Fs.Open(path, 0) if err != nil { return hash, err } - defer f.Close() - _, err = io.Copy(hasher, f) + var reader io.ReadCloser + if f != nil { + reader = f + } else { + reader = r + } + defer reader.Close() + _, err = io.Copy(hasher, reader) if err == nil { hash = fmt.Sprintf("%x", hasher.Sum(nil)) }