diff --git a/CHANGES-4.3.md b/CHANGES-4.3.md index 11debe5e7..8eaa710db 100644 --- a/CHANGES-4.3.md +++ b/CHANGES-4.3.md @@ -22,7 +22,7 @@ File format: * CLI `emqx_ctl pem_cache clean` to force purge x509 certificate cache, to force an immediate reload of all certificates after the files are updated on disk. * Refactor the ExProto so that anonymous clients can also be displayed on the dashboard [#6983] -* Force shutdown of processe that cannot answer takeover event [#7026] +* Force shutdown of processes that cannot answer takeover event [#7026] * `topic` parameter in bridge configuration can have `${node}` substitution (just like in `clientid` parameter) * Add UTF-8 string validity check in `strict_mode` for MQTT packet. When set to true, invalid UTF-8 strings will cause the client to be disconnected. i.e. client ID, topic name. [#7261] @@ -43,6 +43,7 @@ File format: * Fix false alert level log “cannot_find_plugins” caused by duplicate plugin names in `loaded_plugins` files. * Prompt user how to change the dashboard's initial default password when emqx start. * Fix errno=13 'Permission denied' Cannot create FIFO boot error in Amazon Linux 2022 (el8 package) +* Fix user or appid created, name only allow `^[A-Za-z]+[A-Za-z0-9-_]*$` ## v4.3.12 ### Important changes diff --git a/apps/emqx_management/src/emqx_mgmt_api_apps.erl b/apps/emqx_management/src/emqx_mgmt_api_apps.erl index cca0b41f0..abcfe1961 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_apps.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_apps.erl @@ -68,7 +68,7 @@ add_app(_Bindings, Params) -> end. del_app(#{appid := AppId}, _Params) -> - case emqx_mgmt_auth:del_app(AppId) of + case emqx_mgmt_auth:del_app(emqx_mgmt_util:urldecode(AppId)) of ok -> minirest:return(); {error, Reason} -> minirest:return({error, Reason}) end. @@ -77,7 +77,7 @@ list_apps(_Bindings, _Params) -> minirest:return({ok, [format(Apps)|| Apps <- emqx_mgmt_auth:list_apps()]}). lookup_app(#{appid := AppId}, _Params) -> - case emqx_mgmt_auth:lookup_app(AppId) of + case emqx_mgmt_auth:lookup_app(emqx_mgmt_util:urldecode(AppId)) of {AppId, AppSecret, Name, Desc, Status, Expired} -> minirest:return({ok, #{app_id => AppId, secret => AppSecret, @@ -94,7 +94,7 @@ update_app(#{appid := AppId}, Params) -> Desc = proplists:get_value(<<"desc">>, Params), Status = proplists:get_value(<<"status">>, Params), Expired = proplists:get_value(<<"expired">>, Params), - case emqx_mgmt_auth:update_app(AppId, Name, Desc, Status, Expired) of + case emqx_mgmt_auth:update_app(emqx_mgmt_util:urldecode(AppId), Name, Desc, Status, Expired) of ok -> minirest:return(); {error, Reason} -> minirest:return({error, Reason}) end. diff --git a/apps/emqx_management/src/emqx_mgmt_auth.erl b/apps/emqx_management/src/emqx_mgmt_auth.erl index c05cbf581..5f2b96dff 100644 --- a/apps/emqx_management/src/emqx_mgmt_auth.erl +++ b/apps/emqx_management/src/emqx_mgmt_auth.erl @@ -16,6 +16,8 @@ -module(emqx_mgmt_auth). +-include_lib("emqx/include/logger.hrl"). + %% Mnesia Bootstrap -export([mnesia/1]). -boot_mnesia({mnesia, [boot]}). @@ -89,36 +91,45 @@ add_app(AppId, Name, Desc, Status, Expired) when is_binary(AppId) -> -> {ok, appsecret()} | {error, term()}). add_app(AppId, Name, Secret, Desc, Status, Expired) when is_binary(AppId) -> - Secret1 = generate_appsecret_if_need(Secret), - App = #mqtt_app{id = AppId, - secret = Secret1, - name = Name, - desc = Desc, - status = Status, - expired = Expired}, - AddFun = fun() -> - case mnesia:wread({mqtt_app, AppId}) of - [] -> mnesia:write(App); - _ -> mnesia:abort(alread_existed) - end - end, - case mnesia:transaction(AddFun) of - {atomic, ok} -> {ok, Secret1}; - {aborted, Reason} -> {error, Reason} + case emqx_misc:is_sane_id(AppId) of + ok -> + Secret1 = generate_appsecret_if_need(Secret), + App = #mqtt_app{id = AppId, + secret = Secret1, + name = Name, + desc = Desc, + status = Status, + expired = Expired}, + AddFun = fun() -> + case mnesia:wread({mqtt_app, AppId}) of + [] -> mnesia:write(App); + _ -> mnesia:abort(already_existed) + end + end, + case mnesia:transaction(AddFun) of + {atomic, ok} -> {ok, Secret1}; + {aborted, Reason} -> {error, Reason} + end; + {error, Reason} -> {error, Reason} end. force_add_app(AppId, Name, Secret, Desc, Status, Expired) -> - AddFun = fun() -> - mnesia:write(#mqtt_app{id = AppId, - secret = Secret, - name = Name, - desc = Desc, - status = Status, - expired = Expired}) - end, - case mnesia:transaction(AddFun) of - {atomic, ok} -> ok; - {aborted, Reason} -> {error, Reason} + case emqx_misc:is_sane_id(AppId) of + ok -> + AddFun = fun() -> + mnesia:write(#mqtt_app{ + id = AppId, + secret = Secret, + name = Name, + desc = Desc, + status = Status, + expired = Expired}) + end, + case mnesia:transaction(AddFun) of + {atomic, ok} -> ok; + {aborted, Reason} -> {error, Reason} + end; + {error, Reason} -> {error, Reason} end. -spec(generate_appsecret_if_need(binary() | undefined) -> binary()). @@ -207,4 +218,3 @@ is_authorized(AppId, AppSecret) -> is_expired(undefined) -> true; is_expired(Expired) -> Expired >= erlang:system_time(second). - diff --git a/apps/emqx_management/src/emqx_mgmt_sup.erl b/apps/emqx_management/src/emqx_mgmt_sup.erl index f3f5545f2..8ad0bc963 100644 --- a/apps/emqx_management/src/emqx_mgmt_sup.erl +++ b/apps/emqx_management/src/emqx_mgmt_sup.erl @@ -27,4 +27,3 @@ start_link() -> init([]) -> {ok, {{one_for_one, 1, 5}, []}}. - diff --git a/lib-ce/emqx_dashboard/etc/emqx_dashboard.conf b/lib-ce/emqx_dashboard/etc/emqx_dashboard.conf index f67b88c17..2d59264a1 100644 --- a/lib-ce/emqx_dashboard/etc/emqx_dashboard.conf +++ b/lib-ce/emqx_dashboard/etc/emqx_dashboard.conf @@ -3,12 +3,17 @@ ##-------------------------------------------------------------------- ## Default user's login name. -## +## For safety, it should be changed as soon as possible. +## Please use the './bin/emqx_ctl admins' CLI to change it. +## Then comment `dashboard.default_user.login/password` from here ## Value: String dashboard.default_user.login = admin ## Default user's password. -## +## The initial default password for 'dashboard.default_user.login'" +## For safety, it should be changed as soon as possible. +## Please use the './bin/emqx_ctl admins' CLI to change it. +## Then comment `dashboard.default_user.login/password` from here ## Value: String dashboard.default_user.password = public @@ -127,4 +132,3 @@ dashboard.listener.http.ipv6_v6only = false ## ## Value: on | off ## dashboard.listener.https.honor_cipher_order = on - diff --git a/lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl b/lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl index c9e9bee3c..420380f88 100644 --- a/lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl +++ b/lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl @@ -78,18 +78,24 @@ start_link() -> -spec(add_user(binary(), binary(), binary()) -> ok | {error, any()}). add_user(Username, Password, Tags) when is_binary(Username), is_binary(Password) -> - Admin = #mqtt_admin{username = Username, password = hash(Password), tags = Tags}, - return(mnesia:transaction(fun add_user_/1, [Admin])). + case emqx_misc:is_sane_id(Username) of + ok -> + Admin = #mqtt_admin{username = Username, password = hash(Password), tags = Tags}, + return(mnesia:transaction(fun add_user_/1, [Admin])); + {error, Reason} -> {error, Reason} + end. force_add_user(Username, Password, Tags) -> - AddFun = fun() -> - mnesia:write(#mqtt_admin{username = Username, - password = Password, - tags = Tags}) - end, - case mnesia:transaction(AddFun) of - {atomic, ok} -> ok; - {aborted, Reason} -> {error, Reason} + case emqx_misc:is_sane_id(Username) of + ok -> + AddFun = fun() -> + mnesia:write(#mqtt_admin{username = Username, password = Password, tags = Tags}) + end, + case mnesia:transaction(AddFun) of + {atomic, ok} -> ok; + {aborted, Reason} -> {error, Reason} + end; + {error, Reason} -> {error, Reason} end. %% @private diff --git a/lib-ce/emqx_dashboard/src/emqx_dashboard_api.erl b/lib-ce/emqx_dashboard/src/emqx_dashboard_api.erl index e1c89efbb..4dbf2517f 100644 --- a/lib-ce/emqx_dashboard/src/emqx_dashboard_api.erl +++ b/lib-ce/emqx_dashboard/src/emqx_dashboard_api.erl @@ -77,9 +77,10 @@ auth(_Bindings, Params) -> Password = proplists:get_value(<<"password">>, Params), return(emqx_dashboard_admin:check(Username, Password)). -change_pwd(#{username := Username}, Params) -> +change_pwd(#{username := Username0}, Params) -> OldPwd = proplists:get_value(<<"old_pwd">>, Params), NewPwd = proplists:get_value(<<"new_pwd">>, Params), + Username = emqx_mgmt_util:urldecode(Username0), return(emqx_dashboard_admin:change_password(Username, OldPwd, NewPwd)). create(_Bindings, Params) -> @@ -96,14 +97,13 @@ list(_Bindings, _Params) -> update(#{name := Username}, Params) -> Tags = proplists:get_value(<<"tags">>, Params), - return(emqx_dashboard_admin:update_user(Username, Tags)). + return(emqx_dashboard_admin:update_user(emqx_mgmt_util:urldecode(Username), Tags)). delete(#{name := <<"admin">>}, _Params) -> return({error, <<"Cannot delete admin">>}); delete(#{name := Username}, _Params) -> - return(emqx_dashboard_admin:remove_user(Username)). + return(emqx_dashboard_admin:remove_user(emqx_mgmt_util:urldecode(Username))). row(#mqtt_admin{username = Username, tags = Tags}) -> #{username => Username, tags => Tags}. - diff --git a/lib-ce/emqx_dashboard/test/emqx_dashboard_SUITE.erl b/lib-ce/emqx_dashboard/test/emqx_dashboard_SUITE.erl index ef2e747fa..5099d4449 100644 --- a/lib-ce/emqx_dashboard/test/emqx_dashboard_SUITE.erl +++ b/lib-ce/emqx_dashboard/test/emqx_dashboard_SUITE.erl @@ -67,9 +67,13 @@ t_overview(_) -> t_admins_add_delete(_) -> ok = emqx_dashboard_admin:add_user(<<"username">>, <<"password">>, <<"tag">>), ok = emqx_dashboard_admin:add_user(<<"username1">>, <<"password1">>, <<"tag1">>), + ok = emqx_dashboard_admin:add_user(<<"1username1">>, <<"password1">>, <<"tag1">>), + {error, _} = emqx_dashboard_admin:add_user(<<"u/sername1">>, <<"password1">>, <<"tag1">>), + {error, _} = emqx_dashboard_admin:add_user(<<"/username1">>, <<"password1">>, <<"tag1">>), Admins = emqx_dashboard_admin:all_users(), - ?assertEqual(3, length(Admins)), + ?assertEqual(4, length(Admins)), ok = emqx_dashboard_admin:remove_user(<<"username1">>), + ok = emqx_dashboard_admin:remove_user(<<"1username1">>), Users = emqx_dashboard_admin:all_users(), ?assertEqual(2, length(Users)), ok = emqx_dashboard_admin:change_password(<<"username">>, <<"password">>, <<"pwd">>), @@ -77,13 +81,14 @@ t_admins_add_delete(_) -> ?assert(request_dashboard(get, api_path("brokers"), auth_header_("username", "pwd"))), ok = emqx_dashboard_admin:remove_user(<<"username">>), - ?assertNotEqual(true, request_dashboard(get, api_path("brokers"), auth_header_("username", "pwd"))). + ?assertNotEqual(true, request_dashboard(get, api_path("brokers"), + auth_header_("username", "pwd"))). t_rest_api(_Config) -> {ok, Res0} = http_get("users"), - - ?assertEqual([#{<<"username">> => <<"admin">>, - <<"tags">> => <<"administrator">>}], get_http_data(Res0)), + Users = get_http_data(Res0), + ?assert(lists:member(#{<<"username">> => <<"admin">>, <<"tags">> => <<"administrator">>}, + Users)), AssertSuccess = fun({ok, Res}) -> ?assertEqual(#{<<"code">> => 0}, json(Res)) @@ -165,4 +170,3 @@ api_path(Path) -> json(Data) -> {ok, Jsx} = emqx_json:safe_decode(Data, [return_maps]), Jsx. - diff --git a/src/emqx.appup.src b/src/emqx.appup.src index 95e2f48f2..86ca424b3 100644 --- a/src/emqx.appup.src +++ b/src/emqx.appup.src @@ -13,10 +13,12 @@ {load_module,emqx_app,brutal_purge,soft_purge,[]}, {load_module,emqx_ctl,brutal_purge,soft_purge,[]}, {load_module,emqx_cm,brutal_purge,soft_purge,[]}, + {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx_connection,brutal_purge,soft_purge,[]}]}, {"4.3.12", [{load_module,emqx_plugins,brutal_purge,soft_purge,[]}, {load_module,emqx_frame,brutal_purge,soft_purge,[]}, + {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx_sys_mon,brutal_purge,soft_purge,[]}, {load_module,emqx_pmon,brutal_purge,soft_purge,[]}, {load_module,emqx_banned,brutal_purge,soft_purge,[]}, @@ -40,6 +42,7 @@ [{load_module,emqx_plugins,brutal_purge,soft_purge,[]}, {load_module,emqx_pmon,brutal_purge,soft_purge,[]}, {load_module,emqx_banned,brutal_purge,soft_purge,[]}, + {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx_sys,brutal_purge,soft_purge,[]}, {load_module,emqx_cm,brutal_purge,soft_purge,[]}, {load_module,emqx_vm_mon,brutal_purge,soft_purge,[]}, @@ -63,6 +66,7 @@ {"4.3.10", [{load_module,emqx_plugins,brutal_purge,soft_purge,[]}, {load_module,emqx_pmon,brutal_purge,soft_purge,[]}, + {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx_banned,brutal_purge,soft_purge,[]}, {load_module,emqx_sys,brutal_purge,soft_purge,[]}, {load_module,emqx_cm,brutal_purge,soft_purge,[]}, @@ -87,6 +91,7 @@ {"4.3.9", [{load_module,emqx_plugins,brutal_purge,soft_purge,[]}, {load_module,emqx_pmon,brutal_purge,soft_purge,[]}, + {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx_banned,brutal_purge,soft_purge,[]}, {load_module,emqx_sys,brutal_purge,soft_purge,[]}, {load_module,emqx_vm_mon,brutal_purge,soft_purge,[]}, @@ -115,6 +120,7 @@ {"4.3.8", [{load_module,emqx_plugins,brutal_purge,soft_purge,[]}, {load_module,emqx_pmon,brutal_purge,soft_purge,[]}, + {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx_banned,brutal_purge,soft_purge,[]}, {load_module,emqx_sys,brutal_purge,soft_purge,[]}, {load_module,emqx_vm_mon,brutal_purge,soft_purge,[]}, @@ -410,6 +416,7 @@ {load_module,emqx_frame,brutal_purge,soft_purge,[]}, {load_module,emqx_sys_mon,brutal_purge,soft_purge,[]}, {load_module,emqx_pmon,brutal_purge,soft_purge,[]}, + {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx_banned,brutal_purge,soft_purge,[]}, {load_module,emqx_os_mon,brutal_purge,soft_purge,[]}, {load_module,emqx_channel,brutal_purge,soft_purge,[]}, @@ -422,6 +429,7 @@ [{load_module,emqx_plugins,brutal_purge,soft_purge,[]}, {load_module,emqx_frame,brutal_purge,soft_purge,[]}, {load_module,emqx_sys_mon,brutal_purge,soft_purge,[]}, + {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx_pmon,brutal_purge,soft_purge,[]}, {load_module,emqx_banned,brutal_purge,soft_purge,[]}, {load_module,emqx_sys,brutal_purge,soft_purge,[]}, @@ -442,6 +450,7 @@ {"4.3.11", [{load_module,emqx_plugins,brutal_purge,soft_purge,[]}, {load_module,emqx_pmon,brutal_purge,soft_purge,[]}, + {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx_banned,brutal_purge,soft_purge,[]}, {load_module,emqx_sys,brutal_purge,soft_purge,[]}, {load_module,emqx_cm,brutal_purge,soft_purge,[]}, @@ -465,6 +474,7 @@ {"4.3.10", [{load_module,emqx_plugins,brutal_purge,soft_purge,[]}, {load_module,emqx_pmon,brutal_purge,soft_purge,[]}, + {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx_banned,brutal_purge,soft_purge,[]}, {load_module,emqx_sys,brutal_purge,soft_purge,[]}, {load_module,emqx_cm,brutal_purge,soft_purge,[]}, @@ -488,6 +498,7 @@ {"4.3.9", [{load_module,emqx_plugins,brutal_purge,soft_purge,[]}, {load_module,emqx_pmon,brutal_purge,soft_purge,[]}, + {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx_banned,brutal_purge,soft_purge,[]}, {load_module,emqx_sys,brutal_purge,soft_purge,[]}, {load_module,emqx_vm_mon,brutal_purge,soft_purge,[]}, @@ -515,6 +526,7 @@ {"4.3.8", [{load_module,emqx_plugins,brutal_purge,soft_purge,[]}, {load_module,emqx_pmon,brutal_purge,soft_purge,[]}, + {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx_banned,brutal_purge,soft_purge,[]}, {load_module,emqx_sys,brutal_purge,soft_purge,[]}, {load_module,emqx_vm_mon,brutal_purge,soft_purge,[]}, diff --git a/src/emqx_misc.erl b/src/emqx_misc.erl index eb6a25377..3c866b732 100644 --- a/src/emqx_misc.erl +++ b/src/emqx_misc.erl @@ -52,6 +52,26 @@ , hexstr2bin/1 ]). +-export([ is_sane_id/1 + ]). + +-define(VALID_STR_RE, "^[A-Za-z0-9]+[A-Za-z0-9-_]*$"). + +-spec is_sane_id(list() | binary()) -> ok | {error, Reason::binary()}. +is_sane_id(Str) -> + StrLen = len(Str), + case StrLen > 0 andalso StrLen =< 256 of + true -> + case re:run(Str, ?VALID_STR_RE) of + nomatch -> {error, <<"required: " ?VALID_STR_RE>>}; + _ -> ok + end; + false -> {error, <<"0 < Length =< 256">>} + end. + +len(Bin) when is_binary(Bin) -> byte_size(Bin); +len(Str) when is_list(Str) -> length(Str). + -define(OOM_FACTOR, 1.25). %% @doc Parse v4 or v6 string format address to tuple. @@ -309,4 +329,30 @@ hexchar2int(I) when I >= $a andalso I =< $f -> I - $a + 10. ipv6_probe_test() -> ?assertEqual([{ipv6_probe, true}], ipv6_probe([])). +is_sane_id_test() -> + ?assertMatch({error, _}, is_sane_id("")), + ?assertMatch({error, _}, is_sane_id("_")), + ?assertMatch({error, _}, is_sane_id("_aaa")), + ?assertMatch({error, _}, is_sane_id("lkad/oddl")), + ?assertMatch({error, _}, is_sane_id("lkad*oddl")), + ?assertMatch({error, _}, is_sane_id("script>lkadoddl")), + ?assertMatch({error, _}, is_sane_id("