Offline Rekor bundle generation and verification

[woodruffw]

~~WIP.~~

Closes #52.

Closes #194.

[woodruffw]

This is ready for an initial review, but it needs some more testing before merge (I haven't actually confirmed offline bundles work yet, only that the changes don't break online verification).

My plan was to implement #194 in this PR as well, to complete the whole picture (and round out testing). But I can do that separately and use a bundle generated by cosign instead, if we'd like to keep the diff small(er).

[asraa]

But I can do that separately and use a bundle generated by cosign instead, if we'd like to keep the diff small(er).

I may not necessarily totally us cosign's scheme... I'm not really a fan of the duplicated fields as I mentioned in https://github.com/sigstore/sigstore-python/issues/194#issuecomment-1240976109

But I hear the burden of implementing something twice. On the other hand, I think keeping it absolutely minimal to just the Rekor entry stored offline would allow building up to the "Sigstore bundle" (which is WIP design https://github.com/sigstore/cosign/pull/2204/ away from the sorta-faulty cosign bundle)

[woodruffw]

But I hear the burden of implementing something twice. On the other hand, I think keeping it absolutely minimal to just the Rekor entry stored offline would allow building up to the "Sigstore bundle" (which is WIP design sigstore/cosign#2204 away from the sorta-faulty cosign bundle)

Makes sense. Yeah, this PR is limited to only the "Rekor" bundle, not the bigger thing that I understand (maybe incorrectly?) to be unstable/soft-deprecated on cosign's side.

[woodruffw]

Sanity checking myself again: in its current form, this PR isn't sufficient, right? I also need to check the offline Rekor bundle's consistency against the certificate and signature, since someone could conceivably ask me to verify a completely unrelated (but valid) Rekor bundle.

[asraa]

I also need to check the offline Rekor bundle's consistency against the certificate and signature, since someone could conceivably ask me to verify a completely unrelated (but valid) Rekor bundle.

YES!

It would maybe look something like: reconstruct the rekor entry with the hashed_rekord entry, and compare hashes/fields

[woodruffw]

It would maybe look something like: reconstruct the rekor entry with the hashed_rekord entry, and compare hashes/fields

Cool, makes sense! Just making sure I understand: the hashedrekord here should have the same layout as the one we POST to Rekor's REST API, right? Plus canonicalization and base64 encoding, of course.

[asraa]

the hashedrekord here should have the same layout as the one we POST to Rekor's REST API, right? Plus canonicalization and base64 encoding, of course.

Correct. The canonicalization is a tricky point though. E.g. if a client passes in a DER encoded cert, will you canonicalize to the PEM? Rekor canonicalized to de-dupe those.

IMO technically the base64-encoded signature and message digest is the only thing required to validate: if those two are the same as your input, it's statistically unlikely someone will have a distinct valid, Fulcio-issued certificate that correctly signing the artifact with the same sig. EDIT: I'm actually not sure, it's probably best to compare all fields, and I'll think about this more. I'm not sure this works out for all schemes...

[woodruffw]

Correct. The canonicalization is a tricky point though. E.g. if a client passes in a DER encoded cert, will you canonicalize to the PEM? Rekor canonicalized to de-dupe those.

At least for sigstore-python's purposes, we only accept PEM-encoded certs. So, assuming that Rekor's body is always PEM-encoded, that part should be fine!

[woodruffw]

Just for tracking purposes, there are a few things this PR still needs:

• [x] Offline Rekor bundle generation (per #194), so that we can do round-trip testing
• [x] Cross-checking the offline Rekor bundle against the certificate and artifact, to ensure we're verifying the correct tlog entry
• [ ] Anything else I missed?

[haydentherapper]

I'd also check the Cosign security advisories, https://github.com/sigstore/cosign/security/advisories?state=published, since most of them involved bundle verification.

[woodruffw]

Thanks for linking that! https://github.com/sigstore/cosign/security/advisories/GHSA-8gw7-4j42-w388 looks like the relevant one in terms of confusable Rekor verification.

[woodruffw]

Just for anyone playing along at home: offline Rekor bundle generation now works, and can dogfood itself:

# generates README.md.{sig,crt,bundle}
sigstore sign README.md

sigstore verify --rekor-offline README.md

Question for @haydentherapper and @asraa: should I be using the .bundle suffix in this way? I can imagine that that suffix will eventually be used for the "full" Sigstore bundle, so maybe I should use .rekor or something similar to avoid an eventual conflict.

[haydentherapper]

We said .sigstore for the bundle. I'd be fine with .rekor

[woodruffw]

64370f5 should address the consistency requirements between the offline Rekor bundle and the other signing artifacts. I'll refactor it into an independent API, however, since that top level verify API function is rapidly ballooning...

[woodruffw]

We said .sigstore for the bundle. I'd be fine with .rekor

Done! We now emit {input}.rekor and attempt to load that file by default, for offline Rekor bundles.

[woodruffw]

I think this is good to go, at least on my side.

cc @di: what are your thoughts on merging this as-is, vs. waiting for the "sigstore bundle" formal to become stable? I think we could go either way, although we'll likely end up replacing/deprecating these CLI flags once the sigstore bundle is available. As such, it might make sense to avoid the deprecation period and just wait a bit here.

[jku]

• your comment WRT inverting entry matching seems reasonable: that would likely go some way towards fixing the previous complaint?

actually: inverting the entry matching maybe isn't a good idea if the rekor entry could contain full certificate chains but we are ok with just the single certificate in our verfication input ?

[woodruffw]

• I think verify() needs a refactor though: it's now very long and quite complicated -- that could happen in another issue though

Yes, absolutely agreed. I have that earmarked in #250.

[woodruffw]

Not sure why DCO is stalled here...

[woodruffw]

Needs re-app, since I added some warnings to emphasize that the Rekor bundle isn't intended to be a long-term format.