CLI: sigstore verify github

[woodruffw]

Closes #322.

[woodruffw]

Here's the sigstore verify github CLI, in its current state:

usage: sigstore verify github [-h] [--certificate FILE] [--signature FILE]
                              [--rekor-bundle FILE] --cert-identity IDENTITY
                              [--require-rekor-offline]
                              [--workflow-trigger EVENT] [--workflow-sha SHA]
                              [--workflow-name NAME]
                              [--workflow-repository REPO]
                              [--workflow-ref REF] [--staging]
                              [--rekor-url URL] [--rekor-root-pubkey FILE]
                              [--certificate-chain FILE]
                              FILE [FILE ...]

optional arguments:
  -h, --help            show this help message and exit

Verification inputs:
  --certificate FILE, --cert FILE
                        The PEM-encoded certificate to verify against; not
                        used with multiple inputs (default: None)
  --signature FILE      The signature to verify against; not used with
                        multiple inputs (default: None)
  --rekor-bundle FILE   The offline Rekor bundle to verify with; not used with
                        multiple inputs (default: None)
  FILE                  The file to verify

Verification options:
  --cert-identity IDENTITY
                        The identity to check for in the certificate's Subject
                        Alternative Name (default: None)
  --require-rekor-offline
                        Require offline Rekor verification with a bundle;
                        implied by --rekor-bundle (default: False)
  --workflow-trigger EVENT
                        The GitHub Actions event name that triggered the
                        workflow (default: None)
  --workflow-sha SHA    The `git` commit SHA that the workflow run was invoked
                        with (default: None)
  --workflow-name NAME  The name of the workflow that was triggered (default:
                        None)
  --workflow-repository REPO
                        The repository slug that the workflow was triggered
                        under (default: None)
  --workflow-ref REF    The `git` ref that the workflow was invoked with
                        (default: None)

Sigstore instance options:
  --staging             Use sigstore's staging instances, instead of the
                        default production instances (default: False)
  --rekor-url URL       The Rekor instance to use (conflicts with --staging)
                        (default: https://rekor.sigstore.dev)
  --rekor-root-pubkey FILE
                        A PEM-encoded root public key for Rekor itself
                        (conflicts with --staging) (default: None)
  --certificate-chain FILE
                        Path to a list of CA certificates in PEM format which
                        will be needed when building the certificate chain for
                        the Fulcio signing certificate (default: None)

[woodruffw]

This should now be basically functional, and I can confirm that it works on a signed release of sigstore-python itself:

$ sigstore verify github ~/Downloads/sigstore-0.9.0-py3-none-any.whl \
    --cert-identity 'https://github.com/sigstore/sigstore-python/.github/workflows/release.yml@refs/tags/v0.9.0' \
    --workflow-sha 62865f364852c1842e4ccacc821e3190f9b92eb8 \
    --workflow-trigger release \
    --workflow-name Release \
    --workflow-repository sigstore/sigstore-python \
    --workflow-ref refs/tags/v0.9.0
OK: /Users/william/Downloads/sigstore-0.9.0-py3-none-any.whl

(In this case, I had sigstore-0.9.0-py3-none-any.whl{,.crt,.sig} all downloaded.)

[woodruffw]

cc @jku for thoughts on the CLI structure as well!

In particular, I'd like opinions from everyone on:

1. Does it make sense to have --cert-identity be mandatory, as it currently is? The identity in certs issued from GH Actions OIDC tokens aren't the friendliest, so maybe it makes sense to allow a sufficient set of other claims as an alternative.
2. Does the --workflow-FOO naming scheme seem reasonable? It's a little verbose and I could unambiguously shorten them (e.g. --ref, --sha, --repository).
3. Does it make sense to take other sorts of inputs? For example, is there a context in which it makes sense to take the URL of a GitHub release instead, and then iterate through its attached files to validate each? That would require some careful consideration around error/failure modes.

[woodruffw]

NB: From here to the end is pretty much just a dupe of the body of _verify_identity, so it isn't very DRY of us. It turns out that the two are nearly identical except for how the various verification flags are handled, so I need to think some more about whether it really makes sense for them to have separate functions like this.

[tetsuo-cpp]

Yeah, it feels like a lot of this could be wrapped up into a function. It seems like it's mostly the beginning before the (file, materials) loop that differs so this should separate pretty cleanly.

I'm ok with just leaving it as a TODO though.

[di]

Just some nits, LGTM otherwise.

[tetsuo-cpp]

1. Does it make sense to have --cert-identity be mandatory, as it currently is? The identity in certs issued from GH Actions OIDC tokens aren't the friendliest, so maybe it makes sense to allow a sufficient set of other claims as an alternative.

That seems reasonable. I believe that the identity issued from GH is effectively --repository and --ref combined. I'd have to double check that though.

2. Does the --workflow-FOO naming scheme seem reasonable? It's a little verbose and I could unambiguously shorten them (e.g. --ref, --sha, --repository).

I'm mildly in favour of having the shorter names. It doesn't seem like we're getting much from the workflow prefix and seems straightforward to me.

3. Does it make sense to take other sorts of inputs? For example, is there a context in which it makes sense to take the URL of a GitHub release instead, and then iterate through its attached files to validate each? That would require some careful consideration around error/failure modes.

That'd be neat. I think it's a bit fuzzy at this stage because usually not every file in a GitHub release gets signed. So we'd have to have some way of figuring out what we should check.

Either way, I don't think anything in your current design excludes that possibility in the future whether it goes under a flag or an entirely new subcommand.

[woodruffw]

That seems reasonable. I believe that the identity issued from GH is effectively --repository and --ref combined. I'd have to double check that though.

The tricky thing here is that it isn't quite the same: the identity string contains the workflow filename itself, while the closest equivalent claim (--workflow-name) contains the workflow's symbolic name (i.e. release.yml vs name: Release inside of release.yml).

AFAICT there's no equivalent standalone claim for the workflow filename, so no set of alternative options would be fully equivalent to it. This may or may not be super important -- on one hand any action can name itself whatever it would like (e.g. malicious.yaml with name: Release), but on the other any ability to modify or create new GitHub Actions workflows is probably already "game over"/out of scope for our threat model.

[woodruffw]

I'm mildly in favour of having the shorter names. It doesn't seem like we're getting much from the workflow prefix and seems straightforward to me.

Agreed; I went with a middle solution: the flags themselves now use the short names, but the argparse.Namespace and environment fallbacks still use the longer forms (since we don't benefit from brevity as much in those places.)