Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minimum modulus size #445

Open
ryancdotorg opened this issue Aug 9, 2024 · 11 comments
Open

Minimum modulus size #445

ryancdotorg opened this issue Aug 9, 2024 · 11 comments

Comments

@ryancdotorg
Copy link

ryancdotorg commented Aug 9, 2024

RSA is catastrophically weak with 512 bit keys, and anything less than 2048 bits is insecure. A minimum key size should be enforced. Golang has proposed to require 1024 bits minimum:

golang/go#68762

however 2048 would be a good choice where backwards compatibility requirements are minimal

@tarcieri
Copy link
Member

tarcieri commented Aug 9, 2024

See also: #350

@dignifiedquire
Copy link
Member

For testing and supporting existing implementations I don't think a blanket limit makes sense.
But maybe a default generator that limits the range would be helpful for newcomers.

@ryancdotorg
Copy link
Author

ryancdotorg commented Aug 9, 2024 via email

@dignifiedquire
Copy link
Member

Not supporting existing implementations which would require this is a benefit.

I am glad for you that this is how you can run your projects, it is not the same for all of us.

@str4d
Copy link
Contributor

str4d commented Aug 9, 2024

I agree with @ryancdotorg and with the rationales outlined in the Go proposal.

I also note that the rsa crate already enforces limits on key sizes, specifically RsaPublicKey::new which returns an error for any key larger than 4096 bits. This method has a trapdoor for configurability via RsaPublicKey::new_with_max_size.

An intermediate pathway would therefore be to do the same thing for minimum sizes:

  • Have the default pathway enforce a secure minimum size in RsaPrivateKey::new and RsaPublicKey::new, which we can increase as necessary in line with best practices.
  • Have a trapdoor for configurability via e.g. RsaPrivateKey::new_with_insecure_key_size, which enables reviewers to clearly identify codepaths that are insecure in production.

Then the question of whether to expose the insecure / hazmat configuration APIs at all can be separated from the "stop people shooting themselves in the foot because they think they are getting 256-bit security" issue.

@ryancdotorg
Copy link
Author

For reference, here's an example of use of 512 bit RSA that could have damaged a power grid:

https://rya.nc/vpp-hack.html

It's been known to basically worthless in terms of security for decades, let it die.

@pinkforest
Copy link

pinkforest commented Aug 11, 2024

Mis-use resistance would be good - similar with x25519-dalek we added static_secrets opt-in to track it's use.

One way would be to split the constructor for insecure and secure constructions - the insecure constructor for given bit-lengths would be hidden behind a feature - however later on how this whole thing evolves through time requiring approach to avoid breaking changes.

By having to use a feature (or cfg) to override mis-use resistance it could be potentially tracked in at least feature usage - now it is non-trivial (still possible but hard to maintain to track it's use) to find any direct dependencies which use insecure sizes - where as using a feature it can be tracked through analysing crates.io-index that exposes the used features.

When we put static_secrets feat to x25519-dalek, I found several cases of where people were using it wrong.

If we take this to extreme - there could be even a crate with all the possible keysizes as marker types and the type only gets used through feature / cfg and then features can track which keysizes are supported.

e.g. marker type could be categorised as known "minimums" with the size embedded in data-enum BITS_2048(...) 🤔

Also according to some literature 2048 bits is considered obsolete by 2030 - should this be taken into account ? Having this evolution built in without inducing breaking changes when the inevitable factorizations happen - tick tock.

So for this reason I would say having "well known minimums" categories as types could be ok for future backward compat w/o having to do breaking changes allowing depreciation of surface that leads to insecure constructions.

@teknalb
Copy link

teknalb commented Aug 15, 2024

I strongly disagree with this restriction the library should remain as flexible as possible; modulus size is a user's choice.
As an alternative I'd suggest informing the user with a warning or something if the key size is known to be broken.

@ryancdotorg
Copy link
Author

ryancdotorg commented Aug 15, 2024 via email

@teknalb
Copy link

teknalb commented Aug 15, 2024

a feature flag for broken/weak sounds good idea without sacrificing flexibility :)

@tarcieri
Copy link
Member

I'll +1 @str4d's suggestion that we should follow what we already do with RsaPublicKey and have checked RsaPrivateKey APIs with short, convenient names which enforce a minimum modulus, and parallel unchecked APIs with longer names that don't enforce the restriction

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

No branches or pull requests

6 participants