-
Notifications
You must be signed in to change notification settings - Fork 165
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
fix: make rekor verify work with sharded uuids #970
Conversation
Signed-off-by: Asra Ali <asraa@google.com>
Will tackle factoring out as a follow-up, want to resolve the question. |
EntryID is mainly useful if the same UUID exists across shards, and you need to be able to specify which one you want. I think it's more useful for requesting specific entries from shards, and not as necessary when entries are being returned. That's mostly because the UUID is actually needed for verification as you pointed out, so it makes more sense to return that. Totally agree the behavior is unexpected though so I'd be open to changing it if people have strong preferences😅 |
I think my problem though was that people don't hold on to EntryUUIDs because they're not in return responses at all yet. In the Line 473 in 6093a33
This means that cosign's verification flow will need to change: either be shard-aware (which is difficult for blobs), or have rekor return all the matching UUIDs across all shards, and verify if any of the integrated times are valid against cert expiration. |
I guess for now we'll table this and at least fix this behavior! |
They are in return responses, just not easily extracted and returned through the cosign-defined interfaces. |
} | ||
|
||
// Note: the returned entry UUID is the UUID (not include the Tree ID) |
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.
shouldn't the API return the full entryUUID (including the treeID)?
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.
That's my main question in the PR description. However, the behavior is mixed... e.g. rekor-cli get
returns just the raw UUID as well, and the request API here also may take the raw UUID.
Originally I had changed the response payload to fix this. Should I go back to that?
Signed-off-by: Asra Ali asraa@google.com
Summary
Fixes #877
See issue for the problem:
rekor verify
didn't work with sharding: If the requested UUID was a sharded Entry UUID (Tree ID + UUID), then we would fail because the return keys ofmodels.LogEntryAnon
are always plain UUIDs. When we would compare the returned response UUID with the requested, we would fail on mismatch.Question
Entry UUID = Tree ID + UUID
UUID = UUID
Why aren't the returned key UUIDs not the Entry UUIDs? It feels weird to me that if clients are requesting Entry UUIDs, and we are making clients aware of sharded UUIDs, then we better also return those in responses.
Meanwhile, we expose virtual log indices. Aren't Entry UUIDs the equivalent of virtual log indices for uuids?
On the other hand, it makes sense to return the actual UUID because that is the hash of the leaf value, and clients are expected to verify that. In that case, why are Entry UUIDs exposed at all? All of our endpoints handle them, see this bug in question.
Release Note
Documentation