From 5fc095c06d5576ceb0425eb67dd7a013b9f4b79b Mon Sep 17 00:00:00 2001
From: Zac Medico <zmedico@gmail.com>
Date: Wed, 20 Nov 2019 20:20:25 -0800
Subject: [PATCH 1/3] VFS support for user.* xattrs

Preservation of user.* xattrs is supported by most (if not all) other
drivers, and it's especially useful for containers that run under a
PaX kernel where "user.pax.flags" is used to store PaX flags.

Signed-off-by: Zac Medico <zmedico@gmail.com>
---
 drivers/copy/copy_linux.go       | 18 +++++++++++++++-
 drivers/vfs/copy_linux.go        |  2 +-
 pkg/archive/archive.go           | 20 ++++++++++++++++++
 pkg/archive/changes.go           |  5 ++++-
 pkg/archive/changes_linux.go     | 17 ++++++++++++++++
 pkg/system/xattrs_linux.go       | 35 +++++++++++++++++++++++++++++++-
 pkg/system/xattrs_unsupported.go |  5 +++++
 7 files changed, 98 insertions(+), 4 deletions(-)

diff --git a/drivers/copy/copy_linux.go b/drivers/copy/copy_linux.go
index e52545d5b..79784f43b 100644
--- a/drivers/copy/copy_linux.go
+++ b/drivers/copy/copy_linux.go
@@ -16,6 +16,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"strings"
 	"syscall"
 	"time"
 
