From 9ccdd0470c8d124cc560dfb233a20fdae620aa7d Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Sat, 4 Oct 2025 21:03:57 -0400 Subject: [PATCH] PolicyContext: add new `RequireSignatureVerification` method In bootc, we want the ability to assert that signature verification is enforced, but there are no mechanisms for this in the library. Add a new `RequireSignatureVerification` method on the `PolicyContext` object which would allow this. Add a new `isSigned` method on the `PolicyRequirement` interface which then allows `IsRunningImageAllowed` to detect if at least one requirement performed signature verification. Test generation was `Assisted-by: Claude Code v1.0.120`. Part of https://github.com/containers/skopeo/issues/1829. Signed-off-by: Jonathan Lebon --- image/signature/policy_eval.go | 25 ++++++- image/signature/policy_eval_baselayer.go | 4 + image/signature/policy_eval_baselayer_test.go | 6 ++ image/signature/policy_eval_signedby.go | 4 + image/signature/policy_eval_signedby_test.go | 6 ++ image/signature/policy_eval_sigstore.go | 4 + image/signature/policy_eval_sigstore_test.go | 9 +++ image/signature/policy_eval_simple.go | 8 ++ image/signature/policy_eval_simple_test.go | 11 +++ image/signature/policy_eval_test.go | 74 +++++++++++++++++++ 10 files changed, 149 insertions(+), 2 deletions(-) diff --git a/image/signature/policy_eval.go b/image/signature/policy_eval.go index 2d0db05ae4..746298f56f 100644 --- a/image/signature/policy_eval.go +++ b/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 diff --git a/image/signature/policy_eval_baselayer.go b/image/signature/policy_eval_baselayer.go index f310342d10..8f074b47c1 100644 --- a/image/signature/policy_eval_baselayer.go +++ b/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 +} diff --git a/image/signature/policy_eval_baselayer_test.go b/image/signature/policy_eval_baselayer_test.go index 8898cf8167..32660612bf 100644 --- a/image/signature/policy_eval_baselayer_test.go +++ b/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()) +} diff --git a/image/signature/policy_eval_signedby.go b/image/signature/policy_eval_signedby.go index 21ed59494d..149545da8f 100644 --- a/image/signature/policy_eval_signedby.go +++ b/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 +} diff --git a/image/signature/policy_eval_signedby_test.go b/image/signature/policy_eval_signedby_test.go index a1793249ae..f8552e6b57 100644 --- a/image/signature/policy_eval_signedby_test.go +++ b/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()) +} diff --git a/image/signature/policy_eval_sigstore.go b/image/signature/policy_eval_sigstore.go index eb29bd8b57..7069506b81 100644 --- a/image/signature/policy_eval_sigstore.go +++ b/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 +} diff --git a/image/signature/policy_eval_sigstore_test.go b/image/signature/policy_eval_sigstore_test.go index 533a43a52a..f8395c77c5 100644 --- a/image/signature/policy_eval_sigstore_test.go +++ b/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()) +} diff --git a/image/signature/policy_eval_simple.go b/image/signature/policy_eval_simple.go index 4ef35e3ad5..75a83bd579 100644 --- a/image/signature/policy_eval_simple.go +++ b/image/signature/policy_eval_simple.go @@ -20,6 +20,10 @@ 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()))) } @@ -27,3 +31,7 @@ func (pr *prReject) isSignatureAuthorAccepted(ctx context.Context, image private 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 +} diff --git a/image/signature/policy_eval_simple_test.go b/image/signature/policy_eval_simple_test.go index b5b6806b24..a6f880b68d 100644 --- a/image/signature/policy_eval_simple_test.go +++ b/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()) +} diff --git a/image/signature/policy_eval_test.go b/image/signature/policy_eval_test.go index 06c72e1f23..5e4d36854e 100644 --- a/image/signature/policy_eval_test.go +++ b/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) +}