Merge branch 'hans/ssh/pwdfun/OTP-13055' into maint

* hans/ssh/pwdfun/OTP-13055:
  ssh: changes after doc review
  ssh: Document pwdfun
  ssh: make corrections of keyboard-interactive client
  ssh: enable users to give option keyboard_interact_fun
  ssh: pwdfun/4 and simple tests

Conflicts:
	lib/ssh/doc/src/ssh.xml

6 files changed, 280 insertions, 24 deletions

diff --git a/lib/ssh/doc/src/ssh.xml b/lib/ssh/doc/src/ssh.xml
index 5c18c48f65..0e5a0706f5 100644
--- a/lib/ssh/doc/src/ssh.xml
+++ b/lib/ssh/doc/src/ssh.xml
@@ -61,6 +61,29 @@
   </section>
   
   <section>
+    <title>OPTIONS</title>
+    <p>The exact behaviour of some functions can be adjusted with the use of options which are documented together
+    with the functions. Generally could each option be used at most one time in each function call. If given two or more
+    times, the effect is not predictable unless explicitly documented.</p>
+    <p>The options are of different kinds:</p>
+    <taglist>
+      <tag>Limits</tag>
+      <item><p>which alters limits in the system, for example number of simultaneous login attempts.</p></item>
+
+      <tag>Timeouts</tag>
+      <item><p>which give some defined behaviour if too long time elapses before a given event or action,
+      for example time to wait for an answer.</p></item>
+
+      <tag>Callbacks</tag>
+      <item><p>which gives the caller of the function the possibility to execute own code on some events,
+      for example calling an own logging function or to perform an own login function</p></item>
+
+      <tag>Behaviour</tag>
+      <item><p>which changes the systems behaviour.</p></item>
+    </taglist>
+  </section>
+
+  <section>
     <title>DATA TYPES</title>
     <p>Type definitions that are used more than once in
     this module, or abstractions to indicate the intended use of the data
@@ -501,12 +524,43 @@ kex is implicit but public_key is set explicitly.</p>
 	    <p>See RFC 4419 for the function of the Max and Min values.</p>
 	  </item>
 
-	  <tag><c><![CDATA[{pwdfun, fun(User::string(), password::string()) -> boolean()}]]></c></tag>
+	  <tag><c><![CDATA[{pwdfun, fun(User::string(), Password::string(), PeerAddress::{ip_adress(),port_number()}, State::any()) -> boolean() | disconnect | {boolean(),any()} }]]></c></tag>
+	  <item>
+	    <p>Provides a function for password validation. This could used for calling an external system or if
+	    passwords should be stored as a hash. The fun returns:
+	      <list type="bulleted">
+		<item><c>true</c> if the user and password is valid and</item>
+		<item><c>false</c> otherwise.</item> 
+	      </list>
+	    </p>
+	    <p>This fun can also be used to make delays in authentication tries for example by calling
+	      <seealso marker="stdlib:timer#sleep/1">timer:sleep/1</seealso>. To facilitate counting of failed tries
+	      the <c>State</c> variable could be used. This state is per connection only. The first time the pwdfun
+	      is called for a connection, the <c>State</c> variable has the value <c>undefined</c>.  
+	      The pwdfun can return - in addition to the values above - a new state
+	      as:
+	      <list type="bulleted">
+		<item><c>{true,  NewState:any()}</c> if the user and password is valid or</item>
+		<item><c>{false, NewState:any()}</c> if the user or password is invalid</item> 
+	      </list>
+	    </p>
+	    <p>A third usage is to block login attempts from a missbehaving peer. The <c>State</c> described above 
+	    can be used for this. In addition to the responses above, the following return value is introduced:
+	    <list type="bulleted">
+	      <item><c>disconnect</c> if the connection should be closed immediately after sending a SSH_MSG_DISCONNECT
+	      message.</item>
+	    </list>
+	    </p>
+	  </item>
+
+	  <tag><c><![CDATA[{pwdfun, fun(User::string(), Password::string()) -> boolean()}]]></c></tag>
 	  <item>
 	    <p>Provides a function for password validation. This function is called
 	      with user and password as strings, and returns
 	      <c><![CDATA[true]]></c> if the password is valid and
 	      <c><![CDATA[false]]></c> otherwise.</p>
+	      <p>This option (<c>{pwdfun,fun/2}</c>) is the same as a subset of the previous 
+	      (<c>{pwdfun,fun/4}</c>). It is kept for compatibility.</p>
 	  </item>
 
 	  <tag><c><![CDATA[{negotiation_timeout, integer()}]]></c></tag>