@@ -97,7 +98,7 @@ func legacyCopy(srcFile io.Reader, dstFile io.Writer) error {
 
 func copyXattr(srcPath, dstPath, attr string) error {
 	data, err := system.Lgetxattr(srcPath, attr)
-	if err != nil {
+	if err != nil && err != unix.EOPNOTSUPP {
 		return err
 	}
 	if data != nil {
@@ -271,6 +272,21 @@ func doCopyXattrs(srcPath, dstPath string) error {
 		return err
 	}
 
+	xattrs, err := system.Llistxattr(srcPath)
+	if err != nil && err != unix.EOPNOTSUPP {
+		return err
+	}
+
+	if xattrs != nil {
+		for _, key := range xattrs {
+			if strings.HasPrefix(key, "user.") {
+				if err := copyXattr(srcPath, dstPath, key); err != nil {
+					return err
+				}
+			}
+		}
+	}
+
 	// We need to copy this attribute if it appears in an overlay upper layer, as
 	// this function is used to copy those. It is set by overlay if a directory
 	// is removed and then re-created and should not inherit anything from the
diff --git a/drivers/vfs/copy_linux.go b/drivers/vfs/copy_linux.go
index 8137fcf67..bf22a5f6f 100644
--- a/drivers/vfs/copy_linux.go
+++ b/drivers/vfs/copy_linux.go
@@ -3,5 +3,5 @@ package vfs
 import "github.com/containers/storage/drivers/copy"
 
 func dirCopy(srcDir, dstDir string) error {
-	return copy.DirCopy(srcDir, dstDir, copy.Content, false)
+	return copy.DirCopy(srcDir, dstDir, copy.Content, true)
 }
diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go
index ba635ae91..61b49c8fe 100644
--- a/pkg/archive/archive.go
+++ b/pkg/archive/archive.go
@@ -395,6 +395,23 @@ func ReadSecurityXattrToTarHeader(path string, hdr *tar.Header) error {
 	return nil
 }
 
+// ReadUserXattrToTarHeader reads user.* xattr from filesystem to a tar header
+func ReadUserXattrToTarHeader(path string, hdr *tar.Header) error {
+	if xattrs, _ := system.Llistxattr(path); xattrs != nil {
+		for _, key := range xattrs {
+			if strings.HasPrefix(key, "user.") {
+				if value, _ := system.Lgetxattr(path, key); value != nil {
+					if hdr.Xattrs == nil {
+						hdr.Xattrs = make(map[string]string)
+					}
+					hdr.Xattrs[key] = string(value)
+				}
+			}
+		}
+	}
+	return nil
+}
+
 type tarWhiteoutConverter interface {
 	ConvertWrite(*tar.Header, string, os.FileInfo) (*tar.Header, error)
 	ConvertRead(*tar.Header, string) (bool, error)
@@ -469,6 +486,9 @@ func (ta *tarAppender) addTarFile(path, name string) error {
 	if err := ReadSecurityXattrToTarHeader(path, hdr); err != nil {
 		return err
 	}
+	if err := ReadUserXattrToTarHeader(path, hdr); err != nil {
+		return err
+	}
 	if ta.CopyPass {
 		copyPassHeader(hdr)
 	}
diff --git a/pkg/archive/changes.go b/pkg/archive/changes.go
index d3d6c8f74..3ce396070 100644
--- a/pkg/archive/changes.go
+++ b/pkg/archive/changes.go
@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"reflect"
 	"sort"
 	"strings"
 	"syscall"
@@ -263,6 +264,7 @@ type FileInfo struct {
 	children   map[string]*FileInfo
 	capability []byte
 	added      bool
+	xattrs     map[string]string
 }
 
 // LookUp looks up the file information of a file.
@@ -331,7 +333,8 @@ func (info *FileInfo) addChanges(oldInfo *FileInfo, changes *[]Change) {
 			// breaks down is if some code intentionally hides a change by setting
 			// back mtime
 			if statDifferent(oldStat, oldInfo, newStat, info) ||
-				!bytes.Equal(oldChild.capability, newChild.capability) {
+				!bytes.Equal(oldChild.capability, newChild.capability) ||
+				!reflect.DeepEqual(oldChild.xattrs, newChild.xattrs) {
 				change := Change{
 					Path: newChild.path(),
 					Kind: ChangeModify,
diff --git a/pkg/archive/changes_linux.go b/pkg/archive/changes_linux.go
index dc313d1ab..0e308b847 100644
--- a/pkg/archive/changes_linux.go
+++ b/pkg/archive/changes_linux.go
@@ -6,6 +6,7 @@ import (
 	"os"
 	"path/filepath"
 	"sort"
+	"strings"
 	"syscall"
 	"unsafe"
 
@@ -84,6 +85,22 @@ func walkchunk(path string, fi os.FileInfo, dir string, root *FileInfo) error {
 	}
 	info.stat = stat
 	info.capability, _ = system.Lgetxattr(cpath, "security.capability") // lgetxattr(2): fs access
+	xattrs, err := system.Llistxattr(cpath)
+	if err != nil && err != unix.EOPNOTSUPP {
+		return err
+	}
+	if xattrs != nil {
+		for _, key := range xattrs {
+			if strings.HasPrefix(key, "user.") {
+				if value, _ := system.Lgetxattr(cpath, key); value != nil {
+					if info.xattrs == nil {
+						info.xattrs = make(map[string]string)
+					}
+					info.xattrs[key] = string(value)
+				}
+			}
+		}
+	}
 	parent.children[info.name] = info
 	return nil
 }
diff --git a/pkg/system/xattrs_linux.go b/pkg/system/xattrs_linux.go
index 98b111be4..d33dd0d95 100644
--- a/pkg/system/xattrs_linux.go
+++ b/pkg/system/xattrs_linux.go
@@ -1,6 +1,10 @@
 package system
 
-import "golang.org/x/sys/unix"
+import (
+	"bytes"
+
+	"golang.org/x/sys/unix"
+)
 
 // Lgetxattr retrieves the value of the extended attribute identified by attr
 // and associated with the given path in the file system.
@@ -27,3 +31,32 @@ func Lgetxattr(path string, attr string) ([]byte, error) {
 func Lsetxattr(path string, attr string, data []byte, flags int) error {
 	return unix.Lsetxattr(path, attr, data, flags)
 }
+
+// Llistxattr lists extended attributes associated with the given path
+// in the file system.
+func Llistxattr(path string) ([]string, error) {
+	var dest []byte
+
+	for {
+		sz, err := unix.Llistxattr(path, dest)
+		if err != nil {
+			return nil, err
+		}
+
+		if sz > len(dest) {
+			dest = make([]byte, sz)
+		} else {
+			dest = dest[:sz]
+			break
+		}
+	}
+
+	var attrs []string
+	for _, token := range bytes.Split(dest, []byte{0}) {
+		if len(token) > 0 {
+			attrs = append(attrs, string(token))
+		}
+	}
+
+	return attrs, nil
+}
diff --git a/pkg/system/xattrs_unsupported.go b/pkg/system/xattrs_unsupported.go
index 0114f2227..34efbaaa8 100644
--- a/pkg/system/xattrs_unsupported.go
+++ b/pkg/system/xattrs_unsupported.go
@@ -11,3 +11,8 @@ func Lgetxattr(path string, attr string) ([]byte, error) {
 func Lsetxattr(path string, attr string, data []byte, flags int) error {
 	return ErrNotSupportedPlatform
 }
+
+// Llistxattr is not supported on platforms other than linux.
+func Llistxattr(path string) ([]string, error) {
+	return nil, ErrNotSupportedPlatform
+}

From 0da4bc60b3babedb098dd7a75ae4ec0b75031eab Mon Sep 17 00:00:00 2001
From: Zac Medico <zmedico@gmail.com>
Date: Fri, 22 Nov 2019 10:42:11 -0800
Subject: [PATCH 2/3] pkg/archive: strictly handle errors reading xattrs

Suggested-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Zac Medico <zmedico@gmail.com>
---
 pkg/archive/archive.go           | 23 ++++++++++++++++-------
 pkg/archive/changes_linux.go     | 19 ++++++++++++-------
 pkg/system/xattrs_linux.go       |  6 ++++++
 pkg/system/xattrs_unsupported.go |  7 +++++++
 4 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go
index 61b49c8fe..46c9082b9 100644
--- a/pkg/archive/archive.go
+++ b/pkg/archive/archive.go
@@ -387,7 +387,10 @@ func fillGo18FileTypeBits(mode int64, fi os.FileInfo) int64 {
 // ReadSecurityXattrToTarHeader reads security.capability xattr from filesystem
 // to a tar header
 func ReadSecurityXattrToTarHeader(path string, hdr *tar.Header) error {
-	capability, _ := system.Lgetxattr(path, "security.capability")
+	capability, err := system.Lgetxattr(path, "security.capability")
+	if err != nil && err != system.EOPNOTSUPP {
+		return err
+	}
 	if capability != nil {
 		hdr.Xattrs = make(map[string]string)
 		hdr.Xattrs["security.capability"] = string(capability)
@@ -397,15 +400,21 @@ func ReadSecurityXattrToTarHeader(path string, hdr *tar.Header) error {
 
 // ReadUserXattrToTarHeader reads user.* xattr from filesystem to a tar header
 func ReadUserXattrToTarHeader(path string, hdr *tar.Header) error {
-	if xattrs, _ := system.Llistxattr(path); xattrs != nil {
+	xattrs, err := system.Llistxattr(path)
+	if err != nil && err != system.EOPNOTSUPP {
+		return err
+	}
+	if xattrs != nil {
 		for _, key := range xattrs {
 			if strings.HasPrefix(key, "user.") {
-				if value, _ := system.Lgetxattr(path, key); value != nil {
-					if hdr.Xattrs == nil {
-						hdr.Xattrs = make(map[string]string)
-					}
-					hdr.Xattrs[key] = string(value)
+				value, err := system.Lgetxattr(path, key)
+				if err != nil {
+					return err
+				}
+				if hdr.Xattrs == nil {
+					hdr.Xattrs = make(map[string]string)
 				}
+				hdr.Xattrs[key] = string(value)
 			}
 		}
 	}
diff --git a/pkg/archive/changes_linux.go b/pkg/archive/changes_linux.go
index 0e308b847..15d582650 100644
--- a/pkg/archive/changes_linux.go
+++ b/pkg/archive/changes_linux.go
@@ -84,20 +84,25 @@ func walkchunk(path string, fi os.FileInfo, dir string, root *FileInfo) error {
 		return err
 	}
 	info.stat = stat
-	info.capability, _ = system.Lgetxattr(cpath, "security.capability") // lgetxattr(2): fs access
+	info.capability, err = system.Lgetxattr(cpath, "security.capability") // lgetxattr(2): fs access
+	if err != nil && err != system.EOPNOTSUPP {
+		return err
+	}
 	xattrs, err := system.Llistxattr(cpath)
-	if err != nil && err != unix.EOPNOTSUPP {
+	if err != nil && err != system.EOPNOTSUPP {
 		return err
 	}
 	if xattrs != nil {
 		for _, key := range xattrs {
 			if strings.HasPrefix(key, "user.") {
-				if value, _ := system.Lgetxattr(cpath, key); value != nil {
-					if info.xattrs == nil {
-						info.xattrs = make(map[string]string)
-					}
-					info.xattrs[key] = string(value)
+				value, err := system.Lgetxattr(cpath, key)
+				if err != nil {
+					return err
+				}
+				if info.xattrs == nil {
+					info.xattrs = make(map[string]string)
 				}
+				info.xattrs[key] = string(value)
 			}
 		}
 	}
diff --git a/pkg/system/xattrs_linux.go b/pkg/system/xattrs_linux.go
index d33dd0d95..0d6cd95e3 100644
--- a/pkg/system/xattrs_linux.go
+++ b/pkg/system/xattrs_linux.go
@@ -2,10 +2,16 @@ package system
 
 import (
 	"bytes"
+	"syscall"
 
 	"golang.org/x/sys/unix"
 )
 
+const (
+	// Operation not supported
+	EOPNOTSUPP syscall.Errno = unix.EOPNOTSUPP
+)
+
 // Lgetxattr retrieves the value of the extended attribute identified by attr
 // and associated with the given path in the file system.
 // It will returns a nil slice and nil error if the xattr is not set.
diff --git a/pkg/system/xattrs_unsupported.go b/pkg/system/xattrs_unsupported.go
index 34efbaaa8..b4cf4e6ca 100644
--- a/pkg/system/xattrs_unsupported.go
+++ b/pkg/system/xattrs_unsupported.go
@@ -2,6 +2,13 @@
 
 package system
 
+import "syscall"
+
+const (
+	// Operation not supported
+	EOPNOTSUPP syscall.Errno = syscall.Errno(0)
+)
+
 // Lgetxattr is not supported on platforms other than linux.
 func Lgetxattr(path string, attr string) ([]byte, error) {
 	return nil, ErrNotSupportedPlatform

From f39fb8370d82c1cfd3a7b3ce9e332a3c4b65a244 Mon Sep 17 00:00:00 2001
From: Zac Medico <zmedico@gmail.com>
Date: Fri, 22 Nov 2019 13:01:46 -0800
Subject: [PATCH 3/3] Remove redundant if statements for nil slices

Suggested-by: Daniel J Walsh <dwalsh@redhat.com>
Signed-off-by: Zac Medico <zmedico@gmail.com>
---
 drivers/copy/copy_linux.go   | 10 ++++------
 pkg/archive/archive.go       | 20 +++++++++-----------
 pkg/archive/changes_linux.go | 20 +++++++++-----------
 3 files changed, 22 insertions(+), 28 deletions(-)

diff --git a/drivers/copy/copy_linux.go b/drivers/copy/copy_linux.go
index 79784f43b..c00b9e47d 100644
--- a/drivers/copy/copy_linux.go
+++ b/drivers/copy/copy_linux.go
@@ -277,12 +277,10 @@ func doCopyXattrs(srcPath, dstPath string) error {
 		return err
 	}
 
-	if xattrs != nil {
-		for _, key := range xattrs {
-			if strings.HasPrefix(key, "user.") {
-				if err := copyXattr(srcPath, dstPath, key); err != nil {
-					return err
-				}
+	for _, key := range xattrs {
+		if strings.HasPrefix(key, "user.") {
+			if err := copyXattr(srcPath, dstPath, key); err != nil {
+				return err
 			}
 		}
 	}
diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go
index 46c9082b9..d2752ae7c 100644
--- a/pkg/archive/archive.go
+++ b/pkg/archive/archive.go
@@ -404,18 +404,16 @@ func ReadUserXattrToTarHeader(path string, hdr *tar.Header) error {
 	if err != nil && err != system.EOPNOTSUPP {
 		return err
 	}
-	if xattrs != nil {
-		for _, key := range xattrs {
-			if strings.HasPrefix(key, "user.") {
-				value, err := system.Lgetxattr(path, key)
-				if err != nil {
-					return err
-				}
-				if hdr.Xattrs == nil {
-					hdr.Xattrs = make(map[string]string)
-				}
-				hdr.Xattrs[key] = string(value)
+	for _, key := range xattrs {
+		if strings.HasPrefix(key, "user.") {
+			value, err := system.Lgetxattr(path, key)
+			if err != nil {
+				return err
+			}
+			if hdr.Xattrs == nil {
+				hdr.Xattrs = make(map[string]string)
 			}
+			hdr.Xattrs[key] = string(value)
 		}
 	}
 	return nil
diff --git a/pkg/archive/changes_linux.go b/pkg/archive/changes_linux.go
index 15d582650..ceec53ada 100644
--- a/pkg/archive/changes_linux.go
+++ b/pkg/archive/changes_linux.go
@@ -92,18 +92,16 @@ func walkchunk(path string, fi os.FileInfo, dir string, root *FileInfo) error {
 	if err != nil && err != system.EOPNOTSUPP {
 		return err
 	}
-	if xattrs != nil {
-		for _, key := range xattrs {
-			if strings.HasPrefix(key, "user.") {
-				value, err := system.Lgetxattr(cpath, key)
-				if err != nil {
-					return err
-				}
-				if info.xattrs == nil {
-					info.xattrs = make(map[string]string)
-				}
-				info.xattrs[key] = string(value)
+	for _, key := range xattrs {
+		if strings.HasPrefix(key, "user.") {
+			value, err := system.Lgetxattr(cpath, key)
+			if err != nil {
+				return err
+			}
+			if info.xattrs == nil {
+				info.xattrs = make(map[string]string)
 			}
+			info.xattrs[key] = string(value)
 		}
 	}
 	parent.children[info.name] = info
