Skip to content

Commit 11c5783

Browse files
committed
PolicyContext: add new SetRejectInsecure 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 `SetRejectInsecure` method on the `PolicyContext` object which would allow this. Since this only changes the behaviour of the `insecureAcceptAnything` policy requirement, rather than extending the policy requirement interface, I went with a filtering approach directly in `IsRunningImageAllowed()`. Test generation was `Assisted-by: Claude Code v1.0.120`. Part of containers/skopeo#1829. Signed-off-by: Jonathan Lebon <[email protected]>
1 parent 92222dc commit 11c5783

File tree

2 files changed

+138
-2
lines changed

2 files changed

+138
-2
lines changed

image/signature/policy_eval.go

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,9 @@ type PolicyReferenceMatch interface {
7979
// PolicyContext encapsulates a policy and possible cached state
8080
// for speeding up its evaluation.
8181
type PolicyContext struct {
82-
Policy *Policy
83-
state policyContextState // Internal consistency checking
82+
Policy *Policy
83+
state policyContextState // Internal consistency checking
84+
rejectInsecure bool
8485
}
8586

8687
// policyContextState is used internally to verify the users are not misusing a PolicyContext.
@@ -132,6 +133,13 @@ func policyIdentityLogName(ref types.ImageReference) string {
132133
return ref.Transport().Name() + ":" + ref.PolicyConfigurationIdentity()
133134
}
134135

136+
// SetRejectInsecure modifies insecure policy requirement handling. If
137+
// passed `true`, policy checking by IsRunningImageAllowed will ignore the
138+
// "insecureAcceptAnything" policy type.
139+
func (pc *PolicyContext) SetRejectInsecure(val bool) {
140+
pc.rejectInsecure = val
141+
}
142+
135143
// requirementsForImageRef selects the appropriate requirements for ref.
136144
func (pc *PolicyContext) requirementsForImageRef(ref types.ImageReference) PolicyRequirements {
137145
// Do we have a PolicyTransportScopes for this transport?
@@ -274,6 +282,24 @@ func (pc *PolicyContext) IsRunningImageAllowed(ctx context.Context, publicImage
274282
logrus.Debugf("IsRunningImageAllowed for image %s", policyIdentityLogName(image.Reference()))
275283
reqs := pc.requirementsForImageRef(image.Reference())
276284

285+
// Create a filtered version of the policy which always ignores
286+
// insecureAcceptAnything. In practice, it's likely the only requirement, so
287+
// then we'd fall in the `len(reqs) == 0` case just below. In theory, it's
288+
// possible to have additional requirements combined with insecureAcceptAnything
289+
// even if not useful (it'd be the equivalent of a `&& true`).
290+
if pc.rejectInsecure {
291+
var filteredReqs PolicyRequirements
292+
for reqNumber, req := range reqs {
293+
_, ok := req.(*prInsecureAcceptAnything)
294+
if ok {
295+
logrus.Debugf("Requirement %d: ignoring insecureAcceptAnything by runtime request", reqNumber)
296+
continue
297+
}
298+
filteredReqs = append(filteredReqs, req)
299+
}
300+
reqs = filteredReqs
301+
}
302+
277303
if len(reqs) == 0 {
278304
return false, PolicyRequirementError("List of verification policy requirements must not be empty")
279305
}

image/signature/policy_eval_test.go

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,3 +497,113 @@ func assertRunningRejectedPolicyRequirement(t *testing.T, allowed bool, err erro
497497
assertRunningRejected(t, allowed, err)
498498
assert.IsType(t, PolicyRequirementError(""), err)
499499
}
500+
501+
func TestPolicyContextSetRejectInsecure(t *testing.T) {
502+
pc, err := NewPolicyContext(&Policy{Default: PolicyRequirements{NewPRReject()}})
503+
require.NoError(t, err)
504+
defer func() {
505+
err := pc.Destroy()
506+
require.NoError(t, err)
507+
}()
508+
509+
// Test default value is false
510+
assert.False(t, pc.rejectInsecure)
511+
512+
// Test setting to true
513+
pc.SetRejectInsecure(true)
514+
assert.True(t, pc.rejectInsecure)
515+
516+
// Test setting back to false
517+
pc.SetRejectInsecure(false)
518+
assert.False(t, pc.rejectInsecure)
519+
}
520+
521+
func TestPolicyContextIsRunningImageAllowedWithRejectInsecure(t *testing.T) {
522+
pc, err := NewPolicyContext(&Policy{
523+
Default: PolicyRequirements{NewPRReject()},
524+
Transports: map[string]PolicyTransportScopes{
525+
"docker": {
526+
"docker.io/testing/manifest:insecureOnly": {
527+
NewPRInsecureAcceptAnything(),
528+
},
529+
"docker.io/testing/manifest:insecureWithOther": {
530+
NewPRInsecureAcceptAnything(),
531+
xNewPRSignedByKeyPath(SBKeyTypeGPGKeys, "fixtures/public-key.gpg", NewPRMMatchRepository()),
532+
},
533+
"docker.io/testing/manifest:signedOnly": {
534+
xNewPRSignedByKeyPath(SBKeyTypeGPGKeys, "fixtures/public-key.gpg", NewPRMMatchRepository()),
535+
},
536+
},
537+
},
538+
})
539+
require.NoError(t, err)
540+
defer func() {
541+
err := pc.Destroy()
542+
require.NoError(t, err)
543+
}()
544+
545+
// Test with rejectInsecure=false (default behavior)
546+
// insecureAcceptAnything should be accepted
547+
img := pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:insecureOnly")
548+
res, err := pc.IsRunningImageAllowed(context.Background(), img)
549+
assertRunningAllowed(t, res, err)
550+
551+
// Test with rejectInsecure=true
552+
pc.SetRejectInsecure(true)
553+
554+
// insecureAcceptAnything only: should be rejected (empty requirements)
555+
img = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:insecureOnly")
556+
res, err = pc.IsRunningImageAllowed(context.Background(), img)
557+
assertRunningRejectedPolicyRequirement(t, res, err)
558+
559+
// insecureAcceptAnything + signed requirement: should use signed requirement
560+
img = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:insecureWithOther")
561+
res, err = pc.IsRunningImageAllowed(context.Background(), img)
562+
assertRunningAllowed(t, res, err)
563+
564+
// signed requirement only: should work normally
565+
img = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:signedOnly")
566+
res, err = pc.IsRunningImageAllowed(context.Background(), img)
567+
assertRunningAllowed(t, res, err)
568+
569+
// Test with unsigned image and insecureAcceptAnything + signed requirement
570+
img = pcImageMock(t, "fixtures/dir-img-unsigned", "testing/manifest:insecureWithOther")
571+
res, err = pc.IsRunningImageAllowed(context.Background(), img)
572+
assertRunningRejectedPolicyRequirement(t, res, err)
573+
}
574+
575+
func TestPolicyContextRejectInsecureFilteringLogic(t *testing.T) {
576+
pc, err := NewPolicyContext(&Policy{
577+
Default: PolicyRequirements{NewPRReject()},
578+
Transports: map[string]PolicyTransportScopes{
579+
"docker": {
580+
"docker.io/testing/manifest:multipleInsecure": {
581+
NewPRInsecureAcceptAnything(),
582+
NewPRInsecureAcceptAnything(),
583+
NewPRReject(),
584+
},
585+
"docker.io/testing/manifest:allInsecure": {
586+
NewPRInsecureAcceptAnything(),
587+
NewPRInsecureAcceptAnything(),
588+
},
589+
},
590+
},
591+
})
592+
require.NoError(t, err)
593+
defer func() {
594+
err := pc.Destroy()
595+
require.NoError(t, err)
596+
}()
597+
598+
pc.SetRejectInsecure(true)
599+
600+
// Test filtering multiple insecureAcceptAnything requirements but keeping other requirements
601+
img := pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:multipleInsecure")
602+
res, err := pc.IsRunningImageAllowed(context.Background(), img)
603+
assertRunningRejectedPolicyRequirement(t, res, err) // Should fail because only prReject remains
604+
605+
// Test filtering all requirements results in empty requirements error
606+
img = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:allInsecure")
607+
res, err = pc.IsRunningImageAllowed(context.Background(), img)
608+
assertRunningRejectedPolicyRequirement(t, res, err)
609+
}

0 commit comments

Comments
 (0)