diff --git a/lib/ssh/src/ssh.erl b/lib/ssh/src/ssh.erl
index 39cf441090..5bde184070 100644
--- a/lib/ssh/src/ssh.erl
+++ b/lib/ssh/src/ssh.erl
@@ -338,6 +338,8 @@ handle_option([{pwdfun, _} = Opt | Rest], SocketOptions, SshOptions) ->
     handle_option(Rest, SocketOptions, [handle_ssh_option(Opt) | SshOptions]);
 handle_option([{key_cb, _} = Opt | Rest], SocketOptions, SshOptions) ->
     handle_option(Rest, SocketOptions, [handle_ssh_option(Opt) | SshOptions]);
+handle_option([{keyboard_interact_fun, _} = Opt | Rest], SocketOptions, SshOptions) ->
+    handle_option(Rest, SocketOptions, [handle_ssh_option(Opt) | SshOptions]);
 %%Backwards compatibility
 handle_option([{allow_user_interaction, Value}  | Rest], SocketOptions, SshOptions) ->
     handle_option(Rest, SocketOptions, [handle_ssh_option({user_interaction, Value}) | SshOptions]);
@@ -503,10 +505,14 @@ handle_ssh_option({password, Value} = Opt) when is_list(Value) ->
     Opt;
 handle_ssh_option({user_passwords, Value} = Opt) when is_list(Value)->
     Opt;
-handle_ssh_option({pwdfun, Value} = Opt) when is_function(Value) ->
+handle_ssh_option({pwdfun, Value} = Opt) when is_function(Value,2) ->
+    Opt;
+handle_ssh_option({pwdfun, Value} = Opt) when is_function(Value,4) ->
     Opt;
 handle_ssh_option({key_cb, Value} = Opt)  when is_atom(Value) ->
     Opt;
+handle_ssh_option({keyboard_interact_fun, Value} = Opt) when is_function(Value,3) ->
+    Opt;
 handle_ssh_option({compression, Value} = Opt) when is_atom(Value) ->
     Opt;
 handle_ssh_option({exec, {Module, Function, _}} = Opt) when is_atom(Module), 
diff --git a/lib/ssh/src/ssh.hrl b/lib/ssh/src/ssh.hrl
index fc9d60c500..4ad936f742 100644
--- a/lib/ssh/src/ssh.hrl
+++ b/lib/ssh/src/ssh.hrl
@@ -138,6 +138,7 @@
 	  kb_tries_left = 0,                %  integer(), num tries left for "keyboard-interactive"
 	  userauth_preference,
 	  available_host_keys,
+	  pwdfun_user_state,
 	  authenticated = false
 	 }).
 
diff --git a/lib/ssh/src/ssh_auth.erl b/lib/ssh/src/ssh_auth.erl
index 04749fcf8e..4967a2e4cd 100644
--- a/lib/ssh/src/ssh_auth.erl
+++ b/lib/ssh/src/ssh_auth.erl
@@ -174,15 +174,15 @@ handle_userauth_request(#ssh_msg_userauth_request{user = User,
 			#ssh{opts = Opts,
 			     userauth_supported_methods = Methods} = Ssh) ->
     Password = unicode:characters_to_list(BinPwd),
-    case check_password(User, Password, Opts) of
-	true ->
+    case check_password(User, Password, Opts, Ssh) of
+	{true,Ssh1} ->
 	    {authorized, User,
-	     ssh_transport:ssh_packet(#ssh_msg_userauth_success{}, Ssh)};
-	false  ->
+	     ssh_transport:ssh_packet(#ssh_msg_userauth_success{}, Ssh1)};
+	{false,Ssh1}  ->
 	    {not_authorized, {User, {error,"Bad user or password"}}, 
 	     ssh_transport:ssh_packet(#ssh_msg_userauth_failure{
 		     authentications = Methods,
-		     partial_success = false}, Ssh)}
+		     partial_success = false}, Ssh1)}
     end;
 
 handle_userauth_request(#ssh_msg_userauth_request{user = User,
@@ -335,16 +335,16 @@ handle_userauth_info_response(#ssh_msg_userauth_info_response{num_responses = 1,
 				   kb_tries_left = KbTriesLeft,
 				   user = User,
 				   userauth_supported_methods = Methods} = Ssh) ->
-    case check_password(User, unicode:characters_to_list(Password), Opts) of
-	true ->
+    case check_password(User, unicode:characters_to_list(Password), Opts, Ssh) of
+	{true,Ssh1} ->
 	    {authorized, User,
-	     ssh_transport:ssh_packet(#ssh_msg_userauth_success{}, Ssh)};
-	false ->
+	     ssh_transport:ssh_packet(#ssh_msg_userauth_success{}, Ssh1)};
+	{false,Ssh1} ->
 	    {not_authorized, {User, {error,"Bad user or password"}}, 
 	     ssh_transport:ssh_packet(#ssh_msg_userauth_failure{
 					 authentications = Methods,
 					 partial_success = false}, 
-				      Ssh#ssh{kb_tries_left = max(KbTriesLeft-1, 0)}
+				      Ssh1#ssh{kb_tries_left = max(KbTriesLeft-1, 0)}
 				     )}
     end;
 
@@ -364,6 +364,11 @@ method_preference(Algs) ->
 		       [{"publickey", ?MODULE, publickey_msg, [A]} | Acc]
 	       end, 
 	       [{"password", ?MODULE, password_msg, []},
+		{"keyboard-interactive", ?MODULE, keyboard_interactive_msg, []},
+		{"keyboard-interactive", ?MODULE, keyboard_interactive_msg, []},
+		{"keyboard-interactive", ?MODULE, keyboard_interactive_msg, []},
+		{"keyboard-interactive", ?MODULE, keyboard_interactive_msg, []},
+		{"keyboard-interactive", ?MODULE, keyboard_interactive_msg, []},
 		{"keyboard-interactive", ?MODULE, keyboard_interactive_msg, []}
 	       ],
 	       Algs).
@@ -387,13 +392,34 @@ user_name(Opts) ->
 	    {ok, User}
     end.
 
-check_password(User, Password, Opts) ->
+check_password(User, Password, Opts, Ssh) ->
     case proplists:get_value(pwdfun, Opts) of
 	undefined ->
 	    Static = get_password_option(Opts, User),
-	    Password == Static;
-	Cheker ->
-	    Cheker(User, Password)
+	    {Password == Static, Ssh};
+
+	Checker when is_function(Checker,2) ->
+	    {Checker(User, Password), Ssh};
+
+	Checker when is_function(Checker,4) ->
+	    #ssh{pwdfun_user_state = PrivateState,
+		 peer = {_,PeerAddr={_,_}}
+		} = Ssh,
+	    case Checker(User, Password, PeerAddr, PrivateState) of
+		true ->
+		    {true,Ssh};
+		false ->
+		    {false,Ssh};
+		{true,NewState} ->
+		    {true, Ssh#ssh{pwdfun_user_state=NewState}};
+		{false,NewState} ->
+		    {false, Ssh#ssh{pwdfun_user_state=NewState}};
+		disconnect ->
+		    throw(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_SERVICE_NOT_AVAILABLE,
+					      description = 
+						  "Unable to connect using the available authentication methods",
+					      language = ""})
+	    end
     end.
 
 get_password_option(Opts, User) ->
@@ -451,14 +477,14 @@ keyboard_interact_get_responses(false, undefined, undefined, _, _, _, [Prompt|_]
     ssh_no_io:read_line(Prompt, Opts); %% Throws error as keyboard interaction is not allowed
 keyboard_interact_get_responses(true, undefined, _,IoCb, Name, Instr, PromptInfos, Opts, _) ->
     keyboard_interact(IoCb, Name, Instr, PromptInfos, Opts);
-keyboard_interact_get_responses(true, Fun, _, Name, Instr, PromptInfos, _, _, NumPrompts) ->
+keyboard_interact_get_responses(true, Fun, _Pwd, _IoCb, Name, Instr, PromptInfos, _Opts, NumPrompts) ->
     keyboard_interact_fun(Fun, Name, Instr, PromptInfos, NumPrompts).
 
 keyboard_interact(IoCb, Name, Instr, Prompts, Opts) ->
-    if Name /= "" -> IoCb:format("~s", [Name]);
+    if Name /= "" -> IoCb:format("~s~n", [Name]);
        true       -> ok
     end,
-    if Instr /= "" -> IoCb:format("~s", [Instr]);
+    if Instr /= "" -> IoCb:format("~s~n", [Instr]);
        true        -> ok
     end,
     lists:map(fun({Prompt, true})  -> IoCb:read_line(Prompt, Opts);
diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl
index 7fb86c1108..a2d1b5b810 100644
--- a/lib/ssh/src/ssh_connection_handler.erl
+++ b/lib/ssh/src/ssh_connection_handler.erl
@@ -627,14 +627,24 @@ userauth_keyboard_interactive(#ssh_msg_userauth_info_response{} = Msg,
 	    retry_fun(User, Address, Reason, Opts),
 	    send_msg(Reply, State),
 	    {next_state, userauth, next_packet(State#state{ssh_params = Ssh})} 
-    end.
-			
+    end;
+userauth_keyboard_interactive(Msg = #ssh_msg_userauth_failure{},
+			      #state{ssh_params = Ssh0 =
+					 #ssh{role = client,
+					      userauth_preference = Prefs0}} 
+			      = State) ->
+    Prefs = [{Method,M,F,A} || {Method,M,F,A} <- Prefs0,
+			       Method =/= "keyboard-interactive"],
+    userauth(Msg, State#state{ssh_params = Ssh0#ssh{userauth_preference=Prefs}}).
+
 
 
-userauth_keyboard_interactive_info_response(Msg=#ssh_msg_userauth_failure{}, State) ->
+userauth_keyboard_interactive_info_response(Msg=#ssh_msg_userauth_failure{},
+					    #state{ssh_params = #ssh{role = client}} = State) ->
     userauth(Msg, State);
 
-userauth_keyboard_interactive_info_response(Msg=#ssh_msg_userauth_success{}, State) ->
+userauth_keyboard_interactive_info_response(Msg=#ssh_msg_userauth_success{},
+					    #state{ssh_params = #ssh{role = client}} = State) ->
     userauth(Msg, State).
 
 %%--------------------------------------------------------------------
diff --git a/lib/ssh/test/ssh_options_SUITE.erl b/lib/ssh/test/ssh_options_SUITE.erl
index cf15ca4253..6a201d401f 100644
--- a/lib/ssh/test/ssh_options_SUITE.erl
+++ b/lib/ssh/test/ssh_options_SUITE.erl
@@ -45,6 +45,9 @@
 	 max_sessions_ssh_connect_sequential/1, 
 	 server_password_option/1, 
 	 server_userpassword_option/1, 
+	 server_pwdfun_option/1,
+	 server_pwdfun_4_option/1,
+	 server_pwdfun_4_option_repeat/1,
 	 ssh_connect_arg4_timeout/1, 
 	 ssh_connect_negtimeout_parallel/1, 
 	 ssh_connect_negtimeout_sequential/1, 
@@ -83,6 +86,9 @@ all() ->
      connectfun_disconnectfun_client,
      server_password_option,
      server_userpassword_option,
+     server_pwdfun_option,
+     server_pwdfun_4_option,
+     server_pwdfun_4_option_repeat,
      {group, dir_options},
      ssh_connect_timeout,
      ssh_connect_arg4_timeout,
@@ -188,7 +194,9 @@ init_per_testcase(_TestCase, Config) ->
     Config.
 
 end_per_testcase(TestCase, Config) when TestCase == server_password_option;
-					TestCase == server_userpassword_option ->
+					TestCase == server_userpassword_option;
+					TestCase == server_pwdfun_option;
+					TestCase == server_pwdfun_4_option ->
     UserDir = filename:join(?config(priv_dir, Config), nopubkey),
     ssh_test_lib:del_dirs(UserDir),
     end_per_testcase(Config);
@@ -272,6 +280,157 @@ server_userpassword_option(Config) when is_list(Config) ->
     ssh:stop_daemon(Pid).
 
 %%--------------------------------------------------------------------
+%%% validate to server that uses the 'pwdfun' option
+server_pwdfun_option(Config) ->
+    PrivDir = ?config(priv_dir, Config),
+    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
+    file:make_dir(UserDir),
+    SysDir = ?config(data_dir, Config),	  
+    CHKPWD = fun("foo",Pwd) -> Pwd=="bar";
+		(_,_) -> false
+	     end,
+    {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir},
+					     {user_dir, PrivDir},
+					     {pwdfun,CHKPWD}]),
+    ConnectionRef =
+	ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true},
+					  {user, "foo"},
+					  {password, "bar"},
+					  {user_interaction, false},
+					  {user_dir, UserDir}]),
+    ssh:close(ConnectionRef),
+
+    Reason = "Unable to connect using the available authentication methods",
+    
+    {error, Reason} =
+	ssh:connect(Host, Port, [{silently_accept_hosts, true},
+				 {user, "foo"},
+				 {password, "morot"},
+				 {user_interaction, false},
+				 {user_dir, UserDir}]),
+    {error, Reason} =
+	ssh:connect(Host, Port, [{silently_accept_hosts, true},
+				 {user, "vego"},
+				 {password, "foo"},
+				 {user_interaction, false},
+				 {user_dir, UserDir}]),
+    ssh:stop_daemon(Pid).
+
+    
+%%--------------------------------------------------------------------
+%%% validate to server that uses the 'pwdfun/4' option
+server_pwdfun_4_option(Config) ->
+    PrivDir = ?config(priv_dir, Config),
+    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
+    file:make_dir(UserDir),
+    SysDir = ?config(data_dir, Config),	  
+    PWDFUN = fun("foo",Pwd,{_,_},undefined) -> Pwd=="bar";
+		("fie",Pwd,{_,_},undefined) -> {Pwd=="bar",new_state};
+		("bandit",_,_,_) -> disconnect;
+		(_,_,_,_) -> false
+	     end,
+    {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir},
+					     {user_dir, PrivDir},
+					     {pwdfun,PWDFUN}]),
+    ConnectionRef1 =
+	ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true},
+					  {user, "foo"},
+					  {password, "bar"},
+					  {user_interaction, false},
+					  {user_dir, UserDir}]),
+    ssh:close(ConnectionRef1),
+
+    ConnectionRef2 =
+	ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true},
+					  {user, "fie"},
+					  {password, "bar"},
+					  {user_interaction, false},
+					  {user_dir, UserDir}]),
+    ssh:close(ConnectionRef2),
+
+    Reason = "Unable to connect using the available authentication methods",
+    
+    {error, Reason} =
+	ssh:connect(Host, Port, [{silently_accept_hosts, true},
+				 {user, "foo"},
+				 {password, "morot"},
+				 {user_interaction, false},
+				 {user_dir, UserDir}]),
+    {error, Reason} =
+	ssh:connect(Host, Port, [{silently_accept_hosts, true},
+				 {user, "fie"},
+				 {password, "morot"},
+				 {user_interaction, false},
+				 {user_dir, UserDir}]),
+    {error, Reason} =
+	ssh:connect(Host, Port, [{silently_accept_hosts, true},
+				 {user, "vego"},
+				 {password, "foo"},
+				 {user_interaction, false},
+				 {user_dir, UserDir}]),
+
+    {error, Reason} =
+	ssh:connect(Host, Port, [{silently_accept_hosts, true},
+				 {user, "bandit"},
+				 {password, "pwd breaking"},
+				 {user_interaction, false},
+				 {user_dir, UserDir}]),
+    ssh:stop_daemon(Pid).
+
+    
+%%--------------------------------------------------------------------
+server_pwdfun_4_option_repeat(Config) ->
+    PrivDir = ?config(priv_dir, Config),
+    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
+    file:make_dir(UserDir),
+    SysDir = ?config(data_dir, Config),	  
+    %% Test that the state works
+    Parent = self(),
+    PWDFUN = fun("foo",P="bar",_,S) -> Parent!{P,S},true; 
+		(_,P,_,S=undefined) -> Parent!{P,S},{false,1}; 
+		(_,P,_,S) -> Parent!{P,S},          {false,S+1}
+	     end,
+    {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir},
+					     {user_dir, PrivDir},
+					     {auth_methods,"keyboard-interactive"},
+					     {pwdfun,PWDFUN}]),
+
+    %% Try with passwords "incorrect", "Bad again" and finally "bar"
+    KIFFUN = fun(_,_,_) -> 
+		     K={k,self()},
+		     case get(K) of 
+			 undefined -> 
+			     put(K,1),
+			     ["incorrect"]; 
+			 2 ->
+			     put(K,3),
+			     ["bar"];
+			 S->
+			     put(K,S+1),
+			     ["Bad again"]
+		     end
+	     end,
+    
+    ConnectionRef2 = 
+	ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true},
+					  {user, "foo"},
+					  {keyboard_interact_fun, KIFFUN},
+					  {user_dir, UserDir}]),
+    ssh:close(ConnectionRef2),
+    ssh:stop_daemon(Pid),
+
+    lists:foreach(fun(Expect) ->
+			  receive
+			      Expect -> ok;
+			      Other -> ct:fail("Expect: ~p~nReceived ~p",[Expect,Other])
+			  after
+			      2000 -> ct:fail("Timeout expecting ~p",[Expect])
+			  end
+		  end, [{"incorrect",undefined},
+			{"Bad again",1},
+			{"bar",2}]).
+			
+%%--------------------------------------------------------------------
 system_dir_option(Config) ->
     DirUnread = proplists:get_value(unreadable_dir,Config),
     FileRead = proplists:get_value(readable_file,Config),