eclipse / kapua

Eclipse Public License 2.0
225 stars 159 forks source link

Disabling cleanSession on a MQTT client leads to a security exception #2700

Open reda-alaoui opened 5 years ago

reda-alaoui commented 5 years ago

Describe the bug When I create an Eclipse Paho client without clean session option, subscribing to a topic leads to security exception thrown by KapuaSecurityBrokerFilter. Doing the same with the clean session option works fine.

To Reproduce Steps to reproduce the behavior:

  1. On Kapua, create an account 'ACME123'
  2. On Kapua, create a user 'user123' for the account 'ACME123'
  3. Give role ADMIN and permission ALL to the created user
  4. In a separate project, add this dependency:
    <dependency>
    <groupId>org.eclipse.paho</groupId>
    <artifactId>org.eclipse.paho.client.mqttv3</artifactId>
    <version>1.2.1</version>
    </dependency>
  5. Create the client like this:
    MqttClient client = new MqttClient("tcp://localhost:1883", "bar");
    MqttConnectOptions options = new MqttConnectOptions();
        options.setAutomaticReconnect(true);
        options.setUserName("user123");
        options.setCleanSession(false);
        options.setPassword("secret".toCharArray());
        client.connect(options);
  6. Subscribe the client to topic like this:
    client.subscribe("ACME123/+/acme/bus", (topic, message) -> { /* Do nothing */ });

Expected behavior The client should subscribe to the topic.

Screenshots The client fails with a code 128. On the broker side, the following stacktrace is printed:

broker_1         | 16:14:55.384 [ActiveMQ NIO Worker 6] WARN  o.a.a.t.m.s.AbstractMQTTSubscriptionStrategy - Error subscribing to ACME123/+/acme/bus
broker_1         | java.lang.SecurityException: User user123 (bar - tcp://172.19.0.1:43928 - conn id 6704632090685224497) is not authorized to read from: Consumer.2,089,830,197,819,948,377:bar:AT_LEAST_ONCE.VirtualTopic.ACME123.*.acme.bus
broker_1         |     at org.eclipse.kapua.broker.core.plugin.KapuaSecurityBrokerFilter.internalAddConsumer(KapuaSecurityBrokerFilter.java:649)
broker_1         |     at org.eclipse.kapua.broker.core.plugin.KapuaSecurityBrokerFilter.addConsumer(KapuaSecurityBrokerFilter.java:604)

Version of Kapua 1.0.6

Type of deployment Docker

Main component affected Message Broker

reda-alaoui commented 5 years ago

Why the Question/support label? Is it not a bug?

riccardomodanese commented 5 years ago

sorry our fault

reda-alaoui commented 5 years ago

Thanks :)

riccardomodanese commented 5 years ago

Hi @reda-alaoui, I'm working on this issue and I wrote a patch. It's not fully tested so it's still in a branch (fix-MqttCleanSessionAcl). If you have a chance, can you test your application against this branch and give me a feedback?

Before testing the fix please note: with clean session false and the Virtual topic feature active (as in Kapua) when you subscribe (through MQTT protocol) a topic with QoS>0, ActiveMQ "move" this subscription to a virtual queue. So if MQTT client-1 publishes to ACME123/client1/acme/busand MQTT client-2 subscribes to ACME123/+/acme/bus, the client-2 will receive messages from a topic like ACME123/+/acme/bus. It sounds like an MQTT protocol violation. I'll investigate further on that. In any case, if you are using a Kura payload for you messages, you should have the original topic available inside the Kura payload.

reda-alaoui commented 5 years ago

Hi @riccardomodanese ,

Thanks a lot. I am going to test this today.

reda-alaoui commented 5 years ago

Hi @riccardomodanese , Sorry I could not test this earlier.

With your patch, with cleanSession off, the topic subscription does not fail anymore (yay !), but the subscribed client does not receive messages anymore. If I re-enable the cleanSession option, the messages are received by the client. Should I do something in addition in Eclipse Paho to be able to receive messages when cleanSession is off?

riccardomodanese commented 5 years ago

No it should work whithout any additional setting.

Are you following the steps 5 and 6 of the issue description? (your client does nothing on message arrive callback. Did you change the code to be able to detect if the message is sent to the client?)

Which is the topic your client is publishing and which is the client profile?

reda-alaoui commented 5 years ago

