Merge branch 'hans/ssh/check_auth_methods_server/OTP-12790'

* hans/ssh/check_auth_methods_server/OTP-12790:
  ssh: change pwd->password for keyboard-interactive
  ssh save keybard-interactive data in #ssh{}
  ssh: make server check auth_methods when rec request

3 files changed, 105 insertions, 78 deletions

diff --git a/lib/ssh/src/ssh.hrl b/lib/ssh/src/ssh.hrl
index 94154c8a96..a02c87505d 100644
--- a/lib/ssh/src/ssh.hrl
+++ b/lib/ssh/src/ssh.hrl
@@ -128,8 +128,10 @@
 	  user,
 	  service,
 	  userauth_quiet_mode,              %  boolean()
-	  userauth_supported_methods , %  
-	  userauth_methods,
+	  userauth_supported_methods,       %  string() eg "keyboard-interactive,password"
+	  userauth_methods,                 %  list( string() )  eg ["keyboard-interactive", "password"]
+	  kb_tries_left = 0,                %  integer(), num tries left for "keyboard-interactive"
+	  kb_data,
 	  userauth_preference,
 	  available_host_keys,
 	  authenticated = false
diff --git a/lib/ssh/src/ssh_auth.erl b/lib/ssh/src/ssh_auth.erl
index df9a97c8f8..58faee062e 100644
--- a/lib/ssh/src/ssh_auth.erl
+++ b/lib/ssh/src/ssh_auth.erl
@@ -169,7 +169,8 @@ handle_userauth_request(#ssh_msg_userauth_request{user = User,
 						  service = "ssh-connection",
 						  method = "password",
 						  data = <<?FALSE, ?UINT32(Sz), BinPwd:Sz/binary>>}, _, 
-			#ssh{opts = Opts} = Ssh) ->
+			#ssh{opts = Opts,
+			     userauth_supported_methods = Methods} = Ssh) ->
     Password = unicode:characters_to_list(BinPwd),
     case check_password(User, Password, Opts) of
 	true ->
@@ -178,7 +179,7 @@ handle_userauth_request(#ssh_msg_userauth_request{user = User,
 	false  ->
 	    {not_authorized, {User, {error,"Bad user or password"}}, 
 	     ssh_transport:ssh_packet(#ssh_msg_userauth_failure{
-		     authentications = "",
+		     authentications = Methods,
 		     partial_success = false}, Ssh)}
     end;
 
@@ -191,7 +192,7 @@ handle_userauth_request(#ssh_msg_userauth_request{user = User,
 							   %% ?UINT32(Sz2), NewBinPwd:Sz2/binary
 							 >>
 						 }, _, 
-			Ssh) ->
+			#ssh{userauth_supported_methods = Methods} = Ssh) ->
     %% Password change without us having sent SSH_MSG_USERAUTH_PASSWD_CHANGEREQ (because we never do)
     %% RFC 4252 says:
     %%   SSH_MSG_USERAUTH_FAILURE without partial success - The password
@@ -200,7 +201,7 @@ handle_userauth_request(#ssh_msg_userauth_request{user = User,
 
     {not_authorized, {User, {error,"Password change not supported"}}, 
      ssh_transport:ssh_packet(#ssh_msg_userauth_failure{
-				 authentications = "",
+				 authentications = Methods,
 				 partial_success = false}, Ssh)};
 
 handle_userauth_request(#ssh_msg_userauth_request{user = User,
@@ -216,7 +217,9 @@ handle_userauth_request(#ssh_msg_userauth_request{user = User,
 						  service = "ssh-connection",
 						  method = "publickey",
 						  data = Data}, 
-			SessionId, #ssh{opts = Opts} = Ssh) ->
+			SessionId, 
+			#ssh{opts = Opts,
+			     userauth_supported_methods = Methods} = Ssh) ->
     <<?BYTE(HaveSig), ?UINT32(ALen), BAlg:ALen/binary, 
      ?UINT32(KLen), KeyBlob:KLen/binary, SigWLen/binary>> = Data,
     Alg = binary_to_list(BAlg),
@@ -231,7 +234,7 @@ handle_userauth_request(#ssh_msg_userauth_request{user = User,
 		false ->
 		    {not_authorized, {User, undefined}, 
 		     ssh_transport:ssh_packet(#ssh_msg_userauth_failure{
-			     authentications="publickey,password",
+			     authentications = Methods,
 			     partial_success = false}, Ssh)}
 	    end;
 	?FALSE ->
@@ -245,49 +248,60 @@ handle_userauth_request(#ssh_msg_userauth_request{user = User,
 						  service = "ssh-connection",
 						  method = "keyboard-interactive",
 						  data = _},
