I'm struggling to reproduce the NPE. I pulled down Kapua, switched to your
branch (i.e. upgrade-artemis-2_21), configured Docker, etc., but I get this
error when I run RunDeviceBrokerI9nTest:
ERROR o.e.k.q.i.steps.DockerSteps - Error while starting base docker
environment: Image not found: kapua/kapua-sql:2.0.0-ARTEMIS-SNAPSHOT
I assume I missed a step somewhere. If you could outline the steps
necessary to run the test starting from a completely bare environment I'd
be grateful. Also, it might be useful for you to try reproducing the
problem without the Kapua plugins to narrow the problem down a bit.
Regarding the other problems, I'd prefer to focus on one thing at a time if
possible.
Justin
On Mon, Apr 11, 2022 at 10:29 AM Modanese, Riccardo
<riccardo.modan...@eurotech.com.invalid> wrote:
> Hi Justin, I created a small test (using Paho client) and I confirm the
> null pointer while a “regular” stealing link happens (with Kapua security
> and server plugins configured)
>
> ERROR [org.apache.activemq.artemis.core.protocol.mqtt] AMQ834002: Error
> processing control packet:
> MqttConnectMessage[fixedHeader=MqttFixedHeader[messageType=CONNECT,
> isDup=false, qosLevel=AT_MOST_ONCE, isRetain=false, remainingLength=58],
> variableHeader=MqttConnectVariableHeader[name=MQIsdp, version=3,
> hasUserName=true, hasPassword=true, isWillRetain=false, isWillFlag=false,
> isCleanSession=true, keepAliveTimeSeconds=60],
> payload=MqttConnectPayload[clientIdentifier=test-client-admin,
> willTopic=null, willMessage=null, userName=kapua-sys, password=[107, 97,
> 112, 117, 97, 45, 112, 97, 115, 115, 119, 111, 114, 100]]]:
> java.lang.NullPointerException
> at
> org.apache.activemq.artemis.core.protocol.mqtt.MQTTUtil.validateClientId(MQTTUtil.java:524)
> [artemis-mqtt-protocol-2.22.0-SNAPSHOT.jar:2.22.0-SNAPSHOT]
> at
> org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolHandler.handleConnect(MQTTProtocolHandler.java:228)
> [artemis-mqtt-protocol-2.22.0-SNAPSHOT.jar:2.22.0-SNAPSHOT]
>
>
> I saw another couple of weird behaviors while testing the 2.22.0-SNAPSHOT
> (openjdk 13).
> 1) Kapua plugin receives the MQTT client id null during the connect.
> 2) If I set the clientId value, anyway, no messages are received by the
> client (on valid subscriptions)
> The client has the subscriptions granted by the broker (here there is an
> extract of Kapua log):
> ### authorizing address: $EDC/kapua-sys/test-client-1/# - check type:
> CREATE_ADDRESS - clientId: test-client-1 - clientIp: /172.29.0.1:58480 -
> RESULT: true
> ### authorizing address:
> $EDC/kapua-sys/test-client-1/#::test-client-1.$EDC/kapua-sys/test-client-1/#
> - check type: CREATE_DURABLE_QUEUE - clientId: test-client-1 - clientIp: /
> 172.29.0.1:58480 - RESULT: true
> ### authorizing address:
> $EDC/kapua-sys/test-client-1/#::test-client-1.$EDC/kapua-sys/test-client-1/#
> - check type: CONSUME - clientId: test-client-1 - clientIp: /
> 172.29.0.1:58480 - RESULT: true
> But NO messages are received by the client (with 2.19.0 – openjdk 8
> worked)
>
> With the 2.22.0-SNAPSHOT + your PR the client id is correctly received by
> the plugin and from the subscriptions I can say the client id manipulation
> seems to be “acquired” by the broker:
> ### authorizing address: $EDC/kapua-sys/test-client-1/# - check type:
> CREATE_ADDRESS - clientId: test-client-1 - clientIp: /172.29.0.1:58528 -
> RESULT: true
> ### authorizing address:
> $EDC/kapua-sys/test-client-1/#::AQ|test-client-1.$EDC/kapua-sys/test-client-1/#
> - check type: CREATE_DURABLE_QUEUE - clientId: test-client-1 - clientIp: /
> 172.29.0.1:58528 - RESULT: true
> ### authorizing address:
> $EDC/kapua-sys/test-client-1/#::AQ|test-client-1.$EDC/kapua-sys/test-client-1/#
> - check type: CONSUME - clientId: test-client-1 - clientIp: /
> 172.29.0.1:58528 - RESULT: true
> (the client id “test-client-1” become “AQ|test-client-1” and it’s what I
> expect) but, also in this case, NO messages are received by the client on
> valid subscriptions.
>
> If could be helpful we had customized the wildcards to use the MQTT one (I
> didn’t find any documentation about syntax changes on 2.22 so I’m assuming
> is still valid right?)
> <wildcard-addresses>
> <routing-enabled>true</routing-enabled>
> <delimiter>/</delimiter>
> <any-words>#</any-words>
> <single-word>+</single-word>
> </wildcard-addresses>
>
> I’m looking forward for your feedback
> Thanks!
>
> Riccardo
>
> Da: Modanese, Riccardo <riccardo.modan...@eurotech.com.INVALID>
> Data: lunedì, 11 aprile 2022 08:58
> A: users@activemq.apache.org <users@activemq.apache.org>
> Oggetto: R: Artemis security plugin doesn't allow to change clientId
> May be our plugin can be the cause? Anyway I’m still investigating.
>
> Riccardo
>
> Da: Justin Bertram <jbert...@apache.org>
> Data: venerdì, 8 aprile 2022 21:58
> A: users@activemq.apache.org <users@activemq.apache.org>
> Oggetto: Re: Artemis security plugin doesn't allow to change clientId
> That's weird. There's a test in the ActiveMQ Artemis test-suite that
> exercises this use-case and it's running fine. I'll see if I can set up
> Kapua locally and reproduce the failure you're seeing.
>
>
> Justin
>
> On Fri, Apr 8, 2022 at 10:51 AM Modanese, Riccardo
> <riccardo.modan...@eurotech.com.invalid> wrote:
>
> > Hi Justin I got a NPE while broker is handling “normal” stealing link:
> >
> > 2022-04-08 15:23:37,540 ERROR
> > [org.apache.activemq.artemis.core.protocol.mqtt] AMQ834002: Error
> > processing control packet:
> > MqttConnectMessage[fixedHeader=MqttFixedHeader[messageType=CONNECT,
> > isDup=false, qosLevel=AT_MOST_ONCE, isRetain=false, remainingLength=42],
> > variableHeader=MqttConnectVariableHeader[name=MQTT, version=4,
> > hasUserName=true, hasPassword=true, isWillRetain=false, isWillFlag=false,
> > isCleanSession=true, keepAliveTimeSeconds=60],
> > payload=MqttConnectPayload[clientIdentifier=clientId, willTopic=null,
> > willMessage=null, userName=acme-2, password=[75, 101, 101, 112, 67, 97,
> > 108, 109, 49, 50, 51, 46]]]: java.lang.NullPointerException
> > at
> >
> org.apache.activemq.artemis.core.protocol.mqtt.MQTTUtil.validateClientId(MQTTUtil.java:524)
> > [artemis-mqtt-protocol-2.22.0-SNAPSHOT.jar:2.22.0-SNAPSHOT]
> > at
> >
> org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolHandler.handleConnect(MQTTProtocolHandler.java:228)
> > [artemis-mqtt-protocol-2.22.0-SNAPSHOT.jar:2.22.0-SNAPSHOT]
> > at
> >
> org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolHandler.act(MQTTProtocolHandler.java:153)
> > [artemis-mqtt-protocol-2.22.0-SNAPSHOT.jar:2.22.0-SNAPSHOT]
> > at
> > org.apache.activemq.artemis.utils.actors.Actor.doTask(Actor.java:33)
> > [artemis-commons-2.22.0-SNAPSHOT.jar:]
> > at
> >
> org.apache.activemq.artemis.utils.actors.ProcessorBase.executePendingTasks(ProcessorBase.java:65)
> > [artemis-commons-2.22.0-SNAPSHOT.jar:]
> > at
> >
> java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
> > [java.base:]
> > at
> >
> java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
> > [java.base:]
> > at
> >
> org.apache.activemq.artemis.utils.ActiveMQThreadFactory$1.run(ActiveMQThreadFactory.java:118)
> > [artemis-commons-2.22.0-SNAPSHOT.jar:]
> >
> >
> > if (connection != null) {
> > MQTTSession existingSession =
> > session.getProtocolManager().getSessionState(clientId).getSession();
> > if (session.getVersion() == MQTTVersion.MQTT_5) {
> >
> >
> existingSession.getProtocolHandler().sendDisconnect(MQTTReasonCodes.SESSION_TAKEN_OVER);
> > }
> > // [MQTT-3.1.4-2] If the client ID represents a client already
> > connected to the server then the server MUST disconnect the existing
> client
> > existingSession.getConnectionManager().disconnect(false);//NPE
> > }
> > so the connection is not null but has no session defined from what I can
> > understand.
> >
> > If could be helpful I created a new branch on my Kapua fork (
> > https://github.com/riccardomodanese/kapua/tree/upgrade-artemis-2_21)
> > linked to Artemis 2.22.0-SNAPSHOT (I built Artemis locally from main
> branch
> > + cherry-pick your commit)
> > The test I run was: RunDeviceBrokerI9nTest (see DeviceBrokerI9n.feature)
> >
> > Looking forward for your feedback!
> >
> > Riccardo
> >
> >
> >
> >
> >
> >
> > Da: Modanese, Riccardo <riccardo.modan...@eurotech.com>
> > Data: venerdì, 8 aprile 2022 08:42
> > A: users@activemq.apache.org <users@activemq.apache.org>
> > Oggetto: R: Artemis security plugin doesn't allow to change clientId
> > Sure, I’ll test asap thanks!
> > (I’m currently doing my testing on 2.19, I don’t expect conflicts if I
> > cherry-pick the commit)
> >
> > Da: Justin Bertram <jbert...@apache.org>
> > Data: venerdì, 8 aprile 2022 03:05
> > A: users@activemq.apache.org <users@activemq.apache.org>
> > Oggetto: Re: Artemis security plugin doesn't allow to change clientId
> > I just sent a PR [1] for this. Riccardo, could you try this out and see
> if
> > it works for you?
> >
> >
> > Justin
> >
> > [1] https://github.com/apache/activemq-artemis/pull/4021
> >
> > On Thu, Apr 7, 2022 at 2:04 PM Justin Bertram <jbert...@apache.org>
> wrote:
> >
> > > I went ahead and opened ARTEMIS-3770 [1] for this work.
> > >
> > >
> > > Justin
> > >
> > > [1] https://issues.apache.org/jira/browse/ARTEMIS-3770
> > >
> > > On Thu, Apr 7, 2022 at 12:35 PM Justin Bertram <jbert...@apache.org>
> > > wrote:
> > >
> > >> Technically speaking, an implementation of
> > >> o.a.a.a.s.c.s.ActiveMQSecurityManager5 like you have *is* allowed to
> > change
> > >> the client ID on the o.a.a.a.s.c.p.RemotingConnection it receives
> (just
> > as
> > >> you are doing in your implementation). The problem is that MQTT
> > >> implementation doesn't use this client ID and instead overwrites it
> with
> > >> the value from the MQTT CONNECT packet. However, I think the code
> could
> > be
> > >> refactored fairly easily to accommodate this use-case. Can you open a
> > Jira?
> > >>
> > >>
> > >> Justin
> > >>
> > >> On Thu, Apr 7, 2022 at 11:19 AM Modanese, Riccardo
> > >> <riccardo.modan...@eurotech.com.invalid> wrote:
> > >>
> > >>> Hello,
> > >>> we are moving a security plugin from ActiveMQ 5.x broker to
> Artemis
> > >>> 2.x.
> > >>> To summarize the use case:
> > >>> we need to prefix the MQTT client id provided during the connect
> > >>> with the account name (something like account_name|client_id) to
> allow
> > >>> devices with the same clientId, but different accounts, to connect to
> > the
> > >>> broker without triggering the stealing link.
> > >>>
> > >>> Doing that with the ActiveMQ was possible. With Artemis
> SecurityPlugin
> > >>> any clientId set through the proper RemotingConnection setter has no
> > effect
> > >>> (
> > >>>
> >
> https://github.com/riccardomodanese/kapua/blob/feature-artemisAuthentication/broker/artemis/plugin/src/main/java/org/eclipse/kapua/broker/artemis/plugin/security/SecurityPlugin.java#L140
> > >>> ).
> > >>> Also the fully qualified queue name still use the “original” clientId
> > >>> without the account_name prefix
> > >>>
> > >>> We received a suggestion to use the interceptor but unfortunately the
> > >>> MQTTConnect is final and has all the fields final so we cannot change
> > the
> > >>> clientId
> > >>> We tried, just as experiment, using reflection to change the
> > >>> accessibility (no security manager) and it seems to work. But,
> > obviously,
> > >>> is just an experiment and cannot be used in a real environment.
> > >>>
> > >>> The MQTTConnect message is created by MQTTDecoder (
> > >>>
> >
> https://github.com/netty/netty/blob/4.1/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttDecoder.java#L534
> > )
> > >>> but changing this part to introduce a callback that allows to change
> > the
> > >>> decoded clientId is out of the scope of this layer IMHO.
> > >>>
> > >>> If someone has suggestion or, better, a solution please tell me!
> > >>>
> > >>> Thanks!
> > >>>
> > >>> Riccardo Modanese
> > >>>
> > >>>
> >
>
