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

Reply via email to