You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@activemq.apache.org by "Modanese, Riccardo" <Ri...@eurotech.com.INVALID> on 2022/05/03 13:01:34 UTC

R: Artemis security plugin doesn't allow to change clientId

Hi Justin,
    I was able to reproduce the NPE using a very small SecurityPlugin and ServerPlugin (more or less stub classes with just few lines of code).

The code is pushed in my fork (I push forced to the branch upgrade-artemis-2_21).

I removed all the Kapua modules in the latest commit and I added few modules that create an Artemis image (Artemis version is 2.23.0-SNAPSHOT + your commit, I hope I did the right way) including the “dummy” Server and Security plugins.

Once built with
mvn clean install -DskipITs -DskipTests -Pdocker

[INFO] Reactor Summary for kapua 2.0.0-ARTEMIS-SNAPSHOT:
[INFO]
[INFO] kapua .............................................. SUCCESS []
[INFO] kapua-artemis-test ................................. SUCCESS []
[INFO] kapua-assembly-java-base ........................... SUCCESS []
[INFO] kapua-broker-artemis-plugin-test ................... SUCCESS []
[INFO] kapua-assembly-broker-artemis-test ................. SUCCESS []
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------

you can move to
cd deployment/docker/unix

and start the broker with compose (only the broker container will start)
./docker-deploy.sh

Then if you run the test TestStealingLink you can see one NPE at the first run

2022-05-03 12:34:19,022 INFO  [org.eclipse.kapua.broker.artemis.plugin.security.DummySecurityPlugin] Authenticate: updated clientId: newId-1-client-stealing-multi-account
2022-05-03 12:34:19,348 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=71], variableHeader=MqttConnectVariableHeader[name=MQTT, version=4, hasUserName=true, hasPassword=true, isWillRetain=false, isWillFlag=false, isCleanSession=true, keepAliveTimeSeconds=60], payload=MqttConnectPayload[clientIdentifier=client-stealing-multi-account, willTopic=null, willMessage=null, userName=kapua-broker, 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:552) [artemis-mqtt-protocol-2.23.0-SNAPSHOT.jar:2.23.0-SNAPSHOT]
                at org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolHandler.handleConnect(MQTTProtocolHandler.java:229) [artemis-mqtt-protocol-2.23.0-SNAPSHOT.jar:2.23.0-SNAPSHOT]
                at org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolHandler.act(MQTTProtocolHandler.java:154) [artemis-mqtt-protocol-2.23.0-SNAPSHOT.jar:2.23.0-SNAPSHOT]
                at org.apache.activemq.artemis.utils.actors.Actor.doTask(Actor.java:33) [artemis-commons-2.23.0-SNAPSHOT.jar:]
                at org.apache.activemq.artemis.utils.actors.ProcessorBase.executePendingTasks(ProcessorBase.java:65) [artemis-commons-2.23.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.23.0-SNAPSHOT.jar:]

2022-05-03 12:34:19,356 INFO  [org.eclipse.kapua.broker.artemis.plugin.security.DummyServerPlugin] ### cleanUpConnectionData connection: fca276c8 - reason: DESTROY - Error: N/A

or 2 if you run again the test.

Da: Justin Bertram <jb...@apache.org>
Data: martedì, 26 aprile 2022 15:42
A: users@activemq.apache.org <us...@activemq.apache.org>
Oggetto: Re: Artemis security plugin doesn't allow to change clientId
I assumed that since you could only reproduce the NPE with your plugins
added you were able to identify the root cause of the problem. I gave up
trying to reproduce the problem in Kapua due to a lack of clarity around
running the actual test. I'll go back and take another look at reproducing
it, but either way I'm going to merge the change as it allows modifying the
client ID in the test that I wrote.


Justin

On Tue, Apr 26, 2022 at 2:30 AM Modanese, Riccardo
<Ri...@eurotech.com.invalid> wrote:

