diff --git a/docs/containers-storage.conf.5.md b/docs/containers-storage.conf.5.md index 2f8e29b01f..e19c39ad38 100644 --- a/docs/containers-storage.conf.5.md +++ b/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: diff --git a/pkg/chunked/cache_linux.go b/pkg/chunked/cache_linux.go index 47742b05d8..ac0c2dacd4 100644 --- a/pkg/chunked/cache_linux.go +++ b/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 } diff --git a/pkg/chunked/storage_linux.go b/pkg/chunked/storage_linux.go index da43fde90f..85903b8f01 100644 --- a/pkg/chunked/storage_linux.go +++ b/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,38 @@ 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. + // + // Also, layers without a tar-split (estargz layers and old zstd:chunked layers) can't produce an UncompressedDigest that + // matches the expected RootFS.DiffID; they will need to use a special TOC-based layer ID, and we need to somehow (??) + // ensure unambiguity - either by always falling back to full pulls, or by never falling back. + 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 +1942,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) +} diff --git a/storage.conf b/storage.conf index 962325233e..909e47b39b 100644 --- a/storage.conf +++ b/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