layer: numerous overlayfs whiteout fixes

CHANGELOG.md

@@ -34,6 +34,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
     `--compress` flag. You can also disable compression entirely using
     `--compress=none` but `--compress=auto` will never automatically choose
     `none` compression.
+- `GenerateLayer` and `GenerateInsertLayer` with `TranslateOverlayWhiteouts`
+  now support converting `trusted.overlay.opaque=y` and
+  `trusted.overlay.whiteout` whiteouts into OCI whiteouts when generating OCI
+  layers.
 
 ### Changes ###
 - In this release, the primary development branch was renamed to `main`.
@@ -61,6 +65,54 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   #452
 - umoci will now return an explicit error if you pass invalid uid or gid values
   to `--uid-map` and `--gid-map` rather than silently truncating the value.
+- For Go users of umoci, `GenerateLayer` (but not `GenerateInsertLayer`) with
+  the `TranslateOverlayWhiteouts` option enabled had several severe bugs that
+  made the feature unusable:
+  * All OCI whiteouts added to the archive would incorrectly have the full host
+    name of the path rather than the correctly rooted path, making the whiteout
+    practically useless.
+  * Any non-whiteout files would not be included in the layer, making the layer
+    data incomplete and thus resulting in silent data loss.
+  Given how severe these bugs were and the lack of bug reports of this issue in
+  the past 4 years, it seems this feature has not really been used by anyone (I
+  hope...).
+- For Go users of umoci, `UnpackLayer` now correctly handles several aspects of
+  `OverlayFSWhiteout` extraction that weren't handled correctly:
+  * Unlike regular extractions, overlayfs-style extractions require us to
+    create the parent directory of the whiteout (rather than ignoring or
+    assuming the underlying path exists) because the whiteout is being created
+    in a separate layer to the underlying file. We also need to make sure that
+    opaque whiteout targets are directories.
+  * `trusted.overlay.opaque=y` has very peculiar behaviour when a regular
+    whiteout (i.e. `mknod c 0 0`) is placed inside an opaque directory -- the
+    whiteout-ed file appears in `readdir` but the file itself doesn't exist. To
+    avoid this confusion (and possible information leak), umoci will no longer
+    extract plain whiteouts within an opaque whiteout directory in the same
+    layer. (As per the OCI spec requirements, this is regardless of the order
+    of the opaque whiteout and the regular whiteout in the layer archive.)
+- `UnpackLayer` and `Generate(Insert)Layer` now correctly handle
+  `trusted.overlay.*` xattr escaping when extracting and generating layers with
+  the overlayfs on-disk format. This escaping feature [has been supported by
+  overlayfs since Linux 6.7][linux-overlayfs-escaping-dad02fad84cbc], and
+  allows for you to created images that contain an overlayfs layout inside the
+  image (nested to arbitrary levels).
+  * If an image contains `trusted.overlay.*` xattrs, `UnpackLayer` will
+    rewrite the xattrs to instead be in the `trusted.overlay.overlay.*`
+    namespace, so that when merged using overlayfs the user will see the
+    expected xattrs.
+  * If an on-disk overlayfs directory used with `Generate(Insert)Layer`
+    contains escaped `trusted.overlay.overlay.*` xattrs, they will be rewritten
+    so that the generated layer contains `trusted.overlay.*` xattrs. If we
+    encounter an unescaped `trusted.overlay.*` xattr they will not be included
+    in the image (though they may cause the file to be converted to a whiteout
+    in the image) because they are considered to be an internal aspect of the
+    host on-disk format (i.e. `trusted.overlay.origin` might be automatically
+    set by whatever tool is using the overlayfs layers).
+  Note that in the regular extraction mode, these xattrs will be treated like
+  any other xattrs (this is in contrast to the previous behaviour where they
+  would be silently ignored regardless of the on-disk format being used).
+
+[linux-overlayfs-escaping-dad02fad84cbc]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=dad02fad84cbce30f317b69a4f2391f90045f79d
 
 ## [0.4.7] - 2021-04-05 ##
 

