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