Are you following the steps 5 and 6 of the issue description? (your client does nothing on message arrive callback. Did you change the code to be able to detect if the message is sent to the client?)

Yes I am following the same steps. Yes my callback prints a message. The same callback prints a message with cleanSession on but does not with cleanSession off.

Which is the topic your client is publishing and which is the client profile?

The message is published on ACME123/yo/acme/bus . Not sure what you mean by client profile. The message is published with QoS=1 and Retain=false.

reda-alaoui commented 5 years ago

For the record, the consuming client is Eclipse Paho 1.2.2

reda-alaoui commented 5 years ago

When Paho 1.2.2 with clean session OFF is directly connected to activeMQ (without Kapua wrapping), messages are correctly received.

reda-alaoui commented 5 years ago

@riccardomodanese I am now sure I can't receive messages with my configuration because of the wildcard topic subscription.

Subscribing without wildcard is OK with cleanSession ON and cleanSession OFF. Subscribing using wildcard is KO with cleanSession OFF.

reda-alaoui commented 5 years ago

Please note that the issue is specific to Kapua. Wildcard subscription works fine with ActiveMQ and cleanSession OFF.

riccardomodanese commented 5 years ago

Yes during my first tests (see comment) when subscribing with QoS>0 the message topic contains the wildcards. No it doesn't seem a Kapua only issue, I tested ActiveMQ 5.14.5 (whithout any external software) and the behaviour is the same. I'm still doing some investigations to understand if there is something we can do on Kapua side to avoid this behavior.

Can you use the original topic coming from the Kura message?

Anyway this is a report extracted from my tests:

Client 1 is publishing and client 2 is subscribed to # (username and password random since no security plugin is installed) Clean session FALSE - QoS = pub 1 / subs 0

Published Received
kapua-sys/client-id/topic1/topic2 kapua-sys/client-id/topic1/topic2
abcdef abcdef

Clean session FALSE - QoS = pub 1 / subs 1

Published Received
kapua-sys/client-id/topic1/topic2 #
abcdef #

Clean session FALSE - QoS = pub 1 / subs 1

Published Received
kapua-sys/client-id/topic1/topic2 #
abcdef #
riccardomodanese commented 5 years ago

Just for my curiosity, could you repeat your tests using a Paho Mqtt callback implementation instead of a lambda expression on subscription?

reda-alaoui commented 5 years ago

@riccardomodanese , using the callback changes nothing for me. I uploaded my tests to https://github.com/Cosium/paho-module It contains 2 integration tests, one with ActiveMQ the other with Kapua. It uses TestContainer so you should be able to run it easily.

As you will see on this project, the ActiveMQ tests pass while the Kapua ones fail.

riccardomodanese commented 5 years ago

I did a quick look at your tests. They are slightly different from mine since you are connecting through ws. The ActiveMQ container you are using has a different configuration compared to which we are using in Kapua. The virtual topic feature is disabled. You can get the same configuration changing the connector setting in activemq. You can remove just the transport.subscriptionStrategy=mqtt-virtual-topic-subscriptions option. Without the virtual topic option enabled you should see the same behaviour of the ActiveMQ container you are using in your tests (but in that case you should use the standard security broker plugin and not the modified version contained in the branch I suggested you to test).

reda-alaoui commented 5 years ago

but in that case you should use the standard security broker plugin and not the modified version contained in the branch I suggested you to test

Did you mean the plugin of the develop branch? Because when I disable virtual topic on develop branch, the subscription leads to:

User kapua-sys (4c50bbca-b7a4-49a4-8036-bc3da28b7346 - ws://192.168.48.8:48562 - conn id null) is not authorized to read from: topic://kapua-sys.>
riccardomodanese commented 5 years ago

Are you using kapua-sys or kapua-broker as user to connect? Because if you get this error with kapua-broker user it's right. Anyway tomorrow I will try to test Kapua without virtual topic.

reda-alaoui commented 5 years ago

I am using user kapua-sys

riccardomodanese commented 5 years ago

If you clean the VT_TOPIC_PREFIX constant and change the camel subscription to something like (just for testing, see *):

uri="activemq:topic:>?asyncConsumer=false&amp;acknowledgementModeName=CLIENT_ACKNOWLEDGE"

you should get Kapua working without Virtual topics enabled.

* Then if you get the right response from the system and you like to disable virtual topic feature, for performance reason, you should configure a virtual queue (using virtual destination feature) to enable parallel message processing (composite queue).