From b2948a5255433608b56026e7101becf5c6a66767 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sat, 2 Aug 2025 18:49:08 +0200 Subject: [PATCH] sshd: removed Git support Git integration has been removed as it is out of scope for a file transfer solution like SFTPGo. Maintaining Git support introduces unnecessary complexity and potential security risks due to reliance on system commands. In particular, allowing Git operations could enable authorized users to upload repositories containing hooks, which might then be executed and abused. To reduce the attack surface and simplify the codebase, Git support has been fully dropped. Signed-off-by: Nicola Murino --- internal/sftpd/internal_test.go | 92 +--------- internal/sftpd/sftpd.go | 4 +- internal/sftpd/sftpd_test.go | 298 +------------------------------- 3 files changed, 5 insertions(+), 389 deletions(-) diff --git a/internal/sftpd/internal_test.go b/internal/sftpd/internal_test.go index ad65ef76..ba986961 100644 --- a/internal/sftpd/internal_test.go +++ b/internal/sftpd/internal_test.go @@ -527,14 +527,6 @@ func TestSSHCommandErrors(t *testing.T) { err = cmd.handle() assert.Error(t, err, "ssh command must fail, we are requesting an invalid path") - cmd = sshCommand{ - command: "git-receive-pack", - connection: &connection, - args: []string{"/../../testrepo"}, - } - err = cmd.handle() - assert.Error(t, err, "ssh command must fail, we are requesting an invalid path") - user = dataprovider.User{} user.Permissions = map[string][]string{ "/": {dataprovider.PermAny}, @@ -545,18 +537,8 @@ func TestSSHCommandErrors(t *testing.T) { cmd.connection.User = user _, err = cmd.connection.User.GetFilesystem("123") assert.NoError(t, err) - err = cmd.handle() - assert.EqualError(t, err, common.ErrQuotaExceeded.Error()) - cmd.connection.User.QuotaFiles = 0 - cmd.connection.User.UsedQuotaFiles = 0 - cmd.connection.User.Permissions = make(map[string][]string) - cmd.connection.User.Permissions["/"] = []string{dataprovider.PermListItems} - err = cmd.handle() - assert.EqualError(t, err, common.ErrPermissionDenied.Error()) - - cmd.connection.User.Permissions["/"] = []string{dataprovider.PermAny} - cmd.command = "invalid_command" + cmd.command = "git-receive-pack" command, err := cmd.getSystemCommand() assert.NoError(t, err) @@ -664,30 +646,6 @@ func TestCommandsWithExtensionsFilter(t *testing.T) { } _, err = cmd.getSystemCommand() assert.EqualError(t, err, errUnsupportedConfig.Error()) - - cmd = sshCommand{ - command: "git-receive-pack", - connection: connection, - args: []string{"/subdir"}, - } - _, err = cmd.getSystemCommand() - assert.EqualError(t, err, errUnsupportedConfig.Error()) - - cmd = sshCommand{ - command: "git-receive-pack", - connection: connection, - args: []string{"/subdir/dir"}, - } - _, err = cmd.getSystemCommand() - assert.EqualError(t, err, errUnsupportedConfig.Error()) - - cmd = sshCommand{ - command: "git-receive-pack", - connection: connection, - args: []string{"/adir/subdir"}, - } - _, err = cmd.getSystemCommand() - assert.NoError(t, err) } func TestSSHCommandsRemoteFs(t *testing.T) { @@ -777,54 +735,6 @@ func TestSSHCmdGetFsErrors(t *testing.T) { assert.NoError(t, err) } -func TestGitVirtualFolders(t *testing.T) { - permissions := make(map[string][]string) - permissions["/"] = []string{dataprovider.PermAny} - user := dataprovider.User{ - BaseUser: sdk.BaseUser{ - Permissions: permissions, - HomeDir: os.TempDir(), - }, - } - conn := &Connection{ - BaseConnection: common.NewBaseConnection("", common.ProtocolSFTP, "", "", user), - } - cmd := sshCommand{ - command: "git-receive-pack", - connection: conn, - args: []string{"/vdir"}, - } - cmd.connection.User.VirtualFolders = append(cmd.connection.User.VirtualFolders, vfs.VirtualFolder{ - BaseVirtualFolder: vfs.BaseVirtualFolder{ - MappedPath: os.TempDir(), - }, - VirtualPath: "/vdir", - }) - _, err := cmd.getSystemCommand() - assert.NoError(t, err) - cmd.args = []string{"/"} - _, err = cmd.getSystemCommand() - assert.EqualError(t, err, errUnsupportedConfig.Error()) - cmd.args = []string{"/vdir1"} - _, err = cmd.getSystemCommand() - assert.NoError(t, err) - - cmd.connection.User.VirtualFolders = nil - cmd.connection.User.VirtualFolders = append(cmd.connection.User.VirtualFolders, vfs.VirtualFolder{ - BaseVirtualFolder: vfs.BaseVirtualFolder{ - MappedPath: os.TempDir(), - }, - VirtualPath: "/vdir", - }) - cmd.args = []string{"/vdir/subdir"} - _, err = cmd.getSystemCommand() - assert.NoError(t, err) - - cmd.args = []string{"/adir/subdir"} - _, err = cmd.getSystemCommand() - assert.NoError(t, err) -} - func TestRsyncOptions(t *testing.T) { permissions := make(map[string][]string) permissions["/"] = []string{dataprovider.PermAny} diff --git a/internal/sftpd/sftpd.go b/internal/sftpd/sftpd.go index 2a6968f9..13e0a9bd 100644 --- a/internal/sftpd/sftpd.go +++ b/internal/sftpd/sftpd.go @@ -31,10 +31,10 @@ const ( var ( supportedSSHCommands = []string{"scp", "md5sum", "sha1sum", "sha256sum", "sha384sum", "sha512sum", "cd", "pwd", - "git-receive-pack", "git-upload-pack", "git-upload-archive", "rsync", "sftpgo-copy", "sftpgo-remove"} + "rsync", "sftpgo-copy", "sftpgo-remove"} defaultSSHCommands = []string{"md5sum", "sha1sum", "sha256sum", "cd", "pwd", "scp"} sshHashCommands = []string{"md5sum", "sha1sum", "sha256sum", "sha384sum", "sha512sum"} - systemCommands = []string{"git-receive-pack", "git-upload-pack", "git-upload-archive", "rsync"} + systemCommands = []string{"rsync"} serviceStatus ServiceStatus certKeyAlgoNames = map[string]string{ ssh.CertAlgoRSAv01: ssh.KeyAlgoRSA, diff --git a/internal/sftpd/sftpd_test.go b/internal/sftpd/sftpd_test.go index 76a54130..73b76985 100644 --- a/internal/sftpd/sftpd_test.go +++ b/internal/sftpd/sftpd_test.go @@ -9909,67 +9909,6 @@ func TestSSHRemoveCryptFs(t *testing.T) { assert.NoError(t, err) } -func TestBasicGitCommands(t *testing.T) { - if len(gitPath) == 0 || len(sshPath) == 0 || runtime.GOOS == osWindows { - t.Skip("git and/or ssh command not found or OS is windows, unable to execute this test") - } - usePubKey := true - u := getTestUser(usePubKey) - user, _, err := httpdtest.AddUser(u, http.StatusCreated) - assert.NoError(t, err) - - repoName := "testrepo" //nolint:goconst - clonePath := filepath.Join(homeBasePath, repoName) - err = os.RemoveAll(user.GetHomeDir()) - assert.NoError(t, err) - err = os.RemoveAll(filepath.Join(homeBasePath, repoName)) - assert.NoError(t, err) - out, err := initGitRepo(filepath.Join(user.HomeDir, repoName)) - assert.NoError(t, err, "unexpected error, out: %v", string(out)) - - out, err = cloneGitRepo(homeBasePath, "/"+repoName, user.Username) - assert.NoError(t, err, "unexpected error, out: %v", string(out)) - - out, err = addFileToGitRepo(clonePath, 128) - assert.NoError(t, err, "unexpected error, out: %v", string(out)) - - user.QuotaFiles = 100000 - _, _, err = httpdtest.UpdateUser(user, http.StatusOK, "") - assert.NoError(t, err) - - out, err = pushToGitRepo(clonePath) - if !assert.NoError(t, err, "unexpected error, out: %v", string(out)) { - printLatestLogs(10) - } - - out, err = addFileToGitRepo(clonePath, 131072) - assert.NoError(t, err, "unexpected error, out: %v", string(out)) - - user, _, err = httpdtest.GetUserByUsername(user.Username, http.StatusOK) - assert.NoError(t, err) - user.QuotaSize = user.UsedQuotaSize + 1 - _, _, err = httpdtest.UpdateUser(user, http.StatusOK, "") - assert.NoError(t, err) - out, err = pushToGitRepo(clonePath) - assert.Error(t, err, "git push must fail if quota is exceeded, out: %v", string(out)) - - aDir := filepath.Join(user.GetHomeDir(), repoName, "adir") - err = os.MkdirAll(aDir, 0001) - assert.NoError(t, err) - _, err = pushToGitRepo(clonePath) - assert.Error(t, err) - err = os.Chmod(aDir, os.ModePerm) - assert.NoError(t, err) - - _, err = httpdtest.RemoveUser(user, http.StatusOK) - assert.NoError(t, err) - - err = os.RemoveAll(user.GetHomeDir()) - assert.NoError(t, err) - err = os.RemoveAll(clonePath) - assert.NoError(t, err) -} - func TestSSHCommandMaxTransfers(t *testing.T) { if len(gitPath) == 0 || len(sshPath) == 0 || runtime.GOOS == osWindows { t.Skip("git and/or ssh command not found or OS is windows, unable to execute this test") @@ -9988,8 +9927,7 @@ func TestSSHCommandMaxTransfers(t *testing.T) { assert.NoError(t, err) err = os.RemoveAll(filepath.Join(homeBasePath, repoName)) assert.NoError(t, err) - out, err := initGitRepo(filepath.Join(user.HomeDir, repoName)) - assert.NoError(t, err, "unexpected error, out: %v", string(out)) + conn, client, err := getSftpClient(user, usePubKey) if assert.NoError(t, err) { f1, err := client.Create("file1") @@ -10001,7 +9939,7 @@ func TestSSHCommandMaxTransfers(t *testing.T) { _, err = f2.Write([]byte(" ")) assert.NoError(t, err) - _, err = cloneGitRepo(homeBasePath, "/"+repoName, user.Username) + _, err = client.Create("file3") assert.Error(t, err) err = f1.Close() @@ -10026,180 +9964,6 @@ func TestSSHCommandMaxTransfers(t *testing.T) { common.Config.MaxPerHostConnections = oldValue } -func TestGitIncludedVirtualFolders(t *testing.T) { - if len(gitPath) == 0 || len(sshPath) == 0 || runtime.GOOS == osWindows { - t.Skip("git and/or ssh command not found or OS is windows, unable to execute this test") - } - usePubKey := true - repoName := "trepo" - u := getTestUser(usePubKey) - u.QuotaFiles = 10000 - mappedPath := filepath.Join(os.TempDir(), "repo") - folderName := filepath.Base(mappedPath) - u.VirtualFolders = append(u.VirtualFolders, vfs.VirtualFolder{ - BaseVirtualFolder: vfs.BaseVirtualFolder{ - Name: folderName, - }, - VirtualPath: "/" + repoName, - QuotaFiles: -1, - QuotaSize: -1, - }) - f := vfs.BaseVirtualFolder{ - Name: folderName, - MappedPath: mappedPath, - } - _, _, err := httpdtest.AddFolder(f, http.StatusCreated) - assert.NoError(t, err) - user, _, err := httpdtest.AddUser(u, http.StatusCreated) - assert.NoError(t, err) - - clonePath := filepath.Join(homeBasePath, repoName) - err = os.RemoveAll(user.GetHomeDir()) - assert.NoError(t, err) - err = os.RemoveAll(filepath.Join(homeBasePath, repoName)) - assert.NoError(t, err) - out, err := initGitRepo(mappedPath) - assert.NoError(t, err, "unexpected error, out: %v", string(out)) - - out, err = cloneGitRepo(homeBasePath, "/"+repoName, user.Username) - assert.NoError(t, err, "unexpected error, out: %v", string(out)) - - out, err = addFileToGitRepo(clonePath, 128) - assert.NoError(t, err, "unexpected error, out: %v", string(out)) - - out, err = pushToGitRepo(clonePath) - assert.NoError(t, err, "unexpected error, out: %v", string(out)) - - user, _, err = httpdtest.GetUserByUsername(user.Username, http.StatusOK) - assert.NoError(t, err) - if user.UsedQuotaFiles == 0 { - assert.Eventually(t, func() bool { - user, _, err = httpdtest.GetUserByUsername(user.Username, http.StatusOK) - if err != nil { - return false - } - return user.QuotaFiles > 0 - }, 1*time.Second, 100*time.Millisecond) - } - user, _, err = httpdtest.GetUserByUsername(user.Username, http.StatusOK) - assert.NoError(t, err) - assert.Greater(t, user.UsedQuotaFiles, 0) - assert.Greater(t, user.UsedQuotaSize, int64(0)) - - folder, _, err := httpdtest.GetFolderByName(folderName, http.StatusOK) - assert.NoError(t, err) - assert.Equal(t, 0, folder.UsedQuotaFiles) - assert.Equal(t, int64(0), folder.UsedQuotaSize) - - _, err = httpdtest.RemoveUser(user, http.StatusOK) - assert.NoError(t, err) - err = os.RemoveAll(user.GetHomeDir()) - assert.NoError(t, err) - _, err = httpdtest.RemoveFolder(vfs.BaseVirtualFolder{Name: folderName}, http.StatusOK) - assert.NoError(t, err) - err = os.RemoveAll(mappedPath) - assert.NoError(t, err) - err = os.RemoveAll(clonePath) - assert.NoError(t, err) -} - -func TestGitQuotaVirtualFolders(t *testing.T) { - if len(gitPath) == 0 || len(sshPath) == 0 || runtime.GOOS == osWindows { - t.Skip("git and/or ssh command not found or OS is windows, unable to execute this test") - } - usePubKey := true - repoName := "testrepo" - u := getTestUser(usePubKey) - u.QuotaFiles = 1 - u.QuotaSize = 131072 - mappedPath := filepath.Join(os.TempDir(), "repo") - folderName := filepath.Base(mappedPath) - u.VirtualFolders = append(u.VirtualFolders, vfs.VirtualFolder{ - BaseVirtualFolder: vfs.BaseVirtualFolder{ - Name: folderName, - }, - VirtualPath: "/" + repoName, - QuotaFiles: 0, - QuotaSize: 0, - }) - f := vfs.BaseVirtualFolder{ - Name: folderName, - MappedPath: mappedPath, - } - _, _, err := httpdtest.AddFolder(f, http.StatusCreated) - assert.NoError(t, err) - err = os.MkdirAll(mappedPath, os.ModePerm) - assert.NoError(t, err) - user, _, err := httpdtest.AddUser(u, http.StatusCreated) - assert.NoError(t, err) - conn, client, err := getSftpClient(user, usePubKey) - if assert.NoError(t, err) { - // we upload a file so the user is over quota - defer conn.Close() - defer client.Close() - testFilePath := filepath.Join(homeBasePath, testFileName) - err = createTestFile(testFilePath, u.QuotaSize) - assert.NoError(t, err) - err = sftpUploadFile(testFilePath, testFileName, u.QuotaSize, client) - assert.NoError(t, err) - err = os.Remove(testFilePath) - assert.NoError(t, err) - } - - clonePath := filepath.Join(homeBasePath, repoName) - err = os.RemoveAll(user.GetHomeDir()) - assert.NoError(t, err) - err = os.RemoveAll(filepath.Join(homeBasePath, repoName)) - assert.NoError(t, err) - out, err := initGitRepo(mappedPath) - assert.NoError(t, err, "unexpected error, out: %v", string(out)) - - out, err = cloneGitRepo(homeBasePath, "/"+repoName, user.Username) - assert.NoError(t, err, "unexpected error, out: %v", string(out)) - - out, err = addFileToGitRepo(clonePath, 128) - assert.NoError(t, err, "unexpected error, out: %v", string(out)) - - out, err = pushToGitRepo(clonePath) - assert.NoError(t, err, "unexpected error, out: %v", string(out)) - - _, err = httpdtest.RemoveUser(user, http.StatusOK) - assert.NoError(t, err) - err = os.RemoveAll(user.GetHomeDir()) - assert.NoError(t, err) - _, err = httpdtest.RemoveFolder(vfs.BaseVirtualFolder{Name: folderName}, http.StatusOK) - assert.NoError(t, err) - err = os.RemoveAll(mappedPath) - assert.NoError(t, err) - err = os.RemoveAll(clonePath) - assert.NoError(t, err) -} - -func TestGitErrors(t *testing.T) { - if len(gitPath) == 0 || len(sshPath) == 0 || runtime.GOOS == osWindows { - t.Skip("git and/or ssh command not found or OS is windows, unable to execute this test") - } - usePubKey := true - u := getTestUser(usePubKey) - user, _, err := httpdtest.AddUser(u, http.StatusCreated) - assert.NoError(t, err) - repoName := "testrepo" - clonePath := filepath.Join(homeBasePath, repoName) - err = os.RemoveAll(user.GetHomeDir()) - assert.NoError(t, err) - err = os.RemoveAll(filepath.Join(homeBasePath, repoName)) - assert.NoError(t, err) - out, err := cloneGitRepo(homeBasePath, "/"+repoName, user.Username) - assert.Error(t, err, "cloning a missing repo must fail, out: %v", string(out)) - - _, err = httpdtest.RemoveUser(user, http.StatusOK) - assert.NoError(t, err) - err = os.RemoveAll(user.GetHomeDir()) - assert.NoError(t, err) - err = os.RemoveAll(clonePath) - assert.NoError(t, err) -} - // Start SCP tests func TestSCPBasicHandling(t *testing.T) { if scpPath == "" { @@ -11807,64 +11571,6 @@ func checkSystemCommands() { } } -func initGitRepo(path string) ([]byte, error) { - err := os.MkdirAll(path, os.ModePerm) - if err != nil { - return nil, err - } - args := []string{"init", "--bare"} - cmd := exec.Command(gitPath, args...) - cmd.Dir = path - return cmd.CombinedOutput() -} - -func pushToGitRepo(repoPath string) ([]byte, error) { - cmd := exec.Command(gitPath, "push") - cmd.Dir = repoPath - cmd.Env = append(os.Environ(), - fmt.Sprintf("GIT_SSH=%v", gitWrapPath)) - return cmd.CombinedOutput() -} - -func cloneGitRepo(basePath, remotePath, username string) ([]byte, error) { - remoteURL := fmt.Sprintf("ssh://%v@127.0.0.1:2022%v", username, remotePath) - args := []string{"clone", remoteURL} - cmd := exec.Command(gitPath, args...) - cmd.Dir = basePath - cmd.Env = append(os.Environ(), - fmt.Sprintf("GIT_SSH=%v", gitWrapPath)) - return cmd.CombinedOutput() -} - -func addFileToGitRepo(repoPath string, fileSize int64) ([]byte, error) { - path := filepath.Join(repoPath, "test") - err := createTestFile(path, fileSize) - if err != nil { - return []byte(""), err - } - cmd := exec.Command(gitPath, "config", "user.email", "testuser@example.com") - cmd.Dir = repoPath - out, err := cmd.CombinedOutput() - if err != nil { - return out, err - } - cmd = exec.Command(gitPath, "config", "user.name", "testuser") - cmd.Dir = repoPath - out, err = cmd.CombinedOutput() - if err != nil { - return out, err - } - cmd = exec.Command(gitPath, "add", "test") - cmd.Dir = repoPath - out, err = cmd.CombinedOutput() - if err != nil { - return out, err - } - cmd = exec.Command(gitPath, "commit", "-am", "test") - cmd.Dir = repoPath - return cmd.CombinedOutput() -} - func getKeyboardInteractiveScriptForBuiltinChecks(addPasscode bool, result int) []byte { content := []byte("#!/bin/sh\n\n") echos := []bool{false}