diff --git a/copy/copy.go b/copy/copy.go index bb52ea7630..c7af5ed17a 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -129,9 +129,7 @@ func Image(policyContext *signature.PolicyContext, destRef, srcRef types.ImageRe } }() - destSupportedManifestMIMETypes := dest.SupportedManifestMIMETypes() - - rawSource, err := srcRef.NewImageSource(options.SourceCtx, destSupportedManifestMIMETypes) + rawSource, err := srcRef.NewImageSource(options.SourceCtx) if err != nil { return errors.Wrapf(err, "Error initializing source %s", transports.ImageName(srcRef)) } @@ -195,7 +193,7 @@ func Image(policyContext *signature.PolicyContext, destRef, srcRef types.ImageRe // We compute preferredManifestMIMEType only to show it in error messages. // Without having to add this context in an error message, we would be happy enough to know only that no conversion is needed. - preferredManifestMIMEType, otherManifestMIMETypeCandidates, err := determineManifestConversion(&manifestUpdates, src, destSupportedManifestMIMETypes, canModifyManifest) + preferredManifestMIMEType, otherManifestMIMETypeCandidates, err := determineManifestConversion(&manifestUpdates, src, dest.SupportedManifestMIMETypes(), canModifyManifest) if err != nil { return err } diff --git a/directory/directory_test.go b/directory/directory_test.go index 86ff004d0e..ceccd7aa58 100644 --- a/directory/directory_test.go +++ b/directory/directory_test.go @@ -38,7 +38,7 @@ func TestGetPutManifest(t *testing.T) { err = dest.Commit() assert.NoError(t, err) - src, err := ref.NewImageSource(nil, nil) + src, err := ref.NewImageSource(nil) require.NoError(t, err) defer src.Close() m, mt, err := src.GetManifest() @@ -64,7 +64,7 @@ func TestGetPutBlob(t *testing.T) { assert.Equal(t, int64(9), info.Size) assert.Equal(t, digest.FromBytes(blob), info.Digest) - src, err := ref.NewImageSource(nil, nil) + src, err := ref.NewImageSource(nil) require.NoError(t, err) defer src.Close() rc, size, err := src.GetBlob(info) @@ -143,7 +143,7 @@ func TestGetPutSignatures(t *testing.T) { err = dest.Commit() assert.NoError(t, err) - src, err := ref.NewImageSource(nil, nil) + src, err := ref.NewImageSource(nil) require.NoError(t, err) defer src.Close() sigs, err := src.GetSignatures(context.Background()) @@ -155,7 +155,7 @@ func TestSourceReference(t *testing.T) { ref, tmpDir := refToTempDir(t) defer os.RemoveAll(tmpDir) - src, err := ref.NewImageSource(nil, nil) + src, err := ref.NewImageSource(nil) require.NoError(t, err) defer src.Close() ref2 := src.Reference() diff --git a/directory/directory_transport.go b/directory/directory_transport.go index 34f7428935..b9ce01a2e1 100644 --- a/directory/directory_transport.go +++ b/directory/directory_transport.go @@ -143,11 +143,9 @@ func (ref dirReference) NewImage(ctx *types.SystemContext) (types.Image, error) return image.FromSource(src) } -// NewImageSource returns a types.ImageSource for this reference, -// asking the backend to use a manifest from requestedManifestMIMETypes if possible. -// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes. +// NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. -func (ref dirReference) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) { +func (ref dirReference) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { return newImageSource(ref), nil } diff --git a/directory/directory_transport_test.go b/directory/directory_transport_test.go index 1384d6f5b1..508eebd415 100644 --- a/directory/directory_transport_test.go +++ b/directory/directory_transport_test.go @@ -184,7 +184,7 @@ func TestReferenceNewImageNoValidManifest(t *testing.T) { func TestReferenceNewImageSource(t *testing.T) { ref, tmpDir := refToTempDir(t) defer os.RemoveAll(tmpDir) - src, err := ref.NewImageSource(nil, nil) + src, err := ref.NewImageSource(nil) assert.NoError(t, err) defer src.Close() } diff --git a/docker/archive/transport.go b/docker/archive/transport.go index 59c68c3beb..f38d4aced5 100644 --- a/docker/archive/transport.go +++ b/docker/archive/transport.go @@ -134,11 +134,9 @@ func (ref archiveReference) NewImage(ctx *types.SystemContext) (types.Image, err return ctrImage.FromSource(src) } -// NewImageSource returns a types.ImageSource for this reference, -// asking the backend to use a manifest from requestedManifestMIMETypes if possible. -// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes. +// NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. -func (ref archiveReference) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) { +func (ref archiveReference) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { return newImageSource(ctx, ref), nil } diff --git a/docker/archive/transport_test.go b/docker/archive/transport_test.go index 689a512cff..9de10c1791 100644 --- a/docker/archive/transport_test.go +++ b/docker/archive/transport_test.go @@ -154,7 +154,7 @@ func TestReferenceNewImageSource(t *testing.T) { for _, suffix := range []string{"", ":thisisignoredbutaccepted"} { ref, err := ParseReference(tarFixture + suffix) require.NoError(t, err, suffix) - src, err := ref.NewImageSource(nil, nil) + src, err := ref.NewImageSource(nil) assert.NoError(t, err, suffix) defer src.Close() } diff --git a/docker/daemon/daemon_transport.go b/docker/daemon/daemon_transport.go index 41ccd1f197..41be1b2db8 100644 --- a/docker/daemon/daemon_transport.go +++ b/docker/daemon/daemon_transport.go @@ -161,11 +161,9 @@ func (ref daemonReference) NewImage(ctx *types.SystemContext) (types.Image, erro return image.FromSource(src) } -// NewImageSource returns a types.ImageSource for this reference, -// asking the backend to use a manifest from requestedManifestMIMETypes if possible. -// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes. +// NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. -func (ref daemonReference) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) { +func (ref daemonReference) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { return newImageSource(ctx, ref) } diff --git a/docker/docker_image.go b/docker/docker_image.go index 992d920354..8be35b735d 100644 --- a/docker/docker_image.go +++ b/docker/docker_image.go @@ -23,7 +23,7 @@ type Image struct { // a client to the registry hosting the given image. // The caller must call .Close() on the returned Image. func newImage(ctx *types.SystemContext, ref dockerReference) (types.Image, error) { - s, err := newImageSource(ctx, ref, nil) + s, err := newImageSource(ctx, ref) if err != nil { return nil, err } diff --git a/docker/docker_image_dest.go b/docker/docker_image_dest.go index 68404bda54..80fcb365ee 100644 --- a/docker/docker_image_dest.go +++ b/docker/docker_image_dest.go @@ -24,21 +24,6 @@ import ( "github.com/sirupsen/logrus" ) -var manifestMIMETypes = []string{ - // TODO(runcom): we'll add OCI as part of another PR here - manifest.DockerV2Schema2MediaType, - manifest.DockerV2Schema1SignedMediaType, - manifest.DockerV2Schema1MediaType, -} - -func supportedManifestMIMETypesMap() map[string]bool { - m := make(map[string]bool, len(manifestMIMETypes)) - for _, mt := range manifestMIMETypes { - m[mt] = true - } - return m -} - type dockerImageDestination struct { ref dockerReference c *dockerClient @@ -70,7 +55,12 @@ func (d *dockerImageDestination) Close() error { } func (d *dockerImageDestination) SupportedManifestMIMETypes() []string { - return manifestMIMETypes + return []string{ + // TODO(runcom): we'll add OCI as part of another PR here + manifest.DockerV2Schema2MediaType, + manifest.DockerV2Schema1SignedMediaType, + manifest.DockerV2Schema1MediaType, + } } // SupportsSignatures returns an error (to be displayed to the user) if the destination certainly can't store signatures. diff --git a/docker/docker_image_src.go b/docker/docker_image_src.go index 88607210e7..5422207840 100644 --- a/docker/docker_image_src.go +++ b/docker/docker_image_src.go @@ -22,39 +22,21 @@ import ( type dockerImageSource struct { ref dockerReference - requestedManifestMIMETypes []string c *dockerClient // State cachedManifest []byte // nil if not loaded yet cachedManifestMIMEType string // Only valid if cachedManifest != nil } -// newImageSource creates a new ImageSource for the specified image reference, -// asking the backend to use a manifest from requestedManifestMIMETypes if possible. -// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes. +// newImageSource creates a new ImageSource for the specified image reference. // The caller must call .Close() on the returned ImageSource. -func newImageSource(ctx *types.SystemContext, ref dockerReference, requestedManifestMIMETypes []string) (*dockerImageSource, error) { +func newImageSource(ctx *types.SystemContext, ref dockerReference) (*dockerImageSource, error) { c, err := newDockerClient(ctx, ref, false, "pull") if err != nil { return nil, err } - if requestedManifestMIMETypes == nil { - requestedManifestMIMETypes = manifest.DefaultRequestedManifestMIMETypes - } - supportedMIMEs := supportedManifestMIMETypesMap() - acceptableRequestedMIMEs := false - for _, mtrequested := range requestedManifestMIMETypes { - if supportedMIMEs[mtrequested] { - acceptableRequestedMIMEs = true - break - } - } - if !acceptableRequestedMIMEs { - requestedManifestMIMETypes = manifest.DefaultRequestedManifestMIMETypes - } return &dockerImageSource{ ref: ref, - requestedManifestMIMETypes: requestedManifestMIMETypes, c: c, }, nil } @@ -96,7 +78,7 @@ func (s *dockerImageSource) GetManifest() ([]byte, string, error) { func (s *dockerImageSource) fetchManifest(ctx context.Context, tagOrDigest string) ([]byte, string, error) { path := fmt.Sprintf(manifestPath, reference.Path(s.ref.ref), tagOrDigest) headers := make(map[string][]string) - headers["Accept"] = s.requestedManifestMIMETypes + headers["Accept"] = manifest.DefaultRequestedManifestMIMETypes res, err := s.c.makeRequest(ctx, "GET", path, headers, nil) if err != nil { return nil, "", err diff --git a/docker/docker_transport.go b/docker/docker_transport.go index 15d68e993c..1d67cc4fc1 100644 --- a/docker/docker_transport.go +++ b/docker/docker_transport.go @@ -130,12 +130,10 @@ func (ref dockerReference) NewImage(ctx *types.SystemContext) (types.Image, erro return newImage(ctx, ref) } -// NewImageSource returns a types.ImageSource for this reference, -// asking the backend to use a manifest from requestedManifestMIMETypes if possible. -// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes. +// NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. -func (ref dockerReference) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) { - return newImageSource(ctx, ref, requestedManifestMIMETypes) +func (ref dockerReference) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { + return newImageSource(ctx, ref) } // NewImageDestination returns a types.ImageDestination for this reference. diff --git a/docker/docker_transport_test.go b/docker/docker_transport_test.go index a542b3c77f..d753514abb 100644 --- a/docker/docker_transport_test.go +++ b/docker/docker_transport_test.go @@ -160,7 +160,7 @@ func TestReferenceNewImage(t *testing.T) { func TestReferenceNewImageSource(t *testing.T) { ref, err := ParseReference("//busybox") require.NoError(t, err) - src, err := ref.NewImageSource(&types.SystemContext{RegistriesDirPath: "/this/doesnt/exist", DockerPerHostCertDirPath: "/this/doesnt/exist"}, nil) + src, err := ref.NewImageSource(&types.SystemContext{RegistriesDirPath: "/this/doesnt/exist", DockerPerHostCertDirPath: "/this/doesnt/exist"}) assert.NoError(t, err) defer src.Close() } diff --git a/image/docker_schema2_test.go b/image/docker_schema2_test.go index d80a018844..31fce3b677 100644 --- a/image/docker_schema2_test.go +++ b/image/docker_schema2_test.go @@ -326,7 +326,7 @@ func (ref refImageReferenceMock) PolicyConfigurationNamespaces() []string { func (ref refImageReferenceMock) NewImage(ctx *types.SystemContext) (types.Image, error) { panic("unexpected call to a mock function") } -func (ref refImageReferenceMock) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) { +func (ref refImageReferenceMock) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { panic("unexpected call to a mock function") } func (ref refImageReferenceMock) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) { diff --git a/oci/layout/oci_transport.go b/oci/layout/oci_transport.go index 1406dd18ca..7fd826f476 100644 --- a/oci/layout/oci_transport.go +++ b/oci/layout/oci_transport.go @@ -241,11 +241,9 @@ func LoadManifestDescriptor(imgRef types.ImageReference) (imgspecv1.Descriptor, return ociRef.getManifestDescriptor() } -// NewImageSource returns a types.ImageSource for this reference, -// asking the backend to use a manifest from requestedManifestMIMETypes if possible. -// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes. +// NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. -func (ref ociReference) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) { +func (ref ociReference) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { return newImageSource(ref) } diff --git a/oci/layout/oci_transport.go.rej b/oci/layout/oci_transport.go.rej new file mode 100644 index 0000000000..5c1bf24c9e --- /dev/null +++ b/oci/layout/oci_transport.go.rej @@ -0,0 +1,15 @@ +diff a/oci/layout/oci_transport.go b/oci/layout/oci_transport.go (rejected hunks) +@@ -216,11 +216,9 @@ func (ref ociReference) getManifestDescriptor() (imgspecv1.Descriptor, error) { + return *d, nil + } + +-// NewImageSource returns a types.ImageSource for this reference, +-// asking the backend to use a manifest from requestedManifestMIMETypes if possible. +-// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes. ++// NewImageSource returns a types.ImageSource for this reference. + // The caller must call .Close() on the returned ImageSource. +-func (ref ociReference) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) { ++func (ref ociReference) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { + return newImageSource(ref) + } + diff --git a/oci/layout/oci_transport_test.go b/oci/layout/oci_transport_test.go index 10b091086d..6147653587 100644 --- a/oci/layout/oci_transport_test.go +++ b/oci/layout/oci_transport_test.go @@ -240,7 +240,7 @@ func TestReferenceNewImage(t *testing.T) { func TestReferenceNewImageSource(t *testing.T) { ref, tmpDir := refToTempOCI(t) defer os.RemoveAll(tmpDir) - _, err := ref.NewImageSource(nil, nil) + _, err := ref.NewImageSource(nil) assert.NoError(t, err) } diff --git a/openshift/openshift.go b/openshift/openshift.go index 19ad25370f..e8ceb27909 100644 --- a/openshift/openshift.go +++ b/openshift/openshift.go @@ -163,17 +163,14 @@ type openshiftImageSource struct { client *openshiftClient // Values specific to this image ctx *types.SystemContext - requestedManifestMIMETypes []string // State docker types.ImageSource // The Docker Registry endpoint, or nil if not resolved yet imageStreamImageName string // Resolved image identifier, or "" if not known yet } -// newImageSource creates a new ImageSource for the specified reference, -// asking the backend to use a manifest from requestedManifestMIMETypes if possible. -// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes. +// newImageSource creates a new ImageSource for the specified reference. // The caller must call .Close() on the returned ImageSource. -func newImageSource(ctx *types.SystemContext, ref openshiftReference, requestedManifestMIMETypes []string) (types.ImageSource, error) { +func newImageSource(ctx *types.SystemContext, ref openshiftReference) (types.ImageSource, error) { client, err := newOpenshiftClient(ref) if err != nil { return nil, err @@ -182,7 +179,6 @@ func newImageSource(ctx *types.SystemContext, ref openshiftReference, requestedM return &openshiftImageSource{ client: client, ctx: ctx, - requestedManifestMIMETypes: requestedManifestMIMETypes, }, nil } @@ -286,7 +282,7 @@ func (s *openshiftImageSource) ensureImageIsResolved(ctx context.Context) error if err != nil { return err } - d, err := dockerRef.NewImageSource(s.ctx, s.requestedManifestMIMETypes) + d, err := dockerRef.NewImageSource(s.ctx) if err != nil { return err } diff --git a/openshift/openshift_transport.go b/openshift/openshift_transport.go index 108e110232..7db35d96e5 100644 --- a/openshift/openshift_transport.go +++ b/openshift/openshift_transport.go @@ -130,19 +130,17 @@ func (ref openshiftReference) PolicyConfigurationNamespaces() []string { // NOTE: If any kind of signature verification should happen, build an UnparsedImage from the value returned by NewImageSource, // verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage. func (ref openshiftReference) NewImage(ctx *types.SystemContext) (types.Image, error) { - src, err := newImageSource(ctx, ref, nil) + src, err := newImageSource(ctx, ref) if err != nil { return nil, err } return genericImage.FromSource(src) } -// NewImageSource returns a types.ImageSource for this reference, -// asking the backend to use a manifest from requestedManifestMIMETypes if possible. -// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes. +// NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. -func (ref openshiftReference) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) { - return newImageSource(ctx, ref, requestedManifestMIMETypes) +func (ref openshiftReference) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { + return newImageSource(ctx, ref) } // NewImageDestination returns a types.ImageDestination for this reference. diff --git a/ostree/ostree_transport.go b/ostree/ostree_transport.go index f165b13f02..b89dcf0bb7 100644 --- a/ostree/ostree_transport.go +++ b/ostree/ostree_transport.go @@ -183,11 +183,9 @@ func (ref ostreeReference) NewImage(ctx *types.SystemContext) (types.Image, erro return nil, errors.New("Reading ostree: images is currently not supported") } -// NewImageSource returns a types.ImageSource for this reference, -// asking the backend to use a manifest from requestedManifestMIMETypes if possible. -// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes. +// NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. -func (ref ostreeReference) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) { +func (ref ostreeReference) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { return nil, errors.New("Reading ostree: images is currently not supported") } diff --git a/ostree/ostree_transport_test.go b/ostree/ostree_transport_test.go index 2fae742a41..8072d07e04 100644 --- a/ostree/ostree_transport_test.go +++ b/ostree/ostree_transport_test.go @@ -247,7 +247,7 @@ func TestReferenceNewImage(t *testing.T) { func TestReferenceNewImageSource(t *testing.T) { ref, err := Transport.ParseReference("busybox") require.NoError(t, err) - _, err = ref.NewImageSource(nil, nil) + _, err = ref.NewImageSource(nil) assert.Error(t, err) } diff --git a/signature/#policy_reference_match_test.go# b/signature/#policy_reference_match_test.go# new file mode 100644 index 0000000000..3ff38492b4 --- /dev/null +++ b/signature/#policy_reference_match_test.go# @@ -0,0 +1,366 @@ +package signature + +import ( + "context" + "fmt" + "testing" + + "github.com/containers/image/docker/reference" + "github.com/containers/image/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + fullRHELRef = "registry.access.redhat.com/rhel7/rhel:7.2.3" + untaggedRHELRef = "registry.access.redhat.com/rhel7/rhel" + digestSuffix = "@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + digestSuffixOther = "@sha256:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +) + +func TestParseImageAndDockerReference(t *testing.T) { + const ( + ok1 = "busybox" + ok2 = fullRHELRef + bad1 = "UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES" + bad2 = "" + ) + // Success + ref, err := reference.ParseNormalizedNamed(ok1) + require.NoError(t, err) + r1, r2, err := parseImageAndDockerReference(refImageMock{ref}, ok2) + require.NoError(t, err) + assert.Equal(t, ok1, reference.FamiliarString(r1)) + assert.Equal(t, ok2, reference.FamiliarString(r2)) + + // Unidentified images are rejected. + _, _, err = parseImageAndDockerReference(refImageMock{nil}, ok2) + require.Error(t, err) + assert.IsType(t, PolicyRequirementError(""), err) + + // Failures + for _, refs := range [][]string{ + {bad1, ok2}, + {ok1, bad2}, + {bad1, bad2}, + } { + ref, err := reference.ParseNormalizedNamed(refs[0]) + if err == nil { + _, _, err := parseImageAndDockerReference(refImageMock{ref}, refs[1]) + assert.Error(t, err) + } + } +} + +// refImageMock is a mock of types.UnparsedImage which returns itself in Reference().DockerReference. +type refImageMock struct{ reference.Named } + +func (ref refImageMock) Reference() types.ImageReference { + return refImageReferenceMock{ref.Named} +} +func (ref refImageMock) Close() error { + panic("unexpected call to a mock function") +} +func (ref refImageMock) Manifest() ([]byte, string, error) { + panic("unexpected call to a mock function") +} +func (ref refImageMock) Signatures(context.Context) ([][]byte, error) { + panic("unexpected call to a mock function") +} + +// refImageReferenceMock is a mock of types.ImageReference which returns itself in DockerReference. +type refImageReferenceMock struct{ reference.Named } + +func (ref refImageReferenceMock) Transport() types.ImageTransport { + // We use this in error messages, so sady we must return something. But right now we do so only when DockerReference is nil, so restrict to that. + if ref.Named == nil { + return nameImageTransportMock("== Transport mock") + } + panic("unexpected call to a mock function") +} +func (ref refImageReferenceMock) StringWithinTransport() string { + // We use this in error messages, so sadly we must return something. But right now we do so only when DockerReference is nil, so restrict to that. + if ref.Named == nil { + return "== StringWithinTransport for an image with no Docker support" + } + panic("unexpected call to a mock function") +} +func (ref refImageReferenceMock) DockerReference() reference.Named { + return ref.Named +} +func (ref refImageReferenceMock) PolicyConfigurationIdentity() string { + panic("unexpected call to a mock function") +} +func (ref refImageReferenceMock) PolicyConfigurationNamespaces() []string { + panic("unexpected call to a mock function") +} +func (ref refImageReferenceMock) NewImage(ctx *types.SystemContext) (types.Image, error) { + panic("unexpected call to a mock function") +} +func (ref refImageReferenceMock) NewImageSource(ctx *types.SystemContextx) (types.ImageSource, error) { + panic("unexpected call to a mock function") +} +func (ref refImageReferenceMock) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) { + panic("unexpected call to a mock function") +} +func (ref refImageReferenceMock) DeleteImage(ctx *types.SystemContext) error { + panic("unexpected call to a mock function") +} + +// nameImageTransportMock is a mock of types.ImageTransport which returns itself in Name. +type nameImageTransportMock string + +func (name nameImageTransportMock) Name() string { + return string(name) +} +func (name nameImageTransportMock) ParseReference(reference string) (types.ImageReference, error) { + panic("unexpected call to a mock function") +} +func (name nameImageTransportMock) ValidatePolicyConfigurationScope(scope string) error { + panic("unexpected call to a mock function") +} + +type prmSymmetricTableTest struct { + refA, refB string + result bool +} + +// Test cases for exact reference match. The behavior is supposed to be symmetric. +var prmExactMatchTestTable = []prmSymmetricTableTest{ + // Success, simple matches + {"busybox:latest", "busybox:latest", true}, + {fullRHELRef, fullRHELRef, true}, + {"busybox" + digestSuffix, "busybox" + digestSuffix, true}, // NOTE: This is not documented; signing digests is not recommended at this time. + // Non-canonical reference format is canonicalized + {"library/busybox:latest", "busybox:latest", true}, + {"docker.io/library/busybox:latest", "busybox:latest", true}, + {"library/busybox" + digestSuffix, "busybox" + digestSuffix, true}, + // Mismatch + {"busybox:latest", "busybox:notlatest", false}, + {"busybox:latest", "notbusybox:latest", false}, + {"busybox:latest", "hostname/library/busybox:notlatest", false}, + {"hostname/library/busybox:latest", "busybox:notlatest", false}, + {"busybox:latest", fullRHELRef, false}, + {"busybox" + digestSuffix, "notbusybox" + digestSuffix, false}, + {"busybox:latest", "busybox" + digestSuffix, false}, + {"busybox" + digestSuffix, "busybox" + digestSuffixOther, false}, + // NameOnly references + {"busybox", "busybox:latest", false}, + {"busybox", "busybox" + digestSuffix, false}, + {"busybox", "busybox", false}, + // References with both tags and digests: We match them exactly (requiring BOTH to match) + // NOTE: Again, this is not documented behavior; the recommendation is to sign tags, not digests, and then tag-and-digest references won’t match the signed identity. + {"busybox:latest" + digestSuffix, "busybox:latest" + digestSuffix, true}, + {"busybox:latest" + digestSuffix, "busybox:latest" + digestSuffixOther, false}, + {"busybox:latest" + digestSuffix, "busybox:notlatest" + digestSuffix, false}, + {"busybox:latest" + digestSuffix, "busybox" + digestSuffix, false}, + {"busybox:latest" + digestSuffix, "busybox:latest", false}, + // Invalid format + {"UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES", "busybox:latest", false}, + {"", "UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES", false}, + // Even if they are exactly equal, invalid values are rejected. + {"INVALID", "INVALID", false}, +} + +// Test cases for repository-only reference match. The behavior is supposed to be symmetric. +var prmRepositoryMatchTestTable = []prmSymmetricTableTest{ + // Success, simple matches + {"busybox:latest", "busybox:latest", true}, + {fullRHELRef, fullRHELRef, true}, + {"busybox" + digestSuffix, "busybox" + digestSuffix, true}, // NOTE: This is not documented; signing digests is not recommended at this time. + // Non-canonical reference format is canonicalized + {"library/busybox:latest", "busybox:latest", true}, + {"docker.io/library/busybox:latest", "busybox:latest", true}, + {"library/busybox" + digestSuffix, "busybox" + digestSuffix, true}, + // The same as above, but with mismatching tags + {"busybox:latest", "busybox:notlatest", true}, + {fullRHELRef + "tagsuffix", fullRHELRef, true}, + {"library/busybox:latest", "busybox:notlatest", true}, + {"busybox:latest", "library/busybox:notlatest", true}, + {"docker.io/library/busybox:notlatest", "busybox:latest", true}, + {"busybox:notlatest", "docker.io/library/busybox:latest", true}, + {"busybox:latest", "busybox" + digestSuffix, true}, + {"busybox" + digestSuffix, "busybox" + digestSuffixOther, true}, // Even this is accepted here. (This could more reasonably happen with two different digest algorithms.) + // The same as above, but with defaulted tags (should not actually happen) + {"busybox", "busybox:notlatest", true}, + {fullRHELRef, untaggedRHELRef, true}, + {"busybox", "busybox" + digestSuffix, true}, + {"library/busybox", "busybox", true}, + {"docker.io/library/busybox", "busybox", true}, + // Mismatch + {"busybox:latest", "notbusybox:latest", false}, + {"hostname/library/busybox:latest", "busybox:notlatest", false}, + {"busybox:latest", fullRHELRef, false}, + {"busybox" + digestSuffix, "notbusybox" + digestSuffix, false}, + // References with both tags and digests: We ignore both anyway. + {"busybox:latest" + digestSuffix, "busybox:latest" + digestSuffix, true}, + {"busybox:latest" + digestSuffix, "busybox:latest" + digestSuffixOther, true}, + {"busybox:latest" + digestSuffix, "busybox:notlatest" + digestSuffix, true}, + {"busybox:latest" + digestSuffix, "busybox" + digestSuffix, true}, + {"busybox:latest" + digestSuffix, "busybox:latest", true}, + // Invalid format + {"UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES", "busybox:latest", false}, + {"", "UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES", false}, + // Even if they are exactly equal, invalid values are rejected. + {"INVALID", "INVALID", false}, +} + +func testImageAndSig(t *testing.T, prm PolicyReferenceMatch, imageRef, sigRef string, result bool) { + // This assumes that all ways to obtain a reference.Named perform equivalent validation, + // and therefore values refused by reference.ParseNormalizedNamed can not happen in practice. + parsedImageRef, err := reference.ParseNormalizedNamed(imageRef) + if err != nil { + return + } + res := prm.matchesDockerReference(refImageMock{parsedImageRef}, sigRef) + assert.Equal(t, result, res, fmt.Sprintf("%s vs. %s", imageRef, sigRef)) +} + +func TestPRMMatchExactMatchesDockerReference(t *testing.T) { + prm := NewPRMMatchExact() + for _, test := range prmExactMatchTestTable { + testImageAndSig(t, prm, test.refA, test.refB, test.result) + testImageAndSig(t, prm, test.refB, test.refA, test.result) + } + // Even if they are signed with an empty string as a reference, unidentified images are rejected. + res := prm.matchesDockerReference(refImageMock{nil}, "") + assert.False(t, res, `unidentified vs. ""`) +} + +func TestPMMMatchRepoDigestOrExactMatchesDockerReference(t *testing.T) { + prm := NewPRMMatchRepoDigestOrExact() + + // prmMatchRepoDigestOrExact is a middle ground between prmMatchExact and prmMatchRepository: + // It accepts anything prmMatchExact accepts,… + for _, test := range prmExactMatchTestTable { + if test.result == true { + testImageAndSig(t, prm, test.refA, test.refB, test.result) + testImageAndSig(t, prm, test.refB, test.refA, test.result) + } + } + // … and it rejects everything prmMatchRepository rejects. + for _, test := range prmRepositoryMatchTestTable { + if test.result == false { + testImageAndSig(t, prm, test.refA, test.refB, test.result) + testImageAndSig(t, prm, test.refB, test.refA, test.result) + } + } + + // The other cases, possibly assymetrical: + for _, test := range []struct { + imageRef, sigRef string + result bool + }{ + // Tag mismatch + {"busybox:latest", "busybox:notlatest", false}, + {fullRHELRef + "tagsuffix", fullRHELRef, false}, + {"library/busybox:latest", "busybox:notlatest", false}, + {"busybox:latest", "library/busybox:notlatest", false}, + {"docker.io/library/busybox:notlatest", "busybox:latest", false}, + {"busybox:notlatest", "docker.io/library/busybox:latest", false}, + // NameOnly references + {"busybox", "busybox:latest", false}, + {"busybox:latest", "busybox", false}, + {"busybox", "busybox" + digestSuffix, false}, + {"busybox" + digestSuffix, "busybox", false}, + {fullRHELRef, untaggedRHELRef, false}, + {"busybox", "busybox", false}, + // Tag references only accept signatures with matching tags. + {"busybox:latest", "busybox" + digestSuffix, false}, + // Digest references accept any signature with matching repository. + {"busybox" + digestSuffix, "busybox:latest", true}, + {"busybox" + digestSuffix, "busybox" + digestSuffixOther, true}, // Even this is accepted here. (This could more reasonably happen with two different digest algorithms.) + // References with both tags and digests: We match them exactly (requiring BOTH to match). + {"busybox:latest" + digestSuffix, "busybox:latest", false}, + {"busybox:latest" + digestSuffix, "busybox:notlatest", false}, + {"busybox:latest", "busybox:latest" + digestSuffix, false}, + {"busybox:latest" + digestSuffix, "busybox:latest" + digestSuffixOther, false}, + {"busybox:latest" + digestSuffix, "busybox:notlatest" + digestSuffixOther, false}, + } { + testImageAndSig(t, prm, test.imageRef, test.sigRef, test.result) + } +} + +func TestPRMMatchRepositoryMatchesDockerReference(t *testing.T) { + prm := NewPRMMatchRepository() + for _, test := range prmRepositoryMatchTestTable { + testImageAndSig(t, prm, test.refA, test.refB, test.result) + testImageAndSig(t, prm, test.refB, test.refA, test.result) + } + // Even if they are signed with an empty string as a reference, unidentified images are rejected. + res := prm.matchesDockerReference(refImageMock{nil}, "") + assert.False(t, res, `unidentified vs. ""`) +} + +func TestParseDockerReferences(t *testing.T) { + const ( + ok1 = "busybox" + ok2 = fullRHELRef + bad1 = "UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES" + bad2 = "" + ) + + // Success + r1, r2, err := parseDockerReferences(ok1, ok2) + require.NoError(t, err) + assert.Equal(t, ok1, reference.FamiliarString(r1)) + assert.Equal(t, ok2, reference.FamiliarString(r2)) + + // Failures + for _, refs := range [][]string{ + {bad1, ok2}, + {ok1, bad2}, + {bad1, bad2}, + } { + _, _, err := parseDockerReferences(refs[0], refs[1]) + assert.Error(t, err) + } +} + +// forbiddenImageMock is a mock of types.UnparsedImage which ensures Reference is not called +type forbiddenImageMock struct{} + +func (ref forbiddenImageMock) Reference() types.ImageReference { + panic("unexpected call to a mock function") +} +func (ref forbiddenImageMock) Close() error { + panic("unexpected call to a mock function") +} +func (ref forbiddenImageMock) Manifest() ([]byte, string, error) { + panic("unexpected call to a mock function") +} +func (ref forbiddenImageMock) Signatures(context.Context) ([][]byte, error) { + panic("unexpected call to a mock function") +} + +func testExactPRMAndSig(t *testing.T, prmFactory func(string) PolicyReferenceMatch, imageRef, sigRef string, result bool) { + prm := prmFactory(imageRef) + res := prm.matchesDockerReference(forbiddenImageMock{}, sigRef) + assert.Equal(t, result, res, fmt.Sprintf("%s vs. %s", imageRef, sigRef)) +} + +func prmExactReferenceFactory(ref string) PolicyReferenceMatch { + // Do not use NewPRMExactReference, we want to also test the case with an invalid DockerReference, + // even though NewPRMExactReference should never let it happen. + return &prmExactReference{DockerReference: ref} +} + +func TestPRMExactReferenceMatchesDockerReference(t *testing.T) { + for _, test := range prmExactMatchTestTable { + testExactPRMAndSig(t, prmExactReferenceFactory, test.refA, test.refB, test.result) + testExactPRMAndSig(t, prmExactReferenceFactory, test.refB, test.refA, test.result) + } +} + +func prmExactRepositoryFactory(ref string) PolicyReferenceMatch { + // Do not use NewPRMExactRepository, we want to also test the case with an invalid DockerReference, + // even though NewPRMExactRepository should never let it happen. + return &prmExactRepository{DockerRepository: ref} +} + +func TestPRMExactRepositoryMatchesDockerReference(t *testing.T) { + for _, test := range prmRepositoryMatchTestTable { + testExactPRMAndSig(t, prmExactRepositoryFactory, test.refA, test.refB, test.result) + testExactPRMAndSig(t, prmExactRepositoryFactory, test.refB, test.refA, test.result) + } +} diff --git a/signature/policy_eval_signedby_test.go b/signature/policy_eval_signedby_test.go index 19086fcf5b..28b02ea046 100644 --- a/signature/policy_eval_signedby_test.go +++ b/signature/policy_eval_signedby_test.go @@ -27,7 +27,7 @@ func dirImageMock(t *testing.T, dir, dockerReference string) types.UnparsedImage func dirImageMockWithRef(t *testing.T, dir string, ref types.ImageReference) types.UnparsedImage { srcRef, err := directory.NewReference(dir) require.NoError(t, err) - src, err := srcRef.NewImageSource(nil, nil) + src, err := srcRef.NewImageSource(nil) require.NoError(t, err) return image.UnparsedFromSource(&dirImageSourceMock{ ImageSource: src, diff --git a/signature/policy_eval_simple_test.go b/signature/policy_eval_simple_test.go index aae4b6a8b8..fb32c428a5 100644 --- a/signature/policy_eval_simple_test.go +++ b/signature/policy_eval_simple_test.go @@ -37,7 +37,7 @@ func (ref nameOnlyImageReferenceMock) PolicyConfigurationNamespaces() []string { func (ref nameOnlyImageReferenceMock) NewImage(ctx *types.SystemContext) (types.Image, error) { panic("unexpected call to a mock function") } -func (ref nameOnlyImageReferenceMock) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) { +func (ref nameOnlyImageReferenceMock) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { panic("unexpected call to a mock function") } func (ref nameOnlyImageReferenceMock) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) { diff --git a/signature/policy_eval_test.go b/signature/policy_eval_test.go index 7cfcb3fbf6..7ca155b659 100644 --- a/signature/policy_eval_test.go +++ b/signature/policy_eval_test.go @@ -95,7 +95,7 @@ func (ref pcImageReferenceMock) PolicyConfigurationNamespaces() []string { func (ref pcImageReferenceMock) NewImage(ctx *types.SystemContext) (types.Image, error) { panic("unexpected call to a mock function") } -func (ref pcImageReferenceMock) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) { +func (ref pcImageReferenceMock) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { panic("unexpected call to a mock function") } func (ref pcImageReferenceMock) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) { diff --git a/signature/policy_reference_match_test.go b/signature/policy_reference_match_test.go index 2ddd173081..daf29a3184 100644 --- a/signature/policy_reference_match_test.go +++ b/signature/policy_reference_match_test.go @@ -97,7 +97,7 @@ func (ref refImageReferenceMock) PolicyConfigurationNamespaces() []string { func (ref refImageReferenceMock) NewImage(ctx *types.SystemContext) (types.Image, error) { panic("unexpected call to a mock function") } -func (ref refImageReferenceMock) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) { +func (ref refImageReferenceMock) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { panic("unexpected call to a mock function") } func (ref refImageReferenceMock) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) { diff --git a/storage/storage_reference.go b/storage/storage_reference.go index 674330b483..ded5870520 100644 --- a/storage/storage_reference.go +++ b/storage/storage_reference.go @@ -154,7 +154,7 @@ func (s storageReference) DeleteImage(ctx *types.SystemContext) error { return err } -func (s storageReference) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) { +func (s storageReference) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { return newImageSource(s) } diff --git a/storage/storage_test.go b/storage/storage_test.go index 3f84ba086a..91e8ba0cbc 100644 --- a/storage/storage_test.go +++ b/storage/storage_test.go @@ -429,7 +429,7 @@ func TestWriteRead(t *testing.T) { t.Fatalf("Image %q claims to have been created at time 0", ref.StringWithinTransport()) } - src, err := ref.NewImageSource(systemContext(), []string{}) + src, err := ref.NewImageSource(systemContext()) if err != nil { t.Fatalf("NewImageSource(%q) returned error %v", ref.StringWithinTransport(), err) } @@ -900,7 +900,7 @@ func TestDuplicateBlob(t *testing.T) { if err != nil { t.Fatalf("NewImage(%q) returned error %v", ref.StringWithinTransport(), err) } - src, err := ref.NewImageSource(systemContext(), nil) + src, err := ref.NewImageSource(systemContext()) if err != nil { t.Fatalf("NewImageSource(%q) returned error %v", ref.StringWithinTransport(), err) } diff --git a/types/types.go b/types/types.go index 6bcd392f13..98cffcddbb 100644 --- a/types/types.go +++ b/types/types.go @@ -78,11 +78,9 @@ type ImageReference interface { // NOTE: If any kind of signature verification should happen, build an UnparsedImage from the value returned by NewImageSource, // verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage. NewImage(ctx *SystemContext) (Image, error) - // NewImageSource returns a types.ImageSource for this reference, - // asking the backend to use a manifest from requestedManifestMIMETypes if possible. - // nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes. + // NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. - NewImageSource(ctx *SystemContext, requestedManifestMIMETypes []string) (ImageSource, error) + NewImageSource(ctx *SystemContext) (ImageSource, error) // NewImageDestination returns a types.ImageDestination for this reference. // The caller must call .Close() on the returned ImageDestination. NewImageDestination(ctx *SystemContext) (ImageDestination, error)