fix: use entry uuid uniformly

Signed-off-by: Asra Ali <asraa@google.com> update Signed-off-by: Asra Ali <asraa@google.com> update tree id Signed-off-by: Asra Ali <asraa@google.com> update Signed-off-by: Asra Ali <asraa@google.com> fix errors Signed-off-by: Asra Ali <asraa@google.com> update Signed-off-by: Asra Ali <asraa@google.com>
sigstore · Sep 1, 2022 · 0633cbb · 0633cbb
1 parent c828f9c
commit 0633cbb
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 33 deletions.
diff --git a/cmd/rekor-cli/app/get.go b/cmd/rekor-cli/app/get.go
@@ -126,26 +126,17 @@ var getCmd = &cobra.Command{
 		if uuid != "" {
 			params := entries.NewGetLogEntryByUUIDParams()
 			params.SetTimeout(viper.GetDuration("timeout"))
-
-			// NOTE: This undoes the change that let people pass in longer UUIDs without
-			// trouble even if their client is old, a.k.a. it will be able to use the TreeID
-			// (if present) for routing in the GetLogEntryByUUIDHandler
 			params.EntryUUID = uuid
 
 			resp, err := rekorClient.Entries.GetLogEntryByUUID(params)
 			if err != nil {
 				return nil, err
 			}
 
-			u, err := sharding.GetUUIDFromIDString(params.EntryUUID)
-			if err != nil {
-				return nil, err
-			}
-
 			var e models.LogEntryAnon
 			for k, entry := range resp.Payload {
-				if k != u {
-					continue
+				if err := compareEntryUUIDs(params.EntryUUID, k); err != nil {
+					return nil, err
 				}
 
 				// verify log entry
@@ -162,6 +153,48 @@ var getCmd = &cobra.Command{
 	}),
 }
 
+// TODO: Move this to the verify pkg, but only after sharding cleans
+// up it's Trillian client dependency.
+func compareEntryUUIDs(requestEntryUUID string, responseEntryUUID string) error {
+	requestUUID, err := sharding.GetUUIDFromIDString(requestEntryUUID)
+	if err != nil {
+		return err
+	}
+	responseUUID, err := sharding.GetUUIDFromIDString(responseEntryUUID)
+	if err != nil {
+		return err
+	}
+	// Compare UUIDs.
+	if requestUUID != responseUUID {
+		return fmt.Errorf("unexpected entry returned from rekor server: expected %s, got %s", requestEntryUUID, responseEntryUUID)
+	}
+	// If the request contains a Tree ID, then compare that.
+	requestTreeID, err := sharding.GetTreeIDFromIDString(requestEntryUUID)
+	if err != nil {
+		if errors.Is(err, sharding.ErrPlainUUID) {
+			// The request did not contain a Tree ID, we're good.
+			return nil
+		}
+		// The request had a bad Tree ID, error out.
+		return err
+	}
+	// We requested an entry from a given Tree ID.
+	responseTreeID, err := sharding.GetTreeIDFromIDString(responseEntryUUID)
+	if err != nil {
+		if errors.Is(err, sharding.ErrPlainUUID) {
+			// The response does not contain a Tree ID, we can only do so much.
+			// Old rekor instances may not have returned one.
+			return nil
+		}
+		return err
+	}
+	// We have Tree IDs. Compare.
+	if requestTreeID != responseTreeID {
+		return fmt.Errorf("unexpected entry returned from rekor server: expected %s, got %s", requestEntryUUID, responseEntryUUID)
+	}
+	return nil
+}
+
 func parseEntry(uuid string, e models.LogEntryAnon) (interface{}, error) {
 	b, err := base64.StdEncoding.DecodeString(e.Body.(string))
 	if err != nil {

diff --git a/cmd/rekor-cli/app/verify.go b/cmd/rekor-cli/app/verify.go
@@ -31,7 +31,6 @@ import (
 	"github.com/sigstore/rekor/pkg/generated/client/entries"
 	"github.com/sigstore/rekor/pkg/generated/models"
 	"github.com/sigstore/rekor/pkg/log"
-	"github.com/sigstore/rekor/pkg/sharding"
 	"github.com/sigstore/rekor/pkg/types"
 	"github.com/sigstore/rekor/pkg/verify"
 )
@@ -151,13 +150,9 @@ var verifyCmd = &cobra.Command{
 		}
 
 		if viper.IsSet("uuid") {
-			uuid, err := sharding.GetUUIDFromIDString(viper.GetString("uuid"))
-			if err != nil {
+			if err := compareEntryUUIDs(viper.GetString("uuid"), o.EntryUUID); err != nil {
 				return nil, err
 			}
-			if uuid != o.EntryUUID {
-				return nil, fmt.Errorf("unexpected entry returned from rekor server")
-			}
 		}
 
 		// Get Rekor Pub

diff --git a/pkg/api/entries.go b/pkg/api/entries.go
@@ -100,6 +100,13 @@ func logEntryFromLeaf(ctx context.Context, signer signature.Signer, tc TrillianC
 	}
 
 	uuid := hex.EncodeToString(leaf.MerkleLeafHash)
+	treeID := fmt.Sprintf("%x", tid)
+	entryIDstruct, err := sharding.CreateEntryIDFromParts(treeID, uuid)
+	if err != nil {
+		return nil, fmt.Errorf("error creating EntryID from active treeID %v and uuid %v: %w", treeID, uuid, err)
+	}
+	entryID := entryIDstruct.ReturnEntryIDString()
+
 	if viper.GetBool("enable_attestation_storage") {
 		pe, err := models.UnmarshalProposedEntry(bytes.NewReader(leaf.LeafValue), runtime.JSONConsumer())
 		if err != nil {
@@ -123,11 +130,6 @@ func logEntryFromLeaf(ctx context.Context, signer signature.Signer, tc TrillianC
 			}
 			// if looking up by key failed or we weren't able to generate a key, try looking up by uuid
 			if attKey == "" || fetchErr != nil {
-				activeTree := fmt.Sprintf("%x", tc.logID)
-				entryIDstruct, err := sharding.CreateEntryIDFromParts(activeTree, uuid)
-				if err != nil {
-					return nil, fmt.Errorf("error creating EntryID from active treeID %v and uuid %v: %w", activeTree, uuid, err)
-				}
 				att, fetchErr = storageClient.FetchAttestation(ctx, entryIDstruct.UUID)
 				if fetchErr != nil {
 					log.ContextLogger(ctx).Errorf("error fetching attestation by uuid: %s %v", entryIDstruct.UUID, fetchErr)
@@ -147,7 +149,7 @@ func logEntryFromLeaf(ctx context.Context, signer signature.Signer, tc TrillianC
 	}
 
 	return models.LogEntry{
-		uuid: logEntryAnon}, nil
+		entryID: logEntryAnon}, nil
 }
 
 // GetLogEntryAndProofByIndexHandler returns the entry and inclusion proof for a specified log index
@@ -266,7 +268,7 @@ func createLogEntry(params entries.CreateLogEntryParams) (models.LogEntry, middl
 	}
 
 	logEntry := models.LogEntry{
-		uuid: logEntryAnon,
+		entryID: logEntryAnon,
 	}
 	return logEntry, nil
 }

diff --git a/tests/sharding-e2e-test.sh b/tests/sharding-e2e-test.sh
@@ -243,20 +243,14 @@ fi
 echo
 echo "Testing /api/v1/log/entries/retrieve endpoint..."
 
-UUID1=$($REKOR_CLI get --log-index 1 --rekor_server http://localhost:3000 --format json | jq -r .UUID)
-UUID2=$($REKOR_CLI get --log-index 3 --rekor_server http://localhost:3000 --format json | jq -r .UUID)
+ENTRY_ID_1=$($REKOR_CLI get --log-index 1 --rekor_server http://localhost:3000 --format json | jq -r .UUID)
+ENTRY_ID_2=$($REKOR_CLI get --log-index 3 --rekor_server http://localhost:3000 --format json | jq -r .UUID)
 
 
 # Make sure retrieve by UUID in the inactive shard works
-NUM_ELEMENTS=$(curl -f http://localhost:3000/api/v1/log/entries/retrieve -H "Content-Type: application/json" -H "Accept: application/json" -d "{ \"entryUUIDs\": [\"$UUID1\"]}" | jq '. | length')
+NUM_ELEMENTS=$(curl -f http://localhost:3000/api/v1/log/entries/retrieve -H "Content-Type: application/json" -H "Accept: application/json" -d "{ \"entryUUIDs\": [\"$ENTRY_ID_1\"]}" | jq '. | length')
 stringsMatch $NUM_ELEMENTS "1"
 
-HEX_INITIAL_TREE_ID=$(printf "%x" $INITIAL_TREE_ID | awk '{ for(c = 0; c < 16 ; c++) s = s"0"; s = s$1; print substr(s, 1 + length(s) - 16);}')
-HEX_INITIAL_SHARD_ID=$(printf "%x" $SHARD_TREE_ID | awk '{ for(c = 0; c < 16 ; c++) s = s"0"; s = s$1; print substr(s, 1 + length(s) - 16);}')
-
-ENTRY_ID_1=$(echo -n "$HEX_INITIAL_TREE_ID$UUID1" | xargs echo -n)
-ENTRY_ID_2=$(echo -n "$HEX_INITIAL_SHARD_ID$UUID2" | xargs echo -n)
-
 # -f makes sure we exit on failure
 NUM_ELEMENTS=$(curl -f http://localhost:3000/api/v1/log/entries/retrieve -H "Content-Type: application/json" -H "Accept: application/json" -d "{ \"entryUUIDs\": [\"$ENTRY_ID_1\", \"$ENTRY_ID_2\"]}" | jq '. | length')
 stringsMatch $NUM_ELEMENTS "2"