When applying a chunked layer with a tar-split, compute its uncompres…

…sed digest

This will allow c/image to validate the uncompressed digest against the config's
RootFS.DiffID value (ensuring that the layer's contents are the same when pulled
via TOC and traditionally); and the uncompressed digest will be used as a layer ID,
ensuring users see the traditional layer and image IDs they are used to.

This doesn't work for layers without a tar-split (all estargz, and old zstd:chunked
layers); for those, we fall back to traditional pulls.

Alternatively, for EXTREMELY restricted use cases, add an
"insecure_allow_unpredictable_image_contents" option to storage.conf. This option
allows partial pulls of estargz and old zstd:chunked layers, and skips the costly
uncompressed digest computation. It is then up to the user to worry about
images where the tar representaiton and the TOC representation don't match,
and about unpredictable image IDs.

Signed-off-by: Miloslav Trmač <[email protected]>

docs/containers-storage.conf.5.md

@@ -124,6 +124,27 @@ The `storage.options.pull_options` table supports the following keys:
   It is an expensive operation so it is not enabled by default.
   This is a "string bool": "false"|"true" (cannot be native TOML boolean)
 
+**insecure_allow_unpredictable_image_contents="false"|"true"**
+  This should _almost never_ be set.
+  It allows partial pulls ofof images without guaranteeing that "partial
+  pulls" and non-partial pulls both result in consistent image contents.
+  This allows pulling estargz images and early versions of zstd:chunked images;
+  otherwise, these layers always use the traditional non-partial pull path.
+
+  This option should be enabled _extremely_ rarely, only if _all_ images that could
+  EVER be concievably pulled on this system are _guaranteed_ (e.g. using a signature policy)
+  to come from a build system trusted to never attack image integrity.
+
+  If this consistency enforcement were disabled, malicious images could be built
+  in a way designed to evade other audit mechanisms, so presence of most other audit
+  mechanisms is not a replacement for the above-mentioned need for all images to come
+  from a trusted build system.
+
+  As a side effect, enabling this option will also make image IDs unpredictable
+  (usually not equal to the traditional value matching the config digest).
+
+  This is a "string bool": "false"|"true" (cannot be native TOML boolean)
+
 ### STORAGE OPTIONS FOR AUFS TABLE
 
 The `storage.options.aufs` table supports the following options:

pkg/chunked/cache_linux.go

@@ -710,7 +710,7 @@ func prepareCacheFile(manifest []byte, format graphdriver.DifferOutputFormat) ([
 	switch format {
 	case graphdriver.DifferOutputFormatDir:
 	case graphdriver.DifferOutputFormatFlat:
-		entries, err = makeEntriesFlat(entries)
+		entries, err = makeEntriesFlat(entries, nil)
 		if err != nil {
 			return nil, err
 		}

pkg/chunked/storage_linux.go

@@ -2,6 +2,7 @@ package chunked
 
 import (
 	archivetar "archive/tar"
+	"bytes"
 	"context"
 	"encoding/base64"
 	"errors"
@@ -28,12 +29,15 @@ import (
 	"github.com/containers/storage/pkg/fsverity"
 	"github.com/containers/storage/pkg/idtools"
 	"github.com/containers/storage/pkg/system"
+	securejoin "github.com/cyphar/filepath-securejoin"
 	jsoniter "github.com/json-iterator/go"
 	"github.com/klauspost/compress/zstd"
 	"github.com/klauspost/pgzip"
 	digest "github.com/opencontainers/go-digest"
 	"github.com/sirupsen/logrus"
 	"github.com/vbatts/tar-split/archive/tar"
+	"github.com/vbatts/tar-split/tar/asm"
+	tsStorage "github.com/vbatts/tar-split/tar/storage"
 	"golang.org/x/sys/unix"
 )
 
@@ -113,10 +117,11 @@ type chunkedLayerData struct {
 // TO DO: ideally this should be parsed along with the rest of the config file into StoreOptions directly
 // (and then storage.Store.PullOptions would need to be somehow simulated).
 type pullOptions struct {
-	enablePartialImages bool     // enable_partial_images
-	convertImages       bool     // convert_images
-	useHardLinks        bool     // use_hard_links
-	ostreeRepos         []string // ostree_repos
+	enablePartialImages                     bool     // enable_partial_images
+	convertImages                           bool     // convert_images
+	useHardLinks                            bool     // use_hard_links
+	insecureAllowUnpredictableImageContents bool     // insecure_allow_unpredictable_image_contents
+	ostreeRepos                             []string // ostree_repos
 }
 
 func parsePullOptions(store storage.Store) pullOptions {
@@ -131,6 +136,7 @@ func parsePullOptions(store storage.Store) pullOptions {
 		{&res.enablePartialImages, "enable_partial_images", false},
 		{&res.convertImages, "convert_images", false},
 		{&res.useHardLinks, "use_hard_links", false},
+		{&res.insecureAllowUnpredictableImageContents, "insecure_allow_unpredictable_image_contents", false},
 	} {
 		if value, ok := options[e.name]; ok {
 			*e.dest = strings.ToLower(value) == "true"
@@ -294,12 +300,15 @@ func makeZstdChunkedDiffer(store storage.Store, blobSize int64, tocDigest digest
 		var badRequestErr ErrBadRequest
 		return nil, errors.As(err, &badRequestErr), fmt.Errorf("read zstd:chunked manifest: %w", err)
 	}
+
 	var uncompressedTarSize int64 = -1
 	if tarSplit != nil {
 		uncompressedTarSize, err = tarSizeFromTarSplit(tarSplit)
 		if err != nil {
 			return nil, false, fmt.Errorf("computing size from tar-split: %w", err)
 		}
+	} else if !pullOptions.insecureAllowUnpredictableImageContents { // With no tar-split, we can't compute the traditional UncompressedDigest.
+		return nil, true, fmt.Errorf("zstd:chunked layers without tar-split data don't support partial pulls with guaranteed consistency with non-partial pulls")
 	}
 
 	layersCache, err := getLayersCache(store)
@@ -329,6 +338,10 @@ func makeZstdChunkedDiffer(store storage.Store, blobSize int64, tocDigest digest
 // On error, the second return value is true if a fallback to an alternative (either the makeConverToRaw differ, or a non-partial pull)
 // is permissible.
 func makeEstargzChunkedDiffer(store storage.Store, blobSize int64, tocDigest digest.Digest, iss ImageSourceSeekable, pullOptions pullOptions) (*chunkedDiffer, bool, error) {
+	if !pullOptions.insecureAllowUnpredictableImageContents { // With no tar-split, we can't compute the traditional UncompressedDigest.
+		return nil, true, fmt.Errorf("estargz layers don't support partial pulls with guaranteed consistency with non-partial pulls")
+	}
+
 	manifest, tocOffset, err := readEstargzChunkedManifest(iss, blobSize, tocDigest)
 	if err != nil {
 		// If the error is a bad request to the server, then signal to the caller that it can try a different method.
@@ -1150,7 +1163,10 @@ func (c *chunkedDiffer) findAndCopyFile(dirfd int, r *fileMetadata, copyOptions
 	return false, nil
 }
 
-func makeEntriesFlat(mergedEntries []fileMetadata) ([]fileMetadata, error) {
+// makeEntriesFlat collects regular-file entries from mergedEntries, and produces a new list
+// where each file content is only represented once, and uses composefs.RegularFilePathForValidatedDigest for its name.
+// if flatPathNameMap is not nil, this function writes to it a mapping from filepath.Clean(originalName) to the composefs name.
+func makeEntriesFlat(mergedEntries []fileMetadata, flatPathNameMap map[string]string) ([]fileMetadata, error) {
 	var new []fileMetadata
 
 	knownFlatPaths := make(map[string]struct{})
@@ -1169,6 +1185,9 @@ func makeEntriesFlat(mergedEntries []fileMetadata) ([]fileMetadata, error) {
 		if err != nil {
 			return nil, fmt.Errorf("determining physical file path for %q: %w", mergedEntries[i].Name, err)
 		}
+		if flatPathNameMap != nil {
+			flatPathNameMap[filepath.Clean(mergedEntries[i].Name)] = path
+		}
 
 		if _, known := knownFlatPaths[path]; known {
 			continue
@@ -1468,10 +1487,13 @@ func (c *chunkedDiffer) ApplyDiff(dest string, options *archive.TarOptions, diff
 	if err != nil {
 		return output, &fs.PathError{Op: "open", Path: dest, Err: err}
 	}
-	defer unix.Close(dirfd)
+	dirFile := os.NewFile(uintptr(dirfd), dest)
+	defer dirFile.Close()
 
+	var flatPathNameMap map[string]string // = nil
 	if differOpts != nil && differOpts.Format == graphdriver.DifferOutputFormatFlat {
-		mergedEntries, err = makeEntriesFlat(mergedEntries)
+		flatPathNameMap = map[string]string{}
+		mergedEntries, err = makeEntriesFlat(mergedEntries, flatPathNameMap)
 		if err != nil {
 			return output, err
 		}
@@ -1763,6 +1785,39 @@ func (c *chunkedDiffer) ApplyDiff(dest string, options *archive.TarOptions, diff
 		}
 	}
 
+	// To ensure that consumers of the layer who decompress and read the full tar stream,
+	// and consumers who consume the data via the TOC, both see exactly the same data and metadata,
+	// compute the UncompressedDigest.
+	// c/image will then ensure that this value matches the value in the image config’s RootFS.DiffID, i.e. the image must commit
+	// to one UncompressedDigest value for each layer, and that will avoid the ambiguity (in consumers who validate layers against DiffID).
+	//
+	// c/image also uses the UncompressedDigest as a layer ID, allowing it to use the traditional layer and image IDs.
+	//
+	// This is, sadly, quite costly: Up to now we might have only have had to write, and digest, only the new/modified files.
+	// Here we need to read, and digest, the whole layer, even if almost all of it was already present locally previously.
+	// So, really specialized (EXTREMELY RARE) users can opt out of this check using insecureAllowUnpredictableImageContents .
+	//
+	// Layers without a tar-split (estargz layers and old zstd:chunked layers) can't produce an UncompressedDigest that
+	// matches the expected RootFS.DiffID; we always fall back to full pulls, again unless the user opts out
+	// via insecureAllowUnpredictableImageContents .
+	if output.UncompressedDigest == "" {
+		switch {
+		case c.pullOptions.insecureAllowUnpredictableImageContents:
+			// Oh well.  Skip the costly digest computation.
+		case output.TarSplit != nil:
+			metadata := tsStorage.NewJSONUnpacker(bytes.NewReader(output.TarSplit))
+			fg := newStagedFileGetter(dirFile, flatPathNameMap)
+			digester := digest.Canonical.Digester()
+			if err := asm.WriteOutputTarStream(fg, metadata, digester.Hash()); err != nil {
+				return output, fmt.Errorf("digesting staged uncompressed stream: %w", err)
+			}
+			output.UncompressedDigest = digester.Digest()
+		default:
+			// We are checking for this earlier in GetDiffer, so this should not be reachable.
+			return output, fmt.Errorf(`internal error: layer's UncompressedDigest is unknown and "insecure_allow_unpredictable_image_contents" is not set`)
+		}
+	}
+
 	if totalChunksSize > 0 {
 		logrus.Debugf("Missing %d bytes out of %d (%.2f %%)", missingPartsSize, totalChunksSize, float32(missingPartsSize*100.0)/float32(totalChunksSize))
 	}
@@ -1888,3 +1943,33 @@ func validateChunkChecksum(chunk *internal.FileMetadata, root, path string, offs
 
 	return digester.Digest() == digest
 }
+
+// newStagedFileGetter returns an object usable as storage.FileGetter for rootDir.
+// if flatPathNameMap is not nil, it must be used to map logical file names into the backing file paths.
+func newStagedFileGetter(rootDir *os.File, flatPathNameMap map[string]string) *stagedFileGetter {
+	return &stagedFileGetter{
+		rootDir:         rootDir,
+		flatPathNameMap: flatPathNameMap,
+	}
+}
+
+type stagedFileGetter struct {
+	rootDir         *os.File
+	flatPathNameMap map[string]string // nil, or a map from filepath.Clean()ed tar file names to expected on-filesystem names
+}
+
+func (fg *stagedFileGetter) Get(filename string) (io.ReadCloser, error) {
+	if fg.flatPathNameMap != nil {
+		path, ok := fg.flatPathNameMap[filepath.Clean(filename)]
+		if !ok {
+			return nil, fmt.Errorf("no path mapping exists for tar entry %q", filename)
+		}
+		filename = path
+	}
+	pathFD, err := securejoin.OpenatInRoot(fg.rootDir, filename)
+	if err != nil {
+		return nil, err
+	}
+	defer pathFD.Close()
+	return securejoin.Reopen(pathFD, unix.O_RDONLY)
+}

storage.conf

@@ -80,6 +80,25 @@ additionalimagestores = [
 # This is a "string bool": "false" | "true" (cannot be native TOML boolean)
 # convert_images = "false"
 
+# This should ALMOST NEVER be set.
+# It allows partial pulls ofof images without guaranteeing that "partial
+# pulls" and non-partial pulls both result in consistent image contents.
+# This allows pulling estargz images and early versions of zstd:chunked images;
+# otherwise, these layers always use the traditional non-partial pull path.
+#
+# This option should be enabled EXTREMELY rarely, only if ALL images that could
+# EVER be concievably pulled on this system are GUARANTEED (e.g. using a signature policy)
+# to come from a build system trusted to never attack image integrity.
+#
+# If this consistency enforcement were disabled, malicious images could be built
+# in a way designed to evade other audit mechanisms, so presence of most other audit
+# mechanisms is not a replacement for the above-mentioned need for all images to come
+# from a trusted build system.
+#
+# As a side effect, enabling this option will also make image IDs unpredictable
+# (usually not equal to the traditional value matching the config digest).
+# insecure_allow_unpredictable_image_contents = "false"
+
 # Root-auto-userns-user is a user name which can be used to look up one or more UID/GID
 # ranges in the /etc/subuid and /etc/subgid file.  These ranges will be partitioned
 # to containers configured to create automatically a user namespace.  Containers