MQTT 5.0: Connection close on non-authorized topic subscription attempt

[AntonSmolkov]

Describe the bug

The broker closes the MQTT connection when attempting to subscribe to a topic that is not allowed at the topic permissions level.

For example, with the following C# code, the entire connection is closed shortly after line 2:

// GrantedQos0
await mqttClient.SubscribeAsync("allowed_topic", MqttQualityOfServiceLevel.AtMostOnce);
// NotAuthorized 
await mqttClient.SubscribeAsync("denied_topic", MqttQualityOfServiceLevel.AtMostOnce); 
// GrantedQos0 (not works with RabbitMQ)
await mqttClient.SubscribeAsync("allowed_topic_another", MqttQualityOfServiceLevel.AtMostOnce); 

Wireshark looks like this: (notice Reason Code column)

RabbitMq logs report subscribe_error on connection:

rabbit-1  | 2024-11-16 12:45:34.588125+00:00 [debug] <0.989.0> User 'user1' authenticated successfully by backend rabbit_auth_backend_internal
rabbit-1  | 2024-11-16 12:45:34.588474+00:00 [info] <0.989.0> Accepted MQTT connection 127.0.0.1:58966 -> 127.0.0.1:1883 for client ID my-pod-name8
rabbit-1  | 2024-11-16 12:45:36.524394+00:00 [debug] <0.989.0> Received a SUBSCRIBE with subscription(s) [{mqtt_subscription,
rabbit-1  | 2024-11-16 12:45:36.524394+00:00 [debug] <0.989.0>                                             <<"allowed_topic">>,
rabbit-1  | 2024-11-16 12:45:36.524394+00:00 [debug] <0.989.0>                                             {mqtt_subscription_opts,0,false,
rabbit-1  | 2024-11-16 12:45:36.524394+00:00 [debug] <0.989.0>                                              false,0,undefined}}]
rabbit-1  | 2024-11-16 12:45:38.104373+00:00 [debug] <0.989.0> Received a SUBSCRIBE with subscription(s) [{mqtt_subscription,
rabbit-1  | 2024-11-16 12:45:38.104373+00:00 [debug] <0.989.0>                                             <<"denied_topic">>,
rabbit-1  | 2024-11-16 12:45:38.104373+00:00 [debug] <0.989.0>                                             {mqtt_subscription_opts,0,false,
rabbit-1  | 2024-11-16 12:45:38.104373+00:00 [debug] <0.989.0>                                              false,0,undefined}}]
rabbit-1  | 2024-11-16 12:45:38.105474+00:00 [error] <0.989.0> MQTT topic access refused: read access to topic 'denied_topic' in exchange 'amq.topic' in vhost 'mqtt' refused for user 'user1'
rabbit-1  | 2024-11-16 12:45:38.105653+00:00 [error] <0.989.0> Failed to add binding between exchange 'amq.topic' in vhost 'mqtt' and queue 'mqtt-subscription-my-pod-name8qos0' in vhost 'mqtt' for topic filter denied_topic: access_refused
rabbit-1  | 2024-11-16 12:45:38.105786+00:00 [error] <0.989.0> MQTT protocol error on connection 127.0.0.1:58966 -> 127.0.0.1:1883: subscribe_error

As far as i understand, this behavior doesn't complain to the MQTT5 standard.

For instance, the same code/approach, works fine with EMQX broker:

Not an Erlang expert, but seems like the problem is somewhere in this part of the code

Reproduction steps

1. Create user in the befault or any other auth backend
2. Grant ACL permision .* .* .* to the user
3. Grant ^allowed_topic.* read/write topic permission
4. Create MQTT Connection, subscribe to allowed_topic (sucess)
5. Subscribe to any other topic e.g. denied_topic
6. Connection is closed

Expected behavior

The connection remains open, allowing further subscriptions to other allowed topics.

Additional context

RabbitMQ 4.0.3

[michaelklishin]

Thank you for providing details and a traffic capture. According to the stack trace, the code that handles a SUBSCRIBE frame does not expect errors related to insufficient permissions.

[ansd]

As far as i understand, this behavior doesn't complain to the MQTT5 standard.

@AntonSmolkov Where in the MQTT 5.0 spec do you read this?

