Remove DH check from load_dh_private_numbers

[reaperhulk]

DH_check turns out to block some bad values, but not all. It also blocks some good values. This causes problems. Removing this check will fix #3755 and #3364

Also, we don't attempt to check key consistency when loading from PEM or DER for DH anyway, so you could already load the rejected values via a previously serialized key.

Cryptography_DH_check is still called in dh_parameters_supported but I'm unconvinced we should call it there either. That can be debated later though.

[alex]

I haven't follow the DH development process (mostly because DH is 9000 times more complex than "just call X25519()"). I'm considered this removes all safety checks from this API, making it more footgunny than required.

If DH_check prevents certain perfectly good DH values, can we send a PR to OpenSSL to fix that?

[reaperhulk]

#3755 has several links including some conversations dating back to 2002 on the OpenSSL mailing list.

The OpenSSL check also won't check any generator other than 2, 3, or 5. According to #3364, OpenSSL itself uses a 2048-bit generator for 2048-bit DH. This returns DH_UNABLE_TO_CHECK_GENERATOR so we could just ignore that error and not check generators outside 2, 3, 5.

To resolve the specific example in #3755 we could alter our own custom Cryptography_DH_check to allow p % 24 to be either 11 or 23. Some rationale for why this is a reasonable thing to do is here: https://crypto.stackexchange.com/questions/12961/diffie-hellman-parameter-check-when-g-2-must-p-mod-24-11

[reaperhulk]

After thinking about this a bit longer I'd like to pursue a less extreme approach (at least at first).

To resolve #3364 I'll modify this PR to make the generator check in numbers be > 1 (since the generator is used in the equation g^kmod p a generator of less than 2 is unacceptable) and have us ignore DH_NOT_SUITABLE_GENERATOR for other values passed. This will also allow us to add the NIST vectors for testing (which use 512-bit and 1024-bit generators).

As a follow up to resolve #3755 I'll put in a PR to allow 23 as a p % 24 result. We'll CC lvh on that to get some confirmation that this is acceptable, but as far as I can tell it should be fine since it's just a choice between leaking a bit of the secret exponent (which OpenSSL chooses to do) or have your parameters only capable of generating half the possible values in Z^*_p. Those two seem logically equivalent, but I'd like to hear a cryptographer confirm that 😄

[reaperhulk]

Closing in favor of #3768 (and a possible future PR)