Merge pull request #3419 from DanielG/relax-file-modes

Enable admin to decide group access to repository files

changelog/unreleased/pull-3419

@@ -0,0 +1,21 @@
+Enhancement: Use config file permissions to control file group access
+
+Previously files in a local/sftp restic repository would always end up with
+very restrictive access permissions allowing access only to the owner. This
+prevented a number of valid use-cases involving groups and ACLs.
+
+Now we use the config file permissions to decide whether group access
+should be given to newly created repository files or not. We arrange for
+repository files to be created group readable exactly when the repository
+config file is group readable.
+
+To opt-in to group readable repositories a simple `chmod -R g+r` or
+equivalent can be used. For repositories that should be writable by group
+members a tad more setup is required, see the docs.
+
+Posix ACLs can also be used now that the group permissions being forced to
+zero no longer masks the effect of ACL entries.
+
+https://github.com/restic/restic/issues/2351
+https://github.com/restic/restic/pull/3419
+https://forum.restic.net/t/change-permissions-on-repository-files/1391

doc/030_preparing_a_new_repo.rst

@@ -699,3 +699,56 @@ On MSYS2, you can install ``winpty`` as follows:
 
     $ pacman -S winpty
     $ winpty restic -r /srv/restic-repo init
+
+
+Group accessible repositories
+*****************************
+
+Since restic version 0.14 local and SFTP repositories can be made
+accessible to members of a system group. To control this we have to change
+the group permissions of the top-level ``config`` file and restic will use
+this as a hint to determine what permissions to apply to newly created
+files. By default ``restic init`` sets repositories up to be group
+inaccessible.
+
+In order to give group members read-only access we simply add the read
+permission bit to all repository files with ``chmod``:
+
+.. code-block:: console
+
+    $ chmod -R g+r /srv/restic-repo
+
+This serves two purposes: 1) it sets the read permission bit on the
+repository config file triggering restic's logic to create new files as
+group accessible and 2) it actually allows the group read access to the
+files.
+
+.. note:: By default files on Unix systems are created with a user's
+          primary group as defined by the gid (group id) field in
+          ``/etc/passwd``. See `passwd(5)
+          <https://manpages.debian.org/latest/passwd/passwd.5.en.html>`_.
+
+For read-write access things are a bit more complicated. When users other
+than the repository creator add new files in the repository they will be
+group-owned by this user's primary group by default, not that of the
+original repository owner, meaning the original creator wouldn't have
+access to these files. That's hardly what you'd want.
+
+To make this work we can employ the help of the ``setgid`` permission bit
+available on Linux and most other Unix systems. This permission bit makes
+newly created directories inherit both the group owner (gid) and setgid bit
+from the parent directory. Setting this bit requires root but since it
+propagates down to any new directories we only have to do this priviledged
+setup once:
+
+.. code-block:: console
+
+    # find /srv/restic-repo -type d -exec chmod g+s '{}' \;
+    $ chmod -R g+rw /srv/restic-repo
+
+This sets the ``setgid`` bit on all existing directories in the repository
+and then grants read/write permissions for group access.
+
+.. note:: To manage who has access to the repository you can use
+          ``usermod`` on Linux systems, to change which group controls
+          repository access ``chgrp -R`` is your friend.

internal/backend/local/local.go

@@ -24,6 +24,7 @@ type Local struct {
 	Config
 	sem *backend.Semaphore
 	backend.Layout
+	backend.Modes
 }
 
 // ensure statically that *Local implements restic.Backend.
@@ -42,10 +43,15 @@ func open(ctx context.Context, cfg Config) (*Local, error) {
 		return nil, err
 	}
 
+	fi, err := fs.Stat(l.Filename(restic.Handle{Type: restic.ConfigFile}))
+	m := backend.DeriveModesFromFileInfo(fi, err)
+	debug.Log("using (%03O file, %03O dir) permissions", m.File, m.Dir)
+
 	return &Local{
 		Config: cfg,
 		Layout: l,
 		sem:    sem,
+		Modes:  m,
 	}, nil
 }
 
