slsa-framework / slsa-verifier

Verify provenance from SLSA compliant builders
Apache License 2.0
232 stars 50 forks source link

[bug] Delegator workflow verification - should it verify builder ID tag? #474

Open asraa opened 1 year ago

asraa commented 1 year ago

The delegator workflow causes two levels of indirection: TRW -> Delegator workflows

When scoping verification for delegator workflows, I need to do some internal logic changes and checks:

  1. When the delegator workflow signed the certificate, the provenance BuilderID() can be anything - it does not have to match the certificate.
  2. Verify that the delegator workflow was called at a trusted ref (tag), EXCEPT in the example-package and presubmit repository.
  3. The --builder-id MUST be specified, and MUST match the provenance's BuilderID() (the TRW)
  4. Verify that the TRW was called at a trusted ref (tag), EXCEPT in same repository as the TRW

Notice that the check in 4 is generalized - instead of pre-submit in the slsa-github-generator/slsa-framework exception we have the builder's source repository. This allows the TRW owner to create pre-submit workflows against TRW@main in their repositories to test.

WDYT @ianlewis @laurentsimon ?

asraa commented 1 year ago

I'm drafting the verification changes here: https://github.com/slsa-framework/slsa-verifier/compare/main...asraa:slsa-verifier:delegator-verification?expand=1, but I require some fixes (https://github.com/slsa-framework/slsa-github-generator/pull/1619) to actually test this. Then I will add tests.

We have a slight chicken and egg problem about the tag verification tests using delegator@tag and trw@(tag/main) (this has existed for all builders...)

laurentsimon commented 1 year ago

Verify that the TRW was called at a trusted ref (tag), EXCEPT in same repository as the TRW

In pre-submit there is no provenance to verify, so do we need this special case? Do we expect to give users an Go API for their pre-submits?

We have a slight chicken and egg problem about the tag verification tests using delegator@tag and trw@(tag/main) (this has existed for all builders...)

What do you mean?

Changes 1-3 LGTM.

asraa commented 1 year ago

In pre-submit there is no provenance to verify, so do we need this special case? Do we expect to give users an Go API for their pre-submits?

Sorry, i did not mean pre-submit event. I meant a workflow_dispatch in the TRW workflow repo. Otherwise, how will the TRW test it's workflow before distributing it and tagging it for clients? It's a generalization of our exception of using SLSA builders@main in our SLSA repositories.

asraa commented 1 year ago

What do you mean?

I can only test the e2e repo exception for using a non-tagged builder in verifier tests before we tag one for release. (As in, I can't test the path that verifies the builder ref until we have a builder ref)

laurentsimon commented 1 year ago

Got it, that makes sense. There may be use cases where the TRW repo wants to call their own TRW to cut a release too (using a previously-tagged release), so maybe we need a way for the caller to indicate whether it was an e2e test? Something like SLSA_VERIFIER_XXX?

asraa commented 1 year ago

There may be use cases where the TRW repo wants to call their own TRW to cut a release too

Yes... I was thinking there might a case like that. Yes, maybe something that is blatantly labelled insecure like "SLSA_VERIFIER_INSECURE_ALLOW_MAIN` that will allow main ref, but again ONLY when the source repository is the same location as the builder.

laurentsimon commented 1 year ago

SG. In addition, we could also:

  1. verify whether it's running in a GitHub workflow (by checking env variables from GitHub like GITHUB_ENV and CI) which would indicate a test.
  2. ensure the GITHUB_REPOSITORY is the same as the one in the provenance's source URI, which would further indicate a local test
asraa commented 1 year ago

That would restrict them from only being able to test slsa-verifier inside the workflow. It's a little restrictive but maybe that's a good thing! That ensures that someone doesnt download the provenance and use it elsewhere.

laurentsimon commented 1 year ago

yeah. I think being restrictive still gives us the opportunity to remove the restriction in the future (if people really need this flexibility), but the other way around would break folks.

I added a note in the doc issue https://github.com/slsa-framework/slsa-github-generator/issues/1574

asraa commented 1 year ago

How do you think we should handle this before we officially GA delegator? (All current TRW writers are calling @main). Is it possible to use release another 0.0.3 tag (which tags all builders, meanwhile we only care about delegator)

I could allow TRW author "test" workflows who are calling TRW@main to call the delegator @ main... not sure though

asraa commented 1 year ago

Other convo: