Fix loading PKCS#3 DH parameters with privateValueLength

docs/development/test-vectors.rst

@@ -353,6 +353,11 @@ Key exchange
 * ``vectors/cryptography_vectors/asymmetric/DH/dh_key_256.pem`` contains
   a PEM PKCS8 encoded DH key with a 256-bit key size.
 
+* ``vectors/cryptography_vectors/asymmetric/DH/dhp_privatevaluelength.pem``
+  contains PKCS#3 ``DH PARAMETERS`` that include the optional
+  ``privateValueLength`` field, which must not be confused with an X9.42
+  subprime ``q``.
+
 * ``vectors/cryptoraphy_vectors/asymmetric/ECDH/brainpool.txt`` contains
   Brainpool vectors from :rfc:`7027`.
 

src/rust/src/backend/dh.rs

@@ -71,25 +71,42 @@ pub(crate) fn public_key_from_pkey(
     })
 }
 
+// Build DHParameters from the DER encoding of a parameter structure. When
+// `x942` is true the optional trailing INTEGER is the X9.42 subprime `q`;
+// otherwise the structure is PKCS#3 and the optional trailing INTEGER is
+// `privateValueLength`, which we ignore.
+fn load_dh_parameters(data: &[u8], x942: bool) -> CryptographyResult<DHParameters> {
+    let (p, q, g) = if x942 {
+        let asn1_params = asn1::parse_single::<common::DHParams<'_>>(data)?;
+        let p = openssl::bn::BigNum::from_slice(asn1_params.p.as_bytes())?;
+        let q = asn1_params
+            .q
+            .map(|q| openssl::bn::BigNum::from_slice(q.as_bytes()))
+            .transpose()?;
+        let g = openssl::bn::BigNum::from_slice(asn1_params.g.as_bytes())?;
+        (p, q, g)
+    } else {
+        let asn1_params = asn1::parse_single::<common::BasicDHParams<'_>>(data)?;
+        let p = openssl::bn::BigNum::from_slice(asn1_params.p.as_bytes())?;
+        let g = openssl::bn::BigNum::from_slice(asn1_params.g.as_bytes())?;
+        (p, None, g)
+    };
+
+    let dh = openssl::dh::Dh::from_pqg(p, q, g)?;
+    check_dh_parameters(&dh)?;
+    Ok(DHParameters { dh })
+}
+
 #[pyo3::pyfunction]
 #[pyo3(signature = (data, backend=None))]
 fn from_der_parameters(
     data: &[u8],
     backend: Option<pyo3::Bound<'_, pyo3::PyAny>>,
 ) -> CryptographyResult<DHParameters> {
     let _ = backend;
-    let asn1_params = asn1::parse_single::<common::DHParams<'_>>(data)?;
-
-    let p = openssl::bn::BigNum::from_slice(asn1_params.p.as_bytes())?;
-    let q = asn1_params
-        .q
-        .map(|q| openssl::bn::BigNum::from_slice(q.as_bytes()))
-        .transpose()?;
-    let g = openssl::bn::BigNum::from_slice(asn1_params.g.as_bytes())?;
-
-    let dh = openssl::dh::Dh::from_pqg(p, q, g)?;
-    check_dh_parameters(&dh)?;
-    Ok(DHParameters { dh })
+    // DER carries no tag distinguishing PKCS#3 from X9.42, so we permissively
+    // accept an optional trailing `q` for backwards compatibility.
+    load_dh_parameters(data, true)
 }
 
 #[pyo3::pyfunction]
@@ -105,7 +122,7 @@ fn from_pem_parameters(
         "Valid PEM but no BEGIN DH PARAMETERS/END DH PARAMETERS delimiters. Are you sure this is a DH parameters?",
     )?;
 
-    from_der_parameters(parsed.contents(), None)
+    load_dh_parameters(parsed.contents(), parsed.tag() == "X9.42 DH PARAMETERS")
 }
 
 fn dh_parameters_from_numbers(

tests/hazmat/primitives/test_dh.py

@@ -979,6 +979,20 @@ def test_public_bytes_values(
         else:
             assert parameter_numbers.q is None
 
+    def test_load_pkcs3_with_private_value_length(self):
+        # A PKCS#3 "DH PARAMETERS" structure may carry an optional trailing
+        # INTEGER, privateValueLength. It must not be confused with the X9.42
+        # subprime q (which only appears in "X9.42 DH PARAMETERS").
+        param_bytes = load_vectors_from_file(
+            os.path.join("asymmetric", "DH", "dhp_privatevaluelength.pem"),
+            lambda pemfile: pemfile.read(),
+            mode="rb",
+        )
+        parameters = serialization.load_pem_parameters(param_bytes)
+        parameter_numbers = parameters.parameter_numbers()
+        assert parameter_numbers.g == 2
+        assert parameter_numbers.q is None
+
     @pytest.mark.parametrize(
         ("encoding", "fmt"),
         [

vectors/cryptography_vectors/asymmetric/DH/dhp_privatevaluelength.pem

@@ -0,0 +1,8 @@
+-----BEGIN DH PARAMETERS-----
+MIIBDAKCAQEAlIp1fYr3ZNIqhxf5Ekoxi3eeGHtmXjuOXQ6F8cUjnqOCDeel6igI
+r00KTHnv3zTiRAdfK8+doLuBmUwHuE4ahtNi/FIbAbThaR6y2xYTGboTqLO8Jj6Z
+cnFyGRx4qMyhuYW98GDkbRt3MWDTCbKNtPT+W2UrVQhkDQpq+O5qZ5SOnxzlI9b6
+dyesAsWbeCV8aoMS9hxStBujSp1UD7Vbej1frZw1RwWuFY+6EsLXXeWFfZ4AaSJk
+h0TzTXeeUj5sl6xrctWK3noYypRzgidt2D3OxobO3Vh8PvbbXz5Qi/h8dqexZnRE
+Qf3k+DYfQsp5Mcx4ENuppHZoZXIh9+qZDwIBAgICAOE=
+-----END DH PARAMETERS-----