Fix exchange with keys that had Q automatically computed (#11309)

fixes #10790 closes #10864 closes #11218
pyca · Jul 20, 2024 · 42788a0 · 42788a0
1 parent 2dbdfb8
commit 42788a0
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 28 deletions.
diff --git 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/cryptography-key-parsing/src/spki.rs b/src/rust/cryptography-key-parsing/src/spki.rs
@@ -114,13 +114,7 @@ pub fn parse_public_key(
  let pub_key = openssl::bn::BigNum::from_slice(pub_key_int.as_bytes())?;
  let dh = dh.set_public_key(pub_key)?;
 
- cfg_if::cfg_if! {
- if #[cfg(CRYPTOGRAPHY_IS_LIBRESSL)] {
- Ok(openssl::pkey::PKey::from_dh(dh)?)
- } else {
- Ok(openssl::pkey::PKey::from_dhx(dh)?)
- }
- }
+ Ok(openssl::pkey::PKey::from_dh(dh)?)
  }
  #[cfg(not(CRYPTOGRAPHY_IS_BORINGSSL))]
  AlgorithmParameters::DhKeyAgreement(dh_params) => {

diff --git a/src/rust/src/backend/dh.rs b/src/rust/src/backend/dh.rs
@@ -70,23 +70,6 @@ pub(crate) fn public_key_from_pkey(
  }
 }
 
-#[cfg(not(CRYPTOGRAPHY_IS_BORINGSSL))]
-fn pkey_from_dh<T: openssl::pkey::HasParams>(
- dh: openssl::dh::Dh<T>,
-) -> CryptographyResult<openssl::pkey::PKey<T>> {
- cfg_if::cfg_if! {
- if #[cfg(CRYPTOGRAPHY_IS_LIBRESSL)] {
- Ok(openssl::pkey::PKey::from_dh(dh)?)
- } else {
- if dh.prime_q().is_some() {
- Ok(openssl::pkey::PKey::from_dhx(dh)?)
- } else {
- Ok(openssl::pkey::PKey::from_dh(dh)?)
- }
- }
- }
-}
-
 #[pyo3::pyfunction]
 #[pyo3(signature = (data, backend=None))]
 fn from_der_parameters(
@@ -214,7 +197,8 @@ 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 =
+ openssl::pkey::PKey::from_dh(dh.set_public_key(orig_dh.public_key().to_owned()?)?)?;
 
  Ok(DHPublicKey { pkey })
  }
@@ -322,7 +306,7 @@ impl DHParameters {
  fn generate_private_key(&self) -> CryptographyResult<DHPrivateKey> {
  let dh = clone_dh(&self.dh)?.generate_key()?;
  Ok(DHPrivateKey {
- pkey: pkey_from_dh(dh)?,
+ pkey: openssl::pkey::PKey::from_dh(dh)?,
  })
  }
 
@@ -435,7 +419,7 @@ impl DHPrivateNumbers {
  ));
  }
 
- let pkey = pkey_from_dh(dh)?;
+ let pkey = openssl::pkey::PKey::from_dh(dh)?;
  Ok(DHPrivateKey { pkey })
  }
 
@@ -478,7 +462,7 @@ impl DHPublicNumbers {
 
  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 = openssl::pkey::PKey::from_dh(dh.set_public_key(pub_key)?)?;
 
  Ok(DHPublicKey { pkey })
  }

diff --git 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"),

diff --git a/vectors/cryptography_vectors/asymmetric/DH/dhpub_cryptography_old.pem b/vectors/cryptography_vectors/asymmetric/DH/dhpub_cryptography_old.pem
@@ -0,0 +1,15 @@
+-----BEGIN PUBLIC KEY-----
+MIICJTCCARcGCSqGSIb3DQEDATCCAQgCggEBAP//////////yQ/aoiFowjTExmKL
+gNwc0SkCTgiKZ8x0Agu+pjsTmyJRSgh5jjQE3e+VGbPNOkMbMCsKbfJfFDdP4TVt
+bVHCReSFtXZiXn7G9ExC6aY37WsL/1y29Aa37e44a/taiZ+lrp8kEXxLH+ZJKGZR
+7ORbPcIAfLihY78FmNpINhxV05ppFj+o/STPX4NlXSPco62WHGLzViCFUrue1SkH
+cJaWbWcMNU5KvJgE8XRsCMoYIXwykF5GLjbOO+OedywYDoY DmyeDouwHoo+1xV3w
+b0xSyd4ry/aVWBcYOZVJfOqVauUV0iYYmPoFEBVyjlqKrKpo//////////8CAQID
+ggEGAAKCAQEAoely6vSHw+/Q3zGYLaJj7eeQkfd25K8SvtC+FMY9D7jwS4g71pyr
+U3FJ98Fi45Wdksh+d4u7U089trF5Xbgui29bZ0HcQZtfHEEz0Mh69tkipCm2/QIj
+6eDlo6sPk9hhhvgg4MMGiWKhCtHrub3x1FHdmf7KjOhrGeb5apiudo7blGFzGhZ3
+NFnbff+ArVNd+rdVmSoZn0aMhXRConlDu/44IYe5/24VLl7G+BzZlIZO4P2M83fd
+mBOvR13cmYssQjEFTbaZVQvQHa3t0+aywfdCgsXGmTTK6QDCBP8D+vf1bmhEswzs
+oYn1GLtJ3VyYyMBPDBomd2ctchZgTzsX1w==
+-----END PUBLIC KEY-----
+