Expected behavior
The connection remains open, allowing further subscriptions to other allowed topics.

In its current implementation, RabbitMQ closes the MQTT connection if an authorisation check fails. This is done intentionally and is in line with how RabbitMQ behaves with other protocols such as AMQP 1.0 and AMQP 0.9.1.

Note that RabbitMQ complies with the following MQTT 5.0 requirement by sending the SUBACK packet before closing the connection as shown in your Wireshark capture above:

When the Server receives a SUBSCRIBE packet from a Client, the Server MUST respond with a SUBACK packet [MQTT-3.8.4-1].

[AntonSmolkov]

Of course, SUBACK. (Edited the message)

[AntonSmolkov]

Closing the connection immediately can serve as a protective measure.

But tcp reconnections are not limited and can make much more harm.
Why AMQP connections are not closed the same way then?

What is the use case that your client subscribes to unauthorised topic filters and thereafter needs the connection to be open?

MQTT protocol implies that client can subscribe and unsubscribe to/from different topics during the whole connection lifetime and even beyond (persistent sessions).

Why can't you make sure that your client subscribes to only those topic filters it has authorisation for?

Well, this would be another authorization layer. Clients may not know in advance what topics are denied or allowed for them.

[michaelklishin]

Why AMQP connections are not closed the same way then?

Because AMQP 0-9-1 (with the exception of authentication, which was rectified by a RabbitMQ extension to it) and AMQP 1.0 had server-to-client error handling from the early days while MQTT before v5 did not, so the best a broker could do was to close the connection.

This has been the case since 2012-2013, so for a good decade. And it wasn't really a commonly discussed topic.

Why should we change anything if the behavior is quite reasonable and it's been like this for a decade?

[michaelklishin]

Clients may not know in advance what topics are denied or allowed for them

This is not a RabbitMQ's problem to solve.

[AntonSmolkov]

Why should we change anything if the behavior is quite reasonable and it's been like this for a decade?

As you’ve just mentioned - because MQTTv5 was released after this decades, and now it has the server-to-client error handling in its spec.

[michaelklishin]

@AntonSmolkov and how many clients actually follow that spec in this specific scarcely documented area? If it's a minority then we'd be doing everyone and ourselves a disservice.

[michaelklishin]

There are no clear instructions in the section 3.8 SUBSCRIBE in the MQTTv5 spec as to how a server should indicate a lack of permissions to the client.

Closing the connection is not great but that's how things were for the majority of post-authentication errors before MQTTv5, and MQTTv5 does not clarify many cases any way, so it's up to the implementation to decide what to do.

Section 4.13.2 Other Errors is incredibly terse and suggests using a SUBACK with a special code. I have no idea how most popular clients would react to that frame, and if they do not e.g. thrown an exception (or explicitly indicate an error any other way), this solution would be worse than what we have today.

@AntonSmolkov you are welcome to investigate how other MQTT servers and client libraries handle such scenarios.

A "partial success" is the absolute worst scenario to handle, and most messaging protocols make it impossible to perform N operations at a time, some of which may fail. Needless to say, partial failures are not even discussed in the MQTTv5 spec as far as I can tell.

So one more time we learn that MQTT is a poorly designed protocol as far as technical operations and error handling go. This was dead obvious in the MQTT < 5 days and is still true to some extent today.

This is not something our team has the resources to try to address. Avoid features that can "partially succeed" and subscribe to N topics one by one so that your client knows exactly what to do on a retry.

[michaelklishin]

I'm not interesting in spending any more time on this discussion as it is. MQTT before v5 had a catastrophically bad error handling story, this is widely known by now.

The "partial success, partial failure" case is not explicitly mentioned the MQTTv5 spec, so it's up to each individual implementer to decide what should be done. We have decided to close the connection. It is not unreasonable, this is how MQTT largely been for over a decade before it's reached v5.

How many clients handle SUBACK with the "error flag" set remains to be verified. @AntonSmolkov if you care enough about switching to SUBACK, it's on you to investigate how many popular MQTT clients that support v5 also react reasonable to such a SUBACK. My guess is that some don't handle it at all but they sure cannot not handle a closed TCP connection.

If/when you have that data you can start another discussion with it and we will see if switching to SUBACK for 4.1 or 4.2 would be worth it.