Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

verify the SET when getting a log entry #371

Merged
merged 4 commits into from
Jun 17, 2021

Conversation

asraa
Copy link
Contributor

@asraa asraa commented Jun 14, 2021

Signed-off-by: Asra Ali asraa@google.com

I noticed that in non-bundle, we are still using the integratedTime to check expiration without verifying rekor's signature of the signedEntryTimestamp

Signed-off-by: Asra Ali <asraa@google.com>
@@ -135,6 +135,16 @@ func VerifyTLogEntry(rekorClient *client.Rekor, uuid string) (*models.LogEntryAn
if err := v.VerifyInclusionProof(*e.Verification.InclusionProof.LogIndex, *e.Verification.InclusionProof.TreeSize, hashes, rootHash, leafHash); err != nil {
return nil, errors.Wrap(err, "verifying inclusion proof")
}

// Verify rekor's signature over the SET.
rekorPubKey, err := PemToECDSAKey([]byte(rekorPub))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should i just get this from rekorClient instead of using the offline one?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personally i think this is fine

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed! i actually might change it over so we don't have to use the prod instance in the e2e test (which is why it's failing)

Signed-off-by: Asra Ali <asraa@google.com>
@@ -135,6 +135,16 @@ func VerifyTLogEntry(rekorClient *client.Rekor, uuid string) (*models.LogEntryAn
if err := v.VerifyInclusionProof(*e.Verification.InclusionProof.LogIndex, *e.Verification.InclusionProof.TreeSize, hashes, rootHash, leafHash); err != nil {
return nil, errors.Wrap(err, "verifying inclusion proof")
}

// Verify rekor's signature over the SET.
rekorPubKey, err := PemToECDSAKey([]byte(rekorPub))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personally i think this is fine

Signed-off-by: Asra Ali <asraa@google.com>
@asraa
Copy link
Contributor Author

asraa commented Jun 14, 2021

Debugging e2e test. Sees to work fine with a prod rekor instance, but not with the test/local one. Working on it!

@cpanato cpanato added this to the v0.6.0 milestone Jun 15, 2021
@asraa
Copy link
Contributor Author

asraa commented Jun 16, 2021

OK it fails because rekor doesn't add a signature on on the signed entry timestamp when you get an existing entry. i don't know why we don't maintain the signature?

@asraa
Copy link
Contributor Author

asraa commented Jun 16, 2021

Depends on the linked PR, will update when ready for review

@asraa
Copy link
Contributor Author

asraa commented Jun 17, 2021

OK! PR is merged, this is fixed now!

@dlorenc dlorenc merged commit 0d61e85 into sigstore:main Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants