umqtt.simple not working with mosquitto 2.0.12

[cinadr]

Hi!

umqtt.simple fails to connect to mosquito 2.0.12. It throws back MQTTException: 2. Meanwhile mosquitto log says: 'Bad socket read/write on client . Invalid arguments provided.'

This is reproducible with micropython 16 and 17 both in an application and in REPL.

This is not a problem with mosquitto 2.0.11.

All other mqtt clients are working correctly with 2.0.12 (openhab, zwave2mqtt, zigbee2mqtt, etc.). So I assume this is a problem with this library. I rolled back to 2.0.11.

Thank you in advance:

Zsolt Zimmermann

The changelog of mosquitto:

2.0.12 - 2021-08-31
===================

Security:
- An MQTT v5 client connecting with a large number of user-property properties
  could cause excessive CPU usage, leading to a loss of performance and
  possible denial of service. This has been fixed.
- Fix `max_keepalive` not applying to MQTT v3.1.1 and v3.1 connections.
  These clients are now rejected if their keepalive value exceeds
  max_keepalive. This option allows CVE-2020-13849, which is for the MQTT
  v3.1.1 protocol itself rather than an implementation, to be addressed.
- Using certain listener related configuration options e.g. `cafile`, that
  apply to the default listener without defining any listener would cause a
  remotely accessible listener to be opened that was not confined to the local
  machine but did have anonymous access enabled, contrary to the
  documentation. This has been fixed. Closes #2283.
- CVE-2021-34434: If a plugin had granted ACL subscription access to a
  durable/non-clean-session client, then removed that access, the client would
  keep its existing subscription. This has been fixed.
- Incoming QoS 2 messages that had not completed the QoS flow were not being
  checked for ACL access when a clean session=False client was reconnecting.
  This has been fixed.

Broker:
- Fix possible out of bounds memory reads when reading a corrupt/crafted
  configuration file. Unless your configuration file is writable by untrusted
  users this is not a risk. Closes #567213.
- Fix `max_connections` option not being correctly counted.
- Fix TLS certificates and TLS-PSK not being able to be configured at the same
  time.
- Disable TLS v1.3 when using TLS-PSK, because it isn't correctly configured.
- Fix `max_keepalive` not applying to MQTT v3.1.1 and v3.1 connections.
  These clients are now rejected if their keepalive value exceeds
  max_keepalive. This option allows CVE-2020-13849, which is for the MQTT
  v3.1.1 protocol itself rather than an implementation, to be addressed.
- Fix broker not quiting if e.g. the `password_file` is specified as a
  directory. Closes #2241.
- Fix listener mount_point not being removed on outgoing messages.
  Closes #2244.
- Strict protocol compliance fixes, plus test suite.
- Fix $share subscriptions not being recovered for durable clients that
  reconnect.
- Update plugin configuration documentation. Closes #2286.

Client library:
- If a client uses TLS-PSK then force the default cipher list to use "PSK"
  ciphers only. This means that a client connecting to a broker configured
  with x509 certificates only will now fail. Prior to this, the client would
  connect successfully without verifying certificates, because they were not
  configured.
- Disable TLS v1.3 when using TLS-PSK, because it isn't correctly configured.
- Threaded mode is deconfigured when the mosquitto_loop_start() thread ends,
  which allows mosquitto_loop_start() to be called again. Closes #2242.
- Fix MOSQ_OPT_SSL_CTX not being able to be set to NULL. Closes #2289.
- Fix reconnecting failing when MOSQ_OPT_TLS_USE_OS_CERTS was in use, but none
  of capath, cafile, psk, nor MOSQ_OPT_SSL_CTX were set, and
  MOSQ_OPT_SSL_CTX_WITH_DEFAULTS was set to the default value of true.
  Closes #2288.

Apps:
- Fix `mosquitto_ctrl dynsec setDefaultACLAccess` command not working.

Clients:
- mosquitto_sub and mosquitto_rr now open stdout in binary mode on Windows
  so binary payloads are not modified when printing.
- Document TLS certificate behaviour when using `-p 8883`.

Build:
- Fix installation using WITH_TLS=no. Closes #2281.
- Fix builds with libressl 3.4.0. Closes #2198.
- Remove some unnecessary code guards related to libressl.
- Fix printf format build warning on MIPS. Closes #2271.

[amotl]

Dear Zsolt,

thank you for this report. I wanted to let you know that there already has been a sweet conversation with @ralight about this issue at https://github.com/hiveeyes/terkin-datalogger/pull/97. He was very kind to point out the problem with MicroPython's umqtt module as well.

TLDR; Mosquitto 2.0.12 has both become more strict wrt. protocol compliance, and now also honors the max_keepalive option for MQTT v3.x connections, in order to improve the situation with CVE-2020-13849. This is the corresponding quote from the changelog of Mosquitto 2.0.12:

Fix max_keepalive not applying to MQTT v3.1.1 and v3.1 connections. These clients are now rejected if their keepalive value exceeds max_keepalive. This option allows CVE-2020-13849, which is for the MQTT v3.1.1 protocol itself rather than an implementation, to be addressed. [1]

Regarding this conversation, I would like to thank Roger very much for his insights and his prompt response.

With kind regards, Andreas.

[1] https://mosquitto.org/blog/2021/08/version-2-0-12-released/

[amotl]

At https://github.com/hiveeyes/terkin-datalogger/pull/97#issuecomment-915872291, I've referenced two patches to outline how to mitigate the problem by setting a keepalive value other than zero on the client side.

[cinadr]

Dear Andreas,

Thank you for the update. I've already been following the conversation and it is very good to see how prompt you all react to and solve the problem.

Regards, Zsolt.

[amotl]

Dear @ralight,

at https://github.com/hiveeyes/terkin-datalogger/pull/97#issuecomment-916003229, you confirmed that using 60 seconds as a default keepalive time is reasonable. However, umqtt.simple probably does not provide a reconnect mechanism, so the application would be responsible.

So, I am asking if you nevertheless think that I should submit a corresponding patch to the MicroPython standard library to use 60 seconds here, similar to https://github.com/daq-tools/umqtt-example/pull/1?

I've also pushed a commit to mosquitto to allow max_keepalive to be set to 0: https://github.com/eclipse/mosquitto/commit/d942ed7eec1619bc60d3997fecddb85b2fecaa37

I will also reuse this section from Mosquitto's documentation to make users of umqtt.simple aware of this detail when submitting a corresponding patch.

With kind regards, Andreas.

[ralight]

I haven't looked at umqtt in detail, so the answer is - it depends. If the library handles keepalive and sending PINGREQ itself, then changing the default keepalive to 60 seconds is a good idea. If the library does not handle sending PINGREQs, then if you change the default value you will get some confused users at 90 seconds (60*1.5) after they deploy the new code. My guess is that the latter situation is probably true. I'm not sure what the best thing to do would be.

[ashwanisharma512]

Simple assign some time for keep alive argument... eg-> from umqtt.simple import MQTTClient client = MQTTClient("client_id", "192.168.0.xxx", port=1883, user=None, password=None, keepalive=30, ssl=False, ssl_params={}) client.on_connect = on_connect_function client.connect()

[jonnor]

Can this issue be reproduced in the latest versions of this module?