Merge branch 'hans/ssh/robust_pubkey_prechecks/OTP-15424' into maint

* hans/ssh/robust_pubkey_prechecks/OTP-15424:
  ssh: Make host and user key pre-checking better
  ssh: Generalize unpublished test support option
  ssh: Cleaning and polishing of ssh_auth
  ssh: Add a common key_cb call function

5 files changed, 129 insertions, 111 deletions

diff --git a/lib/ssh/src/ssh.hrl b/lib/ssh/src/ssh.hrl
index f645201c4f..3e9b1add3b 100644
--- a/lib/ssh/src/ssh.hrl
+++ b/lib/ssh/src/ssh.hrl
@@ -240,7 +240,12 @@
       | ?COMMON_OPTION .
 
 -type opaque_client_options() ::
-        {keyboard_interact_fun, fun((term(),term(),term()) -> term())}
+        {keyboard_interact_fun, fun((Name::iodata(),
+                                     Instruction::iodata(),
+                                     Prompts::[{Prompt::iodata(),Echo::boolean()}]
+                                    ) ->
+                                      [Response::iodata()]
+                                   )} 
         | opaque_common_options().
 
 -type host_accepting_client_options() ::
@@ -380,9 +385,6 @@
 
 	  algorithms,   %% #alg{}
 	  
