compat API: allow enforcing short-names resolution to Docker Hub

The Docker-compatible REST API has historically behaved just as the rest
of Podman and Buildah (and the atomic Docker in older RHEL/Fedora) where
`containers-registries.conf` is centrally controlling which registries
a short name may resolve to during pull or local image lookups.  Please
refer to a blog for more details [1].

Docker, however, is only resolving short names to docker.io which has
been reported (see #12320) to break certain clients who rely on this
behavior.  In order to support this scenario, `containers.conf(5)`
received a new option to control whether Podman's compat API resolves
to docker.io only or behaves as before.

Most endpoints allow for directly normalizing parameters that represent
an image.  If set in containers.conf, Podman will then normalize the
references directly to docker.io.  The build endpoint is an outlier
since images are also referenced in Dockerfiles.  The Buildah API,
however, supports specifying a custom `types.SystemContext` in which
we can set a field that enforces short-name resolution to docker.io
in `c/image/pkg/shortnames`.

Notice that this a "hybrid" approach of doing the normalization directly
in the compat endpoints *and* in `pkg/shortnames` by passing a system
context.  Doing such a hybrid approach is neccessary since the compat
and the libpod endpoints share the same `libimage.Runtime` which makes
a global enforcement via the `libimage.Runtime.systemContext`
impossible.  Having two separate runtimes for the compat and the libpod
endpoints seems risky and not generally applicable to all endpoints.

[1] https://www.redhat.com/sysadmin/container-image-short-names

Fixes: #12320
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>

6 files changed, 136 insertions, 57 deletions

diff --git a/vendor/github.com/containers/common/libimage/search.go b/vendor/github.com/containers/common/libimage/search.go
index ece81531a..33a4776ce 100644
--- a/vendor/github.com/containers/common/libimage/search.go
+++ b/vendor/github.com/containers/common/libimage/search.go
@@ -58,6 +58,10 @@ type SearchOptions struct {
 	InsecureSkipTLSVerify types.OptionalBool
 	// ListTags returns the search result with available tags
 	ListTags bool
+	// Registries to search if the specified term does not include a
+	// registry.  If set, the unqualified-search registries in
+	// containers-registries.conf(5) are ignored.
+	Registries []string
 }
 
 // SearchFilter allows filtering images while searching.
@@ -105,6 +109,10 @@ func ParseSearchFilter(filter []string) (*SearchFilter, error) {
 	return sFilter, nil
 }
 
+// Search searches term.  If term includes a registry, only this registry will
+// be used for searching.  Otherwise, the unqualified-search registries in
+// containers-registries.conf(5) or the ones specified in the options will be
+// used.
 func (r *Runtime) Search(ctx context.Context, term string, options *SearchOptions) ([]SearchResult, error) {
 	if options == nil {
 		options = &SearchOptions{}
@@ -117,10 +125,14 @@ func (r *Runtime) Search(ctx context.Context, term string, options *SearchOption
 	// that we cannot use the reference parser from the containers/image
 	// library as the search term may container arbitrary input such as
 	// wildcards.  See bugzilla.redhat.com/show_bug.cgi?id=1846629.
-	if spl := strings.SplitN(term, "/", 2); len(spl) > 1 {
-		searchRegistries = append(searchRegistries, spl[0])
+	spl := strings.SplitN(term, "/", 2)
+	switch {
+	case len(spl) > 1:
+		searchRegistries = []string{spl[0]}
 		term = spl[1]
-	} else {
+	case len(options.Registries) > 0:
+		searchRegistries = options.Registries
+	default:
 		regs, err := sysregistriesv2.UnqualifiedSearchRegistries(r.systemContextCopy())
 		if err != nil {
 			return nil, err
diff --git a/vendor/github.com/containers/image/v5/copy/copy.go b/vendor/github.com/containers/image/v5/copy/copy.go
index e1649ba8e..317f8922a 100644
--- a/vendor/github.com/containers/image/v5/copy/copy.go
+++ b/vendor/github.com/containers/image/v5/copy/copy.go
@@ -80,13 +80,13 @@ type copier struct {
 
 // imageCopier tracks state specific to a single image (possibly an item of a manifest list)
 type imageCopier struct {
-	c                  *copier
-	manifestUpdates    *types.ManifestUpdateOptions
-	src                types.Image
-	diffIDsAreNeeded   bool
-	canModifyManifest  bool
-	canSubstituteBlobs bool
-	ociEncryptLayers   *[]int
+	c                          *copier
+	manifestUpdates            *types.ManifestUpdateOptions
+	src                        types.Image
+	diffIDsAreNeeded           bool
+	cannotModifyManifestReason string // The reason the manifest cannot be modified, or an empty string if it can
+	canSubstituteBlobs         bool
+	ociEncryptLayers           *[]int
 }
 
 const (
@@ -129,10 +129,14 @@ type Options struct {
 	DestinationCtx   *types.SystemContext
 	ProgressInterval time.Duration                 // time to wait between reports to signal the progress channel
 	Progress         chan types.ProgressProperties // Reported to when ProgressInterval has arrived for a single artifact+offset.
+
+	// Preserve digests, and fail if we cannot.
+	PreserveDigests bool
 	// manifest MIME type of image set by user. "" is default and means use the autodetection to the the manifest MIME type
 	ForceManifestMIMEType string
 	ImageListSelection    ImageListSelection // set to either CopySystemImage (the default), CopyAllImages, or CopySpecificImages to control which instances we copy when the source reference is a list; ignored if the source reference is not a list
 	Instances             []digest.Digest    // if ImageListSelection is CopySpecificImages, copy only these instances and the list itself
+
 	// If OciEncryptConfig is non-nil, it indicates that an image should be encrypted.
 	// The encryption options is derived from the construction of EncryptConfig object.
 	// Note: During initial encryption process of a layer, the resultant digest is not known
@@ -410,7 +414,36 @@ func (c *copier) copyMultipleImages(ctx context.Context, policyContext *signatur
 			return nil, errors.Wrapf(err, "Can not copy signatures to %s", transports.ImageName(c.dest.Reference()))
 		}
 	}
-	canModifyManifestList := (len(sigs) == 0)
+
+	// If the destination is a digested reference, make a note of that, determine what digest value we're
+	// expecting, and check that the source manifest matches it.
+	destIsDigestedReference := false
+	if named := c.dest.Reference().DockerReference(); named != nil {
+		if digested, ok := named.(reference.Digested); ok {
+			destIsDigestedReference = true
+			matches, err := manifest.MatchesDigest(manifestList, digested.Digest())
+			if err != nil {
+				return nil, errors.Wrapf(err, "computing digest of source image's manifest")
+			}
+			if !matches {
+				return nil, errors.New("Digest of source image's manifest would not match destination reference")
+			}
+		}
+	}
+
+	// Determine if we're allowed to modify the manifest list.
+	// If we can, set to the empty string. If we can't, set to the reason why.
+	// Compare, and perhaps keep in sync with, the version in copyOneImage.
+	cannotModifyManifestListReason := ""
+	if len(sigs) > 0 {
+		cannotModifyManifestListReason = "Would invalidate signatures"
+	}
+	if destIsDigestedReference {
+		cannotModifyManifestListReason = "Destination specifies a digest"
+	}
+	if options.PreserveDigests {
+		cannotModifyManifestListReason = "Instructed to preserve digests"
+	}
 
 	// Determine if we'll need to convert the manifest list to a different format.
 	forceListMIMEType := options.ForceManifestMIMEType
@@ -425,8 +458,8 @@ func (c *copier) copyMultipleImages(ctx context.Context, policyContext *signatur
 		return nil, errors.Wrapf(err, "determining manifest list type to write to destination")
 	}
 	if selectedListType != originalList.MIMEType() {
-		if !canModifyManifestList {
-			return nil, errors.Errorf("manifest list must be converted to type %q to be written to destination, but that would invalidate signatures", selectedListType)
+		if cannotModifyManifestListReason != "" {
+			return nil, errors.Errorf("Manifest list must be converted to type %q to be written to destination, but we cannot modify it: %q", selectedListType, cannotModifyManifestListReason)
 		}
 	}
 
@@ -510,8 +543,8 @@ func (c *copier) copyMultipleImages(ctx context.Context, policyContext *signatur
 
 		// If we can't just use the original value, but we have to change it, flag an error.
 		if !bytes.Equal(attemptedManifestList, originalManifestList) {
-			if !canModifyManifestList {
-				return nil, errors.Errorf(" manifest list must be converted to type %q to be written to destination, but that would invalidate signatures", thisListType)
+			if cannotModifyManifestListReason != "" {
+				return nil, errors.Errorf("Manifest list must be converted to type %q to be written to destination, but we cannot modify it: %q", thisListType, cannotModifyManifestListReason)
 			}
 			logrus.Debugf("Manifest list has been updated")
 		} else {
@@ -629,13 +662,27 @@ func (c *copier) copyOneImage(ctx context.Context, policyContext *signature.Poli
 		}
 	}
 
+	// Determine if we're allowed to modify the manifest.
+	// If we can, set to the empty string. If we can't, set to the reason why.
+	// Compare, and perhaps keep in sync with, the version in copyMultipleImages.
+	cannotModifyManifestReason := ""
+	if len(sigs) > 0 {
+		cannotModifyManifestReason = "Would invalidate signatures"
+	}
+	if destIsDigestedReference {
+		cannotModifyManifestReason = "Destination specifies a digest"
+	}
+	if options.PreserveDigests {
+		cannotModifyManifestReason = "Instructed to preserve digests"
+	}
+
 	ic := imageCopier{
 		c:               c,
 		manifestUpdates: &types.ManifestUpdateOptions{InformationOnly: types.ManifestUpdateInformation{Destination: c.dest}},
 		src:             src,
 		// diffIDsAreNeeded is computed later
-		canModifyManifest: len(sigs) == 0 && !destIsDigestedReference,
-		ociEncryptLayers:  options.OciEncryptLayers,
+		cannotModifyManifestReason: cannotModifyManifestReason,
+		ociEncryptLayers:           options.OciEncryptLayers,
 	}
 	// Ensure _this_ copy sees exactly the intended data when either processing a signed image or signing it.
 	// This may be too conservative, but for now, better safe than sorry, _especially_ on the SignBy path:
@@ -643,7 +690,7 @@ func (c *copier) copyOneImage(ctx context.Context, policyContext *signature.Poli
 	// We do intend the RecordDigestUncompressedPair calls to only work with reliable data, but at least there’s a risk
 	// that the compressed version coming from a third party may be designed to attack some other decompressor implementation,
 	// and we would reuse and sign it.
-	ic.canSubstituteBlobs = ic.canModifyManifest && options.SignBy == ""
+	ic.canSubstituteBlobs = ic.cannotModifyManifestReason == "" && options.SignBy == ""
 
 	if err := ic.updateEmbeddedDockerReference(); err != nil {
 		return nil, "", "", err
@@ -710,10 +757,10 @@ func (c *copier) copyOneImage(ctx context.Context, policyContext *signature.Poli
 		}
 		// If the original MIME type is acceptable, determineManifestConversion always uses it as preferredManifestMIMEType.
 		// So if we are here, we will definitely be trying to convert the manifest.
-		// With !ic.canModifyManifest, that would just be a string of repeated failures for the same reason,
+		// With ic.cannotModifyManifestReason != "", that would just be a string of repeated failures for the same reason,
 		// so let’s bail out early and with a better error message.
-		if !ic.canModifyManifest {
-			return nil, "", "", errors.Wrap(err, "Writing manifest failed (and converting it is not possible, image is signed or the destination specifies a digest)")
+		if ic.cannotModifyManifestReason != "" {
+			return nil, "", "", errors.Wrapf(err, "Writing manifest failed and we cannot try conversions: %q", cannotModifyManifestReason)
 		}
 
 		// errs is a list of errors when trying various manifest types. Also serves as an "upload succeeded" flag when set to nil.
@@ -813,9 +860,9 @@ func (ic *imageCopier) updateEmbeddedDockerReference() error {
 		return nil // No reference embedded in the manifest, or it matches destRef already.
 	}
 
-	if !ic.canModifyManifest {
-		return errors.Errorf("Copying a schema1 image with an embedded Docker reference to %s (Docker reference %s) would change the manifest, which is not possible (image is signed or the destination specifies a digest)",
-			transports.ImageName(ic.c.dest.Reference()), destRef.String())
+	if ic.cannotModifyManifestReason != "" {
+		return errors.Errorf("Copying a schema1 image with an embedded Docker reference to %s (Docker reference %s) would change the manifest, which we cannot do: %q",
+			transports.ImageName(ic.c.dest.Reference()), destRef.String(), ic.cannotModifyManifestReason)
 	}
 	ic.manifestUpdates.EmbeddedDockerReference = destRef
 	return nil
@@ -833,7 +880,7 @@ func isTTY(w io.Writer) bool {
 	return false
 }
 
-// copyLayers copies layers from ic.src/ic.c.rawSource to dest, using and updating ic.manifestUpdates if necessary and ic.canModifyManifest.
+// copyLayers copies layers from ic.src/ic.c.rawSource to dest, using and updating ic.manifestUpdates if necessary and ic.cannotModifyManifestReason == "".
 func (ic *imageCopier) copyLayers(ctx context.Context) error {
 	srcInfos := ic.src.LayerInfos()
 	numLayers := len(srcInfos)
@@ -844,8 +891,8 @@ func (ic *imageCopier) copyLayers(ctx context.Context) error {
 	srcInfosUpdated := false
 	// If we only need to check authorization, no updates required.
 	if updatedSrcInfos != nil && !reflect.DeepEqual(srcInfos, updatedSrcInfos) {
-		if !ic.canModifyManifest {
-			return errors.Errorf("Copying this image requires changing layer representation, which is not possible (image is signed or the destination specifies a digest)")
+		if ic.cannotModifyManifestReason != "" {
+			return errors.Errorf("Copying this image would require changing layer representation, which we cannot do: %q", ic.cannotModifyManifestReason)
 		}
 		srcInfos = updatedSrcInfos
 		srcInfosUpdated = true
@@ -975,8 +1022,8 @@ func layerDigestsDiffer(a, b []types.BlobInfo) bool {
 func (ic *imageCopier) copyUpdatedConfigAndManifest(ctx context.Context, instanceDigest *digest.Digest) ([]byte, digest.Digest, error) {
 	pendingImage := ic.src
 	if !ic.noPendingManifestUpdates() {
-		if !ic.canModifyManifest {
-			return nil, "", errors.Errorf("Internal error: copy needs an updated manifest but that was known to be forbidden")
+		if ic.cannotModifyManifestReason != "" {
+			return nil, "", errors.Errorf("Internal error: copy needs an updated manifest but that was known to be forbidden: %q", ic.cannotModifyManifestReason)
 		}
 		if !ic.diffIDsAreNeeded && ic.src.UpdatedImageNeedsLayerDiffIDs(*ic.manifestUpdates) {
 			// We have set ic.diffIDsAreNeeded based on the preferred MIME type returned by determineManifestConversion.
@@ -1359,7 +1406,7 @@ func (ic *imageCopier) copyLayerFromStream(ctx context.Context, srcStream io.Rea
 		}
 	}
 
-	blobInfo, err := ic.c.copyBlobFromStream(ctx, srcStream, srcInfo, getDiffIDRecorder, ic.canModifyManifest, false, toEncrypt, bar, layerIndex, emptyLayer) // Sets err to nil on success
+	blobInfo, err := ic.c.copyBlobFromStream(ctx, srcStream, srcInfo, getDiffIDRecorder, ic.cannotModifyManifestReason == "", false, toEncrypt, bar, layerIndex, emptyLayer) // Sets err to nil on success
 	return blobInfo, diffIDChan, err
 	// We need the defer … pipeWriter.CloseWithError() to happen HERE so that the caller can block on reading from diffIDChan
 }
diff --git a/vendor/github.com/containers/image/v5/copy/manifest.go b/vendor/github.com/containers/image/v5/copy/manifest.go
index b97edbf08..86ec8863a 100644
--- a/vendor/github.com/containers/image/v5/copy/manifest.go
+++ b/vendor/github.com/containers/image/v5/copy/manifest.go
@@ -79,10 +79,10 @@ func (ic *imageCopier) determineManifestConversion(ctx context.Context, destSupp
 	if _, ok := supportedByDest[srcType]; ok {
 		prioritizedTypes.append(srcType)
 	}
-	if !ic.canModifyManifest {
-		// We could also drop the !ic.canModifyManifest check and have the caller
+	if ic.cannotModifyManifestReason != "" {
+		// We could also drop this check and have the caller
 		// make the choice; it is already doing that to an extent, to improve error
-		// messages.  But it is nice to hide the “if !ic.canModifyManifest, do no conversion”
+		// messages.  But it is nice to hide the “if we can't modify, do no conversion”
 		// special case in here; the caller can then worry (or not) only about a good UI.
 		logrus.Debugf("We can't modify the manifest, hoping for the best...")
 		return srcType, []string{}, nil // Take our chances - FIXME? Or should we fail without trying?
diff --git a/vendor/github.com/containers/image/v5/pkg/shortnames/shortnames.go b/vendor/github.com/containers/image/v5/pkg/shortnames/shortnames.go
index fb0a15b99..46c10ff63 100644
--- a/vendor/github.com/containers/image/v5/pkg/shortnames/shortnames.go
+++ b/vendor/github.com/containers/image/v5/pkg/shortnames/shortnames.go
@@ -118,6 +118,7 @@ type Resolved struct {
 }
 
 func (r *Resolved) addCandidate(named reference.Named) {
+	named = reference.TagNameOnly(named) // Make sure to add ":latest" if needed
 	r.PullCandidates = append(r.PullCandidates, PullCandidate{named, false, r})
 }
 
@@ -138,6 +139,8 @@ const (
 	rationaleUSR
 	// Resolved value has been selected by the user (via the prompt).
 	rationaleUserSelection
+	// Resolved value has been enforced to use Docker Hub (via SystemContext).
+	rationaleEnforcedDockerHub
 )
 
 // Description returns a human-readable description about the resolution
@@ -152,6 +155,8 @@ func (r *Resolved) Description() string {
 		return fmt.Sprintf("Resolved %q as an alias (%s)", r.userInput, r.originDescription)
 	case rationaleUSR:
 		return fmt.Sprintf("Resolving %q using unqualified-search registries (%s)", r.userInput, r.originDescription)
+	case rationaleEnforcedDockerHub:
+		return fmt.Sprintf("Resolving %q to docker.io (%s)", r.userInput, r.originDescription)
 	case rationaleUserSelection, rationaleNone:
 		fallthrough
 	default:
@@ -265,8 +270,20 @@ func Resolve(ctx *types.SystemContext, name string) (*Resolved, error) {
 		return nil, err
 	}
 	if !isShort { // no short name
-		named := reference.TagNameOnly(shortRef) // Make sure to add ":latest" if needed
+		resolved.addCandidate(shortRef)
+		return resolved, nil
+	}
+
+	// Resolve to docker.io only if enforced by the caller (e.g., Podman's
+	// Docker-compatible REST API).
+	if ctx != nil && ctx.PodmanOnlyShortNamesIgnoreRegistriesConfAndForceDockerHub {
+		named, err := reference.ParseNormalizedNamed(name)
+		if err != nil {
+			return nil, errors.Wrapf(err, "cannot normalize input: %q", name)
+		}
 		resolved.addCandidate(named)
+		resolved.rationale = rationaleEnforcedDockerHub
+		resolved.originDescription = "enforced by caller"
 		return resolved, nil
 	}
 
@@ -295,9 +312,6 @@ func Resolve(ctx *types.SystemContext, name string) (*Resolved, error) {
 				return nil, err
 			}
 		}
-		// Make sure to add ":latest" if needed
-		namedAlias = reference.TagNameOnly(namedAlias)
-
 		resolved.addCandidate(namedAlias)
 		resolved.rationale = rationaleAlias
 		resolved.originDescription = aliasOriginDescription
@@ -325,9 +339,6 @@ func Resolve(ctx *types.SystemContext, name string) (*Resolved, error) {
 		if err != nil {
 			return nil, errors.Wrapf(err, "creating reference with unqualified-search registry %q", reg)
 		}
-		// Make sure to add ":latest" if needed
-		named = reference.TagNameOnly(named)
-
 		resolved.addCandidate(named)
 	}
 
@@ -412,6 +423,23 @@ func ResolveLocally(ctx *types.SystemContext, name string) ([]reference.Named, e
 
 	var candidates []reference.Named
 
+	// Complete the candidates with the specified registries.
+	completeCandidates := func(registries []string) ([]reference.Named, error) {
+		for _, reg := range registries {
+			named, err := reference.ParseNormalizedNamed(fmt.Sprintf("%s/%s", reg, name))
+			if err != nil {
+				return nil, errors.Wrapf(err, "creating reference with unqualified-search registry %q", reg)
+			}
+			named = reference.TagNameOnly(named) // Make sure to add ":latest" if needed
+			candidates = append(candidates, named)
+		}
+		return candidates, nil
+	}
+
+	if ctx != nil && ctx.PodmanOnlyShortNamesIgnoreRegistriesConfAndForceDockerHub {
+		return completeCandidates([]string{"docker.io"})
+	}
+
 	// Strip off the tag to normalize the short name for looking it up in
 	// the config files.
 	isTagged, isDigested, shortNameRepo, tag, digest := splitUserInput(shortRef)
@@ -434,9 +462,7 @@ func ResolveLocally(ctx *types.SystemContext, name string) ([]reference.Named, e
 				return nil, err
 			}
 		}
-		// Make sure to add ":latest" if needed
-		namedAlias = reference.TagNameOnly(namedAlias)
-
+		namedAlias = reference.TagNameOnly(namedAlias) // Make sure to add ":latest" if needed
 		candidates = append(candidates, namedAlias)
 	}
 
@@ -447,16 +473,5 @@ func ResolveLocally(ctx *types.SystemContext, name string) ([]reference.Named, e
 	}
 
 	// Note that "localhost" has precedence over the unqualified-search registries.
-	for _, reg := range append([]string{"localhost"}, unqualifiedSearchRegistries...) {
-		named, err := reference.ParseNormalizedNamed(fmt.Sprintf("%s/%s", reg, name))
-		if err != nil {
-			return nil, errors.Wrapf(err, "creating reference with unqualified-search registry %q", reg)
-		}
-		// Make sure to add ":latest" if needed
-		named = reference.TagNameOnly(named)
-
-		candidates = append(candidates, named)
-	}
-
-	return candidates, nil
+	return completeCandidates(append([]string{"localhost"}, unqualifiedSearchRegistries...))
 }
diff --git a/vendor/github.com/containers/image/v5/types/types.go b/vendor/github.com/containers/image/v5/types/types.go
index c98a6c6fd..dcff8caf7 100644
--- a/vendor/github.com/containers/image/v5/types/types.go
+++ b/vendor/github.com/containers/image/v5/types/types.go
@@ -561,6 +561,11 @@ type SystemContext struct {
 	UserShortNameAliasConfPath string
 	// If set, short-name resolution in pkg/shortnames must follow the specified mode
 	ShortNameMode *ShortNameMode
+	// If set, short names will resolve in pkg/shortnames to docker.io only, and unqualified-search registries and
+	// short-name aliases in registries.conf are ignored.  Note that this field is only intended to help enforce
+	// resolving to Docker Hub in the Docker-compatible REST API of Podman; it should never be used outside this
+	// specific context.
+	PodmanOnlyShortNamesIgnoreRegistriesConfAndForceDockerHub bool
 	// If not "", overrides the default path for the authentication file, but only new format files
 	AuthFilePath string
 	// if not "", overrides the default path for the authentication file, but with the legacy format;
diff --git a/vendor/github.com/containers/image/v5/version/version.go b/vendor/github.com/containers/image/v5/version/version.go
index ffb2a4ce2..17639f0d4 100644
--- a/vendor/github.com/containers/image/v5/version/version.go
+++ b/vendor/github.com/containers/image/v5/version/version.go
@@ -8,10 +8,10 @@ const (
 	// VersionMinor is for functionality in a backwards-compatible manner
 	VersionMinor = 17
 	// VersionPatch is for backwards-compatible bug fixes
-	VersionPatch = 0
+	VersionPatch = 1
 
 	// VersionDev indicates development branch. Releases will be empty string.
-	VersionDev = ""
+	VersionDev = "-dev"
 )
 
 // Version is the specification version that the package types support.