all: Copy owner/group from parent (fixes #5445) (#5479)

This adds a folder option "CopyOwnershipFromParent" which, when set,
makes Syncthing attempt to retain the owner/group information when
syncing files. Specifically, at the finisher stage we look at the parent
dir to get owner/group and then attempt a Lchown call on the temp file.
For this to succeed Syncthing must be running with the appropriate
permissions. On Linux this is CAP_FOWNER, which can be granted by the
service manager on startup or set on the binary in the filesystem. Other
operating systems do other things, but often it's not required to run as
full "root". On Windows this patch does nothing - ownership works
differently there and is generally less of a deal, as permissions are
inherited as ACLs anyway.

There are unit tests on the Lchown functionality, which requires the
above permissions to run. There is also a unit test on the folder which
uses the fake filesystem and hence does not need special permissions.

lib/model/folder_sendrecv.go

@@ -8,7 +8,6 @@ package model
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
 	"math/rand"
 	"path/filepath"
@@ -17,6 +16,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/pkg/errors"
+
 	"github.com/syncthing/syncthing/lib/config"
 	"github.com/syncthing/syncthing/lib/db"
 	"github.com/syncthing/syncthing/lib/events"
@@ -587,6 +588,11 @@ func (f *sendReceiveFolder) handleDir(file protocol.FileInfo, dbUpdateChan chan<
 				return err
 			}
 
+			// Copy the parent owner and group, if we are supposed to do that.
+			if err := f.maybeCopyOwner(path); err != nil {
+				return err
+			}
+
 			// Stat the directory so we can check its permissions.
 			info, err := f.fs.Lstat(path)
 			if err != nil {
@@ -707,7 +713,10 @@ func (f *sendReceiveFolder) handleSymlink(file protocol.FileInfo, dbUpdateChan c
 	// We declare a function that acts on only the path name, so
 	// we can pass it to InWritableDir.
 	createLink := func(path string) error {
-		return f.fs.CreateSymlink(file.SymlinkTarget, path)
+		if err := f.fs.CreateSymlink(file.SymlinkTarget, path); err != nil {
+			return err
+		}
+		return f.maybeCopyOwner(path)
 	}
 
 	if err = osutil.InWritableDir(createLink, f.fs, file.Name); err == nil {
@@ -1433,6 +1442,11 @@ func (f *sendReceiveFolder) performFinish(ignores *ignore.Matcher, file, curFile
 		}
 	}
 
+	// Copy the parent owner and group, if we are supposed to do that.
+	if err := f.maybeCopyOwner(tempName); err != nil {
+		return err
+	}
+
 	if stat, err := f.fs.Lstat(file.Name); err == nil {
 		// There is an old file or directory already in place. We need to
 		// handle that.
@@ -1888,6 +1902,26 @@ func (f *sendReceiveFolder) checkToBeDeleted(cur protocol.FileInfo, scanChan cha
 	return nil
 }
 
+func (f *sendReceiveFolder) maybeCopyOwner(path string) error {
+	if !f.CopyOwnershipFromParent {
+		// Not supposed to do anything.
+		return nil
+	}
+	if runtime.GOOS == "windows" {
+		// Can't do anything.
+		return nil
+	}
+
+	info, err := f.fs.Lstat(filepath.Dir(path))
+	if err != nil {
+		return errors.Wrap(err, "copy owner from parent")
+	}
+	if err := f.fs.Lchown(path, info.Owner(), info.Group()); err != nil {
+		return errors.Wrap(err, "copy owner from parent")
+	}
+	return nil
+}
+
 // A []FileError is sent as part of an event and will be JSON serialized.
 type FileError struct {
 	Path string `json:"path"`

lib/model/folder_sendrecv_test.go

@@ -14,6 +14,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"runtime"
 	"testing"
 	"time"
 
@@ -706,7 +707,7 @@ func TestDiffEmpty(t *testing.T) {
 // option is true and the permissions do not match between the file on disk and
 // in the db.
 func TestDeleteIgnorePerms(t *testing.T) {
-	m := setUpModel(protocol.FileInfo{})
+	m := setUpModel()
 	f := setUpSendReceiveFolder(m)
 	f.IgnorePerms = true
 
@@ -743,3 +744,119 @@ func TestDeleteIgnorePerms(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
+func TestCopyOwner(t *testing.T) {
+	// Verifies that owner and group are copied from the parent, for both
+	// files and directories.
+
+	if runtime.GOOS == "windows" {
+		t.Skip("copying owner not supported on Windows")
+	}
+
+	const (
+		expOwner = 1234
+		expGroup = 5678
+	)
+
+	// Set up a folder with the CopyParentOwner bit and backed by a fake
+	// filesystem.
+
+	m := setUpModel()
+	f := &sendReceiveFolder{
+		folder: folder{
+			stateTracker:        newStateTracker("default"),
+			model:               m,
+			initialScanFinished: make(chan struct{}),
+			ctx:                 context.TODO(),
+			FolderConfiguration: config.FolderConfiguration{
+				FilesystemType:          fs.FilesystemTypeFake,
+				Path:                    "/TestCopyOwner",
+				CopyOwnershipFromParent: true,
+			},
+		},
+
+		queue:         newJobQueue(),
+		pullErrors:    make(map[string]string),
+		pullErrorsMut: sync.NewMutex(),
+	}
+
+	f.fs = f.Filesystem()
+
+	// Create a parent dir with a certain owner/group.
+
+	f.fs.Mkdir("foo", 0755)
+	f.fs.Lchown("foo", expOwner, expGroup)
+
+	dir := protocol.FileInfo{
+		Name:        "foo/bar",
+		Type:        protocol.FileInfoTypeDirectory,
+		Permissions: 0755,
+	}
+
+	// Have the folder create a subdirectory, verify that it's the correct
+	// owner/group.
+
+	dbUpdateChan := make(chan dbUpdateJob, 1)
+	defer close(dbUpdateChan)
+	f.handleDir(dir, dbUpdateChan)
+	<-dbUpdateChan // empty the channel for later
+
+	info, err := f.fs.Lstat("foo/bar")
+	if err != nil {
+		t.Fatal("Unexpected error (dir):", err)
+	}
+	if info.Owner() != expOwner || info.Group() != expGroup {
+		t.Fatalf("Expected dir owner/group to be %d/%d, not %d/%d", expOwner, expGroup, info.Owner(), info.Group())
+	}
+
+	// Have the folder create a file, verify it's the correct owner/group.
+	// File is zero sized to avoid having to handle copies/pulls.
+
+	file := protocol.FileInfo{
+		Name:        "foo/bar/baz",
+		Type:        protocol.FileInfoTypeFile,
+		Permissions: 0644,
+	}
+
+	// Wire some stuff. The flow here is handleFile() -[copierChan]->
+	// copierRoutine() -[finisherChan]-> finisherRoutine() -[dbUpdateChan]->
+	// back to us and we're done. The copier routine doesn't do anything,
+	// but it's the way data is passed around. When the database update
+	// comes the finisher is done.
+
+	finisherChan := make(chan *sharedPullerState)
+	defer close(finisherChan)
+	copierChan := make(chan copyBlocksState)
+	defer close(copierChan)
+	go f.copierRoutine(copierChan, nil, finisherChan)
+	go f.finisherRoutine(nil, finisherChan, dbUpdateChan, nil)
+	f.handleFile(file, copierChan, nil, nil)
+	<-dbUpdateChan
+
+	info, err = f.fs.Lstat("foo/bar/baz")
+	if err != nil {
+		t.Fatal("Unexpected error (file):", err)
+	}
+	if info.Owner() != expOwner || info.Group() != expGroup {
+		t.Fatalf("Expected file owner/group to be %d/%d, not %d/%d", expOwner, expGroup, info.Owner(), info.Group())
+	}
+
+	// Have the folder create a symlink. Verify it accordingly.
+	symlink := protocol.FileInfo{
+		Name:          "foo/bar/sym",
+		Type:          protocol.FileInfoTypeSymlink,
+		Permissions:   0644,
+		SymlinkTarget: "over the rainbow",
+	}
+
+	f.handleSymlink(symlink, dbUpdateChan)
+	<-dbUpdateChan
+
+	info, err = f.fs.Lstat("foo/bar/sym")
+	if err != nil {
+		t.Fatal("Unexpected error (file):", err)
+	}
+	if info.Owner() != expOwner || info.Group() != expGroup {
+		t.Fatalf("Expected symlink owner/group to be %d/%d, not %d/%d", expOwner, expGroup, info.Owner(), info.Group())
+	}
+}