Use as small dh key size as possible to support the security #18480
Conversation
t8m
added
branch: master
|
As this fixes a serious performance regression with ffdhe DH parameters in openssl-3.0 when compared to 1.1.1. I regard it as a bug fix. |
github-actions
bot
added
the
severity: fips change
|
We have 3 callers of ossl_ffc_generate_private_key(), and 2 of them set N to qbits, so still generating a too large key. They call it with a fixed s, I think the callers of ossl_ffc_generate_private_key() should use the ossl_ifc_ffc_compute_security_bits() to set s and probably set N to 0. |
No! That would be wrong. These other two callers use small q and the call with q bits as N is correct. |
t8m
force-pushed
the
dh-key-bits-default
branch
3 times, most recently
from
ae2f030
to
3141786
Compare
|
I'm currently not convinced they only call it with a small q, I guess I need to take a closer look. Maybe it can be useful to have a function that properly calculates the security strength based on p, q and g. We now pass a security strength of 80 or 112 depending on fips mode. But if we pass an N that's correct, it doesn't matter, s isn't really used. |
That can be another PR. |
|
ping for reviews |
There was a problem hiding this comment.
Note that RFC 7919 is quite clear that all its named groups use safe primes.
The analogous procedures for SSH in RFC 4419 also restrict the server to picking safe primes, and justify the use of short exponents with a reference to (the conference version of) https://people.scs.carleton.ca/~paulv/papers/Euro96-DH.pdf. The conclusion of that paper seems to pretty clearly state that short exponents are "clearly insecure" when random primes area used.
I think the parts of this change to store the keylength in the named group structure and use the RFC 7919 values is good, but I'm pretty concerned that we're being too aggressive with the switch to 2*security_bits for the general (non-safe-prime) case.
I've dropped that arbitrary parameters part. |
paulidale
removed
the
approval: otc review pending
|
@mattcaswell ping for approval, I think your questions were answered. |
Longer private key sizes unnecessarily raise the cycles needed to compute the shared secret without any increase of the real security. We use minimum key sizes as defined in RFC7919. For arbitrary parameters we cannot know whether they are safe primes (we could test but that would be too inefficient) we have to keep generating large keys. However we now set a small dh->length when we are generating safe prime parameters because we know it is safe to use small keys with them. That means users need to regenerate the parameters if they want to take the performance advantage of small private key.
|
Rebased to resolve conflict in CHANGES.md. @paulidale please re-approve. Ping for second review. |
|
Just thought of another thing... Does this need to survive the import export dance? |
The params.keylength will get reinitialized via the ossl_dh_cache_named_group() so IMO this case is covered. |
t8m
added
approval: done
openssl-machine
removed
the
approval: done
|
This pull request is ready to merge |
openssl-machine
added
the
approval: ready to merge
|
Merged to master. Thank you. |
Longer private key sizes unnecessarily raise the cycles needed to compute the shared secret without any increase of the real security. We use minimum key sizes as defined in RFC7919. For arbitrary parameters we cannot know whether they are safe primes (we could test but that would be too inefficient) we have to keep generating large keys. However we now set a small dh->length when we are generating safe prime parameters because we know it is safe to use small keys with them. That means users need to regenerate the parameters if they want to take the performance advantage of small private key. Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #18480)
Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #18480)
…rint it Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #18480)
Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #18480)
Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #18480)
Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #18480)
Longer private key sizes unnecessarily raise the cycles needed to compute the shared secret without any increase of the real security. We use minimum key sizes as defined in RFC7919. For arbitrary parameters we cannot know whether they are safe primes (we could test but that would be too inefficient) we have to keep generating large keys. However we now set a small dh->length when we are generating safe prime parameters because we know it is safe to use small keys with them. That means users need to regenerate the parameters if they want to take the performance advantage of small private key. Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from openssl#18480)
Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from openssl#18480)
…rint it Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from openssl#18480)
Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from openssl#18480)
Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from openssl#18480)
Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from openssl#18480)
Longer private key sizes unnecessarily raise the cycles needed to compute the shared secret without any increase of the real security. We use minimum key sizes as defined in RFC7919. For arbitrary parameters we cannot know whether they are safe primes (we could test but that would be too inefficient) we have to keep generating large keys. However we now set a small dh->length when we are generating safe prime parameters because we know it is safe to use small keys with them. That means users need to regenerate the parameters if they want to take the performance advantage of small private key. Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from openssl#18480) (cherry picked from commit ddb13b2)
…rint it Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from openssl#18480) (cherry picked from commit 2b11a8e)
Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from openssl#18480) (cherry picked from commit 2885b2c)
Longer private key sizes unnecessarily raise the cycles needed to compute the shared secret without any increase of the real security. We use minimum key sizes as defined in RFC7919. For arbitrary parameters we cannot know whether they are safe primes (we could test but that would be too inefficient) we have to keep generating large keys. However we now set a small dh->length when we are generating safe prime parameters because we know it is safe to use small keys with them. That means users need to regenerate the parameters if they want to take the performance advantage of small private key. Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #18480) (cherry picked from commit ddb13b2)
Longer private key sizes unnecessarily raise the cycles needed to
compute the shared secret without any increase of the real security.
This also fixes a regression where we weren't printing the recommended-private-length
in the DH parameters/key text output.