Validate tree ID on calls to /api/v1/log/entries/retrieve

[priyawadhwa]

fixes #1014, cc @asraa

Signed-off-by: Priya Wadhwa priya@chainguard.dev

[codecov-commenter]

Codecov Report

Merging #1017 (a64668a) into main (5159d01) will increase coverage by 0.02%.
The diff coverage is 0.00%.

❗ Current head a64668a differs from pull request most recent head d0e2a95. Consider uploading reports for the commit d0e2a95 to get more accurate results

@@            Coverage Diff             @@
##             main    #1017      +/-   ##
==========================================
+ Coverage   41.57%   41.60%   +0.02%     
==========================================
  Files          71       71              
  Lines        6929     6932       +3     
==========================================
+ Hits         2881     2884       +3     
- Misses       3746     3747       +1     
+ Partials      302      301       -1     

Impacted Files	Coverage Δ
pkg/api/entries.go	0.00% <0.00%> (ø)
pkg/types/alpine/v0.0.1/entry.go	54.87% <0.00%> (+1.21%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[asraa]

Interesting! Thanks for the fix and debugging. So what was happening before was that we were actually retrieving the result via searchHashes on the raw uuid it seems.

So even now, if you send an something where sharding.ValidateEntryID(entryID) is an error, you will continue to do that behavior and return something from a different tree.

[asraa]

I'm a little brain-fried right now, but I think the fix is going to need to propogate the entire EntryUUID when you go through this case:

rekor/pkg/api/entries.go

Lines 346 to 350 in d0e2a95

	hash, err := hex.DecodeString(uuid)
	if err != nil {
	return handleRekorAPIError(params, http.StatusBadRequest, err, malformedUUID)
	}
	searchHashes = append(searchHashes, hash)

Then in here: extract the uuid and validate the returned tree ID when one was in the EntryUUID request:

rekor/pkg/api/entries.go

Line 396 in d0e2a95

if leafResult != nil && leafResult.Leaf != nil {

[priyawadhwa]

So even now, if you send an something where sharding.ValidateEntryID(entryID) is an error, you will continue to do that behavior and return something from a different tree.

Yep, but I think sharding.ValidateEntryID(entryID) should only be an error if a UUID was originally requested instead of an EntryID, in which case expected behavior (at the moment) is we would search all shards and return all matching entries since there is no Tree ID.

[asraa]

Yep, but I think sharding.ValidateEntryID(entryID) should only be an error if a UUID was originally requested instead of an EntryID,

I think it'll error out if the tree ID doesn't parse as an int64 or if it's 0

rekor/pkg/sharding/sharding.go

Lines 153 to 162 in 5159d01

	i, err := strconv.ParseInt(t, 16, 64)
	if err != nil {
	return fmt.Errorf("could not convert treeID %v to int64: %v", t, err)
	}
	// Check for invalid TreeID values
	// TODO: test for more of these
	if i == 0 {
	return fmt.Errorf("0 is not a valid TreeID")
	}

[priyawadhwa]

I think it's fine if the tree is 0, since that maps to having a UUID and putting a bunch of 0s in front of it to form an EntryID.

To address the failure on parsing int64 I added in a validation for UUID so it should do something like this:

1. Check if EntryID is valid, if yes then try to get it
2. If no, assume we have a UUID and validate it. Fail if we don't have a UUID, otherwise add it to search hashes.

[asraa]

To address the failure on parsing int64 I added in a validation for UUID so it should do something like this:

That's perfect! Thank you!! That made the fix so clean