ostreedev / ostree-rs-ext

Rust library with higher level APIs on top of the core ostree API
Apache License 2.0
79 stars 27 forks source link

containers: Patch skopeo to have `--deny-default-insecure` #79

Open cgwalters opened 3 years ago

cgwalters commented 3 years ago

We should add skopeo copy --deny-default-insecure to implement SignatureSource::ContainerPolicy properly, which tries to deny fetching unsigned images.

I started on this:

diff --git a/signature/policy_eval.go b/signature/policy_eval.go
index edcbf52..dfec8f5 100644
--- a/signature/policy_eval.go
+++ b/signature/policy_eval.go
@@ -131,7 +131,7 @@ func policyIdentityLogName(ref types.ImageReference) string {
 }

 // requirementsForImageRef selects the appropriate requirements for ref.
-func (pc *PolicyContext) requirementsForImageRef(ref types.ImageReference) PolicyRequirements {
+func (pc *PolicyContext) requirementsForImageRef(ref types.ImageReference, allowDefault bool) PolicyRequirements {
    // Do we have a PolicyTransportScopes for this transport?
    transportName := ref.Transport().Name()
    if transportScopes, ok := pc.Policy.Transports[transportName]; ok {
@@ -157,8 +157,11 @@ func (pc *PolicyContext) requirementsForImageRef(ref types.ImageReference) Polic
        }
    }

-   logrus.Debugf(" Using default policy section")
-   return pc.Policy.Default
+   if allowDefault {
+       logrus.Debugf(" Using default policy section")
+       return pc.Policy.Default
+   }
+   return []PolicyRequirement{}
 }

 // GetSignaturesWithAcceptedAuthor returns those signatures from an image
@@ -186,7 +189,7 @@ func (pc *PolicyContext) GetSignaturesWithAcceptedAuthor(ctx context.Context, im
    }()

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

    // FIXME: rename Signatures to UnverifiedSignatures
    // FIXME: pass context.Context
@@ -250,12 +253,7 @@ func (pc *PolicyContext) GetSignaturesWithAcceptedAuthor(ctx context.Context, im
    return res, nil
 }

-// IsRunningImageAllowed returns true iff the policy allows running the 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.
-func (pc *PolicyContext) IsRunningImageAllowed(ctx context.Context, image types.UnparsedImage) (res bool, finalErr error) {
+func (pc *PolicyContext) isRunningImageAllowedImpl(ctx context.Context, image types.UnparsedImage, allowDefault bool) (res bool, finalErr error) {
    if err := pc.changeState(pcReady, pcInUse); err != nil {
        return false, err
    }
@@ -267,7 +265,7 @@ func (pc *PolicyContext) IsRunningImageAllowed(ctx context.Context, image types.
    }()

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

    if len(reqs) == 0 {
        return false, PolicyRequirementError("List of verification policy requirements must not be empty")
@@ -286,3 +284,18 @@ func (pc *PolicyContext) IsRunningImageAllowed(ctx context.Context, image types.
    logrus.Debugf("Overall: allowed")
    return true, nil
 }
+
+// IsRunningImageAllowed returns true iff the policy allows running the 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.
+func (pc *PolicyContext) IsRunningImageAllowed(ctx context.Context, image types.UnparsedImage) (res bool, finalErr error) {
+   return pc.isRunningImageAllowedImpl(ctx, image, true)
+}
+
+// IsRunningImageExplicitlyAllowed returns true iff the policy allows running the image without falling through to the global `default`.
+// This is a variation of IsRunningImageAllowed - see that function for more information.
+func (pc *PolicyContext) IsRunningImageExplictlyAllowed(ctx context.Context, image types.UnparsedImage) (res bool, finalErr error) {
+   return pc.isRunningImageAllowedImpl(ctx, image, false)
+}
diff --git a/signature/policy_eval_test.go b/signature/policy_eval_test.go
index 8cabb3e..77a16d9 100644
--- a/signature/policy_eval_test.go
+++ b/signature/policy_eval_test.go
@@ -135,7 +135,7 @@ func TestPolicyContextRequirementsForImageRefNotRegisteredTransport(t *testing.T
    require.NoError(t, err)
    ref, err := reference.ParseNormalizedNamed("registry.access.redhat.com/rhel7:latest")
    require.NoError(t, err)
-   reqs := pc.requirementsForImageRef(pcImageReferenceMock{"docker", ref})
+   reqs := pc.requirementsForImageRef(pcImageReferenceMock{"docker", ref}, true)
    assert.True(t, &(reqs[0]) == &(pr[0]))
    assert.True(t, len(reqs) == len(pr))

@@ -204,7 +204,7 @@ func TestPolicyContextRequirementsForImageRef(t *testing.T) {

        ref, err := reference.ParseNormalizedNamed(c.input)
        require.NoError(t, err)
-       reqs := pc.requirementsForImageRef(pcImageReferenceMock{c.inputTransport, ref})
+       reqs := pc.requirementsForImageRef(pcImageReferenceMock{c.inputTransport, ref}, true)
        comment := fmt.Sprintf("case %s:%s: %#v", c.inputTransport, c.input, reqs[0])
        // Do not use assert.Equal, which would do a deep contents comparison; we want to compare
        // the pointers. Also, == does not work on slices; so test that the slices start at the
cgwalters commented 3 years ago

This should actually become a feature of https://github.com/cgwalters/container-image-proxy instead. (May require containers/image work)