From 761436c967a61aa32cdfe908535200eb688d4bae Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Thu, 5 May 2022 10:50:53 +0800 Subject: [PATCH 1/2] fix: don't allow empty username if username_as_clientid is true --- src/emqx_channel.erl | 22 ++++++++++++++-------- test/emqx_client_SUITE.erl | 12 +++++++++--- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/emqx_channel.erl b/src/emqx_channel.erl index 92559e58e..530b04250 100644 --- a/src/emqx_channel.erl +++ b/src/emqx_channel.erl @@ -1216,13 +1216,18 @@ check_connect(ConnPkt, #channel{clientinfo = #{zone := Zone}}) -> %% Enrich Client Info enrich_client(ConnPkt, Channel = #channel{clientinfo = ClientInfo}) -> - {ok, NConnPkt, NClientInfo} = pipeline([fun set_username/2, - fun set_bridge_mode/2, - fun maybe_username_as_clientid/2, - fun maybe_assign_clientid/2, - fun fix_mountpoint/2 - ], ConnPkt, ClientInfo), - {ok, NConnPkt, Channel#channel{clientinfo = NClientInfo}}. + Pipe = pipeline([fun set_username/2, + fun set_bridge_mode/2, + fun maybe_username_as_clientid/2, + fun maybe_assign_clientid/2, + fun fix_mountpoint/2 + ], ConnPkt, ClientInfo), + case Pipe of + {ok, NConnPkt, NClientInfo} -> + {ok, NConnPkt, Channel#channel{clientinfo = NClientInfo}}; + {error, ReasonCode, NClientInfo} -> + {error, ReasonCode, Channel#channel{clientinfo = NClientInfo}} + end. set_username(#mqtt_packet_connect{username = Username}, ClientInfo = #{username := undefined}) -> @@ -1237,7 +1242,8 @@ maybe_username_as_clientid(_ConnPkt, ClientInfo = #{username := undefined}) -> {ok, ClientInfo}; maybe_username_as_clientid(_ConnPkt, ClientInfo = #{zone := Zone, username := Username}) -> case emqx_zone:use_username_as_clientid(Zone) of - true -> {ok, ClientInfo#{clientid => Username}}; + true when Username =/= <<>> -> {ok, ClientInfo#{clientid => Username}}; + true -> {error, ?RC_CLIENT_IDENTIFIER_NOT_VALID, ClientInfo}; false -> ok end. diff --git a/test/emqx_client_SUITE.erl b/test/emqx_client_SUITE.erl index 4d69d11b3..d45e7a8cf 100644 --- a/test/emqx_client_SUITE.erl +++ b/test/emqx_client_SUITE.erl @@ -278,9 +278,15 @@ t_username_as_clientid(_) -> {ok, C} = emqtt:start_link([{username, Username}]), {ok, _} = emqtt:connect(C), #{clientinfo := #{clientid := Username}} = emqx_cm:get_chan_info(Username), - emqtt:disconnect(C). - - + emqtt:disconnect(C), + erlang:process_flag(trap_exit, true), + {ok, C1} = emqtt:start_link([{username, <<>>}]), + ?assertEqual({error, {client_identifier_not_valid, undefined}}, emqtt:connect(C1)), + receive + {'EXIT', _, {shutdown, client_identifier_not_valid}} -> ok + after 100 -> + throw({error, "expect_client_identifier_not_valid"}) + end. t_certcn_as_clientid_default_config_tls(_) -> tls_certcn_as_clientid(default). From 554548dbfb88be9bfb8206a64c7f52252877bcfc Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Thu, 5 May 2022 16:52:33 +0800 Subject: [PATCH 2/2] chore: update emqx.appup.src and changelog --- CHANGES-4.3.md | 1 + etc/emqx_cloud/vm.args | 2 +- etc/emqx_edge/vm.args | 2 +- src/emqx.appup.src | 4 ++++ 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGES-4.3.md b/CHANGES-4.3.md index 960d14c10..7b427ec54 100644 --- a/CHANGES-4.3.md +++ b/CHANGES-4.3.md @@ -18,6 +18,7 @@ File format: * Add support for JWT authorization [#7596] Now MQTT clients may be authorized with respect to a specific claim containing publish/subscribe topic whitelists. * Better randomisation of app screts (changed from timestamp seeded sha hash (uuid) to crypto:strong_rand_bytes) +* Return a client_identifier_not_valid error when username is empty and username_as_clientid is set to true [#7862] ### Bug fixes * List subscription topic (/api/v4/subscriptions), the result do not match with multiple conditions. diff --git a/etc/emqx_cloud/vm.args b/etc/emqx_cloud/vm.args index a199091c7..e0f4c70d7 100644 --- a/etc/emqx_cloud/vm.args +++ b/etc/emqx_cloud/vm.args @@ -115,4 +115,4 @@ -shutdown_time 30000 ## patches dir --pa {{ platform_data_dir }}/patches +-pa "{{ platform_data_dir }}/patches" diff --git a/etc/emqx_edge/vm.args b/etc/emqx_edge/vm.args index 2f61faa2d..a87449b79 100644 --- a/etc/emqx_edge/vm.args +++ b/etc/emqx_edge/vm.args @@ -113,4 +113,4 @@ -shutdown_time 10000 ## patches dir --pa {{ platform_data_dir }}/patches +-pa "{{ platform_data_dir }}/patches" diff --git a/src/emqx.appup.src b/src/emqx.appup.src index 7e9ec6d12..be7c60261 100644 --- a/src/emqx.appup.src +++ b/src/emqx.appup.src @@ -3,10 +3,12 @@ {VSN, [{"4.3.15", [{load_module,emqx_frame,brutal_purge,soft_purge,[]}, + {load_module,emqx_channel,brutal_purge,soft_purge,[]}, {load_module,emqx_access_rule,brutal_purge,soft_purge,[]}, {load_module,emqx_app,brutal_purge,soft_purge,[]}]}, {"4.3.14", [{load_module,emqx_access_rule,brutal_purge,soft_purge,[]}, + {load_module,emqx_channel,brutal_purge,soft_purge,[]}, {load_module,emqx,brutal_purge,soft_purge,[]}, {load_module,emqx_sys,brutal_purge,soft_purge,[]}, {load_module,emqx_plugins,brutal_purge,soft_purge,[]}, @@ -458,11 +460,13 @@ {<<".*">>,[]}], [{"4.3.15", [{load_module,emqx_frame,brutal_purge,soft_purge,[]}, + {load_module,emqx_channel,brutal_purge,soft_purge,[]}, {load_module,emqx_access_rule,brutal_purge,soft_purge,[]}, {load_module,emqx_app,brutal_purge,soft_purge,[]}]}, {"4.3.14", [{load_module,emqx_access_rule,brutal_purge,soft_purge,[]}, {load_module,emqx,brutal_purge,soft_purge,[]}, + {load_module,emqx_channel,brutal_purge,soft_purge,[]}, {load_module,emqx_sys,brutal_purge,soft_purge,[]}, {load_module,emqx_plugins,brutal_purge,soft_purge,[]}, {load_module,emqx_shared_sub,brutal_purge,soft_purge,[]},