-			_, #ssh{opts = Opts} = Ssh) ->
-    %% RFC4256
-    %% The data field contains:
-    %%   - language tag (deprecated). If =/=[] SHOULD use it however. We skip
-    %%                                it for simplicity.
-    %%   - submethods. "... the user can give a hint of which actual methods
-    %%                  he wants to use. ...".  It's a "MAY use" so we skip
-    %%                  it. It also needs an understanding between the client
-    %%                  and the server.
-    %%                  
-    %% "The server MUST reply with an SSH_MSG_USERAUTH_SUCCESS,
-    %%  SSH_MSG_USERAUTH_FAILURE, or SSH_MSG_USERAUTH_INFO_REQUEST message."
-    Default = {"SSH server",
-	       "Enter password for \""++User++"\"",
-	       "pwd: ",
-	       false},
-
-    {Name, Instruction, Prompt, Echo} =
-	case proplists:get_value(auth_method_kb_interactive_data, Opts) of
-	    undefined -> 
-		Default;
-	    {_,_,_,_}=V -> 
-		V;
-	    F when is_function(F) ->
-		{_,PeerName} = Ssh#ssh.peer,
-		F(PeerName, User, "ssh-connection")
-	end,
-    EchoEnc = case Echo of
-		  true -> <<?TRUE>>;
-		  false -> <<?FALSE>>
-	      end,
-    Msg = #ssh_msg_userauth_info_request{name = unicode:characters_to_list(Name),
-					 instruction = unicode:characters_to_list(Instruction),
-					 language_tag = "",
-					 num_prompts = 1,
-					 data = <<?STRING(unicode:characters_to_binary(Prompt)),
-						  EchoEnc/binary
-						>>
-					},
-    {not_authorized, {User, undefined}, 
-     ssh_transport:ssh_packet(Msg, Ssh#ssh{user = User,
-					   opts = [{max_kb_tries,3},{kb_userauth_info_msg,Msg}|Opts]
-					  })};
+			_, #ssh{opts = Opts,
+				kb_tries_left = KbTriesLeft,
+				userauth_supported_methods = Methods} = Ssh) ->
+    case KbTriesLeft of
+	N when N<1 ->
+	    {not_authorized, {User, {authmethod, "keyboard-interactive"}}, 
+	     ssh_transport:ssh_packet(
+	       #ssh_msg_userauth_failure{authentications = Methods,
+					 partial_success = false}, Ssh)};
+
+	_ ->
+	    %% RFC4256
+	    %% The data field contains:
+	    %%   - language tag (deprecated). If =/=[] SHOULD use it however. We skip
+	    %%                                it for simplicity.
+	    %%   - submethods. "... the user can give a hint of which actual methods
+	    %%                  he wants to use. ...".  It's a "MAY use" so we skip
+	    %%                  it. It also needs an understanding between the client
+	    %%                  and the server.
+	    %%                  
+	    %% "The server MUST reply with an SSH_MSG_USERAUTH_SUCCESS,
+	    %%  SSH_MSG_USERAUTH_FAILURE, or SSH_MSG_USERAUTH_INFO_REQUEST message."
+	    Default = {"SSH server",
+		       "Enter password for \""++User++"\"",
+		       "password: ",
+		       false},
+
+	    {Name, Instruction, Prompt, Echo} =
+		case proplists:get_value(auth_method_kb_interactive_data, Opts) of
+		    undefined -> 
+			Default;
+		    {_,_,_,_}=V -> 
+			V;
+		    F when is_function(F) ->
+			{_,PeerName} = Ssh#ssh.peer,
+			F(PeerName, User, "ssh-connection")
+		end,
+	    EchoEnc = case Echo of
+			  true -> <<?TRUE>>;
+			  false -> <<?FALSE>>
+		      end,
+	    Msg = #ssh_msg_userauth_info_request{name = unicode:characters_to_list(Name),
+						 instruction = unicode:characters_to_list(Instruction),
+						 language_tag = "",
+						 num_prompts = 1,
+						 data = <<?STRING(unicode:characters_to_binary(Prompt)),
+							  EchoEnc/binary
+							>>
+						},
+	    {not_authorized, {User, undefined}, 
+	     ssh_transport:ssh_packet(Msg, Ssh#ssh{user = User,
+						   kb_data = Msg
+						  })}
+    end;
 
 handle_userauth_request(#ssh_msg_userauth_request{user = User,
 						  service = "ssh-connection",
@@ -314,33 +328,37 @@ handle_userauth_info_request(
 
 handle_userauth_info_response(#ssh_msg_userauth_info_response{num_responses = 1,
 							      data = <<?UINT32(Sz), Password:Sz/binary>>},
-			      #ssh{opts = Opts0,
-				   user = User} = Ssh) ->
-    NumTriesLeft = proplists:get_value(max_kb_tries, Opts0, 0) - 1,
-    Opts = lists:keydelete(max_kb_tries,1,Opts0),
+			      #ssh{opts = Opts,
+				   kb_tries_left = KbTriesLeft0,
+				   kb_data = InfoMsg,
+				   user = User,
+				   userauth_supported_methods = Methods} = Ssh) ->
+    KbTriesLeft = KbTriesLeft0 - 1,
     case check_password(User, unicode:characters_to_list(Password), Opts) of
 	true ->
 	    {authorized, User,
 	     ssh_transport:ssh_packet(#ssh_msg_userauth_success{}, Ssh)};
-	false when NumTriesLeft > 0 ->
+	false when KbTriesLeft > 0 ->
 	    UserAuthInfoMsg = 
-		(proplists:get_value(kb_userauth_info_msg,Opts))
-		#ssh_msg_userauth_info_request{name = "",
-					       instruction = 
-						   lists:concat(
-						     ["Bad user or password, try again. ",
-						      integer_to_list(NumTriesLeft),
-						      " tries left."])},
+		InfoMsg#ssh_msg_userauth_info_request{
+		  name = "",
+		  instruction = 
+		      lists:concat(
+			["Bad user or password, try again. ",
+			 integer_to_list(KbTriesLeft),
+			 " tries left."])
+		 },
 	    {not_authorized, {User, undefined}, 
 	     ssh_transport:ssh_packet(UserAuthInfoMsg,
-				      Ssh#ssh{opts = [{max_kb_tries,NumTriesLeft}|Opts]})};
+				      Ssh#ssh{kb_tries_left = KbTriesLeft})};
 	     
 	false ->
 	    {not_authorized, {User, {error,"Bad user or password"}}, 
 	     ssh_transport:ssh_packet(#ssh_msg_userauth_failure{
-					 authentications = "",
+					 authentications = Methods,
 					 partial_success = false}, 
-				      Ssh#ssh{opts = lists:keydelete(kb_userauth_info_msg,1,Opts)}
+				      Ssh#ssh{kb_data = undefined,
+					      kb_tries_left = 0}
 				     )}
     end;
 
diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl
index ab1fc93a1b..d8bf66d2a8 100644
--- a/lib/ssh/src/ssh_connection_handler.erl
+++ b/lib/ssh/src/ssh_connection_handler.erl
@@ -483,17 +483,22 @@ userauth(#ssh_msg_userauth_request{service = "ssh-connection",
 				  service = "ssh-connection",
 				  peer = {_, Address}} = Ssh0,
 		opts = Opts, starter = Pid} = State) ->
-    case ssh_auth:handle_userauth_request(Msg, SessionId, Ssh0) of
-	{authorized, User, {Reply, Ssh}} ->
-	    send_msg(Reply, State),
-	    Pid ! ssh_connected,
-	    connected_fun(User, Address, Method, Opts),
-	    {next_state, connected, 
-	     next_packet(State#state{auth_user = User, ssh_params = Ssh})};
-	{not_authorized, {User, Reason}, {Reply, Ssh}} ->
-	    retry_fun(User, Address, Reason, Opts),
-	    send_msg(Reply, State),
-	    {next_state, userauth, next_packet(State#state{ssh_params = Ssh})} 
+    case lists:member(Method, Ssh0#ssh.userauth_methods) of
+	true ->
+	    case ssh_auth:handle_userauth_request(Msg, SessionId, Ssh0) of
+		{authorized, User, {Reply, Ssh}} ->
+		    send_msg(Reply, State),
+		    Pid ! ssh_connected,
+		    connected_fun(User, Address, Method, Opts),
+		    {next_state, connected, 
+		     next_packet(State#state{auth_user = User, ssh_params = Ssh})};
+		{not_authorized, {User, Reason}, {Reply, Ssh}} ->
+		    retry_fun(User, Address, Reason, Opts),
+		    send_msg(Reply, State),
+		    {next_state, userauth, next_packet(State#state{ssh_params = Ssh})} 
+	    end;
+	false ->
+	    userauth(Msg#ssh_msg_userauth_request{method="none"}, State)
     end;
 
 userauth(#ssh_msg_userauth_info_request{} = Msg, 
@@ -1148,9 +1153,9 @@ init_ssh(client = Role, Vsn, Version, Options, Socket) ->
 	};
 
 init_ssh(server = Role, Vsn, Version, Options, Socket) ->
-
     AuthMethods = proplists:get_value(auth_methods, Options, 
 				      ?SUPPORTED_AUTH_METHODS),
+    AuthMethodsAsList = string:tokens(AuthMethods, ","),
     {ok, PeerAddr} = inet:peername(Socket),
     KeyCb =  proplists:get_value(key_cb, Options, ssh_file),
 
@@ -1161,6 +1166,8 @@ init_ssh(server = Role, Vsn, Version, Options, Socket) ->
 	 io_cb = proplists:get_value(io_cb, Options, ssh_io),
 	 opts = Options,
 	 userauth_supported_methods = AuthMethods,
+	 userauth_methods = AuthMethodsAsList,
+	 kb_tries_left = 3,
 	 peer = {undefined, PeerAddr},
 	 available_host_keys = supported_host_keys(Role, KeyCb, Options)
 	 }.