Return a reference.Named instead of a string for Docker references

This is somewhat better typed, and avoids unnecessary roundtrips using
strings when both the producer and consumer want a reference.Named value
(like in PolicyContext.requirementsForImage).

This also forces us to explicitly handle IntendedDockerReference()
returning nil, when before we could rely on it returning "", which would
then be rejected by reference.ParseNamed as invalid input; anyway,
handling that case specially just allows for better error messages.

This adds two FIXMEs about error messages which do not tell the user
which image is being rejected; that will be fixed in the future
generalized reference work.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This commit is contained in:
Miloslav Trmač 2016-07-11 20:44:11 +02:00
parent 49cdff8b88
commit ca400b95a2
14 changed files with 240 additions and 142 deletions

View File

@ -1,12 +1,12 @@
package directory
import (
"fmt"
"io"
"io/ioutil"
"os"
"github.com/containers/image/types"
"github.com/docker/docker/reference"
)
type dirImageDestination struct {
@ -18,8 +18,8 @@ func NewImageDestination(dir string) types.ImageDestination {
return &dirImageDestination{dir}
}
func (d *dirImageDestination) CanonicalDockerReference() (string, error) {
return "", fmt.Errorf("Can not determine canonical Docker reference for a local directory")
func (d *dirImageDestination) CanonicalDockerReference() reference.Named {
return nil
}
func (d *dirImageDestination) SupportedManifestMIMETypes() []string {

View File

@ -7,6 +7,7 @@ import (
"os"
"github.com/containers/image/types"
"github.com/docker/docker/reference"
)
type dirImageSource struct {
@ -18,11 +19,12 @@ func NewImageSource(dir string) types.ImageSource {
return &dirImageSource{dir}
}
// IntendedDockerReference returns the full, unambiguous, Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). This can be used e.g. to determine which public keys are trusted for this image.
// May be "" if unknown.
func (s *dirImageSource) IntendedDockerReference() string {
return ""
// IntendedDockerReference returns the Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). Should be fully expanded, i.e. !reference.IsNameOnly.
// This can be used e.g. to determine which public keys are trusted for this image.
// May be nil if unknown.
func (s *dirImageSource) IntendedDockerReference() reference.Named {
return nil
}
// it's up to the caller to determine the MIME type of the returned manifest's bytes

View File

@ -12,8 +12,8 @@ import (
func TestCanonicalDockerReference(t *testing.T) {
dest := NewImageDestination("/path/to/somewhere")
_, err := dest.CanonicalDockerReference()
assert.Error(t, err)
ref := dest.CanonicalDockerReference()
assert.Nil(t, ref)
}
func TestGetPutManifest(t *testing.T) {
@ -85,7 +85,6 @@ func TestDelete(t *testing.T) {
func TestIntendedDockerReference(t *testing.T) {
src := NewImageSource("/path/to/somewhere")
dr := src.IntendedDockerReference()
assert.Equal(t, "", dr)
ref := src.IntendedDockerReference()
assert.Nil(t, ref)
}

View File

@ -43,8 +43,8 @@ func (d *dockerImageDestination) SupportedManifestMIMETypes() []string {
}
}
func (d *dockerImageDestination) CanonicalDockerReference() (string, error) {
return d.ref.String(), nil
func (d *dockerImageDestination) CanonicalDockerReference() reference.Named {
return d.ref
}
func (d *dockerImageDestination) PutManifest(m []byte) error {

View File

@ -49,11 +49,12 @@ func NewImageSource(img, certPath string, tlsVerify bool) (types.ImageSource, er
return newDockerImageSource(img, certPath, tlsVerify)
}
// IntendedDockerReference returns the full, unambiguous, Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). This can be used e.g. to determine which public keys are trusted for this image.
// May be "" if unknown.
func (s *dockerImageSource) IntendedDockerReference() string {
return s.ref.String()
// IntendedDockerReference returns the Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). Should be fully expanded, i.e. !reference.IsNameOnly.
// This can be used e.g. to determine which public keys are trusted for this image.
// May be nil if unknown.
func (s *dockerImageSource) IntendedDockerReference() reference.Named {
return s.ref
}
// simplifyContentType drops parameters from a HTTP media type (see https://tools.ietf.org/html/rfc7231#section-3.1.1.1)

View File

@ -13,6 +13,7 @@ import (
"github.com/containers/image/manifest"
"github.com/containers/image/types"
"github.com/docker/docker/reference"
)
var (
@ -50,10 +51,11 @@ func FromSource(src types.ImageSource, requestedManifestMIMETypes []string) type
return &genericImage{src: src, requestedManifestMIMETypes: requestedManifestMIMETypes}
}
// IntendedDockerReference returns the full, unambiguous, Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). This can be used e.g. to determine which public keys are trusted for this image.
// May be "" if unknown.
func (i *genericImage) IntendedDockerReference() string {
// IntendedDockerReference returns the Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). Should be fully expanded, i.e. !reference.IsNameOnly.
// This can be used e.g. to determine which public keys are trusted for this image.
// May be nil if unknown.
func (i *genericImage) IntendedDockerReference() reference.Named {
return i.src.IntendedDockerReference()
}

View File

@ -12,6 +12,7 @@ import (
"github.com/containers/image/manifest"
"github.com/containers/image/types"
"github.com/docker/docker/reference"
)
type ociManifest struct {
@ -53,8 +54,8 @@ func NewImageDestination(dest string) (types.ImageDestination, error) {
}, nil
}
func (d *ociImageDestination) CanonicalDockerReference() (string, error) {
return "", fmt.Errorf("Can not determine canonical Docker reference for an OCI image")
func (d *ociImageDestination) CanonicalDockerReference() reference.Named {
return nil
}
func createManifest(m []byte) ([]byte, string, error) {

View File

@ -17,6 +17,7 @@ import (
"github.com/containers/image/manifest"
"github.com/containers/image/types"
"github.com/containers/image/version"
"github.com/docker/docker/reference"
)
// openshiftClient is configuration for dealing with a single image stream, for reading or writing.
@ -28,9 +29,10 @@ type openshiftClient struct {
username string // "" if not used
password string // if username != ""
// Values specific to this image
namespace string
stream string
tag string
namespace string
stream string
tag string
canonicalDockerReference reference.Named // Computed from the above in advance, so that later references can not fail.
}
// FIXME: Is imageName like this a good way to refer to OpenShift images?
@ -58,7 +60,7 @@ func newOpenshiftClient(imageName string) (*openshiftClient, error) {
return nil, fmt.Errorf("Invalid image reference %s, %#v", imageName, m)
}
return &openshiftClient{
c := &openshiftClient{
baseURL: baseURL,
httpClient: httpClient,
bearerToken: restConfig.BearerToken,
@ -68,7 +70,21 @@ func newOpenshiftClient(imageName string) (*openshiftClient, error) {
namespace: m[1],
stream: m[2],
tag: m[3],
}, nil
}
// Precompute also c.canonicalDockerReference so that later references can not fail.
// FIXME: This is, strictly speaking, a namespace conflict with images placed in a Docker registry running on the same host.
// Do we need to do something else, perhaps disambiguate (port number?) or namespace Docker and OpenShift separately?
dockerRef, err := reference.WithName(fmt.Sprintf("%s/%s/%s", c.baseURL.Host, c.namespace, c.stream))
if err != nil {
return nil, err
}
c.canonicalDockerReference, err = reference.WithTag(dockerRef, c.tag)
if err != nil {
return nil, err
}
return c, nil
}
// doRequest performs a correctly authenticated request to a specified path, and returns response body or an error object.
@ -133,13 +149,6 @@ func (c *openshiftClient) doRequest(method, path string, requestBody []byte) ([]
return body, nil
}
// canonicalDockerReference returns a canonical reference we use for signing OpenShift images.
// FIXME: This is, strictly speaking, a namespace conflict with images placed in a Docker registry running on the same host.
// Do we need to do something else, perhaps disambiguate (port number?) or namespace Docker and OpenShift separately?
func (c *openshiftClient) canonicalDockerReference() string {
return fmt.Sprintf("%s/%s/%s:%s", c.baseURL.Host, c.namespace, c.stream, c.tag)
}
// convertDockerImageReference takes an image API DockerImageReference value and returns a reference we can actually use;
// currently OpenShift stores the cluster-internal service IPs here, which are unusable from the outside.
func (c *openshiftClient) convertDockerImageReference(ref string) (string, error) {
@ -186,11 +195,12 @@ func NewImageSource(imageName, certPath string, tlsVerify bool) (types.ImageSour
}, nil
}
// IntendedDockerReference returns the full, unambiguous, Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). This can be used e.g. to determine which public keys are trusted for this image.
// May be "" if unknown.
func (s *openshiftImageSource) IntendedDockerReference() string {
return s.client.canonicalDockerReference()
// IntendedDockerReference returns the Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). Should be fully expanded, i.e. !reference.IsNameOnly.
// This can be used e.g. to determine which public keys are trusted for this image.
// May be nil if unknown.
func (s *openshiftImageSource) IntendedDockerReference() reference.Named {
return s.client.canonicalDockerReference
}
func (s *openshiftImageSource) GetManifest(mimetypes []string) ([]byte, string, error) {
@ -290,8 +300,8 @@ func (d *openshiftImageDestination) SupportedManifestMIMETypes() []string {
}
}
func (d *openshiftImageDestination) CanonicalDockerReference() (string, error) {
return d.client.canonicalDockerReference(), nil
func (d *openshiftImageDestination) CanonicalDockerReference() reference.Named {
return d.client.canonicalDockerReference
}
func (d *openshiftImageDestination) PutManifest(m []byte) error {

View File

@ -70,7 +70,8 @@ type PolicyRequirement interface {
// The type is public, but its implementation is private.
type PolicyReferenceMatch interface {
// matchesDockerReference decides whether a specific image identity is accepted for an image
// (or, usually, for the image's IntendedDockerReference()),
// (or, usually, for the image's IntendedDockerReference()). Note that
// image.IntendedDockerReference() may be nil.
matchesDockerReference(image types.Image, signatureDockerReference string) bool
}
@ -154,15 +155,12 @@ func fullyExpandedDockerReference(ref reference.Named) (string, error) {
// requirementsForImage selects the appropriate requirements for image.
func (pc *PolicyContext) requirementsForImage(image types.Image) (PolicyRequirements, error) {
imageIdentity := image.IntendedDockerReference()
// We don't technically need to parse it first in order to match the full name:tag,
// but do so anyway to ensure that the intended identity really does follow that
// format, or at least that it is not demonstrably wrong.
ref, err := reference.ParseNamed(imageIdentity)
if err != nil {
return nil, err
ref := image.IntendedDockerReference()
if ref == nil {
// FIXME: Tell the user which image this is.
return nil, fmt.Errorf("Can not determine policy for an image with no known Docker reference identity")
}
ref = reference.WithDefaultTag(ref)
ref = reference.WithDefaultTag(ref) // This should not be needed, but if we did receive a name-only reference, this is a reasonable thing to do.
// Look for a full match.
fullyExpanded, err := fullyExpandedDockerReference(ref)

View File

@ -9,32 +9,35 @@ import (
"github.com/containers/image/directory"
"github.com/containers/image/image"
"github.com/containers/image/types"
"github.com/docker/docker/reference"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// dirImageMock returns a types.Image for a directory, claiming a specified intendedDockerReference.
func dirImageMock(dir, intendedDockerReference string) types.Image {
func dirImageMock(t *testing.T, dir, intendedDockerReference string) types.Image {
ref, err := reference.ParseNamed(intendedDockerReference)
require.NoError(t, err)
return image.FromSource(&dirImageSourceMock{
ImageSource: directory.NewImageSource(dir),
intendedDockerReference: intendedDockerReference,
intendedDockerReference: ref,
}, nil)
}
// dirImageSourceMock inherits dirImageSource, but overrides its IntendedDockerReference method.
type dirImageSourceMock struct {
types.ImageSource
intendedDockerReference string
intendedDockerReference reference.Named
}
func (d *dirImageSourceMock) IntendedDockerReference() string {
func (d *dirImageSourceMock) IntendedDockerReference() reference.Named {
return d.intendedDockerReference
}
func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) {
ktGPG := SBKeyTypeGPGKeys
prm := NewPRMMatchExact()
testImage := dirImageMock("fixtures/dir-img-valid", "testing/manifest:latest")
testImage := dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest")
testImageSig, err := ioutil.ReadFile("fixtures/dir-img-valid/signature-1")
require.NoError(t, err)
@ -138,7 +141,7 @@ func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) {
assertSARRejectedPolicyRequirement(t, sar, parsedSig, err)
// Error reading image manifest
image := dirImageMock("fixtures/dir-img-no-manifest", "testing/manifest:latest")
image := dirImageMock(t, "fixtures/dir-img-no-manifest", "testing/manifest:latest")
sig, err = ioutil.ReadFile("fixtures/dir-img-no-manifest/signature-1")
require.NoError(t, err)
pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
@ -147,7 +150,7 @@ func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) {
assertSARRejected(t, sar, parsedSig, err)
// Error computing manifest digest
image = dirImageMock("fixtures/dir-img-manifest-digest-error", "testing/manifest:latest")
image = dirImageMock(t, "fixtures/dir-img-manifest-digest-error", "testing/manifest:latest")
sig, err = ioutil.ReadFile("fixtures/dir-img-manifest-digest-error/signature-1")
require.NoError(t, err)
pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
@ -156,7 +159,7 @@ func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) {
assertSARRejected(t, sar, parsedSig, err)
// A valid signature with a non-matching manifest
image = dirImageMock("fixtures/dir-img-modified-manifest", "testing/manifest:latest")
image = dirImageMock(t, "fixtures/dir-img-modified-manifest", "testing/manifest:latest")
sig, err = ioutil.ReadFile("fixtures/dir-img-modified-manifest/signature-1")
require.NoError(t, err)
pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
@ -187,7 +190,7 @@ func TestPRSignedByIsRunningImageAllowed(t *testing.T) {
prm := NewPRMMatchExact()
// A simple success case: single valid signature.
image := dirImageMock("fixtures/dir-img-valid", "testing/manifest:latest")
image := dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest")
pr, err := NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
require.NoError(t, err)
allowed, err := pr.isRunningImageAllowed(image)
@ -196,42 +199,42 @@ func TestPRSignedByIsRunningImageAllowed(t *testing.T) {
// Error reading signatures
invalidSigDir := createInvalidSigDir(t)
defer os.RemoveAll(invalidSigDir)
image = dirImageMock(invalidSigDir, "testing/manifest:latest")
image = dirImageMock(t, invalidSigDir, "testing/manifest:latest")
pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
require.NoError(t, err)
allowed, err = pr.isRunningImageAllowed(image)
assertRunningRejected(t, allowed, err)
// No signatures
image = dirImageMock("fixtures/dir-img-unsigned", "testing/manifest:latest")
image = dirImageMock(t, "fixtures/dir-img-unsigned", "testing/manifest:latest")
pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
require.NoError(t, err)
allowed, err = pr.isRunningImageAllowed(image)
assertRunningRejectedPolicyRequirement(t, allowed, err)
// 1 invalid signature: use dir-img-valid, but a non-matching Docker reference
image = dirImageMock("fixtures/dir-img-valid", "testing/manifest:notlatest")
image = dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:notlatest")
pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
require.NoError(t, err)
allowed, err = pr.isRunningImageAllowed(image)
assertRunningRejectedPolicyRequirement(t, allowed, err)
// 2 valid signatures
image = dirImageMock("fixtures/dir-img-valid-2", "testing/manifest:latest")
image = dirImageMock(t, "fixtures/dir-img-valid-2", "testing/manifest:latest")
pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
require.NoError(t, err)
allowed, err = pr.isRunningImageAllowed(image)
assertRunningAllowed(t, allowed, err)
// One invalid, one valid signature (in this order)
image = dirImageMock("fixtures/dir-img-mixed", "testing/manifest:latest")
image = dirImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:latest")
pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
require.NoError(t, err)
allowed, err = pr.isRunningImageAllowed(image)
assertRunningAllowed(t, allowed, err)
// 2 invalid signatures: use dir-img-valid-2, but a non-matching Docker reference
image = dirImageMock("fixtures/dir-img-valid-2", "testing/manifest:notlatest")
image = dirImageMock(t, "fixtures/dir-img-valid-2", "testing/manifest:notlatest")
pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
require.NoError(t, err)
allowed, err = pr.isRunningImageAllowed(image)

View File

@ -5,6 +5,8 @@ import (
"os"
"testing"
"github.com/containers/image/directory"
"github.com/containers/image/image"
"github.com/docker/docker/reference"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@ -198,7 +200,9 @@ func TestPolicyContextRequirementsForImage(t *testing.T) {
expected = policy.Default
}
reqs, err := pc.requirementsForImage(refImageMock(input))
inputRef, err := reference.ParseNamed(input)
require.NoError(t, err)
reqs, err := pc.requirementsForImage(refImageMock{inputRef})
require.NoError(t, err)
comment := fmt.Sprintf("case %s: %#v", input, reqs[0])
// Do not sue assert.Equal, which would do a deep contents comparison; we want to compare
@ -208,8 +212,8 @@ func TestPolicyContextRequirementsForImage(t *testing.T) {
assert.True(t, len(reqs) == len(expected), comment)
}
// Invalid reference format
_, err = pc.requirementsForImage(refImageMock("UPPERCASEISINVALID"))
// Image without a Docker reference identity
_, err = pc.requirementsForImage(refImageMock{nil})
assert.Error(t, err)
}
@ -254,73 +258,73 @@ func TestPolicyContextGetSignaturesWithAcceptedAuthor(t *testing.T) {
defer pc.Destroy()
// Success
img := dirImageMock("fixtures/dir-img-valid", "testing/manifest:latest")
img := dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest")
sigs, err := pc.GetSignaturesWithAcceptedAuthor(img)
require.NoError(t, err)
assert.Equal(t, []*Signature{expectedSig}, sigs)
// Two signatures
// FIXME? Use really different signatures for this?
img = dirImageMock("fixtures/dir-img-valid-2", "testing/manifest:latest")
img = dirImageMock(t, "fixtures/dir-img-valid-2", "testing/manifest:latest")
sigs, err = pc.GetSignaturesWithAcceptedAuthor(img)
require.NoError(t, err)
assert.Equal(t, []*Signature{expectedSig, expectedSig}, sigs)
// No signatures
img = dirImageMock("fixtures/dir-img-unsigned", "testing/manifest:latest")
img = dirImageMock(t, "fixtures/dir-img-unsigned", "testing/manifest:latest")
sigs, err = pc.GetSignaturesWithAcceptedAuthor(img)
require.NoError(t, err)
assert.Empty(t, sigs)
// Only invalid signatures
img = dirImageMock("fixtures/dir-img-modified-manifest", "testing/manifest:latest")
img = dirImageMock(t, "fixtures/dir-img-modified-manifest", "testing/manifest:latest")
sigs, err = pc.GetSignaturesWithAcceptedAuthor(img)
require.NoError(t, err)
assert.Empty(t, sigs)
// 1 invalid, 1 valid signature (in this order)
img = dirImageMock("fixtures/dir-img-mixed", "testing/manifest:latest")
img = dirImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:latest")
sigs, err = pc.GetSignaturesWithAcceptedAuthor(img)
require.NoError(t, err)
assert.Equal(t, []*Signature{expectedSig}, sigs)
// Two sarAccepted results for one signature
img = dirImageMock("fixtures/dir-img-valid", "testing/manifest:twoAccepts")
img = dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:twoAccepts")
sigs, err = pc.GetSignaturesWithAcceptedAuthor(img)
require.NoError(t, err)
assert.Equal(t, []*Signature{expectedSig}, sigs)
// sarAccepted+sarRejected for a signature
img = dirImageMock("fixtures/dir-img-valid", "testing/manifest:acceptReject")
img = dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:acceptReject")
sigs, err = pc.GetSignaturesWithAcceptedAuthor(img)
require.NoError(t, err)
assert.Empty(t, sigs)
// sarAccepted+sarUnknown for a signature
img = dirImageMock("fixtures/dir-img-valid", "testing/manifest:acceptUnknown")
img = dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:acceptUnknown")
sigs, err = pc.GetSignaturesWithAcceptedAuthor(img)
require.NoError(t, err)
assert.Equal(t, []*Signature{expectedSig}, sigs)
// sarRejected+sarUnknown for a signature
img = dirImageMock("fixtures/dir-img-valid", "testing/manifest:rejectUnknown")
img = dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:rejectUnknown")
sigs, err = pc.GetSignaturesWithAcceptedAuthor(img)
require.NoError(t, err)
assert.Empty(t, sigs)
// sarUnknown only
img = dirImageMock("fixtures/dir-img-valid", "testing/manifest:unknown")
img = dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:unknown")
sigs, err = pc.GetSignaturesWithAcceptedAuthor(img)
require.NoError(t, err)
assert.Empty(t, sigs)
img = dirImageMock("fixtures/dir-img-valid", "testing/manifest:unknown2")
img = dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:unknown2")
sigs, err = pc.GetSignaturesWithAcceptedAuthor(img)
require.NoError(t, err)
assert.Empty(t, sigs)
// Empty list of requirements (invalid)
img = dirImageMock("fixtures/dir-img-valid", "testing/manifest:invalidEmptyRequirements")
img = dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:invalidEmptyRequirements")
sigs, err = pc.GetSignaturesWithAcceptedAuthor(img)
require.NoError(t, err)
assert.Empty(t, sigs)
@ -332,7 +336,7 @@ func TestPolicyContextGetSignaturesWithAcceptedAuthor(t *testing.T) {
require.NoError(t, err)
err = destroyedPC.Destroy()
require.NoError(t, err)
img = dirImageMock("fixtures/dir-img-valid", "testing/manifest:latest")
img = dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest")
sigs, err = destroyedPC.GetSignaturesWithAcceptedAuthor(img)
assert.Error(t, err)
assert.Nil(t, sigs)
@ -340,8 +344,11 @@ func TestPolicyContextGetSignaturesWithAcceptedAuthor(t *testing.T) {
// implementations meddling with the state, or threads. This is for catching trivial programmer
// mistakes only, anyway.
// Invalid IntendedDockerReference value
img = dirImageMock("fixtures/dir-img-valid", "UPPERCASEISINVALID")
// Image without a Docker reference identity
img = image.FromSource(&dirImageSourceMock{
ImageSource: directory.NewImageSource("fixtures/dir-img-valid"),
intendedDockerReference: nil,
}, nil)
sigs, err = pc.GetSignaturesWithAcceptedAuthor(img)
assert.Error(t, err)
assert.Nil(t, sigs)
@ -349,7 +356,7 @@ func TestPolicyContextGetSignaturesWithAcceptedAuthor(t *testing.T) {
// Error reading signatures.
invalidSigDir := createInvalidSigDir(t)
defer os.RemoveAll(invalidSigDir)
img = dirImageMock(invalidSigDir, "testing/manifest:latest")
img = dirImageMock(t, invalidSigDir, "testing/manifest:latest")
sigs, err = pc.GetSignaturesWithAcceptedAuthor(img)
assert.Error(t, err)
assert.Nil(t, sigs)
@ -383,53 +390,53 @@ func TestPolicyContextIsRunningImageAllowed(t *testing.T) {
defer pc.Destroy()
// Success
img := dirImageMock("fixtures/dir-img-valid", "testing/manifest:latest")
img := dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest")
res, err := pc.IsRunningImageAllowed(img)
assertRunningAllowed(t, res, err)
// Two signatures
// FIXME? Use really different signatures for this?
img = dirImageMock("fixtures/dir-img-valid-2", "testing/manifest:latest")
img = dirImageMock(t, "fixtures/dir-img-valid-2", "testing/manifest:latest")
res, err = pc.IsRunningImageAllowed(img)
assertRunningAllowed(t, res, err)
// No signatures
img = dirImageMock("fixtures/dir-img-unsigned", "testing/manifest:latest")
img = dirImageMock(t, "fixtures/dir-img-unsigned", "testing/manifest:latest")
res, err = pc.IsRunningImageAllowed(img)
assertRunningRejectedPolicyRequirement(t, res, err)
// Only invalid signatures
img = dirImageMock("fixtures/dir-img-modified-manifest", "testing/manifest:latest")
img = dirImageMock(t, "fixtures/dir-img-modified-manifest", "testing/manifest:latest")
res, err = pc.IsRunningImageAllowed(img)
assertRunningRejectedPolicyRequirement(t, res, err)
// 1 invalid, 1 valid signature (in this order)
img = dirImageMock("fixtures/dir-img-mixed", "testing/manifest:latest")
img = dirImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:latest")
res, err = pc.IsRunningImageAllowed(img)
assertRunningAllowed(t, res, err)
// Two allowed results
img = dirImageMock("fixtures/dir-img-mixed", "testing/manifest:twoAllows")
img = dirImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:twoAllows")
res, err = pc.IsRunningImageAllowed(img)
assertRunningAllowed(t, res, err)
// Allow + deny results
img = dirImageMock("fixtures/dir-img-mixed", "testing/manifest:allowDeny")
img = dirImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:allowDeny")
res, err = pc.IsRunningImageAllowed(img)
assertRunningRejectedPolicyRequirement(t, res, err)
// prReject works
img = dirImageMock("fixtures/dir-img-mixed", "testing/manifest:reject")
img = dirImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:reject")
res, err = pc.IsRunningImageAllowed(img)
assertRunningRejectedPolicyRequirement(t, res, err)
// prInsecureAcceptAnything works
img = dirImageMock("fixtures/dir-img-mixed", "testing/manifest:acceptAnything")
img = dirImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:acceptAnything")
res, err = pc.IsRunningImageAllowed(img)
assertRunningAllowed(t, res, err)
// Empty list of requirements (invalid)
img = dirImageMock("fixtures/dir-img-valid", "testing/manifest:invalidEmptyRequirements")
img = dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:invalidEmptyRequirements")
res, err = pc.IsRunningImageAllowed(img)
assertRunningRejectedPolicyRequirement(t, res, err)
@ -438,15 +445,18 @@ func TestPolicyContextIsRunningImageAllowed(t *testing.T) {
require.NoError(t, err)
err = destroyedPC.Destroy()
require.NoError(t, err)
img = dirImageMock("fixtures/dir-img-valid", "testing/manifest:latest")
img = dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest")
res, err = destroyedPC.IsRunningImageAllowed(img)
assertRunningRejected(t, res, err)
// Not testing the pcInUse->pcReady transition, that would require custom PolicyRequirement
// implementations meddling with the state, or threads. This is for catching trivial programmer
// mistakes only, anyway.
// Invalid IntendedDockerReference value
img = dirImageMock("fixtures/dir-img-valid", "UPPERCASEISINVALID")
// Image without a Docker reference identity
img = image.FromSource(&dirImageSourceMock{
ImageSource: directory.NewImageSource("fixtures/dir-img-valid"),
intendedDockerReference: nil,
}, nil)
res, err = pc.IsRunningImageAllowed(img)
assertRunningRejected(t, res, err)
}

View File

@ -7,6 +7,40 @@ import (
"github.com/containers/image/types"
)
// parseImageAndDockerReference converts an image and a reference string into two parsed entities, failing on any error and handling unidentified images.
func parseImageAndDockerReference(image types.Image, s2 string) (reference.Named, reference.Named, error) {
r1 := image.IntendedDockerReference()
if r1 == nil {
// FIXME: Tell the user which image this is.
return nil, nil, PolicyRequirementError("Docker reference match attempted on an image with no known Docker reference identity")
}
r2, err := reference.ParseNamed(s2)
if err != nil {
return nil, nil, err
}
return r1, r2, nil
}
func (prm *prmMatchExact) matchesDockerReference(image types.Image, signatureDockerReference string) bool {
intended, signature, err := parseImageAndDockerReference(image, signatureDockerReference)
if err != nil {
return false
}
// Do not add default tags: image.IntendedDockerReference() has it added already per its construction, and signatureDockerReference should be exact; so, verify that now.
if reference.IsNameOnly(intended) || reference.IsNameOnly(signature) {
return false
}
return signature.String() == intended.String()
}
func (prm *prmMatchRepository) matchesDockerReference(image types.Image, signatureDockerReference string) bool {
intended, signature, err := parseImageAndDockerReference(image, signatureDockerReference)
if err != nil {
return false
}
return signature.Name() == intended.Name()
}
// parseDockerReferences converts two reference strings into parsed entities, failing on any error
func parseDockerReferences(s1, s2 string) (reference.Named, reference.Named, error) {
r1, err := reference.ParseNamed(s1)
@ -20,26 +54,6 @@ func parseDockerReferences(s1, s2 string) (reference.Named, reference.Named, err
return r1, r2, nil
}
func (prm *prmMatchExact) matchesDockerReference(image types.Image, signatureDockerReference string) bool {
intended, signature, err := parseDockerReferences(image.IntendedDockerReference(), signatureDockerReference)
if err != nil {
return false
}
// Do not add default tags: image.IntendedDockerReference() has it added already per its construction, and signatureDockerReference should be exact; so, verify that now.
if reference.IsNameOnly(intended) || reference.IsNameOnly(signature) {
return false
}
return signature.String() == intended.String()
}
func (prm *prmMatchRepository) matchesDockerReference(image types.Image, signatureDockerReference string) bool {
intended, signature, err := parseDockerReferences(image.IntendedDockerReference(), signatureDockerReference)
if err != nil {
return false
}
return signature.Name() == intended.Name()
}
func (prm *prmExactReference) matchesDockerReference(image types.Image, signatureDockerReference string) bool {
intended, signature, err := parseDockerReferences(prm.DockerReference, signatureDockerReference)
if err != nil {

View File

@ -5,6 +5,7 @@ import (
"testing"
"github.com/containers/image/types"
"github.com/docker/docker/reference"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@ -15,36 +16,45 @@ const (
untaggedRHELRef = "registry.access.redhat.com/rhel7/rhel"
)
func TestParseDockerReferences(t *testing.T) {
func TestParseImageAndDockerReference(t *testing.T) {
const (
ok1 = "busybox"
ok2 = fullRHELRef
bad1 = "UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES"
bad2 = ""
)
// Success
r1, r2, err := parseDockerReferences(ok1, ok2)
ref, err := reference.ParseNamed(ok1)
require.NoError(t, err)
r1, r2, err := parseImageAndDockerReference(refImageMock{ref}, ok2)
require.NoError(t, err)
assert.Equal(t, ok1, r1.String())
assert.Equal(t, ok2, r2.String())
// 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},
} {
_, _, err := parseDockerReferences(refs[0], refs[1])
assert.Error(t, err)
ref, err := reference.ParseNamed(refs[0])
if err == nil {
_, _, err := parseImageAndDockerReference(refImageMock{ref}, refs[1])
assert.Error(t, err)
}
}
}
// refImageMock is a mock of types.Image which returns itself in IntendedDockerReference.
type refImageMock string
type refImageMock struct{ reference.Named }
func (ref refImageMock) IntendedDockerReference() string {
return string(ref)
func (ref refImageMock) IntendedDockerReference() reference.Named {
return ref.Named
}
func (ref refImageMock) Manifest() ([]byte, string, error) {
panic("unexpected call to a mock function")
@ -137,23 +147,66 @@ var prmRepositoryMatchTestTable = []prmTableTest{
func TestPRMMatchExactMatchesDockerReference(t *testing.T) {
prm := NewPRMMatchExact()
for _, test := range prmExactMatchTestTable {
res := prm.matchesDockerReference(refImageMock(test.imageRef), test.sigRef)
// This assumes that all ways to obtain a reference.Named perform equivalent validation,
// and therefore values refused by reference.ParseNamed can not happen in practice.
imageRef, err := reference.ParseNamed(test.imageRef)
if err != nil {
continue
}
res := prm.matchesDockerReference(refImageMock{imageRef}, test.sigRef)
assert.Equal(t, test.result, res, fmt.Sprintf("%s vs. %s", test.imageRef, test.sigRef))
}
// 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 TestPRMMatchRepositoryMatchesDockerReference(t *testing.T) {
prm := NewPRMMatchRepository()
for _, test := range prmRepositoryMatchTestTable {
res := prm.matchesDockerReference(refImageMock(test.imageRef), test.sigRef)
// This assumes that all ways to obtain a reference.Named perform equivalent validation,
// and therefore values refused by reference.ParseNamed can not happen in practice.
imageRef, err := reference.ParseNamed(test.imageRef)
if err != nil {
continue
}
res := prm.matchesDockerReference(refImageMock{imageRef}, test.sigRef)
assert.Equal(t, test.result, res, fmt.Sprintf("%s vs. %s", test.imageRef, test.sigRef))
}
// 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, r1.String())
assert.Equal(t, ok2, r2.String())
// 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.Image which ensures IntendedDockerReference is not called
type forbiddenImageMock string
func (ref forbiddenImageMock) IntendedDockerReference() string {
func (ref forbiddenImageMock) IntendedDockerReference() reference.Named {
panic("unexpected call to a mock function")
}
func (ref forbiddenImageMock) Manifest() ([]byte, string, error) {

View File

@ -3,16 +3,19 @@ package types
import (
"io"
"time"
"github.com/docker/docker/reference"
)
// ImageSource is a service, possibly remote (= slow), to download components of a single image.
// This is primarily useful for copying images around; for examining their properties, Image (below)
// is usually more useful.
type ImageSource interface {
// IntendedDockerReference returns the full, unambiguous, Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). This can be used e.g. to determine which public keys are trusted for this image.
// May be "" if unknown.
IntendedDockerReference() string
// IntendedDockerReference returns the Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). Should be fully expanded, i.e. !reference.IsNameOnly.
// This can be used e.g. to determine which public keys are trusted for this image.
// May be nil if unknown.
IntendedDockerReference() reference.Named
// GetManifest returns the image's manifest along with its MIME type. The empty string is returned if the MIME type is unknown. The slice parameter indicates the supported mime types the manifest should be when getting it.
// It may use a remote (= slow) service.
GetManifest([]string) ([]byte, string, error)
@ -27,8 +30,9 @@ type ImageSource interface {
// ImageDestination is a service, possibly remote (= slow), to store components of a single image.
type ImageDestination interface {
// CanonicalDockerReference returns the full, unambiguous, Docker reference for this image (even if the user referred to the image using some shorthand notation).
CanonicalDockerReference() (string, error)
// CanonicalDockerReference returns the Docker reference for this image (fully expanded, i.e. !reference.IsNameOnly, but
// reflecting user intent, not e.g. after redirect or alias processing), or nil if unknown.
CanonicalDockerReference() reference.Named
// FIXME? This should also receive a MIME type if known, to differentiate between schema versions.
PutManifest([]byte) error
// Note: Calling PutBlob() and other methods may have ordering dependencies WRT other methods of this type. FIXME: Figure out and document.
@ -42,10 +46,11 @@ type ImageDestination interface {
// Image is the primary API for inspecting properties of images.
type Image interface {
// ref to repository?
// IntendedDockerReference returns the full, unambiguous, Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). This can be used e.g. to determine which public keys are trusted for this image.
// May be "" if unknown.
IntendedDockerReference() string
// IntendedDockerReference returns the Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). Should be fully expanded, i.e. !reference.IsNameOnly.
// This can be used e.g. to determine which public keys are trusted for this image.
// May be nil if unknown.
IntendedDockerReference() reference.Named
// Manifest is like ImageSource.GetManifest, but the result is cached; it is OK to call this however often you need.
// NOTE: It is essential for signature verification that Manifest returns the manifest from which BlobDigests is computed.
Manifest() ([]byte, string, error)