> Sorry but I cannot test the fix due to the NPE.
> And even the “normal” stealing link doesn’t work (same NPE) with our
> plugin code.
> I don’t know which is the best way to investigate further.
> So, unfortunately, I cannot say if this fix fits or not our needs since I
> cannot test it.
>
> Riccardo
>
>
> Da: Justin Bertram <jb...@apache.org>
> Data: martedì, 26 aprile 2022 00:20
> A: users@activemq.apache.org <us...@activemq.apache.org>
> Oggetto: Re: Artemis security plugin doesn't allow to change clientId
> I've given up reproducing this NPE for now. At this point can you confirm
> that the PR [1] fits your needs? If so, I can merge it and it can be
> included in the next release which should be started in the next few days.
>
>
> Justin
>
> [1] https://github.com/apache/activemq-artemis/pull/4021
>
> On Wed, Apr 20, 2022 at 2:41 AM Modanese, Riccardo
> <Ri...@eurotech.com.invalid> wrote:
>
> > For the subscription I’ll start a new thread as you suggested.
> >
> > About the NPE I tried again to run the test from my Eclipse IDE (after
> > rebuilding all the images “mvn clean install -DskipITs -DskipTests
> > -Pdocker”) and the test environment seems to start correctly.
> > Which is your output of “docker images | grep kapua”?
> > Sounds like this one?
> >
> > docker images | grep kapua
> > kapua/kapua-job-engine
> >  2.0.0-ARTEMIS-SNAPSHOT   0f4656b1fbda   15 minutes ago   733MB
> > kapua/kapua-job-engine
> >  2022-04-20               0f4656b1fbda   15 minutes ago   733MB
> > kapua/kapua-job-engine
> >  latest                   0f4656b1fbda   15 minutes ago   733MB
> > kapua/kapua-service-authentication
> >  2.0.0-ARTEMIS-SNAPSHOT   866b2caedd80   15 minutes ago   650MB
> > kapua/kapua-service-authentication
> >  2022-04-20               866b2caedd80   15 minutes ago   650MB
> > kapua/kapua-service-authentication
> >  latest                   866b2caedd80   15 minutes ago   650MB
> > kapua/kapua-consumer-lifecycle
> >  2.0.0-ARTEMIS-SNAPSHOT   6f35d27645b6   15 minutes ago   656MB
> > kapua/kapua-consumer-lifecycle
> >  2022-04-20               6f35d27645b6   15 minutes ago   656MB
> > kapua/kapua-consumer-lifecycle
> >  latest                   6f35d27645b6   15 minutes ago   656MB
> > kapua/kapua-consumer-telemetry
> >  2.0.0-ARTEMIS-SNAPSHOT   e313f89c0dce   15 minutes ago   675MB
> > kapua/kapua-consumer-telemetry
> >  2022-04-20               e313f89c0dce   15 minutes ago   675MB
> > kapua/kapua-consumer-telemetry
> >  latest                   e313f89c0dce   15 minutes ago   675MB
> > kapua/kapua-api
> > 2.0.0-ARTEMIS-SNAPSHOT   c5cb9cb37941   16 minutes ago   799MB
> > kapua/kapua-api
> > 2022-04-20               c5cb9cb37941   16 minutes ago   799MB
> > kapua/kapua-api
> > latest                   c5cb9cb37941   16 minutes ago   799MB
> > kapua/kapua-broker-artemis
> >  2.0.0-ARTEMIS-SNAPSHOT   b417ae53f26a   17 minutes ago   795MB
> > kapua/kapua-broker-artemis
> >  2022-04-20               b417ae53f26a   17 minutes ago   795MB
> > kapua/kapua-broker-artemis
> >  latest                   b417ae53f26a   17 minutes ago   795MB
> > kapua/kapua-events-broker
> > 2.0.0-ARTEMIS-SNAPSHOT   c1046e5af987   27 minutes ago   130MB
> > kapua/kapua-events-broker
> > 2022-04-20               c1046e5af987   27 minutes ago   130MB
> > kapua/kapua-events-broker
> > latest                   c1046e5af987   27 minutes ago   130MB
> > kapua/kapua-sql
> > 2.0.0-ARTEMIS-SNAPSHOT   e1c6753a7cf7   28 minutes ago   587MB
> > kapua/kapua-sql
> > 2022-04-20               e1c6753a7cf7   28 minutes ago   587MB
> > kapua/kapua-sql
> > latest                   e1c6753a7cf7   28 minutes ago   587MB
> > kapua/jetty-base
> >  2.0.0-ARTEMIS-SNAPSHOT   3f2edd64f434   28 minutes ago   610MB
> > kapua/jetty-base
> >  2022-04-20               3f2edd64f434   28 minutes ago   610MB
> > kapua/jetty-base
> >  latest                   3f2edd64f434   28 minutes ago   610MB
> > kapua/java-base
> > 2.0.0-ARTEMIS-SNAPSHOT   6b19bd516e28   28 minutes ago   585MB
> > kapua/java-base
> > 2022-04-20               6b19bd516e28   28 minutes ago   585MB
> > kapua/java-base
> > latest                   6b19bd516e28   28 minutes ago   585MB
> >
> >
> > Da: Justin Bertram <jb...@apache.org>
> > Data: martedì, 19 aprile 2022 18:07
> > A: users@activemq.apache.org <us...@activemq.apache.org>
> > Oggetto: Re: Artemis security plugin doesn't allow to change clientId
> > Even after running `mvn clean install -DskipITs -DskipTests -Pdocker` I
> > still get the same 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
> >
> > As for using docker-compose, I don't know where to find your
> > "compose.diff." Perhaps you attached it to your email, but I don't
> believe
> > the mailing list supports attachments.
> >
> > In any case, since you can't reproduce the NPE without Kapua then it must
> > be something in the Kapua code that's causing the unexpected null. I
> would
> > like to make the code more safe so that it fails more gracefully instead
> of
> > throwing an NPE, but it would still fail which means you would still need
> > to change the Kapua code that's causing the unexpected null. However,
> > without a way to reproduce it I can't make any changes.
> >
> > Please start a new thread or open a Jira regarding the subscription
> issue.
> >
> >
> > Justin
> >
> > On Tue, Apr 12, 2022 at 5:35 AM Modanese, Riccardo
> > <Ri...@eurotech.com.invalid> wrote:
> >
> > > I wasn’t able to reproduce the NPE with Artemis 2.22 without Kapua
> code.
> > >
> > > Anyway, the stealing link NPE with Kapua plugins could be reproduced
> also
> > > with docker-compose (the patch remove the not needed containers)
> > >
> > > So (from root Kapua git repo path):
> > >
> > > git apply compose.diff
> > >
> > > cd deployment/docker/unix/
> > >
> > > ./docker-deploy.sh
> > >
> > >
> > >
> > > once the environment is running you can run the test class
> > (TestMqttClient
> > > requires only log4j and Paho as external dependencies)
> > >
> > >
> > >
> > > About the subscription issue I was able to reproduce it also without
> > Kapua
> > > plugins but since you prefer to focus to NPE I’ll tell you in a second
> > time.
> > >
> > >
> > >
> > > Riccardo
> > >
> > >
> > >
> > > *Da: *Modanese, Riccardo <Ri...@eurotech.com>
> > > *Data: *martedì, 12 aprile 2022 09:12
> > > *A: *users@activemq.apache.org <us...@activemq.apache.org>
> > > *Oggetto: *R: Artemis security plugin doesn't allow to change clientId
> > >
> > > Sure, the ITs are using docker images. You can build all the images
> with
> > > docker profile:
> > >
> > > mvn clean install -DskipITs -DskipTests -Pdocker
> > >
> > >
> > >
> > > if you change Artemis code base you don’t need to rebuild all.
> > >
> > > You can just trigger the assembly/broker-artemis module to rebuild the
> > > message-broker docker image.
> > >
> > > mvn clean install -DskipITs -DskipTests -Pdocker -f
> > > assembly/broker-artemis/pom.xml
> > >
> > >
> > >
> > > I’m planning to do some more investigation using Artemis 2.22 without
> > > Kapua code hoping that could help you. I’ll give you feedback as soon
> as
> > I
> > > have one
> > >
> > >
> > >
> > > Riccardo
> > >
> > >
> > >
> > > *Da: *Justin Bertram <jb...@apache.org>
> > > *Data: *lunedì, 11 aprile 2022 19:10
> > > *A: *users@activemq.apache.org <us...@activemq.apache.org>
> > > *Oggetto: *Re: Artemis security plugin doesn't allow to change clientId
> > >
> > > 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
> > > <Ri...@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 <Ri...@eurotech.com.INVALID>
> > > > Data: lunedì, 11 aprile 2022 08:58
> > > > A: users@activemq.apache.org <us...@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 <jb...@apache.org>
> > > > Data: venerdì, 8 aprile 2022 21:58
> > > > A: users@activemq.apache.org <us...@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
> > > > <Ri...@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 <Ri...@eurotech.com>
> > > > > Data: venerdì, 8 aprile 2022 08:42
> > > > > A: users@activemq.apache.org <us...@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 <jb...@apache.org>
> > > > > Data: venerdì, 8 aprile 2022 03:05
> > > > > A: users@activemq.apache.org <us...@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 <jbertram@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 <
> > jbertram@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
> > > > > >> <Ri...@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
> > > > > >>>
> > > > > >>>
> > > > >
> > > >
> > >
> >
>

R: Artemis security plugin doesn't allow to change clientId

Posted by "Modanese, Riccardo" <Ri...@eurotech.com.INVALID>.
Hi Justin,
     it works for me.
I tried again with the full Kapua ServerPlugin and SecurityPlugin using the Artemis 2.23.0-SNAPSHOT(updated in the morning) + your commit.
Both the "normal stealing link" and the "multi tenant" are handled correctly (in the second case both the clients are able to stay connected).
With your fix I don't see exceptions thrown by Paho client while, for example, 10 clients are connecting at the same time with the same clientId. The client connects and soon after its status changes to disconnect "softly"
I'll do few more tests in the next days.

Regards

Riccardo



Da: Justin Bertram <jb...@apache.org>
Data: mercoledì, 4 maggio 2022 21:10
A: users@activemq.apache.org <us...@activemq.apache.org>
Oggetto: Re: Artemis security plugin doesn't allow to change clientId
I just pushed the update. Let me know if the change works for you now.
Thanks!


Justin

On Wed, May 4, 2022 at 1:58 PM Justin Bertram <jb...@apache.org> wrote:

