Skip to content

add RSA key size to SignatureAlgorithm - #452

Open
jgreeer wants to merge 2 commits into
rustls:mainfrom
jgreeer:rsa-key-size-sig-algos
Open

add RSA key size to SignatureAlgorithm#452
jgreeer wants to merge 2 commits into
rustls:mainfrom
jgreeer:rsa-key-size-sig-algos

Conversation

@jgreeer

@jgreeer jgreeer commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR adds support for having separate SignatureAlgorithm values for different key sizes. It adds a field rsa_key_size to the SignatureAlgorithm struct. We create a static SignatureAlgorithm struct for each combination of hash (SHA256, SHA384, SHA512) + key size (2048, 3072, 4096) adding 6 new algorithms. As a side effect, we have to pass rsa_key_size: None to the other algorithm structs.

The reason for this change is in key_pair.rs, you can now use the generate_for method to generate RSA key pairs of different key sizes by giving different SignatureAlgorithm arguments. This removes the need for generate_rsa_for, so we deprecate it in the same way RcgenError was. Since RsaKeySize is always used now, we removed the aws-lc-rs and crypto gating.

The rsa_key_size field is not used it the identity of the key, because it would mess up loading the key, and we don't want to mess with the OID. At load time, like in from_der_and_sign_algo, the user can give an RSA SigAlg value of any key size since the key size is later retrieved using rsa_key_pair_public_modulus_len.

Tests

We add a test in lib.rs to verify that RSA algs of different key sizes are equal to each other.

@jgreeer
jgreeer force-pushed the rsa-key-size-sig-algos branch from 6498b14 to a8096fa Compare September 9, 2026 22:03

@djc djc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this makes sense with some tweaks?

Comment thread rcgen/src/key_pair.rs Outdated
Comment thread rcgen/src/sign_algo.rs
Comment on lines +42 to +43
// Only read under `aws_lc_rs`. Not part of identity.
#[allow(dead_code)]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How about we just make this conditional on #[cfg(feature = "aws_lc_rs")]? Seems straightforward?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we'd have to revert this back when we add the pluggable crypto provider right? not a big deal but it's extensible in it's current form already.

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.

2 participants