43.0.0 fails test_public_bytes_match with dhpub_rfc5114_2 on Debian sid

[wRAR]

I'm trying to update the Debian package to 43 and it fails two test cases (modulo PEM/DER it's the same one): serialized contains a representation without Q, or at least that's how I read it.

openssl asn1parse for cryptography_vectors/asymmetric/DH/dhpub_rfc5114_2.pem:

    0:d=0  hl=4 l= 836 cons: SEQUENCE
    4:d=1  hl=4 l= 566 cons: SEQUENCE
    8:d=2  hl=2 l=   7 prim: OBJECT            :X9.42 DH
   17:d=2  hl=4 l= 553 cons: SEQUENCE
   21:d=3  hl=4 l= 257 prim: INTEGER           :AD107E1E9123A9D0D660FAA79559C51FA20D64E5683B9FD1B54B1597B61D0A75E6FA141DF95A56DBAF9A3C407BA1DF15EB3D688A309C180E1DE6B85A1274A0A66D3F8152AD6AC2129037C9EDEFDA4DF8D91E8FEF55B7394B7AD5B7D0B6C12207C9F98D11ED34DBF6C6BA0B2C8BBC27BE6A00E0A0B9C49708B3BF8A317091883681286130BC8985DB1602E714415D9330278273C7DE31EFDC7310F7121FD5A07415987D9ADC0A486DCDF93ACC44328387315D75E198C641A480CD86A1B9E587E8BE60E69CC928B2B9C52172E413042E9B23F10B0E16E79763C9B53DCF4BA80A29E3FB73C16B8E75B97EF363E2FFA31F71CF9DE5384E71B81C0AC4DFFE0C10E64F
  282:d=3  hl=4 l= 257 prim: INTEGER           :AC4032EF4F2D9AE39DF30B5C8FFDAC506CDEBE7B89998CAF74866A08CFE4FFE3A6824A4E10B9A6F0DD921F01A70C4AFAAB739D7700C29F52C57DB17C620A8652BE5E9001A8D66AD7C17669101999024AF4D027275AC1348BB8A762D0521BC98AE247150422EA1ED409939D54DA7460CDB5F6C6B250717CBEF180EB34118E98D119529A45D6F834566E3025E316A330EFBB77A86F0C1AB15B051AE3D428C8F8ACB70A8137150B8EEB10E183EDD19963DDD9E263E4770589EF6AA21E7F5F2FF381B539CCE3409D13CD566AFBB48D6C019181E1BCFE94B30269EDFE72FE9B6AA4BD7B5A0F1C71CFFF4C19C418E1F6EC017981BC087F2A7065B384B890D3191F2BFA
  543:d=3  hl=2 l=  29 prim: INTEGER           :801C0D34C58D93FE997177101F80535A4738CEBCBF389A99B36371EB
  574:d=1  hl=4 l= 262 prim: BIT STRING

openssl asn1parse for serialized in the failing test:

   0:d=0  hl=4 l= 807 cons: SEQUENCE
    4:d=1  hl=4 l= 537 cons: SEQUENCE
    8:d=2  hl=2 l=   9 prim: OBJECT            :dhKeyAgreement
   19:d=2  hl=4 l= 522 cons: SEQUENCE
   23:d=3  hl=4 l= 257 prim: INTEGER           :AD107E1E9123A9D0D660FAA79559C51FA20D64E5683B9FD1B54B1597B61D0A75E6FA141DF95A56DBAF9A3C407BA1DF15EB3D688A309C180E1DE6B85A1274A0A66D3F8152AD6AC2129037C9EDEFDA4DF8D91E8FEF55B7394B7AD5B7D0B6C12207C9F98D11ED34DBF6C6BA0B2C8BBC27BE6A00E0A0B9C49708B3BF8A317091883681286130BC8985DB1602E714415D9330278273C7DE31EFDC7310F7121FD5A07415987D9ADC0A486DCDF93ACC44328387315D75E198C641A480CD86A1B9E587E8BE60E69CC928B2B9C52172E413042E9B23F10B0E16E79763C9B53DCF4BA80A29E3FB73C16B8E75B97EF363E2FFA31F71CF9DE5384E71B81C0AC4DFFE0C10E64F
  284:d=3  hl=4 l= 257 prim: INTEGER           :AC4032EF4F2D9AE39DF30B5C8FFDAC506CDEBE7B89998CAF74866A08CFE4FFE3A6824A4E10B9A6F0DD921F01A70C4AFAAB739D7700C29F52C57DB17C620A8652BE5E9001A8D66AD7C17669101999024AF4D027275AC1348BB8A762D0521BC98AE247150422EA1ED409939D54DA7460CDB5F6C6B250717CBEF180EB34118E98D119529A45D6F834566E3025E316A330EFBB77A86F0C1AB15B051AE3D428C8F8ACB70A8137150B8EEB10E183EDD19963DDD9E263E4770589EF6AA21E7F5F2FF381B539CCE3409D13CD566AFBB48D6C019181E1BCFE94B30269EDFE72FE9B6AA4BD7B5A0F1C71CFFF4C19C418E1F6EC017981BC087F2A7065B384B890D3191F2BFA
  545:d=1  hl=4 l= 262 prim: BIT STRING