oci/layer/generate.go

@@ -22,14 +22,13 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"path"
 	"path/filepath"
 	"sort"
 
 	"github.com/apex/log"
 	"github.com/vbatts/go-mtree"
 
-	"github.com/opencontainers/umoci/pkg/unpriv"
+	"github.com/opencontainers/umoci/pkg/fseval"
 )
 
 // inodeDeltas is a wrapper around []mtree.InodeDelta that allows for sorting
@@ -50,6 +49,10 @@ func GenerateLayer(path string, deltas []mtree.InodeDelta, opt *RepackOptions) (
 	if opt != nil {
 		packOptions = *opt
 	}
+	fsEval := fseval.Default
+	if packOptions.MapOptions.Rootless {
+		fsEval = fseval.Rootless
+	}
 
 	reader, writer := io.Pipe()
 
@@ -68,7 +71,7 @@ func GenerateLayer(path string, deltas []mtree.InodeDelta, opt *RepackOptions) (
 		// We can't just dump all of the file contents into a tar file. We need
 		// to emulate a proper tar generator. Luckily there aren't that many
 		// things to emulate (and we can do them all in tar.go).
-		tg := newTarGenerator(writer, packOptions.MapOptions)
+		tg := newTarGenerator(writer, packOptions)
 
 		// Sort the delta paths.
 		// FIXME: We need to add whiteouts first, otherwise we might end up
@@ -87,21 +90,36 @@ func GenerateLayer(path string, deltas []mtree.InodeDelta, opt *RepackOptions) (
 			switch delta.Type() {
 			case mtree.Modified, mtree.Extra:
 				if packOptions.TranslateOverlayWhiteouts {
-					fi, err := os.Stat(fullPath)
+					woType, isWo, err := isOverlayWhiteout(fullPath, fsEval)
 					if err != nil {
-						return fmt.Errorf("couldn't determine overlay whiteout for %s: %w", fullPath, err)
+						return fmt.Errorf("check if %q is a whiteout: %w", fullPath, err)
 					}
-
-					whiteout, err := isOverlayWhiteout(fi)
-					if err != nil {
-						return err
-					}
-					if whiteout {
-						if err := tg.AddWhiteout(fullPath); err != nil {
-							return fmt.Errorf("generate whiteout from overlayfs: %w", err)
+					if isWo {
+						log.Debugf("generate layer: converting overlayfs whiteout %s %q to OCI whiteout", woType, name)
+
+						var err error
+						switch woType {
+						case overlayWhiteoutPlain:
+							err = tg.AddWhiteout(name)
+						case overlayWhiteoutOpaque:
+							// For opaque whiteout directories we need to
+							// output an entry for the directory itself so that
+							// the ownership and modes set on the directory are
+							// included in the archive.
+							if err := tg.AddFile(name, fullPath); err != nil {
+								log.Warnf("generate layer: could not add directory entry for opaque from overlayfs for file %q: %s", name, err)
+								return fmt.Errorf("generate directory entry for opaque whiteout from overlayfs: %w", err)
+							}
+							err = tg.AddOpaqueWhiteout(name)
+						default:
+							return fmt.Errorf("[internal error] unknown overlayfs whiteout type %q", woType)
+						}
+						if err != nil {
+							log.Warnf("generate layer: could not add whiteout %s from overlayfs for file %q: %s", woType, name, err)
+							return fmt.Errorf("generate whiteout %s from overlayfs: %w", woType, err)
 						}
+						continue
 					}
-					continue
 				}
 				if err := tg.AddFile(name, fullPath); err != nil {
 					log.Warnf("generate layer: could not add file %q: %s", name, err)
@@ -126,16 +144,22 @@ func GenerateLayer(path string, deltas []mtree.InodeDelta, opt *RepackOptions) (
 	return reader, nil
 }
 
-// GenerateInsertLayer generates a completely new layer from "root"to be
-// inserted into the image at "target". If "root" is an empty string then the
-// "target" will be removed via a whiteout.
-func GenerateInsertLayer(root string, target string, opaque bool, opt *RepackOptions) io.ReadCloser {
+// GenerateInsertLayer generates a completely new layer from root to be
+// inserted into the image at target. If root is an empty string then the
+// target will be removed via a whiteout. If opaque is true then the target
+// directory will also have an opaque whiteout applied (clearing any files
+// inside the directory), followed by the contents of the root.
+func GenerateInsertLayer(root, target string, opaque bool, opt *RepackOptions) io.ReadCloser {
 	root = CleanPath(root)
 
 	var packOptions RepackOptions
 	if opt != nil {
 		packOptions = *opt
 	}
+	fsEval := fseval.Default
+	if packOptions.MapOptions.Rootless {
+		fsEval = fseval.Rootless
+	}
 
 	reader, writer := io.Pipe()
 
@@ -150,38 +174,61 @@ func GenerateInsertLayer(root string, target string, opaque bool, opt *RepackOpt
 			_ = writer.CloseWithError(closeErr)
 		}()
 
-		tg := newTarGenerator(writer, packOptions.MapOptions)
+		tg := newTarGenerator(writer, packOptions)
 
 		defer func() {
 			if err := tg.tw.Close(); err != nil {
 				log.Warnf("generate insert layer: could not close tar.Writer: %s", err)
 			}
 		}()
 
+		if root == "" {
+			return tg.AddWhiteout(target)
+		}
 		if opaque {
 			if err := tg.AddOpaqueWhiteout(target); err != nil {
 				return err
 			}
+			// Continue on to add the new root contents...
 		}
-		if root == "" {
-			return tg.AddWhiteout(target)
-		}
-		return unpriv.Walk(root, func(curPath string, info os.FileInfo, err error) error {
+		return fsEval.Walk(root, func(fullPath string, _ os.FileInfo, err error) error {
 			if err != nil {
 				return err
 			}
 
-			pathInTar := path.Join(target, curPath[len(root):])
-			whiteout, err := isOverlayWhiteout(info)
+			relName, err := filepath.Rel(root, fullPath)
 			if err != nil {
 				return err
 			}
-			if packOptions.TranslateOverlayWhiteouts && whiteout {
-				log.Debugf("converting overlayfs whiteout %s to OCI whiteout", pathInTar)
-				return tg.AddWhiteout(pathInTar)
+			name := filepath.Join(target, relName)
+
+			if packOptions.TranslateOverlayWhiteouts {
+				woType, isWo, err := isOverlayWhiteout(fullPath, fsEval)
+				if err != nil {
+					return fmt.Errorf("check if %q is a whiteout: %w", fullPath, err)
+				}
+				if isWo {
+					log.Debugf("generate insert layer: converting overlayfs %s %q to OCI whiteout", woType, name)
+					switch woType {
+					case overlayWhiteoutPlain:
+						return tg.AddWhiteout(name)
+					case overlayWhiteoutOpaque:
+						// For opaque whiteout directories we need to
+						// output an entry for the directory itself so that
+						// the ownership and modes set on the directory are
+						// included in the archive.
+						if err := tg.AddFile(name, fullPath); err != nil {
+							log.Warnf("generate insert layer: could not add directory entry for opaque from overlayfs for file %q: %s", name, err)
+							return fmt.Errorf("generate directory entry for opaque whiteout from overlayfs: %w", err)
+						}
+						return tg.AddOpaqueWhiteout(name)
+					default:
+						return fmt.Errorf("[internal error] unknown overlayfs whiteout type %q", woType)
+					}
+				}
 			}
 
-			return tg.AddFile(pathInTar, curPath)
+			return tg.AddFile(name, fullPath)
 		})
 	}()
 	return reader