Merge pull request #355 from jlebon/pr/policy-eval

PolicyContext: add new `RequireSignatureVerification` method

Author: mtrmac

image/signature/policy_eval.go

@@ -65,6 +65,10 @@ type PolicyRequirement interface {
 	// 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 private.UnparsedImage) (bool, error)
+
+	// verifiesSignatures returns true if and only if the requirement performs cryptographic
+	// signature verification on the entire contents of the image before allowing it.
+	verifiesSignatures() bool
 }
 
 // PolicyReferenceMatch specifies a set of image identities accepted in PolicyRequirement.
@@ -79,8 +83,9 @@ type PolicyReferenceMatch interface {
 // PolicyContext encapsulates a policy and possible cached state
 // for speeding up its evaluation.
 type PolicyContext struct {
-	Policy *Policy
-	state  policyContextState // Internal consistency checking
+	Policy        *Policy
+	state         policyContextState // Internal consistency checking
+	requireSigned bool
 }
 
 // policyContextState is used internally to verify the users are not misusing a PolicyContext.
@@ -132,6 +137,13 @@ func policyIdentityLogName(ref types.ImageReference) string {
 	return ref.Transport().Name() + ":" + ref.PolicyConfigurationIdentity()
 }
 
+// RequireSignatureVerification modifies policy requirement handling. If passed
+// `true`, at least one policy requirement which performs signature verification
+// on the entire image contents must be present.
+func (pc *PolicyContext) RequireSignatureVerification(val bool) {
+	pc.requireSigned = val
+}
+
 // requirementsForImageRef selects the appropriate requirements for ref.
 func (pc *PolicyContext) requirementsForImageRef(ref types.ImageReference) PolicyRequirements {
 	// Do we have a PolicyTransportScopes for this transport?
@@ -278,6 +290,7 @@ func (pc *PolicyContext) IsRunningImageAllowed(ctx context.Context, publicImage
 		return false, PolicyRequirementError("List of verification policy requirements must not be empty")
 	}
 
+	wasSignatureVerified := false
 	for reqNumber, req := range reqs {
 		// FIXME: supply state
 		allowed, err := req.isRunningImageAllowed(ctx, image)
@@ -286,7 +299,15 @@ func (pc *PolicyContext) IsRunningImageAllowed(ctx context.Context, publicImage
 			return false, err
 		}
 		logrus.Debugf(" Requirement %d: allowed", reqNumber)
+		if req.verifiesSignatures() {
+			wasSignatureVerified = true
+		}
 	}
+
+	if pc.requireSigned && !wasSignatureVerified {
+		return false, PolicyRequirementError(fmt.Sprintf("No signature verification policy found for image %s", policyIdentityLogName(image.Reference())))
+	}
+
 	// We have tested that len(reqs) != 0, so at least one req must have explicitly allowed this image.
 	logrus.Debugf("Overall: allowed")
 	return true, nil

image/signature/policy_eval_baselayer.go

@@ -18,3 +18,7 @@ func (pr *prSignedBaseLayer) isRunningImageAllowed(ctx context.Context, image pr
 	logrus.Errorf("signedBaseLayer not implemented yet!")
 	return false, PolicyRequirementError("signedBaseLayer not implemented yet!")
 }
+
+func (pr *prSignedBaseLayer) verifiesSignatures() bool {
+	return false
+}

image/signature/policy_eval_baselayer_test.go

@@ -23,3 +23,9 @@ func TestPRSignedBaseLayerIsRunningImageAllowed(t *testing.T) {
 	res, err := pr.isRunningImageAllowed(context.Background(), nil)
 	assertRunningRejectedPolicyRequirement(t, res, err)
 }
+
+func TestPRSignedBaseLayerVerifiesSignatures(t *testing.T) {
+	pr, err := NewPRSignedBaseLayer(NewPRMMatchRepository())
+	require.NoError(t, err)
+	require.False(t, pr.verifiesSignatures())
+}

image/signature/policy_eval_signedby.go

@@ -114,3 +114,7 @@ func (pr *prSignedBy) isRunningImageAllowed(ctx context.Context, image private.U
 	}
 	return false, summary
 }
+
+func (pr *prSignedBy) verifiesSignatures() bool {
+	return true
+}

image/signature/policy_eval_signedby_test.go

@@ -286,3 +286,9 @@ func TestPRSignedByIsRunningImageAllowed(t *testing.T) {
 	allowed, err = pr.isRunningImageAllowed(context.Background(), image)
 	assertRunningAllowed(t, allowed, err)
 }
+
+func TestPRSignedByVerifiesSignatures(t *testing.T) {
+	pr, err := NewPRSignedByKeyPath(SBKeyTypeGPGKeys, "fixtures/public-key.gpg", NewPRMMatchExact())
+	require.NoError(t, err)
+	require.True(t, pr.verifiesSignatures())
+}

image/signature/policy_eval_sigstore.go

@@ -432,3 +432,7 @@ func (pr *prSigstoreSigned) isRunningImageAllowed(ctx context.Context, image pri
 	}
 	return false, summary
 }
+
+func (pr *prSigstoreSigned) verifiesSignatures() bool {
+	return true
+}

image/signature/policy_eval_sigstore_test.go

@@ -1053,3 +1053,12 @@ func TestPRSigstoreSignedIsRunningImageAllowed(t *testing.T) {
 	allowed, err = pr.isRunningImageAllowed(context.Background(), image)
 	assertRunningRejectedPolicyRequirement(t, allowed, err)
 }
