-
Notifications
You must be signed in to change notification settings - Fork 7
Support timestamps in attestations #143
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
base: main
Are you sure you want to change the base?
Conversation
d31fcd9
to
c272629
Compare
* One attestation is from staging (rekor2 entry and timestamp) * Other attestation is from production (rekor1 entry and timestamp)
The offline test likely breaks because I created new test assets. staging trust root has changed fairly recently, and sigstore-python does not contain the most up-to-date one. Test passed for me locally since my cached trust root is up-to-date. I think the test should have a preparation step that makes sure the trust root has been updated, and then should run the verify in offline mode. |
Current test assets require a trust root which is newer than the one embedded in sigstore-python: Update the trust roots before running offline tests. Move the whole offline-test setup into Makefile to make it a little easier to manage.
timestamps = [] | ||
if sigstore_bundle.verification_material.timestamp_verification_data: | ||
ts_data = sigstore_bundle.verification_material.timestamp_verification_data | ||
timestamps = [base64.b64encode(ts.as_bytes()) for ts in ts_data.rfc3161_timestamps] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose this is the main decision in this PR: A timestamp in the attestations serialization format is the base64 encoded timestamp response as defined in RFC3161.
For reference the timestamp use is defined in sigstore client spec, but it's basically the same as the use of integrated time in rekor1: timestamped data is the signature bytes, at least one timestamp time must be within lifetime of the signing certificate (and its chain) to prove the time of signing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR currently contains two changes:
- support for timestamps
- Stop forcing rekor v1 when signing
We could separate these two as well since the latter one really only makes sense to do after Rekor v2 is recommended for signing in the Sigstore signingconfig (whereas the former change could be merged any time)
Is there an ETA for when Sigstore will start recommending Rekor v2 for signing? If not, let's split the PR. |
No date is set: We'd like to have more data on rekor v2 compatible client uptake before that. I would expect end of year. I'll have a look at splitting this. |
* Force Rekor tlog version 1 when signing * Make sure this is the case with a check in test_roundtrip() Not that rekor v2 entries are still considered valid in verification already, and that timestamps are included in the attestation even if the entry is from rekor v1
Ok the PR is split now, current state is documented in the description now:
|
This PR adds support for RFC3161 timestamps in the attestations. It is a draft because:
Supporting timestamps in the annotations is a requirement to using transparency log entries from rekor v2 (see #142 for details and links).
Fixes #142
CC @di
Details
force_tlog_version
is still used (this is tested intest_roundtrip()
)