Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DH exchange fails in 42.0.0 #10790

Closed
daveboutcher opened this issue Apr 11, 2024 · 7 comments · Fixed by #11309
Closed

DH exchange fails in 42.0.0 #10790

daveboutcher opened this issue Apr 11, 2024 · 7 comments · Fixed by #11309

Comments

@daveboutcher
Copy link

The following code works with cryptography 41.0.7 and earlier, and fails with 42.0.0 and later:

from cryptography.hazmat.primitives.serialization import load_pem_public_key

key = b"""-----BEGIN PUBLIC KEY-----
MIICJTCCARcGCSqGSIb3DQEDATCCAQgCggEBAP//////////yQ/aoiFowjTExmKL
gNwc0SkCTgiKZ8x0Agu+pjsTmyJRSgh5jjQE3e+VGbPNOkMbMCsKbfJfFDdP4TVt
bVHCReSFtXZiXn7G9ExC6aY37WsL/1y29Aa37e44a/taiZ+lrp8kEXxLH+ZJKGZR
7ORbPcIAfLihY78FmNpINhxV05ppFj+o/STPX4NlXSPco62WHGLzViCFUrue1SkH
cJaWbWcMNU5KvJgE8XRsCMoYIXwykF5GLjbOO+OedywYDoYDmyeDouwHoo+1xV3w
b0xSyd4ry/aVWBcYOZVJfOqVauUV0iYYmPoFEBVyjlqKrKpo//////////8CAQID
ggEGAAKCAQEAoely6vSHw+/Q3zGYLaJj7eeQkfd25K8SvtC+FMY9D7jwS4g71pyr
U3FJ98Fi45Wdksh+d4u7U089trF5Xbgui29bZ0HcQZtfHEEz0Mh69tkipCm2/QIj
6eDlo6sPk9hhhvgg4MMGiWKhCtHrub3x1FHdmf7KjOhrGeb5apiudo7blGFzGhZ3
NFnbff+ArVNd+rdVmSoZn0aMhXRConlDu/44IYe5/24VLl7G+BzZlIZO4P2M83fd
mBOvR13cmYssQjEFTbaZVQvQHa3t0+aywfdCgsXGmTTK6QDCBP8D+vf1bmhEswzs
oYn1GLtJ3VyYyMBPDBomd2ctchZgTzsX1w==
-----END PUBLIC KEY-----"""

peer_key = load_pem_public_key(key)

params = peer_key.parameters()
private_key = params.generate_private_key()

# Create a shared secret
shared_secret = private_key.exchange(peer_key)

The exchange call fails with a cryptic in 42.0.0 and later

    shared_secret = private_key.exchange(peer_key)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: Error computing shared key.

If I swap out OpenSSL versions (e.g. 3.1.4 with cryptography 42.0.0) the results are the same, so the issue seems to be in the cryptography code.

I have tracked this down to evp_pkey_export_to_provider where, in 42.0.0 the key types are DHX and DH, whereas earlier they seem to be DH and DH. I suspect the issue is in the transition of load_pem_public_key to rust in 42.0.0.

Any insights appreciated.

@daveboutcher
Copy link
Author

daveboutcher commented Apr 12, 2024

There is a partial workaround for this. Reconstruct the peer key as follows:

# Reconstruct the public key
peer_key = DHPublicNumbers(peer_key.public_numbers().y, peer_key.parameters().parameter_numbers()).public_key()

However in my case the server side is unhappy with the result

@daveboutcher
Copy link
Author

This issue exists in keys generated by cryptography 41.0.7 and loaded into 42.0.0. If the following code is run on 41.0.7 to generate a key (to stdout)

from cryptography.hazmat.primitives.serialization import load_pem_parameters
from cryptography.hazmat.primitives import serialization

pem_params = b"""-----BEGIN DH PARAMETERS-----
MIIBCAKCAQEA///////////JD9qiIWjCNMTGYouA3BzRKQJOCIpnzHQCC76mOxOb
IlFKCHmONATd75UZs806QxswKwpt8l8UN0/hNW1tUcJF5IW1dmJefsb0TELppjft
awv/XLb0Brft7jhr+1qJn6WunyQRfEsf5kkoZlHs5Fs9wgB8uKFjvwWY2kg2HFXT
mmkWP6j9JM9fg2VdI9yjrZYcYvNWIIVSu57VKQdwlpZtZww1Tkq8mATxdGwIyhgh
fDKQXkYuNs474553LBgOhgObJ4Oi7Aeij7XFXfBvTFLJ3ivL9pVYFxg5lUl86pVq
5RXSJhiY+gUQFXKOWoqsqmj//////////wIBAg==
-----END DH PARAMETERS-----"""

params = load_pem_parameters(pem_params)
pk = params.generate_private_key()
k = pk.public_key()
x = k.public_bytes(encoding=serialization.Encoding.PEM,
                   format=serialization.PublicFormat.SubjectPublicKeyInfo)
print(x.decode())

And then the following is run on 42.0.0 to load the same key (from stdin), it will fail

import sys
from cryptography.hazmat.primitives.serialization import load_pem_public_key

public_key = load_pem_public_key(sys.stdin.read().encode())

params = public_key.parameters()

private_key = params.generate_private_key()

shared_key = private_key.exchange(public_key)

@alex
Copy link
Member

alex commented Apr 21, 2024 via email

@daveboutcher
Copy link
Author

PR proposed.....passes all the tests and Works For Me™

@daveboutcher
Copy link
Author

@alex is there a process for requesting a review of a PR? Or an irc/discord/mailing list thats worth joining?

@alex
Copy link
Member

alex commented Apr 26, 2024 via email

@alex alex added this to the Forty Third Release milestone May 9, 2024
@alex
Copy link
Member

alex commented Jul 6, 2024

The more I poke at this, the more I'm convinced this is an OpenSSL bug: openssl/openssl#24804

alex added a commit to alex/cryptography that referenced this issue Jul 8, 2024
alex added a commit to alex/cryptography that referenced this issue Jul 8, 2024
alex added a commit to alex/cryptography that referenced this issue Jul 8, 2024
alex added a commit to alex/cryptography that referenced this issue Jul 8, 2024
alex added a commit to alex/cryptography that referenced this issue Jul 8, 2024
alex added a commit to alex/cryptography that referenced this issue Jul 8, 2024
alex added a commit to alex/cryptography that referenced this issue Jul 8, 2024
alex added a commit to alex/cryptography that referenced this issue Jul 8, 2024
alex added a commit to alex/cryptography that referenced this issue Jul 8, 2024
alex added a commit to alex/cryptography that referenced this issue Jul 20, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
2 participants