circuitpython: minimqtt lib problem with TLS broker

CircuitPython version

CP800b6 daily 
Adafruit CircuitPython 8.0.0-beta.6-43-g1c1cf1cf5 on 2023-01-18; Raspberry Pi Pico W with rp2040
Board ID:raspberry_pi_pico_w

Code/REPL

negative number of bytes to read: -115

Behavior

use: CP800b6 latest ( from 14. and 18. ) ( with bundle /lib/ from 15. and 17. ) on a PICO_W ( nuked ) to connect to a local broker ( RPI mosquitto) OK BUT to a remote broker TLS ( HIVEMQ ) ERROR always get error at subscribe or publish about negative number of bytes to read: -115

i think that comes from a recent change of minimqtt lib

++ confirm, all same but replace /lib/minimqtt/ from adafruit-circuitpython-bundle-8.x-mpy-20221221.zip still works

Description

No response

Additional information

adafruit-circuitpython-bundle-8.x-mpy-20221221.zip /lib/minimqtt/ OK

About this issue

  • Original URL
  • State: closed
  • Created a year ago
  • Comments: 42 (4 by maintainers)

Commits related to this issue

Most upvoted comments

INSECURE means plaintext MQTT. If you want TLS, you need to set is_ssl to True.

On Wed, Feb 22, 2023, at 09:15, KLL wrote:

use ‘logging’ again

image https://user-images.githubusercontent.com/19226581/220561096-fe6878a3-f814-4a16-afc3-fb47a4db2934.png

why it say INSECURE

INFO - Establishing an INSECURE connection to <xxx>.hivemq.cloud:8883

— Reply to this email directly, view it on GitHub https://github.com/adafruit/circuitpython/issues/7606#issuecomment-1439595141, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWMMDCRE7TLKFQA7PKG77LWYXDLXANCNFSM6AAAAAAVAAAD3E. You are receiving this because you commented.Message ID: @.***>

@kllsamui , the value of 100 I used was based on the two MQTT brokers I had available to test (1 local, 1remote). Some sites could require even more time. The fix in PR #7623 should take care of the problem regardless of the keep_alive timeout.

Testing (with a lot of tracing) has shown that the 116 is, in fact, an MP_ETIMEDOUT error code being returned from lwip_tcp_receive in common-hal/socketpool/Socket.c. I am unsure why the value is 116, as MP_ETIMEDOUT is defined as 110 in py/mperrno.h. Adding a “keep_alive=100” to the MQTT initialization in the supplied script avoids the timeout in my testing. There may well be some issue here that is keeping the error from being properly reported, perhaps because at some point the return value is being passed back up the chain as an unsigned.

Looking into the code of subscribe(), I can see potential for breakage: https://github.com/adafruit/Adafruit_CircuitPython_MiniMQTT/blob/342b8c99b2c8f10b605cc8311bf5f394f4d18e92/adafruit_minimqtt/adafruit_minimqtt.py#L785-L802

The first issue is that the client expects SUBACK to be sent in direct response to SUBSCRIBE. If any other message type is received from (buggy/rogue) broker, the parsing will get out of sync, because part of the message will remain in the read buffer. In such (rare ?) case the other message should be processed fully or exception thrown.

The second issue is that the code expects fixed length of the SUBACK, specifically 4 bytes (L788 above), possibly assuming single topic to be subscribed. When a client subscribes to multiple topics at once (which the subscribe() function allows), the SUBACK will contain return codes for each topic.

Unsubcribe handling has the same problem, however this will matter only for MQTT protocol version 5 and above, I believe.

There are 2 possibilities I can see how the to_read variable in _sock_exact_recv() can become negative:

  • the bufsize parameter of the function was already negative
  • recv_len returned from recv_into() was strictly bigger than the bufsize

The former might be a problem in MQTT message parsing, the latter would be a bug in CircuitPython core. Now, there are only 2 calls to _sock_exact_recv() that supply non-literal number and these are both in MQTT PUBLISH message parsing.

If you are willing to help debug this, I will prepare a special version of adafruit_minimqtt.py with extra debug logging that can be placed on the Pico W and be used to shed more light on this issue. In the mean time, install adafruit_logging and enable logging with:

import adafruit_logging as logging

...

mqtt_client = MQTT.MQTT(...)
mqtt_client.enable_logger(logging, log_level=logging.DEBUG)

I will begin from the end. The is_ssl parameter as it is implemented currently, is set to True by default and as long as the port is set to 8883 and the ssl_context parameter is valid, TLS will be used. There is no fallback to insecure (plaintext) connection if TLS setup fails (which is a good thing as it avoids downgrade attacks). The default behavior of is_ssl might or might not change with PR adafruit/Adafruit_CircuitPython_MiniMQTT#145. Anyhow, if TLS is desired, setting is_ssl to True explicitly is the right thing to do, even though it is not needed currently.

I registered an account at HiveMQ and extracted relevant code from code.py however cannot replicate the problem with the latest minimqtt module, at least using CPython on MacOS - the subscribe goes through just fine.