Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to configure whether client should be disconnected on unauthorized publish #522

Open
strowk opened this issue Aug 11, 2024 · 0 comments

Comments

@strowk
Copy link

strowk commented Aug 11, 2024

Problem or use case

When clients send PUBLISH and server defines that that publish is unauthorized, it might sometimes be inconvenient to disconnect the client.

Looking at specifications, in 3.1.1 (from line 835):

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].

Note that there are technically two possibilities given here "make a positive acknowledgement" or "close the Network Connection". I read this in the way that it is possible that server would not authorize PUBLISH (and therefore not forward it to other clients), however it would still make a positive acknowledgement.

In MQTT 5 there is more interesting mention, though (from line 3492):

Acknowledgment packets PUBACK, PUBREC, PUBREL, PUBCOMP, SUBACK, UNSUBACK with a Reason Code of 0x80 or greater indicate that the received packet, identified by a Packet Identifier, was in error. There are no consequences for other Sessions or other Packets flowing on the same Session.

The CONNACK and DISCONNECT packets allow a Reason Code of 0x80 or greater to indicate that the Network Connection will be closed. If a Reason Code of 0x80 or greater is specified, then the Network Connection MUST be closed whether or not the CONNACK or DISCONNECT is sent [MQTT-4.13.2-1]. Sending of one of these Reason Codes does not have consequence for any other Session.

The way I read this, it is clear that CONNACK and DISCONNECT with 0x87 (unauthorized) must result in closed network connection, however for PUBACK earlier it is said "There are no consequences for other Sessions or other Packets flowing on the same Session.". I suspect maybe the bit "must result in closed network connection" was mistakenly attributed to PUBACK as well. I believe since that paragraph starts with "The CONNACK and DISCONNECT packets...", this only is applied to establishing of connection process.

Hence it would appear that by automatically disconnecting, HiveMQ actually does not behave according to at least MQTT spec version 5.

Relevant HiveMQ documentation: https://docs.hivemq.com/hivemq/latest/extensions/authorization.html#publish

When a Publish is denied by calling any of the failAuthorization methods, then the following MQTT behaviour can be expected:

MQTT 3.1.1, All QoS levels
Connection is closed by the broker.

MQTT 5...
connection is closed

This last bit is repeated for all three QoS

Preferred solution or suggestions

Allow for PublishAuthorizer to pass to output a boolean indicating whether connection is going to be closed or not.
The relevant method is PublishAuthorizerOutput#failAuthorization ( https://github.com/hivemq/hivemq-extension-sdk/blob/c485b05d4e70a373a1e30fafbe7c1fb3657c8ff1/src/main/java/com/hivemq/extension/sdk/api/auth/parameter/PublishAuthorizerOutput.java#L107-L126 ), that I'd like to have an overloads with boolean disconnect.

If disconnect was set to false, then for MQTT 3 server should respond with positive ack, as if publish was authorized and passed, however clients should not receive it from their subscriptions.
For MQTT 5 server should respond with PUBACK/PUBREC (depending on QoS) with 0x87 NOT_AUTHORIZED (or specified reason code), but not send DISCONNECT and not close connection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant