From 8c02ebb0299cf517aa7a48bfc27bdf1ad5a5b7ba Mon Sep 17 00:00:00 2001 From: Andrew Gunnerson Date: Mon, 19 Jun 2023 14:24:47 -0400 Subject: [PATCH 1/6] Add support for extended attributes on symlinks Linux allows the use of non-`user.` extended attributes on symlinks. One of the main users of this functionality is SELinux's `security.selinux` xattr for storing a path's label. By storing symlink xattrs, restic is now suitable for backing up the root filesystem on Linux distributions that use SELinux. This commit adds support for symlink xattrs when backing up data, restoring data, and mounting snapshots via a fuse mount. All calls to the xattr library have been updated to the use `L` variants of the various functions, which always operate on the path given, without following symlinks. Fixes: #4375 Signed-off-by: Andrew Gunnerson --- changelog/unreleased/issue-4375 | 8 ++++++++ internal/fuse/link.go | 20 ++++++++++++++++++++ internal/restic/node.go | 4 ---- internal/restic/node_xattr.go | 6 +++--- 4 files changed, 31 insertions(+), 7 deletions(-) create mode 100644 changelog/unreleased/issue-4375 diff --git a/changelog/unreleased/issue-4375 b/changelog/unreleased/issue-4375 new file mode 100644 index 000000000..6ce68c2ba --- /dev/null +++ b/changelog/unreleased/issue-4375 @@ -0,0 +1,8 @@ +Enhancement: Add support for extended attributes on symlinks + +Restic now supports extended attributes on symlinks when backing up, +restoring, or FUSE-mounting snapshots. This includes, for example, the +`security.selinux` xattr on Linux distributions that use SELinux. + +https://github.com/restic/restic/issues/4375 +https://github.com/restic/restic/pull/4379 diff --git a/internal/fuse/link.go b/internal/fuse/link.go index c89451602..43b00a855 100644 --- a/internal/fuse/link.go +++ b/internal/fuse/link.go @@ -6,6 +6,8 @@ package fuse import ( "context" + "github.com/restic/restic/internal/debug" + "github.com/anacrolix/fuse" "github.com/anacrolix/fuse/fs" "github.com/restic/restic/internal/restic" @@ -46,3 +48,21 @@ func (l *link) Attr(_ context.Context, a *fuse.Attr) error { return nil } + +func (l *link) Listxattr(_ context.Context, req *fuse.ListxattrRequest, resp *fuse.ListxattrResponse) error { + debug.Log("Listxattr(%v, %v)", l.node.Name, req.Size) + for _, attr := range l.node.ExtendedAttributes { + resp.Append(attr.Name) + } + return nil +} + +func (l *link) Getxattr(_ context.Context, req *fuse.GetxattrRequest, resp *fuse.GetxattrResponse) error { + debug.Log("Getxattr(%v, %v, %v)", l.node.Name, req.Name, req.Size) + attrval := l.node.GetExtendedAttribute(req.Name) + if attrval != nil { + resp.Xattr = attrval + return nil + } + return fuse.ErrNoXattr +} diff --git a/internal/restic/node.go b/internal/restic/node.go index 7d2a1434e..f2d9f2315 100644 --- a/internal/restic/node.go +++ b/internal/restic/node.go @@ -609,10 +609,6 @@ func (node *Node) fillExtra(path string, fi os.FileInfo) error { } func (node *Node) fillExtendedAttributes(path string) error { - if node.Type == "symlink" { - return nil - } - xattrs, err := Listxattr(path) debug.Log("fillExtendedAttributes(%v) %v %v", path, xattrs, err) if err != nil { diff --git a/internal/restic/node_xattr.go b/internal/restic/node_xattr.go index a2eed39c0..ea9eafe94 100644 --- a/internal/restic/node_xattr.go +++ b/internal/restic/node_xattr.go @@ -13,20 +13,20 @@ import ( // Getxattr retrieves extended attribute data associated with path. func Getxattr(path, name string) ([]byte, error) { - b, err := xattr.Get(path, name) + b, err := xattr.LGet(path, name) return b, handleXattrErr(err) } // Listxattr retrieves a list of names of extended attributes associated with the // given path in the file system. func Listxattr(path string) ([]string, error) { - l, err := xattr.List(path) + l, err := xattr.LList(path) return l, handleXattrErr(err) } // Setxattr associates name and data together as an attribute of path. func Setxattr(path, name string, data []byte) error { - return handleXattrErr(xattr.Set(path, name, data)) + return handleXattrErr(xattr.LSet(path, name, data)) } func handleXattrErr(err error) error { From f3c3b0f377ecfed00af10957e145d77ff275ce7d Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 8 Jul 2023 17:41:45 +0200 Subject: [PATCH 2/6] fuse: deduplicate xattr code --- internal/fuse/dir.go | 13 ++----------- internal/fuse/file.go | 13 ++----------- internal/fuse/link.go | 15 ++------------- internal/fuse/xattr.go | 24 ++++++++++++++++++++++++ 4 files changed, 30 insertions(+), 35 deletions(-) create mode 100644 internal/fuse/xattr.go diff --git a/internal/fuse/dir.go b/internal/fuse/dir.go index 7dc157b7e..242b4b03e 100644 --- a/internal/fuse/dir.go +++ b/internal/fuse/dir.go @@ -222,19 +222,10 @@ func (d *dir) Lookup(ctx context.Context, name string) (fs.Node, error) { } func (d *dir) Listxattr(_ context.Context, req *fuse.ListxattrRequest, resp *fuse.ListxattrResponse) error { - debug.Log("Listxattr(%v, %v)", d.node.Name, req.Size) - for _, attr := range d.node.ExtendedAttributes { - resp.Append(attr.Name) - } + nodeToXattrList(d.node, req, resp) return nil } func (d *dir) Getxattr(_ context.Context, req *fuse.GetxattrRequest, resp *fuse.GetxattrResponse) error { - debug.Log("Getxattr(%v, %v, %v)", d.node.Name, req.Name, req.Size) - attrval := d.node.GetExtendedAttribute(req.Name) - if attrval != nil { - resp.Xattr = attrval - return nil - } - return fuse.ErrNoXattr + return nodeGetXattr(d.node, req, resp) } diff --git a/internal/fuse/file.go b/internal/fuse/file.go index fd9d8ccc2..aec39273a 100644 --- a/internal/fuse/file.go +++ b/internal/fuse/file.go @@ -167,19 +167,10 @@ func (f *openFile) Read(ctx context.Context, req *fuse.ReadRequest, resp *fuse.R } func (f *file) Listxattr(_ context.Context, req *fuse.ListxattrRequest, resp *fuse.ListxattrResponse) error { - debug.Log("Listxattr(%v, %v)", f.node.Name, req.Size) - for _, attr := range f.node.ExtendedAttributes { - resp.Append(attr.Name) - } + nodeToXattrList(f.node, req, resp) return nil } func (f *file) Getxattr(_ context.Context, req *fuse.GetxattrRequest, resp *fuse.GetxattrResponse) error { - debug.Log("Getxattr(%v, %v, %v)", f.node.Name, req.Name, req.Size) - attrval := f.node.GetExtendedAttribute(req.Name) - if attrval != nil { - resp.Xattr = attrval - return nil - } - return fuse.ErrNoXattr + return nodeGetXattr(f.node, req, resp) } diff --git a/internal/fuse/link.go b/internal/fuse/link.go index 43b00a855..3aea8b06e 100644 --- a/internal/fuse/link.go +++ b/internal/fuse/link.go @@ -6,8 +6,6 @@ package fuse import ( "context" - "github.com/restic/restic/internal/debug" - "github.com/anacrolix/fuse" "github.com/anacrolix/fuse/fs" "github.com/restic/restic/internal/restic" @@ -50,19 +48,10 @@ func (l *link) Attr(_ context.Context, a *fuse.Attr) error { } func (l *link) Listxattr(_ context.Context, req *fuse.ListxattrRequest, resp *fuse.ListxattrResponse) error { - debug.Log("Listxattr(%v, %v)", l.node.Name, req.Size) - for _, attr := range l.node.ExtendedAttributes { - resp.Append(attr.Name) - } + nodeToXattrList(l.node, req, resp) return nil } func (l *link) Getxattr(_ context.Context, req *fuse.GetxattrRequest, resp *fuse.GetxattrResponse) error { - debug.Log("Getxattr(%v, %v, %v)", l.node.Name, req.Name, req.Size) - attrval := l.node.GetExtendedAttribute(req.Name) - if attrval != nil { - resp.Xattr = attrval - return nil - } - return fuse.ErrNoXattr + return nodeGetXattr(l.node, req, resp) } diff --git a/internal/fuse/xattr.go b/internal/fuse/xattr.go new file mode 100644 index 000000000..f208938a6 --- /dev/null +++ b/internal/fuse/xattr.go @@ -0,0 +1,24 @@ +package fuse + +import ( + "github.com/anacrolix/fuse" + "github.com/restic/restic/internal/debug" + "github.com/restic/restic/internal/restic" +) + +func nodeToXattrList(node *restic.Node, req *fuse.ListxattrRequest, resp *fuse.ListxattrResponse) { + debug.Log("Listxattr(%v, %v)", node.Name, req.Size) + for _, attr := range node.ExtendedAttributes { + resp.Append(attr.Name) + } +} + +func nodeGetXattr(node *restic.Node, req *fuse.GetxattrRequest, resp *fuse.GetxattrResponse) error { + debug.Log("Getxattr(%v, %v, %v)", node.Name, req.Name, req.Size) + attrval := node.GetExtendedAttribute(req.Name) + if attrval != nil { + resp.Xattr = attrval + return nil + } + return fuse.ErrNoXattr +} From 1f1e50f49efd5344ac0d01e206ec4232651a00bc Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 8 Jul 2023 18:02:17 +0200 Subject: [PATCH 3/6] fuse: add test for symlink xattr --- internal/fuse/fuse_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/internal/fuse/fuse_test.go b/internal/fuse/fuse_test.go index 9ca1ec0c6..ccdd2f774 100644 --- a/internal/fuse/fuse_test.go +++ b/internal/fuse/fuse_test.go @@ -271,6 +271,31 @@ func TestInodeFromNode(t *testing.T) { rtest.Assert(t, inoA != inoAbb, "inode(a/b/b) = inode(a)") } +func TestLink(t *testing.T) { + node := &restic.Node{Name: "foo.txt", Type: "symlink", Links: 1, LinkTarget: "dst", ExtendedAttributes: []restic.ExtendedAttribute{ + {Name: "foo", Value: []byte("bar")}, + }} + + lnk, err := newLink(&Root{}, 42, node) + rtest.OK(t, err) + target, err := lnk.Readlink(context.TODO(), nil) + rtest.OK(t, err) + rtest.Equals(t, node.LinkTarget, target) + + exp := &fuse.ListxattrResponse{} + exp.Append("foo") + resp := &fuse.ListxattrResponse{} + rtest.OK(t, lnk.Listxattr(context.TODO(), &fuse.ListxattrRequest{}, resp)) + rtest.Equals(t, exp.Xattr, resp.Xattr) + + getResp := &fuse.GetxattrResponse{} + rtest.OK(t, lnk.Getxattr(context.TODO(), &fuse.GetxattrRequest{Name: "foo"}, getResp)) + rtest.Equals(t, node.ExtendedAttributes[0].Value, getResp.Xattr) + + err = lnk.Getxattr(context.TODO(), &fuse.GetxattrRequest{Name: "invalid"}, nil) + rtest.Assert(t, err != nil, "missing error on reading invalid xattr") +} + var sink uint64 func BenchmarkInode(b *testing.B) { From 4a5ae2ba8451d4a8c64ace57dc4da8e1b36fbd69 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 8 Jul 2023 18:18:13 +0200 Subject: [PATCH 4/6] restic: test NodeFromFileInfo for symlinks --- internal/restic/node_unix_test.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/internal/restic/node_unix_test.go b/internal/restic/node_unix_test.go index c4fef3710..374326bf7 100644 --- a/internal/restic/node_unix_test.go +++ b/internal/restic/node_unix_test.go @@ -5,10 +5,13 @@ package restic import ( "os" + "path/filepath" "runtime" "syscall" "testing" "time" + + rtest "github.com/restic/restic/internal/test" ) func stat(t testing.TB, filename string) (fi os.FileInfo, ok bool) { @@ -25,6 +28,7 @@ func stat(t testing.TB, filename string) (fi os.FileInfo, ok bool) { } func checkFile(t testing.TB, stat *syscall.Stat_t, node *Node) { + t.Helper() if uint32(node.Mode.Perm()) != uint32(stat.Mode&0777) { t.Errorf("Mode does not match, want %v, got %v", stat.Mode&0777, node.Mode) } @@ -37,7 +41,7 @@ func checkFile(t testing.TB, stat *syscall.Stat_t, node *Node) { t.Errorf("Dev does not match, want %v, got %v", stat.Dev, node.DeviceID) } - if node.Size != uint64(stat.Size) { + if node.Size != uint64(stat.Size) && node.Type != "symlink" { t.Errorf("Size does not match, want %v, got %v", stat.Size, node.Size) } @@ -83,6 +87,10 @@ func checkDevice(t testing.TB, stat *syscall.Stat_t, node *Node) { } func TestNodeFromFileInfo(t *testing.T) { + tmp := t.TempDir() + symlink := filepath.Join(tmp, "symlink") + rtest.OK(t, os.Symlink("target", symlink)) + type Test struct { filename string canSkip bool @@ -90,6 +98,7 @@ func TestNodeFromFileInfo(t *testing.T) { var tests = []Test{ {"node_test.go", false}, {"/dev/sda", true}, + {symlink, false}, } // on darwin, users are not permitted to list the extended attributes of @@ -125,7 +134,7 @@ func TestNodeFromFileInfo(t *testing.T) { } switch node.Type { - case "file": + case "file", "symlink": checkFile(t, s, node) case "dev", "chardev": checkFile(t, s, node) From cc84884d2e21f3847830fa7a507946967feb9101 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 8 Jul 2023 18:49:21 +0200 Subject: [PATCH 5/6] restic: basic xattr test for files/dirs --- internal/restic/node_test.go | 62 +++++++++++++++++++++++++++++------- 1 file changed, 51 insertions(+), 11 deletions(-) diff --git a/internal/restic/node_test.go b/internal/restic/node_test.go index 60342e9a4..c89bc6ac3 100644 --- a/internal/restic/node_test.go +++ b/internal/restic/node_test.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "reflect" "runtime" "testing" "time" @@ -163,22 +164,56 @@ var nodeTests = []restic.Node{ AccessTime: parseTime("2005-05-14 21:07:04.222"), ChangeTime: parseTime("2005-05-14 21:07:05.333"), }, + { + Name: "testXattrFile", + Type: "file", + Content: restic.IDs{}, + UID: uint32(os.Getuid()), + GID: uint32(os.Getgid()), + Mode: 0604, + ModTime: parseTime("2005-05-14 21:07:03.111"), + AccessTime: parseTime("2005-05-14 21:07:04.222"), + ChangeTime: parseTime("2005-05-14 21:07:05.333"), + ExtendedAttributes: []restic.ExtendedAttribute{ + {"user.foo", []byte("bar")}, + }, + }, + { + Name: "testXattrDir", + Type: "dir", + Subtree: nil, + UID: uint32(os.Getuid()), + GID: uint32(os.Getgid()), + Mode: 0750 | os.ModeDir, + ModTime: parseTime("2005-05-14 21:07:03.111"), + AccessTime: parseTime("2005-05-14 21:07:04.222"), + ChangeTime: parseTime("2005-05-14 21:07:05.333"), + ExtendedAttributes: []restic.ExtendedAttribute{ + {"user.foo", []byte("bar")}, + }, + }, } func TestNodeRestoreAt(t *testing.T) { - tempdir, err := os.MkdirTemp(rtest.TestTempDir, "restic-test-") - rtest.OK(t, err) - - defer func() { - if rtest.TestCleanupTempDirs { - rtest.RemoveAll(t, tempdir) - } else { - t.Logf("leaving tempdir at %v", tempdir) - } - }() + tempdir := t.TempDir() for _, test := range nodeTests { - nodePath := filepath.Join(tempdir, test.Name) + var nodePath string + if test.ExtendedAttributes != nil { + if runtime.GOOS == "windows" { + // restic does not support xattrs on windows + return + } + + // tempdir might be backed by a filesystem that does not support + // extended attributes + nodePath = test.Name + defer func() { + _ = os.Remove(nodePath) + }() + } else { + nodePath = filepath.Join(tempdir, test.Name) + } rtest.OK(t, test.CreateAt(context.TODO(), nodePath, nil)) rtest.OK(t, test.RestoreMetadata(nodePath)) @@ -215,6 +250,11 @@ func TestNodeRestoreAt(t *testing.T) { AssertFsTimeEqual(t, "AccessTime", test.Type, test.AccessTime, n2.AccessTime) AssertFsTimeEqual(t, "ModTime", test.Type, test.ModTime, n2.ModTime) + if len(n2.ExtendedAttributes) == 0 { + n2.ExtendedAttributes = nil + } + rtest.Assert(t, reflect.DeepEqual(test.ExtendedAttributes, n2.ExtendedAttributes), + "%v: xattrs don't match (%v != %v)", test.Name, test.ExtendedAttributes, n2.ExtendedAttributes) } } From ea9ad77e05918731625500a4335f285f5156dc79 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 8 Jul 2023 18:49:47 +0200 Subject: [PATCH 6/6] restic: refactor node test --- internal/restic/node_test.go | 100 ++++++++++++++++++----------------- 1 file changed, 51 insertions(+), 49 deletions(-) diff --git a/internal/restic/node_test.go b/internal/restic/node_test.go index c89bc6ac3..45ccd790c 100644 --- a/internal/restic/node_test.go +++ b/internal/restic/node_test.go @@ -198,63 +198,65 @@ func TestNodeRestoreAt(t *testing.T) { tempdir := t.TempDir() for _, test := range nodeTests { - var nodePath string - if test.ExtendedAttributes != nil { - if runtime.GOOS == "windows" { - // restic does not support xattrs on windows - return + t.Run("", func(t *testing.T) { + var nodePath string + if test.ExtendedAttributes != nil { + if runtime.GOOS == "windows" { + // restic does not support xattrs on windows + return + } + + // tempdir might be backed by a filesystem that does not support + // extended attributes + nodePath = test.Name + defer func() { + _ = os.Remove(nodePath) + }() + } else { + nodePath = filepath.Join(tempdir, test.Name) + } + rtest.OK(t, test.CreateAt(context.TODO(), nodePath, nil)) + rtest.OK(t, test.RestoreMetadata(nodePath)) + + if test.Type == "dir" { + rtest.OK(t, test.RestoreTimestamps(nodePath)) } - // tempdir might be backed by a filesystem that does not support - // extended attributes - nodePath = test.Name - defer func() { - _ = os.Remove(nodePath) - }() - } else { - nodePath = filepath.Join(tempdir, test.Name) - } - rtest.OK(t, test.CreateAt(context.TODO(), nodePath, nil)) - rtest.OK(t, test.RestoreMetadata(nodePath)) + fi, err := os.Lstat(nodePath) + rtest.OK(t, err) - if test.Type == "dir" { - rtest.OK(t, test.RestoreTimestamps(nodePath)) - } + n2, err := restic.NodeFromFileInfo(nodePath, fi) + rtest.OK(t, err) - fi, err := os.Lstat(nodePath) - rtest.OK(t, err) + rtest.Assert(t, test.Name == n2.Name, + "%v: name doesn't match (%v != %v)", test.Type, test.Name, n2.Name) + rtest.Assert(t, test.Type == n2.Type, + "%v: type doesn't match (%v != %v)", test.Type, test.Type, n2.Type) + rtest.Assert(t, test.Size == n2.Size, + "%v: size doesn't match (%v != %v)", test.Size, test.Size, n2.Size) - n2, err := restic.NodeFromFileInfo(nodePath, fi) - rtest.OK(t, err) - - rtest.Assert(t, test.Name == n2.Name, - "%v: name doesn't match (%v != %v)", test.Type, test.Name, n2.Name) - rtest.Assert(t, test.Type == n2.Type, - "%v: type doesn't match (%v != %v)", test.Type, test.Type, n2.Type) - rtest.Assert(t, test.Size == n2.Size, - "%v: size doesn't match (%v != %v)", test.Size, test.Size, n2.Size) - - if runtime.GOOS != "windows" { - rtest.Assert(t, test.UID == n2.UID, - "%v: UID doesn't match (%v != %v)", test.Type, test.UID, n2.UID) - rtest.Assert(t, test.GID == n2.GID, - "%v: GID doesn't match (%v != %v)", test.Type, test.GID, n2.GID) - if test.Type != "symlink" { - // On OpenBSD only root can set sticky bit (see sticky(8)). - if runtime.GOOS != "openbsd" && runtime.GOOS != "netbsd" && runtime.GOOS != "solaris" && test.Name == "testSticky" { - rtest.Assert(t, test.Mode == n2.Mode, - "%v: mode doesn't match (0%o != 0%o)", test.Type, test.Mode, n2.Mode) + if runtime.GOOS != "windows" { + rtest.Assert(t, test.UID == n2.UID, + "%v: UID doesn't match (%v != %v)", test.Type, test.UID, n2.UID) + rtest.Assert(t, test.GID == n2.GID, + "%v: GID doesn't match (%v != %v)", test.Type, test.GID, n2.GID) + if test.Type != "symlink" { + // On OpenBSD only root can set sticky bit (see sticky(8)). + if runtime.GOOS != "openbsd" && runtime.GOOS != "netbsd" && runtime.GOOS != "solaris" && test.Name == "testSticky" { + rtest.Assert(t, test.Mode == n2.Mode, + "%v: mode doesn't match (0%o != 0%o)", test.Type, test.Mode, n2.Mode) + } } } - } - AssertFsTimeEqual(t, "AccessTime", test.Type, test.AccessTime, n2.AccessTime) - AssertFsTimeEqual(t, "ModTime", test.Type, test.ModTime, n2.ModTime) - if len(n2.ExtendedAttributes) == 0 { - n2.ExtendedAttributes = nil - } - rtest.Assert(t, reflect.DeepEqual(test.ExtendedAttributes, n2.ExtendedAttributes), - "%v: xattrs don't match (%v != %v)", test.Name, test.ExtendedAttributes, n2.ExtendedAttributes) + AssertFsTimeEqual(t, "AccessTime", test.Type, test.AccessTime, n2.AccessTime) + AssertFsTimeEqual(t, "ModTime", test.Type, test.ModTime, n2.ModTime) + if len(n2.ExtendedAttributes) == 0 { + n2.ExtendedAttributes = nil + } + rtest.Assert(t, reflect.DeepEqual(test.ExtendedAttributes, n2.ExtendedAttributes), + "%v: xattrs don't match (%v != %v)", test.Name, test.ExtendedAttributes, n2.ExtendedAttributes) + }) } }