Skip to content

Commit

Permalink
Add private.UnparsedImage, use it for signature handling
Browse files Browse the repository at this point in the history
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
  • Loading branch information
mtrmac committed Jul 7, 2022
1 parent a2a33d2 commit 81f5e6d
Show file tree
Hide file tree
Showing 15 changed files with 100 additions and 45 deletions.
2 changes: 1 addition & 1 deletion copy/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ func (c *copier) copyOneImage(ctx context.Context, policyContext *signature.Poli
sigs = []internalsig.Signature{}
} else {
c.Printf("Getting image source signatures\n")
s, err := src.SignaturesWithFormat(ctx)
s, err := src.UntrustedSignatures(ctx)
if err != nil {
return nil, "", "", perrors.Wrap(err, "reading signatures")
}
Expand Down
6 changes: 0 additions & 6 deletions internal/image/sourced.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package image
import (
"context"

"github.com/containers/image/v5/internal/signature"
"github.com/containers/image/v5/types"
)

Expand Down Expand Up @@ -130,11 +129,6 @@ func (i *SourcedImage) Manifest(ctx context.Context) ([]byte, string, error) {
return i.ManifestBlob, i.ManifestMIMEType, nil
}

// SignaturesWithFormat is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need.
func (i *SourcedImage) SignaturesWithFormat(ctx context.Context) ([]signature.Signature, error) {
return i.UnparsedImage.signaturesWithFormat(ctx)
}

func (i *SourcedImage) LayerInfosForCopy(ctx context.Context) ([]types.BlobInfo, error) {
return i.UnparsedImage.src.LayerInfosForCopy(ctx, i.UnparsedImage.instanceDigest)
}
8 changes: 5 additions & 3 deletions internal/image/unparsed.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@ func (i *UnparsedImage) expectedManifestDigest() (digest.Digest, bool) {

// Signatures is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need.
func (i *UnparsedImage) Signatures(ctx context.Context) ([][]byte, error) {
sigs, err := i.signaturesWithFormat(ctx)
// It would be consistent to make this an internal/unparsedimage/impl.Compat wrapper,
// but this is very likely to be the only implementation ever.
sigs, err := i.UntrustedSignatures(ctx)
if err != nil {
return nil, err
}
Expand All @@ -105,8 +107,8 @@ func (i *UnparsedImage) Signatures(ctx context.Context) ([][]byte, error) {
return simpleSigs, nil
}

// signaturesWithFormat is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need.
func (i *UnparsedImage) signaturesWithFormat(ctx context.Context) ([]signature.Signature, error) {
// UntrustedSignatures is like ImageSource.GetSignaturesWithFormat, but the result is cached; it is OK to call this however often you need.
func (i *UnparsedImage) UntrustedSignatures(ctx context.Context) ([]signature.Signature, error) {
if i.cachedSignatures == nil {
sigs, err := i.src.GetSignaturesWithFormat(ctx, i.instanceDigest)
if err != nil {
Expand Down
7 changes: 7 additions & 0 deletions internal/private/private.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,10 @@ type BadPartialRequestError struct {
func (e BadPartialRequestError) Error() string {
return e.Status
}

// UnparsedImage is an internal extension to the types.UnparsedImage interface.
type UnparsedImage interface {
types.UnparsedImage
// UntrustedSignatures is like ImageSource.GetSignaturesWithFormat, but the result is cached; it is OK to call this however often you need.
UntrustedSignatures(ctx context.Context) ([]signature.Signature, error)
}
6 changes: 6 additions & 0 deletions internal/testing/mocks/unparsed_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package mocks
import (
"context"

"github.com/containers/image/v5/internal/signature"
"github.com/containers/image/v5/types"
)

Expand All @@ -23,3 +24,8 @@ func (ref ForbiddenUnparsedImage) Manifest(ctx context.Context) ([]byte, string,
func (ref ForbiddenUnparsedImage) Signatures(context.Context) ([][]byte, error) {
panic("unexpected call to a mock function")
}

// UntrustedSignatures is a mock that panics.
func (ref ForbiddenUnparsedImage) UntrustedSignatures(ctx context.Context) ([]signature.Signature, error) {
panic("unexpected call to a mock function")
}
38 changes: 38 additions & 0 deletions internal/unparsedimage/wrapper.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package unparsedimage

import (
"context"

"github.com/containers/image/v5/internal/private"
"github.com/containers/image/v5/internal/signature"
"github.com/containers/image/v5/types"
)

// wrapped provides the private.UnparsedImage operations
// for an object that only implements types.UnparsedImage
type wrapped struct {
types.UnparsedImage
}

// FromPublic(unparsed) returns an object that provides the private.UnparsedImage API
func FromPublic(unparsed types.UnparsedImage) private.UnparsedImage {
if unparsed2, ok := unparsed.(private.UnparsedImage); ok {
return unparsed2
}
return &wrapped{
UnparsedImage: unparsed,
}
}

// UntrustedSignatures is like ImageSource.GetSignaturesWithFormat, but the result is cached; it is OK to call this however often you need.
func (w *wrapped) UntrustedSignatures(ctx context.Context) ([]signature.Signature, error) {
sigs, err := w.Signatures(ctx)
if err != nil {
return nil, err
}
res := []signature.Signature{}
for _, sig := range sigs {
res = append(res, signature.SimpleSigningFromBlob(sig))
}
return res, nil
}
19 changes: 12 additions & 7 deletions signature/policy_eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"context"
"fmt"

"github.com/containers/image/v5/internal/private"
"github.com/containers/image/v5/internal/unparsedimage"
"github.com/containers/image/v5/types"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -55,14 +57,14 @@ type PolicyRequirement interface {
// a container based on this image; use IsRunningImageAllowed instead.
// - Just because a signature is accepted does not automatically mean the contents of the
// signature are authorized to run code as root, or to affect system or cluster configuration.
isSignatureAuthorAccepted(ctx context.Context, image types.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error)
isSignatureAuthorAccepted(ctx context.Context, image private.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error)

// isRunningImageAllowed returns true if the requirement allows running an image.
// If it returns false, err must be non-nil, and should be an PolicyRequirementError if evaluation
// succeeded but the result was rejection.
// WARNING: This validates signatures and the manifest, but does not download or validate the
// layers. Users must validate that the layers match their expected digests.
isRunningImageAllowed(ctx context.Context, image types.UnparsedImage) (bool, error)
isRunningImageAllowed(ctx context.Context, image private.UnparsedImage) (bool, error)
}

// PolicyReferenceMatch specifies a set of image identities accepted in PolicyRequirement.
Expand All @@ -71,7 +73,7 @@ type PolicyReferenceMatch interface {
// matchesDockerReference decides whether a specific image identity is accepted for an image
// (or, usually, for the image's Reference().DockerReference()). Note that
// image.Reference().DockerReference() may be nil.
matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool
matchesDockerReference(image private.UnparsedImage, signatureDockerReference string) bool
}

// PolicyContext encapsulates a policy and possible cached state
Expand Down Expand Up @@ -174,7 +176,7 @@ func (pc *PolicyContext) requirementsForImageRef(ref types.ImageReference) Polic
// a container based on this image; use IsRunningImageAllowed instead.
// - Just because a signature is accepted does not automatically mean the contents of the
// signature are authorized to run code as root, or to affect system or cluster configuration.
func (pc *PolicyContext) GetSignaturesWithAcceptedAuthor(ctx context.Context, image types.UnparsedImage) (sigs []*Signature, finalErr error) {
func (pc *PolicyContext) GetSignaturesWithAcceptedAuthor(ctx context.Context, publicImage types.UnparsedImage) (sigs []*Signature, finalErr error) {
if err := pc.changeState(pcReady, pcInUse); err != nil {
return nil, err
}
Expand All @@ -185,11 +187,12 @@ func (pc *PolicyContext) GetSignaturesWithAcceptedAuthor(ctx context.Context, im
}
}()

image := unparsedimage.FromPublic(publicImage)

logrus.Debugf("GetSignaturesWithAcceptedAuthor for image %s", policyIdentityLogName(image.Reference()))
reqs := pc.requirementsForImageRef(image.Reference())

// FIXME: rename Signatures to UnverifiedSignatures
// FIXME: pass context.Context
// FIXME: Use image.UntrustedSignatures, use that to improve error messages (needs tests!)
unverifiedSignatures, err := image.Signatures(ctx)
if err != nil {
return nil, err
Expand Down Expand Up @@ -255,7 +258,7 @@ func (pc *PolicyContext) GetSignaturesWithAcceptedAuthor(ctx context.Context, im
// succeeded but the result was rejection.
// WARNING: This validates signatures and the manifest, but does not download or validate the
// layers. Users must validate that the layers match their expected digests.
func (pc *PolicyContext) IsRunningImageAllowed(ctx context.Context, image types.UnparsedImage) (res bool, finalErr error) {
func (pc *PolicyContext) IsRunningImageAllowed(ctx context.Context, publicImage types.UnparsedImage) (res bool, finalErr error) {
if err := pc.changeState(pcReady, pcInUse); err != nil {
return false, err
}
Expand All @@ -266,6 +269,8 @@ func (pc *PolicyContext) IsRunningImageAllowed(ctx context.Context, image types.
}
}()

image := unparsedimage.FromPublic(publicImage)

logrus.Debugf("IsRunningImageAllowed for image %s", policyIdentityLogName(image.Reference()))
reqs := pc.requirementsForImageRef(image.Reference())

Expand Down
6 changes: 3 additions & 3 deletions signature/policy_eval_baselayer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ package signature
import (
"context"

"github.com/containers/image/v5/types"
"github.com/containers/image/v5/internal/private"
"github.com/sirupsen/logrus"
)

func (pr *prSignedBaseLayer) isSignatureAuthorAccepted(ctx context.Context, image types.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) {
func (pr *prSignedBaseLayer) isSignatureAuthorAccepted(ctx context.Context, image private.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) {
return sarUnknown, nil, nil
}

func (pr *prSignedBaseLayer) isRunningImageAllowed(ctx context.Context, image types.UnparsedImage) (bool, error) {
func (pr *prSignedBaseLayer) isRunningImageAllowed(ctx context.Context, image private.UnparsedImage) (bool, error) {
// FIXME? Reject this at policy parsing time already?
logrus.Errorf("signedBaseLayer not implemented yet!")
return false, PolicyRequirementError("signedBaseLayer not implemented yet!")
Expand Down
9 changes: 5 additions & 4 deletions signature/policy_eval_signedby.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ import (
"os"
"strings"

"github.com/containers/image/v5/internal/private"
"github.com/containers/image/v5/manifest"
"github.com/containers/image/v5/types"
digest "github.com/opencontainers/go-digest"
)

func (pr *prSignedBy) isSignatureAuthorAccepted(ctx context.Context, image types.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) {
func (pr *prSignedBy) isSignatureAuthorAccepted(ctx context.Context, image private.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) {
switch pr.KeyType {
case SBKeyTypeGPGKeys:
case SBKeyTypeSignedByGPGKeys, SBKeyTypeX509Certificates, SBKeyTypeSignedByX509CAs:
Expand Down Expand Up @@ -89,8 +89,9 @@ func (pr *prSignedBy) isSignatureAuthorAccepted(ctx context.Context, image types
return sarAccepted, signature, nil
}

func (pr *prSignedBy) isRunningImageAllowed(ctx context.Context, image types.UnparsedImage) (bool, error) {
// FIXME: pass context.Context
func (pr *prSignedBy) isRunningImageAllowed(ctx context.Context, image private.UnparsedImage) (bool, error) {
// FIXME: Use image.UntrustedSignatures, use that to improve error messages
// (needs tests!)
sigs, err := image.Signatures(ctx)
if err != nil {
return false, err
Expand Down
9 changes: 5 additions & 4 deletions signature/policy_eval_signedby_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,21 @@ import (
"github.com/containers/image/v5/directory"
"github.com/containers/image/v5/docker/reference"
"github.com/containers/image/v5/internal/image"
"github.com/containers/image/v5/internal/private"
"github.com/containers/image/v5/types"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// dirImageMock returns a types.UnparsedImage for a directory, claiming a specified dockerReference.
func dirImageMock(t *testing.T, dir, dockerReference string) types.UnparsedImage {
// dirImageMock returns a private.UnparsedImage for a directory, claiming a specified dockerReference.
func dirImageMock(t *testing.T, dir, dockerReference string) private.UnparsedImage {
ref, err := reference.ParseNormalizedNamed(dockerReference)
require.NoError(t, err)
return dirImageMockWithRef(t, dir, refImageReferenceMock{ref: ref})
}

// dirImageMockWithRef returns a types.UnparsedImage for a directory, claiming a specified ref.
func dirImageMockWithRef(t *testing.T, dir string, ref types.ImageReference) types.UnparsedImage {
// dirImageMockWithRef returns a private.UnparsedImage for a directory, claiming a specified ref.
func dirImageMockWithRef(t *testing.T, dir string, ref types.ImageReference) private.UnparsedImage {
srcRef, err := directory.NewReference(dir)
require.NoError(t, err)
src, err := srcRef.NewImageSource(context.Background(), nil)
Expand Down
10 changes: 5 additions & 5 deletions signature/policy_eval_simple.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,24 @@ import (
"context"
"fmt"

"github.com/containers/image/v5/internal/private"
"github.com/containers/image/v5/transports"
"github.com/containers/image/v5/types"
)

func (pr *prInsecureAcceptAnything) isSignatureAuthorAccepted(ctx context.Context, image types.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) {
func (pr *prInsecureAcceptAnything) isSignatureAuthorAccepted(ctx context.Context, image private.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) {
// prInsecureAcceptAnything semantics: Every image is allowed to run,
// but this does not consider the signature as verified.
return sarUnknown, nil, nil
}

func (pr *prInsecureAcceptAnything) isRunningImageAllowed(ctx context.Context, image types.UnparsedImage) (bool, error) {
func (pr *prInsecureAcceptAnything) isRunningImageAllowed(ctx context.Context, image private.UnparsedImage) (bool, error) {
return true, nil
}

func (pr *prReject) isSignatureAuthorAccepted(ctx context.Context, image types.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) {
func (pr *prReject) isSignatureAuthorAccepted(ctx context.Context, image private.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) {
return sarRejected, nil, PolicyRequirementError(fmt.Sprintf("Any signatures for image %s are rejected by policy.", transports.ImageName(image.Reference())))
}

func (pr *prReject) isRunningImageAllowed(ctx context.Context, image types.UnparsedImage) (bool, error) {
func (pr *prReject) isRunningImageAllowed(ctx context.Context, image private.UnparsedImage) (bool, error) {
return false, PolicyRequirementError(fmt.Sprintf("Running image %s is rejected by policy.", transports.ImageName(image.Reference())))
}
2 changes: 1 addition & 1 deletion signature/policy_eval_simple_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/containers/image/v5/types"
)

// nameOnlyImageMock is a mock of types.UnparsedImage which only allows transports.ImageName to work
// nameOnlyImageMock is a mock of private.UnparsedImage which only allows transports.ImageName to work
type nameOnlyImageMock struct {
mocks.ForbiddenUnparsedImage
}
Expand Down
5 changes: 3 additions & 2 deletions signature/policy_eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/containers/image/v5/docker"
"github.com/containers/image/v5/docker/policyconfiguration"
"github.com/containers/image/v5/docker/reference"
"github.com/containers/image/v5/internal/private"
"github.com/containers/image/v5/internal/testing/mocks"
"github.com/containers/image/v5/transports"
"github.com/containers/image/v5/types"
Expand Down Expand Up @@ -202,8 +203,8 @@ func TestPolicyContextRequirementsForImageRef(t *testing.T) {
}
}

// pcImageMock returns a types.UnparsedImage for a directory, claiming a specified dockerReference and implementing PolicyConfigurationIdentity/PolicyConfigurationNamespaces.
func pcImageMock(t *testing.T, dir, dockerReference string) types.UnparsedImage {
// pcImageMock returns a private.UnparsedImage for a directory, claiming a specified dockerReference and implementing PolicyConfigurationIdentity/PolicyConfigurationNamespaces.
func pcImageMock(t *testing.T, dir, dockerReference string) private.UnparsedImage {
ref, err := reference.ParseNormalizedNamed(dockerReference)
require.NoError(t, err)
return dirImageMockWithRef(t, dir, pcImageReferenceMock{transportName: "docker", ref: ref})
Expand Down
16 changes: 8 additions & 8 deletions signature/policy_reference_match.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import (
"strings"

"github.com/containers/image/v5/docker/reference"
"github.com/containers/image/v5/internal/private"
"github.com/containers/image/v5/transports"
"github.com/containers/image/v5/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.UnparsedImage, s2 string) (reference.Named, reference.Named, error) {
func parseImageAndDockerReference(image private.UnparsedImage, s2 string) (reference.Named, reference.Named, error) {
r1 := image.Reference().DockerReference()
if r1 == nil {
return nil, nil, PolicyRequirementError(fmt.Sprintf("Docker reference match attempted on image %s with no known Docker reference identity",
Expand All @@ -25,7 +25,7 @@ func parseImageAndDockerReference(image types.UnparsedImage, s2 string) (referen
return r1, r2, nil
}

func (prm *prmMatchExact) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool {
func (prm *prmMatchExact) matchesDockerReference(image private.UnparsedImage, signatureDockerReference string) bool {
intended, signature, err := parseImageAndDockerReference(image, signatureDockerReference)
if err != nil {
return false
Expand Down Expand Up @@ -56,15 +56,15 @@ func matchRepoDigestOrExactReferenceValues(intended, signature reference.Named)
return false
}
}
func (prm *prmMatchRepoDigestOrExact) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool {
func (prm *prmMatchRepoDigestOrExact) matchesDockerReference(image private.UnparsedImage, signatureDockerReference string) bool {
intended, signature, err := parseImageAndDockerReference(image, signatureDockerReference)
if err != nil {
return false
}
return matchRepoDigestOrExactReferenceValues(intended, signature)
}

func (prm *prmMatchRepository) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool {
func (prm *prmMatchRepository) matchesDockerReference(image private.UnparsedImage, signatureDockerReference string) bool {
intended, signature, err := parseImageAndDockerReference(image, signatureDockerReference)
if err != nil {
return false
Expand All @@ -85,7 +85,7 @@ func parseDockerReferences(s1, s2 string) (reference.Named, reference.Named, err
return r1, r2, nil
}

func (prm *prmExactReference) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool {
func (prm *prmExactReference) matchesDockerReference(image private.UnparsedImage, signatureDockerReference string) bool {
intended, signature, err := parseDockerReferences(prm.DockerReference, signatureDockerReference)
if err != nil {
return false
Expand All @@ -97,7 +97,7 @@ func (prm *prmExactReference) matchesDockerReference(image types.UnparsedImage,
return signature.String() == intended.String()
}

func (prm *prmExactRepository) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool {
func (prm *prmExactRepository) matchesDockerReference(image private.UnparsedImage, signatureDockerReference string) bool {
intended, signature, err := parseDockerReferences(prm.DockerRepository, signatureDockerReference)
if err != nil {
return false
Expand Down Expand Up @@ -141,7 +141,7 @@ func (prm *prmRemapIdentity) remapReferencePrefix(ref reference.Named) (referenc
return newParsedRef, nil
}

func (prm *prmRemapIdentity) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool {
func (prm *prmRemapIdentity) matchesDockerReference(image private.UnparsedImage, signatureDockerReference string) bool {
intended, signature, err := parseImageAndDockerReference(image, signatureDockerReference)
if err != nil {
return false
Expand Down
Loading

0 comments on commit 81f5e6d

Please sign in to comment.