This stuff is above my current level of OpenSSL etc. knowledge, but as far as I can check all Rust (and Python) objects containing the loaded key actually have the Q value, yet this is how it's serialized. I've tracked the change to #11309 and changing from_dh to from_dhx in spki.rs makes the test succeed.

Note that the tests don't fail when simply running them in the cryptography source with non-system Rust, I assume that mode doesn't use the system OpenSSL but I'm open to suggestions how to debug these differences further.

[wRAR]

A simplified Python test:

from cryptography.hazmat.primitives import serialization

key_bytes = b"""-----BEGIN PUBLIC KEY-----
MIIDRDCCAjYGByqGSM4+AgEwggIpAoIBAQCtEH4ekSOp0NZg+qeVWcUfog1k5Wg7
n9G1SxWXth0Kdeb6FB35Wlbbr5o8QHuh3xXrPWiKMJwYDh3muFoSdKCmbT+BUq1q
whKQN8nt79pN+Nkej+9VtzlLetW30LbBIgfJ+Y0R7TTb9sa6CyyLvCe+agDgoLnE
lwizv4oxcJGINoEoYTC8iYXbFgLnFEFdkzAngnPH3jHv3HMQ9xIf1aB0FZh9mtwK
SG3N+TrMRDKDhzFddeGYxkGkgM2Gobnlh+i+YOacySiyucUhcuQTBC6bI/ELDhbn
l2PJtT3PS6gKKeP7c8FrjnW5fvNj4v+jH3HPneU4TnG4HArE3/4MEOZPAoIBAQCs
QDLvTy2a453zC1yP/axQbN6+e4mZjK90hmoIz+T/46aCSk4Quabw3ZIfAacMSvqr
c513AMKfUsV9sXxiCoZSvl6QAajWatfBdmkQGZkCSvTQJydawTSLuKdi0FIbyYri
RxUEIuoe1AmTnVTadGDNtfbGslBxfL7xgOs0EY6Y0RlSmkXW+DRWbjAl4xajMO+7
d6hvDBqxWwUa49QoyPistwqBNxULjusQ4YPt0Zlj3dniY+R3BYnvaqIef18v84G1
OczjQJ0TzVZq+7SNbAGRgeG8/pSzAmnt/nL+m2qkvXtaDxxxz/9MGcQY4fbsAXmB
vAh/KnBls4S4kNMZHyv6Ah0AgBwNNMWNk/6ZcXcQH4BTWkc4zry/OJqZs2Nx6wOC
AQYAAoIBAQCCwWW7V2JD7PRtWMPRUBYWlV/KAyD6leoR0ubBuc8hdnZyDcHAjIW/
IMTSMrYKKaHlHHt3O8ZFAUWHxSXIYVGzDXVIbse2yY77X3SVW4MRbQHQrxIyr4kh
PC3ldDadcBq6k1cwC5INPYuYJS1GxGlSwWpfM1VLODF4Cce5rdRwH1wVjBtwNen+
OTZuzsuQ0olreMUjxKV3KH71unomY+1YqiC17GbjDzFmEN+qOFg+SVq2r3ccKEOH
5mDtvvTtuHLi6A4dJE7pViLnbQKOYcHoh8KqeScXNiE59N0m6v1JsjZu6yNQsB/h
tWAiooCeN5VZw3s3W6AcTqrMFP0bJHg3
-----END PUBLIC KEY-----
"""
pub_key = serialization.load_pem_public_key(key_bytes, None)
serialized = pub_key.public_bytes(
    serialization.Encoding.PEM,
    serialization.PublicFormat.SubjectPublicKeyInfo,
)
assert serialized == key_bytes

Crafting a self-contained minimal Rust test will take too much time for me but this was handy:

