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>
This commit is contained in:
Valentin Rothberg
2021-11-23 11:08:31 +01:00
parent 8de68b1707
commit 5bdd571b1e
21 changed files with 486 additions and 118 deletions

View File

@ -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

View File

@ -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 theres 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 lets 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
}

View File

@ -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?

View File

@ -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...))
}

View File

@ -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;

View File

@ -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.