Fixed unmatched BN_CTX_start/end if an invalid exponent is used.

[slontis]

Fixed unmatched BN_CTX_start/end if an invalid exponent is used.

Checklist

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

[kroeckx]

Why do we even have this code? I've asked about this in the original pull request, but it was never answered. This code is only needed when generating keys smaller then 2048 bits, the previous code we had was good for generating 2048 and larger keys.

[slontis]

Why do we even have this code? I've asked about this in the original pull request, but it was never answered. This code is only needed when generating keys smaller then 2048 bits, the previous code we had was good for generating 2048 and larger keys.

Hi Kurt,
I cant see where you specifically asked this question in the old PR. There are tables in the old FIPS 186-4 that only specify values for 2048 and 3072 (related to size of probable primes, and MR rounds). So values lower than 2048 are definitely not allowed.

The latest spec (SP800-56B r1 final) says this:
Any modulus with an even bit length that provides at least 112 bits of security strength may be used.
Doesnt that mean even values between 2048 and 3072 are allowed?

[kroeckx]

There is also a method that does not require 4 primes, where we just generate 2 random numbers. This is what the code did in 1.1.1. This other method is only allowed for keys >= 2048 bits, which are the also the only keys we're allowed to generate in FIPS mode. So all this new code was not needed.

It could be that any even number of bits >= 2048 is allowed if you're at the 112 bit level. I think if you create a key for a certain security level, that you should always use it at that level. Generating a 3072 bit key when you're actually in the 112 bit level might be confusing that they prefer you don't do that, and might prefer that you just generate a 2048 bit key instead.

Maybe the wording is there for the case where at some future point in time, for the 112 bit level 2048 bits is not enough, but that's really just a guess.

But I think the change is wrong. If you generate a 3000 bit RSA key with the new code, it would still only be at the 112 bit level, and not something closer to the 123 bit level. I guess it would make more sense if the API had the security level and not the size of the modulus.

This code also prevents you from actually generating keys that are stronger than the 128 bit level, while 192 and 256 are also defined.

[slontis]

I guess the difference with the new code is that it follows the standard exactly listing the steps as it goes without confusing code related to multi-primes. Please look at the code in rsa_sp800_56b_generate_key() and see which one is clearer :)
It could be changed to use the other method, but it would still need to be cleaned up. (as was done for the old fips module)

Yes there is currently no concept of security strength being passed to the keygen (it probably should be). I will change the code back to only do 2048, >3072. There is not much I can do about the >3072 case (there are only 2 table values and it just says to use the last value in the table for higher values in the FIPS140-2 IG - As stated as the comment at the top of the file).

[slontis]

ping
This issue has collapsed down to just a simple bug fix now.

[mspncp]

It would be nice if you could add your GitHub description to the body of your commit message, too.

Fixed unmatched BN_CTX_start/end if an invalid exponent is used.

(In accordance with the spirit of a distributed VCS our git logs should remain readable and selfcontained offline, without having to make a roundtrip to a centralized GitHub server.)

[slontis]

Renamed the commit description now that the bug is more specific.

[paulidale]

Merged, thanks all.