> Thanks for that. I've reproduced the failure in a local test, and I'm
> working on an update for the PR. It should be ready for you to test soon.
>
>
> Justin
>
> On Tue, May 3, 2022 at 3:26 PM Modanese, Riccardo
> <Ri...@eurotech.com.invalid> wrote:
>
>> Hi Justin,
>>     I was able to reproduce the NPE using a very small SecurityPlugin and
>> ServerPlugin (more or less stub classes with just few lines of code).
>>
>> The code is pushed in my fork (I push forced to the branch
>> upgrade-artemis-2_21).
>>
>> I removed all the Kapua modules in the latest commit and I added few
>> modules that create an Artemis image (Artemis version is 2.23.0-SNAPSHOT +
>> your commit, I hope I did the right way) including the “dummy” Server and
>> Security plugins.
>>
>> Once built with
>> mvn clean install -DskipITs -DskipTests -Pdocker
>>
>> [INFO] Reactor Summary for kapua 2.0.0-ARTEMIS-SNAPSHOT:
>> [INFO]
>> [INFO] kapua .............................................. SUCCESS []
>> [INFO] kapua-artemis-test ................................. SUCCESS []
>> [INFO] kapua-assembly-java-base ........................... SUCCESS []
>> [INFO] kapua-broker-artemis-plugin-test ................... SUCCESS []
>> [INFO] kapua-assembly-broker-artemis-test ................. SUCCESS []
>> [INFO]
>> ------------------------------------------------------------------------
>> [INFO] BUILD SUCCESS
>> [INFO]
>> ------------------------------------------------------------------------
>>
>> you can move to
>> cd deployment/docker/unix
>>
>> and start the broker with compose (only the broker container will start)
>> ./docker-deploy.sh
>>
>> Then if you run the test TestStealingLink you can see one NPE at the
>> first run
>>
>> 2022-05-03 12:34:19,022 INFO
>> [org.eclipse.kapua.broker.artemis.plugin.security.DummySecurityPlugin]
>> Authenticate: updated clientId: newId-1-client-stealing-multi-account
>> 2022-05-03 12:34:19,348 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=71],
>> variableHeader=MqttConnectVariableHeader[name=MQTT, version=4,
>> hasUserName=true, hasPassword=true, isWillRetain=false, isWillFlag=false,
>> isCleanSession=true, keepAliveTimeSeconds=60],
>> payload=MqttConnectPayload[clientIdentifier=client-stealing-multi-account,
>> willTopic=null, willMessage=null, userName=kapua-broker, 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:552)
>> [artemis-mqtt-protocol-2.23.0-SNAPSHOT.jar:2.23.0-SNAPSHOT]
>>                 at
>> org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolHandler.handleConnect(MQTTProtocolHandler.java:229)
>> [artemis-mqtt-protocol-2.23.0-SNAPSHOT.jar:2.23.0-SNAPSHOT]
>>                 at
>> org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolHandler.act(MQTTProtocolHandler.java:154)
>> [artemis-mqtt-protocol-2.23.0-SNAPSHOT.jar:2.23.0-SNAPSHOT]
>>                 at
>> org.apache.activemq.artemis.utils.actors.Actor.doTask(Actor.java:33)
>> [artemis-commons-2.23.0-SNAPSHOT.jar:]
>>                 at
>> org.apache.activemq.artemis.utils.actors.ProcessorBase.executePendingTasks(ProcessorBase.java:65)
>> [artemis-commons-2.23.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.23.0-SNAPSHOT.jar:]
>>
>> 2022-05-03 12:34:19,356 INFO
>> [org.eclipse.kapua.broker.artemis.plugin.security.DummyServerPlugin] ###
>> cleanUpConnectionData connection: fca276c8 - reason: DESTROY - Error: N/A
>>
>> or 2 if you run again the test.
>>
>> Da: Justin Bertram <jb...@apache.org>
>> Data: martedì, 26 aprile 2022 15:42
>> A: users@activemq.apache.org <us...@activemq.apache.org>
>> Oggetto: Re: Artemis security plugin doesn't allow to change clientId
>> I assumed that since you could only reproduce the NPE with your plugins
>> added you were able to identify the root cause of the problem. I gave up
>> trying to reproduce the problem in Kapua due to a lack of clarity around
>> running the actual test. I'll go back and take another look at reproducing
>> it, but either way I'm going to merge the change as it allows modifying
>> the
>> client ID in the test that I wrote.
>>
>>
>> Justin
>>
>> On Tue, Apr 26, 2022 at 2:30 AM Modanese, Riccardo
>> <Ri...@eurotech.com.invalid> wrote:
>>
>> > Sorry but I cannot test the fix due to the NPE.
>> > And even the “normal” stealing link doesn’t work (same NPE) with our
>> > plugin code.
>> > I don’t know which is the best way to investigate further.
>> > So, unfortunately, I cannot say if this fix fits or not our needs since
>> I
>> > cannot test it.
>> >
>> > Riccardo
>> >
>> >
>> > Da: Justin Bertram <jb...@apache.org>
>> > Data: martedì, 26 aprile 2022 00:20
>> > A: users@activemq.apache.org <us...@activemq.apache.org>
>> > Oggetto: Re: Artemis security plugin doesn't allow to change clientId
>> > I've given up reproducing this NPE for now. At this point can you
>> confirm
>> > that the PR [1] fits your needs? If so, I can merge it and it can be
>> > included in the next release which should be started in the next few
>> days.
>> >
>> >
>> > Justin
>> >
>> > [1] https://github.com/apache/activemq-artemis/pull/4021
>> >
>> > On Wed, Apr 20, 2022 at 2:41 AM Modanese, Riccardo
>> > <Ri...@eurotech.com.invalid> wrote:
>> >
>> > > For the subscription I’ll start a new thread as you suggested.
>> > >
>> > > About the NPE I tried again to run the test from my Eclipse IDE (after
>> > > rebuilding all the images “mvn clean install -DskipITs -DskipTests
>> > > -Pdocker”) and the test environment seems to start correctly.
>> > > Which is your output of “docker images | grep kapua”?
>> > > Sounds like this one?
>> > >
>> > > docker images | grep kapua
>> > > kapua/kapua-job-engine
>> > >  2.0.0-ARTEMIS-SNAPSHOT   0f4656b1fbda   15 minutes ago   733MB
>> > > kapua/kapua-job-engine
>> > >  2022-04-20               0f4656b1fbda   15 minutes ago   733MB
>> > > kapua/kapua-job-engine
>> > >  latest                   0f4656b1fbda   15 minutes ago   733MB
>> > > kapua/kapua-service-authentication
>> > >  2.0.0-ARTEMIS-SNAPSHOT   866b2caedd80   15 minutes ago   650MB
>> > > kapua/kapua-service-authentication
>> > >  2022-04-20               866b2caedd80   15 minutes ago   650MB
>> > > kapua/kapua-service-authentication
>> > >  latest                   866b2caedd80   15 minutes ago   650MB
>> > > kapua/kapua-consumer-lifecycle
>> > >  2.0.0-ARTEMIS-SNAPSHOT   6f35d27645b6   15 minutes ago   656MB
>> > > kapua/kapua-consumer-lifecycle
>> > >  2022-04-20               6f35d27645b6   15 minutes ago   656MB
>> > > kapua/kapua-consumer-lifecycle
>> > >  latest                   6f35d27645b6   15 minutes ago   656MB
>> > > kapua/kapua-consumer-telemetry
>> > >  2.0.0-ARTEMIS-SNAPSHOT   e313f89c0dce   15 minutes ago   675MB
>> > > kapua/kapua-consumer-telemetry
>> > >  2022-04-20               e313f89c0dce   15 minutes ago   675MB
>> > > kapua/kapua-consumer-telemetry
>> > >  latest                   e313f89c0dce   15 minutes ago   675MB
>> > > kapua/kapua-api
>> > > 2.0.0-ARTEMIS-SNAPSHOT   c5cb9cb37941   16 minutes ago   799MB
>> > > kapua/kapua-api
>> > > 2022-04-20               c5cb9cb37941   16 minutes ago   799MB
>> > > kapua/kapua-api
>> > > latest                   c5cb9cb37941   16 minutes ago   799MB
>> > > kapua/kapua-broker-artemis
>> > >  2.0.0-ARTEMIS-SNAPSHOT   b417ae53f26a   17 minutes ago   795MB
>> > > kapua/kapua-broker-artemis
>> > >  2022-04-20               b417ae53f26a   17 minutes ago   795MB
>> > > kapua/kapua-broker-artemis
>> > >  latest                   b417ae53f26a   17 minutes ago   795MB
>> > > kapua/kapua-events-broker
>> > > 2.0.0-ARTEMIS-SNAPSHOT   c1046e5af987   27 minutes ago   130MB
>> > > kapua/kapua-events-broker
>> > > 2022-04-20               c1046e5af987   27 minutes ago   130MB
>> > > kapua/kapua-events-broker
>> > > latest                   c1046e5af987   27 minutes ago   130MB
>> > > kapua/kapua-sql
>> > > 2.0.0-ARTEMIS-SNAPSHOT   e1c6753a7cf7   28 minutes ago   587MB
>> > > kapua/kapua-sql
>> > > 2022-04-20               e1c6753a7cf7   28 minutes ago   587MB
>> > > kapua/kapua-sql
>> > > latest                   e1c6753a7cf7   28 minutes ago   587MB
>> > > kapua/jetty-base
>> > >  2.0.0-ARTEMIS-SNAPSHOT   3f2edd64f434   28 minutes ago   610MB
>> > > kapua/jetty-base
>> > >  2022-04-20               3f2edd64f434   28 minutes ago   610MB
>> > > kapua/jetty-base
>> > >  latest                   3f2edd64f434   28 minutes ago   610MB
>> > > kapua/java-base
>> > > 2.0.0-ARTEMIS-SNAPSHOT   6b19bd516e28   28 minutes ago   585MB
>> > > kapua/java-base
>> > > 2022-04-20               6b19bd516e28   28 minutes ago   585MB
>> > > kapua/java-base
>> > > latest                   6b19bd516e28   28 minutes ago   585MB
>> > >
>> > >
>> > > Da: Justin Bertram <jb...@apache.org>
>> > > Data: martedì, 19 aprile 2022 18:07
>> > > A: users@activemq.apache.org <us...@activemq.apache.org>
>> > > Oggetto: Re: Artemis security plugin doesn't allow to change clientId
>> > > Even after running `mvn clean install -DskipITs -DskipTests -Pdocker`
>> I
>> > > still get the same 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
>> > >
>> > > As for using docker-compose, I don't know where to find your
>> > > "compose.diff." Perhaps you attached it to your email, but I don't
>> > believe
>> > > the mailing list supports attachments.
>> > >
>> > > In any case, since you can't reproduce the NPE without Kapua then it
>> must
>> > > be something in the Kapua code that's causing the unexpected null. I
>> > would
>> > > like to make the code more safe so that it fails more gracefully
>> instead
>> > of
>> > > throwing an NPE, but it would still fail which means you would still
>> need
>> > > to change the Kapua code that's causing the unexpected null. However,
>> > > without a way to reproduce it I can't make any changes.
>> > >
>> > > Please start a new thread or open a Jira regarding the subscription
>> > issue.
>> > >
>> > >
>> > > Justin
>> > >
>> > > On Tue, Apr 12, 2022 at 5:35 AM Modanese, Riccardo
>> > > <Ri...@eurotech.com.invalid> wrote:
>> > >
>> > > > I wasn’t able to reproduce the NPE with Artemis 2.22 without Kapua
>> > code.
>> > > >
>> > > > Anyway, the stealing link NPE with Kapua plugins could be reproduced
>> > also
>> > > > with docker-compose (the patch remove the not needed containers)
>> > > >
>> > > > So (from root Kapua git repo path):
>> > > >
>> > > > git apply compose.diff
>> > > >
>> > > > cd deployment/docker/unix/
>> > > >
>> > > > ./docker-deploy.sh
>> > > >
>> > > >
>> > > >
>> > > > once the environment is running you can run the test class
>> > > (TestMqttClient
>> > > > requires only log4j and Paho as external dependencies)
>> > > >
>> > > >
>> > > >
>> > > > About the subscription issue I was able to reproduce it also without
>> > > Kapua
>> > > > plugins but since you prefer to focus to NPE I’ll tell you in a
>> second
>> > > time.
>> > > >
>> > > >
>> > > >
>> > > > Riccardo
>> > > >
>> > > >
>> > > >
>> > > > *Da: *Modanese, Riccardo <Ri...@eurotech.com>
>> > > > *Data: *martedì, 12 aprile 2022 09:12
>> > > > *A: *users@activemq.apache.org <us...@activemq.apache.org>
>> > > > *Oggetto: *R: Artemis security plugin doesn't allow to change
>> clientId
>> > > >
>> > > > Sure, the ITs are using docker images. You can build all the images
>> > with
>> > > > docker profile:
>> > > >
>> > > > mvn clean install -DskipITs -DskipTests -Pdocker
>> > > >
>> > > >
>> > > >
>> > > > if you change Artemis code base you don’t need to rebuild all.
>> > > >
>> > > > You can just trigger the assembly/broker-artemis module to rebuild
>> the
>> > > > message-broker docker image.
>> > > >
>> > > > mvn clean install -DskipITs -DskipTests -Pdocker -f
>> > > > assembly/broker-artemis/pom.xml
>> > > >
>> > > >
>> > > >
>> > > > I’m planning to do some more investigation using Artemis 2.22
>> without
>> > > > Kapua code hoping that could help you. I’ll give you feedback as
>> soon
>> > as
>> > > I
>> > > > have one
>> > > >
>> > > >
>> > > >
>> > > > Riccardo
>> > > >
>> > > >
>> > > >
>> > > > *Da: *Justin Bertram <jb...@apache.org>
>> > > > *Data: *lunedì, 11 aprile 2022 19:10
>> > > > *A: *users@activemq.apache.org <us...@activemq.apache.org>
>> > > > *Oggetto: *Re: Artemis security plugin doesn't allow to change
>> clientId
>> > > >
>> > > > 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
>> > > > <Ri...@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 <Ri...@eurotech.com.INVALID>
>> > > > > Data: lunedì, 11 aprile 2022 08:58
>> > > > > A: users@activemq.apache.org <us...@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 <jb...@apache.org>
>> > > > > Data: venerdì, 8 aprile 2022 21:58
>> > > > > A: users@activemq.apache.org <us...@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
>> > > > > <Ri...@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 <Ri...@eurotech.com>
>> > > > > > Data: venerdì, 8 aprile 2022 08:42
>> > > > > > A: users@activemq.apache.org <us...@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 <jb...@apache.org>
>> > > > > > Data: venerdì, 8 aprile 2022 03:05
>> > > > > > A: users@activemq.apache.org <us...@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 <
>> jbertram@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 <
>> > > jbertram@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
>> > > > > > >> <Ri...@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
>> > > > > > >>>
>> > > > > > >>>
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>

