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

Support different verification options per image #6

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

ribbybibby
Copy link

Different images will require different verification options. This PR adds configuration that allows you to define different 'verifiers' for specific image references, or image reference patterns.

At the moment it supports verification by public key, or rekor URL, but should probably be expanded to include roughly the same options as cosign verify.

Different images will require different verification options. This
commit adds configuration that allows you to define different
'verifiers' for specific image references, or image reference patterns.

At the moment it supports verification by public key, or the existing
options, but should be expanded to include all supported options.

Also modifies the response from the provider to include an error
per-image checked, rather than returning any error as a 'system' error.

I've also removed the _invalid suffix from the key returned in the
response when there's an error. The presence of the 'error' field
indicates this better, I think.

Signed-off-by: Rob Best <[email protected]>
@dlorenc
Copy link
Member

dlorenc commented Jan 12, 2022

Cc @developer-guy

An image can have multiple signatures and therefore in some cases you'll
want multiple verifiers for the same images.

Signed-off-by: Rob Best <[email protected]>
Modify the configuration so that multiple verifiers can be associated
directly with an image reference/pattern. Images will only be verified
for the first pattern they match.

This makes it possible to provide multiple verification options for a
specific image pattern/reference but also fall through to a less-specific
pattern (with different verification options) for images that don't
match a more specific pattern.

Signed-off-by: Rob Best <[email protected]>
Checking the count of errors is enough.

Signed-off-by: Rob Best <[email protected]>
@ribbybibby
Copy link
Author

I think we need to take the discussion on this issue into account. Or, even wait for the approach to policy in the sigstore/cosign ecosystem to formalise a bit.

The changes here support the AND case mentioned there where it's possible to assert that a particular image should be verified by all the configured keys.

It doesn't support the OR case where you want to assert that at least one (or two, or three) signatures should be verified, or that certain signatures are optional.

I also don't like that the policy is asserted on the provider here, rather than in the actual policy calling the provider.

Maybe what we need to do is define a list of 'verifiers' on the provider and then return the signatures in the response.

What we actually return would require more thought, but something like this:

{
  "apiVersion": "externaldata.gatekeeper.sh/v1alpha1",
  "kind": "ProviderResponse",
  "response": {
    "items": [
      {
        "key": "foobar:latest",
        "value": {
          "signatures": [
            {
              "verified": true,
              "verifier": {
                "name": "my-key"
                "options": {
                  "key": "/cosign.pub"
                }
              },
              "payload": "<payload>"
            },
            {
              "verified": false,
              "payload": "<payload>"
            }
          ]
        }
      }
    ]
  }
}

This shifts all the responsibility for making policy decisions to the Rego policies, where it belongs. The provider configuration just contains a list of trusted keys/verification options.

With this you could start writing policies that say things like:

  • The image must have at least one verified signature
  • The image must be verified by this particular key
  • The image must be verified by keys A + C, or B + C
  • All the signatures for an image must be verified

What do you think @developer-guy @dlorenc?

@dlorenc
Copy link
Member

dlorenc commented Jan 16, 2022

This shifts all the responsibility for making policy decisions to the Rego policies, where it belongs. The provider configuration just contains a list of trusted keys/verification options.

I like it! That's the direction we've been leaning elsewhere too.

@ribbybibby
Copy link
Author

I've opened sigstore/cosign#1334 in cosign which should make iterating across multiple verifiers much quicker.

@simongottschlag
Copy link

A thought I've had is that this configuration may be quite hard for someone who haven't spent time on the code base.

Could an alternative be to have a more opinionated set of configurations that are enabled by flags? If you need multiple verifiers, maybe you could run multiple instances of the provider instead?

I also think it would be nice if we could provide the KMS key by annotation on either the POD or the namespace and pick it up from here.

I haven't thought this fully through yet, but are my initial thoughts so far. 😊

@ribbybibby
Copy link
Author

@simongottschlag

Could an alternative be to have a more opinionated set of configurations that are enabled by flags? If you need multiple verifiers, maybe you could run multiple instances of the provider instead?

Running multiple providers for multiple keys would be an added management burden and I'm not sure if there's any benefit there. The options to each 'verifier' in the configuration are equivalent to the flags you would pass to an individual provider.

A thought I've had is that this configuration may be quite hard for someone who haven't spent time on the code base.

In my view, the way that you make using the provider simple for people who don't necessarily understand 'how' to check the validity of a signature is for this repo to provide example constraint templates with good input parameters for common use cases.

That way, consumers don't need to think about the implementation too much unless they have some particularly niche use case.

