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
> >>>
> >>>
>