+
+func TestPRSigstoreSignedVerifiesSignatures(t *testing.T) {
+	pr, err := NewPRSigstoreSigned(
+		PRSigstoreSignedWithKeyPath("fixtures/cosign.pub"),
+		PRSigstoreSignedWithSignedIdentity(NewPRMMatchRepository()),
+	)
+	require.NoError(t, err)
+	require.True(t, pr.verifiesSignatures())
+}

image/signature/policy_eval_simple.go

@@ -20,10 +20,18 @@ func (pr *prInsecureAcceptAnything) isRunningImageAllowed(ctx context.Context, i
 	return true, nil
 }
 
+func (pr *prInsecureAcceptAnything) verifiesSignatures() bool {
+	return false
+}
+
 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 private.UnparsedImage) (bool, error) {
 	return false, PolicyRequirementError(fmt.Sprintf("Running image %s is rejected by policy.", transports.ImageName(image.Reference())))
 }
+
+func (pr *prReject) verifiesSignatures() bool {
+	return false
+}

image/signature/policy_eval_simple_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"testing"
 
+	"github.com/stretchr/testify/require"
 	"go.podman.io/image/v5/internal/testing/mocks"
 	"go.podman.io/image/v5/types"
 )
@@ -56,3 +57,13 @@ func TestPRRejectIsRunningImageAllowed(t *testing.T) {
 	res, err := pr.isRunningImageAllowed(context.Background(), nameOnlyImageMock{})
 	assertRunningRejectedPolicyRequirement(t, res, err)
 }
+
+func TestPRInsecureAcceptAnythingVerifiesSignatures(t *testing.T) {
+	pr := NewPRInsecureAcceptAnything()
+	require.False(t, pr.verifiesSignatures())
+}
+
+func TestPRRejectVerifiesSignatures(t *testing.T) {
+	pr := NewPRReject()
+	require.False(t, pr.verifiesSignatures())
+}

image/signature/policy_eval_test.go

@@ -500,3 +500,77 @@ func assertRunningRejectedPolicyRequirement(t *testing.T, allowed bool, err erro
 	assertRunningRejected(t, allowed, err)
 	assert.IsType(t, PolicyRequirementError(""), err)
 }
+
+func TestPolicyContextRequireSignatureVerification(t *testing.T) {
+	pc, err := NewPolicyContext(&Policy{Default: PolicyRequirements{NewPRReject()}})
+	require.NoError(t, err)
+	defer func() {
+		err := pc.Destroy()
+		require.NoError(t, err)
+	}()
+
+	// Test default value is false
+	assert.False(t, pc.requireSigned)
+
+	// Test setting to true
+	pc.RequireSignatureVerification(true)
+	assert.True(t, pc.requireSigned)
+
+	// Test setting back to false
+	pc.RequireSignatureVerification(false)
+	assert.False(t, pc.requireSigned)
+}
+
+func TestPolicyContextIsRunningImageAllowedWithRequireSigned(t *testing.T) {
+	pc, err := NewPolicyContext(&Policy{
+		Default: PolicyRequirements{NewPRReject()},
+		Transports: map[string]PolicyTransportScopes{
+			"docker": {
+				"docker.io/testing/manifest:insecureOnly": {
+					NewPRInsecureAcceptAnything(),
+				},
+				"docker.io/testing/manifest:insecureWithOther": {
+					NewPRInsecureAcceptAnything(),
+					xNewPRSignedByKeyPath(SBKeyTypeGPGKeys, "fixtures/public-key.gpg", NewPRMMatchRepository()),
+				},
+				"docker.io/testing/manifest:signedOnly": {
+					xNewPRSignedByKeyPath(SBKeyTypeGPGKeys, "fixtures/public-key.gpg", NewPRMMatchRepository()),
+				},
+			},
+		},
+	})
+	require.NoError(t, err)
+	defer func() {
+		err := pc.Destroy()
+		require.NoError(t, err)
+	}()
+
+	// Test with requireSigned=false (default behavior)
+	// insecureAcceptAnything should be accepted
+	img := pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:insecureOnly")
+	res, err := pc.IsRunningImageAllowed(context.Background(), img)
+	assertRunningAllowed(t, res, err)
+
+	// Test with rejectInsecure=true
+	pc.RequireSignatureVerification(true)
+
+	// insecureAcceptAnything only: should be rejected
+	img = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:insecureOnly")
+	res, err = pc.IsRunningImageAllowed(context.Background(), img)
+	assertRunningRejectedPolicyRequirement(t, res, err)
+
+	// insecureAcceptAnything + signed requirement: first requirement has no effect, second is secure and valid
+	img = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:insecureWithOther")
+	res, err = pc.IsRunningImageAllowed(context.Background(), img)
+	assertRunningAllowed(t, res, err)
+
+	// signed requirement only: should work normally
+	img = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:signedOnly")
+	res, err = pc.IsRunningImageAllowed(context.Background(), img)
+	assertRunningAllowed(t, res, err)
+
+	// Test with unsigned image and insecureAcceptAnything + signed requirement: first requirement has no effect, second is secure but rejects
+	img = pcImageMock(t, "fixtures/dir-img-unsigned", "testing/manifest:insecureWithOther")
+	res, err = pc.IsRunningImageAllowed(context.Background(), img)
+	assertRunningRejectedPolicyRequirement(t, res, err)
+}