Merge branch 'hans/ssh/use_open_socket/OTP-12860'

7 files changed, 223 insertions, 74 deletions

diff --git a/lib/ssh/doc/src/ssh.xml b/lib/ssh/doc/src/ssh.xml
index bd330e479f..e6c54d27bf 100644
--- a/lib/ssh/doc/src/ssh.xml
+++ b/lib/ssh/doc/src/ssh.xml
@@ -124,10 +124,10 @@
     </func>
     
     <func>
-      <name>connect(TcpSocket, Options) -> </name>
-      <name>connect(TcpSocket, Options, Timeout) -> </name>
       <name>connect(Host, Port, Options) -> </name>
-      <name>connect(Host, Port, Options, Timeout) -> 
+      <name>connect(Host, Port, Options, Timeout) -> </name>
+      <name>connect(TcpSocket, Options) -> </name>
+      <name>connect(TcpSocket, Options, Timeout) -> 
       {ok, ssh_connection_ref()} | {error, Reason}</name>
       <fsummary>Connects to an SSH server.</fsummary>
       <type>
@@ -140,7 +140,7 @@
 	<d>Negotiation time-out in milli-seconds. The default value is <c>infinity</c>.
 	For connection time-out, use option <c>{connect_timeout, timeout()}</c>.</d>
 	<v>TcpSocket = port()</v>
-	<d>The socket is supposed to be from <c>gen_tcp:connect</c> with option <c>{active,false}</c></d>
+	<d>The socket is supposed to be from <seealso marker="kernel:gen_tcp#connect-3">gen_tcp:connect</seealso> or <seealso marker="kernel:gen_tcp#accept-1">gen_tcp:accept</seealso> with option <c>{active,false}</c></d>
       </type>
       <desc>
 	<p>Connects to an SSH server. No channel is started. This is done
@@ -351,8 +351,9 @@
     <func>
       <name>daemon(Port) -> </name>
       <name>daemon(Port, Options) -> </name>
-      <name>daemon(HostAddress, Port, Options) -> {ok,
-      ssh_daemon_ref()} | {error, atom()}</name>
+      <name>daemon(HostAddress, Port, Options) -> </name>
+      <name>daemon(TcpSocket) -> </name>
+      <name>daemon(TcpSocket, Options) -> {ok, ssh_daemon_ref()} | {error, atom()}</name>
       <fsummary>Starts a server listening for SSH connections 
       on the given port.</fsummary>
       <type>
@@ -361,6 +362,8 @@
 	<v>Options = [{Option, Value}]</v>
         <v>Option = atom()</v>
 	<v>Value = term()</v>
+	<v>TcpSocket = port()</v>
+	<d>The socket is supposed to be from <seealso marker="kernel:gen_tcp#connect-3">gen_tcp:connect</seealso> or <seealso marker="kernel:gen_tcp#accept-1">gen_tcp:accept</seealso> with option <c>{active,false}</c></d>
       </type>
       <desc>
         <p>Starts a server listening for SSH connections on the given
@@ -722,12 +725,15 @@
     <func>
       <name>shell(Host) -> </name>
       <name>shell(Host, Option) -> </name>
-      <name>shell(Host, Port, Option) -> _</name>
+      <name>shell(Host, Port, Option) -> </name>
+      <name>shell(TcpSocket) -> _</name>
       <fsummary>Starts an interactive shell over an SSH server.</fsummary>
       <type>
         <v>Host = string()</v>
 	<v>Port = integer()</v>
 	<v>Options - see ssh:connect/3</v>
+	<v>TcpSocket = port()</v>
+	<d>The socket is supposed to be from <seealso marker="kernel:gen_tcp#connect-3">gen_tcp:connect</seealso> or <seealso marker="kernel:gen_tcp#accept-1">gen_tcp:accept</seealso> with option <c>{active,false}</c></d>
       </type>
       <desc>
         <p>Starts an interactive shell over an SSH server on the
diff --git a/lib/ssh/doc/src/ssh_sftp.xml b/lib/ssh/doc/src/ssh_sftp.xml
index 67531b7d99..eb6f43d417 100644
--- a/lib/ssh/doc/src/ssh_sftp.xml
+++ b/lib/ssh/doc/src/ssh_sftp.xml
@@ -526,10 +526,6 @@
     </func>
 
     <func>
-      <name>start_channel(TcpSocket) -></name>
-      <name>start_channel(TcpSocket, Options) ->
-      {ok, Pid, ConnectionRef} | {error, reason()|term()}</name>
-
       <name>start_channel(ConnectionRef) -></name>
       <name>start_channel(ConnectionRef, Options) ->
       {ok, Pid} | {error, reason()|term()}</name>
@@ -537,13 +533,18 @@
       <name>start_channel(Host, Options) -></name>
       <name>start_channel(Host, Port, Options) -> 
       {ok, Pid, ConnectionRef} | {error, reason()|term()}</name>
+
+      <name>start_channel(TcpSocket) -></name>
+      <name>start_channel(TcpSocket, Options) ->
+      {ok, Pid, ConnectionRef} | {error, reason()|term()}</name>
+
       <fsummary>Starts an SFTP client.</fsummary>
       <type>
         <v>Host = string()</v>
         <v>ConnectionRef = ssh_connection_ref()</v>
         <v>Port = integer()</v>
 	<v>TcpSocket = port()</v>
-	<d>The socket is supposed to be from <c>gen_tcp:connect</c> with option <c>{active,false}</c></d>
+	<d>The socket is supposed to be from <seealso marker="kernel:gen_tcp#connect-3">gen_tcp:connect</seealso> or <seealso marker="kernel:gen_tcp#accept-1">gen_tcp:accept</seealso> with option <c>{active,false}</c></d>
         <v>Options = [{Option, Value}]</v>
       </type>
       <desc>
diff --git a/lib/ssh/src/ssh.erl b/lib/ssh/src/ssh.erl
index 50dfe55798..65f1acc6a6 100644
--- a/lib/ssh/src/ssh.erl
+++ b/lib/ssh/src/ssh.erl
@@ -86,29 +86,19 @@ connect(Socket, Options) ->
 
 connect(Socket, Options, Timeout) when is_port(Socket) ->
     case handle_options(Options) of
-	{error, _Reason} = Error ->
-	    Error;
+	{error, Error} ->
+	    {error, Error};
 	{_SocketOptions, SshOptions} ->
-	    case proplists:get_value(transport, Options, {tcp, gen_tcp, tcp_closed}) of
-		{tcp,_,_} ->
-		    %% Is the socket a valid tcp socket?
-		    case {{ok,[]} =/= inet:getopts(Socket, [delay_send]),
-			  {ok,[{active,false}]} == inet:getopts(Socket, [active])
-			 }
-		    of
-			{true, true} ->
-			    {ok, {Host,_Port}} = inet:sockname(Socket),
-			    Opts =  [{user_pid,self()}, {host,fmt_host(Host)} | SshOptions],
-			    ssh_connection_handler:start_connection(client, Socket, Opts, Timeout);
-			{true, false} ->
-			    {error, not_passive_mode};
-			_ ->
-			    {error, not_tcp_socket}
-		    end;
-		{L4,_,_} ->
-		    {error, {unsupported,L4}}
+	    case valid_socket_to_use(Socket, Options) of
+		ok -> 
+		    {ok, {Host,_Port}} = inet:sockname(Socket),
+		    Opts =  [{user_pid,self()}, {host,fmt_host(Host)} | SshOptions],
+		    ssh_connection_handler:start_connection(client, Socket, Opts, Timeout);
+		{error,SockError} ->
+		    {error,SockError}
 	    end
     end;
+
 connect(Host, Port, Options) when is_integer(Port), Port>0 ->
     connect(Host, Port, Options, infinity).
 
@@ -160,7 +150,7 @@ channel_info(ConnectionRef, ChannelId, Options) ->
 
 %%--------------------------------------------------------------------
 -spec daemon(integer()) -> {ok, pid()} | {error, term()}.
--spec daemon(integer(), proplists:proplist()) -> {ok, pid()} | {error, term()}.
+-spec daemon(integer()|port(), proplists:proplist()) -> {ok, pid()} | {error, term()}.
 -spec daemon(any | inet:ip_address(), integer(), proplists:proplist()) -> {ok, pid()} | {error, term()}.
 
 %% Description: Starts a server listening for SSH connections 
@@ -169,28 +159,16 @@ channel_info(ConnectionRef, ChannelId, Options) ->
 daemon(Port) ->
     daemon(Port, []).
 
-daemon(Port, Options) ->
-    daemon(any, Port, Options).
+daemon(Port, Options) when is_integer(Port) ->
+    daemon(any, Port, Options);
+
+daemon(Socket, Options0) when is_port(Socket) ->
+    Options = daemon_shell_opt(Options0),
+    start_daemon(Socket, Options).
 
 daemon(HostAddr, Port, Options0) ->
-    Options1 = case proplists:get_value(shell, Options0) of
-		   undefined ->
-		       [{shell, {shell, start, []}}  | Options0];
-		   _ ->
-		       Options0
-	       end,
-
-    {Host, Inet, Options} = case HostAddr of
-				any ->
-				    {ok, Host0} = inet:gethostname(), 
-				    {Host0,  proplists:get_value(inet, Options1, inet), Options1};
-				{_,_,_,_} ->
-				    {HostAddr, inet, 
-				     [{ip, HostAddr} | Options1]};
-				{_,_,_,_,_,_,_,_} ->
-				    {HostAddr, inet6, 
-				     [{ip, HostAddr} | Options1]}
-			    end,
+    Options1 = daemon_shell_opt(Options0),
+    {Host, Inet, Options} = daemon_host_inet_opt(HostAddr, Options1),
     start_daemon(Host, Port, Options, Inet).
 
 %%--------------------------------------------------------------------
@@ -284,19 +262,128 @@ default_algorithms() ->
 %%--------------------------------------------------------------------
 %%% Internal functions
 %%--------------------------------------------------------------------
+valid_socket_to_use(Socket, Options) ->
+    case proplists:get_value(transport, Options, {tcp, gen_tcp, tcp_closed}) of
+	{tcp,_,_} ->
+	    %% Is this tcp-socket a valid socket?
+	    case {is_tcp_socket(Socket),
+		  {ok,[{active,false}]} == inet:getopts(Socket, [active])
+		 }
+	    of
+		{true, true} ->
+		    ok;
+		{true, false} ->
+		    {error, not_passive_mode};
+		_ ->
+		    {error, not_tcp_socket}
+	    end;
+	{L4,_,_} ->
+	    {error, {unsupported,L4}}
+    end.
+
+is_tcp_socket(Socket) -> {ok,[]} =/= inet:getopts(Socket, [delay_send]).
+
+
+
+daemon_shell_opt(Options) ->
+     case proplists:get_value(shell, Options) of
+	 undefined ->
+	     [{shell, {shell, start, []}}  | Options];
+	 _ ->
+	     Options
+     end.
+
+daemon_host_inet_opt(HostAddr, Options1) ->
+    case HostAddr of
+	any ->
+	    {ok, Host0} = inet:gethostname(), 
+	    {Host0,  proplists:get_value(inet, Options1, inet), Options1};
+	{_,_,_,_} ->
+	    {HostAddr, inet, 
+	     [{ip, HostAddr} | Options1]};
+	{_,_,_,_,_,_,_,_} ->
+	    {HostAddr, inet6, 
+	     [{ip, HostAddr} | Options1]}
+    end.
+
+
+start_daemon(Socket, Options) ->
+    case handle_options(Options) of
+	{error, Error} ->
+	    {error, Error};
+	{SocketOptions, SshOptions} ->
+	    case valid_socket_to_use(Socket, Options) of
+		ok -> 
+		    try 
+			do_start_daemon(Socket, [{role,server}|SshOptions], SocketOptions)
+		    catch
+			throw:bad_fd -> {error,bad_fd};
+			_C:_E -> {error,{cannot_start_daemon,_C,_E}}
+		    end;
+		{error,SockError} ->
+		    {error,SockError}
+	    end
+    end.
+
 start_daemon(Host, Port, Options, Inet) ->
     case handle_options(Options) of
 	{error, _Reason} = Error ->
 	    Error;
 	{SocketOptions, SshOptions}->
 	    try 
-		do_start_daemon(Host, Port,[{role, server} |SshOptions] , [Inet | SocketOptions])
+		do_start_daemon(Host, Port, [{role,server}|SshOptions] , [Inet|SocketOptions])
 	    catch
 		throw:bad_fd -> {error,bad_fd};
 		_C:_E -> {error,{cannot_start_daemon,_C,_E}}
 	    end
     end.
     
+do_start_daemon(Socket, SshOptions, SocketOptions) ->
+    {ok, {IP,Port}} = 
+	try {ok,_} = inet:sockname(Socket)
+	catch
+	    _:_ -> throw(bad_socket)
+	end,
+    Host = fmt_host(IP),
+    Profile = proplists:get_value(profile, SshOptions, ?DEFAULT_PROFILE),
+    Opts = [{asocket, Socket},
+	    {asock_owner,self()},
+	    {address, Host},
+	    {port, Port},
+	    {role, server},
+	    {socket_opts, SocketOptions}, 
+	    {ssh_opts, SshOptions}],
+    {_, Callback, _} = proplists:get_value(transport, SshOptions, {tcp, gen_tcp, tcp_closed}),
+    case ssh_system_sup:system_supervisor(Host, Port, Profile) of
+	undefined ->
+	    %% It would proably make more sense to call the
+	    %% address option host but that is a too big change at the
+	    %% monent. The name is a legacy name!
+	    try sshd_sup:start_child(Opts) of
+		{error, {already_started, _}} ->
+		    {error, eaddrinuse};
+		Result = {ok,_} ->
+		    ssh_acceptor:handle_connection(Callback, Host, Port, Opts, Socket),
+		    Result;
+		Result = {error, _} ->
+		    Result
+	    catch
+		exit:{noproc, _} ->
+		    {error, ssh_not_started}
+	    end;
+	Sup  ->
+	    AccPid = ssh_system_sup:acceptor_supervisor(Sup),
+	    case ssh_acceptor_sup:start_child(AccPid, Opts) of
+		{error, {already_started, _}} ->
+		    {error, eaddrinuse};
+		{ok, _} ->
+		    ssh_acceptor:handle_connection(Callback, Host, Port, Opts, Socket),
+		    {ok, Sup};
+		Other ->
+		    Other
+	    end
+    end.
+
 do_start_daemon(Host0, Port0, SshOptions, SocketOptions) ->
     {Host,Port1} = 
 	try
@@ -312,7 +399,7 @@ do_start_daemon(Host0, Port0, SshOptions, SocketOptions) ->
 	    _:_ -> throw(bad_fd)
 	end,
     Profile = proplists:get_value(profile, SshOptions, ?DEFAULT_PROFILE),
-    {Port, WaitRequestControl, Opts} =
+    {Port, WaitRequestControl, Opts0} =
 	case Port1 of
 	    0 -> %% Allocate the socket here to get the port number...
 		{_, Callback, _} =  
@@ -326,17 +413,17 @@ do_start_daemon(Host0, Port0, SshOptions, SocketOptions) ->
 	    _ ->
 		{Port1, false, []}
 	end,
+    Opts = [{address, Host}, 
+	    {port, Port},
+	    {role, server},
+	    {socket_opts, SocketOptions}, 
+	    {ssh_opts, SshOptions} | Opts0],
     case ssh_system_sup:system_supervisor(Host, Port, Profile) of
 	undefined ->
 	    %% It would proably make more sense to call the
 	    %% address option host but that is a too big change at the
 	    %% monent. The name is a legacy name!