Re: Artemis security plugin doesn't allow to change clientId

Posted by Justin Bertram <jb...@apache.org>.
I just pushed the update. Let me know if the change works for you now.
Thanks!


Justin

On Wed, May 4, 2022 at 1:58 PM Justin Bertram <jb...@apache.org> wrote:

> Thanks for that. I've reproduced the failure in a local test, and I'm
> working on an update for the PR. It should be ready for you to test soon.
>
>
> Justin
>
> On Tue, May 3, 2022 at 3:26 PM Modanese, Riccardo
> <Ri...@eurotech.com.invalid> wrote:
>
>> Hi Justin,
>>     I was able to reproduce the NPE using a very small SecurityPlugin and
>> ServerPlugin (more or less stub classes with just few lines of code).
>>
>> The code is pushed in my fork (I push forced to the branch
>> upgrade-artemis-2_21).
>>
>> I removed all the Kapua modules in the latest commit and I added few
>> modules that create an Artemis image (Artemis version is 2.23.0-SNAPSHOT +
>> your commit, I hope I did the right way) including the “dummy” Server and
>> Security plugins.
>>
>> Once built with
>> mvn clean install -DskipITs -DskipTests -Pdocker
>>
>> [INFO] Reactor Summary for kapua 2.0.0-ARTEMIS-SNAPSHOT:
>> [INFO]
>> [INFO] kapua .............................................. SUCCESS []
>> [INFO] kapua-artemis-test ................................. SUCCESS []
>> [INFO] kapua-assembly-java-base ........................... SUCCESS []
>> [INFO] kapua-broker-artemis-plugin-test ................... SUCCESS []
>> [INFO] kapua-assembly-broker-artemis-test ................. SUCCESS []
>> [INFO]
>> ------------------------------------------------------------------------
>> [INFO] BUILD SUCCESS
>> [INFO]
>> ------------------------------------------------------------------------
>>
>> you can move to
>> cd deployment/docker/unix
>>
>> and start the broker with compose (only the broker container will start)
>> ./docker-deploy.sh
>>
>> Then if you run the test TestStealingLink you can see one NPE at the
>> first run
>>
>> 2022-05-03 12:34:19,022 INFO
>> [org.eclipse.kapua.broker.artemis.plugin.security.DummySecurityPlugin]
>> Authenticate: updated clientId: newId-1-client-stealing-multi-account
>> 2022-05-03 12:34:19,348 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=71],
>> variableHeader=MqttConnectVariableHeader[name=MQTT, version=4,
>> hasUserName=true, hasPassword=true, isWillRetain=false, isWillFlag=false,
>> isCleanSession=true, keepAliveTimeSeconds=60],
>> payload=MqttConnectPayload[clientIdentifier=client-stealing-multi-account,
>> willTopic=null, willMessage=null, userName=kapua-broker, 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:552)
>> [artemis-mqtt-protocol-2.23.0-SNAPSHOT.jar:2.23.0-SNAPSHOT]
>>                 at
>> org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolHandler.handleConnect(MQTTProtocolHandler.java:229)
>> [artemis-mqtt-protocol-2.23.0-SNAPSHOT.jar:2.23.0-SNAPSHOT]
>>                 at
>> org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolHandler.act(MQTTProtocolHandler.java:154)
>> [artemis-mqtt-protocol-2.23.0-SNAPSHOT.jar:2.23.0-SNAPSHOT]
>>                 at
>> org.apache.activemq.artemis.utils.actors.Actor.doTask(Actor.java:33)
>> [artemis-commons-2.23.0-SNAPSHOT.jar:]
>>                 at
>> org.apache.activemq.artemis.utils.actors.ProcessorBase.executePendingTasks(ProcessorBase.java:65)
>> [artemis-commons-2.23.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.23.0-SNAPSHOT.jar:]
>>
>> 2022-05-03 12:34:19,356 INFO
>> [org.eclipse.kapua.broker.artemis.plugin.security.DummyServerPlugin] ###
>> cleanUpConnectionData connection: fca276c8 - reason: DESTROY - Error: N/A
>>
>> or 2 if you run again the test.
>>
>> Da: Justin Bertram <jb...@apache.org>
>> Data: martedì, 26 aprile 2022 15:42
>> A: users@activemq.apache.org <us...@activemq.apache.org>
>> Oggetto: Re: Artemis security plugin doesn't allow to change clientId
>> I assumed that since you could only reproduce the NPE with your plugins
>> added you were able to identify the root cause of the problem. I gave up
>> trying to reproduce the problem in Kapua due to a lack of clarity around
>> running the actual test. I'll go back and take another look at reproducing
>> it, but either way I'm going to merge the change as it allows modifying
>> the
>> client ID in the test that I wrote.
>>
>>
>> Justin
>>
>> On Tue, Apr 26, 2022 at 2:30 AM Modanese, Riccardo
>> <Ri...@eurotech.com.invalid> wrote:
>>
>> > Sorry but I cannot test the fix due to the NPE.
>> > And even the “normal” stealing link doesn’t work (same NPE) with our
>> > plugin code.
>> > I don’t know which is the best way to investigate further.
>> > So, unfortunately, I cannot say if this fix fits or not our needs since
>> I
>> > cannot test it.
>> >
>> > Riccardo
>> >
>> >
>> > Da: Justin Bertram <jb...@apache.org>
>> > Data: martedì, 26 aprile 2022 00:20
>> > A: users@activemq.apache.org <us...@activemq.apache.org>
>> > Oggetto: Re: Artemis security plugin doesn't allow to change clientId
>> > I've given up reproducing this NPE for now. At this point can you
>> confirm
>> > that the PR [1] fits your needs? If so, I can merge it and it can be
>> > included in the next release which should be started in the next few
>> days.
>> >
>> >
>> > Justin
>> >
>> > [1] https://github.com/apache/activemq-artemis/pull/4021
>> >
>> > On Wed, Apr 20, 2022 at 2:41 AM Modanese, Riccardo
>> > <Ri...@eurotech.com.invalid> wrote:
>> >
>> > > For the subscription I’ll start a new thread as you suggested.
>> > >
>> > > About the NPE I tried again to run the test from my Eclipse IDE (after
>> > > rebuilding all the images “mvn clean install -DskipITs -DskipTests
>> > > -Pdocker”) and the test environment seems to start correctly.
>> > > Which is your output of “docker images | grep kapua”?
>> > > Sounds like this one?
>> > >
>> > > docker images | grep kapua
>> > > kapua/kapua-job-engine
>> > >  2.0.0-ARTEMIS-SNAPSHOT   0f4656b1fbda   15 minutes ago   733MB
>> > > kapua/kapua-job-engine
>> > >  2022-04-20               0f4656b1fbda   15 minutes ago   733MB
>> > > kapua/kapua-job-engine
>> > >  latest                   0f4656b1fbda   15 minutes ago   733MB
>> > > kapua/kapua-service-authentication
>> > >  2.0.0-ARTEMIS-SNAPSHOT   866b2caedd80   15 minutes ago   650MB
>> > > kapua/kapua-service-authentication
>> > >  2022-04-20               866b2caedd80   15 minutes ago   650MB
>> > > kapua/kapua-service-authentication
>> > >  latest                   866b2caedd80   15 minutes ago   650MB
>> > > kapua/kapua-consumer-lifecycle
>> > >  2.0.0-ARTEMIS-SNAPSHOT   6f35d27645b6   15 minutes ago   656MB
>> > > kapua/kapua-consumer-lifecycle
>> > >  2022-04-20               6f35d27645b6   15 minutes ago   656MB
>> > > kapua/kapua-consumer-lifecycle
>> > >  latest                   6f35d27645b6   15 minutes ago   656MB
>> > > kapua/kapua-consumer-telemetry
>> > >  2.0.0-ARTEMIS-SNAPSHOT   e313f89c0dce   15 minutes ago   675MB
>> > > kapua/kapua-consumer-telemetry
>> > >  2022-04-20               e313f89c0dce   15 minutes ago   675MB
>> > > kapua/kapua-consumer-telemetry
>> > >  latest                   e313f89c0dce   15 minutes ago   675MB
>> > > kapua/kapua-api
>> > > 2.0.0-ARTEMIS-SNAPSHOT   c5cb9cb37941   16 minutes ago   799MB
>> > > kapua/kapua-api
>> > > 2022-04-20               c5cb9cb37941   16 minutes ago   799MB
>> > > kapua/kapua-api
>> > > latest                   c5cb9cb37941   16 minutes ago   799MB
>> > > kapua/kapua-broker-artemis
>> > >  2.0.0-ARTEMIS-SNAPSHOT   b417ae53f26a   17 minutes ago   795MB
>> > > kapua/kapua-broker-artemis
>> > >  2022-04-20               b417ae53f26a   17 minutes ago   795MB
>> > > kapua/kapua-broker-artemis
>> > >  latest                   b417ae53f26a   17 minutes ago   795MB
>> > > kapua/kapua-events-broker
>> > > 2.0.0-ARTEMIS-SNAPSHOT   c1046e5af987   27 minutes ago   130MB
>> > > kapua/kapua-events-broker
>> > > 2022-04-20               c1046e5af987   27 minutes ago   130MB
>> > > kapua/kapua-events-broker
>> > > latest                   c1046e5af987   27 minutes ago   130MB
>> > > kapua/kapua-sql
>> > > 2.0.0-ARTEMIS-SNAPSHOT   e1c6753a7cf7   28 minutes ago   587MB
>> > > kapua/kapua-sql
>> > > 2022-04-20               e1c6753a7cf7   28 minutes ago   587MB
>> > > kapua/kapua-sql
>> > > latest                   e1c6753a7cf7   28 minutes ago   587MB
>> > > kapua/jetty-base
>> > >  2.0.0-ARTEMIS-SNAPSHOT   3f2edd64f434   28 minutes ago   610MB
>> > > kapua/jetty-base
>> > >  2022-04-20               3f2edd64f434   28 minutes ago   610MB
>> > > kapua/jetty-base
>> > >  latest                   3f2edd64f434   28 minutes ago   610MB
>> > > kapua/java-base
>> > > 2.0.0-ARTEMIS-SNAPSHOT   6b19bd516e28   28 minutes ago   585MB
>> > > kapua/java-base
>> > > 2022-04-20               6b19bd516e28   28 minutes ago   585MB
>> > > kapua/java-base
>> > > latest                   6b19bd516e28   28 minutes ago   585MB
>> > >
>> > >
>> > > Da: Justin Bertram <jb...@apache.org>
>> > > Data: martedì, 19 aprile 2022 18:07
>> > > A: users@activemq.apache.org <us...@activemq.apache.org>
>> > > Oggetto: Re: Artemis security plugin doesn't allow to change clientId
>> > > Even after running `mvn clean install -DskipITs -DskipTests -Pdocker`
>> I
>> > > still get the same 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
>> > >
>> > > As for using docker-compose, I don't know where to find your
>> > > "compose.diff." Perhaps you attached it to your email, but I don't
>> > believe
>> > > the mailing list supports attachments.
>> > >
>> > > In any case, since you can't reproduce the NPE without Kapua then it
>> must
>> > > be something in the Kapua code that's causing the unexpected null. I
>> > would
>> > > like to make the code more safe so that it fails more gracefully
>> instead
>> > of
>> > > throwing an NPE, but it would still fail which means you would still
>> need
>> > > to change the Kapua code that's causing the unexpected null. However,
>> > > without a way to reproduce it I can't make any changes.
>> > >
>> > > Please start a new thread or open a Jira regarding the subscription
>> > issue.
>> > >
>> > >
>> > > Justin
>> > >
>> > > On Tue, Apr 12, 2022 at 5:35 AM Modanese, Riccardo
>> > > <Ri...@eurotech.com.invalid> wrote:
>> > >
>> > > > I wasn’t able to reproduce the NPE with Artemis 2.22 without Kapua
>> > code.
>> > > >
>> > > > Anyway, the stealing link NPE with Kapua plugins could be reproduced
>> > also
>> > > > with docker-compose (the patch remove the not needed containers)
>> > > >
>> > > > So (from root Kapua git repo path):
>> > > >
>> > > > git apply compose.diff
>> > > >
>> > > > cd deployment/docker/unix/
>> > > >
>> > > > ./docker-deploy.sh
>> > > >
>> > > >
>> > > >
>> > > > once the environment is running you can run the test class
>> > > (TestMqttClient
>> > > > requires only log4j and Paho as external dependencies)
>> > > >
>> > > >
>> > > >
>> > > > About the subscription issue I was able to reproduce it also without
>> > > Kapua
>> > > > plugins but since you prefer to focus to NPE I’ll tell you in a
>> second
>> > > time.
>> > > >
>> > > >
>> > > >
>> > > > Riccardo
>> > > >
>> > > >
>> > > >
>> > > > *Da: *Modanese, Riccardo <Ri...@eurotech.com>
>> > > > *Data: *martedì, 12 aprile 2022 09:12
>> > > > *A: *users@activemq.apache.org <us...@activemq.apache.org>
>> > > > *Oggetto: *R: Artemis security plugin doesn't allow to change
>> clientId
>> > > >
>> > > > Sure, the ITs are using docker images. You can build all the images
>> > with
>> > > > docker profile:
>> > > >
>> > > > mvn clean install -DskipITs -DskipTests -Pdocker
>> > > >
>> > > >
>> > > >
>> > > > if you change Artemis code base you don’t need to rebuild all.
>> > > >
>> > > > You can just trigger the assembly/broker-artemis module to rebuild
>> the
>> > > > message-broker docker image.
>> > > >
>> > > > mvn clean install -DskipITs -DskipTests -Pdocker -f
>> > > > assembly/broker-artemis/pom.xml
>> > > >
>> > > >
>> > > >
>> > > > I’m planning to do some more investigation using Artemis 2.22
>> without
>> > > > Kapua code hoping that could help you. I’ll give you feedback as
>> soon
>> > as
>> > > I
>> > > > have one
>> > > >
>> > > >
>> > > >
>> > > > Riccardo
>> > > >
>> > > >
>> > > >
>> > > > *Da: *Justin Bertram <jb...@apache.org>
>> > > > *Data: *lunedì, 11 aprile 2022 19:10
>> > > > *A: *users@activemq.apache.org <us...@activemq.apache.org>
>> > > > *Oggetto: *Re: Artemis security plugin doesn't allow to change
>> clientId
>> > > >
>> > > > 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
>> > > > <Ri...@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 <Ri...@eurotech.com.INVALID>
>> > > > > Data: lunedì, 11 aprile 2022 08:58
>> > > > > A: users@activemq.apache.org <us...@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 <jb...@apache.org>
>> > > > > Data: venerdì, 8 aprile 2022 21:58
>> > > > > A: users@activemq.apache.org <us...@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
>> > > > > <Ri...@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 <Ri...@eurotech.com>
>> > > > > > Data: venerdì, 8 aprile 2022 08:42
>> > > > > > A: users@activemq.apache.org <us...@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 <jb...@apache.org>
>> > > > > > Data: venerdì, 8 aprile 2022 03:05
>> > > > > > A: users@activemq.apache.org <us...@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 <
>> jbertram@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 <
>> > > jbertram@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
>> > > > > > >> <Ri...@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
>> > > > > > >>>
>> > > > > > >>>
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>

