fix xattrs on dangling symlinks and pin Linux xattr destinations

archive.go

@@ -674,35 +674,9 @@ func createTarFile(root *os.Root, dstPath string, hdr *tar.Header, reader io.Rea
 		}
 	}
 
-	var (
-		xattrErrs         []string
-		xattrPath         string
-		resolvedXattrPath bool
-	)
-	for key, value := range hdr.PAXRecords {
-		xattr, ok := strings.CutPrefix(key, paxSchilyXattr)
-		if !ok {
-			continue
-		}
-		if !resolvedXattrPath {
-			var err error
-			xattrPath, err = fsRootPath(root.Name(), dstPath)
-			if err != nil {
-				return err
-			}
-			resolvedXattrPath = true
-		}
-		// os.Root has no xattr support; use the absolute path derived from
-		// the root so the path remains bounded.
-		if err := lsetxattr(xattrPath, xattr, []byte(value), 0); err != nil {
-			if bestEffortXattrs && errors.Is(err, syscall.ENOTSUP) || errors.Is(err, syscall.EPERM) {
-				// EPERM occurs if modifying xattrs is not allowed. This can
-				// happen when running in userns with restrictions (ChromeOS).
-				xattrErrs = append(xattrErrs, err.Error())
-				continue
-			}
-			return err
-		}
+	xattrErrs, err := applyXattrs(root, dstPath, hdr.PAXRecords, bestEffortXattrs, internalOpts)
+	if err != nil {
+		return err
 	}
 
 	if len(xattrErrs) > 0 {
@@ -743,6 +717,39 @@ func createTarFile(root *os.Root, dstPath string, hdr *tar.Header, reader io.Rea
 	return nil
 }
 
+// applyXattrs resolves the xattr destination once per entry, and only when
+// there are xattrs to restore.
+func applyXattrs(root *os.Root, name string, records map[string]string, bestEffort bool, opts *archiveoptions.Options) ([]string, error) {
+	for key := range records {
+		if !strings.HasPrefix(key, paxSchilyXattr) {
+			continue
+		}
+		var ignored []string
+		err := withXattrPath(root, name, opts, func(path string) error {
+			for key, value := range records {
+				attr, ok := strings.CutPrefix(key, paxSchilyXattr)
+				if !ok {
+					continue
+				}
+				if err := lsetxattr(path, attr, []byte(value), 0); err != nil {
+					// Keep the archive entry visible even when the syscall uses
+					// a procfd path or an isolated thread's relative basename.
+					err = fmt.Errorf("entry %q: %w", name, err)
+					if bestEffort && errors.Is(err, syscall.ENOTSUP) || errors.Is(err, syscall.EPERM) {
+						// EPERM also occurs in restricted user namespaces.
+						ignored = append(ignored, err.Error())
+						continue
+					}
+					return err
+				}
+			}
+			return nil
+		})
+		return ignored, err
+	}
+	return nil, nil
+}
+
 // Tar creates an archive from the directory at `srcPath`, and returns it as a
 // stream of bytes.
 func Tar(srcPath string, comp compression.Compression) (io.ReadCloser, error) {
@@ -991,6 +998,9 @@ type unpackedDir struct {
 }
 
 // Unpack unpacks the decompressedArchive to dest with options.
+//
+// On Linux, restoring xattrs without /proc/self/fd requires permission to
+// call unshare(CLONE_FS). Otherwise extraction returns an error.
 func Unpack(decompressedArchive io.Reader, dest string, options *TarOptions) error {
 	if options == nil {
 		options = &TarOptions{}

archive_linux_chrooted_test.go

@@ -78,3 +78,68 @@ func TestChmodNoSymlinkFallbackInChrootWithoutProc(t *testing.T) {
 	assert.NilError(t, err)
 	assert.NilError(t, testErr)
 }
+
+func TestApplyXattrsInChrootWithoutProc(t *testing.T) {
+	skip.If(t, os.Getuid() != 0, "test requires root")
+	skip.If(t, userns.RunningInUserNS(), "test requires the initial user namespace")
+
+	opts, cleanup, err := WithProcSelfFD(nil)
+	assert.NilError(t, err)
+	defer cleanup()
+
+	for _, tc := range []struct {
+		name string
+		opts *TarOptions
+	}{
+		{name: "direct", opts: &TarOptions{}},
+		{name: "prepared", opts: opts},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			dest := t.TempDir()
+			assert.NilError(t, os.WriteFile(filepath.Join(dest, "file"), nil, 0o200))
+			assert.NilError(t, os.Symlink("missing", filepath.Join(dest, "link")))
+
+			done := make(chan error, 1)
+			err := unshare.Go(unix.CLONE_FS|unix.CLONE_NEWNS, func() error {
+				if err := mount.MakeRSlave("/"); err != nil {
+					return err
+				}
+				return mounttree.SwitchRoot(dest)
+			}, func() {
+				root, err := os.OpenRoot("/")
+				if err != nil {
+					done <- err
+					return
+				}
+				defer root.Close()
+				if _, err := os.Stat("/proc/self/fd"); err == nil {
+					done <- errors.New("procfs unexpectedly available")
+					return
+				} else if !errors.Is(err, os.ErrNotExist) {
+					done <- err
+					return
+				}
+				if _, err := applyXattrs(root, "file", map[string]string{paxSchilyXattr + "user.archive": "value"}, false, tc.opts.internalOptions); err != nil {
+					done <- err
+					return
+				}
+				// An ignored EPERM proves the syscall reached the symlink itself.
+				ignored, err := applyXattrs(root, "link", map[string]string{paxSchilyXattr + "user.archive": "value"}, false, tc.opts.internalOptions)
+				if err != nil {
+					done <- err
+					return
+				}
+				if len(ignored) != 1 {
+					done <- fmt.Errorf("symlink xattr: got %d ignored errors, want 1", len(ignored))
+					return
+				}
+				done <- nil
+			})
+			assert.NilError(t, err)
+			assert.NilError(t, <-done)
+			value, err := lgetxattr(filepath.Join(dest, "file"), "user.archive")
+			assert.NilError(t, err)
+			assert.Equal(t, string(value), "value")
+		})
+	}
+}

archive_linux_test.go

@@ -17,6 +17,154 @@ import (
 	"gotest.tools/v3/skip"
 )
 
+func TestApplyXattrsOnDanglingSymlink(t *testing.T) {
+	for _, target := range []string{"usr/bin", "/missing", "../../outside"} {
+		t.Run(target, func(t *testing.T) {
+			dest := t.TempDir()
+			assert.NilError(t, os.Mkdir(filepath.Join(dest, "dir"), 0o755))
+			assert.NilError(t, os.Symlink(target, filepath.Join(dest, "dir", "link")))
+			root, err := os.OpenRoot(dest)
+			assert.NilError(t, err)
+			defer root.Close()
+
+			// user xattrs on Linux symlinks normally return EPERM. Compare
+			// against the direct syscall so this test needs no SELinux support.
+			directErr := unix.Lsetxattr(filepath.Join(dest, "dir", "link"), "user.archive", []byte("value"), 0)
+			ignored, err := applyXattrs(root, "dir/link", map[string]string{paxSchilyXattr + "user.archive": "value"}, false, nil)
+			if errors.Is(directErr, unix.EPERM) {
+				assert.NilError(t, err)
+				assert.Equal(t, len(ignored), 1)
+				assert.Assert(t, is.Contains(ignored[0], `entry "dir/link"`))
+			} else if directErr == nil {
+				assert.NilError(t, err)
+			} else {
+				assert.ErrorIs(t, err, directErr)
+			}
+			assert.Assert(t, !errors.Is(err, os.ErrNotExist), "must address the symlink, not its missing target")
+		})
+	}
+}
+
+func TestXattrPathPinsParent(t *testing.T) {
+	dest := t.TempDir()
+	assert.NilError(t, os.Mkdir(filepath.Join(dest, "dir"), 0o755))
+	assert.NilError(t, os.WriteFile(filepath.Join(dest, "dir", "file"), nil, 0o600))
+	outside := t.TempDir()
+	assert.NilError(t, os.WriteFile(filepath.Join(outside, "file"), nil, 0o600))
+	root, err := os.OpenRoot(dest)
+	assert.NilError(t, err)
+	defer root.Close()
+
+	err = withXattrPath(root, "dir/file", nil, func(path string) error {
+		// Replace the parent after it has been pinned, before the syscall.
+		if err := os.Rename(filepath.Join(dest, "dir"), filepath.Join(dest, "moved")); err != nil {
+			return err
+		}
+		if err := os.Symlink(outside, filepath.Join(dest, "dir")); err != nil {
+			return err
+		}
+		return lsetxattr(path, "user.archive", []byte("value"), 0)
+	})
+	assert.NilError(t, err)
+	value, err := lgetxattr(filepath.Join(dest, "moved", "file"), "user.archive")
+	assert.NilError(t, err)
+	assert.Equal(t, string(value), "value")
+	value, err = lgetxattr(filepath.Join(outside, "file"), "user.archive")
+	assert.NilError(t, err)
+	assert.Assert(t, len(value) == 0)
+}
+
+func TestApplyXattrsKeepsErrors(t *testing.T) {
+	dest := t.TempDir()
+	assert.NilError(t, os.WriteFile(filepath.Join(dest, "file"), nil, 0o600))
+	root, err := os.OpenRoot(dest)
+	assert.NilError(t, err)
+	defer root.Close()
+	for _, bestEffort := range []bool{false, true} {
+		_, err := applyXattrs(root, "file", map[string]string{paxSchilyXattr + "user.bad\x00name": "value"}, bestEffort, nil)
+		assert.ErrorIs(t, err, unix.EINVAL)
+		assert.Assert(t, is.Contains(err.Error(), `entry "file"`))
+		_, err = applyXattrs(root, "missing/file", map[string]string{paxSchilyXattr + "user.archive": "value"}, bestEffort, nil)
+		assert.ErrorIs(t, err, os.ErrNotExist)
+	}
+	// Headers without xattrs must not resolve an xattr destination.
+	_, err = applyXattrs(root, "missing/file", map[string]string{"unrelated": "value"}, false, nil)
+	assert.NilError(t, err)
+}
+
+func TestApplyXattrsAfterRootRename(t *testing.T) {
+	dest := filepath.Join(t.TempDir(), "root")
+	assert.NilError(t, os.Mkdir(dest, 0o755))
+	assert.NilError(t, os.WriteFile(filepath.Join(dest, "file"), nil, 0o200))
+	root, err := os.OpenRoot(dest)
+	assert.NilError(t, err)
+	defer root.Close()
+
+	moved := dest + "-moved"
+	assert.NilError(t, os.Rename(dest, moved))
+	outside := t.TempDir()
+	assert.NilError(t, os.WriteFile(filepath.Join(outside, "file"), nil, 0o600))
+	assert.NilError(t, os.Symlink(outside, dest))
+	_, err = applyXattrs(root, "file", map[string]string{paxSchilyXattr + "user.archive": "value"}, false, nil)
+	assert.NilError(t, err)
+	// Reading the xattr needs read permission, even though setting it does not.
+	assert.NilError(t, os.Chmod(filepath.Join(moved, "file"), 0o600))
+	value, err := lgetxattr(filepath.Join(moved, "file"), "user.archive")
+	assert.NilError(t, err)
+	assert.Equal(t, string(value), "value")
+	value, err = lgetxattr(filepath.Join(outside, "file"), "user.archive")
+	assert.NilError(t, err)
+	assert.Assert(t, len(value) == 0)
+}
+
+func TestUntarXattrsThroughSymlinkedParent(t *testing.T) {
+	var buf bytes.Buffer
+	tw := tar.NewWriter(&buf)
+	for _, hdr := range []*tar.Header{
+		{Name: "bin", Typeflag: tar.TypeSymlink, Linkname: "usr/bin", Mode: 0o777},
+		{Name: "usr", Typeflag: tar.TypeDir, Mode: 0o755},
+		{Name: "usr/bin", Typeflag: tar.TypeDir, Mode: 0o755},
+		{Name: "bin/tool", Typeflag: tar.TypeReg, Mode: 0o600, PAXRecords: map[string]string{paxSchilyXattr + "user.archive": "value"}},
+	} {
+		assert.NilError(t, tw.WriteHeader(hdr))
+	}
+	assert.NilError(t, tw.Close())
+	dest := t.TempDir()
+	assert.NilError(t, Untar(&buf, dest, &TarOptions{NoLchown: true}))
+	value, err := lgetxattr(filepath.Join(dest, "usr", "bin", "tool"), "user.archive")
+	assert.NilError(t, err)
+	assert.Equal(t, string(value), "value")
+}
+
+// Regression coverage for https://github.com/moby/go-archive/issues/109.
+func TestUntarSELinuxXattrOnDanglingSymlink(t *testing.T) {
+	const (
+		attr  = "security.selinux"
+		value = "system_u:object_r:container_file_t:s0"
+	)
+	probe := filepath.Join(t.TempDir(), "probe")
+	assert.NilError(t, os.Symlink("usr/bin", probe))
+	if err := lsetxattr(probe, attr, []byte(value), 0); err != nil {
+		t.Skipf("cannot set %q on symlinks: %v", attr, err)
+	}
+
+	var buf bytes.Buffer
+	tw := tar.NewWriter(&buf)
+	for _, hdr := range []*tar.Header{
+		{Name: "bin", Typeflag: tar.TypeSymlink, Linkname: "usr/bin", Mode: 0o777, PAXRecords: map[string]string{paxSchilyXattr + attr: value}},
+		{Name: "usr/", Typeflag: tar.TypeDir, Mode: 0o755},
+		{Name: "usr/bin/", Typeflag: tar.TypeDir, Mode: 0o755},
+	} {
+		assert.NilError(t, tw.WriteHeader(hdr))
+	}
+	assert.NilError(t, tw.Close())
+	dest := t.TempDir()
+	assert.NilError(t, Untar(&buf, dest, &TarOptions{NoLchown: true}))
+	got, err := lgetxattr(filepath.Join(dest, "bin"), attr)
+	assert.NilError(t, err)
+	assert.Equal(t, string(got), value)
+}
+
 // setupOverlayTestDir creates files in a directory with overlay whiteouts
 // Tree layout
 //

xattr_supported_linux.go

@@ -1,5 +1,50 @@
 package archive
 
-import "golang.org/x/sys/unix"
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"strconv"
+
+	"github.com/moby/go-archive/internal/archiveoptions"
+	"github.com/moby/go-archive/internal/unshare"
+	"golang.org/x/sys/unix"
+)
 
 var noattr = unix.ENODATA
+
+// withXattrPath pins the parent through root and preserves the final
+// component for lsetxattr's no-follow semantics, including dangling symlinks.
+func withXattrPath(root *os.Root, name string, opts *archiveoptions.Options, apply func(string) error) error {
+	parent, err := root.OpenFile(filepath.Dir(name), unix.O_PATH|unix.O_DIRECTORY, 0)
+	if err != nil {
+		return err
+	}
+	defer parent.Close()
+
+	base := filepath.Base(name)
+	if opts == nil || opts.ProcSelfFD == nil {
+		procPath := "/proc/self/fd/" + strconv.FormatUint(uint64(parent.Fd()), 10) + "/" + base
+		err := apply(procPath)
+		if err == nil || !errors.Is(err, os.ErrNotExist) {
+			return err
+		}
+		// A missing leaf must keep its error. Fall back only when procfs
+		// itself is absent in this thread's filesystem context.
+		if _, procErr := os.Stat("/proc/self/fd"); !errors.Is(procErr, os.ErrNotExist) {
+			return err
+		}
+	}
+
+	// Extraction can run without procfs, including callers that did not
+	// prepare options through chrootarchive. Change cwd only on an isolated
+	// thread so lsetxattr still addresses the pinned parent directly.
+	done := make(chan error, 1)
+	err = unshare.Go(unix.CLONE_FS, func() error { return parent.Chdir() }, func() {
+		done <- apply(base)
+	})
+	if err != nil {
+		return err
+	}
+	return <-done
+}

xattr_supported_unix.go

@@ -2,6 +2,20 @@
 
 package archive
 
-import "golang.org/x/sys/unix"
+import (
+	"os"
+	"path/filepath"
+
+	"github.com/moby/go-archive/internal/archiveoptions"
+	"golang.org/x/sys/unix"
+)
 
 var noattr = unix.ENOATTR
+
+func withXattrPath(root *os.Root, name string, _ *archiveoptions.Options, apply func(string) error) error {
+	parent, err := fsRootPath(root.Name(), filepath.Dir(name))
+	if err != nil {
+		return err
+	}
+	return apply(filepath.Join(parent, filepath.Base(name)))
+}