From 4c3e66d5969e1abe2c8827b756edd860555038a8 Mon Sep 17 00:00:00 2001
From: Hans Nilsson <hans@erlang.org>
Date: Wed, 31 Oct 2018 15:34:39 +0100
Subject: ssh: Cleaning and polishing of ssh_auth

No intentional api changes. Only to make the code less hard to read.
---
 lib/ssh/src/ssh.hrl                    |  2 -
 lib/ssh/src/ssh_auth.erl               | 94 +++++++++++++++++++++-------------
 lib/ssh/src/ssh_connection_handler.erl | 10 ++--
 lib/ssh/src/ssh_transport.erl          |  5 +-
 4 files changed, 66 insertions(+), 45 deletions(-)

diff --git a/lib/ssh/src/ssh.hrl b/lib/ssh/src/ssh.hrl
index b0a35e0eab..aa4b3ef098 100644
--- a/lib/ssh/src/ssh.hrl
+++ b/lib/ssh/src/ssh.hrl
@@ -380,8 +380,6 @@
 
 	  algorithms,   %% #alg{}
 	  
-	  io_cb,        %% Interaction callback module
-
 	  send_mac = none, %% send MAC algorithm
 	  send_mac_key,  %% key used in send MAC algorithm
 	  send_mac_size = 0,
diff --git a/lib/ssh/src/ssh_auth.erl b/lib/ssh/src/ssh_auth.erl
index a2f016a837..1972468b8b 100644
--- a/lib/ssh/src/ssh_auth.erl
+++ b/lib/ssh/src/ssh_auth.erl
@@ -91,8 +91,10 @@ unique(L) ->
     
 
 %%%---- userauth_request_msg "callbacks"
-password_msg([#ssh{opts = Opts, io_cb = IoCb,
-		   user = User, service = Service} = Ssh0]) ->
+password_msg([#ssh{opts = Opts,
+		   user = User,
+                   service = Service} = Ssh0]) ->
+    IoCb = ?GET_INTERNAL_OPT(io_cb, Opts),
     {Password,Ssh} = 
 	case ?GET_OPT(password, Opts) of
 	    undefined when IoCb == ssh_no_io ->
@@ -385,11 +387,9 @@ handle_userauth_info_request(#ssh_msg_userauth_info_request{name = Name,
 							    instruction = Instr,
 							    num_prompts = NumPrompts,
 							    data  = Data},
-			     #ssh{opts = Opts,
-				  io_cb = IoCb
-				 } = Ssh) ->
+			     #ssh{opts=Opts} = Ssh) ->
     PromptInfos = decode_keyboard_interactive_prompts(NumPrompts,Data),
-    case keyboard_interact_get_responses(IoCb, Opts, Name, Instr, PromptInfos) of
+    case keyboard_interact_get_responses(Opts, Name, Instr, PromptInfos) of
 	not_ok ->
 	    not_ok;
 	Responses ->
@@ -530,56 +530,78 @@ build_sig_data(SessionId, User, Service, KeyBlob, Alg) ->
 
 
 
+key_alg('rsa-sha2-256') -> 'ssh-rsa';
+key_alg('rsa-sha2-512') -> 'ssh-rsa';
+key_alg(Alg) -> Alg.
+
+%%%================================================================
+%%%
+%%% Keyboard-interactive
+%%% 
+
 decode_keyboard_interactive_prompts(_NumPrompts, Data) ->
     ssh_message:decode_keyboard_interactive_prompts(Data, []).
 
-keyboard_interact_get_responses(IoCb, Opts, Name, Instr, PromptInfos) ->
-    NumPrompts = length(PromptInfos),
+keyboard_interact_get_responses(Opts, Name, Instr, PromptInfos) ->
     keyboard_interact_get_responses(?GET_OPT(user_interaction, Opts),
 				    ?GET_OPT(keyboard_interact_fun, Opts),
-				    ?GET_OPT(password, Opts), IoCb, Name,
-				    Instr, PromptInfos, Opts, NumPrompts).
+				    ?GET_OPT(password, Opts),
+                                    Name,
+				    Instr,
+                                    PromptInfos,
+                                    Opts).
 
 
-keyboard_interact_get_responses(_, _, not_ok, _, _, _, _, _, _) ->
+%% Don't re-try an already rejected password. This could happen if both keyboard-interactive
+%% and password methods are tried:
+keyboard_interact_get_responses(_, _, not_ok, _, _, _, _) ->
     not_ok;
-keyboard_interact_get_responses(_, undefined, Password, _, _, _, _, _,
-				1) when Password =/= undefined ->
-    [Password]; %% Password auth implemented with keyboard-interaction and passwd is known
-keyboard_interact_get_responses(_, _, _, _, _, _, _, _, 0)  ->
+
+%% Only one password requestedm and we have got one via the 'password' option for the daemon:
+keyboard_interact_get_responses(_, undefined, Pwd, _, _, [_], _) when Pwd =/= undefined ->
+    [Pwd]; %% Password auth implemented with keyboard-interaction and passwd is known
+
+%% No password requested (keyboard-interactive):
+keyboard_interact_get_responses(_, _, _, _, _, [], _)  ->
     [];
