Merge pull request #2049 from cgwalters/unpack-via-fd

chrootarchive: Pass root via fd

pkg/chrootarchive/archive.go

@@ -83,6 +83,12 @@ func untarHandler(tarArchive io.Reader, dest string, options *archive.TarOptions
 		}
 	}
 
+	destVal, err := newUnpackDestination(root, dest)
+	if err != nil {
+		return err
+	}
+	defer destVal.Close()
+
 	r := tarArchive
 	if decompress {
 		decompressedArchive, err := archive.DecompressStream(tarArchive)
@@ -93,7 +99,7 @@ func untarHandler(tarArchive io.Reader, dest string, options *archive.TarOptions
 		r = decompressedArchive
 	}
 
-	return invokeUnpack(r, dest, options, root)
+	return invokeUnpack(r, destVal, options)
 }
 
 // Tar tars the requested path while chrooted to the specified root.

pkg/chrootarchive/archive_darwin.go

@@ -6,12 +6,26 @@ import (
 	"github.com/containers/storage/pkg/archive"
 )
 
+type unpackDestination struct {
+	dest string
+}
+
+func (dst *unpackDestination) Close() error {
+	return nil
+}
+
+// newUnpackDestination is a no-op on this platform
+func newUnpackDestination(root, dest string) (*unpackDestination, error) {
+	return &unpackDestination{
+		dest: dest,
+	}, nil
+}
+
 func invokeUnpack(decompressedArchive io.Reader,
-	dest string,
-	options *archive.TarOptions, root string,
+	dest *unpackDestination,
+	options *archive.TarOptions,
 ) error {
-	_ = root // Restricting the operation to this root is not implemented on macOS
-	return archive.Unpack(decompressedArchive, dest, options)
+	return archive.Unpack(decompressedArchive, dest.dest, options)
 }
 
 func invokePack(srcPath string, options *archive.TarOptions, root string) (io.ReadCloser, error) {

pkg/chrootarchive/archive_unix.go

@@ -9,15 +9,41 @@ import (
 	"flag"
 	"fmt"
 	"io"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"runtime"
 	"strings"
 
+	"golang.org/x/sys/unix"
+
 	"github.com/containers/storage/pkg/archive"
 	"github.com/containers/storage/pkg/reexec"
 )
 
+type unpackDestination struct {
+	root *os.File
+	dest string
+}
+
+func (dst *unpackDestination) Close() error {
+	return dst.root.Close()
+}
+
+// tarOptionsDescriptor is passed as an extra file
+const tarOptionsDescriptor = 3
+
+// rootFileDescriptor is passed as an extra file
+const rootFileDescriptor = 4
+
+// procPathForFd gives us a string for a descriptor.
+// Note that while Linux supports actually *reading* this
+// path, FreeBSD and other platforms don't; but in this codebase
+// we only compare strings.
+func procPathForFd(fd int) string {
+	return fmt.Sprintf("/proc/self/fd/%d", fd)
+}
+
 // untar is the entry-point for storage-untar on re-exec. This is not used on
 // Windows as it does not support chroot, hence no point sandboxing through
 // chroot and rexec.
@@ -28,7 +54,7 @@ func untar() {
 	var options archive.TarOptions
 
 	// read the options from the pipe "ExtraFiles"
-	if err := json.NewDecoder(os.NewFile(3, "options")).Decode(&options); err != nil {
+	if err := json.NewDecoder(os.NewFile(tarOptionsDescriptor, "options")).Decode(&options); err != nil {
 		fatal(err)
 	}
 
@@ -38,7 +64,17 @@ func untar() {
 		root = flag.Arg(1)
 	}
 
-	if root == "" {
+	// FreeBSD doesn't have proc/self, but we can handle it here
+	if root == procPathForFd(rootFileDescriptor) {
+		// Take ownership to ensure it's closed; no need to leak
+		// this afterwards.
+		rootFd := os.NewFile(rootFileDescriptor, "tar-root")
+		defer rootFd.Close()
+		if err := unix.Fchdir(int(rootFd.Fd())); err != nil {
+			fatal(err)
+		}
+		root = "."
+	} else if root == "" {
 		root = dst
 	}
 
@@ -57,11 +93,35 @@ func untar() {
 	os.Exit(0)
 }
 
-func invokeUnpack(decompressedArchive io.Reader, dest string, options *archive.TarOptions, root string) error {
+// newUnpackDestination takes a root directory and a destination which
+// must be underneath it, and returns an object that can unpack
+// in the target root using a file descriptor.
+func newUnpackDestination(root, dest string) (*unpackDestination, error) {
 	if root == "" {
-		return errors.New("must specify a root to chroot to")
+		return nil, errors.New("must specify a root to chroot to")
+	}
+	relDest, err := filepath.Rel(root, dest)
+	if err != nil {
+		return nil, err
+	}
+	if relDest == "." {
+		relDest = "/"
+	}
+	if relDest[0] != '/' {
+		relDest = "/" + relDest
 	}
 
+	rootfdRaw, err := unix.Open(root, unix.O_RDONLY|unix.O_DIRECTORY|unix.O_CLOEXEC, 0)
+	if err != nil {
+		return nil, &fs.PathError{Op: "open", Path: root, Err: err}
+	}
+	return &unpackDestination{
+		root: os.NewFile(uintptr(rootfdRaw), "rootfs"),
+		dest: relDest,
+	}, nil
+}
+
+func invokeUnpack(decompressedArchive io.Reader, dest *unpackDestination, options *archive.TarOptions) error {
 	// We can't pass a potentially large exclude list directly via cmd line
 	// because we easily overrun the kernel's max argument/environment size
 	// when the full image list is passed (e.g. when this is used by
@@ -72,24 +132,13 @@ func invokeUnpack(decompressedArchive io.Reader, dest string, options *archive.T
 		return fmt.Errorf("untar pipe failure: %w", err)
 	}
 
-	if root != "" {
-		relDest, err := filepath.Rel(root, dest)
-		if err != nil {
-			return err
-		}
-		if relDest == "." {
-			relDest = "/"
-		}
-		if relDest[0] != '/' {
-			relDest = "/" + relDest
-		}
-		dest = relDest
-	}
-
-	cmd := reexec.Command("storage-untar", dest, root)
+	cmd := reexec.Command("storage-untar", dest.dest, procPathForFd(rootFileDescriptor))
 	cmd.Stdin = decompressedArchive
 
-	cmd.ExtraFiles = append(cmd.ExtraFiles, r)
+	// If you change this, change tarOptionsDescriptor above
+	cmd.ExtraFiles = append(cmd.ExtraFiles, r) // fd 3
+	// If you change this, change rootFileDescriptor above too
+	cmd.ExtraFiles = append(cmd.ExtraFiles, dest.root) // fd 4
 	output := bytes.NewBuffer(nil)
 	cmd.Stdout = output
 	cmd.Stderr = output

pkg/chrootarchive/archive_windows.go

@@ -7,19 +7,34 @@ import (
 	"github.com/containers/storage/pkg/longpath"
 )
 
+type unpackDestination struct {
+	dest string
+}
+
+func (dst *unpackDestination) Close() error {
+	return nil
+}
+
+// newUnpackDestination is a no-op on this platform
+func newUnpackDestination(root, dest string) (*unpackDestination, error) {
+	return &unpackDestination{
+		dest: dest,
+	}, nil
+}
+
 // chroot is not supported by Windows
 func chroot(path string) error {
 	return nil
 }
 
 func invokeUnpack(decompressedArchive io.Reader,
-	dest string,
-	options *archive.TarOptions, root string,
+	dest *unpackDestination,
+	options *archive.TarOptions,
 ) error {
 	// Windows is different to Linux here because Windows does not support
 	// chroot. Hence there is no point sandboxing a chrooted process to
 	// do the unpack. We call inline instead within the daemon process.
-	return archive.Unpack(decompressedArchive, longpath.AddPrefix(dest), options)
+	return archive.Unpack(decompressedArchive, longpath.AddPrefix(dest.dest), options)
 }
 
 func invokePack(srcPath string, options *archive.TarOptions, root string) (io.ReadCloser, error) {