mpp_make_prime takes an mp_int named "start" which it uses as the starting point to search for a prime. (Note that this is one of the rare cases where an input variable is NOT const. mpp_make_prime modifies it as it goes along, and if mpp_make_prime succeeds in finding a prime, it uses "start" to return the value.) mpp_make_prime searches the space [start..start+64k] for a prime. If it does not find a prime in that space, it returns MP_NO, and that should be treated like SEC_ERROR_NEED_RANDOM; that is, it should cause another attempt to generate that prime. In RSA_NewKey, the calls to mpp_make_prime are wrapped with CHECK_MPI_OK(). This jumps to cleanup if the function returns anything but MP_OKAY. If/when mpp_make_prime returns MP_NO, the caller should generate a new random value for "start" and try again. I'd suggest this code be structured so that if you've gotten one prime, and the attempt to get the other prime fails (that is, you've got p, and mpp_make_prime returns MP_NO for q), that you regenerate only q, and not p. Also, It would be a good idea, I think, to put some kind of counter in RSA_NewKey to limit the number of times that it tries to find a prime or to generate a key pair from the primes. When mpp_make_prime returns MP_NO, it means that mpp_make_prime could not find any prime in the 64k number space. That may be legitimate (i.e., it may indeed be that there are no primes in a particular 64k space), or it may be that mpp_make_prime is broken, and is failing to find primes when primes actually do exist. If mpp_make_prime returns MP_NO, we want to retry the operation (that is, create a new random start value and call mpp_make_prime again) some number of times, but not inifinitely. That way, if a bug arises that causes mpp_make_prime to always return MP_NO, RSA_NewKey won't become an infinite loop.
Priority: -- → P2
Target Milestone: --- → 3.2
Created attachment 22150 [details] [diff] [review] splits up generation of p and q, retries on MP_NO
Might I suggest a subroutine instead of duplicted code for generating primes p and q?
I reviewed this patch. It looks correct. The 4 lines immediately following the call to mpp_make_prime in generate_prime() are equivalent to the following: if (err != MP_NO) break; but it's OK to leave them as is.
the fix has been checked in.
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.