From 6d1c9556f68bf0850ad230d9debdc07386aec058 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Mon, 15 Jun 2026 18:47:17 -0400 Subject: [PATCH] Fix loading PKCS#3 DH parameters with privateValueLength A PKCS#3 "DH PARAMETERS" structure may carry an optional trailing INTEGER, privateValueLength. The loader parsed every DH parameters blob with the X9.42-shaped struct (p, g, q?), so it misread privateValueLength as the subprime q. Since #15016 added a check_key() validation, this now fails with "Invalid DH parameters". Route the PEM loader by tag: "DH PARAMETERS" (PKCS#3) ignores privateValueLength, while "X9.42 DH PARAMETERS" keeps q. The DER loader stays X9.42-permissive since DER carries no tag to disambiguate and the existing rfc5114 DER vectors require q to be parsed. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/development/test-vectors.rst | 5 +++ src/rust/src/backend/dh.rs | 43 +++++++++++++------ tests/hazmat/primitives/test_dh.py | 14 ++++++ .../asymmetric/DH/dhp_privatevaluelength.pem | 8 ++++ 4 files changed, 57 insertions(+), 13 deletions(-) create mode 100644 vectors/cryptography_vectors/asymmetric/DH/dhp_privatevaluelength.pem diff --git a/docs/development/test-vectors.rst b/docs/development/test-vectors.rst index b9cff0c14f0f..b6ca02081b20 100644 --- a/docs/development/test-vectors.rst +++ b/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`. diff --git a/src/rust/src/backend/dh.rs b/src/rust/src/backend/dh.rs index 7fc0ad2fffdc..ee522c15876f 100644 --- a/src/rust/src/backend/dh.rs +++ b/src/rust/src/backend/dh.rs @@ -71,6 +71,32 @@ 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 { + let (p, q, g) = if x942 { + let asn1_params = asn1::parse_single::>(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::>(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( @@ -78,18 +104,9 @@ fn from_der_parameters( backend: Option>, ) -> CryptographyResult { let _ = backend; - let asn1_params = asn1::parse_single::>(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( diff --git a/tests/hazmat/primitives/test_dh.py b/tests/hazmat/primitives/test_dh.py index e6d9a7e61e50..71b5b36e0b24 100644 --- a/tests/hazmat/primitives/test_dh.py +++ b/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"), [ diff --git a/vectors/cryptography_vectors/asymmetric/DH/dhp_privatevaluelength.pem b/vectors/cryptography_vectors/asymmetric/DH/dhp_privatevaluelength.pem new file mode 100644 index 000000000000..4097210a7f63 --- /dev/null +++ b/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-----