cryptfs: add support for atomic uploads

devopsmakers · May 31, 2021 · c63b923 · c63b923
1 parent 574c402
commit c63b923
Show file tree

Hide file tree

Showing 17 changed files with 112 additions and 102 deletions.
diff --git a/common/common.go b/common/common.go
@@ -151,6 +151,7 @@ func Initialize(c Configuration) error {
 		}
 	}
 	vfs.SetTempPath(c.TempPath)
+	dataprovider.SetTempPath(c.TempPath)
 	return nil
 }
 

diff --git a/common/common_test.go b/common/common_test.go
@@ -435,9 +435,9 @@ func TestConnectionStatus(t *testing.T) {
 	fakeConn1 := &fakeConnection{
 		BaseConnection: c1,
 	}
-	t1 := NewBaseTransfer(nil, c1, nil, "/p1", "/r1", TransferUpload, 0, 0, 0, true, fs)
+	t1 := NewBaseTransfer(nil, c1, nil, "/p1", "/p1", "/r1", TransferUpload, 0, 0, 0, true, fs)
 	t1.BytesReceived = 123
-	t2 := NewBaseTransfer(nil, c1, nil, "/p2", "/r2", TransferDownload, 0, 0, 0, true, fs)
+	t2 := NewBaseTransfer(nil, c1, nil, "/p2", "/p2", "/r2", TransferDownload, 0, 0, 0, true, fs)
 	t2.BytesSent = 456
 	c2 := NewBaseConnection("id2", ProtocolSSH, user)
 	fakeConn2 := &fakeConnection{
@@ -449,7 +449,7 @@ func TestConnectionStatus(t *testing.T) {
 		BaseConnection: c3,
 		command:        "PROPFIND",
 	}
-	t3 := NewBaseTransfer(nil, c3, nil, "/p2", "/r2", TransferDownload, 0, 0, 0, true, fs)
+	t3 := NewBaseTransfer(nil, c3, nil, "/p2", "/p2", "/r2", TransferDownload, 0, 0, 0, true, fs)
 	Connections.Add(fakeConn1)
 	Connections.Add(fakeConn2)
 	Connections.Add(fakeConn3)

diff --git a/common/transfer.go b/common/transfer.go
@@ -20,46 +20,48 @@ var (
 
 // BaseTransfer contains protocols common transfer details for an upload or a download.
 type BaseTransfer struct { //nolint:maligned
-	ID             uint64
-	BytesSent      int64
-	BytesReceived  int64
-	Fs             vfs.Fs
-	File           vfs.File
-	Connection     *BaseConnection
-	cancelFn       func()
-	fsPath         string
-	requestPath    string
-	start          time.Time
-	MaxWriteSize   int64
-	MinWriteOffset int64
-	InitialSize    int64
-	isNewFile      bool
-	transferType   int
-	AbortTransfer  int32
+	ID              uint64
+	BytesSent       int64
+	BytesReceived   int64
+	Fs              vfs.Fs
+	File            vfs.File
+	Connection      *BaseConnection
+	cancelFn        func()
+	fsPath          string
+	effectiveFsPath string
+	requestPath     string
+	start           time.Time
+	MaxWriteSize    int64
+	MinWriteOffset  int64
+	InitialSize     int64
+	isNewFile       bool
+	transferType    int
+	AbortTransfer   int32
 	sync.Mutex
 	ErrTransfer error
 }
 
 // NewBaseTransfer returns a new BaseTransfer and adds it to the given connection
-func NewBaseTransfer(file vfs.File, conn *BaseConnection, cancelFn func(), fsPath, requestPath string, transferType int,
-	minWriteOffset, initialSize, maxWriteSize int64, isNewFile bool, fs vfs.Fs) *BaseTransfer {
+func NewBaseTransfer(file vfs.File, conn *BaseConnection, cancelFn func(), fsPath, effectiveFsPath, requestPath string,
+	transferType int, minWriteOffset, initialSize, maxWriteSize int64, isNewFile bool, fs vfs.Fs) *BaseTransfer {
 	t := &BaseTransfer{
-		ID:             conn.GetTransferID(),
-		File:           file,
-		Connection:     conn,
-		cancelFn:       cancelFn,
-		fsPath:         fsPath,
-		start:          time.Now(),
-		transferType:   transferType,
-		MinWriteOffset: minWriteOffset,
-		InitialSize:    initialSize,
-		isNewFile:      isNewFile,
-		requestPath:    requestPath,
-		BytesSent:      0,
-		BytesReceived:  0,
-		MaxWriteSize:   maxWriteSize,
-		AbortTransfer:  0,
-		Fs:             fs,
+		ID:              conn.GetTransferID(),
+		File:            file,
+		Connection:      conn,
+		cancelFn:        cancelFn,
+		fsPath:          fsPath,
+		effectiveFsPath: effectiveFsPath,
+		start:           time.Now(),
+		transferType:    transferType,
+		MinWriteOffset:  minWriteOffset,
+		InitialSize:     initialSize,
+		isNewFile:       isNewFile,
+		requestPath:     requestPath,
+		BytesSent:       0,
+		BytesReceived:   0,
+		MaxWriteSize:    maxWriteSize,
+		AbortTransfer:   0,
+		Fs:              fs,
 	}
 
 	conn.AddTransfer(t)
@@ -215,15 +217,15 @@ func (t *BaseTransfer) Close() error {
 		}
 		t.Connection.Log(logger.LevelWarn, "upload denied due to space limit, delete temporary file: %#v, deletion error: %v",
 			t.File.Name(), err)
-	} else if t.transferType == TransferUpload && t.File != nil && t.File.Name() != t.fsPath {
+	} else if t.transferType == TransferUpload && t.effectiveFsPath != t.fsPath {
 		if t.ErrTransfer == nil || Config.UploadMode == UploadModeAtomicWithResume {
-			err = t.Fs.Rename(t.File.Name(), t.fsPath)
+			err = t.Fs.Rename(t.effectiveFsPath, t.fsPath)
 			t.Connection.Log(logger.LevelDebug, "atomic upload completed, rename: %#v -> %#v, error: %v",
-				t.File.Name(), t.fsPath, err)
+				t.effectiveFsPath, t.fsPath, err)
 		} else {
-			err = t.Fs.Remove(t.File.Name(), false)
+			err = t.Fs.Remove(t.effectiveFsPath, false)
 			t.Connection.Log(logger.LevelWarn, "atomic upload completed with error: \"%v\", delete temporary file: %#v, "+
-				"deletion error: %v", t.ErrTransfer, t.File.Name(), err)
+				"deletion error: %v", t.ErrTransfer, t.effectiveFsPath, err)
 			if err == nil {
 				numFiles--
 				atomic.StoreInt64(&t.BytesReceived, 0)

diff --git a/common/transfer_test.go b/common/transfer_test.go
@@ -62,7 +62,7 @@ func TestTransferThrottling(t *testing.T) {
 	wantedUploadElapsed -= wantedDownloadElapsed / 10
 	wantedDownloadElapsed -= wantedDownloadElapsed / 10
 	conn := NewBaseConnection("id", ProtocolSCP, u)
-	transfer := NewBaseTransfer(nil, conn, nil, "", "", TransferUpload, 0, 0, 0, true, fs)
+	transfer := NewBaseTransfer(nil, conn, nil, "", "", "", TransferUpload, 0, 0, 0, true, fs)
 	transfer.BytesReceived = testFileSize
 	transfer.Connection.UpdateLastActivity()
 	startTime := transfer.Connection.GetLastActivity()
@@ -72,7 +72,7 @@ func TestTransferThrottling(t *testing.T) {
 	err := transfer.Close()
 	assert.NoError(t, err)
 
-	transfer = NewBaseTransfer(nil, conn, nil, "", "", TransferDownload, 0, 0, 0, true, fs)
+	transfer = NewBaseTransfer(nil, conn, nil, "", "", "", TransferDownload, 0, 0, 0, true, fs)
 	transfer.BytesSent = testFileSize
 	transfer.Connection.UpdateLastActivity()
 	startTime = transfer.Connection.GetLastActivity()
@@ -96,7 +96,7 @@ func TestRealPath(t *testing.T) {
 	file, err := os.Create(testFile)
 	require.NoError(t, err)
 	conn := NewBaseConnection(fs.ConnectionID(), ProtocolSFTP, u)
-	transfer := NewBaseTransfer(file, conn, nil, testFile, "/transfer_test_file", TransferUpload, 0, 0, 0, true, fs)
+	transfer := NewBaseTransfer(file, conn, nil, testFile, testFile, "/transfer_test_file", TransferUpload, 0, 0, 0, true, fs)
 	rPath := transfer.GetRealFsPath(testFile)
 	assert.Equal(t, testFile, rPath)
 	rPath = conn.getRealFsPath(testFile)
@@ -131,7 +131,7 @@ func TestTruncate(t *testing.T) {
 	_, err = file.Write([]byte("hello"))
 	assert.NoError(t, err)
 	conn := NewBaseConnection(fs.ConnectionID(), ProtocolSFTP, u)
-	transfer := NewBaseTransfer(file, conn, nil, testFile, "/transfer_test_file", TransferUpload, 0, 5, 100, false, fs)
+	transfer := NewBaseTransfer(file, conn, nil, testFile, testFile, "/transfer_test_file", TransferUpload, 0, 5, 100, false, fs)
 
 	err = conn.SetStat("/transfer_test_file", &StatAttributes{
 		Size:  2,
@@ -148,7 +148,7 @@ func TestTruncate(t *testing.T) {
 		assert.Equal(t, int64(2), fi.Size())
 	}
 
-	transfer = NewBaseTransfer(file, conn, nil, testFile, "/transfer_test_file", TransferUpload, 0, 0, 100, true, fs)
+	transfer = NewBaseTransfer(file, conn, nil, testFile, testFile, "/transfer_test_file", TransferUpload, 0, 0, 100, true, fs)
 	// file.Stat will fail on a closed file
 	err = conn.SetStat("/transfer_test_file", &StatAttributes{
 		Size:  2,
@@ -158,7 +158,7 @@ func TestTruncate(t *testing.T) {
 	err = transfer.Close()
 	assert.NoError(t, err)
 
-	transfer = NewBaseTransfer(nil, conn, nil, testFile, "", TransferUpload, 0, 0, 0, true, fs)
+	transfer = NewBaseTransfer(nil, conn, nil, testFile, testFile, "", TransferUpload, 0, 0, 0, true, fs)
 	_, err = transfer.Truncate("mismatch", 0)
 	assert.EqualError(t, err, errTransferMismatch.Error())
 	_, err = transfer.Truncate(testFile, 0)
@@ -193,7 +193,7 @@ func TestTransferErrors(t *testing.T) {
 		assert.FailNow(t, "unable to open test file")
 	}
 	conn := NewBaseConnection("id", ProtocolSFTP, u)
-	transfer := NewBaseTransfer(file, conn, nil, testFile, "/transfer_test_file", TransferUpload, 0, 0, 0, true, fs)
+	transfer := NewBaseTransfer(file, conn, nil, testFile, testFile, "/transfer_test_file", TransferUpload, 0, 0, 0, true, fs)
 	assert.Nil(t, transfer.cancelFn)
 	assert.Equal(t, testFile, transfer.GetFsPath())
 	transfer.SetCancelFn(cancelFn)
@@ -219,7 +219,7 @@ func TestTransferErrors(t *testing.T) {
 		assert.FailNow(t, "unable to open test file")
 	}
 	fsPath := filepath.Join(os.TempDir(), "test_file")
-	transfer = NewBaseTransfer(file, conn, nil, fsPath, "/test_file", TransferUpload, 0, 0, 0, true, fs)
+	transfer = NewBaseTransfer(file, conn, nil, fsPath, file.Name(), "/test_file", TransferUpload, 0, 0, 0, true, fs)
 	transfer.BytesReceived = 9
 	transfer.TransferError(errFake)
 	assert.Error(t, transfer.ErrTransfer, errFake.Error())
@@ -238,7 +238,7 @@ func TestTransferErrors(t *testing.T) {
 	if !assert.NoError(t, err) {
 		assert.FailNow(t, "unable to open test file")
 	}
-	transfer = NewBaseTransfer(file, conn, nil, fsPath, "/test_file", TransferUpload, 0, 0, 0, true, fs)
+	transfer = NewBaseTransfer(file, conn, nil, fsPath, file.Name(), "/test_file", TransferUpload, 0, 0, 0, true, fs)
 	transfer.BytesReceived = 9
 	// the file is closed from the embedding struct before to call close
 	err = file.Close()
@@ -262,7 +262,7 @@ func TestRemovePartialCryptoFile(t *testing.T) {
 		HomeDir:  os.TempDir(),
 	}
 	conn := NewBaseConnection(fs.ConnectionID(), ProtocolSFTP, u)
-	transfer := NewBaseTransfer(nil, conn, nil, testFile, "/transfer_test_file", TransferUpload, 0, 0, 0, true, fs)
+	transfer := NewBaseTransfer(nil, conn, nil, testFile, testFile, "/transfer_test_file", TransferUpload, 0, 0, 0, true, fs)
 	transfer.ErrTransfer = errors.New("test error")
 	_, err = transfer.getUploadFileSize()
 	assert.Error(t, err)

diff --git a/dataprovider/dataprovider.go b/dataprovider/dataprovider.go
@@ -141,6 +141,7 @@ var (
 	argon2Params            *argon2id.Params
 	lastLoginMinDelay       = 10 * time.Minute
 	usernameRegex           = regexp.MustCompile("^[a-zA-Z0-9-_.~]+$")
+	tempPath                string
 )
 
 type schemaVersion struct {
@@ -449,6 +450,11 @@ type fsValidatorHelper interface {
 	GetEncrytionAdditionalData() string
 }
 
+// SetTempPath sets the path for temporary files
+func SetTempPath(fsPath string) {
+	tempPath = fsPath
+}
+
 // Initialize the data provider.
 // An error is returned if the configured driver is invalid or if the data provider cannot be initialized
 func Initialize(cnf Config, basePath string, checkAdmins bool) error {
@@ -1097,8 +1103,15 @@ func buildUserHomeDir(user *User) {
 	if user.HomeDir == "" {
 		if config.UsersBaseDir != "" {
 			user.HomeDir = filepath.Join(config.UsersBaseDir, user.Username)
-		} else if user.FsConfig.Provider == vfs.SFTPFilesystemProvider {
-			user.HomeDir = filepath.Join(os.TempDir(), user.Username)
+			return
+		}
+		switch user.FsConfig.Provider {
+		case vfs.SFTPFilesystemProvider, vfs.S3FilesystemProvider, vfs.AzureBlobFilesystemProvider, vfs.GCSFilesystemProvider:
+			if tempPath != "" {
+				user.HomeDir = filepath.Join(tempPath, user.Username)
+			} else {
+				user.HomeDir = filepath.Join(os.TempDir(), user.Username)
+			}
 		}
 	}
 }

diff --git a/docs/dare.md b/docs/dare.md
@@ -15,5 +15,4 @@ The encrypted filesystem has some limitations compared to the local, unencrypted
 - Resuming uploads is not supported.
 - Opening a file for both reading and writing at the same time is not supported and so clients that require advanced filesystem-like features such as `sshfs` are not supported too.
 - Truncate is not supported.
-- Atomic uploads are not supported.
 - System commands such as `git` or `rsync` are not supported: they will store data unencrypted.
diff --git a/ftpd/handler.go b/ftpd/handler.go
@@ -308,7 +308,7 @@ func (c *Connection) downloadFile(fs vfs.Fs, fsPath, ftpPath string, offset int6
 		return nil, c.GetFsError(fs, err)
 	}
 
-	baseTransfer := common.NewBaseTransfer(file, c.BaseConnection, cancelFn, fsPath, ftpPath, common.TransferDownload,
+	baseTransfer := common.NewBaseTransfer(file, c.BaseConnection, cancelFn, fsPath, fsPath, ftpPath, common.TransferDownload,
 		0, 0, 0, false, fs)
 	t := newTransfer(baseTransfer, nil, r, offset)
 
@@ -373,7 +373,7 @@ func (c *Connection) handleFTPUploadToNewFile(fs vfs.Fs, resolvedPath, filePath,
 	// we can get an error only for resume
 	maxWriteSize, _ := c.GetMaxWriteSize(quotaResult, false, 0, fs.IsUploadResumeSupported())
 
-	baseTransfer := common.NewBaseTransfer(file, c.BaseConnection, cancelFn, resolvedPath, requestPath,
+	baseTransfer := common.NewBaseTransfer(file, c.BaseConnection, cancelFn, resolvedPath, filePath, requestPath,
 		common.TransferUpload, 0, 0, maxWriteSize, true, fs)
 	t := newTransfer(baseTransfer, w, nil, 0)
 
@@ -449,7 +449,7 @@ func (c *Connection) handleFTPUploadToExistingFile(fs vfs.Fs, flags int, resolve
 
 	vfs.SetPathPermissions(fs, filePath, c.User.GetUID(), c.User.GetGID())
 
-	baseTransfer := common.NewBaseTransfer(file, c.BaseConnection, cancelFn, resolvedPath, requestPath,
+	baseTransfer := common.NewBaseTransfer(file, c.BaseConnection, cancelFn, resolvedPath, filePath, requestPath,
 		common.TransferUpload, minWriteOffset, initialSize, maxWriteSize, false, fs)
 	t := newTransfer(baseTransfer, w, nil, 0)
 

diff --git a/ftpd/internal_test.go b/ftpd/internal_test.go
@@ -724,8 +724,8 @@ func TestTransferErrors(t *testing.T) {
 		BaseConnection: common.NewBaseConnection(connID, common.ProtocolFTP, user),
 		clientContext:  mockCC,
 	}
-	baseTransfer := common.NewBaseTransfer(file, connection.BaseConnection, nil, file.Name(), testfile, common.TransferDownload,
-		0, 0, 0, false, fs)
+	baseTransfer := common.NewBaseTransfer(file, connection.BaseConnection, nil, file.Name(), file.Name(), testfile,
+		common.TransferDownload, 0, 0, 0, false, fs)
 	tr := newTransfer(baseTransfer, nil, nil, 0)
 	err = tr.Close()
 	assert.NoError(t, err)
@@ -742,7 +742,7 @@ func TestTransferErrors(t *testing.T) {
 
 	r, _, err := pipeat.Pipe()
 	assert.NoError(t, err)
-	baseTransfer = common.NewBaseTransfer(nil, connection.BaseConnection, nil, testfile, testfile,
+	baseTransfer = common.NewBaseTransfer(nil, connection.BaseConnection, nil, testfile, testfile, testfile,
 		common.TransferUpload, 0, 0, 0, false, fs)
 	tr = newTransfer(baseTransfer, nil, r, 10)
 	pos, err := tr.Seek(10, 0)
@@ -754,7 +754,7 @@ func TestTransferErrors(t *testing.T) {
 	r, w, err := pipeat.Pipe()
 	assert.NoError(t, err)
 	pipeWriter := vfs.NewPipeWriter(w)
-	baseTransfer = common.NewBaseTransfer(nil, connection.BaseConnection, nil, testfile, testfile,
+	baseTransfer = common.NewBaseTransfer(nil, connection.BaseConnection, nil, testfile, testfile, testfile,
 		common.TransferUpload, 0, 0, 0, false, fs)
 	tr = newTransfer(baseTransfer, pipeWriter, nil, 0)
 

diff --git a/httpd/handler.go b/httpd/handler.go
@@ -105,7 +105,7 @@ func (c *Connection) getFileReader(name string, offset int64, method string) (io
 		return nil, c.GetFsError(fs, err)
 	}
 
-	baseTransfer := common.NewBaseTransfer(file, c.BaseConnection, cancelFn, p, name, common.TransferDownload,
+	baseTransfer := common.NewBaseTransfer(file, c.BaseConnection, cancelFn, p, p, name, common.TransferDownload,
 		0, 0, 0, false, fs)
 	return newHTTPDFile(baseTransfer, r), nil
 }
diff --git a/httpd/internal_test.go b/httpd/internal_test.go
@@ -1428,7 +1428,7 @@ func TestHTTPDFile(t *testing.T) {
 	err = file.Close()
 	assert.NoError(t, err)
 
-	baseTransfer := common.NewBaseTransfer(file, connection.BaseConnection, nil, p, name, common.TransferDownload,
+	baseTransfer := common.NewBaseTransfer(file, connection.BaseConnection, nil, p, p, name, common.TransferDownload,
 		0, 0, 0, false, fs)
 	httpdFile := newHTTPDFile(baseTransfer, nil)
 	// the file is closed, read should fail

diff --git a/sftpd/handler.go b/sftpd/handler.go
@@ -70,7 +70,7 @@ func (c *Connection) Fileread(request *sftp.Request) (io.ReaderAt, error) {
 		return nil, c.GetFsError(fs, err)
 	}
 
-	baseTransfer := common.NewBaseTransfer(file, c.BaseConnection, cancelFn, p, request.Filepath, common.TransferDownload,
+	baseTransfer := common.NewBaseTransfer(file, c.BaseConnection, cancelFn, p, p, request.Filepath, common.TransferDownload,
 		0, 0, 0, false, fs)
 	t := newTransfer(baseTransfer, nil, r, nil)
 
@@ -346,7 +346,7 @@ func (c *Connection) handleSFTPUploadToNewFile(fs vfs.Fs, resolvedPath, filePath
 	// we can get an error only for resume
 	maxWriteSize, _ := c.GetMaxWriteSize(quotaResult, false, 0, fs.IsUploadResumeSupported())
 
-	baseTransfer := common.NewBaseTransfer(file, c.BaseConnection, cancelFn, resolvedPath, requestPath,
+	baseTransfer := common.NewBaseTransfer(file, c.BaseConnection, cancelFn, resolvedPath, filePath, requestPath,
 		common.TransferUpload, 0, 0, maxWriteSize, true, fs)
 	t := newTransfer(baseTransfer, w, nil, errForRead)
 
@@ -423,7 +423,7 @@ func (c *Connection) handleSFTPUploadToExistingFile(fs vfs.Fs, pflags sftp.FileO
 
 	vfs.SetPathPermissions(fs, filePath, c.User.GetUID(), c.User.GetGID())
 
-	baseTransfer := common.NewBaseTransfer(file, c.BaseConnection, cancelFn, resolvedPath, requestPath,
+	baseTransfer := common.NewBaseTransfer(file, c.BaseConnection, cancelFn, resolvedPath, filePath, requestPath,
 		common.TransferUpload, minWriteOffset, initialSize, maxWriteSize, false, fs)
 	t := newTransfer(baseTransfer, w, nil, errForRead)
-Original file line number
+Diff line change
@@ Expand Up / @@ -151,6 +151,7 @@ func Initialize(c Configuration) error { @@
     		}
     	}
     	vfs.SetTempPath(c.TempPath)
+    	dataprovider.SetTempPath(c.TempPath)
     	return nil
     }
@@ Expand Down @@