Merge pull request #5849 from savonarola/dashboard-login-security

fix(dashboard rest api): improve auth method security

.github/workflows/run_fvt_tests.yaml

@@ -273,7 +273,7 @@ jobs:
             make
             make install
         - name: run relup test
-          timeout-minutes: 20
+          timeout-minutes: 30
           run: |
             set -e -x -u
             if [ -n "$OLD_VSNS" ]; then

lib-ce/emqx_dashboard/src/emqx_dashboard.app.src

@@ -1,6 +1,6 @@
 {application, emqx_dashboard,
  [{description, "EMQ X Web Dashboard"},
-  {vsn, "4.3.4"}, % strict semver, bump manually!
+  {vsn, "4.3.5"}, % strict semver, bump manually!
   {modules, []},
   {registered, [emqx_dashboard_sup]},
   {applications, [kernel,stdlib,mnesia,minirest]},

lib-ce/emqx_dashboard/src/emqx_dashboard.appup.src

@@ -1,6 +1,6 @@
 %% -*- mode: erlang -*-
 {VSN,
- [ {<<"4.3.[0-3]">>,
+ [ {<<"4.3.[0-4]">>,
     %% load all plugins
     %% NOTE: this depends on the fact that emqx_dashboard is always
     %% the last application gets upgraded
@@ -10,7 +10,7 @@
     ]},
    {<<".*">>, []}
  ],
- [ {<<"4.3.[0-3]">>,
+ [ {<<"4.3.[0-4]">>,
     [ {apply, {emqx_rule_engine, load_providers, []}}
     , {restart_application, emqx_dashboard}
     , {apply, {emqx_plugins, load, []}}

lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl

@@ -168,10 +168,10 @@ check(Username, Password) ->
         [#mqtt_admin{password = <<Salt:4/binary, Hash/binary>>}] ->
             case Hash =:= md5_hash(Salt, Password) of
                 true  -> ok;
-                false -> {error, <<"Password Error">>}
+                false -> {error, <<"Username/Password error">>}
             end;
         [] ->
-            {error, <<"Username Not Found">>}
+            {error, <<"Username/Password error">>}
     end.
 
 %%--------------------------------------------------------------------

lib-ce/emqx_dashboard/test/emqx_dashboard_SUITE.erl

@@ -49,7 +49,7 @@ all() ->
 groups() ->
     [{overview, [sequence], [t_overview]},
      {admins, [sequence], [t_admins_add_delete]},
-     {rest, [sequence], [t_rest_api]},
+     {rest, [sequence], [t_rest_api, t_auth_exhaustive_attack]},
      {cli, [sequence], [t_cli]}
     ].
 
@@ -98,6 +98,11 @@ t_rest_api(_Config) ->
              ]],
     ok.
 
+t_auth_exhaustive_attack(_Config) ->
+    {ok, Res0} = http_post("auth", #{<<"username">> => <<"invalid_login">>, <<"password">> => <<"newpwd">>}),
+    {ok, Res1} = http_post("auth", #{<<"username">> => <<"admin">>, <<"password">> => <<"invalid_password">>}),
+    ?assertEqual(Res0, Res1).
+
 t_cli(_Config) ->
     [mnesia:dirty_delete({mqtt_admin, Admin}) ||  Admin <- mnesia:dirty_all_keys(mqtt_admin)],
     emqx_dashboard_cli:admins(["add", "username", "password"]),