Clean up verified time handling #1489
Conversation
Try to handle TSA timestamps and rekor v1 integrated time in a sensible manner: * no special cases for when TSA timestamps are present * require one verified time by default * Only allow integrated time to be a verified time if entry is from rekor v1 * VERIFY_TIMESTAMP_THRESHOLD now refers to "number of verified times", not just TSA timestamps * Tests use a rekor v1 bundle but expect it to be invalid if the timestamp is invalid -- but the integrated time is enough. Fix this by monkeypatching VERIFY_TIMESTAMP_THRESHOLD Signed-off-by: Jussi Kukkonen <[email protected]>
jku
force-pushed
the
cleanup-verified-time
branch
from
60b5906 to
f638e6d
Compare
|
Thanks for poking at this @jku! Let me know when I should do a review.
Perhaps we could make it
Yeah, I think that would be ideal! |
VERIFIED_TIME_THRESHOLD makes more sense since integrated time is also in this threshold. Strictly speaking this is an API change but since the meaning has (slightly) changed already that makes sense. Signed-off-by: Jussi Kukkonen <[email protected]>
| kv = bundle.log_entry._kind_version | ||
| if not (kv.kind in ["dsse", "hashedrekord"] and kv.version == "0.0.1"): |
There was a problem hiding this comment.
this check will not work when backported to 3.x (as log_entry does not have kindversion there) but the check can be removed there -- only 0.0.1 versions are supported anyway
I originally avoided this thinking about the potential backport and the fact that technically this is API... but I think I agree with you:
|
|
This should be ready. It's backportable to 3.x (with the small caveat mentioned in a code comment) and that makes bundles created with current main using |
Thanks, I'll merge this now. I'll make the backport a new PR today/tomorrow |
* Clean verified time handling Try to handle TSA timestamps and rekor v1 integrated time in a sensible manner: * no special cases for when TSA timestamps are present * require one verified time by default * Only allow integrated time to be a verified time if entry is from rekor v1 * VERIFY_TIMESTAMP_THRESHOLD now refers to "number of verified times", not just TSA timestamps * Tests use a rekor v1 bundle but expect it to be invalid if the timestamp is invalid -- but the integrated time is enough. Fix this by monkeypatching VERIFY_TIMESTAMP_THRESHOLD Signed-off-by: Jussi Kukkonen <[email protected]> * verify: Rename VERIFY_TIMESTAMP_THRESHOLD VERIFIED_TIME_THRESHOLD makes more sense since integrated time is also in this threshold. Strictly speaking this is an API change but since the meaning has (slightly) changed already that makes sense. Signed-off-by: Jussi Kukkonen <[email protected]> --------- Signed-off-by: Jussi Kukkonen <[email protected]>
DRAFT While I think about it -- there's a few ways to handle this, not sure this is right.
Try to handle TSA timestamps and rekor v1 integrated time in a sensible manner:
Fixes #1488.
This would be back portable to make 3.x branch be able to verify newer bundles (that contain an "extra" TSA timestamp)