From 6be7b29b6a92a388714846d529e9365dc05425f5 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Mon, 8 Jul 2024 18:06:05 -0400 Subject: [PATCH] Fix exchange with keys that had Q automatically computed fixes #10790 closes #10864 --- docs/development/test-vectors.rst | 4 +++ src/rust/src/backend/dh.rs | 46 +++++++++++++++++++++++------- tests/hazmat/primitives/test_dh.py | 10 +++++++ 3 files changed, 50 insertions(+), 10 deletions(-) diff --git a/docs/development/test-vectors.rst b/docs/development/test-vectors.rst index 4f564d79b24f1..c906f611ceff8 100644 --- a/docs/development/test-vectors.rst +++ b/docs/development/test-vectors.rst @@ -224,6 +224,10 @@ Key exchange * ``vectors/cryptoraphy_vectors/asymmetric/ECDH/brainpool.txt`` contains Brainpool vectors from :rfc:`7027`. +* ``vectors/cryptography_vectors/asymmetric/DH/dhpub_cryptography_old.pem`` + contains a Diffie-Hellman public key generated with a previous version of + ``cryptography``. + X.509 ~~~~~ diff --git a/src/rust/src/backend/dh.rs b/src/rust/src/backend/dh.rs index e615d623ffa34..15688c9a4efc6 100644 --- a/src/rust/src/backend/dh.rs +++ b/src/rust/src/backend/dh.rs @@ -25,6 +25,8 @@ pub(crate) struct DHPublicKey { #[pyo3::pyclass(frozen, module = "cryptography.hazmat.bindings._rust.openssl.dh")] struct DHParameters { dh: openssl::dh::Dh, + + is_dhx: bool, } #[pyo3::pyfunction] @@ -51,7 +53,7 @@ fn generate_parameters( let dh = openssl::dh::Dh::generate_params(key_size, generator) .map_err(|_| pyo3::exceptions::PyValueError::new_err("Unable to generate DH parameters"))?; - Ok(DHParameters { dh }) + Ok(DHParameters { dh, is_dhx: false }) } pub(crate) fn private_key_from_pkey( @@ -73,12 +75,13 @@ pub(crate) fn public_key_from_pkey( #[cfg(not(CRYPTOGRAPHY_IS_BORINGSSL))] fn pkey_from_dh( dh: openssl::dh::Dh, + is_dhx: bool, ) -> CryptographyResult> { cfg_if::cfg_if! { if #[cfg(CRYPTOGRAPHY_IS_LIBRESSL)] { Ok(openssl::pkey::PKey::from_dh(dh)?) } else { - if dh.prime_q().is_some() { + if is_dhx { Ok(openssl::pkey::PKey::from_dhx(dh)?) } else { Ok(openssl::pkey::PKey::from_dh(dh)?) @@ -87,6 +90,16 @@ fn pkey_from_dh( } } +fn is_dhx(id: openssl::pkey::Id) -> bool { + cfg_if::cfg_if! { + if #[cfg(CRYPTOGRAPHY_IS_LIBRESSL)] { + false + } else { + id == openssl::pkey::Id::DHX + } + } +} + #[pyo3::pyfunction] #[pyo3(signature = (data, backend=None))] fn from_der_parameters( @@ -105,6 +118,7 @@ fn from_der_parameters( Ok(DHParameters { dh: openssl::dh::Dh::from_pqg(p, q, g)?, + is_dhx: asn1_params.q.is_some(), }) } @@ -214,7 +228,10 @@ impl DHPrivateKey { let orig_dh = self.pkey.dh().unwrap(); let dh = clone_dh(&orig_dh)?; - let pkey = pkey_from_dh(dh.set_public_key(orig_dh.public_key().to_owned()?)?)?; + let pkey = pkey_from_dh( + dh.set_public_key(orig_dh.public_key().to_owned()?)?, + is_dhx(self.pkey.id()), + )?; Ok(DHPublicKey { pkey }) } @@ -222,6 +239,7 @@ impl DHPrivateKey { fn parameters(&self) -> CryptographyResult { Ok(DHParameters { dh: clone_dh(&self.pkey.dh().unwrap())?, + is_dhx: is_dhx(self.pkey.id()), }) } @@ -280,6 +298,8 @@ impl DHPublicKey { fn parameters(&self) -> CryptographyResult { Ok(DHParameters { dh: clone_dh(&self.pkey.dh().unwrap())?, + + is_dhx: is_dhx(self.pkey.id()), }) } @@ -322,7 +342,7 @@ impl DHParameters { fn generate_private_key(&self) -> CryptographyResult { let dh = clone_dh(&self.dh)?.generate_key()?; Ok(DHPrivateKey { - pkey: pkey_from_dh(dh)?, + pkey: pkey_from_dh(dh, self.is_dhx)?, }) } @@ -421,9 +441,11 @@ impl DHPrivateNumbers { ) -> CryptographyResult { let _ = backend; - let dh = dh_parameters_from_numbers(py, self.public_numbers.get().parameter_numbers.get())?; + let public_numbers = self.public_numbers.get(); + let parameter_numbers = public_numbers.parameter_numbers.get(); + let dh = dh_parameters_from_numbers(py, parameter_numbers)?; - let pub_key = utils::py_int_to_bn(py, self.public_numbers.get().y.bind(py))?; + let pub_key = utils::py_int_to_bn(py, public_numbers.y.bind(py))?; let priv_key = utils::py_int_to_bn(py, self.x.bind(py))?; let dh = dh.set_key(pub_key, priv_key)?; @@ -435,7 +457,7 @@ impl DHPrivateNumbers { )); } - let pkey = pkey_from_dh(dh)?; + let pkey = pkey_from_dh(dh, parameter_numbers.q.is_some())?; Ok(DHPrivateKey { pkey }) } @@ -474,11 +496,12 @@ impl DHPublicNumbers { ) -> CryptographyResult { let _ = backend; - let dh = dh_parameters_from_numbers(py, self.parameter_numbers.get())?; + let parameter_numbers = self.parameter_numbers.get(); + let dh = dh_parameters_from_numbers(py, parameter_numbers)?; let pub_key = utils::py_int_to_bn(py, self.y.bind(py))?; - let pkey = pkey_from_dh(dh.set_public_key(pub_key)?)?; + let pkey = pkey_from_dh(dh.set_public_key(pub_key)?, parameter_numbers.q.is_some())?; Ok(DHPublicKey { pkey }) } @@ -535,7 +558,10 @@ impl DHParameterNumbers { let _ = backend; let dh = dh_parameters_from_numbers(py, self)?; - Ok(DHParameters { dh }) + Ok(DHParameters { + dh, + is_dhx: self.q.is_some(), + }) } fn __eq__( diff --git a/tests/hazmat/primitives/test_dh.py b/tests/hazmat/primitives/test_dh.py index d287d29460ae5..c1f847a212a16 100644 --- a/tests/hazmat/primitives/test_dh.py +++ b/tests/hazmat/primitives/test_dh.py @@ -441,6 +441,16 @@ def test_dh_vectors_with_q(self, backend, vector): assert int.from_bytes(symkey1, "big") == int(vector["z"], 16) assert int.from_bytes(symkey2, "big") == int(vector["z"], 16) + def test_exchange_old_key(self, backend): + k = load_vectors_from_file( + os.path.join("asymmetric", "DH", "dhpub_cryptography_old.pem"), + lambda f: serialization.load_pem_public_key(f.read()), + mode="rb", + ) + assert isinstance(k, dh.DHPublicKey) + # Ensure this doesn't raise. + k.parameters().generate_private_key().exchange(k) + def test_public_key_equality(self, backend): key_bytes = load_vectors_from_file( os.path.join("asymmetric", "DH", "dhpub.pem"),