Ensure opaque directory permissions respected

When converting an opaque directory always keep the original
directory tar entry to ensure directory is created with correct
permissions on restore.

Closes #27298

Signed-off-by: Derek McGowan <derek@mcgstyle.net> (github: dmcgowan)

pkg/archive/archive.go

@@ -241,7 +241,7 @@ func (compression *Compression) Extension() string {
 }
 
 type tarWhiteoutConverter interface {
-	ConvertWrite(*tar.Header, string, os.FileInfo) error
+	ConvertWrite(*tar.Header, string, os.FileInfo) (*tar.Header, error)
 	ConvertRead(*tar.Header, string) (bool, error)
 }
 
@@ -348,9 +348,25 @@ func (ta *tarAppender) addTarFile(path, name string) error {
 	}
 
 	if ta.WhiteoutConverter != nil {
-		if err := ta.WhiteoutConverter.ConvertWrite(hdr, path, fi); err != nil {
+		wo, err := ta.WhiteoutConverter.ConvertWrite(hdr, path, fi)
+		if err != nil {
 			return err
 		}
+
+		// If a new whiteout file exists, write original hdr, then
+		// replace hdr with wo to be written after. Whiteouts should
+		// always be written after the original. Note the original
+		// hdr may have been updated to be a whiteout with returning
+		// a whiteout header
+		if wo != nil {
+			if err := ta.TarWriter.WriteHeader(hdr); err != nil {
+				return err
+			}
+			if hdr.Typeflag == tar.TypeReg && hdr.Size > 0 {
+				return fmt.Errorf("tar: cannot use whiteout for non-empty file")
+			}
+			hdr = wo
+		}
 	}
 
 	if err := ta.TarWriter.WriteHeader(hdr); err != nil {

pkg/archive/archive_linux.go

@@ -19,7 +19,7 @@ func getWhiteoutConverter(format WhiteoutFormat) tarWhiteoutConverter {
 
 type overlayWhiteoutConverter struct{}
 
-func (overlayWhiteoutConverter) ConvertWrite(hdr *tar.Header, path string, fi os.FileInfo) error {
+func (overlayWhiteoutConverter) ConvertWrite(hdr *tar.Header, path string, fi os.FileInfo) (wo *tar.Header, err error) {
 	// convert whiteouts to AUFS format
 	if fi.Mode()&os.ModeCharDevice != 0 && hdr.Devmajor == 0 && hdr.Devminor == 0 {
 		// we just rename the file and make it normal
@@ -34,12 +34,16 @@ func (overlayWhiteoutConverter) ConvertWrite(hdr *tar.Header, path string, fi os
 		// convert opaque dirs to AUFS format by writing an empty file with the prefix
 		opaque, err := system.Lgetxattr(path, "trusted.overlay.opaque")
 		if err != nil {
-			return err
+			return nil, err
 		}
 		if len(opaque) == 1 && opaque[0] == 'y' {
+			if hdr.Xattrs != nil {
+				delete(hdr.Xattrs, "trusted.overlay.opaque")
+			}
+
 			// create a header for the whiteout file
 			// it should inherit some properties from the parent, but be a regular file
-			*hdr = tar.Header{
+			wo = &tar.Header{
 				Typeflag:   tar.TypeReg,
 				Mode:       hdr.Mode & int64(os.ModePerm),
 				Name:       filepath.Join(hdr.Name, WhiteoutOpaqueDir),
@@ -54,7 +58,7 @@ func (overlayWhiteoutConverter) ConvertWrite(hdr *tar.Header, path string, fi os
 		}
 	}
 
-	return nil
+	return
 }
 
 func (overlayWhiteoutConverter) ConvertRead(hdr *tar.Header, path string) (bool, error) {

pkg/archive/archive_linux_test.go

@@ -0,0 +1,187 @@
+package archive
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"syscall"
+	"testing"
+
+	"github.com/docker/docker/pkg/system"
+)
+
+// setupOverlayTestDir creates files in a directory with overlay whiteouts
+// Tree layout
+// .
+// ├── d1     # opaque, 0700
+// │   └── f1 # empty file, 0600
+// ├── d2     # opaque, 0750
+// │   └── f1 # empty file, 0660
+// └── d3     # 0700
+//     └── f1 # whiteout, 0644
+func setupOverlayTestDir(t *testing.T, src string) {
+	// Create opaque directory containing single file and permission 0700
+	if err := os.Mkdir(filepath.Join(src, "d1"), 0700); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := system.Lsetxattr(filepath.Join(src, "d1"), "trusted.overlay.opaque", []byte("y"), 0); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := ioutil.WriteFile(filepath.Join(src, "d1", "f1"), []byte{}, 0600); err != nil {
+		t.Fatal(err)
+	}
+
+	// Create another opaque directory containing single file but with permission 0750
+	if err := os.Mkdir(filepath.Join(src, "d2"), 0750); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := system.Lsetxattr(filepath.Join(src, "d2"), "trusted.overlay.opaque", []byte("y"), 0); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := ioutil.WriteFile(filepath.Join(src, "d2", "f1"), []byte{}, 0660); err != nil {
+		t.Fatal(err)
+	}
+
+	// Create regular directory with deleted file
+	if err := os.Mkdir(filepath.Join(src, "d3"), 0700); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := system.Mknod(filepath.Join(src, "d3", "f1"), syscall.S_IFCHR, 0); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func checkOpaqueness(t *testing.T, path string, opaque string) {
+	xattrOpaque, err := system.Lgetxattr(path, "trusted.overlay.opaque")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(xattrOpaque) != opaque {
+		t.Fatalf("Unexpected opaque value: %q, expected %q", string(xattrOpaque), opaque)
+	}
+
+}
+
+func checkOverlayWhiteout(t *testing.T, path string) {
+	stat, err := os.Stat(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	statT, ok := stat.Sys().(*syscall.Stat_t)
+	if !ok {
+		t.Fatalf("Unexpected type: %t, expected *syscall.Stat_t", stat.Sys())
+	}
+	if statT.Rdev != 0 {
+		t.Fatalf("Non-zero device number for whiteout")
+	}
+}
+
+func checkFileMode(t *testing.T, path string, perm os.FileMode) {
+	stat, err := os.Stat(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if stat.Mode() != perm {
+		t.Fatalf("Unexpected file mode for %s: %o, expected %o", path, stat.Mode(), perm)
+	}
+}
+
+func TestOverlayTarUntar(t *testing.T) {
+	oldmask, err := system.Umask(0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer system.Umask(oldmask)
+
+	src, err := ioutil.TempDir("", "docker-test-overlay-tar-src")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(src)
+
+	setupOverlayTestDir(t, src)
+
+	dst, err := ioutil.TempDir("", "docker-test-overlay-tar-dst")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dst)
+
+	options := &TarOptions{
+		Compression:    Uncompressed,
+		WhiteoutFormat: OverlayWhiteoutFormat,
+	}
+	archive, err := TarWithOptions(src, options)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer archive.Close()
+
+	if err := Untar(archive, dst, options); err != nil {
+		t.Fatal(err)
+	}
+
+	checkFileMode(t, filepath.Join(dst, "d1"), 0700|os.ModeDir)
+	checkFileMode(t, filepath.Join(dst, "d2"), 0750|os.ModeDir)
+	checkFileMode(t, filepath.Join(dst, "d3"), 0700|os.ModeDir)
+	checkFileMode(t, filepath.Join(dst, "d1", "f1"), 0600)
+	checkFileMode(t, filepath.Join(dst, "d2", "f1"), 0660)
+	checkFileMode(t, filepath.Join(dst, "d3", "f1"), os.ModeCharDevice|os.ModeDevice)
+
+	checkOpaqueness(t, filepath.Join(dst, "d1"), "y")
+	checkOpaqueness(t, filepath.Join(dst, "d2"), "y")
+	checkOpaqueness(t, filepath.Join(dst, "d3"), "")
+	checkOverlayWhiteout(t, filepath.Join(dst, "d3", "f1"))
+}
+
+func TestOverlayTarAUFSUntar(t *testing.T) {
+	oldmask, err := system.Umask(0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer system.Umask(oldmask)
+
+	src, err := ioutil.TempDir("", "docker-test-overlay-tar-src")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(src)
+
+	setupOverlayTestDir(t, src)
+
+	dst, err := ioutil.TempDir("", "docker-test-overlay-tar-dst")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dst)
+
+	archive, err := TarWithOptions(src, &TarOptions{
+		Compression:    Uncompressed,
+		WhiteoutFormat: OverlayWhiteoutFormat,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer archive.Close()
+
+	if err := Untar(archive, dst, &TarOptions{
+		Compression:    Uncompressed,
+		WhiteoutFormat: AUFSWhiteoutFormat,
+	}); err != nil {
+		t.Fatal(err)
+	}
+
+	checkFileMode(t, filepath.Join(dst, "d1"), 0700|os.ModeDir)
+	checkFileMode(t, filepath.Join(dst, "d1", WhiteoutOpaqueDir), 0700)
+	checkFileMode(t, filepath.Join(dst, "d2"), 0750|os.ModeDir)
+	checkFileMode(t, filepath.Join(dst, "d2", WhiteoutOpaqueDir), 0750)
+	checkFileMode(t, filepath.Join(dst, "d3"), 0700|os.ModeDir)
+	checkFileMode(t, filepath.Join(dst, "d1", "f1"), 0600)
+	checkFileMode(t, filepath.Join(dst, "d2", "f1"), 0660)
+	checkFileMode(t, filepath.Join(dst, "d3", WhiteoutPrefix+"f1"), 0600)
+}