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

Setting of cipher suite string happens too late in Client.set_tls #736

Open
agalauner-r7 opened this issue Jul 15, 2023 · 1 comment
Open
Labels
Status: Available No one has claimed responsibility for resolving this issue. Type: Enhancement A new feature for a minor or major release.

Comments

@agalauner-r7
Copy link

agalauner-r7 commented Jul 15, 2023

Hi,

I need to connect to an MQTT broker that is not under my control and might not be configured so that modern TLS security standards are met.

I need to use TLS client cert authentication with an RSA key length of only 1024 bits. OpenSSL is everything but happy about this when trying to connect:

$ ./subscribe.py               
Traceback (most recent call last):
  File "/home/andy/Documents/foo/subscribe.py", line 39, in <module>
    main()
  File "/home/andy/Documents/foo/subscribe.py", line 23, in main
    client.tls_set(
  File "/usr/lib/python3.11/site-packages/paho/mqtt/client.py", line 796, in tls_set
    context.load_cert_chain(certfile, keyfile, keyfile_password)
ssl.SSLError: [SSL: EE_KEY_TOO_SMALL] ee key too small (_ssl.c:3900)

I encountered a similar issue with mosquitto_sub where it complained about the wrong outdated set of ciphers being used. To fix it there, I passed the cipher suite DEFAULT@SECLEVEL=0 to it, and OpenSSL finally established a TLS session.

I tried doing the same when using paho:

client.tls_set(
        ca_certs="server_chain.cert",
        certfile="cert.txt",
        keyfile="key.txt",
        keyfile_password="lololololol",
        ciphers="DEFAULT@SECLEVEL=0",
        tls_version=ssl.PROTOCOL_TLSv1_2
    )

However, the error remained. After checking the code where all of this is handled, I found out that the keys are loaded first (which caused the error) and THEN the cipher suite is set: https://github.com/eclipse/paho.mqtt.python/blob/master/src/paho/mqtt/client.py#L794-L809

After moving lines 808-809 directly below line 792 where the context is created, the issue was fixed.

However, I don't know if this has any other side-effects like not being able to specify all cipher suites at this time, otherwise I already would've created a PR. Is there a reason why the call to this function happens so late? If not, moving these lines up might be in order to fix issues like this.

Again, I know that I should update my key length and reconfigure my MQTT broker. The issue is, it is not my server, so I can't. I need to connect to this broker, no matter if it's secure right now or not, so being able to overwrite the default behaviour using the cipher suite string is the only way I can connect to it.

@github-actions github-actions bot added the Status: Available No one has claimed responsibility for resolving this issue. label Jul 15, 2023
@mhils
Copy link

mhils commented Jul 15, 2023

However, I don't know if this has any other side-effects like not being able to specify all cipher suites at this time, otherwise I already would've created a PR. Is there a reason why the call to this function happens so late? If not, moving these lines up might be in order to fix issues like this.

This should be perfectly fine. We do set ciphers first in @mitmproxy, and I have never heard of anyone report problems from doing so in pyOpenSSL either (where I am one of the maintainers). :)

@MattBrittan MattBrittan added the Type: Enhancement A new feature for a minor or major release. label Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Available No one has claimed responsibility for resolving this issue. Type: Enhancement A new feature for a minor or major release.
Projects
None yet
Development

No branches or pull requests

3 participants