I also think it would be nice if we could provide the KMS key by annotation on either the POD or the namespace and pick it up from here.

This would be possible by comparing the provider response to the annotation on the review object.

Something like:

signatures[_].verified == true
signatures[_].verifier.key == input.review.object.metadata.annotations[annotation]

@ribbybibby
Copy link
Author

ribbybibby commented Jan 20, 2022

I've pushed changes that return the signatures for each image in the response.

I've taken some inspiration from sigstore/cosign#1233 in terms of how to represent the information in the response. I definitely think this provider could benefit from some agreement upstream on how to represent signatures/attestations to policies, though.

The signatures that the Rego gets look like this:

{
  "id": {
    "iss": "https://github.com/login/oauth",
    "sub": "[email protected]"
  },
  "payload": {
    "critical": {
      "identity": {
        "docker-reference": "my-registry/foobar"
      },
      "image": {
        "docker-manifest-digest": "sha256:3b2c249b772868bcfba7e4389dffe22fe5a7074c2d4b3cff777e860aacf76fbc"
      },
      "type": "cosign container image signature"
    },
    "optional": null
  },
  "verified": true
}

The id looks like this with a key:

"id": {
  "key": "LS0tLS1CRUdJTiBQVUJMSUMgS0VZLS0tLS0KTUZrd0V3WUhLb1pJemowQ0FRWUlLb1pJemowREFRY0RRZ0FFbGNvM3kvUjIzaXI3NnBDWnVWRTNPdjdUMC9kawo0Z0plNFd5Tk4xRzh5VmhpelAvaTc1MUdrZHZqTmZMQmZoVnFkc2xSVzhHWGRXakNqK2lXNTY1cHJnPT0KLS0tLS1FTkQgUFVCTElDIEtFWS0tLS0tCg=="
}

I've updated the example policy so it basically does this to check that at least one of the signatures is verified:

verified(value) {
  value.signatures[_].verified == true
}

Other, more specific policies, could check the subject/issuer or key values though.

@simongottschlag
Copy link

@ribbybibby

Could an alternative be to have a more opinionated set of configurations that are enabled by flags? If you need multiple verifiers, maybe you could run multiple instances of the provider instead?

Running multiple providers for multiple keys would be an added management burden and I'm not sure if there's any benefit there. The options to each 'verifier' in the configuration are equivalent to the flags you would pass to an individual provider.

After thinking about it, I do agree with you. 👍

A thought I've had is that this configuration may be quite hard for someone who haven't spent time on the code base.

In my view, the way that you make using the provider simple for people who don't necessarily understand 'how' to check the validity of a signature is for this repo to provide example constraint templates with good input parameters for common use cases.

That way, consumers don't need to think about the implementation too much unless they have some particularly niche use case.

Yeah, I'm taking back the idea about running the service multiple times. Better to have the complexity in the code and try to make it easy for the consumers through examples.

I also think it would be nice if we could provide the KMS key by annotation on either the POD or the namespace and pick it up from here.

This would be possible by comparing the provider response to the annotation on the review object.

Something like:

signatures[_].verified == true
signatures[_].verifier.key == input.review.object.metadata.annotations[annotation]

I think this is where I'm still not completely agree with you. I think it would be nice to be able to use the provider like the cosign cli and where you pass --key to the CLI we should be able to pass the kms uri to the provider from an annotation (not sure if it should be on the namespace or pod though).

In that case, if a key (kms uri) is passed to the provider, it should be used instead of any of the pre-configured.

What's your thoughts about that?

@ribbybibby
Copy link
Author

we should be able to pass the kms uri to the provider from an annotation (not sure if it should be on the namespace or pod though)

I think there's a technical limitation here in that the external data request only supports a list of keys (a list of images), so there's no real way to 'pass' the KMS URI to the provider. The provider also has no idea which object is being reviewed, so it can't extract an annotation from it.

I don't know if there are any discussions out there around allowing more information in the request. Or good reasons it's implemented the way it is.

I also have a few thoughts on the idea of providing the key as an annotation:

  • On the pod, you're opening yourself up to an attack where someone who can run a pod can run a malicious image by modifying the key ref to a key you don't actually trust.
  • On the pod and the namespace, it's a lot of a repetition. I think typically you would have verification policies about particular images defined centrally. Is there many cases where the policy about a given image would be different between namespaces or pods?
    • Maybe in a multi-tenant environment between different teams? Even in that case you could represent different policies in constraints with different namespace selectors.

Signed-off-by: Rob Best <[email protected]>
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

Successfully merging this pull request may close these issues.

3 participants