-
Notifications
You must be signed in to change notification settings - Fork 39
Make TLE optional for Rekor v2 compat #632
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
protos/sigstore_rekor.proto
Outdated
// The inclusion promise/signed entry timestamp from the log. | ||
// The SET may be provided by Rekor v1 logs, but log entries |
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.
nit, SET isn't defined til later, can we say inclusion_promise
?
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.
yeah
protos/sigstore_rekor.proto
Outdated
// The inclusion promise/signed entry timestamp from the log. | ||
// The SET may be provided by Rekor v1 logs, but log entries |
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.
"log upload responses" rather than "log entries"?
Signed-off-by: Appu Goundan <[email protected]>
23b01d0
to
d709d62
Compare
I've been doing a lot of flipflopping on how to handle this. While clients can know during rekorv2/putEntries how to parse the returned rekor request, handling it at verification time for bundles is a bit different. Clients that try to parse the canonicalized entry will break if we try to re-use the canonicalized_body code path. So the idea that we wont break clients was not necessarily true, there is no "free verification" of rekor v2 bundles. Clients will fail in unexpected ways. We need new verification paths and branching anyway. If we want to reuse the TransparencyLogEntry from V1, clients need to rewrite parsing so they use the kind/version information in the TransparencyLogEntry to determine how to parse the CanonicalizedBody -- and not parse the CanonicalizedBody partially to determine kind/version (most client do this -- java/go/python). @steiza @kommendorkapten @jku @haydentherapper @codysoyland -- Current clients cannot handle rekorv2 bundles and they may fail in strange ways (probably mostly json unmarshalling error) Failures:
go (via examples/sigstore-go-verification)
Failure modes:
Both approaches will still require code changes and failures will occur on all existing clients. |
Thanks for digging into it. I've not had to deal with code that handles canonicalized body so I have trouble following the argument here. I suppose the core failure here is that the proto is unclear: "contents of this field are the same as the In any case, I'd appreciate more details on what has changed. I'm sure the canonicalized body is documented for both rekor versions... but I have trouble finding those docs and I can't see the issue by just looking at python client code. |
moved discussion to: #633 |
Summary
Changes required for a TLE that is compatible with both V1 and V2 logs.
Release Note
TransparencyLogEntry.integrated_time
is now optional as future log implementations may not return anything. Clients will be expected to attach another form of verified time to the bundle, such as an rfc3161 timestamp.Documentation
architecture docs may need to be updated