feat: embed Rekor bundle in OCI attestation layer annotations#1610
feat: embed Rekor bundle in OCI attestation layer annotations#1610ab-ghosh wants to merge 1 commit into
Conversation
tekton-robot
added
the
kind/feature
tekton-robot
added
the
size/L
infernus01
left a comment
infernus01
left a comment
There was a problem hiding this comment.
@ab-ghosh Would it be good to write this feature in the docs as well?
| @@ -41,4 +42,6 @@ type Bundle struct { | |||
| Cert []byte | |||
| // Cert is an optional PEM encoded x509 certificate chain, if one was used for signing. | |||
There was a problem hiding this comment.
Not related to this PR but I just got eyes on this.
| // Cert is an optional PEM encoded x509 certificate chain, if one was used for signing. | |
| // Chain is an optional PEM encoded x509 certificate chain, if one was used for signing. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
@infernus01 Good eyes on the comment typo And yes, documenting this feature makes sense, let me add that.
When transparency.enabled=true, the Rekor bundle (SignedEntryTimestamp, integratedTime, logIndex, logID) is now embedded as a dev.sigstore.cosign/bundle annotation on the OCI attestation layer. This enables offline signature verification of attestations signed by Tekton Chains using keyless (Fulcio) signing. Previously, the Rekor upload happened after storage, so the bundle was not available when writing attestations to OCI registries. The upload is now performed before storage so the bundle can be passed through to the OCI backend. Fixes tektoncd#1598 Signed-off-by: ab-ghosh <abghosh@redhat.com>
ab-ghosh
force-pushed
the
feat/embed-rekor-bundle-oci-attestation
branch
from
ee8dec2 to
1b32257
Compare
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
This seems to be a hard return. What if getRekor() fails (maybe due to bad rekor URL config or anything) and this might prevent all attestation storage. WDYT?
There was a problem hiding this comment.
this hard return is pre-existing, IMO if getRekor fails, it's a config error, should not be treated as a transient failure
| Bundle: &signing.Bundle{ | ||
| Content: rawPayload, | ||
| Signature: []byte(signature), | ||
| Cert: []byte(storageOpts.Cert), | ||
| Chain: []byte(storageOpts.Chain), |
There was a problem hiding this comment.
One thing i noticed that the RekorBundle gets passed through in uploadAttestation below but not in uploadSignature here. since simplesigning + x509/fulcio + rekor is a valid config, the same offline-verification gap might exist for OCI image signatures too. right?
was this intentionally out of scope for this PR, or worth handling here as well?
There was a problem hiding this comment.
Yeah that totally makes sense, but #1598 is more aligned with attestations. We can address this in a separate issue/PR, what say?
3 participants
Changes
This PR adds support for embedding the Rekor transparency log bundle (
dev.sigstore.cosign/bundle) in OCI attestation layer annotations whentransparency.enabled=true. This enables offline signature verification of attestations signed by Tekton Chains using keyless (Fulcio) signing, bringing Chains' attestation format in line with whatcosign attestproduces.Fixes #1598
/kind feature
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
Release Notes