Fix exchange with keys that had Q automatically computed

fixes pyca#10790
closes pyca#10864

Author: alex

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
 ~~~~~
 

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<openssl::pkey::Params>,
+
+    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<T: openssl::pkey::HasParams>(
     dh: openssl::dh::Dh<T>,
+    is_dhx: bool,
 ) -> 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() {
+            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<T: openssl::pkey::HasParams>(
     }
 }
 
+fn is_dhx(id: openssl::pkey::Id) -> bool {
+    cfg_if::cfg_if! {
+        if #[cfg(any(CRYPTOGRAPHY_IS_LIBRESSL, CRYPTOGRAPHY_IS_BORINGSSL))] {
+            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,14 +228,18 @@ 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 })
     }
 
     fn parameters(&self) -> CryptographyResult<DHParameters> {
         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<DHParameters> {
         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<DHPrivateKey> {
         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<DHPrivateKey> {
         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<DHPublicKey> {
         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__(

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

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-----
+