Implement default signing algorithms based on the key type#2014
Implement default signing algorithms based on the key type#2014Hayden-IO merged 4 commits intosigstore:mainfrom
Conversation
Signed-off-by: Riccardo Schirone <riccardo.schirone@trailofbits.com>
Signed-off-by: Riccardo Schirone <riccardo.schirone@trailofbits.com>
Signed-off-by: Riccardo Schirone <riccardo.schirone@trailofbits.com>
|
I'm considering adding a |
Hayden-IO
left a comment
There was a problem hiding this comment.
I like the suggestion to add Default to the method names. Otherwise this looks great.
|
Would you like me to merge as-is, or update the func names now with |
I was planning on doing a bunch of changes tomorrow to the various PRs, but I can do this now! |
|
No rush, just wanted to check! |
Signed-off-by: Riccardo Schirone <riccardo.schirone@trailofbits.com>
eiffel-fl
left a comment
There was a problem hiding this comment.
Hi,
We use sigstore in our project and I always take a look at the release notes while bumping the package version.
This time, I have some comments, but nothing critical.
Best regards,
| case elliptic.P384(): | ||
| return v1.PublicKeyDetails_PKIX_ECDSA_P384_SHA_384, nil | ||
| case elliptic.P521(): | ||
| return v1.PublicKeyDetails_PKIX_ECDSA_P521_SHA_512, nil |
There was a problem hiding this comment.
There is a typo in the name of this const:
PublicKeyDetails_PKIX_ECDSA_P521_SHA_512 -> PublicKeyDetails_PKIX_ECDSA_P512_SHA_512
There was a problem hiding this comment.
it is ECDSA P521 not P512 , see https://csrc.nist.gov/CSRC/media/Projects/Cryptographic-Standards-and-Guidelines/documents/examples/P521_SHA512.pdf
| filteredOpts := []LoadOption{options.WithHash(algorithmDetails.hashType)} | ||
| for _, opt := range opts { | ||
| var useED25519ph bool | ||
| var rsaPSSOptions *rsa.PSSOptions | ||
| opt.ApplyED25519ph(&useED25519ph) | ||
| opt.ApplyRSAPSS(&rsaPSSOptions) | ||
| if useED25519ph || rsaPSSOptions != nil { | ||
| filteredOpts = append(filteredOpts, opt) | ||
| } | ||
| } |
There was a problem hiding this comment.
You repeat this code in verifier.go, it would be easier for maintenance to factorize it in a function and then call this function from these two places.
The function could just return a filteredOpts.
Summary
As we make the Sigstore ecosystem more crypto agile, we realized that right now it is using some uncommon algorithms, like ECDSA-P384 with the SHA256 hash algorithm. However, we want the crypto-agility to be controlled and limited to a set of algorithms (see
AlgorithmRegistry). Moreover, we want the agility to be mainly about supporting multiple key types, rather than multiple algorithms for the same key type. Thus, this PR introduces a single point where the defaults are defined.This is going to simplify other changes across the Sigstore ecosystem, given that often enough the only information you have is the public key. Being able to derive the signing algorithm to use (or used to sign a blob) means we don't need to store or pass around any extra information (e.g. the hash function), because that can be derived automatically from the key.
See sigstore/sig-clients#16 .
Release Note
AlgorithmDetailsfor public keys based on their types.