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.

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

Fix idempotency error with signing #3371

Merged
merged 1 commit into from
Nov 18, 2023

Conversation

haydentherapper
Copy link
Contributor

@haydentherapper haydentherapper commented Nov 17, 2023

The expected behavior is to fetch an existing entry from the log when signing generates the same signature and that entry is uploaded to the log, so that signing is idempotent.

The bug was that the prepended shard ID was compared to the log ID. The log ID is a sha256 hash of the log's public key, which is not the same as the shard ID. Now we compare the prepended shard IDs from both the request and response entry UUID when present.

This was not caught in tests since we used a valid shard ID as input rather than the log ID.

Verified by generating an RSA key (which will generate the same signature given the same input) and signing a container.

Fixes #3356

Summary

Release Note

Documentation

The expected behavior is to fetch an existing entry from the log when
signing generates the same signature, so that signing is idempotent.

The bug was that the prepended shard ID was compared to the log ID. The
log ID is a sha256 hash of the log's public key, which is not the same
as the shard ID. Now we compare the prepended shard IDs from both the
request and response entry UUID when present.

This was not caught in tests since we used a valid shard ID as input
rather than the log ID.

Verified by generating an RSA key (which will generate the same
signature given the same input) and signing a container.

Signed-off-by: Hayden Blauzvern <[email protected]>
Copy link

codecov bot commented Nov 17, 2023

Codecov Report

Attention: 22 lines in your changes are missing coverage. Please review.

Comparison is base (f2300e0) 30.26% compared to head (e7dd433) 30.23%.
Report is 1 commits behind head on main.

Files Patch % Lines
cmd/cosign/cli/verify.go 0.00% 16 Missing ⚠️
cmd/cosign/cli/options/verify.go 0.00% 3 Missing ⚠️
pkg/cosign/tlog.go 78.57% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3371      +/-   ##
==========================================
- Coverage   30.26%   30.23%   -0.04%     
==========================================
  Files         155      155              
  Lines        9941     9958      +17     
==========================================
+ Hits         3009     3011       +2     
- Misses       6482     6497      +15     
  Partials      450      450              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@haydentherapper haydentherapper merged commit 2a34b4c into sigstore:main Nov 18, 2023
28 checks passed
@haydentherapper haydentherapper deleted the fix-shard-id-comp branch November 18, 2023 04:01
@github-actions github-actions bot added this to the v2.3.0 milestone Nov 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Signing with tlog upload is not idempotent
2 participants