diff --git a/src/rust/cryptography-key-parsing/src/spki.rs b/src/rust/cryptography-key-parsing/src/spki.rs
index db4f69d..4dbd0f9 100644
--- a/src/rust/cryptography-key-parsing/src/spki.rs
+++ b/src/rust/cryptography-key-parsing/src/spki.rs
@@ -5,6 +5,7 @@
 use cryptography_x509::common::{AlgorithmParameters, EcParameters, SubjectPublicKeyInfo};

 use crate::{KeyParsingError, KeyParsingResult};
+use std::str;

 pub fn parse_public_key(
     data: &[u8],
@@ -113,8 +114,12 @@ pub fn parse_public_key(
                 asn1::parse_single::<asn1::BigUint<'_>>(k.subject_public_key.as_bytes())?;
             let pub_key = openssl::bn::BigNum::from_slice(pub_key_int.as_bytes())?;
             let dh = dh.set_public_key(pub_key)?;
+            let pkey = openssl::pkey::PKey::from_dh(dh)?;
+            let pem_bytes = pkey.public_key_to_pem()?;
+            let pem_str = str::from_utf8(&*pem_bytes).unwrap();
+            println!("pem_bytes: {pem_str}");

-            Ok(openssl::pkey::PKey::from_dh(dh)?)
+            Ok(pkey)
         }
         #[cfg(not(CRYPTOGRAPHY_IS_BORINGSSL))]
         AlgorithmParameters::DhKeyAgreement(dh_params) => {

[alex]

What version of OpenSSL does Debian have?

[wRAR]

The one I'm running is 3.3.2.

[alex]

Hmm. We test in CI against 3.3.2, and I also tested locally against 3.3.2, and both passed.

In fact, come to think of it, we also test against debian sid in CI.

Does your Debian's OpenSSL have any additional patches?

[wRAR]

Well, I didn't add any patches to the official Debian package if that's what you mean :) Official patches are at https://sources.debian.org/src/openssl/3.3.2-1/debian/patches/

[alex]

The good news is none of those patches look remotely related. The bad news is it means we cross off another hypothesis from the list.

https://github.com/pyca/cryptography/actions/runs/10755295817/job/29826803330 is an example of a recent passing debian sid job.

[wRAR]

Does that job use the system libssl.so? I see there are vendoring changes in https://sources.debian.org/src/rust-openssl-sys/0.9.101-1/debian/patches/ and https://sources.debian.org/src/rust-openssl/0.10.64-1/debian/patches/

[wRAR]

I also wonder if it's possible to translate the code in question to C with direct libcrypto calls to test OpenSSL directly.

[alex]

Yes, that job uses the system openssl, but not the system rust-openssl (not that any of those rust-openssl patches look related).

[wRAR]

Can I easily run the same on my checkout of the git repo?

[alex]

You should be able to, yes. https://github.com/pyca/cryptography/blob/main/.github/workflows/ci.yml#L173-L215 are the steps to run.

https://github.com/pyca/infra/blob/main/runners/debian/Dockerfile is the dockerfile for our debian containers, though as you can see it's not really doing anything besides pre-installing some packages.

[wRAR]

OK, I cannot make it fail when doing that. So can it be something in Debian-shipped Rust code?

[alex]

I'm not sure -- it seems much more likely this would be caused by a change to OpenSSL than to any of the Rust code.

[wRAR]

I ripped out cryptography_key_parsing::spki::parse_public_key() and its deps into a separate project and found that the bad behavior happens with openssl = "=0.10.64" and the good behavior happens with openssl = "=0.10.65". I guess we can close this unless some workarounds are good to have?

[wRAR]

A part of the diff is s/EVP_PKEY_assign_DH/EVP_PKEY_set1_DH/, no idea if that's the reason.

[alex]

Oh yes, that's almost certainly related. Our Cargo.toml seems to reflect this version

…

On Mon, Sep 9, 2024, 1:56 PM Andrey Rakhmatullin ***@***.***> wrote: A part of the diff is s/EVP_PKEY_assign_DH/EVP_PKEY_set1_DH/, no idea if that's the reason. — Reply to this email directly, view it on GitHub <#11568 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAGBEOIZ6ZBZDZFTEUVSTZVXONNAVCNFSM6AAAAABN22H6TGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZYG4ZTMMBSHE> . You are receiving this because you commented.Message ID: ***@***.***>

[wRAR]

(Debian currently only has 0.10.64, that's the reason I noticed these)