allow p % 24 == 23 when generator == 2 in DH_check (#3768)
* allow p % 24 == 23 when generator == 2 in DH_check
* short url
* update and expand comments
* even better language!
diff --git a/src/_cffi_src/openssl/dh.py b/src/_cffi_src/openssl/dh.py
index be761b9..7ab06ae 100644
--- a/src/_cffi_src/openssl/dh.py
+++ b/src/_cffi_src/openssl/dh.py
@@ -10,6 +10,8 @@
TYPES = """
typedef ... DH;
+
+const long DH_NOT_SUITABLE_GENERATOR;
"""
FUNCTIONS = """
diff --git a/src/cryptography/hazmat/backends/openssl/backend.py b/src/cryptography/hazmat/backends/openssl/backend.py
index 878bbe4..6c9ef84 100644
--- a/src/cryptography/hazmat/backends/openssl/backend.py
+++ b/src/cryptography/hazmat/backends/openssl/backend.py
@@ -1776,8 +1776,21 @@
res = self._lib.Cryptography_DH_check(dh_cdata, codes)
self.openssl_assert(res == 1)
- if codes[0] != 0:
- raise ValueError("DH private numbers did not pass safety checks.")
+ # DH_check will return DH_NOT_SUITABLE_GENERATOR if p % 24 does not
+ # equal 11 when the generator is 2 (a quadratic nonresidue).
+ # We want to ignore that error because p % 24 == 23 is also fine.
+ # Specifically, g is then a quadratic residue. Within the context of
+ # Diffie-Hellman this means it can only generate half the possible
+ # values. That sounds bad, but quadratic nonresidues leak a bit of
+ # the key to the attacker in exchange for having the full key space
+ # available. See: https://crypto.stackexchange.com/questions/12961
+ if codes[0] != 0 and not (
+ parameter_numbers.g == 2 and
+ codes[0] ^ self._lib.DH_NOT_SUITABLE_GENERATOR == 0
+ ):
+ raise ValueError(
+ "DH private numbers did not pass safety checks."
+ )
evp_pkey = self._dh_cdata_to_evp_pkey(dh_cdata)
diff --git a/tests/hazmat/primitives/test_dh.py b/tests/hazmat/primitives/test_dh.py
index 1fdabe5..fa658ae 100644
--- a/tests/hazmat/primitives/test_dh.py
+++ b/tests/hazmat/primitives/test_dh.py
@@ -4,6 +4,7 @@
from __future__ import absolute_import, division, print_function
+import binascii
import os
import pytest
@@ -161,6 +162,24 @@
@pytest.mark.parametrize(
"vector",
load_vectors_from_file(
+ os.path.join("asymmetric", "DH", "rfc3526.txt"),
+ load_nist_vectors
+ )
+ )
+ def test_dh_parameters_allows_rfc3526_groups(self, backend, vector):
+ p = int_from_bytes(binascii.unhexlify(vector["p"]), 'big')
+ params = dh.DHParameterNumbers(p, int(vector["g"]))
+ param = params.parameters(backend)
+ key = param.generate_private_key()
+ # This confirms that a key generated with this group
+ # will pass DH_check when we serialize and de-serialize it via
+ # the Numbers path.
+ roundtripped_key = key.private_numbers().private_key(backend)
+ assert key.private_numbers() == roundtripped_key.private_numbers()
+
+ @pytest.mark.parametrize(
+ "vector",
+ load_vectors_from_file(
os.path.join("asymmetric", "DH", "RFC5114.txt"),
load_nist_vectors))
def test_dh_parameters_supported_with_q(self, backend, vector):
@@ -202,7 +221,12 @@
dh.DHPrivateKeyWithSerialization)
def test_numbers_unsupported_parameters(self, backend):
- params = dh.DHParameterNumbers(23, 2)
+ # p is set to 21 because when calling private_key we want it to
+ # fail the DH_check call OpenSSL does. Originally this was 23, but
+ # we are allowing p % 24 to == 23 with this PR (see #3768 for more)
+ # By setting it to 21 it fails later in DH_check in a primality check
+ # which triggers the code path we want to test
+ params = dh.DHParameterNumbers(21, 2)
public = dh.DHPublicNumbers(1, params)
private = dh.DHPrivateNumbers(2, public)