eclipse / paho.mqtt.python

paho.mqtt.python
Other
2.19k stars 723 forks source link

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

Open agalauner-r7 opened 1 year ago

agalauner-r7 commented 1 year ago

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.

mhils commented 1 year ago

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). :)