rekor icon indicating copy to clipboard operation
rekor copied to clipboard

fix: make rekor verify work with sharded uuids

Open asraa opened this issue 1 year ago • 5 comments

Signed-off-by: Asra Ali [email protected]

Summary

Fixes https://github.com/sigstore/rekor/issues/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 of models.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

asraa avatar Aug 12 '22 18:08 asraa

Will tackle factoring out as a follow-up, want to resolve the question.

asraa avatar Aug 12 '22 18:08 asraa

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😅

priyawadhwa avatar Aug 12 '22 19:08 priyawadhwa

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 my problem though was that people don't hold on to EntryUUIDs because they're not in return responses at all yet. In the cosign verify flow we reconstruct the UUID by the proposed entry. If I want to DoS a large project's artifact that was uploaded on an inactive shard, I'll just re-upload the entry to the active shard, which means that the latest shard (https://github.com/sigstore/rekor/blob/6093a3391c2399d384b50bb9b6f48e4011111729/pkg/api/entries.go#L473) will return with an invalid integratedTime for verification.

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.

asraa avatar Aug 12 '22 20:08 asraa

I guess for now we'll table this and at least fix this behavior! I'll file a follow-up issue about client/cosign verification with sharding.

asraa avatar Aug 12 '22 20:08 asraa

I think my problem though was that people don't hold on to EntryUUIDs because they're not in return responses at all yet

They are in return responses, just not easily extracted and returned through the cosign-defined interfaces.

bobcallaway avatar Aug 12 '22 20:08 bobcallaway