increase rsa min bits to 1024

[ryancdotorg]

Per #25092, prevents generation of RSA keys smaller than 1024, though still allows existing keys to be used.

Please let me know if a CLA is needed - this seems like a trivial change to me, but... 🤷

Checklist

• documentation is added or updated
• tests are added or updated

[nhorman]

Can you please sign an icla.so we can review this?

[slontis]

This is a breaking change. Might need a discussion by OTC. It would require a CHANGES entry.

[t8m]

OTC: Do we want such change, if so, when?

[t8m]

This is also not acceptable with CLA: trivial. Would you please consider signing a CLA? https://openssl-library.org/policies/cla/index.html

[ryancdotorg]

@nhorman @t8m Thank you for clarifying regarding triviality, I've signed the CLA and will email it over later today after I scan it.

@slontis I will add a CHANGES entry.

@slontis @t8m To contextualize for discussion, last month I came across a recently built system that falls under "critical infrastructure" which was using 512 bit RSA signatures for access control. The details will be published soon. It is my opinion that making this error should have required considerably more effort than running openssl genrsa 512.

I've been running across production use of 512 bit RSA keys repeatedly since 2012 - they seem to be properly dead for TLS, but they still turn up in things like DKIM, DNSSEC, and JWT.

Time to crack on a single mid-range PC is now under a week. I'm pretty sure if I tried to speed run it, I could crack a key in under two hours for the cost of a couple pizzas.

Rather than playing whack-a-mole, I would prefer to address this systemically. This is not specifically an OpenSSL problem, but OpenSSL seems to be the place to make the biggest impact. I'm planning to request similar changes in other cryptography libraries.

[ryancdotorg]

If there are breakage concerns, the OpenSSL command line tools could be changed to print a large warning and generate a 1024 bit key when a smaller one is requested. This would avoid breaking test scripts that use 512 bit keys, however this behaviour blatantly violates "principle of least surprise", and probably isn't a good idea, but may still be better than the status quo.

[ryancdotorg]

It looks like this is failing with one of the FIPS tests, and I'm not sure how to fix that, can someone please advise?

[t8m]

It looks like this is failing with one of the FIPS tests, and I'm not sure how to fix that, can someone please advise?

The problem is the FIPS provider still allows signature verification (nothing else) with 1024 bit keys for legacy purposes. As you changed the non-fips key to have 1024 bits instead of 512 bits, it will now pass the verification with the FIPS provider.

[ryancdotorg]

@t8m Ah. How about bundling a 512 bit public key and signature rather than generating it when the tests are run, then?

[t8m]

@t8m Ah. How about bundling a 512 bit public key and signature rather than generating it when the tests are run, then?

That would certainly work.

[ryancdotorg]

@t8m I probably still remember enough perl to make that change to the fips test. Would it be possible to get some idea of how OTC is inclined on this before I put more work into this PR?

[nhorman]

next OTC meeting is a week from today, their response will be posted here.

[ryancdotorg]

I've emailed the CLA over.

For context, I'm asking for this due to use observed in the wild:

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

[t8m]

OTC: We've discussed this and we do not think this can be applied in 3.4.

A project issue created to make this minimum configurable:
openssl/project#809

[t8m]

OTC: We will not apply this PR in this form.

Changing the minimum key size will break some use cases - most likely testing. On the other hand 1024 bits is not secure sufficiently anyway so eventually only 2048 bit and above keys should be generated. However this would break even more legacy use cases. To allow them but not allow generating insecure keys by default these minimum (and possibly maximum) key sizes should be made configurable.

Either we have to make this configurable or this will have to be postponed to 4.0.

[ryancdotorg]

I think there's a strong argument to be made that allowing 512 bit RSA keys to be generated is worse than any breakage that would be caused by not doing so at this point. Searching github for projects generating 512 bit tests keys doesn't return very many results.

I don't think making this configurable at runtime make sense - changes required to test with larger keys are minimal, and unlikely to be more significant than adding the code to change the limit.

What about an enable-broken-rsa compile time flag that lowers the limit to 512 bit, and defaulting to 1024 bit for 3.4, and plan to do something more polished in 4.0?

[nhorman]

If we were just updating from 512 to 1024 that might be acceptable, we could just fix up the test cases (you can see the failures in the CI jobs below), but there are still lots of uses of openssl for 1024 bit keys that we need to support. Given that the consensus desire is a default minimum of 2048, and that these need to be run time settable to a lower value for legacy cases, a config setting makes more sense (note that builders of openssl are not always the end users, and the former may opt for a higher default that end users may not find agreeable).

The approach I would take would be:

1. add a static global variable to crypto/rsa/rsa_gen.c (ossl_max_modulus_bits = 2048)
2. in rsa_keygen, add a check for the bits parameter. If its less than that value, raise an error and fail (this should cover all use cases for key generation, both programatic, and via the openssl apps themselves)
3. in rsa_gen.c, add a conf file module init/finish function (rsa_conf_module_init / rsa_conf_module_finish perhaps). Give it a signature such that its passable to CONF_module_add, and implement them to parse out a max bits integer parameter, and assign any found value to oss_max_modulus_bits as created in (1)
4. Add a call to CONF_module add with these functions to OPENSSL_load_builtin_modules
5. document it in the openssl-config pod file

That should allow us to augment the default config file such that users can set:

[rsa_section]
max_bits = 1024

If they have a need for legacy keys

[ryancdotorg]

I don't agree that it should be possible to enable 512 bit RSA at runtime, but allowing the limit to be lowered to 1024 bit via a config option makes sense.

What about...

• runtime default to 1024 bits for key generation in 3.4.0
• update example config to set

  [rsa_section]
  generate_min_modulus_bits = 2048
  

• completely disable RSA below 1024 bits at compile time by default, with the enable-broken-rsa flag to turn it back on

I think I can implement that. This would still allow existing smaller keys to be used for the time being - that could be addressed later.

[paulidale]

The config option should be per libctx I suspect.

[nhorman]

As long as there is a per-ctx settable config option for the min modulus bits, the rest is reasonable

[xnox]

thinking about this and all other similar cases we really need SECURITY_LEVEL to be a thing for all algorithms. And then distributions can compile the default security level, allow or not to lower it, and users can opt into lowering it too.
Note half of fips changes would go away if we had SECURITY_LEVEL not just for TLS but for all primitives too. The current transition in FIPS module for 140-3 effectively is what this PR tries to achieve. Not just for RSA key sizes but for all other things too (hashes, digests, HMAC keys, KDF input keys, and so on). And the fips changes will have to be repeated again for the next transition to 128 bit level of security (~= SECURITY_LEVEL 3).

@ryancdotorg if you want this today, enable-fips and use fips provider always, even if it is not certified it has a higher floor for keygeneration and encryption and many other things.

The fips indicator framework is nice too. If we implement it for the default provider, we could signal to all users if their operation meets desired SECURITY_LEVEL or not.

It would also make FIPS code easier to maintain, as lots of things would just fall off by themselves upon setting security level to 3.