Merge branch 'ia/ssh/error-handling' into maint

* ia/ssh/error-handling:
  Prevent client hanging. (OTP-8111)
  Added checks of API input
  Improve check so that we will not try to read ssh packet length indicator if not sure we have enough data.
  Improved error handling

4 files changed, 90 insertions, 44 deletions

diff --git a/lib/ssh/test/ssh_basic_SUITE.erl b/lib/ssh/test/ssh_basic_SUITE.erl
index 9c13180159..012367a6df 100644
--- a/lib/ssh/test/ssh_basic_SUITE.erl
+++ b/lib/ssh/test/ssh_basic_SUITE.erl
@@ -88,7 +88,7 @@ end_per_testcase(TestCase, Config) when TestCase == server_password_option;
     UserDir = filename:join(?config(priv_dir, Config), nopubkey),
     ssh_test_lib:del_dirs(UserDir),
     end_per_testcase(Config);
-end_per_testcase(TestCase, Config) ->
+end_per_testcase(_TestCase, Config) ->
     end_per_testcase(Config).
 end_per_testcase(_Config) ->    
     ssh:stop(),
@@ -108,14 +108,16 @@ all() ->
      {group, rsa_key},
      {group, dsa_pass_key},
      {group, rsa_pass_key},
+     {group, internal_error},
      daemon_already_started,
      server_password_option, server_userpassword_option].
 
 groups() -> 
     [{dsa_key, [], [exec, exec_compressed, shell, known_hosts]},
-     {rsa_key, [], [exec, exec_compressed, shell, known_hosts]},
+     {rsa_key, [], [exec, exec_compressed, shell, known_hosts]},     
      {dsa_pass_key, [], [pass_phrase]},
-     {rsa_pass_key, [], [pass_phrase]}
+     {rsa_pass_key, [], [pass_phrase]},
+     {internal_error, [], [internal_error]}
     ].
 
 init_per_group(dsa_key, Config) ->
@@ -138,6 +140,12 @@ init_per_group(dsa_pass_key, Config) ->
     PrivDir = ?config(priv_dir, Config),
     ssh_test_lib:setup_dsa_pass_pharse(DataDir, PrivDir, "Password"),
     [{pass_phrase, {dsa_pass_phrase, "Password"}}| Config];
+init_per_group(internal_error, Config) ->
+    DataDir = ?config(data_dir, Config),
+    PrivDir = ?config(priv_dir, Config),
+    ssh_test_lib:setup_dsa(DataDir, PrivDir),
+    file:delete(filename:join(PrivDir, "system/ssh_host_dsa_key")),
+    Config;
 init_per_group(_, Config) ->
     Config.
 
@@ -157,6 +165,11 @@ end_per_group(rsa_pass_key, Config) ->
     PrivDir = ?config(priv_dir, Config),
     ssh_test_lib:clean_rsa(PrivDir),
     Config;
+end_per_group(internal_error, Config) ->
+    PrivDir = ?config(priv_dir, Config),
+    ssh_test_lib:clean_dsa(PrivDir),
+    Config;
+
 end_per_group(_, Config) ->
     Config.
 
@@ -268,9 +281,14 @@ shell(Config) when is_list(Config) ->
     IO = ssh_test_lib:start_io_server(),
     Shell = ssh_test_lib:start_shell(Port, IO, UserDir),
     receive
+	{'EXIT', _, _} ->
+	    test_server:fail(no_ssh_connection);  
 	ErlShellStart ->
-	    test_server:format("Erlang shell start: ~p~n", [ErlShellStart])
-    end,
+	    test_server:format("Erlang shell start: ~p~n", [ErlShellStart]),
+	    do_shell(IO, Shell)
+    end.
+    
+do_shell(IO, Shell) ->
     receive
 	ErlPrompt0 ->
 	    test_server:format("Erlang prompt: ~p~n", [ErlPrompt0])
