fix: fix rekor verification #277
Conversation
Signed-off-by: Asra Ali <asraa@google.com>
|
This change will need backporting to all major releases! TODO tomorrow. |
| @@ -470,15 +470,6 @@ func Test_runVerifyGHAArtifactPath(t *testing.T) { | |||
| err: serrors.ErrorMismatchWorkflowInputs, | |||
| noversion: true, | |||
| }, | |||
| // Regression test of sharded UUID. | |||
There was a problem hiding this comment.
do we need to remove this or only ommit the v0.0.2?
| RootCerts: roots, | ||
| CertOidcIssuer: certOidcIssuer, | ||
| } | ||
| verifier, err := cosign.ValidateAndUnpackCert(certs[0], co) |
There was a problem hiding this comment.
maybe create cert : cert[0] variable.
| // GetRekorEntriesWithCert finds all entry UUIDs with the full intoto attestation. | ||
| // The attestation generated by the slsa-github-generator libraries contain a signing certificate. | ||
| func GetRekorEntriesWithCert(rClient *client.Rekor, provenance []byte) (*dsselib.Envelope, *x509.Certificate, error) { | ||
| func GetRekorEntriesWithCert(rClient *client.Rekor, provenance []byte) ( | ||
| *models.LogEntryAnon, error) { | ||
| // Use intoto attestation to find rekor entry UUIDs. | ||
| params := entries.NewSearchLogQueryParams() | ||
| searchLogQuery := models.SearchLogQuery{} | ||
| certPem, err := envelope.GetCertFromEnvelope(provenance) |
There was a problem hiding this comment.
We. already extract the cert in the caller certPem, err := envelope.GetCertFromEnvelope(provenance), shall we pass it as argument to this function?
| } | ||
|
|
||
| if len(resp.GetPayload()) != 1 { |
There was a problem hiding this comment.
any reason to change != 1 to ==0?
| if err != nil { | ||
| fmt.Fprintf(os.Stderr, "Error computing leaf hash for tlog entry at index: %d\n", *entry.LogIndex) | ||
| continue | ||
| if err := rverify.VerifyLogEntry(context.Background(), &e, verifier); err != nil { |
There was a problem hiding this comment.
maybe add a comment what this function verifies. This verifies the SET only? Do we need to verify inclusion proof + root signature to verify the entry is part of the current tree... is there even a way to verify it's the "current" top of the tree...?
|
Thank you! I realized in doing the backported fixes that I can make a much less invasive change because that helped me pinpoint the exact issue; I will make the full fixes here (relying on rekor libraries) after the backports just FYI |
Signed-off-by: Asra Ali asraa@google.com
This fixes rekor verification by:
VerifyProvenanceSignature: verifies the provenance in a step-by-step manner: covering signature validation, rekor lookup, and signature time validation (against entry upload time)