vfs: migrate OsFs/CryptFs to os.Root and remove temp_path

Confine all local filesystem access to an os.Root, with path resoultion
enforced by the underlying OS.

Behavior changes an upgrading deployment may notice:

- temp_path is removed and ignored if still configured. Its purpose
  was to stage files (atomic uploads, scratch and streaming data)
  outside the user's storage and rename them into place, and that is
  fundamentally incompatible with the confinement: an os.Root cannot
  rename a file in from outside the root, so every staged upload would
  degrade to a full copy-then-delete even when temp_path is on the
  same filesystem, defeating the very atomicity the option existed to
  provide. It was not a great idea before this change either: a
  temp_path on a different device already turned the final rename into
  a silent full copy. Staged and scratch files (the atomic upload
  temporary, the CryptFs download pipe backing file) now live in the
  storage root itself, on the same filesystem as the data, so the
  final rename stays atomic.

- Cross-folder / cross-root renames are no longer atomic. A rename
  spanning the home and a local virtual folder, or two local folders,
  falls back to copy-then-delete: slower for large data even on the
  same device, and a midway failure can leave a partial tree at the
  destination. Symlinks and other non-regular entries inside a moved
  directory are skipped, not copied, and the rename still reports
  success. File mode bits are not preserved; regular files keep their
  modification time, directories do not.

- Absolute symlinks created outside SFTPGo, or by versions predating
  this change, are no longer traversable and Readlink rejects them with
  permission denied, even when the target is inside the root: os.Root
  refuses an absolute link target regardless of where it points.
  Symlinks created through SFTPGo are stored relative to the link
  directory and keep working.

