From dc6e7624154809340fb38fc884ad30d840a3ff5e Mon Sep 17 00:00:00 2001 From: Paul Kehrer Date: Sun, 9 Jul 2017 23:20:35 -0500 Subject: 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! --- src/_cffi_src/openssl/dh.py | 2 ++ .../hazmat/backends/openssl/backend.py | 17 ++++++++++++-- tests/hazmat/primitives/test_dh.py | 26 +++++++++++++++++++++- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/_cffi_src/openssl/dh.py b/src/_cffi_src/openssl/dh.py index be761b97..7ab06ae9 100644 --- a/src/_cffi_src/openssl/dh.py +++ b/src/_cffi_src/openssl/dh.py @@ -10,6 +10,8 @@ INCLUDES = """ 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 878bbe43..6c9ef84f 100644 --- a/src/cryptography/hazmat/backends/openssl/backend.py +++ b/src/cryptography/hazmat/backends/openssl/backend.py @@ -1776,8 +1776,21 @@ class Backend(object): 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 1fdabe57..fa658ae5 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 @@ -158,6 +159,24 @@ class TestDH(object): assert backend.dh_parameters_supported(23, 5) assert not backend.dh_parameters_supported(23, 18) + @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( @@ -202,7 +221,12 @@ class TestDH(object): 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) -- cgit v1.2.3