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

Sugar for common GitHub Actions (and other CI/CD workflows) #2691

Open
znewman01 opened this issue Feb 6, 2023 · 3 comments
Open

Sugar for common GitHub Actions (and other CI/CD workflows) #2691

znewman01 opened this issue Feb 6, 2023 · 3 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@znewman01
Copy link
Contributor

(This is written about GitHub so we have a specific example, but applies to all future CI/CD providers!)

Right now, you need to deal with a menagerie of flags to verify GitHub actions workflows (cosign verify-blob):

      --certificate-github-workflow-name string                                                  contains the workflow claim from the GitHub OIDC Identity token that contains the name of the executed workflow.
      --certificate-github-workflow-ref string                                                   contains the ref claim from the GitHub OIDC Identity token that contains the git ref that the workflow run was based upon.
      --certificate-github-workflow-repository string                                            contains the repository claim from the GitHub OIDC Identity token that contains the repository that the workflow run was based upon
      --certificate-github-workflow-sha string                                                   contains the sha claim from the GitHub OIDC Identity token that contains the commit SHA that the workflow run was based upon.
      --certificate-github-workflow-trigger string                                               contains the event_name claim from the GitHub OIDC Identity token that contains the name of the event that triggered the workflow run

We've talked about simplifying this (CC @haydentherapper) by splitting into specific verify-github subcommands. This helps a bit, but still requires values for many (all?) of these fields. That's required for full power for sure.

But @sudo-bmitch points out that there's a really common query we can optimize for: did this signature come from a workflow in "its" GitHub repository? This would boil down to something like just --certificate-github-workflow-repository.

Need to be careful here but I think if we're clear about what it's checking, we can make this common use case really nice! Let's think through this if we're building a verify-github command.

@znewman01
Copy link
Contributor Author

znewman01 commented Mar 19, 2023

In #2805 @philpennock makes some good, related points: namely, we may want to do this for other code forges. Please check out that issue for a proposed syntax as well.

How will we get from here to there (you can ignore if you're not interested in implementation details)?

First, the way that we check predicates like those from the --certificate-* flags is starting to get a little hairy. I'm somewhat nervous to add much more complexity there. Instead, what I'd really like to see is a unified interface to Verify that takes in a "policy" or "predicate". This will prevent the construction of shoot-yourself-in-the-foot predicates (e.g., failing to specify the OIDC issuer—like #1947 but enforced deeper in the stack) but also allow more-complicated assertions (like requested in #2630 or #2279) and even support new Fulcio features without needing a new Cosign release (#2719), possibly by passing in a Rego or CUE policy. This also plays really nicely with the goals of #2804 and will make error messages better.

Then, once that's tamed, the existing flags can "compile" to a predicate. This compilation process will make it really easy to add "sugar" that prevents common antipatterns, support testability and debuggability, and allows consistency across verification commands.

CC @ivanayov as this issue is similar to those you've been working on; it might be a fun one to take a look at—if you're interested, happy to dig into it with you.

CC @asraa and @haydentherapper who have been interested in related issues in the past

@haydentherapper
Copy link
Contributor

Love this idea! As Phil said, what you want is to know that what you're verifying came from some repository - You shouldn't need to know the exact structure of the subject and issuer.

cc @laurentsimon you might be interested in this too for slsa-verifier

@lcarva
Copy link
Contributor

lcarva commented Jul 6, 2023

I really like the idea of moving towards a policy-based approach because of the flexibility it adds. Sigstore could even provide a set of canned policies (or policy templates) for different use cases.

One of the things to consider here is how the certificate data will be available as an input to the policy. Currently, verify-attestation passes the base64 decoded payload of the envelope. This enables a rego policy like this:

package signature

default allow := false

allow {
	input.predicateType == "https://slsa.dev/provenance/v0.2"
}

In other words, input is the attestation statement (https://in-toto.io/Statement/v0.1).

If we want policies to evaluate the certificate, we need to pass that to the policy. There are different approaches that come to mind:

  1. Stuff the certificate information under data, leaving input as is. This certainly seems unusual. The certificate information is, after all, an input.
  2. Change input to be a list of documents. One item is the attestation statement, the others are certificates. This is how conftest handles this limitation.
  3. Change input to be an umbrella document. For example, input.statement refers to the statement, and
    input.certificates is a list of the certificates.
  4. Change cosign to take an additional policy to be applied just for the certificates. Then, change cosign to call opa multiple times, one for the statement, and one for each certificate, using the corresponding policy. This may be tricky when there are multiple certificates and the user only cares about one of them. Not sure how realistic this scenario is though.

I like option 3, or a variation of it, because it allows us to add attributes in the future without breaking the format.

On the Enterprise Contract side, we chose a variation of 3. But in our case, we combine verify and verify-attestation into a single command. input contains the certificates for the image signature, the statements, and the certificates for each statement. Something like this:

input: 
  image:
    signatures:
      - {... cert ...}
  attestations:
    - predicateType: ...
      signatures:
        - {... cert ...}

It would be really great if we could come up with an approach where a certain policy could work with cosign, Enterprise Contract, sigstore/policy-controller, etc...

The benefit for cosign is that the same policy could be used in the verify and the verify-attestation commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants