Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix idempotency error with signing #3371

Merged
merged 1 commit into from
Nov 18, 2023
Merged

Fix idempotency error with signing #3371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 17 additions & 15 deletions pkg/cosign/tlog.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func doUpload(ctx context.Context, rekorClient *client.Rekor, pe models.Proposed
// Here, we display the proof and succeed.
var existsErr *entries.CreateLogEntryConflict
if errors.As(err, &existsErr) {
ui.Infof(ctx, "Signature already exists. Displaying proof")
ui.Infof(ctx, "Signature already exists. Fetching and verifying inclusion proof.")
uriSplit := strings.Split(existsErr.Location.String(), "/")
uuid := uriSplit[len(uriSplit)-1]
e, err := GetTlogEntry(ctx, rekorClient, uuid)
Expand Down Expand Up @@ -299,8 +299,8 @@ func getTreeUUID(entryUUID string) (string, error) {
}
}

// Validates UUID and also TreeID if present.
func isExpectedResponseUUID(requestEntryUUID string, responseEntryUUID string, treeid string) error {
// Validates UUID and also shard if present.
func isExpectedResponseUUID(requestEntryUUID string, responseEntryUUID string) error {
// Comparare UUIDs
requestUUID, err := getUUID(requestEntryUUID)
if err != nil {
Expand All @@ -313,19 +313,21 @@ func isExpectedResponseUUID(requestEntryUUID string, responseEntryUUID string, t
if requestUUID != responseUUID {
return fmt.Errorf("expected EntryUUID %s got UUID %s", requestEntryUUID, responseEntryUUID)
}
// Compare tree ID if it is in the request.
requestTreeID, err := getTreeUUID(requestEntryUUID)
// Compare shards if it is in the request.
requestShardID, err := getTreeUUID(requestEntryUUID)
if err != nil {
return err
}
if requestTreeID != "" {
tid, err := getTreeUUID(treeid)
if err != nil {
return err
}
if requestTreeID != tid {
return fmt.Errorf("expected EntryUUID %s got UUID %s from Tree %s", requestEntryUUID, responseEntryUUID, treeid)
}
responseShardID, err := getTreeUUID(responseEntryUUID)
if err != nil {
return err
}
// no shard ID prepends the entry UUID
if requestShardID == "" || responseShardID == "" {
return nil
}
if requestShardID != responseShardID {
return fmt.Errorf("expected UUID %s from shard %s: got UUID %s from shard %s", requestEntryUUID, responseEntryUUID, requestShardID, responseShardID)
}
return nil
}
Expand Down Expand Up @@ -357,8 +359,8 @@ func GetTlogEntry(ctx context.Context, rekorClient *client.Rekor, entryUUID stri
return nil, err
}
for k, e := range resp.Payload {
// Validate that request EntryUUID matches the response UUID and response Tree ID
if err := isExpectedResponseUUID(entryUUID, k, *e.LogID); err != nil {
// Validate that request EntryUUID matches the response UUID and response shard ID
if err := isExpectedResponseUUID(entryUUID, k); err != nil {
return nil, fmt.Errorf("unexpected entry returned from rekor server: %w", err)
}
// Check that body hash matches UUID
Expand Down
15 changes: 2 additions & 13 deletions pkg/cosign/tlog_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,78 +67,67 @@ func TestExpectedRekorResponse(t *testing.T) {
name string
requestUUID string
responseUUID string
treeID string
wantErr bool
}{
{
name: "valid match with request & response entry UUID",
requestUUID: validTreeID + validUUID,
responseUUID: validTreeID + validUUID,
treeID: validTreeID,
wantErr: false,
},
// The following is the current typical Rekor behavior.
{
name: "valid match with request entry UUID",
requestUUID: validTreeID + validUUID,
responseUUID: validUUID,
treeID: validTreeID,
wantErr: false,
},
{
name: "valid match with request UUID",
requestUUID: validUUID,
responseUUID: validUUID,
treeID: validTreeID,
wantErr: false,
},
{
name: "valid match with response entry UUID",
requestUUID: validUUID,
responseUUID: validTreeID + validUUID,
treeID: validTreeID,
wantErr: false,
},
{
name: "mismatch uuid with response tree id",
requestUUID: validUUID,
responseUUID: validTreeID + validUUID1,
treeID: validTreeID,
wantErr: true,
},
{
name: "mismatch uuid with request tree id",
requestUUID: validTreeID + validUUID1,
responseUUID: validUUID,
treeID: validTreeID,
wantErr: true,
},
{
name: "mismatch tree id",
requestUUID: validTreeID + validUUID,
responseUUID: validUUID,
treeID: validTreeID1,
responseUUID: validTreeID1 + validUUID,
wantErr: true,
},
{
name: "invalid response tree id",
requestUUID: validTreeID + validUUID,
responseUUID: invalidTreeID + validUUID,
treeID: invalidTreeID,
wantErr: true,
},
{
name: "invalid request tree id",
requestUUID: invalidTreeID + validUUID,
responseUUID: validUUID,
treeID: invalidTreeID,
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := isExpectedResponseUUID(tt.requestUUID,
tt.responseUUID, tt.treeID); (got != nil) != tt.wantErr {
if got := isExpectedResponseUUID(tt.requestUUID, tt.responseUUID); (got != nil) != tt.wantErr {
t.Errorf("isExpectedResponseUUID() = %v, want %v", got, tt.wantErr)
}
})
Expand Down