-	    try sshd_sup:start_child([{address, Host}, 
-				      {port, Port},
-				      {role, server},
-				      {socket_opts, SocketOptions}, 
-				      {ssh_opts, SshOptions}
-				      | Opts]) of
+	    try sshd_sup:start_child(Opts) of
 		{error, {already_started, _}} ->
 		    {error, eaddrinuse};
 		Result = {ok,_} ->
@@ -350,12 +437,7 @@ do_start_daemon(Host0, Port0, SshOptions, SocketOptions) ->
 	    end;
 	Sup  ->
 	    AccPid = ssh_system_sup:acceptor_supervisor(Sup),
-	    case ssh_acceptor_sup:start_child(AccPid, [{address, Host},
-						       {port, Port},
-						       {role, server},
-						       {socket_opts, SocketOptions},
-						       {ssh_opts, SshOptions}
-						       | Opts]) of
+	    case ssh_acceptor_sup:start_child(AccPid, Opts) of
 		{error, {already_started, _}} ->
 		    {error, eaddrinuse};
 		{ok, _} ->
diff --git a/lib/ssh/src/ssh_acceptor.erl b/lib/ssh/src/ssh_acceptor.erl
index 90fd951dcd..9f3e60bd62 100644
--- a/lib/ssh/src/ssh_acceptor.erl
+++ b/lib/ssh/src/ssh_acceptor.erl
@@ -27,7 +27,8 @@
 %% Internal application API
 -export([start_link/5,
 	 number_of_connections/1,
-	 callback_listen/3]).
+	 callback_listen/3,
+	 handle_connection/5]).
 
 %% spawn export  
 -export([acceptor_init/6, acceptor_loop/6]).
diff --git a/lib/ssh/src/ssh_system_sup.erl b/lib/ssh/src/ssh_system_sup.erl
index 5035bc8f80..e97ac7b01a 100644
--- a/lib/ssh/src/ssh_system_sup.erl
+++ b/lib/ssh/src/ssh_system_sup.erl
@@ -131,7 +131,10 @@ init([ServerOpts]) ->
     RestartStrategy = one_for_one,
     MaxR = 0,
     MaxT = 3600,
-    Children = child_specs(ServerOpts),
+    Children = case proplists:get_value(asocket,ServerOpts) of
+		   undefined -> child_specs(ServerOpts);
+		   _ -> []
+	       end,
     {ok, {{RestartStrategy, MaxR, MaxT}, Children}}.
 
 %%%=========================================================================
diff --git a/lib/ssh/test/ssh_connection_SUITE.erl b/lib/ssh/test/ssh_connection_SUITE.erl
index c9a321fbbd..a52633a269 100644
--- a/lib/ssh/test/ssh_connection_SUITE.erl
+++ b/lib/ssh/test/ssh_connection_SUITE.erl
@@ -48,6 +48,9 @@ all() ->
      start_shell_exec,
      start_shell_exec_fun,
      start_shell_sock_exec_fun,
+     start_shell_sock_daemon_exec,
+     connect_sock_not_tcp,
+     daemon_sock_not_tcp,
      gracefull_invalid_version,
      gracefull_invalid_start,
      gracefull_invalid_long_start,
@@ -57,13 +60,11 @@ all() ->
      max_channels_option
     ].
 groups() ->
-    [{openssh, [], payload() ++ ptty()}].
+    [{openssh, [], payload() ++ ptty() ++ sock()}].
 
 payload() ->
     [simple_exec,
      simple_exec_sock,
-     connect_sock_not_tcp,
-     connect_sock_not_passive,
      small_cat,
      big_cat,
      send_after_exit].
@@ -73,6 +74,11 @@ ptty() ->
      ptty_alloc,
      ptty_alloc_pixel].
 
+sock() ->
+    [connect_sock_not_passive,
+     daemon_sock_not_passive
+    ].
+
 %%--------------------------------------------------------------------
 init_per_suite(Config) ->
     Config.
@@ -159,18 +165,30 @@ do_simple_exec(ConnectionRef) ->
     end.
 
 %%--------------------------------------------------------------------
