From 02a9885aa5bac909a4253138422276f8be6790e1 Mon Sep 17 00:00:00 2001
From: JimMoen <LnJimMoen@outlook.com>
Date: Tue, 25 Jun 2024 23:13:15 +0800
Subject: [PATCH] fix(mqtt): check password flag to respect protocol spec

---
 apps/emqx/src/emqx_frame.erl | 26 +++++++++++++++++++++++---
 changes/ce/fix-13334.en.md   |  4 ++++
 2 files changed, 27 insertions(+), 3 deletions(-)
 create mode 100644 changes/ce/fix-13334.en.md

diff --git a/apps/emqx/src/emqx_frame.erl b/apps/emqx/src/emqx_frame.erl
index 873311ed4..a1c9084dd 100644
--- a/apps/emqx/src/emqx_frame.erl
+++ b/apps/emqx/src/emqx_frame.erl
@@ -286,7 +286,7 @@ parse_connect(FrameBin, StrictMode) ->
 
 parse_connect2(
     ProtoName,
-    <<BridgeTag:4, ProtoVer:4, UsernameFlag:1, PasswordFlag:1, WillRetainB:1, WillQoS:2,
+    <<BridgeTag:4, ProtoVer:4, UsernameFlagB:1, PasswordFlagB:1, WillRetainB:1, WillQoS:2,
         WillFlagB:1, CleanStart:1, Reserved:1, KeepAlive:16/big, Rest2/binary>>,
     StrictMode
 ) ->
@@ -296,6 +296,12 @@ parse_connect2(
         WillRetain = bool(WillRetainB),
         WillQoS
     ),
+    _ = validate_connect_password_flag(
+        StrictMode,
+        ProtoVer,
+        UsernameFlag = bool(UsernameFlagB),
+        PasswordFlag = bool(PasswordFlagB)
+    ),
     {Properties, Rest3} = parse_properties(Rest2, ProtoVer, StrictMode),
     {ClientId, Rest4} = parse_utf8_string_with_cause(Rest3, StrictMode, invalid_clientid),
     ConnPacket = #mqtt_packet_connect{
@@ -318,14 +324,14 @@ parse_connect2(
         fun(Bin) ->
             parse_utf8_string_with_cause(Bin, StrictMode, invalid_username)
         end,
-        bool(UsernameFlag)
+        UsernameFlag
     ),
     {Password, Rest7} = parse_optional(
         Rest6,
         fun(Bin) ->
             parse_utf8_string_with_cause(Bin, StrictMode, invalid_password)
         end,
-        bool(PasswordFlag)
+        PasswordFlag
     ),
     case Rest7 of
         <<>> ->
@@ -1135,6 +1141,20 @@ validate_connect_will(true, _, WillQoS) when WillQoS > 2 -> ?PARSE_ERR(invalid_w
 validate_connect_will(false, WillRetain, _) when WillRetain -> ?PARSE_ERR(invalid_will_retain);
 validate_connect_will(_, _, _) -> ok.
 
+%% MQTT-v3.1
+%% Username flag and password flag are not strongly related
+%% https://public.dhe.ibm.com/software/dw/webservices/ws-mqtt/mqtt-v3r1.html#connect
+validate_connect_password_flag(true, ?MQTT_PROTO_V3, _, _) ->
+    ok;
+%% MQTT-v3.1.1-[MQTT-3.1.2-22]
+validate_connect_password_flag(true, ?MQTT_PROTO_V4, UsernameFlag, PasswordFlag) ->
+    %% BUG-FOR-BUG compatible, only check when `strict-mode`
+    UsernameFlag orelse PasswordFlag andalso ?PARSE_ERR(invalid_password_flag);
+validate_connect_password_flag(true, ?MQTT_PROTO_V5, _, _) ->
+    ok;
+validate_connect_password_flag(_, _, _, _) ->
+    ok.
+
 bool(0) -> false;
 bool(1) -> true.
 
diff --git a/changes/ce/fix-13334.en.md b/changes/ce/fix-13334.en.md
new file mode 100644
index 000000000..e638be9ce
--- /dev/null
+++ b/changes/ce/fix-13334.en.md
@@ -0,0 +1,4 @@
+Check the `PasswordFlag` of the MQTT v3.1.1 CONNECT packet in strict mode to comply with the protocol.
+
+> [!NOTE]
+> To ensure BUG-TO-BUG compatibility, this check is performed only in strict mode.