Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Linux] xattrs, like security.selinux, are not backed up for symlinks #4375

Closed
chenxiaolong opened this issue Jun 18, 2023 · 3 comments · Fixed by #4379
Closed

[Linux] xattrs, like security.selinux, are not backed up for symlinks #4375

chenxiaolong opened this issue Jun 18, 2023 · 3 comments · Fixed by #4379

Comments

@chenxiaolong
Copy link
Contributor

Currently, it seems like restic does not store symlink xattrs, at least on Linux. While Linux does not support user.* xattrs on symlinks, other xattrs, like security.* are supported. The biggest use I'm aware of is security.selinux, which stores the file's SELinux label.

Currently, if restic is used to back up the root filesystem of a Linux distro that uses SELinux (eg. Fedora), the restored backup is not bootable because all of the symlinks (eg. /lib and /lib64) are missing SELinux labels.

(The reproduction steps I list below demonstrate the problem using a custom xattr so that it can be run on any Linux distro.)


Output of restic version

restic 0.15.2 (v0.15.2-346-g74ca82a6f) compiled with go1.20.4 on linux/amd64

How did you run restic exactly?

restic -r <repo> backup <directory with symlink with xattrs>

What backend/server/service did you use to store the repository?

Local path, but occurs with any backend.

Expected behavior

xattrs on symlinks would be preserved in the backup.

Actual behavior

xattrs on symlinks don't seem to be stored.

Steps to reproduce the behavior

  1. Create a directory tree containing a symlink with xattrs.

    mkdir data
    touch data/file
    ln -s file data/symlink
    
    sudo setfattr -h -n security.comment -v 'this is a file' data/file
    sudo setfattr -h -n security.comment -v 'this is a symlink' data/symlink
  2. Back up the directory tree with restic.

    restic -r repo init
    restic -r repo backup data
  3. Check the extended attributes via a mount.

    mkdir mnt
    restic -r repo mount mnt
    $ getfattr -h -n security.comment mnt/snapshots/latest/data/{file,symlink}
    # file: mnt/snapshots/latest/data/file
    security.comment="this is a file"
    
    mnt/snapshots/latest/data/symlink: security.comment: Operation not supported
    
  4. Restore to a directory and check the extended attributes.

    mkdir data_restored
    sudo restic -r repo restore latest -t data_restored
    $ getfattr -h -n security.comment data_restored/data/{file,symlink}
    # file: data_restored/data/file
    security.comment="this is a file"
    
    data_restored/data/symlink: security.comment: No such attribute
    

Do you have any idea what may have caused this?

It seems like symlink xattrs aren't stored in the restic repo at all.

Do you have an idea how to solve the issue?

Assuming the repository data format supports it, it'd be nice if restic could save and restore the symlink xattrs via lgetxattr()/lsetxattr().

Did restic help you today? Did it make you happy in any way?

Always does! I still find it to be the best tool for the job.

@chenxiaolong
Copy link
Contributor Author

I'm not sure if additional work is required, but I made the following change and it seems to be working perfectly for both fuse mounts and restores:

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 {

@MichaelEischer
Copy link
Member

Looks reasonable at first glance, could you open a PR? We'd have to test whether this causes issues on other operating systems, our automated tests cover Windows, Linux and macOS, which would cover the most important OS.

@chenxiaolong
Copy link
Contributor Author

Sure thing! I've opened a PR at #4379.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants