Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cosigned: create a CRD to declare signed image policies per cluster #594

Closed
hectorj2f opened this issue Aug 30, 2021 · 17 comments
Closed

cosigned: create a CRD to declare signed image policies per cluster #594

hectorj2f opened this issue Aug 30, 2021 · 17 comments
Assignees

Comments

@hectorj2f
Copy link
Contributor

hectorj2f commented Aug 30, 2021

Description

The idea is to create a cluster-scoped CR to specify a signed image policy on their kubernetes cluster. This policy can be configured to verify signed images from specified patterns.

Based on our implementation of a similar admission controller, we expose a CR with several fields:

publicKeys: defines the list of valid public keys.
registries: defines the link between registries and defined keys where to apply these rules.
unmatchedImagePolicy: specifies the behavior of the admission controller (Allow|Deny) whenever the policy is satisfied or not.

An example of a Policy:

kind: ClusterImagePolicy
metadata:
 name: image-policy
spec:
 unmatchedImagePolicy: <Allow/Deny>
 publicKeys:
 - name: cosign-key-1
   publicKey: <public-key>
 registries:
 - pattern: my.registry.org/library/*
   keys:
   - name: cosign-key-1

I cc @stormqueen1990 and @kkavitha @elfotografo007 as creators of this approach. So they can chime in here.

@dlorenc
Copy link
Member

dlorenc commented Aug 30, 2021

For pattern - what glob matching semantics do we envision? Would it be simpler to limit to "prefix"? Or are we envisioning ** and * stuff for multiple nested "directories"?

@hectorj2f
Copy link
Contributor Author

Yes, we rely on github.com/gobwas/glob, so any pattern you could think it'll work here. Maybe we could limit it to a simpler use case for now.

@hectorj2f hectorj2f self-assigned this Aug 31, 2021
@hectorj2f
Copy link
Contributor Author

@stormqueen1990 and @kkavitha @elfotografo007 Please, could you update the issue with the latest changes to the spec ?

@DennyHoang
Copy link
Contributor

DennyHoang commented Oct 7, 2021

Updated proposal spec is:

---
kind: ClusterImagePolicy
metadata:
  name: image-policy
spec:
  verification:
    exclude:
      resources:
        namespaces:
        - kube-system
    keys:
    - name: cosign-key-1
      publicKey: |
        -- COSIGN PUBLIC KEY ---
    images:
    - namePattern: registry.example.com/signed-images/*
      keys:
      - name: cosign-key-1

We have added have added a verification section with the option to exclude verification on namespaces.

We have renamed the following keys:

  • publicKeys to keys.
  • pattern to namePattern.
  • registries to images

We have removed the unmatchedImagePolicy for now in the spec, however it is an important feature to keep in mind as we want operators to be able to allow images that do not match any patterned policies defined.

For example, do we foresee a lot of fine-tuning from the operator for what they want such as allowlisting particular paths to not go through the verification flow? If this is the case, just having unmatchedImagePolicy defined in the spec at the top level may not allow enough configuration.

@hectorj2f
Copy link
Contributor Author

It sounds good to me @DennyHoang. I suggest you open a PR with the proposed changes to show the benefits of using this CRD.

@mattmoor
Copy link
Member

Generally +1 on the direction of moving the configuration out of the verification-key secret.

A grab bag of other things:

(cc @dekkagaijin this is increasingly feeling like we'll need the library work to be further along!)


One meta concern I have is how conflict resolution is handled. Documentation is ofc one way to solve this, but if we can use more defensive API design than even better! An example of this would be using the registry domain as the name for custom objects. This has the downside of requiring all configuration for a registry (yikes DockerHub) to be contained within a single CRD, but the upside that there is zero ambiguity which CRD handles a given registry hostname.


Another thought on the above:

    exclude:
     resources:
       namespaces:
       - kube-system

Are you thinking this would replace the namespace selector? We should be careful about removing that entirely because a cycle here can really blow up a cluster (what if the webhook pod's node is pre-empted and it isn't available to validate its own recreation?).


I think we should probably start a doc enumerating key scenarios (perhaps @mlieberman85 @blackcat-lin can help provide a "voice of the user" to keep us honest?), and then sketch a more formal API spec where we spell out the fields and semantics. I'd be happy to collaborate if we want to get a group of interested folks spun up around this?

@hectorj2f
Copy link
Contributor Author

The idea is to deny by default following a similar approach to NetworkPolicies. Obviously if you don't have your containers signed, then you should not create a policy.

Another possible thought would be to include a include expression.

@hectorj2f
Copy link
Contributor Author

hectorj2f commented Oct 29, 2021

what if the webhook pod's node is pre-empted and it isn't available to validate its own recreation?)

You should have multiple replicas, or delete the webhook in such a situation, that is also done as an emergency recovery by gatekeeper.

Are you thinking this would replace the namespace selector?

Yes, the intention is to be more explicit about that. You could also exclude resource types such as DaemonSets, ...

@mattmoor
Copy link
Member

You should have multiple replicas, or delete the webhook in such a situation, that is also done as an emergency recovery by gatekeeper.

Yeah, there are solutions, these are just things we need to think though and (in cases like this) be intentional about.

@mlieberman85
Copy link
Contributor

@mattmoor I've also been talking to the folks at Kyverno on some of this stuff as well. At a high level, the main things are we want an easy way to add/change/remove keys from any sort of admission controller, e.g. via a configmap or some other store.

The primary reason for this is inevitably those keys might be used by more than just cosigned and we want to make sure those keys are easily accessible to other components. An example of this is when running a "secure software factory" and you will be validating signatures at both the admission controller level and inside of build/CI pods.

@hectorj2f
Copy link
Contributor Author

@mlieberman85 We initially started by being explicit defining the list of keys as part of the policy definition. But those could also be references to existing secrets secretRef too.

@hectorj2f
Copy link
Contributor Author

@mattmoor @mlieberman85 Our team created a PR #1064 with a first approach, so we could start again the discussions.

@JimBugwadia
Copy link
Contributor

Looks like a number of the items discussed above and future considerations (match/exclude, HA, fail-closed vs open, external data, wildcarding, etc.) have been addressed for Kyverno along with other features like support for attestations.

Does it make sense to explore how to best reuse?

If a stand-alone admission controller is desirable, perhaps we can consider a library approach like was done for Pod Security Admission, so we are not replicating work with minor variations.

@hectorj2f @dlorenc @mattmoor - would love to discuss thoughts around this.

@mattmoor
Copy link
Member

@JimBugwadia IMO yeah, for sure.

I think one of the goals with #666 and @dekkagaijin follow-up work for signing are basically trying to improve the "cosign-as-an-SDK" story. My hope would be that folks can consume the cosign-level functionality several ways:

  1. Via the cosign CLI,
  2. Via a focused cosigned webhook,
  3. Via more comprehensive tools that integrate cosign as an SDK (alongside much richer policy, e.g. kyverno, OPA, ...).

@hectorj2f
Copy link
Contributor Author

If a stand-alone admission controller is desirable, perhaps we can consider a library approach like was done for Pod Security Admission, so we are not replicating work with minor variations.

@JimBugwadia @mattmoor I 100% agree on that. Let's come with a common vision and as a result a library.

@zosocanuck
Copy link

+1 on extending the public key management of this to allow for external attestation implementations, whereby enterprises may have many public keys requiring frequent policy updates. Keyless verification via Rekor makes sense here as well. Perhaps there is an additional field (e.g. attestationServer) whereby the cosign public key/cert could be sent during verification. While you are going down this path one may need to sign the policy so key entries could not be tampered.

@hectorj2f
Copy link
Contributor Author

Closing this issue. We now have an implementation described in: https://docs.google.com/document/d/1gBLEOOHWOmvHVsoJbgGU74GdwA6CGxMRp3MAeEB50l4/edit#. We'll continue evolving this CR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants