Prevent internal authN backend from accepting blank passwords

Passwordless users were never meant to be used this way. Since
the EXTERNAL authentication mechanism won't use this backend at all,
this is a reasonable safeguard to put in place.

[#153435857]

Author: michaelklishin

src/rabbit_auth_backend_internal.erl

@@ -93,6 +93,11 @@ hashing_module_for_user(#internal_user{
     hashing_algorithm = ModOrUndefined}) ->
         rabbit_password:hashing_mod(ModOrUndefined).
 
+-define(BLANK_PASSWORD_REJECTION_MESSAGE,
+         "user '~s' attempted to log in with a blank password, which is prohibited by the internal authN backend. "
+        "To use TLS/x509 certificate-based autentication, set the rabbitmq_auth_mechanism_ssl plugin and configure the client to use the EXTERNAL authentication mechanism. "
+         "Alternatively change the password for the user to be non-blank.").
+
 %% For cases when we do not have a set of credentials,
 %% namely when x509 (TLS) certificates are used. This should only be
 %% possible when the EXTERNAL authentication mechanism is used, see
@@ -103,6 +108,14 @@ user_login_authentication(Username, []) ->
 %% performs initial validation.
 user_login_authentication(Username, AuthProps) ->
     case lists:keyfind(password, 1, AuthProps) of
+        %% Passwordless users are not supposed to be used with
+        %% this backend (and PLAIN authentication mechanism in general).
+        {password, <<"">>} ->
+            {refused, ?BLANK_PASSWORD_REJECTION_MESSAGE,
+             [Username]};
+        {password, ""} ->
+            {refused, ?BLANK_PASSWORD_REJECTION_MESSAGE,
+             [Username]};
         {password, Cleartext} ->
             internal_check_user_login(
               Username,
@@ -111,6 +124,13 @@ user_login_authentication(Username, AuthProps) ->
                     } = U) ->
                   Hash =:= rabbit_password:salted_hash(
                       hashing_module_for_user(U), Salt, Cleartext);
+                  %% rabbitmqctl clear_password will set password_hash to an empty
+                  %% binary.
+                  %% 
+                  %% See the comment on passwordless users above.
+                  (#internal_user{
+                        password_hash = <<"">>}) ->
+                      false;
                   (#internal_user{}) ->
                       false
               end);

src/rabbit_auth_mechanism_plain.erl

@@ -28,12 +28,6 @@
                     {requires,    rabbit_registry},
                     {enables,     kernel_ready}]}).
 
-%% SASL PLAIN, as used by the Qpid Java client and our clients. Also,
-%% apparently, by OpenAMQ.
-
-%% TODO: reimplement this using the binary module? - that makes use of
-%% BIFs to do binary matching and will thus be much faster.
-
 description() ->
     [{description, <<"SASL PLAIN authentication mechanism">>}].
 

test/unit_inbroker_parallel_SUITE.erl

@@ -19,6 +19,7 @@
 -include_lib("common_test/include/ct.hrl").
 -include_lib("kernel/include/file.hrl").
 -include_lib("amqp_client/include/amqp_client.hrl").
+-include_lib("eunit/include/eunit.hrl").
 
 -compile(export_all).
 
@@ -47,6 +48,10 @@ groups() ->
               password_hashing,
               change_password
             ]},
+          {auth_backend_internal, [parallel], [
+              login_with_credentials_but_no_password,
+              login_of_passwordless_user
+            ]},
           {policy_validation, [parallel, {repeat, 20}], [
               ha_policy_validation,
               policy_validation,
@@ -684,9 +689,12 @@ user_management1(_Config) ->
     %% user authentication
     ok = control_action(authenticate_user,
       ["user_management-user", "user_management-newpassword"]),
-    {refused, _User, _Format, _Params} =
+    ?assertMatch({refused, _User, _Format, _Params},
         control_action(authenticate_user,
-          ["user_management-user", "user_management-password"]),
+          ["user_management-user", "user_management-password"])),
+    ?assertMatch({refused, _User, _Format, _Params},
+        control_action(authenticate_user,
+          ["user_management-user", ""])),
 
     %% vhost creation
     ok = control_action(add_vhost,
@@ -748,6 +756,55 @@ user_management1(_Config) ->
 
     passed.
 
+
+login_with_credentials_but_no_password(Config) ->
+    passed = rabbit_ct_broker_helpers:rpc(Config, 0,
+      ?MODULE, login_with_credentials_but_no_password1, [Config]).
+
+login_with_credentials_but_no_password1(_Config) ->
+    Username = "login_with_credentials_but_no_password-user",
+    Password = "login_with_credentials_but_no_password-password",
+    ok = control_action(add_user, [Username, Password]),
+
+    try
+        rabbit_auth_backend_internal:user_login_authentication(Username,
+                                                              [{key, <<"value">>}]),
+        ?assert(false)
+    catch exit:{unknown_auth_props, Username, [{key, <<"value">>}]} ->
+            ok
+    end,
+
+    ok = control_action(delete_user,
+      [Username]),
+
+    passed.
+
+%% passwordless users are not supposed to be used with
+%% this backend (and PLAIN authentication mechanism in general)
+login_of_passwordless_user(Config) ->
+    passed = rabbit_ct_broker_helpers:rpc(Config, 0,
+      ?MODULE, login_of_passwordless_user1, [Config]).
+
+login_of_passwordless_user1(_Config) ->
+    Username = "login_of_passwordless_user-user",
+    Password = "",
+    ok = control_action(add_user, [Username, Password]),
+
+    ?assertMatch(
+       {refused, _Message, [Username]},
+       rabbit_auth_backend_internal:user_login_authentication(Username,
+                                                              [{password, <<"">>}])),
+
+    ?assertMatch(
+       {refused, _Format, [Username]},
+       rabbit_auth_backend_internal:user_login_authentication(Username,
+                                                              [{password, ""}])),
+
+    ok = control_action(delete_user,
+      [Username]),
+
+    passed.
+
 runtime_parameters(Config) ->
     passed = rabbit_ct_broker_helpers:rpc(Config, 0,
       ?MODULE, runtime_parameters1, [Config]).