sigstore / sigstore-conformance

Conformance testing for Sigstore clients
https://sigstore.dev
7 stars 10 forks source link

Conformance suite: support verifying by hash instead of file #157

Open woodruffw opened 1 week ago

woodruffw commented 1 week ago

This is a blocker for #156: we don't want to have to download every CPython release artifact to verify it, especially since we already have their hashes.

To accomplish this, the verify-bundle and verify commands will probably need to learn to take digests as well, e.g.

${ENTRYPOINT} verify ... sha256:hexstringhere

This shouldn't be too difficult to plumb in, although clients will need to learn about it.

Core parts here:

  1. Document hashes as an acceptable input in the CLI protocol: https://github.com/sigstore/sigstore-conformance/blob/main/docs/cli_protocol.md
  2. Plumb them into the sigstore-python conformance suite helper: https://github.com/sigstore/sigstore-conformance/blob/main/sigstore-python-conformance
    • This may require us to move away from a "basic" wrapper over the sigstore CLI and towards using the sigstore APIs directly.

CC @facutuesca

segiddins commented 1 week ago

If we're refactoring the conformance interface, I think it makes sense to have the input be the canonical proto for verification? https://github.com/sigstore/protobuf-specs/blob/b288f87c935c02102866e2c8df2f3d66a812f446/protos/sigstore_verification.proto#L144

steiza commented 1 week ago

sigstore-go supports verifying an artifact by providing the digest instead of the artifact itself - this is also quite useful when verifying large container images.

I haven't looked closely at the verification protobuf, but does it support supplying the artifact digest? I didn't see it when giving it a quick look over.

loosebazooka commented 1 week ago

I don't know if anyone is using the verification proto directly yet. But it does want an actual artifact reference or actual artfiact: https://github.com/sigstore/protobuf-specs/blob/main/protos/sigstore_verification.proto#L133 -- we probably have to edit it to allow digests.

sigstore-java's API takes either a digest or a file reference. But our CLI doesn't care because it's not published -- happy to integrate with whatever.

woodruffw commented 1 week ago

(@segiddins and I talked about this IRL, so summarizing here).

If we're refactoring the conformance interface, I think it makes sense to have the input be the canonical proto for verification? sigstore/protobuf-specs@b288f87/protos/sigstore_verification.proto#L144

I'm not opposed to this per se, but IMO it has a stack of dependencies/path assumptions behind it that would be an unnecessary blocker to short-term expansions of the conformance suite (the main one being that we'd need to first update the Artifact message to include digest support).

So TL;DR I think we should refactor the conformance interface, but IMO not as a blocker to shoter-term coverage improvements 🙂

jku commented 1 week ago

This is a blocker for #156: we don't want to have to download every CPython release artifact to verify it, especially since we already have their hashes.

I suppose there is the alternative of using cache action on CI and only downloading if needed. It's a bit complicated and there's still some downloading on first run... Not ideal.

facutuesca commented 5 days ago

@woodruffw Given that for verify we specify two flows: one for verifying artifacts with bundles, and another for verifying artifacts with cert+signature. Do we want to add 2 new flows, this time with digests instead of artifacts?:

### Verify

#### Signature and certificate flow (artifact)
#### Bundle flow (artifact)

#### Signature and certificate flow (digest)
#### Bundle flow (digest)
woodruffw commented 5 days ago

Do we want to add 2 new flows, this time with digests instead of artifacts?

My weak preference is to start with just bundles, since we're trying to migrate off of detached material tests anyways 🙂

steiza commented 5 days ago

My preference would be for an optional flag on the existing verify-bundle command.

loosebazooka commented 5 days ago

I think I strongly prefer we only add it to bundles.

facutuesca commented 5 days ago

My preference would be for an optional flag on the existing verify-bundle command.

Something like:

${ENTRYPOINT} verify-bundle [--staging] --verify-digest --bundle FILE --certificate-identity IDENTITY --certificate-oidc-issuer URL [--trusted-root FILE] FILE_OR_DIGEST

where --verify-digest means we interpret FILE_OR_DIGEST as a digest?

We could also do it without a flag, like on sigstore-python (link). Rather, we interpret FILE_OR_DIGEST as a digest if it starts with sha256:..., and otherwise as a file path.

woodruffw commented 5 days ago

An optional flag sounds good to me -- technically it's unambiguous given the sha256: prefix, but having the flag makes it more explicitly a conformance protocol change 🙂

facutuesca commented 5 days ago

PR open here: https://github.com/sigstore/sigstore-conformance/pull/158