Re: Artemis security plugin doesn't allow to change clientId

Posted by Justin Bertram <jb...@apache.org>.
Thanks for that. I've reproduced the failure in a local test, and I'm
working on an update for the PR. It should be ready for you to test soon.


Justin

On Tue, May 3, 2022 at 3:26 PM Modanese, Riccardo
<Ri...@eurotech.com.invalid> wrote:

> Hi Justin,
>     I was able to reproduce the NPE using a very small SecurityPlugin and
> ServerPlugin (more or less stub classes with just few lines of code).
>
> The code is pushed in my fork (I push forced to the branch
> upgrade-artemis-2_21).
>
> I removed all the Kapua modules in the latest commit and I added few
> modules that create an Artemis image (Artemis version is 2.23.0-SNAPSHOT +
> your commit, I hope I did the right way) including the “dummy” Server and
> Security plugins.
>
> Once built with
> mvn clean install -DskipITs -DskipTests -Pdocker
>
> [INFO] Reactor Summary for kapua 2.0.0-ARTEMIS-SNAPSHOT:
> [INFO]
> [INFO] kapua .............................................. SUCCESS []
> [INFO] kapua-artemis-test ................................. SUCCESS []
> [INFO] kapua-assembly-java-base ........................... SUCCESS []
> [INFO] kapua-broker-artemis-plugin-test ................... SUCCESS []
> [INFO] kapua-assembly-broker-artemis-test ................. SUCCESS []
> [INFO]
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO]
> ------------------------------------------------------------------------
>
> you can move to
> cd deployment/docker/unix
>
> and start the broker with compose (only the broker container will start)
> ./docker-deploy.sh
>
> Then if you run the test TestStealingLink you can see one NPE at the first
> run
>
> 2022-05-03 12:34:19,022 INFO
> [org.eclipse.kapua.broker.artemis.plugin.security.DummySecurityPlugin]
> Authenticate: updated clientId: newId-1-client-stealing-multi-account
> 2022-05-03 12:34:19,348 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=71],
> variableHeader=MqttConnectVariableHeader[name=MQTT, version=4,
> hasUserName=true, hasPassword=true, isWillRetain=false, isWillFlag=false,
> isCleanSession=true, keepAliveTimeSeconds=60],
> payload=MqttConnectPayload[clientIdentifier=client-stealing-multi-account,
> willTopic=null, willMessage=null, userName=kapua-broker, 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:552)
> [artemis-mqtt-protocol-2.23.0-SNAPSHOT.jar:2.23.0-SNAPSHOT]
>                 at
> org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolHandler.handleConnect(MQTTProtocolHandler.java:229)
> [artemis-mqtt-protocol-2.23.0-SNAPSHOT.jar:2.23.0-SNAPSHOT]
>                 at
> org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolHandler.act(MQTTProtocolHandler.java:154)
> [artemis-mqtt-protocol-2.23.0-SNAPSHOT.jar:2.23.0-SNAPSHOT]
>                 at
> org.apache.activemq.artemis.utils.actors.Actor.doTask(Actor.java:33)
> [artemis-commons-2.23.0-SNAPSHOT.jar:]
>                 at
> org.apache.activemq.artemis.utils.actors.ProcessorBase.executePendingTasks(ProcessorBase.java:65)
> [artemis-commons-2.23.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.23.0-SNAPSHOT.jar:]
>
> 2022-05-03 12:34:19,356 INFO
> [org.eclipse.kapua.broker.artemis.plugin.security.DummyServerPlugin] ###
> cleanUpConnectionData connection: fca276c8 - reason: DESTROY - Error: N/A
>
> or 2 if you run again the test.
>
> Da: Justin Bertram <jb...@apache.org>
> Data: martedì, 26 aprile 2022 15:42
> A: users@activemq.apache.org <us...@activemq.apache.org>
> Oggetto: Re: Artemis security plugin doesn't allow to change clientId
> I assumed that since you could only reproduce the NPE with your plugins
> added you were able to identify the root cause of the problem. I gave up
> trying to reproduce the problem in Kapua due to a lack of clarity around
> running the actual test. I'll go back and take another look at reproducing
> it, but either way I'm going to merge the change as it allows modifying the
> client ID in the test that I wrote.
>
>
> Justin
>
> On Tue, Apr 26, 2022 at 2:30 AM Modanese, Riccardo
> <Ri...@eurotech.com.invalid> wrote:
>
> > Sorry but I cannot test the fix due to the NPE.
> > And even the “normal” stealing link doesn’t work (same NPE) with our
> > plugin code.
> > I don’t know which is the best way to investigate further.
> > So, unfortunately, I cannot say if this fix fits or not our needs since I
> > cannot test it.
> >
> > Riccardo
> >
> >
> > Da: Justin Bertram <jb...@apache.org>
> > Data: martedì, 26 aprile 2022 00:20
> > A: users@activemq.apache.org <us...@activemq.apache.org>
> > Oggetto: Re: Artemis security plugin doesn't allow to change clientId
> > I've given up reproducing this NPE for now. At this point can you confirm
> > that the PR [1] fits your needs? If so, I can merge it and it can be
> > included in the next release which should be started in the next few
> days.
> >
> >
> > Justin
> >
> > [1] https://github.com/apache/activemq-artemis/pull/4021
> >
> > On Wed, Apr 20, 2022 at 2:41 AM Modanese, Riccardo
> > <Ri...@eurotech.com.invalid> wrote:
> >
> > > For the subscription I’ll start a new thread as you suggested.
> > >
> > > About the NPE I tried again to run the test from my Eclipse IDE (after
> > > rebuilding all the images “mvn clean install -DskipITs -DskipTests
> > > -Pdocker”) and the test environment seems to start correctly.
> > > Which is your output of “docker images | grep kapua”?
> > > Sounds like this one?
> > >
> > > docker images | grep kapua
> > > kapua/kapua-job-engine
> > >  2.0.0-ARTEMIS-SNAPSHOT   0f4656b1fbda   15 minutes ago   733MB
> > > kapua/kapua-job-engine
> > >  2022-04-20               0f4656b1fbda   15 minutes ago   733MB
> > > kapua/kapua-job-engine
> > >  latest                   0f4656b1fbda   15 minutes ago   733MB
> > > kapua/kapua-service-authentication
> > >  2.0.0-ARTEMIS-SNAPSHOT   866b2caedd80   15 minutes ago   650MB
> > > kapua/kapua-service-authentication
> > >  2022-04-20               866b2caedd80   15 minutes ago   650MB
> > > kapua/kapua-service-authentication
> > >  latest                   866b2caedd80   15 minutes ago   650MB
> > > kapua/kapua-consumer-lifecycle
> > >  2.0.0-ARTEMIS-SNAPSHOT   6f35d27645b6   15 minutes ago   656MB
> > > kapua/kapua-consumer-lifecycle
> > >  2022-04-20               6f35d27645b6   15 minutes ago   656MB
> > > kapua/kapua-consumer-lifecycle
> > >  latest                   6f35d27645b6   15 minutes ago   656MB
> > > kapua/kapua-consumer-telemetry
> > >  2.0.0-ARTEMIS-SNAPSHOT   e313f89c0dce   15 minutes ago   675MB
> > > kapua/kapua-consumer-telemetry
> > >  2022-04-20               e313f89c0dce   15 minutes ago   675MB
> > > kapua/kapua-consumer-telemetry
> > >  latest                   e313f89c0dce   15 minutes ago   675MB
> > > kapua/kapua-api
> > > 2.0.0-ARTEMIS-SNAPSHOT   c5cb9cb37941   16 minutes ago   799MB
> > > kapua/kapua-api
> > > 2022-04-20               c5cb9cb37941   16 minutes ago   799MB
> > > kapua/kapua-api
> > > latest                   c5cb9cb37941   16 minutes ago   799MB
> > > kapua/kapua-broker-artemis
> > >  2.0.0-ARTEMIS-SNAPSHOT   b417ae53f26a   17 minutes ago   795MB
> > > kapua/kapua-broker-artemis
> > >  2022-04-20               b417ae53f26a   17 minutes ago   795MB
> > > kapua/kapua-broker-artemis
> > >  latest                   b417ae53f26a   17 minutes ago   795MB
> > > kapua/kapua-events-broker
> > > 2.0.0-ARTEMIS-SNAPSHOT   c1046e5af987   27 minutes ago   130MB
> > > kapua/kapua-events-broker
> > > 2022-04-20               c1046e5af987   27 minutes ago   130MB
> > > kapua/kapua-events-broker
> > > latest                   c1046e5af987   27 minutes ago   130MB
> > > kapua/kapua-sql
> > > 2.0.0-ARTEMIS-SNAPSHOT   e1c6753a7cf7   28 minutes ago   587MB
> > > kapua/kapua-sql
> > > 2022-04-20               e1c6753a7cf7   28 minutes ago   587MB
> > > kapua/kapua-sql
> > > latest                   e1c6753a7cf7   28 minutes ago   587MB
> > > kapua/jetty-base
> > >  2.0.0-ARTEMIS-SNAPSHOT   3f2edd64f434   28 minutes ago   610MB
> > > kapua/jetty-base
> > >  2022-04-20               3f2edd64f434   28 minutes ago   610MB
> > > kapua/jetty-base
> > >  latest                   3f2edd64f434   28 minutes ago   610MB
> > > kapua/java-base
> > > 2.0.0-ARTEMIS-SNAPSHOT   6b19bd516e28   28 minutes ago   585MB
> > > kapua/java-base
> > > 2022-04-20               6b19bd516e28   28 minutes ago   585MB
> > > kapua/java-base
> > > latest                   6b19bd516e28   28 minutes ago   585MB
> > >
> > >
> > > Da: Justin Bertram <jb...@apache.org>
> > > Data: martedì, 19 aprile 2022 18:07
> > > A: users@activemq.apache.org <us...@activemq.apache.org>
> > > Oggetto: Re: Artemis security plugin doesn't allow to change clientId
> > > Even after running `mvn clean install -DskipITs -DskipTests -Pdocker` I
> > > still get the same 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
> > >
> > > As for using docker-compose, I don't know where to find your
> > > "compose.diff." Perhaps you attached it to your email, but I don't
> > believe
> > > the mailing list supports attachments.
> > >
> > > In any case, since you can't reproduce the NPE without Kapua then it
> must
> > > be something in the Kapua code that's causing the unexpected null. I
> > would
> > > like to make the code more safe so that it fails more gracefully
> instead
> > of
> > > throwing an NPE, but it would still fail which means you would still
> need
> > > to change the Kapua code that's causing the unexpected null. However,
> > > without a way to reproduce it I can't make any changes.
> > >
> > > Please start a new thread or open a Jira regarding the subscription
> > issue.
> > >
> > >
> > > Justin
> > >
> > > On Tue, Apr 12, 2022 at 5:35 AM Modanese, Riccardo
> > > <Ri...@eurotech.com.invalid> wrote:
> > >
> > > > I wasn’t able to reproduce the NPE with Artemis 2.22 without Kapua
> > code.
> > > >
> > > > Anyway, the stealing link NPE with Kapua plugins could be reproduced
> > also
> > > > with docker-compose (the patch remove the not needed containers)
> > > >
> > > > So (from root Kapua git repo path):
> > > >
> > > > git apply compose.diff
> > > >
> > > > cd deployment/docker/unix/
> > > >
> > > > ./docker-deploy.sh
> > > >
> > > >
> > > >
> > > > once the environment is running you can run the test class
> > > (TestMqttClient
> > > > requires only log4j and Paho as external dependencies)
> > > >
> > > >
> > > >
> > > > About the subscription issue I was able to reproduce it also without
> > > Kapua
> > > > plugins but since you prefer to focus to NPE I’ll tell you in a
> second
> > > time.
> > > >
> > > >
> > > >
> > > > Riccardo
> > > >
> > > >
> > > >
> > > > *Da: *Modanese, Riccardo <Ri...@eurotech.com>
> > > > *Data: *martedì, 12 aprile 2022 09:12
> > > > *A: *users@activemq.apache.org <us...@activemq.apache.org>
> > > > *Oggetto: *R: Artemis security plugin doesn't allow to change
> clientId
> > > >
> > > > Sure, the ITs are using docker images. You can build all the images
> > with
> > > > docker profile:
> > > >
> > > > mvn clean install -DskipITs -DskipTests -Pdocker
> > > >
> > > >
> > > >
> > > > if you change Artemis code base you don’t need to rebuild all.
> > > >
> > > > You can just trigger the assembly/broker-artemis module to rebuild
> the
> > > > message-broker docker image.
> > > >
> > > > mvn clean install -DskipITs -DskipTests -Pdocker -f
> > > > assembly/broker-artemis/pom.xml
> > > >
> > > >
> > > >
> > > > I’m planning to do some more investigation using Artemis 2.22 without
> > > > Kapua code hoping that could help you. I’ll give you feedback as soon
> > as
> > > I
> > > > have one
> > > >
> > > >
> > > >
> > > > Riccardo
> > > >
> > > >
> > > >
> > > > *Da: *Justin Bertram <jb...@apache.org>
> > > > *Data: *lunedì, 11 aprile 2022 19:10
> > > > *A: *users@activemq.apache.org <us...@activemq.apache.org>
> > > > *Oggetto: *Re: Artemis security plugin doesn't allow to change
> clientId
> > > >
> > > > 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
> > > > <Ri...@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 <Ri...@eurotech.com.INVALID>
> > > > > Data: lunedì, 11 aprile 2022 08:58
> > > > > A: users@activemq.apache.org <us...@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 <jb...@apache.org>
> > > > > Data: venerdì, 8 aprile 2022 21:58
> > > > > A: users@activemq.apache.org <us...@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
> > > > > <Ri...@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 <Ri...@eurotech.com>
> > > > > > Data: venerdì, 8 aprile 2022 08:42
> > > > > > A: users@activemq.apache.org <us...@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 <jb...@apache.org>
> > > > > > Data: venerdì, 8 aprile 2022 03:05
> > > > > > A: users@activemq.apache.org <us...@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 <
> jbertram@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 <
> > > jbertram@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
> > > > > > >> <Ri...@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
> > > > > > >>>
> > > > > > >>>
> > > > > >
> > > > >
> > > >
> > >
> >
>