-	  key_cb,       %% Private/Public key callback module
-	  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 4e4aa440de..9632168e65 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 ->
@@ -137,9 +139,7 @@ keyboard_interactive_msg([#ssh{user = User,
 
 get_public_key(SigAlg, #ssh{opts = Opts}) ->
     KeyAlg = key_alg(SigAlg),
-    {KeyCb,KeyCbOpts} = ?GET_OPT(key_cb, Opts),
-    UserOpts = ?GET_OPT(user_options, Opts),
-    case KeyCb:user_key(KeyAlg, [{key_cb_private,KeyCbOpts}|UserOpts]) of
+    case ssh_transport:call_KeyCb(user_key, [KeyAlg], Opts) of
         {ok, PrivKey} ->
             try
                 %% Check the key - the KeyCb may be a buggy plugin
@@ -387,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 ->
@@ -498,9 +496,7 @@ get_password_option(Opts, User) ->
 pre_verify_sig(User, KeyBlob, Opts) ->
     try
 	Key = public_key:ssh_decode(KeyBlob, ssh2_pubkey), % or exception
-        {KeyCb,KeyCbOpts} = ?GET_OPT(key_cb, Opts),
-        UserOpts = ?GET_OPT(user_options, Opts),
-        KeyCb:is_auth_key(Key, User, [{key_cb_private,KeyCbOpts}|UserOpts])
+        ssh_transport:call_KeyCb(is_auth_key, [Key, User], Opts)
     catch
 	_:_ ->
 	    false
@@ -509,10 +505,8 @@ pre_verify_sig(User, KeyBlob, Opts) ->
 verify_sig(SessionId, User, Service, AlgBin, KeyBlob, SigWLen, #ssh{opts = Opts} = Ssh) ->
     try
         Alg = binary_to_list(AlgBin),
-        {KeyCb,KeyCbOpts} = ?GET_OPT(key_cb, Opts),
-        UserOpts = ?GET_OPT(user_options, Opts),
         Key = public_key:ssh_decode(KeyBlob, ssh2_pubkey), % or exception
-        true = KeyCb:is_auth_key(Key, User, [{key_cb_private,KeyCbOpts}|UserOpts]),
+        true = ssh_transport:call_KeyCb(is_auth_key, [Key, User], Opts),
         PlainText = build_sig_data(SessionId, User, Service, KeyBlob, Alg),
         <<?UINT32(AlgSigLen), AlgSig:AlgSigLen/binary>> = SigWLen,
         <<?UINT32(AlgLen), _Alg:AlgLen/binary,
@@ -536,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).
+	      PromptInfos).
 
-write_if_nonempty(_, "") -> ok;
-write_if_nonempty(_, <<>>) -> ok;
-write_if_nonempty(IoCb, Text) -> IoCb:format("~s~n",[Text]).
-
-
-keyboard_interact_fun(KbdInteractFun, Name, Instr,  PromptInfos, NumPrompts) ->
-    Prompts = lists:map(fun({Prompt, _Echo}) -> Prompt end,
-			PromptInfos),
-    case KbdInteractFun(Name, Instr, Prompts) of
-	Rs when length(Rs) == NumPrompts ->
-	    Rs;
-	_Rs ->
+keyboard_interact_fun(KbdInteractFun, Name, Instr,  PromptInfos) ->
+    case KbdInteractFun(Name, Instr, PromptInfos) of
+	Responses when is_list(Responses),
+                     length(Responses) == length(PromptInfos) ->
+	    Responses;
+	_ ->
             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 30eafc2f2a..7c87591cf2 100644
--- a/lib/ssh/src/ssh_connection_handler.erl
+++ b/lib/ssh/src/ssh_connection_handler.erl
@@ -447,7 +447,6 @@ init_ssh_record(Role, Socket, Opts) ->
 init_ssh_record(Role, Socket, PeerAddr, Opts) ->
     AuthMethods = ?GET_OPT(auth_methods, Opts),
     S0 = #ssh{role = Role,
-	      key_cb = ?GET_OPT(key_cb, Opts),
 	      opts = Opts,
 	      userauth_supported_methods = AuthMethods,
 	      available_host_keys = available_hkey_algorithms(Role, Opts),
@@ -472,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
@@ -488,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},
@@ -1686,18 +1685,19 @@ peer_role(client) -> server;
 peer_role(server) -> client.
 
 %%--------------------------------------------------------------------
-available_hkey_algorithms(Role, Options) ->
-    KeyCb = ?GET_OPT(key_cb, Options),
-    case [A || A <- available_hkey_algos(Options),
-               (Role==client) orelse available_host_key(KeyCb, A, Options)
-         ] of
-        
-        [] when Role==client ->
-	    error({shutdown, "No public key algs"});
-
-        [] when Role==server ->
-	    error({shutdown, "No host key available"});
+available_hkey_algorithms(client, Options) ->
+    case available_hkey_algos(Options) of
+        [] ->
+            error({shutdown, "No public key algs"});
+        Algs ->
+	    [atom_to_list(A) || A<-Algs]
+    end;
 
+available_hkey_algorithms(server, Options) ->
+    case [A || A <- available_hkey_algos(Options),
+               is_usable_host_key(A, Options)] of
+        [] ->
+            error({shutdown, "No host key available"});
 	Algs ->
 	    [atom_to_list(A) || A<-Algs]
     end.
@@ -1713,18 +1713,6 @@ available_hkey_algos(Options) ->
     AvailableAndSupported.
 
 
-%% Alg :: atom()
-available_host_key({KeyCb,KeyCbOpts}, Alg, Opts) ->
-    UserOpts = ?GET_OPT(user_options, Opts),
-    case KeyCb:host_key(Alg, [{key_cb_private,KeyCbOpts}|UserOpts]) of
-        {ok,Key} ->
-            %% Check the key - the KeyCb may be a buggy plugin
-            ssh_transport:valid_key_sha_alg(Key, Alg);
-        _ ->
-            false
-    end.
-
-
 send_msg(Msg, State=#data{ssh_params=Ssh0}) when is_tuple(Msg) ->
     {Bytes, Ssh} = ssh_transport:ssh_packet(Msg, Ssh0),
     send_bytes(Bytes, State),
@@ -1844,10 +1832,21 @@ ext_info(_, D0) ->
     D0.
 
 %%%----------------------------------------------------------------
-is_usable_user_pubkey(A, Ssh) ->
-    case ssh_auth:get_public_key(A, Ssh) of
+is_usable_user_pubkey(Alg, Ssh) ->
+    try ssh_auth:get_public_key(Alg, Ssh) of
         {ok,_} -> true;
         _ -> false
+    catch
+        _:_ -> false
+    end.
+
+%%%----------------------------------------------------------------
+is_usable_host_key(Alg, Opts) ->
+    try ssh_transport:get_host_key(Alg, Opts)
+    of
+        _PrivHostKey -> true
+    catch
+        _:_ -> false
     end.
 
 %%%----------------------------------------------------------------
diff --git a/lib/ssh/src/ssh_file.erl b/lib/ssh/src/ssh_file.erl
index 669b0f9be2..c5824b2519 100644
--- a/lib/ssh/src/ssh_file.erl
+++ b/lib/ssh/src/ssh_file.erl
@@ -268,7 +268,8 @@ identity_pass_phrase("rsa-sha2-384"  ) -> rsa_pass_phrase;
 identity_pass_phrase("rsa-sha2-512"  ) -> rsa_pass_phrase;
 identity_pass_phrase("ecdsa-sha2-"++_) -> ecdsa_pass_phrase;
 identity_pass_phrase(P) when is_atom(P) -> 
-    identity_pass_phrase(atom_to_list(P)).
+    identity_pass_phrase(atom_to_list(P));
+identity_pass_phrase(_) -> undefined.
     
 lookup_host_key_fd(Fd, KeyToMatch, Host, KeyType) ->
     case io:get_line(Fd, '') of
diff --git a/lib/ssh/src/ssh_transport.erl b/lib/ssh/src/ssh_transport.erl
index 7424c9bcaf..cc3ef46fc2 100644
--- a/lib/ssh/src/ssh_transport.erl
+++ b/lib/ssh/src/ssh_transport.erl
@@ -51,7 +51,9 @@
 	 extract_public_key/1,
 	 ssh_packet/2, pack/2,
          valid_key_sha_alg/2,
-	 sha/1, sign/3, verify/5]).
+	 sha/1, sign/3, verify/5,
+         get_host_key/2,
+         call_KeyCb/3]).
 
 -export([dbg_trace/3]).
 
@@ -431,7 +433,8 @@ key_exchange_first_msg(Kex, Ssh0) when Kex == 'ecdh-sha2-nistp256' ;
 %%% 
 handle_kexdh_init(#ssh_msg_kexdh_init{e = E}, 
 		  Ssh0 = #ssh{algorithms = #alg{kex=Kex,
-                                                hkey=SignAlg} = Algs}) ->
+                                                hkey=SignAlg} = Algs,
+                              opts = Opts}) ->
     %% server
     {G, P} = dh_group(Kex),
     if
@@ -439,7 +442,7 @@ handle_kexdh_init(#ssh_msg_kexdh_init{e = E},
             Sz = dh_bits(Algs),
 	    {Public, Private} = generate_key(dh, [P,G,2*Sz]),
 	    K = compute_key(dh, E, Private, [P,G]),
-	    MyPrivHostKey = get_host_key(Ssh0, SignAlg),
+	    MyPrivHostKey = get_host_key(SignAlg, Opts),
 	    MyPubHostKey = extract_public_key(MyPrivHostKey),
             H = kex_hash(Ssh0, MyPubHostKey, sha(Kex), {E,Public,K}),
             H_SIG = sign(H, sha(SignAlg), MyPrivHostKey),
@@ -578,14 +581,15 @@ handle_kex_dh_gex_init(#ssh_msg_kex_dh_gex_init{e = E},
 		       #ssh{keyex_key = {{Private, Public}, {G, P}},
 			    keyex_info = {Min, Max, NBits},
                             algorithms = #alg{kex=Kex,
-                                              hkey=SignAlg}} = Ssh0) ->
+                                              hkey=SignAlg},
+                            opts = Opts} = Ssh0) ->
     %% server
     if
 	1=<E, E=<(P-1) ->
 	    K = compute_key(dh, E, Private, [P,G]),
 	    if
 		1<K, K<(P-1) ->
-		    MyPrivHostKey = get_host_key(Ssh0, SignAlg),
+		    MyPrivHostKey = get_host_key(SignAlg, Opts),
 		    MyPubHostKey = extract_public_key(MyPrivHostKey),
                     H = kex_hash(Ssh0, MyPubHostKey, sha(Kex), {Min,NBits,Max,P,G,E,Public,K}),
                     H_SIG = sign(H, sha(SignAlg), MyPrivHostKey),
@@ -653,7 +657,8 @@ handle_kex_dh_gex_reply(#ssh_msg_kex_dh_gex_reply{public_host_key = PeerPubHostK
 %%% 
 handle_kex_ecdh_init(#ssh_msg_kex_ecdh_init{q_c = PeerPublic},
 		     Ssh0 = #ssh{algorithms = #alg{kex=Kex,
-                                                   hkey=SignAlg}}) ->
+                                                   hkey=SignAlg},
+                                 opts = Opts}) ->
     %% at server
     Curve = ecdh_curve(Kex),
     {MyPublic, MyPrivate} = generate_key(ecdh, Curve),
