Allow configurable client signing algorithms #1517
Conversation
|
@woodruffw @ret2libc |
|
I've shown in d95ff74 how we can use an enum in |
|
xref: sigstore/protobuf-specs#189 will ultimately standardize the full registry of signing algorithms, but doesn't block initial work here (since we know a couple of ECDSA variants that'll need verification support already). |
|
I'll follow up with some unit tests tomorrow. |
cmd/app/serve.go
Outdated
| @@ -106,6 +109,7 @@ func newServeCmd() *cobra.Command { | |||
| cmd.Flags().Duration("read-header-timeout", 10*time.Second, "The time allowed to read the headers of the requests in seconds") | |||
| cmd.Flags().String("grpc-tls-certificate", "", "the certificate file to use for secure connections - only applies to grpc-port") | |||
| cmd.Flags().String("grpc-tls-key", "", "the private key file to use for secure connections (without passphrase) - only applies to grpc-port") | |||
| cmd.Flags().StringSlice("client-signing-algorithms", buildDefaultClientSigningAlgorithms([]v1.KnownSignatureAlgorithm{v1.KnownSignatureAlgorithm_ECDSA_SHA2_256_NISTP256, v1.KnownSignatureAlgorithm_ECDSA_SHA2_384_NISTP384, v1.KnownSignatureAlgorithm_ECDSA_SHA2_512_NISTP521, v1.KnownSignatureAlgorithm_ED25519}), "the list of allowed client signing algorithms") | |||
There was a problem hiding this comment.
Shall we default to the 256 algos only right now? I think that's the most conservative option as it does not add new algos. Other SHAs were not working before.
There was a problem hiding this comment.
Wondering the same for Rekor as well.
There was a problem hiding this comment.
Good point, I think that'd be a reasonable default.
| @@ -1182,6 +1188,94 @@ func TestAPIWithIssuerClaimConfig(t *testing.T) { | |||
| } | |||
| } | |||
|
|
|||
| // Tests API with an RSA key | |||
There was a problem hiding this comment.
I didn't want to unit test the algorithm registry code too heavily here since that's sigstore/sigstore's responsibility.
I'm focusing on:
- Testing that other key types work (RSA in this case).
- Testing that non-permitted algorithms are rejected (ECDSA with P521 curve in this case).
- Testing both the CSR and non-CSR paths.
Co-authored-by: Alex Cameron <asc@tetsuo.sh> Co-authored-by: Riccardo Schirone <riccardo.schirone@trailofbits.com> Signed-off-by: Alex Cameron <asc@tetsuo.sh> Signed-off-by: Riccardo Schirone <riccardo.schirone@trailofbits.com>
ret2libc
force-pushed
the
alex/configurable-crypto
branch
from
e4d084e to
117abc4
Compare
Signed-off-by: Riccardo Schirone <riccardo.schirone@trailofbits.com>
Signed-off-by: Riccardo Schirone <riccardo.schirone@trailofbits.com>
|
I think we can close this in favour of #1938 |
Summary
This PR adds a
--client-signing-algorithmsflag to Fulcio to restrict what key/hash combinations are allowed.Release Note
Documentation