-connect_sock_not_tcp(Config) ->
+connect_sock_not_tcp(_Config) ->
     {ok,Sock} = gen_udp:open(0, []), 
     {error, not_tcp_socket} = ssh:connect(Sock, []),
     gen_udp:close(Sock).
 
 %%--------------------------------------------------------------------
-connect_sock_not_passive(Config) ->
+daemon_sock_not_tcp(_Config) ->
+    {ok,Sock} = gen_udp:open(0, []), 
+    {error, not_tcp_socket} = ssh:daemon(Sock),
+    gen_udp:close(Sock).
+
+%%--------------------------------------------------------------------
+connect_sock_not_passive(_Config) ->
     {ok,Sock} = gen_tcp:connect("localhost", ?SSH_DEFAULT_PORT, []), 
     {error, not_passive_mode} = ssh:connect(Sock, []),
     gen_tcp:close(Sock).
 
 %%--------------------------------------------------------------------
+daemon_sock_not_passive(_Config) ->
+    {ok,Sock} = gen_tcp:connect("localhost", ?SSH_DEFAULT_PORT, []), 
+    {error, not_passive_mode} = ssh:daemon(Sock),
+    gen_tcp:close(Sock).
+
+%%--------------------------------------------------------------------
 small_cat() ->
     [{doc, "Use 'cat' to echo small data block back to us."}].
 
@@ -520,7 +538,44 @@ start_shell_sock_exec_fun(Config) when is_list(Config) ->
     ssh:stop_daemon(Pid).
 
 %%--------------------------------------------------------------------
+start_shell_sock_daemon_exec(Config) ->
+    PrivDir = proplists:get_value(priv_dir, Config),
+    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
+    file:make_dir(UserDir),
+    SysDir = proplists:get_value(data_dir, Config),
 
+    {ok,Sl} = gen_tcp:listen(0, [{active,false}]),
+    {ok,{_IP,Port}} = inet:sockname(Sl),	% _IP is likely to be {0,0,0,0}. Win don't like...
+    
+    spawn_link(fun() ->
+		       {ok,Ss} = gen_tcp:connect("localhost", Port, [{active,false}]),
+		       {ok, Pid} = ssh:daemon(Ss, [{system_dir, SysDir},
+						   {user_dir, UserDir},
+						   {password, "morot"},
+						   {exec, fun ssh_exec/1}])
+	       end),
+    {ok,Sc} = gen_tcp:accept(Sl),
+    {ok,ConnectionRef} = ssh:connect(Sc, [{silently_accept_hosts, true},
+					  {user, "foo"},
+					  {password, "morot"},
+					  {user_interaction, true},
+					  {user_dir, UserDir}]),
+    
+    {ok, ChannelId0} = ssh_connection:session_channel(ConnectionRef, infinity),
+
+    success = ssh_connection:exec(ConnectionRef, ChannelId0,
+				  "testing", infinity),
+
+    receive
+	{ssh_cm, ConnectionRef, {data, _ChannelId, 0, <<"testing\r\n">>}} ->
+	    ok
+    after 5000 ->
+	    ct:fail("Exec Timeout")
+    end,
+
+    ssh:close(ConnectionRef).
+    
+%%--------------------------------------------------------------------
 gracefull_invalid_version(Config) when is_list(Config) ->
     PrivDir = proplists:get_value(priv_dir, Config),
     UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
diff --git a/lib/ssh/test/ssh_sftp_SUITE.erl b/lib/ssh/test/ssh_sftp_SUITE.erl
index 4d40b4647c..19cf6d446e 100644
--- a/lib/ssh/test/ssh_sftp_SUITE.erl
+++ b/lib/ssh/test/ssh_sftp_SUITE.erl
@@ -673,6 +673,7 @@ start_channel_sock(Config) ->
 
     %% Test that the socket is closed when the Connection closes
     ok = ssh:close(Conn),
+    timer:sleep(400), %% Until the stop sequence is fixed
     {error,einval} = inet:getopts(Sock, [active]),
 
     ok.