search

kubewarden / policy-server

Webhook server that evaluates WebAssembly policies to validate Kubernetes requests
https://kubewarden.io
Apache License 2.0
138 stars 18 forks source link

EPIC: verification of policies signed using sigstore's keyless signing #142

Closed flavio closed 2 years ago

flavio commented 2 years ago

Policy server must be able to verify policies that have been signed using Sigstore's keyless signing.

The verification requires the user to provide a list of email addresses that are expected to be inside of the certificate issued by Fulcio.

Implementation details

The verification.yaml file should be enriched to have also another top level key: cert_emails.

This is how the file will look like:

---
verification_keys: #optional
  key-name-irrelevant: |
            -----BEGIN PUBLIC KEY-----
            MBFKHFDGHKIJH0CAQYIKoZIzj0DAQcDQgAEX0HFTtCfTtPmkx5p1RbDE6HJSGAVD
            BVDF6SKFSF87AASUspkQsN3FO4iyWodCy5j3o0CdIJD/KJHDJFHDFIu6sA==
            -----END PUBLIC KEY-----
  another-key: |
            -----BEGIN PUBLIC KEY-----
            AUYSDHKJFGSHGSHCAQYIKoZIzj0DAQcDQgAEX0HFTtCfTtPmkx5p1RbtwDE1EVzu
            wjQs1cCRKb5Pz/yUspkQsN3FO4iyWodCy5j3o0CdIJD/1gvq98pf4IG9tA==
            -----END PUBLIC KEY-----
cert_emails: #optional 
- flavio@foo.bar.com   
- victor@foo.bar.com
verification_annotations: # optional
  env: prod
  foo: bar

All the cert_emails mentioned must be found inside of the policy signatures. This is the same behaviour of multiple verification_keys.

The following table shows the behaviour of the verification check, depending on what the user provides

verification keys cert emails annotations outcome
✅ ❌ ❌ each policy must be signed with all the specified verification keys
✅ ✅ ❌ each policy must be signed with all the specified verification keys and with all the specified emails
✅ ✅ ✅ each policy must be signed with all the specified verification keys and with all the specified emails; plus each signature must satisfy all the annotations provided by the user
✅ ❌ ✅ each policy must be signed with all the specified verification keys; plus each signature must satisfy all the annotations provided by the user
❌ ✅ ✅ each policy must be signed with all the specified emails; plus each signature must satisfy all the annotations provided by the user
❌ ❌ ✅ I don't think scenario makes any sense, we should error out when the configuration looks like that
viccuad commented 2 years ago

Sounds perfect. For the case of only providing annotations, IIRC the current implementation should just go through without verifying anything as expected and without errors. We can fail when deserializing instead. I wonder what's better UX, if people are going to comment the verification_annotations if they comment keys or emails when testing.

flavio commented 2 years ago

Thinking about the implementation, the struct holding the contents of verification_keys should have keys, emails and annotations as Option<>.

I think we should write some check against the object we just deserialized and then error if only annotations are provided. I prefer to fail because providing only annotations would lead to accept all policies. That gives a false sense of security, which is not what someone providing an explicit configuration for the verification feature expects.

viccuad commented 2 years ago

after discussion today, I would like to go with something as:

allOf: # all info listed here needs to be verified for accepting
  pub_keys:
    key-name-irrelevant: |
              -----BEGIN PUBLIC KEY-----
              MBFKHFDGHKIJH0CAQYIKoZIzj0DAQcDQgAEX0HFTtCfTtPmkx5p1RbDE6HJSGAVD
              BVDF6SKFSF87AASUspkQsN3FO4iyWodCy5j3o0CdIJD/KJHDJFHDFIu6sA==
              -----END PUBLIC KEY-----
    another-key: |
              -----BEGIN PUBLIC KEY-----
              AUYSDHKJFGSHGSHCAQYIKoZIzj0DAQcDQgAEX0HFTtCfTtPmkx5p1RbtwDE1EVzu
              wjQs1cCRKb5Pz/yUspkQsN3FO4iyWodCy5j3o0CdIJD/1gvq98pf4IG9tA==
              -----END PUBLIC KEY-----

anyOf: # at least 1 of all info listed here needs to be verified for accepting
  keyless:
    - oidc_issuer: https://token.actions.githubusercontent.com # default tuple, we add it to our minimum config
      provenance: https://github.com/kubewarden
    - oidc_issuer: https://token.actions.githubusercontent.com
      provenance: https://github.com/kubewarden/pod-privileged-policy
    - oidc_issuer: https://oauth2.suse.com/auth
      provenance: kubewarden@suse.de
  annotations:
    env: prod
    foo: bar

That is, add a root level that can be one of allOf, anyOf, or both, which inside contain any number of pub_keys/keyless/annotations.

flavio commented 2 years ago

Let's move the discussion about the verification.yaml file over there: https://github.com/kubewarden/kubewarden-controller/pull/147

I gotta say, I like the idea of allOf and anyOf; that makes things really explicit.

viccuad commented 2 years ago

Since there's a lot of work that comes from consuming the big refactor of sigstore-rs >= 0.2.0, I will start noting here the Epic.

Almost all changes are for policy-fetcher, and it may take a couple of days to settle. So I have created the branch feat-sigstore-0.2 in policy-fetcher, and I will open PRs against that branch.

flavio commented 2 years ago

closing, because we have broken down it into smaller epics and their sub-tasks