-keyboard_interact_get_responses(false, undefined, undefined, _, _, _, [Prompt|_], Opts, _) ->
-    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, _Pwd, _IoCb, Name, Instr, PromptInfos, _Opts, NumPrompts) ->
-    keyboard_interact_fun(Fun, Name, Instr, PromptInfos, NumPrompts).
-
-keyboard_interact(IoCb, Name, Instr, Prompts, Opts) ->
+
+%% user_interaction is forbidden (by option user_interaction) and we have to ask
+%% the user for one or more.
+%% Throw an error:
+keyboard_interact_get_responses(false, undefined, undefined, _, _, [Prompt|_], Opts) ->
+    ssh_no_io:read_line(Prompt, Opts);
+
+%% One or more passwords are requested, we may prompt the user and no fun is used
+%% to get the responses:
+keyboard_interact_get_responses(true, undefined, _, Name, Instr, PromptInfos, Opts) ->
+    prompt_user_for_passwords(Name, Instr, PromptInfos, Opts);
+
+%% The passwords are provided with a fun. Use that one!
+keyboard_interact_get_responses(true, Fun, _Pwd, Name, Instr, PromptInfos, _Opts) ->
+    keyboard_interact_fun(Fun, Name, Instr, PromptInfos).
+
+
+
+prompt_user_for_passwords(Name, Instr, PromptInfos, Opts) ->
+    IoCb = ?GET_INTERNAL_OPT(io_cb, Opts),
     write_if_nonempty(IoCb, Name),
     write_if_nonempty(IoCb, Instr),
     lists:map(fun({Prompt, true})  -> IoCb:read_line(Prompt, Opts);
 		 ({Prompt, false}) -> IoCb:read_password(Prompt, Opts)
 	      end,
-	      Prompts).
-
-write_if_nonempty(_, "") -> ok;
-write_if_nonempty(_, <<>>) -> ok;
-write_if_nonempty(IoCb, Text) -> IoCb:format("~s~n",[Text]).
-
+	      PromptInfos).
 
-keyboard_interact_fun(KbdInteractFun, Name, Instr,  PromptInfos, NumPrompts) ->
-    Prompts = lists:map(fun({Prompt, _Echo}) -> Prompt end,
-			PromptInfos),
+keyboard_interact_fun(KbdInteractFun, Name, Instr,  PromptInfos) ->
+    Prompts = lists:map(fun({Prompt,_Echo}) -> Prompt end,  PromptInfos),
     case KbdInteractFun(Name, Instr, Prompts) of
-	Rs when length(Rs) == NumPrompts ->
+	Rs when length(Rs) == length(PromptInfos) ->
 	    Rs;
 	_Rs ->
             nok
     end.
 
 
-key_alg('rsa-sha2-256') -> 'ssh-rsa';
-key_alg('rsa-sha2-512') -> 'ssh-rsa';
-key_alg(Alg) -> Alg.
+write_if_nonempty(_, "") -> ok;
+write_if_nonempty(_, <<>>) -> ok;
+write_if_nonempty(IoCb, Text) -> IoCb:format("~s~n",[Text]).
 
diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl
index b64b799327..e23df6ceca 100644
--- a/lib/ssh/src/ssh_connection_handler.erl
+++ b/lib/ssh/src/ssh_connection_handler.erl
@@ -471,10 +471,11 @@ init_ssh_record(Role, Socket, PeerAddr, Opts) ->
             S1 =
                 S0#ssh{c_vsn = Vsn,
                        c_version = Version,
-                       io_cb = case ?GET_OPT(user_interaction, Opts) of
-                                   true ->  ssh_io;
-                                   false -> ssh_no_io
-                               end,
+                       opts = ?PUT_INTERNAL_OPT({io_cb, case ?GET_OPT(user_interaction, Opts) of
+                                                            true ->  ssh_io;
+                                                            false -> ssh_no_io
+                                                        end},
+                                                Opts),
                        userauth_quiet_mode = ?GET_OPT(quiet_mode, Opts),
                        peer = {PeerName, PeerAddr},
                        local = LocalName
@@ -487,7 +488,6 @@ init_ssh_record(Role, Socket, PeerAddr, Opts) ->
 	server ->
 	    S0#ssh{s_vsn = Vsn,
 		   s_version = Version,
-		   io_cb = ?GET_INTERNAL_OPT(io_cb, Opts, ssh_io),
 		   userauth_methods = string:tokens(AuthMethods, ","),
 		   kb_tries_left = 3,
 		   peer = {undefined, PeerAddr},
diff --git a/lib/ssh/src/ssh_transport.erl b/lib/ssh/src/ssh_transport.erl
index 96b03abb94..f49b49b2df 100644
--- a/lib/ssh/src/ssh_transport.erl
+++ b/lib/ssh/src/ssh_transport.erl
@@ -861,8 +861,9 @@ accepted_host(Ssh, PeerName, Public, Opts) ->
     end.
 
 
-yes_no(Ssh, Prompt)  ->
-    (Ssh#ssh.io_cb):yes_no(Prompt, Ssh#ssh.opts).
+yes_no(#ssh{opts=Opts}, Prompt)  ->
+    IoCb = ?GET_INTERNAL_OPT(io_cb, Opts, ssh_io),
+    IoCb:yes_no(Prompt, Opts).
 
 
 fmt_hostkey('ssh-rsa') -> "RSA";
-- 
cgit v1.2.3