@@ -661,7 +666,7 @@ handle_kex_ecdh_init(#ssh_msg_kex_ecdh_init{q_c = PeerPublic},
 	compute_key(ecdh, PeerPublic, MyPrivate, Curve)
     of
 	K ->
-	    MyPrivHostKey = get_host_key(Ssh0, SignAlg),
+	    MyPrivHostKey = get_host_key(SignAlg, Opts),
 	    MyPubHostKey = extract_public_key(MyPrivHostKey),
             H = kex_hash(Ssh0, MyPubHostKey, sha(Curve), {PeerPublic, MyPublic, K}),
             H_SIG = sign(H, sha(SignAlg), MyPrivHostKey),
@@ -777,10 +782,8 @@ sid(#ssh{session_id = Id},        _) -> Id.
 %%
 %% The host key should be read from storage
 %%
-get_host_key(SSH, SignAlg) ->
-    #ssh{key_cb = {KeyCb,KeyCbOpts}, opts = Opts} = SSH,
-    UserOpts = ?GET_OPT(user_options, Opts),
-    case KeyCb:host_key(SignAlg, [{key_cb_private,KeyCbOpts}|UserOpts]) of
+get_host_key(SignAlg, Opts) ->
+    case call_KeyCb(host_key, [SignAlg], Opts) of
 	{ok, PrivHostKey} ->
             %% Check the key - the KeyCb may be a buggy plugin
             case valid_key_sha_alg(PrivHostKey, SignAlg) of
@@ -791,6 +794,11 @@ get_host_key(SSH, SignAlg) ->
             exit({error, {Result, unsupported_key_type}})
     end.
 
+call_KeyCb(F, Args, Opts) ->
+    {KeyCb,KeyCbOpts} = ?GET_OPT(key_cb, Opts),
+    UserOpts = ?GET_OPT(user_options, Opts),
+    apply(KeyCb, F, Args ++ [[{key_cb_private,KeyCbOpts}|UserOpts]]).
+
 extract_public_key(#'RSAPrivateKey'{modulus = N, publicExponent = E}) ->
     #'RSAPublicKey'{modulus = N, publicExponent = E};
 extract_public_key(#'DSAPrivateKey'{y = Y, p = P, q = Q, g = G}) ->
@@ -857,8 +865,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";
@@ -868,18 +877,16 @@ fmt_hostkey("ecdsa"++_) -> "ECDSA";
 fmt_hostkey(X) -> X.
 
 
-known_host_key(#ssh{opts = Opts, key_cb = {KeyCb,KeyCbOpts}, peer = {PeerName,_}} = Ssh, 
+known_host_key(#ssh{opts = Opts, peer = {PeerName,_}} = Ssh, 
 	       Public, Alg) ->
-    UserOpts = ?GET_OPT(user_options, Opts),
-    case is_host_key(KeyCb, Public, PeerName, Alg, [{key_cb_private,KeyCbOpts}|UserOpts]) of
-	{_,true} ->
+    case call_KeyCb(is_host_key, [Public, PeerName, Alg], Opts) of
+	true ->
 	    ok;
-	{_,false} ->
+	false ->
             DoAdd = ?GET_OPT(save_accepted_host, Opts),
 	    case accepted_host(Ssh, PeerName, Public, Opts) of
 		true when DoAdd == true ->
-		    {_,R} = add_host_key(KeyCb, PeerName, Public, [{key_cb_private,KeyCbOpts}|UserOpts]),
-                    R;
+		    call_KeyCb(add_host_key, [PeerName, Public], Opts);
 		true when DoAdd == false ->
                     ok;
 		false ->
@@ -889,13 +896,6 @@ known_host_key(#ssh{opts = Opts, key_cb = {KeyCb,KeyCbOpts}, peer = {PeerName,_}
 	    end
     end.
 	    
-is_host_key(KeyCb, Public, PeerName, Alg, Data) ->
-    {KeyCb, KeyCb:is_host_key(Public, PeerName, Alg, Data)}.
-
-add_host_key(KeyCb, PeerName, Public, Data) ->
-    {KeyCb, KeyCb:add_host_key(PeerName, Public, Data)}.
-    
-
 %%   Each of the algorithm strings MUST be a comma-separated list of
 %%   algorithm names (see ''Algorithm Naming'' in [SSH-ARCH]).  Each
 %%   supported (allowed) algorithm MUST be listed in order of preference.