When a MQTTv311 client tries to publish with qos2, and fails OnACLCheck, the client hangs forever and the connection stays open. Should we disconnect the client instead?

[SimonMacIntyre]

http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718042

If a Server implementation does not authorize a PUBLISH to be performed by a Client; it has no way of informing that Client. It MUST either make a positive acknowledgement, according to the normal QoS rules, or close the Network Connection [MQTT-3.3.5-2].

I encountered this when testing my implementation and testing a publish qos2 which succesfully Connects (authenticates), but fails OnACLCheck, in my case using Python Paho client 3.1.1, the publish fails but the client hangs forever on the publish.

When I looked into the broker code it appears on failure of ACL check that it returns nil with no handling.

Please let me know if I am missing anything or misinformed!

[SimonMacIntyre]

This case is the same using client of MQTTv5 too

[mochi-co]

I believe you are correct @SimonMacIntyre, this is a bug, at least for MQTTv3. If I remember, for v5 there is no provision to disconnect on ACL errors as some servers may wish to obscure their restrictions. I may be wrong though.

[SimonMacIntyre]

I'll take a quick peek! I see these in the v5 spec:

The Client or Server sending the PUBACK packet MUST use one of the PUBACK Reason Codes [MQTT-3.4.2-1] The Client or Server sending the PUBREC packet MUST use one of the PUBREC Reason Code values. [MQTT-3.5.2-1]

Same PUBACK reason code for both would be:

135 | 0x87 | Not authorized | The PUBLISH is not authorized.

For qos0, it appears to be handled correctly! (In the sense that doing nothing is not hanging the client and the client thinks its 'published', of course thats the nature of qos0)

[dgduncan]

So from reading that it seems like the spec for v3.1.1 should be to either ack and not publish or disconnect. A disconnect seems like the preferred solution as it makes it obvious there is a breach in the server rules; however, I can absolutely imagine a world in which a user of the server may not have control over some functionality of their MQTT client because it may be third part and a disconnection may make the server useless to them. Perhaps a capability could be in order to adjust functionality but that may just add unwanted complexity

[SimonMacIntyre]

Personally I lean towards the disconnect, over the ack and not publish.

I do like the idea of in the future making that configurable in the file configuration, but maybe the disconnect is a reasonable path forward for now and if it becomes a problem for users we can explore making the server configurable on how to handle that only for v3.1.1?

[thedevop]

Created a PR #292 to address this.

The server will:

	QOS 0	QOS 1	QOS 2
MQTT v3	Ignore	Disconnect	Disconnect
MQTT v3.1.1	Ignore	Disconnect	Disconnect
MQTT v5	Ignore	Puback NotAuthorized	Pubrec NotAuthorized

[mochi-co]

This has been fixed and released in v2.4.0. Thank you @thedevop!