Skip to content

Conversation

@mkmks
Copy link

@mkmks mkmks commented Jan 13, 2026

This PR adds ECSDA with secp256k1 to the list of allowed certificate signature algorithms. The use case for it is explained in rustls/pki-types#96.

While it was pointed out in the discussion under the related rustls/webpki#427 PR that the secp256k1 curve isn't mentioned in any standardised TLS version, rcgen is meant to be a certificate generation library agnostic to the use of certificates generated with it. X.509 certificates aren't only used in TLS, so, perhaps, the same argument won't hold here.

For comparison, OpenSSL and its forks do support signing certificates with ECDSA using the secp256k1 curve. It'd be nice to be able to do the same with rcgen.

This PR also happens to fix #400 (since it requires the aws_lc_rs feature), or so it seems to its author.

@djc
Copy link
Member

djc commented Jan 13, 2026

Without this change, is it still possible to generate secp256k1 certificates with rcgen? If so, how much boilerplate is there?

@mkmks
Copy link
Author

mkmks commented Jan 13, 2026

I could try creating an ECDSA/secpk256k1 KeyPair by hand, without using KeyPair::generate_for(). However, that would require constructing a KeyPairKind which has only pub(crate) visibility in rcgen and a SignAlgo (same pub(crate) visibility), so it's still impossible to do entirely without modyfing rcgen.

I can see that having to modify rcgen to be able to use arbitrary algorithms from the underlying aws_lc_rs library isn't ideal from the maintainability point of view, but, unfortunately, I don't see a better approach at this moment in the current rcgen design than putting additional SignatureAlgorithm values in the crate.

Still, being able to use arbitrary signature algorithms already supported by the underlying cryptographic libraries anywhere seems not to be unreasonable. OpenSSL and its forks do allow it, after all (although by the virtue of developing the cryptography, the PKI and the TLS implementations in the same project).

@djc
Copy link
Member

djc commented Jan 13, 2026

I think you could just implement SigningKey and PublicKeyData for a type that wraps an underlying key pair, which seems fairly straightforward and reasonable? Not sure if many people have done it so happy to hear about speed bumps with that direction.

@mkmks
Copy link
Author

mkmks commented Jan 22, 2026

@djc So, I've tried implementing SigningKey and PublicKeyData, and I don't think it's possible without modifying the rcgen anyway.

Implementing SigningKey is, indeed, straightforward.

Implementing PublicKeyData is somewhat problematic. Implementing PublicKeyData::algorithm() requires constructing a SignatureAlgorithm value. That requires constructing a SignAlgo value and a SignatureAlgorithmParams value. Alas, both SignAlgo and SignatureAlgorithmParams types have pub(crate) visibility.

It makes sense to me that these types aren't public in rcgen because who'd normally need constructing a new SignatureAlgorithm when there are so many already? However, it can happen that not everything that is implemented in the underlying cryptographic library (such as aws_lc_rs) has a corresponding SignatureAlgorithm value in rcgen. Which is exactly my case!

The solution that I'd take would be to simply add another SignatureAlgorithm value in rcgen (which is what this PR does) but if there's a restriction of which algorithms from aws_lc_rs should be allowed for use in rcgen, I could, of course, add it to my own project, if it were also possible to define SignatureAlgorithm values outside of rcgen.

Would it be preferable to open another PR that would change the visibility of SignAlgo and SignatureAlgorithmParams from pub(crate) to pub?

P.S. I've also just noticed that b370764 exposes signing algorithms already implemented in aws_lc_rs but not standardised in TLS 1.3. This looks very much like what I'm trying to achieve with this PR (although for a different signing algorithm).

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.

latest release (0.14.6) broke aws-lc-rs users due to misuse of verify propagation

2 participants