From d7d109ca8ab68d7a62772305d543123233d4eea4 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Wed, 1 Jun 2016 16:43:29 +0200 Subject: ssh: update connect doc --- lib/ssh/doc/src/ssh.xml | 13 ++++++++----- lib/ssh/doc/src/ssh_sftp.xml | 11 ++++++----- 2 files changed, 14 insertions(+), 10 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/doc/src/ssh.xml b/lib/ssh/doc/src/ssh.xml index bd330e479f..501ca36b09 100644 --- a/lib/ssh/doc/src/ssh.xml +++ b/lib/ssh/doc/src/ssh.xml @@ -124,10 +124,10 @@ - connect(TcpSocket, Options) -> - connect(TcpSocket, Options, Timeout) -> connect(Host, Port, Options) -> - connect(Host, Port, Options, Timeout) -> + connect(Host, Port, Options, Timeout) -> + connect(TcpSocket, Options) -> + connect(TcpSocket, Options, Timeout) -> {ok, ssh_connection_ref()} | {error, Reason} Connects to an SSH server. @@ -140,7 +140,7 @@ Negotiation time-out in milli-seconds. The default value is infinity. For connection time-out, use option {connect_timeout, timeout()}. TcpSocket = port() - The socket is supposed to be from gen_tcp:connect with option {active,false} + The socket is supposed to be from gen_tcp:connect or gen_tcp:accept with option {active,false}

Connects to an SSH server. No channel is started. This is done @@ -722,12 +722,15 @@ shell(Host) -> shell(Host, Option) -> - shell(Host, Port, Option) -> _ + shell(Host, Port, Option) -> + shell(TcpSocket) -> _ Starts an interactive shell over an SSH server. Host = string() Port = integer() Options - see ssh:connect/3 + TcpSocket = port() + The socket is supposed to be from gen_tcp:connect or gen_tcp:accept with option {active,false}

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 @@ - start_channel(TcpSocket) -> - start_channel(TcpSocket, Options) -> - {ok, Pid, ConnectionRef} | {error, reason()|term()} - start_channel(ConnectionRef) -> start_channel(ConnectionRef, Options) -> {ok, Pid} | {error, reason()|term()} @@ -537,13 +533,18 @@ start_channel(Host, Options) -> start_channel(Host, Port, Options) -> {ok, Pid, ConnectionRef} | {error, reason()|term()} + + start_channel(TcpSocket) -> + start_channel(TcpSocket, Options) -> + {ok, Pid, ConnectionRef} | {error, reason()|term()} + Starts an SFTP client. Host = string() ConnectionRef = ssh_connection_ref() Port = integer() TcpSocket = port() - The socket is supposed to be from gen_tcp:connect with option {active,false} + The socket is supposed to be from gen_tcp:connect or gen_tcp:accept with option {active,false} Options = [{Option, Value}] -- cgit v1.2.3 From 3b769cfe66b81467d756fe670fe25e2fd984fdba Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Wed, 1 Jun 2016 16:09:35 +0200 Subject: ssh: daemon taking open socket as input --- lib/ssh/doc/src/ssh.xml | 7 ++- lib/ssh/src/ssh.erl | 134 ++++++++++++++++++++++++++++++----------- lib/ssh/src/ssh_acceptor.erl | 3 +- lib/ssh/src/ssh_system_sup.erl | 5 +- 4 files changed, 110 insertions(+), 39 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/doc/src/ssh.xml b/lib/ssh/doc/src/ssh.xml index 501ca36b09..e6c54d27bf 100644 --- a/lib/ssh/doc/src/ssh.xml +++ b/lib/ssh/doc/src/ssh.xml @@ -351,8 +351,9 @@ daemon(Port) -> daemon(Port, Options) -> - daemon(HostAddress, Port, Options) -> {ok, - ssh_daemon_ref()} | {error, atom()} + daemon(HostAddress, Port, Options) -> + daemon(TcpSocket) -> + daemon(TcpSocket, Options) -> {ok, ssh_daemon_ref()} | {error, atom()} Starts a server listening for SSH connections on the given port. @@ -361,6 +362,8 @@ Options = [{Option, Value}] Option = atom() Value = term() + TcpSocket = port() + The socket is supposed to be from gen_tcp:connect or gen_tcp:accept with option {active,false}

