Skip to content

Conversation

@ret2libc
Copy link
Contributor

@ret2libc ret2libc commented Mar 4, 2025

Summary

Closes #1953

Release Note

Documentation

This is a breaking change because before all algorithms used SHA256 (except ED25519 which cannot work with it). Now, based on the public key type, a different hash algorithm might be used. In practice, only ECDSA/P384 and ECDSA/P521 are going to be affected by this change because before they were using SHA256 and with this patch they are going to use SHA384 and SHA512.

Considering ECDSA/P256 is the de-facto standard so far for sigstore clients, most users won't notice any difference. However, Cosign supports BYOK, so some users might use ECDSA/P384 (or P521) keys to request Fulcio certificates. Cosign needs to be updated to use the correct hash algorithm based on the key.

@ret2libc ret2libc requested review from a team as code owners March 4, 2025 16:18
Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

LGTM pending sigstore/sigstore PR being merged, and pending the discussion about ed25519 defaults.

@codecov
Copy link

codecov bot commented Mar 5, 2025

Codecov Report

Attention: Patch coverage is 64.28571% with 5 lines in your changes missing coverage. Please review.

Project coverage is 43.56%. Comparing base (cf238ac) to head (43ab45d).
Report is 311 commits behind head on main.

Files with missing lines Patch % Lines
pkg/server/grpc_server.go 54.54% 3 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1959       +/-   ##
===========================================
- Coverage   57.93%   43.56%   -14.38%     
===========================================
  Files          50       73       +23     
  Lines        3119     5723     +2604     
===========================================
+ Hits         1807     2493      +686     
- Misses       1154     2999     +1845     
- Partials      158      231       +73     

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

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ret2libc
Copy link
Contributor Author

ret2libc commented Mar 5, 2025

TBH I'm a bit unsure what's the problem with the golangci-lint. I don't have any issue locally and those reported issues seems to be false positives to me.

@ret2libc ret2libc force-pushed the pop-agility branch 2 times, most recently from 56594f6 to 9bb539a Compare March 10, 2025 17:16
Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

Thanks!

@haydentherapper haydentherapper merged commit e29595f into sigstore:main Mar 11, 2025
13 checks passed
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.

Algorithms that should not use SHA256 are not permitted

2 participants