LabVIEW-Open-Source / MQTT-Client

A LabVIEW-based client for MQTT
Other
27 stars 5 forks source link

Password in connect packet is not UTF8 encoded #3

Closed joerghampel closed 3 years ago

joerghampel commented 3 years ago

I'm trying to connect to an ActiveMQ broker, which requires username ("system") and password ("manager"). The connection attempt fails with the broker showing "Invalid message encoding" in its debug log. I can connect, though, when using another client (MQTT.fx).

Using Wireshark, I looked at the connect packets of both clients and could see that MQTT.fx sends the password UTF8 encoded (the length is prepended) but the MQTT-Client does not encode the password as UTF8 (the length bytes are missing):

mqtt fx mqtt-client

Looking at http://public.dhe.ibm.com/software/dw/webservices/ws-mqtt/MQTT_V3.1_Protocol_Specific.pdf on page 17, this document says that the password "is the next UTF8 encoded string".

I modified Create CONNECT Packet.vi to send the password also UTF8 encoded, and now the connection does work with my broker:

Create Connect Packet
francois-normandin commented 3 years ago

@joerghampel thanks for the detailed report. It looks like the server you connect to is requiring a UTF8 encoded password, but the normative requirement for password does not reference any encoding. Passwords can be any binary data, regardless of encoding. That is to allow for encrypted passwords to be supplied through an upgraded protocol.

The MQTT protocol standard I'm following is found here: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718031

You can see in section 3.5.1.5 that no encoding is specified, so this requirement is a particularity of the server ActiveMQ you are connecting to.

I'm very much interested in understanding the differences of the major MQTT servers out there, so I can improve ease of use of the LVOS MQTT client. There could be a constructor made especially for each broker to ensure full compatibility with the extra requirements of those brokers.

In this case, I'll check to find a way to inject a password encoding into the constructor. It might require that we override the client with a specific client for ActiveMQ. I'm sure other brokers will have similar behavior, so definitely a thing I'll investigate right away.

I gather you have a satisfying workaround for now. I'll keep you posted!

joerghampel commented 3 years ago

After reading your comment and the linked document, I now better understand the problem: It's not that the password has to be UTF8 encoded, but it needs to be prepended by two bytes containing its length:

The Password field contains 0 to 65535 bytes of binary data prefixed with a two byte length field which indicates the number of bytes used by the binary data (it does not include the two bytes taken up by the length field itself).

So maybe this is a bug in the LVOS MQTT Client after all, and the fix would not be to UTF8 encode it but to add the two bytes for the length?

francois-normandin commented 3 years ago

Yes, it is supposed to contain two bytes and then the password itself. I see what you mean. The utf8 node prepends those two bytes while not altering the characters which were already utf8 compliant, so I can see why it would work after you've introduced the node. I'll confirm with code today and provide a fix quickly.

Thanks @joerghampel

francois-normandin commented 3 years ago

I've extended the node to bypass UTF8 encoding and applied it to password field. You should see a MQTT Control Packet update later today on VIPM Community.

image

image

For the record (on the importance of complete Unit Testing), here was the unit tests on the Connect packet: image

francois-normandin commented 3 years ago

Additional tests all pass. image

francois-normandin commented 3 years ago

Package will be uploaded to VIPM Community and should appear shortly in your stream. The release can be found here for immediate download: https://github.com/LabVIEW-Open-Source/LV-MQTT-Control-Packets/releases/tag/3.1.3

joerghampel commented 3 years ago

Fantastic, thank you so much!