- Execute-only home directories (e.g. mode 0111) are unusable:
  os.OpenRoot requires read permission, so every operation fails with
  permission denied until the directory is made readable, where
  previously only the listing failed. Since the root open is retried on
  demand, fixing the permissions (or creating a missing home
  externally) makes the same connection work again without
  reconnecting.

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>
This commit is contained in:
Nicola Murino
2026-07-06 13:08:12 +02:00
parent b163bab867
commit 661d77f47b
29 changed files with 1110 additions and 510 deletions
-2
View File
@@ -38,7 +38,6 @@ require (
github.com/mattn/go-sqlite3 v1.14.47
github.com/mhale/smtpd v0.8.3
github.com/minio/sio v0.5.1
github.com/otiai10/copy v1.14.1
github.com/pires/go-proxyproto v0.12.0
github.com/pkg/sftp v1.13.10
github.com/pquerna/otp v1.5.0
@@ -136,7 +135,6 @@ require (
github.com/miekg/dns v1.1.72 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/oklog/run v1.2.0 // indirect
github.com/otiai10/mint v1.6.3 // indirect
github.com/pelletier/go-toml/v2 v2.4.2 // indirect
github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect
github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect
-4
View File
@@ -254,10 +254,6 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
github.com/oklog/run v1.2.0 h1:O8x3yXwah4A73hJdlrwo/2X6J62gE5qTMusH0dvz60E=
github.com/oklog/run v1.2.0/go.mod h1:mgDbKRSwPhJfesJ4PntqFUbKQRZ50NgmZTSPlFA0YFk=
github.com/otiai10/copy v1.14.1 h1:5/7E6qsUMBaH5AnQ0sSLzzTg1oTECmcCmT6lvF45Na8=
github.com/otiai10/copy v1.14.1/go.mod h1:oQwrEDDOci3IM8dJF0d8+jnbfPDllW6vUjNc3DoZm9I=
github.com/otiai10/mint v1.6.3 h1:87qsV/aw1F5as1eH1zS/yqHY85ANKVMgkDrf9rcxbQs=
github.com/otiai10/mint v1.6.3/go.mod h1:MJm72SBthJjz8qhefc4z1PYEieWmy8Bku7CjcAqyUSM=
github.com/pelletier/go-toml/v2 v2.4.2 h1:M2fKKbmyvI+hGId/D0W64qDBMVhJnNR10O5gIbMc//Q=
github.com/pelletier/go-toml/v2 v2.4.2/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY=
github.com/pires/go-proxyproto v0.12.0 h1:TTCxD66dU898tahivkqc3hoceZp7P44FnorWyo9d5vM=
-8
View File
@@ -254,8 +254,6 @@ func Initialize(c Configuration, isShared int) error {
if err := c.EventManager.validate(); err != nil {
return err
}
vfs.SetTempPath(c.TempPath)
dataprovider.SetTempPath(c.TempPath)
vfs.SetAllowSelfConnections(c.AllowSelfConnections)
vfs.SetRenameMode(c.RenameMode)
vfs.SetReadMetadataMode(c.Metadata.Read)
@@ -596,12 +594,6 @@ type Configuration struct {
// Set to a value greater than 0 to allow resuming uploads of files smaller than or equal to the
// defined size.
ResumeMaxSize int64 `json:"resume_max_size" mapstructure:"resume_max_size"`
// TempPath defines the path for temporary files such as those used for atomic uploads or file pipes.
// If you set this option you must make sure that the defined path exists, is accessible for writing
// by the user running SFTPGo, and is on the same filesystem as the users home directories otherwise
// the renaming for atomic uploads will become a copy and therefore may take a long time.
// The temporary files are not namespaced. The default is generally fine. Leave empty for the default.
TempPath string `json:"temp_path" mapstructure:"temp_path"`
// SecretMinEntropy defines the minimum entropy required for plain-text
// data-encryption secrets: the CryptFs passphrase and the S3 SSE-C key. These
// secrets must be random key material rather than a memorable password. The
+23 -3
View File
@@ -837,7 +837,7 @@ func (c *BaseConnection) renameInternal(virtualSourcePath, virtualTargetPath str
initialSize := int64(-1)
dstInfo, err := fsDst.Lstat(fsTargetPath)
if err != nil && !fsDst.IsNotExist(err) {
return err
return c.GetFsError(fsDst, err)
}
if err == nil {
checkParentDestination = false
@@ -872,8 +872,19 @@ func (c *BaseConnection) renameInternal(virtualSourcePath, virtualTargetPath str
defer stopKeepAlive()
files, size, err := fsDst.Rename(fsSourcePath, fsTargetPath, checks)
if errors.Is(err, vfs.ErrCrossRename) {
// The backend cannot rename as a single operation across two
// confinement roots: fall back to a non-atomic copy + delete. Symbolic
// links in the moved tree are skipped.
files, size, err = vfs.RenameAcrossRoots(fsSrc, fsDst, fsSourcePath, fsTargetPath, srcInfo,
checks, c.User.GetUID(), c.User.GetGID())
}
if err != nil {
c.Log(logger.LevelError, "failed to rename %q -> %q: %+v", fsSourcePath, fsTargetPath, err)
// Best effort quota update.
if files > 0 {
c.updateQuotaAfterRename(fsDst, virtualSourcePath, virtualTargetPath, fsTargetPath, initialSize, files, size) //nolint:errcheck
}
return c.GetFsError(fsSrc, err)
}
vfs.SetPathPermissions(fsDst, fsTargetPath, c.User.GetUID(), c.User.GetGID())
@@ -1581,7 +1592,7 @@ func (c *BaseConnection) updateQuotaMoveToVFolder(dstFolder *vfs.VirtualFolder,
dataprovider.UpdateUserFolderQuota(dstFolder, &c.User, 0, filesSize-initialSize, false)
}
func (c *BaseConnection) updateQuotaAfterRename(fs vfs.Fs, virtualSourcePath, virtualTargetPath, targetPath string,
func (c *BaseConnection) updateQuotaAfterRename(fs vfs.Fs, virtualSourcePath, virtualTargetPath, targetPath string, //nolint:gocyclo
initialSize int64, numFiles int, filesSize int64,
) error {
if dataprovider.GetQuotaTracking() == 0 {
@@ -1603,9 +1614,18 @@ func (c *BaseConnection) updateQuotaAfterRename(fs vfs.Fs, virtualSourcePath, vi
}
return nil
}
if errSrc == nil && errDst == nil && sourceFolder.Name == dstFolder.Name {
// Move within the same virtual folder, only overwritten files are to update.
// Returning here avoids the racy, best-effort, post-rename stat.
if initialSize != -1 {
dataprovider.UpdateUserFolderQuota(&sourceFolder, &c.User, -1, -initialSize, false)
}
return nil
}
if filesSize == -1 {
// fs.Rename didn't return the affected files/sizes, we need to calculate them
// fs.Rename didn't return the affected files/sizes, we need to
// calculate them (best effort, may be inaccurate).
numFiles = 1
if fi, err := fs.Stat(targetPath); err == nil {
if fi.Mode().IsDir() {
+22 -1
View File
@@ -365,7 +365,7 @@ func TestUpdateQuotaAfterRename(t *testing.T) {
assert.NoError(t, err)
err = os.MkdirAll(mappedPath, os.ModePerm)
assert.NoError(t, err)
fs, err := user.GetFilesystem("id")
fs, err := user.GetFilesystemForPath("/vdir/file", "id")
assert.NoError(t, err)
c := NewBaseConnection("", ProtocolSFTP, "", "", user)
request := sftp.NewRequest("Rename", "/testfile")
@@ -1567,3 +1567,24 @@ func TestOsFsGetRelativePath(t *testing.T) {
})
}
}
func TestOsFsLinkTargetEscapes(t *testing.T) {
linkTargetEscapes := func(target string) bool {
return filepath.IsAbs(target) || filepath.VolumeName(target) != "" ||
(len(target) > 0 && os.IsPathSeparator(target[0]))
}
assert.True(t, linkTargetEscapes("/etc/passwd"))
assert.False(t, linkTargetEscapes("target"))
assert.False(t, linkTargetEscapes("sub/target"))
assert.False(t, linkTargetEscapes("../sibling/target"))
assert.False(t, linkTargetEscapes(""))
if runtime.GOOS == osWindows {
for _, target := range []string{`\foo`, `/foo`, `C:foo`, `C:\foo`, `\\server\share\foo`} {
assert.True(t, linkTargetEscapes(target), "target %q must be treated as an escape", target)
}
for _, target := range []string{`sub\target`, `..\sibling\target`, "target"} {
assert.False(t, linkTargetEscapes(target), "relative target %q must not be treated as an escape", target)
}
}
}
+140 -11
View File
@@ -3094,6 +3094,135 @@ func TestCrossFolderRename(t *testing.T) {
}
}
func TestCryptFsCrossDomainRenameQuota(t *testing.T) {
mappedPath := filepath.Join(os.TempDir(), "vdir_crypt_quota")
folderName := filepath.Base(mappedPath)
vdirPath := "/vdir_crypt_quota"
f := vfs.BaseVirtualFolder{
Name: folderName,
MappedPath: mappedPath,
}
f.FsConfig.Provider = sdk.CryptedFilesystemProvider
f.FsConfig.CryptConfig.Passphrase = kms.NewPlainSecret(defaultPassword)
_, _, err := httpdtest.AddFolder(f, http.StatusCreated)
assert.NoError(t, err)
u := getCryptFsUser()
// the user must have quota restrictions so home uploads are tracked (per-user
// quota tracking skips unrestricted users)
u.QuotaFiles = 100
u.QuotaSize = 1 << 20
u.VirtualFolders = append(u.VirtualFolders, vfs.VirtualFolder{
BaseVirtualFolder: vfs.BaseVirtualFolder{
Name: folderName,
},
VirtualPath: vdirPath,
// separate quota domain (not included in the user quota) with room to spare
QuotaFiles: 100,
QuotaSize: 1 << 20,
})
user, _, err := httpdtest.AddUser(u, http.StatusCreated)
assert.NoError(t, err)
testFileSize := int64(len(testFileContent))
encryptedSize, err := getEncryptedFileSize(testFileSize)
assert.NoError(t, err)
conn, client, err := getSftpClient(user)
if assert.NoError(t, err) {
defer conn.Close()
defer client.Close()
subDir := "cryptdir"
err = client.Mkdir(subDir)
assert.NoError(t, err)
err = writeSFTPFile(path.Join(subDir, testFileName), testFileSize, client)
assert.NoError(t, err)
// the encrypted size is charged to the user quota domain
user, _, err = httpdtest.GetUserByUsername(user.Username, http.StatusOK)
assert.NoError(t, err)
assert.Equal(t, 1, user.UsedQuotaFiles)
assert.Equal(t, encryptedSize, user.UsedQuotaSize)
// cross-root rename of the directory into the CryptFs virtual folder domain
err = client.Rename(subDir, path.Join(vdirPath, subDir))
assert.NoError(t, err)
// the whole encrypted size must move to the folder, leaving the user empty
user, _, err = httpdtest.GetUserByUsername(user.Username, http.StatusOK)
assert.NoError(t, err)
assert.Equal(t, 0, user.UsedQuotaFiles)
assert.Equal(t, int64(0), user.UsedQuotaSize)
folder, _, err := httpdtest.GetFolderByName(folderName, http.StatusOK)
assert.NoError(t, err)
assert.Equal(t, 1, folder.UsedQuotaFiles)
assert.Equal(t, encryptedSize, 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)
}
func TestCrossRootRenamePreservesFileModTime(t *testing.T) {
u := getTestUser()
mappedPath := filepath.Join(os.TempDir(), "vdir_mtime")
folderName := filepath.Base(mappedPath)
vdirPath := "/vdir_mtime"
f := vfs.BaseVirtualFolder{
Name: folderName,
MappedPath: mappedPath,
}
_, _, err := httpdtest.AddFolder(f, http.StatusCreated)
assert.NoError(t, err)
u.VirtualFolders = append(u.VirtualFolders, vfs.VirtualFolder{
BaseVirtualFolder: vfs.BaseVirtualFolder{
Name: folderName,
},
VirtualPath: vdirPath,
QuotaFiles: -1,
QuotaSize: -1,
})
user, _, err := httpdtest.AddUser(u, http.StatusCreated)
assert.NoError(t, err)
conn, client, err := getSftpClient(user)
if assert.NoError(t, err) {
defer conn.Close()
defer client.Close()
err = writeSFTPFile(testFileName, 32, client)
assert.NoError(t, err)
// stamp a known modification time in the past
mtime := time.Now().Add(-72 * time.Hour).Truncate(time.Second)
err = client.Chtimes(testFileName, mtime, mtime)
assert.NoError(t, err)
// cross-root rename into the virtual folder (home and folder are different
// os.Root confinements, so this goes through the copy+delete fallback)
err = client.Rename(testFileName, path.Join(vdirPath, testFileName))
assert.NoError(t, err)
fi, err := client.Stat(path.Join(vdirPath, testFileName))
if assert.NoError(t, err) {
assert.WithinDuration(t, mtime, fi.ModTime(), time.Second,
"the cross-root rename must preserve the file modification time")
}
}
_, 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)
}
func TestDirs(t *testing.T) {
u := getTestUser()
mappedPath := filepath.Join(os.TempDir(), "vdir")
@@ -3231,16 +3360,13 @@ func TestFsPermissionErrors(t *testing.T) {
assert.NoError(t, err)
}
func TestRenameErrorOutsideHomeDir(t *testing.T) {
func TestAtomicUploadErrorReadOnlyHomeDir(t *testing.T) {
if runtime.GOOS == osWindows {
t.Skip("this test is not available on Windows")
}
oldUploadMode := common.Config.UploadMode
oldTempPath := common.Config.TempPath
common.Config.UploadMode = common.UploadModeAtomicWithResume
common.Config.TempPath = filepath.Clean(os.TempDir())
vfs.SetTempPath(common.Config.TempPath)
u := getTestUser()
u.QuotaFiles = 1000
@@ -3254,14 +3380,19 @@ func TestRenameErrorOutsideHomeDir(t *testing.T) {
err = os.Chmod(user.GetHomeDir(), 0555)
assert.NoError(t, err)
defer os.Chmod(user.GetHomeDir(), os.ModePerm) //nolint:errcheck
err = checkBasicSFTP(client)
assert.NoError(t, err)
// the atomic temp file lives in the home directory: with the home
// read-only the upload fails (create or close) and the quota is not
// updated
f, err := client.Create(testFileName)
assert.NoError(t, err)
_, err = f.Write(testFileContent)
assert.NoError(t, err)
err = f.Close()
if err == nil {
if _, err = f.Write(testFileContent); err == nil {
err = f.Close()
}
}
assert.ErrorIs(t, err, os.ErrPermission)
user, _, err = httpdtest.GetUserByUsername(user.Username, http.StatusOK)
@@ -3269,7 +3400,7 @@ func TestRenameErrorOutsideHomeDir(t *testing.T) {
assert.Equal(t, 0, user.UsedQuotaFiles)
assert.Equal(t, int64(0), user.UsedQuotaSize)
err = os.Chmod(user.GetHomeDir(), os.ModeDir)
err = os.Chmod(user.GetHomeDir(), os.ModePerm)
assert.NoError(t, err)
}
@@ -3279,8 +3410,6 @@ func TestRenameErrorOutsideHomeDir(t *testing.T) {
assert.NoError(t, err)
common.Config.UploadMode = oldUploadMode
common.Config.TempPath = oldTempPath
vfs.SetTempPath(oldTempPath)
}
func TestResolvePathError(t *testing.T) {
+4 -23
View File
@@ -362,32 +362,12 @@ func (t *BaseTransfer) getUploadFileSize() (int64, int, error) {
return fileSize, deletedFiles, err
}
// return 1 if the file is outside the user home dir
func (t *BaseTransfer) checkUploadOutsideHomeDir(err error) int {
if err == nil {
return 0
}
if t.ErrTransfer == nil {
t.ErrTransfer = err
}
if Config.TempPath == "" {
return 0
}
err = t.Fs.Remove(t.effectiveFsPath, false)
t.Connection.Log(logger.LevelWarn, "upload in temp path cannot be renamed, delete temporary file: %q, deletion error: %v",
t.effectiveFsPath, err)
// the file is outside the home dir so don't update the quota
t.BytesReceived.Store(0)
t.MinWriteOffset = 0
return 1
}
// Close it is called when the transfer is completed.
// It logs the transfer info, updates the user quota (for uploads)
// and executes any defined action.
// If there is an error no action will be executed and, in atomic mode,
// we try to delete the temporary file
func (t *BaseTransfer) Close() error {
func (t *BaseTransfer) Close() error { //nolint:gocyclo
defer t.Connection.RemoveTransfer(t)
var err error
@@ -412,8 +392,9 @@ func (t *BaseTransfer) Close() error {
_, _, err = t.Fs.Rename(t.effectiveFsPath, t.fsPath, 0)
t.Connection.Log(logger.LevelDebug, "atomic upload completed, rename: %q -> %q, error: %v",
t.effectiveFsPath, t.fsPath, err)
// the file must be removed if it is uploaded to a path outside the home dir and cannot be renamed
t.checkUploadOutsideHomeDir(err)
if err != nil && t.ErrTransfer == nil {
t.ErrTransfer = err
}
} else {
err = t.Fs.Remove(t.effectiveFsPath, false)
t.Connection.Log(logger.LevelWarn, "atomic upload completed with error: \"%v\", delete temporary file: %q, deletion error: %v",
+19 -24
View File
@@ -445,33 +445,28 @@ func TestTransferQuota(t *testing.T) {
assert.Equal(t, int32(0), Connections.GetTotalTransfers())
}
func TestUploadOutsideHomeRenameError(t *testing.T) {
oldTempPath := Config.TempPath
func TestUploadAtomicRenameError(t *testing.T) {
homeDir := filepath.Join(os.TempDir(), "home")
require.NoError(t, os.MkdirAll(homeDir, os.ModePerm))
defer os.RemoveAll(homeDir) //nolint:errcheck
conn := NewBaseConnection("", ProtocolSFTP, "", "", dataprovider.User{})
transfer := BaseTransfer{
Connection: conn,
transferType: TransferUpload,
Fs: vfs.NewOsFs("", filepath.Join(os.TempDir(), "home"), "", nil),
}
fs := vfs.NewOsFs("", homeDir, "", nil)
// the atomic upload temp file lives inside the home dir but does not exist, so the
// rename performed on Close fails and must be recorded as the transfer error
fsPath := filepath.Join(homeDir, "file.txt")
effectiveFsPath := filepath.Join(homeDir, ".sftpgo-upload.missing")
transfer := NewBaseTransfer(nil, conn, nil, fsPath, effectiveFsPath, "/file.txt", TransferUpload,
0, 0, 0, 0, true, fs, dataprovider.TransferQuota{})
require.True(t, transfer.isAtomicUpload())
transfer.BytesReceived.Store(123)
fileName := filepath.Join(os.TempDir(), "_temp")
err := os.WriteFile(fileName, []byte(`data`), 0644)
assert.NoError(t, err)
transfer.effectiveFsPath = fileName
res := transfer.checkUploadOutsideHomeDir(os.ErrPermission)
assert.Equal(t, 0, res)
Config.TempPath = filepath.Clean(os.TempDir())
res = transfer.checkUploadOutsideHomeDir(nil)
assert.Equal(t, 0, res)
err := transfer.Close()
assert.Error(t, err)
// the failed atomic rename is recorded as the transfer error
assert.Equal(t, transfer.ErrTransfer, err)
assert.True(t, fs.IsNotExist(transfer.ErrTransfer))
// the temp file is always inside the home dir now, so the quota counters stay
assert.Greater(t, transfer.BytesReceived.Load(), int64(0))
res = transfer.checkUploadOutsideHomeDir(os.ErrPermission)
assert.Equal(t, 1, res)
assert.Equal(t, int64(0), transfer.BytesReceived.Load())
assert.NoFileExists(t, fileName)
Config.TempPath = oldTempPath
assert.Len(t, conn.GetTransfers(), 0)
}
-2
View File
@@ -214,7 +214,6 @@ func Init() {
SymlinkMode: 0,
ResumeMaxSize: 0,
SecretMinEntropy: 80,
TempPath: "",
ProxyProtocol: 0,
ProxyAllowed: []string{},
ProxySkipped: []string{},
@@ -2066,7 +2065,6 @@ func setViperDefaults() {
viper.SetDefault("common.symlink_mode", globalConf.Common.SymlinkMode)
viper.SetDefault("common.resume_max_size", globalConf.Common.ResumeMaxSize)
viper.SetDefault("common.secret_min_entropy", globalConf.Common.SecretMinEntropy)
viper.SetDefault("common.temp_path", globalConf.Common.TempPath)
viper.SetDefault("common.proxy_protocol", globalConf.Common.ProxyProtocol)
viper.SetDefault("common.proxy_allowed", globalConf.Common.ProxyAllowed)
viper.SetDefault("common.proxy_skipped", globalConf.Common.ProxySkipped)
+1 -11
View File
@@ -235,7 +235,6 @@ var (
argon2Params *argon2id.Params
lastLoginMinDelay = 10 * time.Minute
usernameRegex = regexp.MustCompile("^[a-zA-Z0-9-_.~]+$")
tempPath string
allowSelfConnections int
fnReloadRules FnReloadRules
fnRemoveRule FnRemoveRule
@@ -896,11 +895,6 @@ func SetAllowSelfConnections(value int) {
allowSelfConnections = value
}
// SetTempPath sets the path for temporary files
func SetTempPath(fsPath string) {
tempPath = fsPath
}
func checkSharedMode() {
if !slices.Contains(sharedProviders, config.Driver) {
config.IsShared = 0
@@ -2711,11 +2705,7 @@ func buildUserHomeDir(user *User) {
}
switch user.FsConfig.Provider {
case sdk.SFTPFilesystemProvider, sdk.S3FilesystemProvider, sdk.AzureBlobFilesystemProvider, sdk.GCSFilesystemProvider, sdk.HTTPFilesystemProvider:
if tempPath != "" {
user.HomeDir = filepath.Join(tempPath, user.Username)
} else {
user.HomeDir = filepath.Join(os.TempDir(), user.Username)
}
user.HomeDir = filepath.Join(os.TempDir(), user.Username)
}
} else {
user.HomeDir = filepath.Clean(user.HomeDir)
+71 -31
View File
@@ -26,6 +26,7 @@ import (
"slices"
"strconv"
"strings"
"sync"
"time"
"github.com/rs/xid"
@@ -146,14 +147,29 @@ type User struct {
FsConfig vfs.Filesystem `json:"filesystem"`
// groups associated with this user
Groups []sdk.GroupMapping `json:"groups,omitempty"`
// we store the filesystem here using the base path as key.
fsCache map[string]vfs.Fs `json:"-"`
// per-connection filesystem cache, see fsCache. Never share it between
// concurrently running connections: it is owned and closed by a single one.
fsCache *fsCache `json:"-"`
// true if group settings are already applied for this user
groupSettingsApplied bool `json:"-"`
// in multi node setups we mark the user as deleted to be able to update the webdav cache
DeletedAt int64 `json:"-"`
}
// fsCache holds the per-virtual-path Fs instances used by a connection. In SFTP
// handling we copy a single User into several concurrent connections: each
// channel calls ResetFsCache to get its own cache.
type fsCache struct {
sync.Mutex
entries map[string]vfs.Fs
}
// ResetFsCache gives the user a new, empty and independent filesystem cache,
// detaching it from any cache shared with the User it was value-copied from.
func (u *User) ResetFsCache() {
u.fsCache = &fsCache{entries: make(map[string]vfs.Fs)}
}
// GetFilesystem returns the base filesystem for this user
func (u *User) GetFilesystem(connectionID string) (fs vfs.Fs, err error) {
return u.GetFilesystemForPath("/", connectionID)
@@ -225,6 +241,7 @@ func (u *User) checkLocalHomeDir(connectionID string) {
return
default:
osFs := vfs.NewOsFs(connectionID, u.GetHomeDir(), "", nil)
defer osFs.Close() //nolint:errcheck
osFs.CheckRootPath(u.Username, u.GetUID(), u.GetGID())
}
}
@@ -491,16 +508,19 @@ func (u *User) hasRedactedSecret() bool {
// CloseFs closes the underlying filesystems
func (u *User) CloseFs() error {
if u.fsCache == nil {
cache := u.fsCache
if cache == nil {
return nil
}
cache.Lock()
defer cache.Unlock()
var err error
for _, fs := range u.fsCache {
errClose := fs.Close()
if err == nil {
for key, fs := range cache.entries {
if errClose := fs.Close(); errClose != nil && err == nil {
err = errClose
}
delete(cache.entries, key)
}
return err
}
@@ -606,41 +626,61 @@ func (u *User) GetFsConfigForPath(virtualPath string) vfs.Filesystem {
// GetFilesystemForPath returns the filesystem for the given path
func (u *User) GetFilesystemForPath(virtualPath, connectionID string) (vfs.Fs, error) {
// Lazily create the cache for callers that did not call ResetFsCache: those
// accesses are from a single goroutine so creating it here cannot race.
if u.fsCache == nil {
u.fsCache = make(map[string]vfs.Fs)
u.ResetFsCache()
}
// allow to override the `/` path with a virtual folder
cache := u.fsCache
// resolve the virtual folder and the cache key without touching the cache
var folder vfs.VirtualFolder
hasFolder := false
if len(u.VirtualFolders) > 0 {
folder, err := u.GetVirtualFolderForPath(virtualPath)
if err == nil {
if fs, ok := u.fsCache[folder.VirtualPath]; ok {
return fs, nil
}
forbiddenSelfUsers := []string{u.Username}
if folder.FsConfig.Provider == sdk.SFTPFilesystemProvider {
forbiddens, err := u.getForbiddenSFTPSelfUsers(folder.FsConfig.SFTPConfig.Username)
if err != nil {
return nil, err
}
forbiddenSelfUsers = append(forbiddenSelfUsers, forbiddens...)
}
fs, err := folder.GetFilesystem(connectionID, forbiddenSelfUsers)
if err == nil {
u.fsCache[folder.VirtualPath] = fs
}
return fs, err
if f, err := u.GetVirtualFolderForPath(virtualPath); err == nil {
folder = f
hasFolder = true
}
}
cacheKey := "/"
if hasFolder {
cacheKey = folder.VirtualPath
}
if val, ok := u.fsCache["/"]; ok {
return val, nil
cache.Lock()
fs, ok := cache.entries[cacheKey]
cache.Unlock()
if ok {
return fs, nil
}
var err error
if hasFolder {
forbiddenSelfUsers := []string{u.Username}
if folder.FsConfig.Provider == sdk.SFTPFilesystemProvider {
forbiddens, errSelf := u.getForbiddenSFTPSelfUsers(folder.FsConfig.SFTPConfig.Username)
if errSelf != nil {
return nil, errSelf
}
forbiddenSelfUsers = append(forbiddenSelfUsers, forbiddens...)
}
fs, err = folder.GetFilesystem(connectionID, forbiddenSelfUsers)
} else {
fs, err = u.getRootFs(connectionID)
}
fs, err := u.getRootFs(connectionID)
if err != nil {
return fs, err
}
u.fsCache["/"] = fs
return fs, err
cache.Lock()
defer cache.Unlock()
if existing, ok := cache.entries[cacheKey]; ok {
fs.Close() //nolint:errcheck
return existing, nil
}
cache.entries[cacheKey] = fs
return fs, nil
}
// GetVirtualFolderForPath returns the virtual folder containing the specified virtual path.
+40
View File
@@ -669,6 +669,46 @@ func TestBasicFTPHandling(t *testing.T) {
assert.Equal(t, int32(0), common.Connections.GetTotalTransfers())
}
func TestFTPSessionCloseDoesNotAffectOthers(t *testing.T) {
u := getTestUser()
user, _, err := httpdtest.AddUser(u, http.StatusCreated)
assert.NoError(t, err)
testFilePath := filepath.Join(homeBasePath, testFileName)
testFileSize := int64(65535)
err = createTestFile(testFilePath, testFileSize)
assert.NoError(t, err)
client1, err := getFTPClient(user, false, nil)
if assert.NoError(t, err) {
err = ftpUploadFile(testFilePath, testFileName, testFileSize, client1, 0)
assert.NoError(t, err)
client2, err := getFTPClient(user, false, nil)
if assert.NoError(t, err) {
err = checkBasicFTP(client2)
assert.NoError(t, err)
err = client1.Quit()
assert.NoError(t, err)
assert.Eventually(t, func() bool { return len(common.Connections.GetStats("")) == 1 },
2*time.Second, 100*time.Millisecond)
// the second connection must still be able to open files
localDownloadPath := filepath.Join(homeBasePath, testDLFileName)
err = ftpDownloadFile(testFileName, localDownloadPath, testFileSize, client2, 0)
assert.NoError(t, err)
err = client2.Quit()
assert.NoError(t, err)
err = os.Remove(localDownloadPath)
assert.NoError(t, err)
}
}
assert.Eventually(t, func() bool { return len(common.Connections.GetStats("")) == 0 },
2*time.Second, 100*time.Millisecond)
err = os.Remove(testFilePath)
assert.NoError(t, err)
_, err = httpdtest.RemoveUser(user, http.StatusOK)
assert.NoError(t, err)
err = os.RemoveAll(user.GetHomeDir())
assert.NoError(t, err)
}
func TestHTTPFs(t *testing.T) {
u := getTestUserWithHTTPFs()
user, _, err := httpdtest.AddUser(u, http.StatusCreated)
+10 -21
View File
@@ -16327,7 +16327,7 @@ func TestBrowseShares(t *testing.T) {
assert.NoError(t, err)
err = createTestFile(filepath.Join(user.GetHomeDir(), shareDir, subDir, testFileName), testFileSize)
assert.NoError(t, err)
err = os.Symlink(testFilePath, testLinkPath)
err = os.Symlink(testFileName, testLinkPath)
assert.NoError(t, err)
token, err := getJWTAPIUserTokenFromTestServer(defaultUsername, defaultPassword)
@@ -16532,8 +16532,9 @@ func TestBrowseShares(t *testing.T) {
req, err = http.NewRequest(http.MethodGet, path.Join(webClientPubSharesPath, objectID, "getpdf?path=link.pdf"), nil)
assert.NoError(t, err)
rr = executeRequest(req)
checkResponseCode(t, http.StatusOK, rr)
// downloading a symlink will fail, usage should not change
// link.pdf is an absolute symlink; os.Root refuses to traverse it, so the
// download is blocked and the share usage does not change
checkResponseCode(t, http.StatusForbidden, rr)
s, err = dataprovider.ShareExists(objectID, defaultUsername)
assert.NoError(t, err)
assert.Equal(t, usedTokens, s.UsedTokens)
@@ -18639,7 +18640,8 @@ func TestWebFilesAPI(t *testing.T) {
setBearerForReq(req, webAPIToken)
rr = executeRequest(req)
checkResponseCode(t, http.StatusBadRequest, rr)
// make a symlink outside the home dir and then try to delete it
// make a symlink pointing outside the home dir and delete it: removing the link
// does not follow it, so the in-home link is removed and the target is untouched
extPath := filepath.Join(os.TempDir(), "file")
err = os.WriteFile(extPath, []byte("contents"), os.ModePerm)
assert.NoError(t, err)
@@ -18649,7 +18651,8 @@ func TestWebFilesAPI(t *testing.T) {
assert.NoError(t, err)
setBearerForReq(req, webAPIToken)
rr = executeRequest(req)
checkResponseCode(t, http.StatusForbidden, rr)
checkResponseCode(t, http.StatusOK, rr)
// the symlink target outside the home must still exist
err = os.Remove(extPath)
assert.NoError(t, err)
// remove delete and overwrite permissions
@@ -19401,17 +19404,6 @@ func TestWebUploadErrors(t *testing.T) {
rr = executeRequest(req)
checkResponseCode(t, http.StatusCreated, rr)
vfs.SetTempPath(filepath.Join(os.TempDir(), "missingpath"))
_, err = reader.Seek(0, io.SeekStart)
assert.NoError(t, err)
req, err = http.NewRequest(http.MethodPost, userFilesPath, reader)
assert.NoError(t, err)
req.Header.Add("Content-Type", writer.FormDataContentType())
setBearerForReq(req, webAPIToken)
rr = executeRequest(req)
checkResponseCode(t, http.StatusNotFound, rr)
if runtime.GOOS != osWindows {
req, err = http.NewRequest(http.MethodDelete, userFilesPath+"?path=file.zip", nil)
assert.NoError(t, err)
@@ -19419,7 +19411,6 @@ func TestWebUploadErrors(t *testing.T) {
rr = executeRequest(req)
checkResponseCode(t, http.StatusOK, rr)
vfs.SetTempPath(filepath.Clean(os.TempDir()))
err = os.Chmod(user.GetHomeDir(), 0555)
assert.NoError(t, err)
@@ -19431,21 +19422,19 @@ func TestWebUploadErrors(t *testing.T) {
setBearerForReq(req, webAPIToken)
rr = executeRequest(req)
checkResponseCode(t, http.StatusForbidden, rr)
assert.Contains(t, rr.Body.String(), "Error closing file")
assert.Contains(t, rr.Body.String(), "Unable to write file")
req, err = http.NewRequest(http.MethodPost, userUploadFilePath+"?path=file.zip", bytes.NewBuffer(nil))
assert.NoError(t, err)
setBearerForReq(req, webAPIToken)
rr = executeRequest(req)
checkResponseCode(t, http.StatusForbidden, rr)
assert.Contains(t, rr.Body.String(), "Error closing file")
assert.Contains(t, rr.Body.String(), "Unable to write file")
err = os.Chmod(user.GetHomeDir(), os.ModePerm)
assert.NoError(t, err)
}
vfs.SetTempPath("")
// upload a multipart form with no files
body = new(bytes.Buffer)
writer = multipart.NewWriter(body)
+41 -19
View File
@@ -1380,8 +1380,8 @@ func TestUploadError(t *testing.T) {
BaseConnection: common.NewBaseConnection("", common.ProtocolSCP, "", "", user),
}
testfile := "testfile"
fileTempName := "temptestfile"
testfile := filepath.Join(os.TempDir(), "testfile")
fileTempName := filepath.Join(os.TempDir(), "temptestfile")
file, err := os.Create(fileTempName)
assert.NoError(t, err)
baseTransfer := common.NewBaseTransfer(file, connection.BaseConnection, nil, testfile, file.Name(),
@@ -1932,6 +1932,8 @@ func TestOsFsRootEscapeMatrix(t *testing.T) { //nolint:gocyclo
t.Fatalf("test setup error: evil_existing does not escape the home, resolved %q", tgt)
}
// ResolvePath is lexical so each escaping path resolves without error but
// every operation on it must be blocked, while in-home paths stay usable.
mustReject := []string{
"/evil_rel", // dangling leaf rel
"/evil_abs", // dangling leaf abs
@@ -1945,8 +1947,13 @@ func TestOsFsRootEscapeMatrix(t *testing.T) { //nolint:gocyclo
"/a/sub/evil_existing", // nested in-home symlink + escaping existing leaf
}
for _, p := range mustReject {
if r, err := fs.ResolvePath(p); err == nil {
t.Errorf("ESCAPE: %q must be rejected, got nil error (resolved=%q)", p, r)
r, err := fs.ResolvePath(p)
if err != nil {
t.Errorf("ResolvePath(%q) returned %v, want nil (confinement is enforced at operation time)", p, err)
continue
}
if _, statErr := fs.Stat(r); !fs.IsPermission(statErr) {
t.Errorf("ESCAPE: %q must be blocked at operation time, got %v", p, statErr)
}
}
@@ -1961,8 +1968,14 @@ func TestOsFsRootEscapeMatrix(t *testing.T) { //nolint:gocyclo
"/a/b/c/deepnew", // several new nested dirs
}
for _, p := range mustAllow {
if _, err := fs.ResolvePath(p); err != nil {
t.Errorf("false denial: in-home path %q must be allowed, got %v", p, err)
r, err := fs.ResolvePath(p)
if err != nil {
t.Errorf("false denial: in-home path %q ResolvePath error: %v", p, err)
continue
}
// an in-home path is never a confinement error: it either exists or not
if _, statErr := fs.Stat(r); fs.IsPermission(statErr) {
t.Errorf("false denial: in-home path %q blocked: %v", p, statErr)
}
}
@@ -1970,7 +1983,7 @@ func TestOsFsRootEscapeMatrix(t *testing.T) { //nolint:gocyclo
// confirm nothing is written outside the home
probe := filepath.Join(base, "outside", "x")
if rp, err := fs.ResolvePath("/evil_rel"); err == nil {
if f, oerr := os.OpenFile(rp, os.O_CREATE|os.O_WRONLY, 0o644); oerr == nil {
if f, _, _, oerr := fs.Create(rp, 0, 0); oerr == nil {
f.Close()
}
}
@@ -1997,19 +2010,23 @@ func TestOsFsResolvePathDotDotThroughSymlink(t *testing.T) {
}
fs := vfs.NewOsFs("conn", home, "", nil).(*vfs.OsFs)
// ResolvePath is lexical; the escape is blocked when the path is used.
r, err := fs.ResolvePath("/p")
if err == nil {
if f, oerr := os.OpenFile(r, os.O_CREATE|os.O_WRONLY, 0o644); oerr == nil {
if err != nil {
t.Fatalf("ResolvePath(/p) returned %v, want nil", err)
}
if f, _, _, oerr := fs.Create(r, 0, 0); oerr == nil {
if f != nil {
f.Close()
}
t.Errorf("ESCAPE: /p resolves outside the home but was allowed (resolved=%q)", r)
t.Errorf("ESCAPE: /p resolves outside the home but the create was allowed (resolved=%q)", r)
}
if _, serr := os.Stat(filepath.Join(base, "escape")); !os.IsNotExist(serr) {
t.Errorf("ESCAPE: a file was created outside the home")
}
}
func TestOsFsReadlinkSymlinkedHome(t *testing.T) {
func TestOsFsReadlinkSymlinkedHome(t *testing.T) { //nolint:gocyclo
base := t.TempDir()
realhome := filepath.Join(base, "realhome")
if err := os.MkdirAll(filepath.Join(realhome, "sub"), 0o755); err != nil {
@@ -2044,19 +2061,24 @@ func TestOsFsReadlinkSymlinkedHome(t *testing.T) {
}
fs := vfs.NewOsFs("c", filepath.Join(base, "homelink"), "", nil).(*vfs.OsFs)
cases := map[string]string{
"link": "/sub/target",
"esc": "/",
"esc2": "/", // embedded ".." escaping must be clamped
"abs_in": "/sub/target",
"abs_out": "/",
"a": "/b", // one-level (not /c)
inRoot := map[string]string{
"link": "/sub/target",
"a": "/b", // one-level (not /c)
}
for link, want := range cases {
for link, want := range inRoot {
if got, err := fs.Readlink(filepath.Join(base, "homelink", link)); err != nil || got != want {
t.Errorf("Readlink(%s) = %q, %v; want %q", link, got, err, want)
}
}
// targets that escape the root are rejected, matching what an access through
// the link would do: "esc2" escapes through an intermediate symlink and must
// not be collapsed away lexically, an absolute target is reinterpreted by
// os.Root and rejected too
for _, link := range []string{"esc", "esc2", "abs_in", "abs_out"} {
if _, err := fs.Readlink(filepath.Join(base, "homelink", link)); err == nil {
t.Errorf("Readlink(%s) did not reject an escaping target", link)
}
}
}
func TestOsFsRelativeToRootUNCNoLoop(t *testing.T) {
+3
View File
@@ -652,6 +652,7 @@ func (c *Configuration) AcceptInboundConnection(conn net.Conn, config *ssh.Serve
go discardAllChannels(chans, "invalid root fs", connectionID)
return
}
user.CloseFs() //nolint:errcheck
logger.LoginLog(user.Username, ipAddr, loginType, common.ProtocolSSH, connectionID,
string(sconn.ClientVersion()), true,
@@ -702,6 +703,7 @@ func (c *Configuration) AcceptInboundConnection(conn net.Conn, config *ssh.Serve
LocalAddr: conn.LocalAddr(),
channel: channel,
}
connection.User.ResetFsCache()
go c.handleSftpConnection(channel, connection)
}
case "exec":
@@ -714,6 +716,7 @@ func (c *Configuration) AcceptInboundConnection(conn net.Conn, config *ssh.Serve
LocalAddr: conn.LocalAddr(),
channel: channel,
}
connection.User.ResetFsCache()
ok = processSSHCommand(req.Payload, &connection, c.EnabledSSHCommands)
if ok {
sshConnection.UpdateLastActivity()
+191 -3
View File
@@ -603,6 +603,159 @@ func TestBasicSFTPHandling(t *testing.T) {
assert.NotEmpty(t, status.GetPublicKeysAlgosAsString())
}
func TestMultipleSftpSessionsSameSSHConnection(t *testing.T) {
usePubKey := false
u := getTestUser(usePubKey)
user, _, err := httpdtest.AddUser(u, http.StatusCreated)
assert.NoError(t, err)
testFileSize := int64(65535)
conn, client1, err := getSftpClient(user, usePubKey)
if assert.NoError(t, err) {
defer conn.Close()
err = writeSFTPFile(testFileName, testFileSize, client1)
assert.NoError(t, err)
client2, err := sftp.NewClient(conn)
if assert.NoError(t, err) {
defer client2.Close()
matches, err := client2.Glob(testFileName)
assert.NoError(t, err)
assert.Len(t, matches, 1)
_, err = client2.Lstat(testFileName)
assert.NoError(t, err)
err = client1.Close()
assert.NoError(t, err)
assert.Eventually(t, func() bool { return len(common.Connections.GetStats("")) == 1 },
2*time.Second, 100*time.Millisecond)
f, err := client2.OpenFile(testFileName, os.O_RDONLY)
if assert.NoError(t, err) {
contents := make([]byte, testFileSize)
_, err = io.ReadFull(f, contents)
assert.NoError(t, err)
err = f.Close()
assert.NoError(t, err)
}
}
err = conn.Close()
assert.NoError(t, err)
}
assert.Eventually(t, func() bool { return len(common.Connections.GetStats("")) == 0 },
2*time.Second, 100*time.Millisecond)
// update the user so the filesystem root checks run again on the next
// login: a recent login would skip them via the fast path
user, _, err = httpdtest.UpdateUser(user, http.StatusOK, "")
assert.NoError(t, err)
// a new session opened on the same SSH connection after closing the
// previous one must work too
conn, client1, err = getSftpClient(user, usePubKey)
if assert.NoError(t, err) {
defer conn.Close()
matches, err := client1.Glob(testFileName)
assert.NoError(t, err)
assert.Len(t, matches, 1)
_, err = client1.Lstat(testFileName)
assert.NoError(t, err)
err = client1.Close()
assert.NoError(t, err)
assert.Eventually(t, func() bool { return len(common.Connections.GetStats("")) == 0 },
2*time.Second, 100*time.Millisecond)
client2, err := sftp.NewClient(conn)
if assert.NoError(t, err) {
defer client2.Close()
f, err := client2.OpenFile(testFileName, os.O_RDONLY)
if assert.NoError(t, err) {
contents := make([]byte, testFileSize)
_, err = io.ReadFull(f, contents)
assert.NoError(t, err)
err = f.Close()
assert.NoError(t, err)
}
}
}
_, err = httpdtest.RemoveUser(user, http.StatusOK)
assert.NoError(t, err)
err = os.RemoveAll(user.GetHomeDir())
assert.NoError(t, err)
}
func TestConcurrentSftpChannelsSameSSHConnection(t *testing.T) {
usePubKey := false
u := getTestUser(usePubKey)
user, _, err := httpdtest.AddUser(u, http.StatusCreated)
assert.NoError(t, err)
testFileSize := int64(65535)
testFilePath := filepath.Join(homeBasePath, testFileName)
err = createTestFile(testFilePath, testFileSize)
assert.NoError(t, err)
conn, client0, err := getSftpClient(user, usePubKey)
if assert.NoError(t, err) {
defer conn.Close()
const numChannels = 8
const iterations = 30
clients := []*sftp.Client{client0}
for range numChannels - 1 {
client, err := sftp.NewClient(conn)
require.NoError(t, err)
clients = append(clients, client)
}
var wg sync.WaitGroup
for idx, client := range clients {
wg.Add(1)
go func(channelIdx int, client *sftp.Client) {
defer wg.Done()
remoteName := fmt.Sprintf("file_%d.dat", channelIdx)
for range iterations {
if err := sftpUploadFile(testFilePath, remoteName, testFileSize, client); err != nil {
t.Errorf("channel %d upload: %v", channelIdx, err)
return
}
localDownloadPath := filepath.Join(homeBasePath, fmt.Sprintf("%s_%d", testDLFileName, channelIdx))
if err := sftpDownloadFile(remoteName, localDownloadPath, testFileSize, client); err != nil {
t.Errorf("channel %d download: %v", channelIdx, err)
return
}
if _, err := client.ReadDir("/"); err != nil {
t.Errorf("channel %d readdir: %v", channelIdx, err)
return
}
}
}(idx, client)
}
wg.Wait()
for i := range numChannels {
err = os.Remove(filepath.Join(homeBasePath, fmt.Sprintf("%s_%d", testDLFileName, i)))
assert.NoError(t, err)
}
for _, client := range clients {
err = client.Close()
assert.NoError(t, err)
}
err = conn.Close()
assert.NoError(t, err)
}
assert.Eventually(t, func() bool { return len(common.Connections.GetStats("")) == 0 },
2*time.Second, 100*time.Millisecond)
err = os.Remove(testFilePath)
assert.NoError(t, err)
_, err = httpdtest.RemoveUser(user, http.StatusOK)
assert.NoError(t, err)
err = os.RemoveAll(user.GetHomeDir())
assert.NoError(t, err)
}
func TestBasicSFTPFsHandling(t *testing.T) {
usePubKey := true
baseUser, _, err := httpdtest.AddUser(getTestUser(usePubKey), http.StatusCreated)
@@ -1348,11 +1501,16 @@ func TestRealPath(t *testing.T) {
p, err = client.RealPath(path.Join(subdir, linkName))
assert.NoError(t, err)
assert.Equal(t, path.Join("/", subdir, linkName), p)
// now a link outside the home dir
// now a link outside the home dir: realpath is lexical and advisory, so it
// returns the in-home virtual path without resolving the symlink; access
// through it is still blocked at operation time
err = os.Symlink(filepath.Clean(os.TempDir()), filepath.Join(localUser.GetHomeDir(), subdir, "temp"))
assert.NoError(t, err)
_, err = client.RealPath(path.Join(subdir, "temp"))
assert.ErrorIs(t, err, os.ErrPermission)
p, err = client.RealPath(path.Join(subdir, "temp"))
assert.NoError(t, err)
assert.Equal(t, path.Join("/", subdir, "temp"), p)
_, err = client.Stat(p)
assert.Error(t, err)
conn.Close()
client.Close()
@@ -1954,6 +2112,13 @@ func TestChtimes(t *testing.T) {
// basic tests to verify virtual chroot, should be improved to cover more cases ...
func TestEscapeHomeDir(t *testing.T) {
// use direct (non-atomic) uploads so writes go through os.Root and an escaping
// symlink is blocked; with atomic uploads the temp file would replace the
// symlink in-home instead.
oldUploadMode := common.Config.UploadMode
common.Config.UploadMode = common.UploadModeStandard
defer func() { common.Config.UploadMode = oldUploadMode }()
usePubKey := true
user, _, err := httpdtest.AddUser(getTestUser(usePubKey), http.StatusCreated)
assert.NoError(t, err)
@@ -2068,6 +2233,14 @@ func TestEscapeSFTPFsPrefix(t *testing.T) {
}
func TestSymlinkWriteConfinementLocalFs(t *testing.T) {
// use direct (non-atomic) uploads: with atomic uploads the temp file is renamed
// onto the symlink, replacing it in-home instead of writing through it. Direct
// writes go through os.Root, which blocks an escaping symlink, so this confirms
// that writing to an escaping symlink fails.
oldUploadMode := common.Config.UploadMode
common.Config.UploadMode = common.UploadModeStandard
defer func() { common.Config.UploadMode = oldUploadMode }()
usePubKey := true
user, _, err := httpdtest.AddUser(getTestUser(usePubKey), http.StatusCreated)
assert.NoError(t, err)
@@ -4876,6 +5049,14 @@ func TestMaxSessions(t *testing.T) {
c.Close()
s.Close()
}
c2, err := sftp.NewClient(conn)
if !assert.Error(t, err, "max sessions exceeded, new session on the same SSH connection should not succeed") {
c2.Close()
}
// The initial connection still works.
assert.NoError(t, checkBasicSFTP(client))
err = writeSFTPFile(testFileName, 4096, client)
assert.NoError(t, err)
}
_, err = httpdtest.RemoveUser(user, http.StatusOK)
assert.NoError(t, err)
@@ -7909,6 +8090,13 @@ func TestOpenError(t *testing.T) {
if runtime.GOOS == osWindows {
t.Skip("this test is not available on Windows")
}
// use direct (non-atomic) uploads: with atomic uploads, overwriting an existing
// permission-denied file renames it to the temp path before opening the temp,
// which fails and leaves the source moved; direct writes keep the source in place.
oldUploadMode := common.Config.UploadMode
common.Config.UploadMode = common.UploadModeStandard
defer func() { common.Config.UploadMode = oldUploadMode }()
usePubKey := false
u := getTestUser(usePubKey)
user, _, err := httpdtest.AddUser(u, http.StatusCreated)
+6
View File
@@ -527,6 +527,8 @@ func (*AzureBlobFs) isBadRequestError(err error) bool {
func (fs *AzureBlobFs) CheckRootPath(username string, uid int, gid int) bool {
// we need a local directory for temporary files
osFs := NewOsFs(fs.ConnectionID(), fs.localTempDir, "", nil)
defer osFs.Close() //nolint:errcheck
return osFs.CheckRootPath(username, uid, gid)
}
@@ -847,6 +849,10 @@ func (fs *AzureBlobFs) renameInternal(source, target string, srcInfo os.FileInfo
filesSize += srcInfo.Size()
}
err := fs.skipNotExistErr(fs.Remove(source, srcInfo.IsDir()))
if err != nil && !srcInfo.IsDir() {
numFiles--
filesSize -= srcInfo.Size()
}
return numFiles, filesSize, err
}
+19 -12
View File
@@ -41,8 +41,7 @@ const (
// CryptFs is a Fs implementation that allows to encrypts/decrypts local files
type CryptFs struct {
*OsFs
localTempDir string
masterKey []byte
masterKey []byte
}
// NewCryptFs returns a CryptFs object
@@ -64,11 +63,7 @@ func NewCryptFs(connectionID, rootDir, mountPath string, config CryptFsConfig) (
},
masterKey: []byte(config.Passphrase.GetPayload()),
}
if tempPath == "" {
fs.localTempDir = rootDir
} else {
fs.localTempDir = tempPath
}
fs.openRoot() //nolint:errcheck
return fs, nil
}
@@ -88,7 +83,7 @@ func (fs *CryptFs) Open(name string, offset int64) (File, PipeReader, func(), er
f.Close()
return nil, nil, nil, err
}
r, w, err := createPipeFn(fs.localTempDir, 0)
r, w, err := createPipeFn(fs.rootDir, 0)
if err != nil {
f.Close()
return nil, nil, nil, err
@@ -154,7 +149,11 @@ func (fs *CryptFs) Open(name string, offset int64) (File, PipeReader, func(), er
// Create creates or opens the named file for writing
func (fs *CryptFs) Create(name string, _, _ int) (File, PipeWriter, func(), error) {
f, err := os.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
root, rel, err := fs.toRootRelative(name)
if err != nil {
return nil, nil, nil, err
}
f, err := root.OpenFile(rel, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
if err != nil {
return nil, nil, nil, err
}
@@ -174,7 +173,7 @@ func (fs *CryptFs) Create(name string, _, _ int) (File, PipeWriter, func(), erro
return nil, nil, nil, err
}
copy(key[:], keyMaterial)
r, w, err := createPipeFn(fs.localTempDir, 0)
r, w, err := createPipeFn(fs.rootDir, 0)
if err != nil {
f.Close()
return nil, nil, nil, err
@@ -221,7 +220,11 @@ func (*CryptFs) Truncate(_ string, _ int64) error {
// ReadDir reads the directory named by dirname and returns
// a list of directory entries.
func (fs *CryptFs) ReadDir(dirname string) (DirLister, error) {
f, err := os.Open(dirname)
root, rel, err := fs.toRootRelative(dirname)
if err != nil {
return nil, err
}
f, err := root.Open(rel)
if err != nil {
if isInvalidNameError(err) {
err = os.ErrNotExist
@@ -288,7 +291,11 @@ func (fs *CryptFs) ConvertFileInfo(info os.FileInfo) os.FileInfo {
func (fs *CryptFs) getFileAndEncryptionKey(name string) (*os.File, [32]byte, error) {
var key [32]byte
f, err := os.Open(name)
root, rel, err := fs.toRootRelative(name)
if err != nil {
return nil, key, err
}
f, err := root.Open(rel)
if err != nil {
return nil, key, err
}
+6
View File
@@ -467,6 +467,8 @@ func (*GCSFs) IsNotSupported(err error) bool {
func (fs *GCSFs) CheckRootPath(username string, uid int, gid int) bool {
// we need a local directory for temporary files
osFs := NewOsFs(fs.ConnectionID(), fs.localTempDir, "", nil)
defer osFs.Close() //nolint:errcheck
return osFs.CheckRootPath(username, uid, gid)
}
@@ -847,6 +849,10 @@ func (fs *GCSFs) renameInternal(source, target string, srcInfo os.FileInfo, recu
if fs.IsNotExist(err) {
err = nil
}
if err != nil && !srcInfo.IsDir() {
numFiles--
filesSize -= srcInfo.Size()
}
return numFiles, filesSize, err
}
+2
View File
@@ -556,6 +556,8 @@ func (*HTTPFs) IsNotSupported(err error) bool {
func (fs *HTTPFs) CheckRootPath(username string, uid int, gid int) bool {
// we need a local directory for temporary files
osFs := NewOsFs(fs.ConnectionID(), fs.localTempDir, "", nil)
defer osFs.Close() //nolint:errcheck
return osFs.CheckRootPath(username, uid, gid)
}
+331 -311
View File
@@ -19,16 +19,16 @@ import (
"errors"
"fmt"
"io"
"io/fs"
iofs "io/fs"
"net/http"
"os"
"path"
"path/filepath"
"slices"
"strings"
"sync"
"sync/atomic"
"time"
fscopy "github.com/otiai10/copy"
"github.com/pkg/sftp"
"github.com/rs/xid"
"github.com/sftpgo/sdk"
@@ -41,6 +41,10 @@ const (
osFsName = "osfs"
)
// errNoRoot is returned by OsFs methods invoked after Close, or on a filesystem
// whose rootDir is not absolute.
var errNoRoot = errors.New("filesystem is not ready, root directory not initialized")
type pathResolutionError struct {
err string
}
@@ -49,6 +53,11 @@ func (e *pathResolutionError) Error() string {
return fmt.Sprintf("Path resolution error: %s", e.err)
}
func isPathResolutionError(err error) bool {
var pErr *pathResolutionError
return errors.As(err, &pErr)
}
// OsFs is a Fs implementation that uses functions provided by the os package.
type OsFs struct {
name string
@@ -56,9 +65,12 @@ type OsFs struct {
rootDir string
// if not empty this fs is mouted as virtual folder in the specified path
mountPath string
localTempDir string
readBufferSize int
writeBufferSize int
root atomic.Pointer[os.Root]
rootMu sync.Mutex
// closed is read and written only under rootMu
closed bool
}
// NewOsFs returns an OsFs object that allows to interact with local Os filesystem
@@ -68,15 +80,69 @@ func NewOsFs(connectionID, rootDir, mountPath string, config *sdk.OSFsConfig) Fs
readBufferSize = config.ReadBufferSize * 1024 * 1024
writeBufferSize = config.WriteBufferSize * 1024 * 1024
}
return &OsFs{
fs := &OsFs{
name: osFsName,
connectionID: connectionID,
rootDir: rootDir,
mountPath: getMountPath(mountPath),
localTempDir: getLocalTempDir(),
readBufferSize: readBufferSize,
writeBufferSize: writeBufferSize,
}
fs.openRoot() //nolint:errcheck // best-effort: a missing home is created by CheckRootPath
return fs
}
// openRoot returns the os.Root confinement, opening it on first use. A failed
// open is retried on the next call and reports the real error (not-exist,
// permission); a closed fs reports errNoRoot instead of reopening.
func (fs *OsFs) openRoot() (*os.Root, error) {
if root := fs.root.Load(); root != nil {
return root, nil
}
if !filepath.IsAbs(fs.rootDir) {
return nil, errNoRoot
}
fs.rootMu.Lock()
defer fs.rootMu.Unlock()
if root := fs.root.Load(); root != nil {
return root, nil
}
if fs.closed {
return nil, errNoRoot
}
root, err := os.OpenRoot(fs.rootDir)
if err != nil {
return nil, err
}
fs.root.Store(root)
return root, nil
}
// toRootRelative maps an absolute fsPath to the os.Root and a path relative to
// rootDir, rejecting anything not contained in the root and opening the
// confinement on first use. It returns the loaded *os.Root so callers operate on
// a single, stable handle.
func (fs *OsFs) toRootRelative(name string) (*os.Root, string, error) {
root, err := fs.openRoot()
if err != nil {
return nil, "", err
}
cleanName := filepath.Clean(name)
if strings.TrimRight(cleanName, `\/`) == strings.TrimRight(fs.rootDir, `\/`) {
return root, ".", nil
}
rel, err := filepath.Rel(fs.rootDir, cleanName)
if err != nil {
return nil, "", &pathResolutionError{err: fmt.Sprintf("cannot resolve %q inside root %q: %v", name, fs.rootDir, err)}
}
if rel == "." {
return root, ".", nil
}
if !filepath.IsLocal(rel) {
return nil, "", &pathResolutionError{err: fmt.Sprintf("path %q is not inside root %q", name, fs.rootDir)}
}
return root, rel, nil
}
// Name returns the name for the Fs implementation
@@ -91,17 +157,29 @@ func (fs *OsFs) ConnectionID() string {
// Stat returns a FileInfo describing the named file
func (fs *OsFs) Stat(name string) (os.FileInfo, error) {
return os.Stat(name)
root, rel, err := fs.toRootRelative(name)
if err != nil {
return nil, err
}
return root.Stat(rel)
}
// Lstat returns a FileInfo describing the named file
func (fs *OsFs) Lstat(name string) (os.FileInfo, error) {
return os.Lstat(name)
root, rel, err := fs.toRootRelative(name)
if err != nil {
return nil, err
}
return root.Lstat(rel)
}
// Open opens the named file for reading
func (fs *OsFs) Open(name string, offset int64) (File, PipeReader, func(), error) {
f, err := os.Open(name)
root, rel, err := fs.toRootRelative(name)
if err != nil {
return nil, nil, nil, err
}
f, err := root.Open(rel)
if err != nil {
return nil, nil, nil, err
}
@@ -115,7 +193,7 @@ func (fs *OsFs) Open(name string, offset int64) (File, PipeReader, func(), error
if fs.readBufferSize <= 0 {
return f, nil, nil, err
}
r, w, err := createPipeFn(fs.localTempDir, 0)
r, w, err := createPipeFn(fs.rootDir, 0)
if err != nil {
f.Close()
return nil, nil, nil, err
@@ -134,21 +212,24 @@ func (fs *OsFs) Open(name string, offset int64) (File, PipeReader, func(), error
// Create creates or opens the named file for writing
func (fs *OsFs) Create(name string, flag, _ int) (File, PipeWriter, func(), error) {
if !fs.useWriteBuffering(flag) {
var err error
var f *os.File
if flag == 0 {
f, err = os.Create(name)
} else {
f, err = os.OpenFile(name, flag, 0666)
}
return f, nil, nil, err
}
f, err := os.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
root, rel, err := fs.toRootRelative(name)
if err != nil {
return nil, nil, nil, err
}
r, w, err := createPipeFn(fs.localTempDir, 0)
if !fs.useWriteBuffering(flag) {
var f *os.File
if flag == 0 {
f, err = root.Create(rel)
} else {
f, err = root.OpenFile(rel, flag, 0666)
}
return f, nil, nil, err
}
f, err := root.OpenFile(rel, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
if err != nil {
return nil, nil, nil, err
}
r, w, err := createPipeFn(fs.rootDir, 0)
if err != nil {
f.Close()
return nil, nil, nil, err
@@ -174,108 +255,206 @@ func (fs *OsFs) Create(name string, flag, _ int) (File, PipeWriter, func(), erro
return nil, p, nil, nil
}
// Rename renames (moves) source to target
// Rename renames (moves) source to target. A move that would cross the os.Root
// boundary is reported as ErrCrossRename so the connection layer performs a
// confined copy + delete instead.
func (fs *OsFs) Rename(source, target string, checks int) (int, int64, error) {
if source == target {
return -1, -1, nil
}
err := os.Rename(source, target)
if err != nil && isCrossDeviceError(err) {
fsLog(fs, logger.LevelError, "cross device error detected while renaming %q -> %q. Trying a copy and remove, this could take a long time",
source, target)
var readBufferSize uint
if fs.readBufferSize > 0 {
readBufferSize = uint(fs.readBufferSize)
// only a path resolution failure means the move crosses confinement roots
root, relSource, errSource := fs.toRootRelative(source)
if errSource != nil && !isPathResolutionError(errSource) {
return -1, -1, errSource
}
_, relTarget, errTarget := fs.toRootRelative(target)
if errTarget != nil && !isPathResolutionError(errTarget) {
return -1, -1, errTarget
}
if errSource != nil || errTarget != nil {
return -1, -1, ErrCrossRename
}
err := root.Rename(relSource, relTarget)
if err != nil {
if isCrossDeviceError(err) {
return -1, -1, ErrCrossRename
}
err = fscopy.Copy(source, target, fscopy.Options{
OnSymlink: func(_ string) fscopy.SymlinkAction {
return fscopy.Skip
},
CopyBufferSize: readBufferSize,
})
if err != nil {
fsLog(fs, logger.LevelError, "cross device copy error: %v", err)
return -1, -1, err
}
if checks&CheckUpdateModTime != 0 {
fs.Chtimes(target, time.Now(), time.Now(), false) //nolint:errcheck
}
err = os.RemoveAll(source)
return -1, -1, err
}
if checks&CheckUpdateModTime != 0 && err == nil {
if checks&CheckUpdateModTime != 0 {
fs.Chtimes(target, time.Now(), time.Now(), false) //nolint:errcheck
}
return -1, -1, err
return -1, -1, nil
}
// Remove removes the named file or (empty) directory.
func (*OsFs) Remove(name string, _ bool) error {
return os.Remove(name)
func (fs *OsFs) Remove(name string, _ bool) error {
root, rel, err := fs.toRootRelative(name)
if err != nil {
return err
}
return root.Remove(rel)
}
// Mkdir creates a new directory with the specified name and default permissions
func (*OsFs) Mkdir(name string) error {
return os.Mkdir(name, os.ModePerm)
func (fs *OsFs) Mkdir(name string) error {
root, rel, err := fs.toRootRelative(name)
if err != nil {
return err
}
return root.Mkdir(rel, os.ModePerm)
}
func (*OsFs) Symlink(source, target string) error {
return os.Symlink(source, target)
func (fs *OsFs) Symlink(source, target string) error {
root, relTarget, err := fs.toRootRelative(target)
if err != nil {
return err
}
if filepath.IsAbs(source) {
if rel, err := filepath.Rel(filepath.Dir(target), source); err == nil {
source = rel
}
}
source = filepath.FromSlash(source)
return root.Symlink(source, relTarget)
}
func (fs *OsFs) Readlink(name string) (string, error) {
target, err := os.Readlink(name)
root, rel, err := fs.toRootRelative(name)
if err != nil {
return "", err
}
target, err := root.Readlink(rel)
if err != nil {
return "", err
}
if linkTargetEscapes(target) {
return "", &pathResolutionError{err: fmt.Sprintf("link target %q escapes from root", target)}
}
resolved, err := fs.resolveLinkTarget(root, rel, target)
if err != nil {
return "", err
}
return fs.GetRelativePath(filepath.Join(fs.rootDir, resolved)), nil
}
func (fs *OsFs) resolveLinkTarget(root *os.Root, linkRel, target string) (string, error) {
rest := append(splitPathComponents(filepath.Dir(linkRel)), splitPathComponents(target)...)
resolved := "."
linksWalked := 0
// resolve the directory the target points through component by component
// but do not follow the final component, keeping one-level readlink semantics
startDir := filepath.VolumeName(target) + string(os.PathSeparator)
if !filepath.IsAbs(target) {
if startDir, err = fs.resolveForConfinement(filepath.Dir(name), &linksWalked); err != nil {
for len(rest) > 0 {
comp := rest[0]
rest = rest[1:]
switch comp {
case "", ".":
continue
case "..":
if resolved == "." {
return "", &pathResolutionError{err: fmt.Sprintf("link target %q escapes from root", target)}
}
resolved = filepath.Dir(resolved)
continue
}
candidate := filepath.Join(resolved, comp)
// the final component is reported one level: do not follow it
if len(rest) == 0 {
return candidate, nil
}
info, err := root.Lstat(candidate)
if err != nil {
if isRootEscapeError(err) {
return "", err
}
if fs.IsNotExist(err) {
// the tail does not exist, so it holds no symlinks
resolved = candidate
continue
}
return "", err
}
if info.Mode()&os.ModeSymlink == 0 {
resolved = candidate
continue
}
linksWalked++
if linksWalked > maxResolvedSymlinks {
fsLog(fs, logger.LevelError, "unable to resolve link, too many links: %d", linksWalked)
return "", &pathResolutionError{err: "too many symbolic links"}
}
linkTarget, err := root.Readlink(candidate)
if err != nil {
return "", err
}
if linkTargetEscapes(linkTarget) {
return "", &pathResolutionError{err: fmt.Sprintf("link target %q escapes from root", linkTarget)}
}
rest = append(splitPathComponents(linkTarget), rest...)
}
// strip the volume name (e.g. "C:" on Windows) before splitting: it belongs to
// startDir, not to the components, otherwise it would be treated as a path entry
comps := splitPathComponents(target[len(filepath.VolumeName(target)):])
if len(comps) == 0 {
return fs.relativeToRoot(fs.normalize(fs.rootDir), fs.normalize(startDir)), nil
return resolved, nil
}
func linkTargetEscapes(target string) bool {
return filepath.IsAbs(target) || filepath.VolumeName(target) != "" ||
(len(target) > 0 && os.IsPathSeparator(target[0]))
}
func splitPathComponents(p string) []string {
p = strings.Trim(filepath.ToSlash(p), "/")
if p == "" {
return nil
}
resolvedDir, err := fs.walkResolve(startDir, comps[:len(comps)-1], &linksWalked)
if err != nil {
return "", err
}
resolved := filepath.Join(resolvedDir, comps[len(comps)-1])
return fs.relativeToRoot(fs.normalize(fs.rootDir), resolved), nil
return strings.Split(p, "/")
}
// Chown changes the numeric uid and gid of the named file.
func (*OsFs) Chown(name string, uid int, gid int) error {
return os.Chown(name, uid, gid)
func (fs *OsFs) Chown(name string, uid int, gid int) error {
root, rel, err := fs.toRootRelative(name)
if err != nil {
return err
}
return root.Chown(rel, uid, gid)
}
// Chmod changes the mode of the named file to mode
func (*OsFs) Chmod(name string, mode os.FileMode) error {
return os.Chmod(name, mode)
func (fs *OsFs) Chmod(name string, mode os.FileMode) error {
root, rel, err := fs.toRootRelative(name)
if err != nil {
return err
}
return root.Chmod(rel, mode)
}
// Chtimes changes the access and modification times of the named file
func (*OsFs) Chtimes(name string, atime, mtime time.Time, _ bool) error {
return os.Chtimes(name, atime, mtime)
func (fs *OsFs) Chtimes(name string, atime, mtime time.Time, _ bool) error {
root, rel, err := fs.toRootRelative(name)
if err != nil {
return err
}
return root.Chtimes(rel, atime, mtime)
}
// Truncate changes the size of the named file
func (*OsFs) Truncate(name string, size int64) error {
return os.Truncate(name, size)
func (fs *OsFs) Truncate(name string, size int64) error {
root, rel, err := fs.toRootRelative(name)
if err != nil {
return err
}
f, err := root.OpenFile(rel, os.O_WRONLY, 0)
if err != nil {
return err
}
defer f.Close()
return f.Truncate(size)
}
// ReadDir reads the directory named by dirname and returns
// a list of directory entries.
func (*OsFs) ReadDir(dirname string) (DirLister, error) {
f, err := os.Open(dirname)
func (fs *OsFs) ReadDir(dirname string) (DirLister, error) {
root, rel, err := fs.toRootRelative(dirname)
if err != nil {
return nil, err
}
f, err := root.Open(rel)
if err != nil {
if isInvalidNameError(err) {
err = os.ErrNotExist
@@ -304,16 +483,22 @@ func (*OsFs) IsAtomicUploadSupported() bool {
// IsNotExist returns a boolean indicating whether the error is known to
// report that a file or directory does not exist
func (*OsFs) IsNotExist(err error) bool {
return errors.Is(err, fs.ErrNotExist)
return errors.Is(err, iofs.ErrNotExist)
}
// IsPermission returns a boolean indicating whether the error is known to
// report that permission is denied.
func (*OsFs) IsPermission(err error) bool {
if err == nil {
return false
}
if _, ok := err.(*pathResolutionError); ok {
return true
}
return errors.Is(err, fs.ErrPermission)
if isRootEscapeError(err) {
return true
}
return errors.Is(err, iofs.ErrPermission)
}
// IsNotSupported returns true if the error indicate an unsupported operation
@@ -324,18 +509,23 @@ func (*OsFs) IsNotSupported(err error) bool {
return err == ErrVfsUnsupported
}
// CheckRootPath creates the root directory if it does not exists
// CheckRootPath creates the root directory if it does not exist.
func (fs *OsFs) CheckRootPath(username string, uid int, gid int) bool {
var err error
if _, err = fs.Stat(fs.rootDir); fs.IsNotExist(err) {
err = os.MkdirAll(fs.rootDir, os.ModePerm)
if err == nil {
SetPathPermissions(fs, fs.rootDir, uid, gid)
} else {
_, err := fs.openRoot()
if fs.IsNotExist(err) {
if err := os.MkdirAll(fs.rootDir, os.ModePerm); err != nil {
fsLog(fs, logger.LevelError, "error creating root directory %q for user %q: %v", fs.rootDir, username, err)
return false
}
if _, err = fs.openRoot(); err == nil {
SetPathPermissions(fs, fs.rootDir, uid, gid)
}
}
return err == nil
if err != nil {
fsLog(fs, logger.LevelError, "unable to open root directory %q for user %q: %v", fs.rootDir, username, err)
return false
}
return true
}
// ScanRootDirContents returns the number of files contained in the root
@@ -349,12 +539,11 @@ func (*OsFs) CheckMetadata() error {
return nil
}
// GetAtomicUploadPath returns the path to use for an atomic upload
// GetAtomicUploadPath returns the path to use for an atomic upload. The temp file
// is always created in the same directory as the target so the atomic rename
// stays on the same filesystem and the file remains reachable through the VFS.
func (*OsFs) GetAtomicUploadPath(name string) string {
dir := filepath.Dir(name)
if tempPath != "" {
dir = tempPath
}
guid := xid.New().String()
return filepath.Join(dir, ".sftpgo-upload."+guid+"."+filepath.Base(name))
}
@@ -389,9 +578,22 @@ func (fs *OsFs) relativeToRoot(root, name string) string {
}
// Walk walks the file tree rooted at root, calling walkFn for each file or
// directory in the tree, including root
func (*OsFs) Walk(root string, walkFn filepath.WalkFunc) error {
return filepath.Walk(root, walkFn)
// directory in the tree, including root. The traversal is confined by the os.Root
// and the walked path passed to walkFn is rebuilt as an absolute fsPath so callers
// keep seeing the same path format as a plain filepath.Walk.
func (fs *OsFs) Walk(root string, walkFn filepath.WalkFunc) error {
osRoot, rel, err := fs.toRootRelative(root)
if err != nil {
return err
}
return iofs.WalkDir(osRoot.FS(), filepath.ToSlash(rel), func(p string, d iofs.DirEntry, err error) error {
absPath := filepath.Join(fs.rootDir, filepath.FromSlash(p))
var info os.FileInfo
if err == nil {
info, err = d.Info()
}
return walkFn(absPath, info, err)
})
}
// Join joins any number of path elements into a single path
@@ -399,7 +601,8 @@ func (*OsFs) Join(elem ...string) string {
return filepath.Join(elem...)
}
// ResolvePath returns the matching filesystem path for the specified sftp path
// ResolvePath returns the matching filesystem path for the specified virtual
// path.
func (fs *OsFs) ResolvePath(virtualPath string) (string, error) {
if !filepath.IsAbs(fs.rootDir) {
return "", fmt.Errorf("invalid root path %q", fs.rootDir)
@@ -410,35 +613,7 @@ func (fs *OsFs) ResolvePath(virtualPath string) (string, error) {
}
}
virtualPath = path.Clean("/" + virtualPath)
r := filepath.Clean(filepath.Join(fs.rootDir, virtualPath))
p, err := filepath.EvalSymlinks(r)
if isInvalidNameError(err) {
err = os.ErrNotExist
}
isNotExist := fs.IsNotExist(err)
if err != nil && !isNotExist {
return "", err
} else if isNotExist {
if linkErr := fs.checkDanglingSymlinks(r); linkErr != nil {
fsLog(fs, logger.LevelError, "Invalid path resolution, original path %q resolved %q err: %v",
virtualPath, r, linkErr)
return r, linkErr
}
// The requested path doesn't exist, so at this point we need to iterate up the
// path chain until we hit a directory that _does_ exist and can be validated.
_, err = fs.findFirstExistingDir(r)
if err != nil {
fsLog(fs, logger.LevelError, "error resolving non-existent path %q", err)
}
return r, err
}
err = fs.isSubDir(p)
if err != nil {
fsLog(fs, logger.LevelError, "Invalid path resolution, path %q original path %q resolved %q err: %v",
p, virtualPath, r, err)
}
return r, err
return filepath.Clean(filepath.Join(fs.rootDir, virtualPath)), nil
}
const maxResolvedSymlinks = 10
@@ -450,75 +625,6 @@ func (fs *OsFs) RealPath(p string) (string, error) {
return fs.GetRelativePath(p), nil
}
func (fs *OsFs) resolveForConfinement(name string, linksWalked *int) (string, error) {
vol := filepath.VolumeName(name)
return fs.walkResolve(vol+string(os.PathSeparator), splitPathComponents(name[len(vol):]), linksWalked)
}
func (fs *OsFs) walkResolve(resolved string, rest []string, linksWalked *int) (string, error) {
for len(rest) > 0 {
comp := rest[0]
rest = rest[1:]
switch comp {
case "", ".":
continue
case "..":
resolved = filepath.Dir(resolved)
continue
}
candidate := filepath.Join(resolved, comp)
info, err := os.Lstat(candidate)
if err != nil {
if !fs.IsNotExist(err) {
return "", err
}
// the tail does not exist, so it holds no symlinks: resolve it lexically
// against the normalized existing prefix
parts := append([]string{fs.normalize(resolved), comp}, rest...)
return filepath.Clean(filepath.Join(parts...)), nil
}
if info.Mode()&os.ModeSymlink == 0 {
resolved = candidate
continue
}
*linksWalked++
if *linksWalked > maxResolvedSymlinks {
fsLog(fs, logger.LevelError, "unable to resolve path, too many links: %d", *linksWalked)
return "", &pathResolutionError{err: "too many symbolic links"}
}
target, err := os.Readlink(candidate)
if err != nil {
return "", err
}
if filepath.IsAbs(target) {
// an absolute target restarts from the volume root; a relative one is
// resolved against the link's directory, which is the current resolved
resolved = filepath.VolumeName(target) + string(os.PathSeparator)
}
rest = append(splitPathComponents(target[len(filepath.VolumeName(target)):]), rest...)
}
return fs.normalize(resolved), nil
}
// normalize returns p with symbolic links and platform aliases resolved
// (filepath.EvalSymlinks), or p unchanged if it cannot be resolved.
func (fs *OsFs) normalize(p string) string {
if resolved, err := filepath.EvalSymlinks(p); err == nil {
return resolved
}
return p
}
// splitPathComponents splits p into its slash- or separator-delimited components,
// dropping leading and trailing empties.
func splitPathComponents(p string) []string {
p = strings.Trim(filepath.ToSlash(p), "/")
if p == "" {
return nil
}
return strings.Split(p, "/")
}
// GetDirSize returns the number of files and the size for a folder
// including any subfolders
func (fs *OsFs) GetDirSize(dirname string) (int, int64, error) {
@@ -526,7 +632,7 @@ func (fs *OsFs) GetDirSize(dirname string) (int, int64, error) {
size := int64(0)
isDir, err := isDirectory(fs, dirname)
if err == nil && isDir {
err = filepath.Walk(dirname, func(_ string, info os.FileInfo, err error) error {
err = fs.Walk(dirname, func(_ string, info os.FileInfo, err error) error {
if err != nil {
return err
}
@@ -548,118 +654,13 @@ func (*OsFs) HasVirtualFolders() bool {
return false
}
func (fs *OsFs) findNonexistentDirs(filePath string) ([]string, error) {
results := []string{}
cleanPath := filepath.Clean(filePath)
parent := filepath.Dir(cleanPath)
_, err := os.Stat(parent)
for fs.IsNotExist(err) {
results = append(results, parent)
parent = filepath.Dir(parent)
if slices.Contains(results, parent) {
break
}
_, err = os.Stat(parent)
}
if err != nil {
return results, err
}
p, err := filepath.EvalSymlinks(parent)
if err != nil {
return results, err
}
err = fs.isSubDir(p)
if err != nil {
fsLog(fs, logger.LevelError, "error finding non existing dir: %v", err)
}
return results, err
}
func (fs *OsFs) findFirstExistingDir(path string) (string, error) {
results, err := fs.findNonexistentDirs(path)
if err != nil {
fsLog(fs, logger.LevelError, "unable to find non existent dirs: %v", err)
return "", err
}
var parent string
if len(results) > 0 {
lastMissingDir := results[len(results)-1]
parent = filepath.Dir(lastMissingDir)
} else {
parent = fs.rootDir
}
p, err := filepath.EvalSymlinks(parent)
if err != nil {
return "", err
}
fileInfo, err := os.Stat(p)
if err != nil {
return "", err
}
if !fileInfo.IsDir() {
return "", fmt.Errorf("resolved path is not a dir: %q", p)
}
err = fs.isSubDir(p)
return p, err
}
func (fs *OsFs) checkDanglingSymlinks(name string) error {
current := filepath.Clean(name)
for {
info, err := os.Lstat(current)
if err == nil {
if info.Mode()&os.ModeSymlink == 0 {
return nil
}
linksWalked := 0
resolved, err := fs.resolveForConfinement(current, &linksWalked)
if err != nil {
return err
}
return fs.isSubDir(resolved)
}
if !fs.IsNotExist(err) {
return err
}
parent := filepath.Dir(current)
if parent == current {
return nil
}
current = parent
}
}
func (fs *OsFs) isSubDir(sub string) error {
// fs.rootDir must exist and it is already a validated absolute path
parent, err := filepath.EvalSymlinks(fs.rootDir)
if err != nil {
fsLog(fs, logger.LevelError, "invalid root path %q: %v", fs.rootDir, err)
return err
}
if parent == sub {
return nil
}
if len(sub) < len(parent) {
err = fmt.Errorf("path %q is not inside %q", sub, parent)
return &pathResolutionError{err: err.Error()}
}
separator := string(os.PathSeparator)
if parent == filepath.Dir(parent) {
// parent is the root dir, on Windows we can have C:\, D:\ and so on here
// so we still need the prefix check
separator = ""
}
if !strings.HasPrefix(sub, parent+separator) {
err = fmt.Errorf("path %q is not inside %q", sub, parent)
return &pathResolutionError{err: err.Error()}
}
return nil
}
// GetMimeType returns the content type
func (fs *OsFs) GetMimeType(name string) (string, error) {
f, err := os.OpenFile(name, os.O_RDONLY, 0)
root, rel, err := fs.toRootRelative(name)
if err != nil {
return "", err
}
f, err := root.OpenFile(rel, os.O_RDONLY, 0)
if err != nil {
return "", err
}
@@ -675,14 +676,33 @@ func (fs *OsFs) GetMimeType(name string) (string, error) {
return ctype, err
}
// Close closes the fs
func (*OsFs) Close() error {
func (fs *OsFs) Close() error {
fs.rootMu.Lock()
defer fs.rootMu.Unlock()
fs.closed = true
if root := fs.root.Swap(nil); root != nil {
return root.Close()
}
return nil
}
// GetAvailableDiskSize returns the available size for the specified path
func (*OsFs) GetAvailableDiskSize(dirName string) (*sftp.StatVFS, error) {
return getStatFS(dirName)
func (fs *OsFs) GetAvailableDiskSize(dirName string) (*sftp.StatVFS, error) {
root, rel, err := fs.toRootRelative(dirName)
if err != nil {
return nil, err
}
if _, err := root.Lstat(rel); err != nil {
return nil, err
}
f, err := root.Open(".")
if err != nil {
return nil, err
}
defer f.Close()
return getStatFS(f, fs.rootDir)
}
func (fs *OsFs) useWriteBuffering(flag int) bool {
+6
View File
@@ -483,6 +483,8 @@ func (*S3Fs) IsNotSupported(err error) bool {
func (fs *S3Fs) CheckRootPath(username string, uid int, gid int) bool {
// we need a local directory for temporary files
osFs := NewOsFs(fs.ConnectionID(), fs.localTempDir, "", nil)
defer osFs.Close() //nolint:errcheck
return osFs.CheckRootPath(username, uid, gid)
}
@@ -746,6 +748,10 @@ func (fs *S3Fs) renameInternal(source, target string, srcInfo os.FileInfo, recur
if fs.IsNotExist(err) {
err = nil
}
if err != nil && !srcInfo.IsDir() {
numFiles--
filesSize -= srcInfo.Size()
}
return numFiles, filesSize, err
}
+2
View File
@@ -644,6 +644,8 @@ func (*SFTPFs) IsNotSupported(err error) bool {
func (fs *SFTPFs) CheckRootPath(username string, uid int, gid int) bool {
// local directory for temporary files in buffer mode
osFs := NewOsFs(fs.ConnectionID(), fs.localTempDir, "", nil)
defer osFs.Close() //nolint:errcheck
osFs.CheckRootPath(username, uid, gid)
if fs.config.Prefix == "/" {
return true
+4 -2
View File
@@ -17,14 +17,16 @@
package vfs
import (
"os"
"github.com/pkg/sftp"
"github.com/shirou/gopsutil/v3/disk"
)
const bsize = uint64(4096)
func getStatFS(path string) (*sftp.StatVFS, error) {
usage, err := disk.Usage(path)
func getStatFS(_ *os.File, fallbackPath string) (*sftp.StatVFS, error) {
usage, err := disk.Usage(fallbackPath)
if err != nil {
return nil, err
}
+4 -2
View File
@@ -17,13 +17,15 @@
package vfs
import (
"os"
"github.com/pkg/sftp"
"golang.org/x/sys/unix"
)
func getStatFS(path string) (*sftp.StatVFS, error) {
func getStatFS(f *os.File, _ string) (*sftp.StatVFS, error) {
stat := unix.Statfs_t{}
err := unix.Statfs(path, &stat)
err := unix.Fstatfs(int(f.Fd()), &stat)
if err != nil {
return nil, err
}
+4 -2
View File
@@ -17,13 +17,15 @@
package vfs
import (
"os"
"github.com/pkg/sftp"
"golang.org/x/sys/unix"
)
func getStatFS(path string) (*sftp.StatVFS, error) {
func getStatFS(f *os.File, _ string) (*sftp.StatVFS, error) {
stat := unix.Statfs_t{}
err := unix.Statfs(path, &stat)
err := unix.Fstatfs(int(f.Fd()), &stat)
if err != nil {
return nil, err
}
+158 -16
View File
@@ -64,9 +64,11 @@ var (
// ErrStorageSizeUnavailable is returned if the storage backend does not support getting the size
ErrStorageSizeUnavailable = errors.New("unable to get available size for this storage backend")
// ErrVfsUnsupported defines the error for an unsupported VFS operation
ErrVfsUnsupported = errors.New("not supported")
ErrVfsUnsupported = errors.New("not supported")
// ErrCrossRename is returned by a backend when a rename cannot be performed as a
// single operation because source and target live in different confinement roots.
ErrCrossRename = errors.New("cross-root rename")
errInvalidDirListerLimit = errors.New("dir lister: invalid limit, must be > 0")
tempPath string
sftpFingerprints []string
allowSelfConnections int
renameMode int
@@ -87,16 +89,6 @@ func SetAllowSelfConnections(value int) {
allowSelfConnections = value
}
// SetTempPath sets the path for temporary files
func SetTempPath(fsPath string) {
tempPath = fsPath
}
// GetTempPath returns the path for temporary files
func GetTempPath() string {
return tempPath
}
// SetSFTPFingerprints sets the SFTP host key fingerprints
func SetSFTPFingerprints(fp []string) {
sftpFingerprints = fp
@@ -1162,7 +1154,7 @@ func SetPathPermissions(fs Fs, path string, uid int, gid int) {
if uid == -1 && gid == -1 {
return
}
if IsLocalOsFs(fs) {
if IsLocalOrCryptoFs(fs) {
if runtime.GOOS == "windows" {
return
}
@@ -1257,12 +1249,16 @@ func getMountPath(mountPath string) string {
}
func getLocalTempDir() string {
if tempPath != "" {
return tempPath
}
return filepath.Clean(os.TempDir())
}
// isRootEscapeError reports whether err is the os.Root boundary violation
// ("path escapes from parent"). The os package does not export the sentinel
// (errPathEscapes), so it is matched by message.
func isRootEscapeError(err error) bool {
return err != nil && strings.Contains(err.Error(), "path escapes from parent")
}
func doRecursiveRename(fs Fs, source, target string,
renameFn func(string, string, os.FileInfo, int, bool) (int, int64, error),
recursion int, updateModTime bool,
@@ -1307,6 +1303,152 @@ func doRecursiveRename(fs Fs, source, target string,
}
}
// RenameAcrossRoots moves source on fsSrc to target on fsDst. It is the
// fallback for ErrCrossRename. Each regular file is moved by copying it to the
// destination and then removing it from the source, mirroring the cloud rename;
// the move is not atomic. Symbolic links and other non-regular files are not
// recreated at the destination, they are dropped from the source.
func RenameAcrossRoots(fsSrc, fsDst Fs, source, target string, srcInfo os.FileInfo, checks, uid, gid int) (int, int64, error) {
return moveAcrossRoots(fsSrc, fsDst, source, target, srcInfo, checks, uid, gid, 0)
}
func moveAcrossRoots(fsSrc, fsDst Fs, source, target string, info os.FileInfo, checks, uid, gid, recursion int) (int, int64, error) { //nolint:gocyclo
var numFiles int
var filesSize int64
if info.IsDir() {
if recursion > util.MaxRecursion {
return numFiles, filesSize, util.ErrRecursionTooDeep
}
recursion++
if err := fsDst.Mkdir(target); err != nil && !errors.Is(err, os.ErrExist) {
return numFiles, filesSize, err
}
SetPathPermissions(fsDst, target, uid, gid)
lister, err := fsSrc.ReadDir(source)
if err != nil {
return numFiles, filesSize, err
}
defer lister.Close()
for {
entries, err := lister.Next(ListerBatchSize)
finished := errors.Is(err, io.EOF)
if err != nil && !finished {
return numFiles, filesSize, err
}
for _, entry := range entries {
files, size, err := moveAcrossRoots(fsSrc, fsDst, fsSrc.Join(source, entry.Name()),
fsDst.Join(target, entry.Name()), entry, checks, uid, gid, recursion)
numFiles += files
filesSize += size
if err != nil {
return numFiles, filesSize, err
}
}
if finished {
// the contents are moved, remove the now-empty source directory
lister.Close()
return numFiles, filesSize, fsSrc.Remove(source, info.IsDir())
}
}
}
if !info.Mode().IsRegular() {
// a symbolic link (or other non-regular entry) is not recreated at the
// destination on a cross-root move, it is dropped from the source. Recreating an
// attacker-controlled link under a different root could reintroduce a confinement
// escape, so it is intentionally not preserved; losing a link is not data loss.
fsLog(fsSrc, logger.LevelWarn, "non-regular file %q (mode %s) is not preserved on a cross-root move",
source, info.Mode().String())
return numFiles, filesSize, fsSrc.Remove(source, info.IsDir())
}
if err := copyFileAcrossRoots(fsSrc, fsDst, source, target); err != nil {
return numFiles, filesSize, err
}
SetPathPermissions(fsDst, target, uid, gid)
if checks&CheckUpdateModTime == 0 {
// Best effort: a metadata failure must not fail the whole move
// (directory mtimes are intentionally not restored, they are rarely
// relied upon and would need to be set after their contents to survive
// the child writes).
if mtime := info.ModTime(); !mtime.IsZero() {
fsDst.Chtimes(target, mtime, mtime, false) //nolint:errcheck
}
}
// account the on-disk size actually written under the destination root, not
// info.Size(): quota is tracked as the stored size (uploads use fs.Stat, quota
// scans use GetDirSize) but info may carry a converted size. In particular
// CryptFs.ReadDir reports the decrypted size.
size := info.Size()
if fi, statErr := fsDst.Stat(target); statErr == nil {
size = fi.Size()
}
if err := fsSrc.Remove(source, info.IsDir()); err != nil {
return numFiles, filesSize, err
}
return numFiles + 1, filesSize + size, nil
}
func copyFileAcrossRoots(fsSrc, fsDst Fs, source, target string) error {
f, r, cancelR, err := fsSrc.Open(source, 0)
if err != nil {
return err
}
var reader io.Reader
var readCloser io.Closer
if r != nil {
reader, readCloser = r, r
} else {
reader, readCloser = f, f
}
if fi, err := fsDst.Lstat(target); err == nil && fi.Mode()&os.ModeSymlink != 0 {
// If the target is a symlink remove it, otherwise Create will replace
// the linked file.
fsDst.Remove(target, false) //nolint:errcheck
}
wf, w, cancelW, err := fsDst.Create(target, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0)
if err != nil {
readCloser.Close()
if cancelR != nil {
cancelR()
}
return err
}
var writer io.Writer
var writeCloser io.Closer
if w != nil {
writer, writeCloser = w, w
} else {
writer, writeCloser = wf, wf
}
_, copyErr := doCopy(writer, reader, nil)
if copyErr != nil {
if cancelR != nil {
cancelR()
}
if cancelW != nil {
cancelW()
}
readCloser.Close()
writeCloser.Close() //nolint:errcheck
// Best effort removal of a partial copy.
fsDst.Remove(target, false) //nolint:errcheck
return copyErr
}
closeErr := writeCloser.Close()
readCloser.Close()
if cancelR != nil {
cancelR()
}
if cancelW != nil {
cancelW()
}
if closeErr != nil {
// Best effort removal of a partial copy.
fsDst.Remove(target, false) //nolint:errcheck
return copyErr
}
return nil
}
// copied from rclone
func readFill(r io.Reader, buf []byte) (n int, err error) {
var nn int
+3 -1
View File
@@ -583,7 +583,9 @@ func TestFileAccessErrors(t *testing.T) {
p := filepath.Join(user.HomeDir, "adir", missingPath)
_, err = connection.handleUploadToNewFile(fs, p, p, path.Join("adir", missingPath))
assert.ErrorIs(t, err, os.ErrNotExist)
_, err = connection.handleUploadToExistingFile(fs, p, "_"+p, 0, path.Join("adir", missingPath))
// the atomic upload temp path lives in the same directory as the target
pTemp := filepath.Join(filepath.Dir(p), "_"+filepath.Base(p))
_, err = connection.handleUploadToExistingFile(fs, p, pTemp, 0, path.Join("adir", missingPath))
if assert.Error(t, err) {
assert.ErrorIs(t, err, os.ErrNotExist)
}
-1
View File
@@ -12,7 +12,6 @@
"symlink_mode": 0,
"resume_max_size": 0,
"secret_min_entropy": 80,
"temp_path": "",
"proxy_protocol": 0,
"proxy_allowed": [],
"proxy_skipped": [],