@@ -361,12 +379,15 @@ server_password_option(Config) when is_list(Config) ->
 					  {password, "morot"},
 					  {user_interaction, false},
 					  {user_dir, UserDir}]),
+
+    Reason = "Unable to connect using the available authentication methods",
+    
     {error, Reason} =
-	ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true},
-					  {user, "vego"},
-					  {password, "foo"},
-					  {user_interaction, false},
-					  {user_dir, UserDir}]),
+	ssh:connect(Host, Port, [{silently_accept_hosts, true},
+				 {user, "vego"},
+				 {password, "foo"},
+				 {user_interaction, false},
+				 {user_dir, UserDir}]),
     
     test_server:format("Test of wrong password: Error msg: ~p ~n", [Reason]),
 
@@ -396,25 +417,20 @@ server_userpassword_option(Config) when is_list(Config) ->
 					  {user_dir, UserDir}]),
     ssh:close(ConnectionRef),
 
-    {error, Reason0} =
-	ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true},
-					  {user, "foo"},
-					  {password, "morot"},
-					  {user_interaction, false},
-					  {user_dir, UserDir}]),
-    
-    test_server:format("Test of user foo that does not exist. "
-		       "Error msg: ~p ~n", [Reason0]),
+    Reason = "Unable to connect using the available authentication methods",
 
-    {error, Reason1} =
-	ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true},
-					  {user, "vego"},
-					  {password, "foo"},
-					  {user_interaction, false},
-					  {user_dir, UserDir}]),
-    test_server:format("Test of wrong Password. "
-		       "Error msg: ~p ~n", [Reason1]),
-    
+    {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).
 
 %%--------------------------------------------------------------------
@@ -445,6 +461,7 @@ known_hosts(Config) when is_list(Config) ->
     [Host, _Ip] = string:tokens(HostAndIp, ","),
     "ssh-" ++ _ = Alg,
     ssh:stop_daemon(Pid).
+%%--------------------------------------------------------------------
 
 pass_phrase(doc) ->
     ["Test that we can use keyes protected by pass phrases"];
@@ -470,5 +487,27 @@ pass_phrase(Config) when is_list(Config) ->
     ssh:stop_daemon(Pid).
 
 %%--------------------------------------------------------------------
+
+internal_error(doc) ->
+    ["Test that client does not hang if disconnects due to internal error"];
+
+internal_error(suite) ->
+    [];
+
+internal_error(Config) when is_list(Config) ->
+    process_flag(trap_exit, true),
+    SystemDir = filename:join(?config(priv_dir, Config), system),
+    UserDir = ?config(priv_dir, Config),
+    
+    {_Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SystemDir},
+					     {user_dir, UserDir},
+					     {failfun, fun ssh_test_lib:failfun/2}]),
+    {error,"Internal error"} =
+	ssh:connect(Host, Port, [{silently_accept_hosts, true},
+				 {user_dir, UserDir},
+				 {user_interaction, false}]).
+
+    
+%%--------------------------------------------------------------------
 %% Internal functions
 %%--------------------------------------------------------------------
diff --git a/lib/ssh/test/ssh_sftpd_SUITE.erl b/lib/ssh/test/ssh_sftpd_SUITE.erl
index 6e4480ee9d..de946d4c4c 100644
--- a/lib/ssh/test/ssh_sftpd_SUITE.erl
+++ b/lib/ssh/test/ssh_sftpd_SUITE.erl
@@ -30,7 +30,6 @@
 
 -include_lib("kernel/include/file.hrl").
 
--define(SFPD_PORT, 9999).
 -define(USER, "Alladin").
 -define(PASSWD, "Sesame").
 -define(XFER_PACKET_SIZE, 32768).
@@ -102,13 +101,15 @@ init_per_testcase(TestCase, Config) ->
     ClientUserDir = filename:join(PrivDir, nopubkey),
     SystemDir = filename:join(?config(priv_dir, Config), system),
 
