Fix gitsign for public Sigstore changes. #50
Conversation
There were a few updates to public sigstore today, that ended up breaking gitsign. 1. Fulcio now uses an intermediary cert. TIL that DER encoding will sort values for stability, which means that the ordering of these certs can change. This PR fixes gitsign for the public instance, but is not a complete solution. We'll need to follow up with another change to be smarter about how we ID the certificate to use. 2. Rekor now uses a sharded entry ID, which breaks how we were looking up Rekor entries since the sharded entry ID didn't match the underlying UUID (even though it did). Added normalization to work for either case. Also updates rekor/cosign deps, we need rekor >= 0.5 (https://blog.sigstore.dev/sigstore-project-update-march-2022-bf34aa632388) Signed-off-by: Billy Lynch <billy@chainguard.dev>
|
The test status is lying - this is still failing https://github.com/sigstore/gitsign/runs/6698779041?check_suite_focus=true#step:4:233 (which probably means it's sorting by something non-deterministic like key ID 😭 ) |
Turns out cert ordering was still non-deterministic, so restricting the certs included to just the leaf cert (which is probably okay in most cases?) We might add support back for this later, but for now this should be okay. Also fix e2e test so failing validation actually causes a non-zero exit code. Signed-off-by: Billy Lynch <billy@chainguard.dev>
|
e2e test is pulling code from main, not head (since pull_request_target is considered trusted by default). Latest commit is signed with its own tool if you want to verify though: https://gist.github.com/wlynch/9c9e86858af0873cca77079699dff7b9 |
|
FYI, we had to rollback to Rekor 0.5 before the sharding work - This will work with 0.5 and 0.7, correct? |
It should work for either. I didn't have a strong preference so I just bumped to the latest version. Seems to work fine, but if there are any sharp edges to be weary of let me know! |
| KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, | ||
| Roots: fulcioroots.Get(), | ||
| Intermediates: fulcioroots.GetIntermediates(), | ||
| KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, |
There was a problem hiding this comment.
Can this be changed to ExtKeyUsageCodeSigning to more narrowly scope the verification?
There was a problem hiding this comment.
Yup we actually do this else where -
Line 123 in 61a4195
I'll look into refactoring this in another PR (mainly so we don't lose the 50/50 on whether the e2e test will pick the right cert to validate)
Summary
There were a few updates to public sigstore today, that ended up
breaking gitsign.
values for stability, which means that the ordering of these certs
can change. This PR fixes gitsign for the public instance, but is not
a complete solution. We'll need to follow up with another change to
be smarter about how we ID the certificate to use.
up Rekor entries since the sharded entry ID didn't match the
underlying UUID (even though it did). Added normalization to work for
either case.
Also updates rekor/cosign deps, we need rekor >= 0.5
(https://blog.sigstore.dev/sigstore-project-update-march-2022-bf34aa632388)
Signed-off-by: Billy Lynch billy@chainguard.dev
Ticket Link
Fixes #49
Release Note