From b48037a1e683f4e926c0a6a797faf2dcfcd2df3e Mon Sep 17 00:00:00 2001 From: Justin Bertram Date: Wed, 6 Apr 2022 11:38:29 -0500 Subject: [PATCH] ARTEMIS-3764 wrong CONNACK return code for MQTT5 This bug is causing tests in o.a.a.a.t.i.m.s.c.ConnectTestsWithSecurity to fail. This commit fixes the problem by setting the session's version earlier in the logic handling the CONNECT packet so that the proper CONNACK return code can be supplied to the remote client in case of authentication failure. --- .../core/protocol/mqtt/MQTTConnectionManager.java | 1 - .../core/protocol/mqtt/MQTTProtocolHandler.java | 11 +++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnectionManager.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnectionManager.java index d56e18f664..8a36d83c2d 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnectionManager.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnectionManager.java @@ -58,7 +58,6 @@ public class MQTTConnectionManager { } void connect(MqttConnectMessage connect, String validatedUser) throws Exception { - session.setVersion(MQTTVersion.getVersion(connect.variableHeader().version())); if (session.getVersion() == MQTTVersion.MQTT_5) { session.getConnection().setProtocolVersion(Byte.toString(MqttVersion.MQTT_5.protocolLevel())); String authenticationMethod = MQTTUtil.getProperty(String.class, connect.variableHeader().properties(), AUTHENTICATION_METHOD); diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolHandler.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolHandler.java index 488d8bd93e..7624df52c9 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolHandler.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolHandler.java @@ -39,7 +39,6 @@ import io.netty.handler.codec.mqtt.MqttSubscribeMessage; import io.netty.handler.codec.mqtt.MqttUnsubAckMessage; import io.netty.handler.codec.mqtt.MqttUnsubAckPayload; import io.netty.handler.codec.mqtt.MqttUnsubscribeMessage; -import io.netty.handler.codec.mqtt.MqttVersion; import io.netty.util.CharsetUtil; import io.netty.util.ReferenceCountUtil; import org.apache.activemq.artemis.api.core.ActiveMQSecurityException; @@ -223,6 +222,7 @@ public class MQTTProtocolHandler extends ChannelInboundHandlerAdapter { } void handleConnect(MqttConnectMessage connect) throws Exception { + session.setVersion(MQTTVersion.getVersion(connect.variableHeader().version())); /* * Perform authentication *before* attempting redirection because redirection may be based on the user's role. */ @@ -243,12 +243,11 @@ public class MQTTProtocolHandler extends ChannelInboundHandlerAdapter { if (connection.getTransportConnection().getRouter() == null || !protocolManager.getRoutingHandler().route(connection, session, connect)) { /* [MQTT-3.1.2-2] Reject unsupported clients. */ - int packetVersion = connect.variableHeader().version(); - if (packetVersion != MqttVersion.MQTT_3_1.protocolLevel() && - packetVersion != MqttVersion.MQTT_3_1_1.protocolLevel() && - packetVersion != MqttVersion.MQTT_5.protocolLevel()) { + if (session.getVersion() != MQTTVersion.MQTT_3_1 && + session.getVersion() != MQTTVersion.MQTT_3_1_1 && + session.getVersion() != MQTTVersion.MQTT_5) { - if (packetVersion <= MqttVersion.MQTT_3_1_1.protocolLevel()) { + if (session.getVersion().getVersion() <= MQTTVersion.MQTT_3_1_1.getVersion()) { // See MQTT-3.1.2-2 at http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718030 sendConnack(MQTTReasonCodes.UNACCEPTABLE_PROTOCOL_VERSION_3); } else {