+    Port = ssh_test_lib:inet_port(node()),
+
     {ok, Sftpd} =
-	ssh_sftpd:listen(?SFPD_PORT, [{system_dir, SystemDir},
+	ssh_sftpd:listen(Port, [{system_dir, SystemDir},
 				      {user_dir, PrivDir},
 				      {user_passwords,[{?USER, ?PASSWD}]},
 				      {pwdfun, fun(_,_) -> true end}]),
     
-    Cm = ssh_test_lib:connect(?SFPD_PORT,
+    Cm = ssh_test_lib:connect(Port,
 			      [{user_dir, ClientUserDir},
 			       {user, ?USER}, {password, ?PASSWD},
 			       {user_interaction, false},
diff --git a/lib/ssh/test/ssh_test_lib.erl b/lib/ssh/test/ssh_test_lib.erl
index 26bbdf5c5c..609663c87a 100644
--- a/lib/ssh/test/ssh_test_lib.erl
+++ b/lib/ssh/test/ssh_test_lib.erl
@@ -43,12 +43,8 @@ connect(Host, Options) ->
 connect(any, Port, Options) ->
     connect(hostname(), Port, Options);
 connect(Host, Port, Options) ->
-    case ssh:connect(Host, Port, Options) of
-	{ok, ConnectionRef} ->
-	    ConnectionRef;
-	Error ->
-	    Error
-    end.
+    {ok, ConnectionRef} = ssh:connect(Host, Port, Options),
+    ConnectionRef.
 
 daemon(Options) ->
     daemon(any, inet_port(), Options).
@@ -338,3 +334,13 @@ del_dirs(Dir) ->
 	_ ->
 	    ok
     end.
+
+inet_port(Node) ->
+    {Port, Socket} = do_inet_port(Node),
+     rpc:call(Node, gen_tcp, close, [Socket]),
+     Port.
+
+do_inet_port(Node) ->
+    {ok, Socket} = rpc:call(Node, gen_tcp, listen, [0, [{reuseaddr, true}]]),
+    {ok, Port} = rpc:call(Node, inet, port, [Socket]),
+    {Port, Socket}.
diff --git a/lib/ssh/test/ssh_to_openssh_SUITE.erl b/lib/ssh/test/ssh_to_openssh_SUITE.erl
index dfe526564d..c337617ee4 100644
--- a/lib/ssh/test/ssh_to_openssh_SUITE.erl
+++ b/lib/ssh/test/ssh_to_openssh_SUITE.erl
@@ -445,7 +445,7 @@ erlang_client_openssh_server_password(Config) when is_list(Config) ->
     %% to make sure we don't public-key-auth
     UserDir = ?config(data_dir, Config),
     {error, Reason0} =
-	ssh_test_lib:connect(?SSH_DEFAULT_PORT, [{silently_accept_hosts, true},
+	ssh:connect(any, ?SSH_DEFAULT_PORT, [{silently_accept_hosts, true},
 						 {user, "foo"},
 						 {password, "morot"},
 						 {user_interaction, false},
@@ -459,12 +459,12 @@ erlang_client_openssh_server_password(Config) when is_list(Config) ->
     case length(string:tokens(User, " ")) of
 	1 ->
 	    {error, Reason1} =
-		ssh_test_lib:connect(?SSH_DEFAULT_PORT,
-				     [{silently_accept_hosts, true},
-				      {user, User},
-				      {password, "foo"},
-				      {user_interaction, false},
-				      {user_dir, UserDir}]),
+		ssh:connect(any, ?SSH_DEFAULT_PORT,
+			    [{silently_accept_hosts, true},
+			     {user, User},
+			     {password, "foo"},
+			     {user_interaction, false},
+			     {user_dir, UserDir}]),
 	    test_server:format("Test of wrong Pasword.  "
 			       "Error msg: ~p~n", [Reason1]);
 	_ ->