Starts a server listening for SSH connections on the given diff --git a/lib/ssh/src/ssh.erl b/lib/ssh/src/ssh.erl index 50dfe55798..fa2354a3df 100644 --- a/lib/ssh/src/ssh.erl +++ b/lib/ssh/src/ssh.erl @@ -160,7 +160,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 +169,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 +272,100 @@ default_algorithms() -> %%-------------------------------------------------------------------- %%% Internal functions %%-------------------------------------------------------------------- +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, _Reason} = Error -> + Error; + {SocketOptions, SshOptions}-> + 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 + 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 +381,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 +395,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 +419,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}}. %%%========================================================================= -- cgit v1.2.3 From 2f0d43a9da600a3835b9dbbb20eeaf43565363f6 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Wed, 1 Jun 2016 19:54:35 +0200 Subject: ssh: better validation --- lib/ssh/src/ssh.erl | 74 +++++++++++++++++++++++++++++++++-------------------- 1 file changed, 46 insertions(+), 28 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/src/ssh.erl b/lib/ssh/src/ssh.erl index fa2354a3df..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). @@ -272,6 +262,29 @@ 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 -> @@ -296,14 +309,19 @@ daemon_host_inet_opt(HostAddr, Options1) -> start_daemon(Socket, Options) -> case handle_options(Options) of - {error, _Reason} = Error -> - Error; - {SocketOptions, SshOptions}-> - try - do_start_daemon(Socket, [{role,server}|SshOptions], SocketOptions) - catch - throw:bad_fd -> {error,bad_fd}; - _C:_E -> {error,{cannot_start_daemon,_C,_E}} + {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. -- cgit v1.2.3 From 875782025e19047bc8c25da8ed1c121a33521d71 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Wed, 1 Jun 2016 19:55:08 +0200 Subject: ssh: test cases on daemon using open socket --- lib/ssh/test/ssh_connection_SUITE.erl | 65 ++++++++++++++++++++++++++++++++--- 1 file changed, 60 insertions(+), 5 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/test/ssh_connection_SUITE.erl b/lib/ssh/test/ssh_connection_SUITE.erl index c9a321fbbd..105cd6def5 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,17 +165,29 @@ 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), + + spawn_link(fun() -> + {ok,Ss} = gen_tcp:connect(IP,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 -- cgit v1.2.3 From fba5d467360c18d58ddf664c581dd7e78823a4d4 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Thu, 2 Jun 2016 08:37:13 +0200 Subject: ssh: fix ssh_connection_SUITE error on Windows --- lib/ssh/test/ssh_connection_SUITE.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/test/ssh_connection_SUITE.erl b/lib/ssh/test/ssh_connection_SUITE.erl index 105cd6def5..a52633a269 100644 --- a/lib/ssh/test/ssh_connection_SUITE.erl +++ b/lib/ssh/test/ssh_connection_SUITE.erl @@ -545,10 +545,10 @@ start_shell_sock_daemon_exec(Config) -> SysDir = proplists:get_value(data_dir, Config), {ok,Sl} = gen_tcp:listen(0, [{active,false}]), - {ok,{IP,Port}} = inet:sockname(Sl), + {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(IP,Port, [{active,false}]), + {ok,Ss} = gen_tcp:connect("localhost", Port, [{active,false}]), {ok, Pid} = ssh:daemon(Ss, [{system_dir, SysDir}, {user_dir, UserDir}, {password, "morot"}, -- cgit v1.2.3 From 3cd8fe84c7a3b1f40d824d243447e4bf82ac28b3 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Thu, 2 Jun 2016 20:13:11 +0200 Subject: ssh: temp fix hazard in test suite --- lib/ssh/test/ssh_sftp_SUITE.erl | 1 + 1 file changed, 1 insertion(+) (limited to 'lib/ssh') 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. -- cgit v1.2.3