Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 74 additions & 2 deletions cmd/rekor-cli/app/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
package app

import (
"bytes"
"context"
"encoding/base64"
"encoding/hex"
"fmt"
"math/bits"
Expand Down Expand Up @@ -104,6 +107,8 @@ var verifyCmd = &cobra.Command{
uuid := viper.GetString("uuid")
logIndex := viper.GetString("log-index")

var proposedEntry models.ProposedEntry

if uuid != "" {
searchLogQuery.EntryUUIDs = append(searchLogQuery.EntryUUIDs, uuid)
} else if logIndex != "" {
Expand All @@ -120,12 +125,12 @@ var verifyCmd = &cobra.Command{

props := CreatePropsFromPflags()

entry, err := types.NewProposedEntry(ctx, typeStr, versionStr, *props)
proposedEntry, err = types.NewProposedEntry(ctx, typeStr, versionStr, *props)
if err != nil {
return nil, fmt.Errorf("error: %w", err)
}

entries := []models.ProposedEntry{entry}
entries := []models.ProposedEntry{proposedEntry}
searchLogQuery.SetEntries(entries)
}
searchParams.SetEntry(&searchLogQuery)
Expand Down Expand Up @@ -158,12 +163,22 @@ var verifyCmd = &cobra.Command{
entry = v
}

if err := verifyBodyMatchesUUID(entry, o.EntryUUID); err != nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of this check? o.EntryUUID and entry both come from the server.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this instead compare uuid to the entry's hash when uuid is provided?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the check is to ensure the response from the server is coherent (body matches leaf hash)

the check on line 171 does what you asked (compares client provided uuid to the entry in the response's hash

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The underlying issue is the server shouldn’t return a leaf hash because a client should calculate it itself, but we aren’t gonna the API, this is fine

return nil, err
}

if viper.IsSet("uuid") {
if err := compareEntryUUIDs(viper.GetString("uuid"), o.EntryUUID); err != nil {
return nil, err
}
}

if proposedEntry != nil {
if err := verifyProposedEntryMatchesBody(ctx, proposedEntry, entry); err != nil {
return nil, err
}
}

treeID, err := sharding.TreeID(o.EntryUUID)
if err != nil {
return nil, err
Expand All @@ -185,6 +200,63 @@ var verifyCmd = &cobra.Command{
}),
}

// verifyBodyMatchesUUID verifies that the entry body's leaf hash matches
// the UUID in the response. This ensures the server returned an entry
// whose body is consistent with the claimed entry ID.
func verifyBodyMatchesUUID(entry models.LogEntryAnon, entryUUID string) error {
entryBytes, err := decodeEntryBody(entry)
if err != nil {
return err
}
computedLeafHash := rfc6962.DefaultHasher.HashLeaf(entryBytes)
responseUUID, err := sharding.GetUUIDFromIDString(entryUUID)
if err != nil {
return fmt.Errorf("getting UUID from response: %w", err)
}
if hex.EncodeToString(computedLeafHash) != responseUUID {
return fmt.Errorf("entry body hash does not match response UUID: computed %s, got %s",
hex.EncodeToString(computedLeafHash), responseUUID)
}
return nil
}

// verifyProposedEntryMatchesBody verifies the returned entry matches the
// locally-computed canonical entry derived from the user's inputs.
// Without this check, a malicious server could return a valid but
// unrelated log entry and all other cryptographic checks would pass.
func verifyProposedEntryMatchesBody(ctx context.Context, proposedEntry models.ProposedEntry, entry models.LogEntryAnon) error {
entryImpl, err := types.UnmarshalEntry(proposedEntry)
if err != nil {
return fmt.Errorf("unmarshalling proposed entry for verification: %w", err)
}
expectedCanonical, err := types.CanonicalizeEntry(ctx, entryImpl)
if err != nil {
return fmt.Errorf("canonicalizing proposed entry: %w", err)
}
entryBytes, err := decodeEntryBody(entry)
if err != nil {
return err
}
expectedLeafHash := rfc6962.DefaultHasher.HashLeaf(expectedCanonical)
computedLeafHash := rfc6962.DefaultHasher.HashLeaf(entryBytes)
if !bytes.Equal(expectedLeafHash, computedLeafHash) {
return fmt.Errorf("returned entry does not match provided artifact inputs")
}
return nil
}

func decodeEntryBody(entry models.LogEntryAnon) ([]byte, error) {
bodyStr, ok := entry.Body.(string)
if !ok {
return nil, fmt.Errorf("entry body must be a string, was %T", entry.Body)
}
entryBytes, err := base64.StdEncoding.DecodeString(bodyStr)
if err != nil {
return nil, fmt.Errorf("decoding entry body: %w", err)
}
return entryBytes, nil
}

func init() {
initializePFlagMap()
if err := addArtifactPFlags(verifyCmd); err != nil {
Expand Down
Loading
Loading