@@ -73,7 +79,7 @@ func Create(ctx context.Context, cfg Config) (*Local, error) {
 
 	// create paths for data and refs
 	for _, d := range be.Paths() {
-		err := fs.MkdirAll(d, backend.Modes.Dir)
+		err := fs.MkdirAll(d, be.Modes.Dir)
 		if err != nil {
 			return nil, errors.WithStack(err)
 		}
@@ -129,7 +135,7 @@ func (b *Local) Save(ctx context.Context, h restic.Handle, rd restic.RewindReade
 		debug.Log("error %v: creating dir", err)
 
 		// error is caused by a missing directory, try to create it
-		mkdirErr := fs.MkdirAll(dir, backend.Modes.Dir)
+		mkdirErr := fs.MkdirAll(dir, b.Modes.Dir)
 		if mkdirErr != nil {
 			debug.Log("error creating dir %v: %v", dir, mkdirErr)
 		} else {
@@ -189,7 +195,7 @@ func (b *Local) Save(ctx context.Context, h restic.Handle, rd restic.RewindReade
 	// try to mark file as read-only to avoid accidential modifications
 	// ignore if the operation fails as some filesystems don't allow the chmod call
 	// e.g. exfat and network file systems with certain mount options
-	err = setFileReadonly(finalname, backend.Modes.File)
+	err = setFileReadonly(finalname, b.Modes.File)
 	if err != nil && !os.IsPermission(err) {
 		return errors.WithStack(err)
 	}

internal/backend/paths.go

@@ -21,6 +21,28 @@ var Paths = struct {
 	"config",
 }
 
-// Modes holds the default modes for directories and files for file-based
-// backends.
-var Modes = struct{ Dir, File os.FileMode }{0700, 0600}
+type Modes struct {
+	Dir  os.FileMode
+	File os.FileMode
+}
+
+// DefaultModes defines the default permissions to apply to new repository
+// files and directories stored on file-based backends.
+var DefaultModes = Modes{Dir: 0700, File: 0600}
+
+// DeriveModesFromFileInfo will, given the mode of a regular file, compute
+// the mode we should use for new files and directories. If the passed
+// error is non-nil DefaultModes are returned.
+func DeriveModesFromFileInfo(fi os.FileInfo, err error) Modes {
+	m := DefaultModes
+	if err != nil {
+		return m
+	}
+
+	if fi.Mode()&0040 != 0 { // Group has read access
+		m.Dir |= 0070
+		m.File |= 0060
+	}
+
+	return m
+}

internal/backend/sftp/sftp.go

@@ -34,6 +34,7 @@ type SFTP struct {
 	sem *backend.Semaphore
 	backend.Layout
 	Config
+	backend.Modes
 }
 
 var _ restic.Backend = &SFTP{}
@@ -140,9 +141,14 @@ func Open(ctx context.Context, cfg Config) (*SFTP, error) {
 
 	debug.Log("layout: %v\n", sftp.Layout)
 
+	fi, err := sftp.c.Stat(Join(cfg.Path, backend.Paths.Config))
+	m := backend.DeriveModesFromFileInfo(fi, err)
+	debug.Log("using (%03O file, %03O dir) permissions", m.File, m.Dir)
+
 	sftp.Config = cfg
 	sftp.p = cfg.Path
 	sftp.sem = sem
+	sftp.Modes = m
 	return sftp, nil
 }
 
@@ -225,6 +231,8 @@ func Create(ctx context.Context, cfg Config) (*SFTP, error) {
 		return nil, err
 	}
 
+	sftp.Modes = backend.DefaultModes
+
 	// test if config file already exists
 	_, err = sftp.c.Lstat(Join(cfg.Path, backend.Paths.Config))
 	if err == nil {
@@ -311,7 +319,7 @@ func (r *SFTP) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader
 	// pkg/sftp doesn't allow creating with a mode.
 	// Chmod while the file is still empty.
 	if err == nil {
-		err = f.Chmod(backend.Modes.File)
+		err = f.Chmod(r.Modes.File)
 	}
 	if err != nil {
 		return errors.Wrap(err, "OpenFile")