Add option to preserve digests on copy

When enabled, if digests can't be preserved an error will be raised.

Signed-off-by: James Hewitt <james.hewitt@uk.ibm.com>
This commit is contained in:
James Hewitt
2021-11-21 16:20:33 +00:00
parent 25868f17c0
commit 2046bfdaaa
15 changed files with 207 additions and 43 deletions

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

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

View File

@@ -61,16 +61,30 @@ func ClassIndex(class string) (int, error) {
return classIndex(class)
}
// SetFileLabel sets the SELinux label for this path or returns an error.
// SetFileLabel sets the SELinux label for this path, following symlinks,
// or returns an error.
func SetFileLabel(fpath string, label string) error {
return setFileLabel(fpath, label)
}
// FileLabel returns the SELinux label for this path or returns an error.
// LsetFileLabel sets the SELinux label for this path, not following symlinks,
// or returns an error.
func LsetFileLabel(fpath string, label string) error {
return lSetFileLabel(fpath, label)
}
// FileLabel returns the SELinux label for this path, following symlinks,
// or returns an error.
func FileLabel(fpath string) (string, error) {
return fileLabel(fpath)
}
// LfileLabel returns the SELinux label for this path, not following symlinks,
// or returns an error.
func LfileLabel(fpath string) (string, error) {
return lFileLabel(fpath)
}
// SetFSCreateLabel tells the kernel what label to use for all file system objects
// created by this task.
// Set the label to an empty string to return to the default label. Calls to SetFSCreateLabel

View File

@@ -316,8 +316,9 @@ func classIndex(class string) (int, error) {
return index, nil
}
// setFileLabel sets the SELinux label for this path or returns an error.
func setFileLabel(fpath string, label string) error {
// lSetFileLabel sets the SELinux label for this path, not following symlinks,
// or returns an error.
func lSetFileLabel(fpath string, label string) error {
if fpath == "" {
return ErrEmptyPath
}
@@ -334,12 +335,50 @@ func setFileLabel(fpath string, label string) error {
return nil
}
// fileLabel returns the SELinux label for this path or returns an error.
// setFileLabel sets the SELinux label for this path, following symlinks,
// or returns an error.
func setFileLabel(fpath string, label string) error {
if fpath == "" {
return ErrEmptyPath
}
for {
err := unix.Setxattr(fpath, xattrNameSelinux, []byte(label), 0)
if err == nil {
break
}
if err != unix.EINTR { //nolint:errorlint // unix errors are bare
return &os.PathError{Op: "setxattr", Path: fpath, Err: err}
}
}
return nil
}
// fileLabel returns the SELinux label for this path, following symlinks,
// or returns an error.
func fileLabel(fpath string) (string, error) {
if fpath == "" {
return "", ErrEmptyPath
}
label, err := getxattr(fpath, xattrNameSelinux)
if err != nil {
return "", &os.PathError{Op: "getxattr", Path: fpath, Err: err}
}
// Trim the NUL byte at the end of the byte buffer, if present.
if len(label) > 0 && label[len(label)-1] == '\x00' {
label = label[:len(label)-1]
}
return string(label), nil
}
// lFileLabel returns the SELinux label for this path, not following symlinks,
// or returns an error.
func lFileLabel(fpath string) (string, error) {
if fpath == "" {
return "", ErrEmptyPath
}
label, err := lgetxattr(fpath, xattrNameSelinux)
if err != nil {
return "", &os.PathError{Op: "lgetxattr", Path: fpath, Err: err}

View File

@@ -17,10 +17,18 @@ func setFileLabel(fpath string, label string) error {
return nil
}
func lSetFileLabel(fpath string, label string) error {
return nil
}
func fileLabel(fpath string) (string, error) {
return "", nil
}
func lFileLabel(fpath string) (string, error) {
return "", nil
}
func setFSCreateLabel(label string) error {
return nil
}

View File

@@ -36,3 +36,36 @@ func doLgetxattr(path, attr string, dest []byte) (int, error) {
}
}
}
// getxattr returns a []byte slice containing the value of
// an extended attribute attr set for path.
func getxattr(path, attr string) ([]byte, error) {
// Start with a 128 length byte array
dest := make([]byte, 128)
sz, errno := dogetxattr(path, attr, dest)
for errno == unix.ERANGE { //nolint:errorlint // unix errors are bare
// Buffer too small, use zero-sized buffer to get the actual size
sz, errno = dogetxattr(path, attr, []byte{})
if errno != nil {
return nil, errno
}
dest = make([]byte, sz)
sz, errno = dogetxattr(path, attr, dest)
}
if errno != nil {
return nil, errno
}
return dest[:sz], nil
}
// dogetxattr is a wrapper that retries on EINTR
func dogetxattr(path, attr string, dest []byte) (int, error) {
for {
sz, err := unix.Getxattr(path, attr, dest)
if err != unix.EINTR { //nolint:errorlint // unix errors are bare
return sz, err
}
}
}