From 3319a8d28ef578e8825e6430b07686671d02721f Mon Sep 17 00:00:00 2001 From: Erik Timan Date: Tue, 3 Jan 2023 14:32:40 +0100 Subject: [PATCH 1/3] fix(mgmt_api): remove possibility to set clientid in /publish API To avoid security confusion, we remove the possibility to specify the client ID in the request body for /publish and /publish/bulk. --- apps/emqx_management/src/emqx_mgmt_api_publish.erl | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/apps/emqx_management/src/emqx_mgmt_api_publish.erl b/apps/emqx_management/src/emqx_mgmt_api_publish.erl index 672de661b..b7c48a26a 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_publish.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_publish.erl @@ -102,12 +102,6 @@ fields(message) -> required => false, default => 0 })}, - {clientid, - hoconsc:mk(binary(), #{ - desc => ?DESC(clientid), - required => false, - example => <<"api_example_client">> - })}, {payload, hoconsc:mk(binary(), #{ desc => ?DESC(payload), @@ -254,7 +248,6 @@ is_ok_deliver({_NodeOrShare, _MatchedTopic, {error, _}}) -> false. %% %%%%%% Below error codes are not implemented so far %%%% %% %% If HTTP request passes HTTP authentication, it is considered trusted. -%% In the future, we may choose to check ACL for the provided MQTT Client ID %% 135 Not authorized 401 %% %% %%%%%% Below error codes are not applicable %%%%%%% @@ -326,7 +319,6 @@ make_message(Map) -> Encoding = maps:get(<<"payload_encoding">>, Map, plain), case decode_payload(Encoding, maps:get(<<"payload">>, Map)) of {ok, Payload} -> - From = maps:get(<<"clientid">>, Map, http_api), QoS = maps:get(<<"qos">>, Map, 0), Topic = maps:get(<<"topic">>, Map), Retain = maps:get(<<"retain">>, Map, false), @@ -346,7 +338,9 @@ make_message(Map) -> error:_Reason -> throw(invalid_topic_name) end, - Message = emqx_message:make(From, QoS, Topic, Payload, #{retain => Retain}, Headers), + Message = emqx_message:make( + http_api, QoS, Topic, Payload, #{retain => Retain}, Headers + ), Size = emqx_message:estimate_size(Message), (Size > size_limit()) andalso throw(packet_too_large), {ok, Message}; From 19033c812ab671438bcd97fec6b4caa7537f683b Mon Sep 17 00:00:00 2001 From: Erik Timan Date: Tue, 3 Jan 2023 15:41:01 +0100 Subject: [PATCH 2/3] chore: update changes --- changes/v5.0.14/fix-9667.en.md | 1 + changes/v5.0.14/fix-9667.zh.md | 1 + 2 files changed, 2 insertions(+) create mode 100644 changes/v5.0.14/fix-9667.en.md create mode 100644 changes/v5.0.14/fix-9667.zh.md diff --git a/changes/v5.0.14/fix-9667.en.md b/changes/v5.0.14/fix-9667.en.md new file mode 100644 index 000000000..4b0fe7aef --- /dev/null +++ b/changes/v5.0.14/fix-9667.en.md @@ -0,0 +1 @@ +Remove possibility to set `clientid` for `/publish` and `/publish/bulk` HTTP APIs. This is to reduce the risk for security confusion. diff --git a/changes/v5.0.14/fix-9667.zh.md b/changes/v5.0.14/fix-9667.zh.md new file mode 100644 index 000000000..f3952ca14 --- /dev/null +++ b/changes/v5.0.14/fix-9667.zh.md @@ -0,0 +1 @@ +从 HTTP API /publish 和 /publish/bulk 中移除 clientid, 降低安全风险 From fb97096405a37ac3faf530fc3939f8f20f03dc36 Mon Sep 17 00:00:00 2001 From: Erik Timan Date: Tue, 10 Jan 2023 16:48:04 +0100 Subject: [PATCH 3/3] fix(mgmt_api): deprecate clientid field instead of removing it --- apps/emqx_management/i18n/emqx_mgmt_api_publish_i18n.conf | 6 ------ apps/emqx_management/src/emqx_mgmt_api_publish.erl | 4 ++++ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/apps/emqx_management/i18n/emqx_mgmt_api_publish_i18n.conf b/apps/emqx_management/i18n/emqx_mgmt_api_publish_i18n.conf index d845bff4b..4ba7e8dba 100644 --- a/apps/emqx_management/i18n/emqx_mgmt_api_publish_i18n.conf +++ b/apps/emqx_management/i18n/emqx_mgmt_api_publish_i18n.conf @@ -63,12 +63,6 @@ result of each individual message in the batch. zh: "MQTT 消息的 QoS" } } - clientid { - desc { - en: "Each message can be published as if it is done on behalf of an MQTT client whos ID can be specified in this field." - zh: "每个消息都可以带上一个 MQTT 客户端 ID,用于模拟 MQTT 客户端的发布行为。" - } - } payload { desc { en: "The MQTT message payload." diff --git a/apps/emqx_management/src/emqx_mgmt_api_publish.erl b/apps/emqx_management/src/emqx_mgmt_api_publish.erl index b7c48a26a..245b56c1d 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_publish.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_publish.erl @@ -102,6 +102,10 @@ fields(message) -> required => false, default => 0 })}, + {clientid, + hoconsc:mk(binary(), #{ + deprecated => {since, "v5.0.14"} + })}, {payload, hoconsc:mk(binary(), #{ desc => ?DESC(payload),