From 57d994270d63e7a9ce80eece3c1c3aeca79d3ea4 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Tue, 21 Mar 2017 15:44:44 +0100 Subject: ssh: fix ssh_system_sup naming of Host-Port-Profile --- lib/ssh/src/ssh.erl | 2 +- lib/ssh/src/ssh_acceptor_sup.erl | 7 +------ lib/ssh/src/ssh_system_sup.erl | 16 ++-------------- lib/ssh/src/sshd_sup.erl | 7 +------ 4 files changed, 5 insertions(+), 27 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/src/ssh.erl b/lib/ssh/src/ssh.erl index ff424b738c..9047b7e0f0 100644 --- a/lib/ssh/src/ssh.erl +++ b/lib/ssh/src/ssh.erl @@ -475,6 +475,6 @@ finalize_start(Host, Port, Profile, Options0, F) -> end. %%%---------------------------------------------------------------- -fmt_host(any) -> any; +fmt_host(any) -> "any"; fmt_host(IP) when is_tuple(IP) -> inet:ntoa(IP); fmt_host(Str) when is_list(Str) -> Str. diff --git a/lib/ssh/src/ssh_acceptor_sup.erl b/lib/ssh/src/ssh_acceptor_sup.erl index 77f7826918..613d8fbc75 100644 --- a/lib/ssh/src/ssh_acceptor_sup.erl +++ b/lib/ssh/src/ssh_acceptor_sup.erl @@ -93,10 +93,5 @@ child_spec(Options) -> {Name, StartFunc, Restart, Shutdown, Type, Modules}. id(Address, Port, Profile) -> - case is_list(Address) of - true -> - {ssh_acceptor_sup, any, Port, Profile}; - false -> - {ssh_acceptor_sup, Address, Port, Profile} - end. + {ssh_acceptor_sup, Address, Port, Profile}. diff --git a/lib/ssh/src/ssh_system_sup.erl b/lib/ssh/src/ssh_system_sup.erl index 5a58ef1c44..4083f666c3 100644 --- a/lib/ssh/src/ssh_system_sup.erl +++ b/lib/ssh/src/ssh_system_sup.erl @@ -166,22 +166,10 @@ ssh_subsystem_child_spec(Options) -> id(Sup, Address, Port, Profile) -> - case is_list(Address) of - true -> - {Sup, any, Port, Profile}; - false -> - {Sup, Address, Port, Profile} - end. + {Sup, Address, Port, Profile}. make_name(Address, Port, Profile) -> - case is_list(Address) of - true -> - list_to_atom(lists:flatten(io_lib:format("ssh_system_~p_~p_~p_sup", - [any, Port, Profile]))); - false -> - list_to_atom(lists:flatten(io_lib:format("ssh_system_~p_~p_~p_sup", - [Address, Port, Profile]))) - end. + list_to_atom(lists:flatten(io_lib:format("ssh_system_~s_~p_~p_sup", [Address, Port, Profile]))). ssh_subsystem_sup([{_, Child, _, [ssh_subsystem_sup]} | _]) -> Child; diff --git a/lib/ssh/src/sshd_sup.erl b/lib/ssh/src/sshd_sup.erl index 14f1937abd..791456839d 100644 --- a/lib/ssh/src/sshd_sup.erl +++ b/lib/ssh/src/sshd_sup.erl @@ -103,12 +103,7 @@ child_spec(Address, Port, Options) -> {Name, StartFunc, Restart, Shutdown, Type, Modules}. id(Address, Port, Profile) -> - case is_list(Address) of - true -> - {server, ssh_system_sup, any, Port, Profile}; - false -> - {server, ssh_system_sup, Address, Port, Profile} - end. + {server, ssh_system_sup, Address, Port, Profile}. system_name([], _ ) -> undefined; -- cgit v1.2.3 From 6158cb432092c47e178b4dc1177b46cb8c310ab4 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Tue, 21 Mar 2017 19:50:49 +0100 Subject: ssh: Fix supervisors, start daemon and connect code Remove many internal options and made them as explicit arguments. --- lib/ssh/src/ssh.erl | 240 +++++++++++++++++++++------------ lib/ssh/src/ssh_acceptor.erl | 3 +- lib/ssh/src/ssh_acceptor_sup.erl | 22 ++- lib/ssh/src/ssh_connection_handler.erl | 7 +- lib/ssh/src/ssh_file.erl | 2 + lib/ssh/src/ssh_subsystem_sup.erl | 34 ++--- lib/ssh/src/ssh_sup.erl | 53 +------- lib/ssh/src/ssh_system_sup.erl | 43 +++--- lib/ssh/src/sshc_sup.erl | 14 +- lib/ssh/src/sshd_sup.erl | 55 +++----- 10 files changed, 237 insertions(+), 236 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/src/ssh.erl b/lib/ssh/src/ssh.erl index 9047b7e0f0..680047dffd 100644 --- a/lib/ssh/src/ssh.erl +++ b/lib/ssh/src/ssh.erl @@ -108,7 +108,7 @@ connect(Socket, UserOptions, Timeout) when is_port(Socket), case valid_socket_to_use(Socket, ?GET_OPT(transport,Options)) of ok -> {ok, {Host,_Port}} = inet:sockname(Socket), - Opts = ?PUT_INTERNAL_OPT([{user_pid,self()}, {host,fmt_host(Host)}], Options), + Opts = ?PUT_INTERNAL_OPT([{user_pid,self()}, {host,Host}], Options), ssh_connection_handler:start_connection(client, Socket, Opts, Timeout); {error,SockError} -> {error,SockError} @@ -132,7 +132,7 @@ connect(Host, Port, UserOptions, Timeout) when is_integer(Port), SocketOpts = [{active,false} | ?GET_OPT(socket_options,Options)], try Transport:connect(Host, Port, SocketOpts, ConnectionTimeout) of {ok, Socket} -> - Opts = ?PUT_INTERNAL_OPT([{user_pid,self()}, {host,fmt_host(Host)}], Options), + Opts = ?PUT_INTERNAL_OPT([{user_pid,self()}, {host,Host}], Options), ssh_connection_handler:start_connection(client, Socket, Opts, Timeout); {error, Reason} -> {error, Reason} @@ -188,14 +188,11 @@ daemon(Socket, UserOptions) when is_port(Socket) -> case valid_socket_to_use(Socket, ?GET_OPT(transport,Options)) of ok -> {ok, {IP,Port}} = inet:sockname(Socket), - finalize_start(fmt_host(IP), Port, ?GET_OPT(profile, Options), + finalize_start(IP, Port, ?GET_OPT(profile, Options), ?PUT_INTERNAL_OPT({connected_socket, Socket}, Options), fun(Opts, DefaultResult) -> try ssh_acceptor:handle_established_connection( - ?GET_INTERNAL_OPT(address, Opts), - ?GET_INTERNAL_OPT(port, Opts), - Opts, - Socket) + IP, Port, Opts, Socket) of {error,Error} -> {error,Error}; @@ -238,7 +235,7 @@ daemon(Host0, Port0, UserOptions0) when 0 =< Port0, Port0 =< 65535 -> %% and ListenSocket is for listening on connections. But it is still owned %% by self()... - finalize_start(fmt_host(Host), Port, ?GET_OPT(profile, Options0), + finalize_start(Host, Port, ?GET_OPT(profile, Options0), ?PUT_INTERNAL_OPT({lsocket,{ListenSocket,self()}}, Options0), fun(Opts, Result) -> {_, Callback, _} = ?GET_OPT(transport, Opts), @@ -269,17 +266,27 @@ daemon(Host0, Port0, UserOptions0) when 0 =< Port0, Port0 =< 65535 -> daemon_info(Pid) -> case catch ssh_system_sup:acceptor_supervisor(Pid) of AsupPid when is_pid(AsupPid) -> - [{ListenAddr,Port,Profile}] = - [{LA,Prt,Prf} || {{ssh_acceptor_sup,LA,Prt,Prf}, - _WorkerPid,worker,[ssh_acceptor]} <- supervisor:which_children(AsupPid)], + [{Name,Port,Profile}] = + [{Nam,Prt,Prf} + || {{ssh_acceptor_sup,Hst,Prt,Prf},_Pid,worker,[ssh_acceptor]} + <- supervisor:which_children(AsupPid), + Nam <- [case inet:parse_strict_address(Hst) of + {ok,IP} -> IP; + _ when Hst=="any" -> any; + _ when Hst=="loopback" -> loopback; + _ -> Hst + end] + ], {ok, [{port,Port}, - {listen_address,ListenAddr}, + {name,Name}, {profile,Profile} ]}; _ -> {error,bad_daemon_ref} end. + + %%-------------------------------------------------------------------- -spec stop_listener(daemon_ref()) -> ok. -spec stop_listener(inet:ip_address(), inet:port_number()) -> ok. @@ -361,49 +368,128 @@ default_algorithms() -> %%-------------------------------------------------------------------- %%% Internal functions %%-------------------------------------------------------------------- -handle_daemon_args(HostAddr, Opts) -> + +%% - if Address is 'any' and no ip-option is present, the name is +%% 'any' and the socket will listen to all addresses +%% +%% - if Address is 'any' and an ip-option is present, the name is +%% set to the value of the ip-option and the socket will listen +%% to that address +%% +%% - if Address is 'loopback' and no ip-option is present, the name +%% is 'loopback' and an loopback address will be choosen by the +%% underlying layers +%% +%% - if Address is 'loopback' and an ip-option is present, the name +%% is set to the value of the ip-option kept and the socket will +%% listen to that address +%% +%% - if Address is an ip-address, that ip-address is the name and +%% the listening address. An ip-option will be discarded. +%% +%% - if Address is a HostName, and that resolves to an ip-address, +%% that ip-address is the name and the listening address. An +%% ip-option will be discarded. +%% +%% - if Address is a string or an atom other than thoose defined +%% above, that Address will be the name and the listening address +%% will be choosen by the lower layers taking an ip-option in +%% consideration +%% + +handle_daemon_args(any, Opts) -> + case proplists:get_value(ip, Opts) of + undefined -> {any, Opts}; + IP -> {IP, Opts} + end; + +handle_daemon_args(loopback, Opts) -> + case proplists:get_value(ip, Opts) of + undefined -> {loopback, [{ip,loopback}|Opts]}; + IP -> {IP, Opts} + end; + +handle_daemon_args(IPaddr, Opts) when is_tuple(IPaddr) -> + case proplists:get_value(ip, Opts) of + undefined -> {IPaddr, [{ip,IPaddr}|Opts]}; + IPaddr -> {IPaddr, Opts}; + IP -> {IPaddr, [{ip,IPaddr}|Opts--[{ip,IP}]]} %% Backward compatibility + end; + +handle_daemon_args(Address, Opts) when is_list(Address) ; is_atom(Address) -> IP = proplists:get_value(ip, Opts), - IPh = case inet:parse_strict_address(HostAddr) of - {ok, IPtuple} -> IPtuple; - {error, einval} when is_tuple(HostAddr), - size(HostAddr)==4 ; size(HostAddr)==6 -> HostAddr; - _ -> undefined - end, - handle_daemon_args(HostAddr, IPh, IP, Opts). - - -%% HostAddr is 'any' -handle_daemon_args(any, undefined, undefined, Opts) -> {any, Opts}; -handle_daemon_args(any, undefined, IP, Opts) -> {IP, Opts}; - -%% HostAddr is 'loopback' or "localhost" -handle_daemon_args(loopback, undefined, {127,_,_,_}=IP, Opts) -> {IP, Opts}; -handle_daemon_args(loopback, undefined, {0,0,0,0,0,0,0,1}=IP, Opts) -> {IP, Opts}; -handle_daemon_args(loopback, undefined, undefined, Opts) -> - IP = case proplists:get_value(inet,Opts) of - true -> {127,0,0,1}; - inet -> {127,0,0,1}; - inet6 -> {0,0,0,0,0,0,0,1}; - _ -> case proplists:get_value(inet6,Opts) of - true -> {0,0,0,0,0,0,0,1}; - _ -> {127,0,0,1} % default if no 'inet' nor 'inet6' - end - end, - {IP, [{ip,IP}|Opts]}; -handle_daemon_args("localhost", IPh, IP, Opts) -> - handle_daemon_args(loopback, IPh, IP, Opts); - -%% HostAddr is ip and no ip-option -handle_daemon_args(_, IP, undefined, Opts) when is_tuple(IP) -> {IP, [{ip,IP}|Opts]}; - -%% HostAddr and ip-option are equal -handle_daemon_args(_, IP, IP, Opts) when is_tuple(IP) -> {IP, Opts}; - -%% HostAddr is ip, but ip-option is different! -handle_daemon_args(_, IPh, IPo, _) when is_tuple(IPh), is_tuple(IPo) -> error({eoption,{ip,IPo}}); - -%% Something else. Whatever it is, it is wrong. -handle_daemon_args(_, _, _, _) -> error(badarg). + case inet:parse_strict_address(Address) of + {ok, IP} -> {IP, Opts}; + {ok, OtherIP} -> {OtherIP, [{ip,OtherIP}|Opts--[{ip,IP}]]}; + _ -> + case inet:getaddr(Address, family(Opts)) of + {ok, IP} -> {Address, Opts}; + {ok, OtherIP} -> {Address, [{ip,OtherIP}|Opts--[{ip,IP}]]}; + _ -> {Address, Opts} + end + end. + + +-ifdef(hulahopp). +%% Check the Address parameter and set an ip-option in some cases. The +%% Address parameter is left unchanged because ssh:stop_listener and +%% ssh:stop_daemon needs to find the system supervisor by name + +handle_daemon_args(any, Opts) -> + %% Listen to 0.0.0.0. The caller may have set an ip-option. Trust + %% that one in such a case. + {any, Opts}; + +handle_daemon_args(loopback, Opts) -> + %% Listen to a loopback address. Let the underlying layers decide + %% in case the caller hasn't set the ip-option. + {loopback, ensure_ip_option(loopback,Opts)}; + +handle_daemon_args(IP, Opts) when is_tuple(IP) -> + %% An IP address in Erlang tuple format: + {IP, ensure_ip_option(IP,Opts)}; + +handle_daemon_args(Address, Opts) when is_list(Address) ; is_atom(Address) -> + %% This might be a host name, an FQDN, an IP address in string format ("127.1.1.1") + %% etc. It might be a string or an atom since inet:hostname() is defined in that way + case inet:parse_strict_address(Address) of + {ok, IP} -> + {Address, ensure_ip_option(IP,Opts)}; + _ -> + %% Try to lookup as a hostname: + case inet:getaddr(Address, family(Opts)) of + {ok, IP} -> + {Address, ensure_ip_option(IP,Opts)}; + _ -> + %% Give up and let the underlying system handle this + {Address, Opts} + end + end. + + +%% Add an ip-option if not already present. +ensure_ip_option(Address, Opts) -> + case proplists:get_value(ip, Opts) of + undefined -> [{ip,Address}|Opts]; + _ -> Opts + end. +-endif. + + +%% Has the caller indicated the address family? +family(Opts) -> + family(Opts, inet). + +family(Opts, Default) -> + case proplists:get_value(inet,Opts) of + true -> inet; + inet -> inet; + inet6 -> inet6; + _ -> case proplists:get_value(inet6,Opts) of + true -> inet6; + _ -> Default + end + end. %%%---------------------------------------------------------------- valid_socket_to_use(Socket, {tcp,_,_}) -> @@ -434,8 +520,9 @@ open_listen_socket(Host0, Port0, Options0) -> case ?GET_SOCKET_OPT(fd, Options0) of undefined -> {ok,LSock} = ssh_acceptor:listen(Port0, Options0), - {ok,{_,LPort}} = inet:sockname(LSock), - {{Host0,LPort}, LSock}; + {ok,{_LHost,LPort}} = inet:sockname(LSock), + {{_LHost,LPort}, LSock}; +%% {{Host0,LPort}, LSock}; Fd when is_integer(Fd) -> %% Do gen_tcp:listen with the option {fd,Fd}: @@ -446,35 +533,18 @@ open_listen_socket(Host0, Port0, Options0) -> %%%---------------------------------------------------------------- finalize_start(Host, Port, Profile, Options0, F) -> - Options = ?PUT_INTERNAL_OPT([{address, Host}, - {port, Port}, - {role, server}], Options0), - case ssh_system_sup:system_supervisor(Host, Port, Profile) of - undefined -> - try sshd_sup:start_child(Options) of - {error, {already_started, _}} -> - {error, eaddrinuse}; - {error, Error} -> - {error, Error}; - Result = {ok,_} -> - F(Options, Result) - catch - exit:{noproc, _} -> - {error, ssh_not_started} - end; - Sup -> - AccPid = ssh_system_sup:acceptor_supervisor(Sup), - case ssh_acceptor_sup:start_child(AccPid, Options) of - {error, {already_started, _}} -> - {error, eaddrinuse}; - {error, Error} -> - {error, Error}; - {ok, _} -> - F(Options, {ok,Sup}) - end + try + sshd_sup:start_child(Host, Port, Profile, Options0) + of + {error, {already_started, _}} -> + {error, eaddrinuse}; + {error, Error} -> + {error, Error}; + Result = {ok,_} -> + F(Options0, Result) + catch + exit:{noproc, _} -> + {error, ssh_not_started} end. %%%---------------------------------------------------------------- -fmt_host(any) -> "any"; -fmt_host(IP) when is_tuple(IP) -> inet:ntoa(IP); -fmt_host(Str) when is_list(Str) -> Str. diff --git a/lib/ssh/src/ssh_acceptor.erl b/lib/ssh/src/ssh_acceptor.erl index f9e2280212..f7fbd7ccad 100644 --- a/lib/ssh/src/ssh_acceptor.erl +++ b/lib/ssh/src/ssh_acceptor.erl @@ -129,7 +129,8 @@ handle_connection(Callback, Address, Port, Options, Socket) -> MaxSessions = ?GET_OPT(max_sessions, Options), case number_of_connections(SystemSup) < MaxSessions of true -> - {ok, SubSysSup} = ssh_system_sup:start_subsystem(SystemSup, Options), + {ok, SubSysSup} = + ssh_system_sup:start_subsystem(SystemSup, server, Address, Port, Profile, Options), ConnectionSup = ssh_subsystem_sup:connection_supervisor(SubSysSup), NegTimeout = ?GET_OPT(negotiation_timeout, Options), ssh_connection_handler:start_connection(server, Socket, diff --git a/lib/ssh/src/ssh_acceptor_sup.erl b/lib/ssh/src/ssh_acceptor_sup.erl index 613d8fbc75..4606107f56 100644 --- a/lib/ssh/src/ssh_acceptor_sup.erl +++ b/lib/ssh/src/ssh_acceptor_sup.erl @@ -29,7 +29,7 @@ -include("ssh.hrl"). --export([start_link/1, start_child/2, stop_child/4]). +-export([start_link/4, start_child/5, stop_child/4]). %% Supervisor callback -export([init/1]). @@ -41,16 +41,13 @@ %%%========================================================================= %%% API %%%========================================================================= -start_link(Servers) -> - supervisor:start_link(?MODULE, [Servers]). +start_link(Address, Port, Profile, Options) -> + supervisor:start_link(?MODULE, [Address, Port, Profile, Options]). -start_child(AccSup, Options) -> - Spec = child_spec(Options), +start_child(AccSup, Address, Port, Profile, Options) -> + Spec = child_spec(Address, Port, Profile, Options), case supervisor:start_child(AccSup, Spec) of {error, already_present} -> - Address = ?GET_INTERNAL_OPT(address, Options), - Port = ?GET_INTERNAL_OPT(port, Options), - Profile = ?GET_OPT(profile, Options), stop_child(AccSup, Address, Port, Profile), supervisor:start_child(AccSup, Spec); Reply -> @@ -69,21 +66,18 @@ stop_child(AccSup, Address, Port, Profile) -> %%%========================================================================= %%% Supervisor callback %%%========================================================================= -init([Options]) -> +init([Address, Port, Profile, Options]) -> RestartStrategy = one_for_one, MaxR = 10, MaxT = 3600, - Children = [child_spec(Options)], + Children = [child_spec(Address, Port, Profile, Options)], {ok, {{RestartStrategy, MaxR, MaxT}, Children}}. %%%========================================================================= %%% Internal functions %%%========================================================================= -child_spec(Options) -> - Address = ?GET_INTERNAL_OPT(address, Options), - Port = ?GET_INTERNAL_OPT(port, Options), +child_spec(Address, Port, Profile, Options) -> Timeout = ?GET_INTERNAL_OPT(timeout, Options, ?DEFAULT_TIMEOUT), - Profile = ?GET_OPT(profile, Options), Name = id(Address, Port, Profile), StartFunc = {ssh_acceptor, start_link, [Port, Address, Options, Timeout]}, Restart = transient, diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl index 50a29bbb53..ff94e5dfb6 100644 --- a/lib/ssh/src/ssh_connection_handler.erl +++ b/lib/ssh/src/ssh_connection_handler.erl @@ -440,7 +440,12 @@ init_ssh_record(Role, Socket, Opts) -> {Vsn, Version} = ssh_transport:versions(Role, Opts), case Role of client -> - PeerName = ?GET_INTERNAL_OPT(host, Opts), + PeerName = case ?GET_INTERNAL_OPT(host, Opts) of + PeerIP when is_tuple(PeerIP) -> + inet_parse:ntoa(PeerIP); + PeerName0 -> + PeerName0 + end, S0#ssh{c_vsn = Vsn, c_version = Version, io_cb = case ?GET_OPT(user_interaction, Opts) of diff --git a/lib/ssh/src/ssh_file.erl b/lib/ssh/src/ssh_file.erl index 898b4cc5c4..88f4d10792 100644 --- a/lib/ssh/src/ssh_file.erl +++ b/lib/ssh/src/ssh_file.erl @@ -221,6 +221,8 @@ file_name(Type, Name, Opts) -> %% in: "host" out: "host,1.2.3.4. +add_ip(IP) when is_tuple(IP) -> + ssh_connection:encode_ip(IP); add_ip(Host) -> case inet:getaddr(Host, inet) of {ok, Addr} -> diff --git a/lib/ssh/src/ssh_subsystem_sup.erl b/lib/ssh/src/ssh_subsystem_sup.erl index cf82db458f..c5ab422265 100644 --- a/lib/ssh/src/ssh_subsystem_sup.erl +++ b/lib/ssh/src/ssh_subsystem_sup.erl @@ -28,7 +28,7 @@ -include("ssh.hrl"). --export([start_link/1, +-export([start_link/5, connection_supervisor/1, channel_supervisor/1 ]). @@ -39,8 +39,8 @@ %%%========================================================================= %%% API %%%========================================================================= -start_link(Options) -> - supervisor:start_link(?MODULE, [Options]). +start_link(Role, Address, Port, Profile, Options) -> + supervisor:start_link(?MODULE, [Role, Address, Port, Profile, Options]). connection_supervisor(SupPid) -> Children = supervisor:which_children(SupPid), @@ -53,30 +53,23 @@ channel_supervisor(SupPid) -> %%%========================================================================= %%% Supervisor callback %%%========================================================================= --spec init( [term()] ) -> {ok,{supervisor:sup_flags(),[supervisor:child_spec()]}} | ignore . - -init([Options]) -> +init([Role, Address, Port, Profile, Options]) -> RestartStrategy = one_for_all, MaxR = 0, MaxT = 3600, - Children = child_specs(Options), + Children = child_specs(Role, Address, Port, Profile, Options), {ok, {{RestartStrategy, MaxR, MaxT}, Children}}. %%%========================================================================= %%% Internal functions %%%========================================================================= -child_specs(Options) -> - case ?GET_INTERNAL_OPT(role, Options) of - client -> - []; - server -> - [ssh_channel_child_spec(Options), ssh_connectinon_child_spec(Options)] - end. +child_specs(client, _Address, _Port, _Profile, _Options) -> + []; +child_specs(server, Address, Port, Profile, Options) -> + [ssh_channel_child_spec(server, Address, Port, Profile, Options), + ssh_connection_child_spec(server, Address, Port, Profile, Options)]. -ssh_connectinon_child_spec(Options) -> - Address = ?GET_INTERNAL_OPT(address, Options), - Port = ?GET_INTERNAL_OPT(port, Options), - Role = ?GET_INTERNAL_OPT(role, Options), +ssh_connection_child_spec(Role, Address, Port, _Profile, Options) -> Name = id(Role, ssh_connection_sup, Address, Port), StartFunc = {ssh_connection_sup, start_link, [Options]}, Restart = temporary, @@ -85,10 +78,7 @@ ssh_connectinon_child_spec(Options) -> Type = supervisor, {Name, StartFunc, Restart, Shutdown, Type, Modules}. -ssh_channel_child_spec(Options) -> - Address = ?GET_INTERNAL_OPT(address, Options), - Port = ?GET_INTERNAL_OPT(port, Options), - Role = ?GET_INTERNAL_OPT(role, Options), +ssh_channel_child_spec(Role, Address, Port, _Profile, Options) -> Name = id(Role, ssh_channel_sup, Address, Port), StartFunc = {ssh_channel_sup, start_link, [Options]}, Restart = temporary, diff --git a/lib/ssh/src/ssh_sup.erl b/lib/ssh/src/ssh_sup.erl index 8b57387589..5463401dcd 100644 --- a/lib/ssh/src/ssh_sup.erl +++ b/lib/ssh/src/ssh_sup.erl @@ -31,63 +31,20 @@ %%%========================================================================= %%% Supervisor callback %%%========================================================================= --spec init( [term()] ) -> {ok,{supervisor:sup_flags(),[supervisor:child_spec()]}} | ignore . - -init([]) -> +init(_) -> SupFlags = {one_for_one, 10, 3600}, - Children = children(), + Children = [child_spec(sshd_sup), child_spec(sshc_sup)], %%children(), {ok, {SupFlags, Children}}. %%%========================================================================= %%% Internal functions %%%========================================================================= -get_services() -> - case (catch application:get_env(ssh, services)) of - {ok, Services} -> - Services; - _ -> - [] - end. - -children() -> - Services = get_services(), - Clients = [Service || Service <- Services, is_client(Service)], - Servers = [Service || Service <- Services, is_server(Service)], - - [server_child_spec(Servers), client_child_spec(Clients)]. - -server_child_spec(Servers) -> - Name = sshd_sup, - StartFunc = {sshd_sup, start_link, [Servers]}, +child_spec(Name) -> + StartFunc = {Name, start_link, []}, Restart = permanent, Shutdown = infinity, - Modules = [sshd_sup], + Modules = [Name], Type = supervisor, {Name, StartFunc, Restart, Shutdown, Type, Modules}. -client_child_spec(Clients) -> - Name = sshc_sup, - StartFunc = {sshc_sup, start_link, [Clients]}, - Restart = permanent, - Shutdown = infinity, - Modules = [sshc_sup], - Type = supervisor, - {Name, StartFunc, Restart, Shutdown, Type, Modules}. - -is_server({sftpd, _}) -> - true; -is_server({shelld, _}) -> - true; -is_server(_) -> - false. - -is_client({sftpc, _}) -> - true; -is_client({shellc, _}) -> - true; -is_client(_) -> - false. - - - diff --git a/lib/ssh/src/ssh_system_sup.erl b/lib/ssh/src/ssh_system_sup.erl index 4083f666c3..a923b5ef71 100644 --- a/lib/ssh/src/ssh_system_sup.erl +++ b/lib/ssh/src/ssh_system_sup.erl @@ -31,12 +31,12 @@ -include("ssh.hrl"). --export([start_link/1, stop_listener/1, +-export([start_link/4, stop_listener/1, stop_listener/3, stop_system/1, stop_system/3, system_supervisor/3, subsystem_supervisor/1, channel_supervisor/1, connection_supervisor/1, - acceptor_supervisor/1, start_subsystem/2, restart_subsystem/3, + acceptor_supervisor/1, start_subsystem/6, restart_subsystem/3, restart_acceptor/3, stop_subsystem/2]). %% Supervisor callback @@ -45,12 +45,9 @@ %%%========================================================================= %%% Internal API %%%========================================================================= -start_link(Options) -> - Address = ?GET_INTERNAL_OPT(address, Options), - Port = ?GET_INTERNAL_OPT(port, Options), - Profile = ?GET_OPT(profile, Options), +start_link(Address, Port, Profile, Options) -> Name = make_name(Address, Port, Profile), - supervisor:start_link({local, Name}, ?MODULE, [Options]). + supervisor:start_link({local, Name}, ?MODULE, [Address, Port, Profile, Options]). stop_listener(SysSup) -> stop_acceptor(SysSup). @@ -86,8 +83,8 @@ connection_supervisor(SystemSup) -> acceptor_supervisor(SystemSup) -> ssh_acceptor_sup(supervisor:which_children(SystemSup)). -start_subsystem(SystemSup, Options) -> - Spec = ssh_subsystem_child_spec(Options), +start_subsystem(SystemSup, Role, Address, Port, Profile, Options) -> + Spec = ssh_subsystem_child_spec(Role, Address, Port, Profile, Options), supervisor:start_child(SystemSup, Spec). stop_subsystem(SystemSup, SubSys) -> @@ -125,14 +122,12 @@ restart_acceptor(Address, Port, Profile) -> %%%========================================================================= %%% Supervisor callback %%%========================================================================= --spec init( [term()] ) -> {ok,{supervisor:sup_flags(),[supervisor:child_spec()]}} | ignore . - -init([Options]) -> +init([Address, Port, Profile, Options]) -> RestartStrategy = one_for_one, MaxR = 0, MaxT = 3600, Children = case ?GET_INTERNAL_OPT(connected_socket,Options,undefined) of - undefined -> child_specs(Options); + undefined -> child_specs(Address, Port, Profile, Options); _ -> [] end, {ok, {{RestartStrategy, MaxR, MaxT}, Children}}. @@ -140,24 +135,21 @@ init([Options]) -> %%%========================================================================= %%% Internal functions %%%========================================================================= -child_specs(Options) -> - [ssh_acceptor_child_spec(Options)]. +child_specs(Address, Port, Profile, Options) -> + [ssh_acceptor_child_spec(Address, Port, Profile, Options)]. -ssh_acceptor_child_spec(Options) -> - Address = ?GET_INTERNAL_OPT(address, Options), - Port = ?GET_INTERNAL_OPT(port, Options), - Profile = ?GET_OPT(profile, Options), +ssh_acceptor_child_spec(Address, Port, Profile, Options) -> Name = id(ssh_acceptor_sup, Address, Port, Profile), - StartFunc = {ssh_acceptor_sup, start_link, [Options]}, + StartFunc = {ssh_acceptor_sup, start_link, [Address, Port, Profile, Options]}, Restart = transient, Shutdown = infinity, Modules = [ssh_acceptor_sup], Type = supervisor, {Name, StartFunc, Restart, Shutdown, Type, Modules}. -ssh_subsystem_child_spec(Options) -> +ssh_subsystem_child_spec(Role, Address, Port, Profile, Options) -> Name = make_ref(), - StartFunc = {ssh_subsystem_sup, start_link, [Options]}, + StartFunc = {ssh_subsystem_sup, start_link, [Role, Address, Port, Profile, Options]}, Restart = temporary, Shutdown = infinity, Modules = [ssh_subsystem_sup], @@ -169,7 +161,12 @@ id(Sup, Address, Port, Profile) -> {Sup, Address, Port, Profile}. make_name(Address, Port, Profile) -> - list_to_atom(lists:flatten(io_lib:format("ssh_system_~s_~p_~p_sup", [Address, Port, Profile]))). + list_to_atom(lists:flatten(io_lib:format("ssh_system_~s_~p_~p_sup", [fmt_host(Address), Port, Profile]))). + +fmt_host(IP) when is_tuple(IP) -> inet:ntoa(IP); +fmt_host(A) when is_atom(A) -> A; +fmt_host(S) when is_list(S) -> S. + ssh_subsystem_sup([{_, Child, _, [ssh_subsystem_sup]} | _]) -> Child; diff --git a/lib/ssh/src/sshc_sup.erl b/lib/ssh/src/sshc_sup.erl index 15858f36e1..9aab9d57e9 100644 --- a/lib/ssh/src/sshc_sup.erl +++ b/lib/ssh/src/sshc_sup.erl @@ -27,7 +27,7 @@ -behaviour(supervisor). --export([start_link/1, start_child/1, stop_child/1]). +-export([start_link/0, start_child/1, stop_child/1]). %% Supervisor callback -export([init/1]). @@ -35,8 +35,8 @@ %%%========================================================================= %%% API %%%========================================================================= -start_link(Args) -> - supervisor:start_link({local, ?MODULE}, ?MODULE, [Args]). +start_link() -> + supervisor:start_link({local, ?MODULE}, ?MODULE, []). start_child(Args) -> supervisor:start_child(?MODULE, Args). @@ -51,18 +51,16 @@ stop_child(Client) -> %%%========================================================================= %%% Supervisor callback %%%========================================================================= --spec init( [term()] ) -> {ok,{supervisor:sup_flags(),[supervisor:child_spec()]}} | ignore . - -init(Args) -> +init(_) -> RestartStrategy = simple_one_for_one, MaxR = 0, MaxT = 3600, - {ok, {{RestartStrategy, MaxR, MaxT}, [child_spec(Args)]}}. + {ok, {{RestartStrategy, MaxR, MaxT}, [child_spec()]}}. %%%========================================================================= %%% Internal functions %%%========================================================================= -child_spec(_) -> +child_spec() -> Name = undefined, % As simple_one_for_one is used. StartFunc = {ssh_connection_handler, start_link, []}, Restart = temporary, diff --git a/lib/ssh/src/sshd_sup.erl b/lib/ssh/src/sshd_sup.erl index 791456839d..d4805e9465 100644 --- a/lib/ssh/src/sshd_sup.erl +++ b/lib/ssh/src/sshd_sup.erl @@ -29,8 +29,11 @@ -include("ssh.hrl"). --export([start_link/1, start_child/1, stop_child/1, - stop_child/3, system_name/1]). +-export([start_link/0, + start_child/4, + stop_child/1, + stop_child/3, + system_name/1]). %% Supervisor callback -export([init/1]). @@ -38,27 +41,23 @@ %%%========================================================================= %%% API %%%========================================================================= -start_link(Servers) -> - supervisor:start_link({local, ?MODULE}, ?MODULE, [Servers]). +start_link() -> + supervisor:start_link({local, ?MODULE}, ?MODULE, []). -start_child(Options) -> - Address = ?GET_INTERNAL_OPT(address, Options), - Port = ?GET_INTERNAL_OPT(port, Options), - Profile = ?GET_OPT(profile, Options), +start_child(Address, Port, Profile, Options) -> +io:format("~p:~p ~p:~p~n",[?MODULE,?LINE,Address, Port]), case ssh_system_sup:system_supervisor(Address, Port, Profile) of undefined -> - Spec = child_spec(Address, Port, Options), - case supervisor:start_child(?MODULE, Spec) of - {error, already_present} -> - Name = id(Address, Port, Profile), - supervisor:delete_child(?MODULE, Name), - supervisor:start_child(?MODULE, Spec); - Reply -> - Reply - end; +io:format("~p:~p undefined~n",[?MODULE,?LINE]), + Spec = child_spec(Address, Port, Profile, Options), + Reply = supervisor:start_child(?MODULE, Spec), +io:format("~p:~p Reply=~p~n",[?MODULE,?LINE,Reply]), + Reply; Pid -> +io:format("~p:~p Pid=~p~n",[?MODULE,?LINE,Pid]), AccPid = ssh_system_sup:acceptor_supervisor(Pid), - ssh_acceptor_sup:start_child(AccPid, Options) + ssh_acceptor_sup:start_child(AccPid, Address, Port, Profile, Options), + {ok,Pid} end. stop_child(Name) -> @@ -75,27 +74,15 @@ system_name(SysSup) -> %%%========================================================================= %%% Supervisor callback %%%========================================================================= --spec init( [term()] ) -> {ok,{supervisor:sup_flags(),[supervisor:child_spec()]}} | ignore . - -init([Servers]) -> - RestartStrategy = one_for_one, - MaxR = 10, - MaxT = 3600, - Fun = fun(ServerOpts) -> - Address = ?GET_INTERNAL_OPT(address, ServerOpts), - Port = ?GET_INTERNAL_OPT(port, ServerOpts), - child_spec(Address, Port, ServerOpts) - end, - Children = lists:map(Fun, Servers), - {ok, {{RestartStrategy, MaxR, MaxT}, Children}}. +init(_) -> + {ok, {{one_for_one, 10, 3600}, []}}. %%%========================================================================= %%% Internal functions %%%========================================================================= -child_spec(Address, Port, Options) -> - Profile = ?GET_OPT(profile, Options), +child_spec(Address, Port, Profile, Options) -> Name = id(Address, Port,Profile), - StartFunc = {ssh_system_sup, start_link, [Options]}, + StartFunc = {ssh_system_sup, start_link, [Address, Port, Profile, Options]}, Restart = temporary, Shutdown = infinity, Modules = [ssh_system_sup], -- cgit v1.2.3 From 8f4bb9b0bd3aed663521371726ea3ec460e231a0 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Thu, 23 Mar 2017 16:53:53 +0100 Subject: ssh: Mappify supervisors --- lib/ssh/src/ssh_acceptor_sup.erl | 30 ++++--- lib/ssh/src/ssh_connection_sup.erl | 28 +++---- lib/ssh/src/ssh_subsystem_sup.erl | 39 ++++----- lib/ssh/src/ssh_sup.erl | 30 +++---- lib/ssh/src/ssh_system_sup.erl | 159 ++++++++++++++++--------------------- lib/ssh/src/sshc_sup.erl | 35 ++++---- lib/ssh/src/sshd_sup.erl | 73 +++++++++-------- 7 files changed, 190 insertions(+), 204 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/src/ssh_acceptor_sup.erl b/lib/ssh/src/ssh_acceptor_sup.erl index 4606107f56..3ad842f98c 100644 --- a/lib/ssh/src/ssh_acceptor_sup.erl +++ b/lib/ssh/src/ssh_acceptor_sup.erl @@ -48,9 +48,12 @@ start_child(AccSup, Address, Port, Profile, Options) -> Spec = child_spec(Address, Port, Profile, Options), case supervisor:start_child(AccSup, Spec) of {error, already_present} -> + %% Is this ever called? stop_child(AccSup, Address, Port, Profile), supervisor:start_child(AccSup, Spec); Reply -> + %% Reply = {ok,SystemSupPid} when the user calls ssh:daemon + %% after having called ssh:stop_listening Reply end. @@ -67,24 +70,27 @@ stop_child(AccSup, Address, Port, Profile) -> %%% Supervisor callback %%%========================================================================= init([Address, Port, Profile, Options]) -> - RestartStrategy = one_for_one, - MaxR = 10, - MaxT = 3600, - Children = [child_spec(Address, Port, Profile, Options)], - {ok, {{RestartStrategy, MaxR, MaxT}, Children}}. + %% Initial start of ssh_acceptor_sup for this port or new start after + %% ssh:stop_daemon + SupFlags = #{strategy => one_for_one, + intensity => 10, + period => 3600 + }, + ChildSpecs = [child_spec(Address, Port, Profile, Options)], + {ok, {SupFlags,ChildSpecs}}. %%%========================================================================= %%% Internal functions %%%========================================================================= child_spec(Address, Port, Profile, Options) -> Timeout = ?GET_INTERNAL_OPT(timeout, Options, ?DEFAULT_TIMEOUT), - Name = id(Address, Port, Profile), - StartFunc = {ssh_acceptor, start_link, [Port, Address, Options, Timeout]}, - Restart = transient, - Shutdown = brutal_kill, - Modules = [ssh_acceptor], - Type = worker, - {Name, StartFunc, Restart, Shutdown, Type, Modules}. + #{id => id(Address, Port, Profile), + start => {ssh_acceptor, start_link, [Port, Address, Options, Timeout]}, + restart => transient, + shutdown => brutal_kill, + type => worker, + modules => [ssh_acceptor] + }. id(Address, Port, Profile) -> {ssh_acceptor_sup, Address, Port, Profile}. diff --git a/lib/ssh/src/ssh_connection_sup.erl b/lib/ssh/src/ssh_connection_sup.erl index 0f54053f52..fad796f196 100644 --- a/lib/ssh/src/ssh_connection_sup.erl +++ b/lib/ssh/src/ssh_connection_sup.erl @@ -45,19 +45,17 @@ start_child(Sup, Args) -> %%%========================================================================= %%% Supervisor callback %%%========================================================================= --spec init( [term()] ) -> {ok,{supervisor:sup_flags(),[supervisor:child_spec()]}} | ignore . - init(_) -> - RestartStrategy = simple_one_for_one, - MaxR = 0, - MaxT = 3600, - - Name = undefined, % As simple_one_for_one is used. - StartFunc = {ssh_connection_handler, start_link, []}, - Restart = temporary, % E.g. should not be restarted - Shutdown = 4000, - Modules = [ssh_connection_handler], - Type = worker, - - ChildSpec = {Name, StartFunc, Restart, Shutdown, Type, Modules}, - {ok, {{RestartStrategy, MaxR, MaxT}, [ChildSpec]}}. + SupFlags = #{strategy => simple_one_for_one, + intensity => 0, + period => 3600 + }, + ChildSpecs = [#{id => undefined, % As simple_one_for_one is used. + start => {ssh_connection_handler, start_link, []}, + restart => temporary, + shutdown => 4000, + type => worker, + modules => [ssh_connection_handler] + } + ], + {ok, {SupFlags,ChildSpecs}}. diff --git a/lib/ssh/src/ssh_subsystem_sup.erl b/lib/ssh/src/ssh_subsystem_sup.erl index c5ab422265..cf409ade6b 100644 --- a/lib/ssh/src/ssh_subsystem_sup.erl +++ b/lib/ssh/src/ssh_subsystem_sup.erl @@ -54,11 +54,12 @@ channel_supervisor(SupPid) -> %%% Supervisor callback %%%========================================================================= init([Role, Address, Port, Profile, Options]) -> - RestartStrategy = one_for_all, - MaxR = 0, - MaxT = 3600, - Children = child_specs(Role, Address, Port, Profile, Options), - {ok, {{RestartStrategy, MaxR, MaxT}, Children}}. + SupFlags = #{strategy => one_for_all, + intensity => 0, + period => 3600 + }, + ChildSpecs = child_specs(Role, Address, Port, Profile, Options), + {ok, {SupFlags,ChildSpecs}}. %%%========================================================================= %%% Internal functions @@ -70,22 +71,22 @@ child_specs(server, Address, Port, Profile, Options) -> ssh_connection_child_spec(server, Address, Port, Profile, Options)]. ssh_connection_child_spec(Role, Address, Port, _Profile, Options) -> - Name = id(Role, ssh_connection_sup, Address, Port), - StartFunc = {ssh_connection_sup, start_link, [Options]}, - Restart = temporary, - Shutdown = 5000, - Modules = [ssh_connection_sup], - Type = supervisor, - {Name, StartFunc, Restart, Shutdown, Type, Modules}. + #{id => id(Role, ssh_connection_sup, Address, Port), + start => {ssh_connection_sup, start_link, [Options]}, + restart => temporary, + shutdown => 5000, + type => supervisor, + modules => [ssh_connection_sup] + }. ssh_channel_child_spec(Role, Address, Port, _Profile, Options) -> - Name = id(Role, ssh_channel_sup, Address, Port), - StartFunc = {ssh_channel_sup, start_link, [Options]}, - Restart = temporary, - Shutdown = infinity, - Modules = [ssh_channel_sup], - Type = supervisor, - {Name, StartFunc, Restart, Shutdown, Type, Modules}. + #{id => id(Role, ssh_channel_sup, Address, Port), + start => {ssh_channel_sup, start_link, [Options]}, + restart => temporary, + shutdown => infinity, + type => supervisor, + modules => [ssh_channel_sup] + }. id(Role, Sup, Address, Port) -> {Role, Sup, Address, Port}. diff --git a/lib/ssh/src/ssh_sup.erl b/lib/ssh/src/ssh_sup.erl index 5463401dcd..6be809b1bd 100644 --- a/lib/ssh/src/ssh_sup.erl +++ b/lib/ssh/src/ssh_sup.erl @@ -32,19 +32,19 @@ %%% Supervisor callback %%%========================================================================= init(_) -> - SupFlags = {one_for_one, 10, 3600}, - Children = [child_spec(sshd_sup), child_spec(sshc_sup)], %%children(), - {ok, {SupFlags, Children}}. - -%%%========================================================================= -%%% Internal functions -%%%========================================================================= -child_spec(Name) -> - StartFunc = {Name, start_link, []}, - Restart = permanent, - Shutdown = infinity, - Modules = [Name], - Type = supervisor, - {Name, StartFunc, Restart, Shutdown, Type, Modules}. - + SupFlags = #{strategy => one_for_one, + intensity => 10, + period => 3600 + }, + ChildSpecs = [#{id => Module, + start => {Module, start_link, []}, + restart => permanent, + shutdown => brutal_kill, + type => supervisor, + modules => [Module] + } + || Module <- [sshd_sup, + sshc_sup] + ], + {ok, {SupFlags,ChildSpecs}}. diff --git a/lib/ssh/src/ssh_system_sup.erl b/lib/ssh/src/ssh_system_sup.erl index a923b5ef71..84b4cd3241 100644 --- a/lib/ssh/src/ssh_system_sup.erl +++ b/lib/ssh/src/ssh_system_sup.erl @@ -21,7 +21,7 @@ %% %%---------------------------------------------------------------------- %% Purpose: The ssh server instance supervisor, an instans of this supervisor -%% exists for every ip-address and port combination, hangs under +%% exists for every ip-address and port combination, hangs under %% sshd_sup. %%---------------------------------------------------------------------- @@ -34,58 +34,100 @@ -export([start_link/4, stop_listener/1, stop_listener/3, stop_system/1, stop_system/3, system_supervisor/3, - subsystem_supervisor/1, channel_supervisor/1, - connection_supervisor/1, - acceptor_supervisor/1, start_subsystem/6, restart_subsystem/3, - restart_acceptor/3, stop_subsystem/2]). + subsystem_supervisor/1, channel_supervisor/1, + connection_supervisor/1, + acceptor_supervisor/1, start_subsystem/6, + stop_subsystem/2]). %% Supervisor callback -export([init/1]). %%%========================================================================= -%%% Internal API +%%% API %%%========================================================================= start_link(Address, Port, Profile, Options) -> Name = make_name(Address, Port, Profile), supervisor:start_link({local, Name}, ?MODULE, [Address, Port, Profile, Options]). -stop_listener(SysSup) -> - stop_acceptor(SysSup). +%%%========================================================================= +%%% Supervisor callback +%%%========================================================================= +init([Address, Port, Profile, Options]) -> + SupFlags = #{strategy => one_for_one, + intensity => 0, + period => 3600 + }, + ChildSpecs = + case ?GET_INTERNAL_OPT(connected_socket,Options,undefined) of + undefined -> + [#{id => id(ssh_acceptor_sup, Address, Port, Profile), + start => {ssh_acceptor_sup, start_link, [Address, Port, Profile, Options]}, + restart => transient, + shutdown => infinity, + type => supervisor, + modules => [ssh_acceptor_sup] + }]; + _ -> + [] + end, + {ok, {SupFlags,ChildSpecs}}. + +%%%========================================================================= +%%% Service API +%%%========================================================================= +stop_listener(SystemSup) -> + {Name, AcceptorSup, _, _} = lookup(ssh_acceptor_sup, SystemSup), + case supervisor:terminate_child(AcceptorSup, Name) of + ok -> + supervisor:delete_child(AcceptorSup, Name); + Error -> + Error + end. stop_listener(Address, Port, Profile) -> - Name = make_name(Address, Port, Profile), - stop_acceptor(whereis(Name)). - + stop_listener( + system_supervisor(Address, Port, Profile)). + + stop_system(SysSup) -> - Name = sshd_sup:system_name(SysSup), - spawn(fun() -> sshd_sup:stop_child(Name) end), + spawn(fun() -> sshd_sup:stop_child(SysSup) end), ok. -stop_system(Address, Port, Profile) -> +stop_system(Address, Port, Profile) -> spawn(fun() -> sshd_sup:stop_child(Address, Port, Profile) end), ok. + system_supervisor(Address, Port, Profile) -> Name = make_name(Address, Port, Profile), whereis(Name). subsystem_supervisor(SystemSup) -> - ssh_subsystem_sup(supervisor:which_children(SystemSup)). + {_, Child, _, _} = lookup(ssh_subsystem_sup, SystemSup), + Child. channel_supervisor(SystemSup) -> - SubSysSup = ssh_subsystem_sup(supervisor:which_children(SystemSup)), - ssh_subsystem_sup:channel_supervisor(SubSysSup). + ssh_subsystem_sup:channel_supervisor( + subsystem_supervisor(SystemSup)). connection_supervisor(SystemSup) -> - SubSysSup = ssh_subsystem_sup(supervisor:which_children(SystemSup)), - ssh_subsystem_sup:connection_supervisor(SubSysSup). + ssh_subsystem_sup:connection_supervisor( + subsystem_supervisor(SystemSup)). acceptor_supervisor(SystemSup) -> - ssh_acceptor_sup(supervisor:which_children(SystemSup)). + {_, Child, _, _} = lookup(ssh_acceptor_sup, SystemSup), + Child. + start_subsystem(SystemSup, Role, Address, Port, Profile, Options) -> - Spec = ssh_subsystem_child_spec(Role, Address, Port, Profile, Options), - supervisor:start_child(SystemSup, Spec). + SubsystemSpec = + #{id => make_ref(), + start => {ssh_subsystem_sup, start_link, [Role, Address, Port, Profile, Options]}, + restart => temporary, + shutdown => infinity, + type => supervisor, + modules => [ssh_subsystem_sup]}, + supervisor:start_child(SystemSup, SubsystemSpec). stop_subsystem(SystemSup, SubSys) -> case catch lists:keyfind(SubSys, 2, supervisor:which_children(SystemSup)) of @@ -103,60 +145,9 @@ stop_subsystem(SystemSup, SubSys) -> ok end. - -restart_subsystem(Address, Port, Profile) -> - SysSupName = make_name(Address, Port, Profile), - SubSysName = id(ssh_subsystem_sup, Address, Port, Profile), - case supervisor:terminate_child(SysSupName, SubSysName) of - ok -> - supervisor:restart_child(SysSupName, SubSysName); - Error -> - Error - end. - -restart_acceptor(Address, Port, Profile) -> - SysSupName = make_name(Address, Port, Profile), - AcceptorName = id(ssh_acceptor_sup, Address, Port, Profile), - supervisor:restart_child(SysSupName, AcceptorName). - -%%%========================================================================= -%%% Supervisor callback -%%%========================================================================= -init([Address, Port, Profile, Options]) -> - RestartStrategy = one_for_one, - MaxR = 0, - MaxT = 3600, - Children = case ?GET_INTERNAL_OPT(connected_socket,Options,undefined) of - undefined -> child_specs(Address, Port, Profile, Options); - _ -> [] - end, - {ok, {{RestartStrategy, MaxR, MaxT}, Children}}. - %%%========================================================================= %%% Internal functions %%%========================================================================= -child_specs(Address, Port, Profile, Options) -> - [ssh_acceptor_child_spec(Address, Port, Profile, Options)]. - -ssh_acceptor_child_spec(Address, Port, Profile, Options) -> - Name = id(ssh_acceptor_sup, Address, Port, Profile), - StartFunc = {ssh_acceptor_sup, start_link, [Address, Port, Profile, Options]}, - Restart = transient, - Shutdown = infinity, - Modules = [ssh_acceptor_sup], - Type = supervisor, - {Name, StartFunc, Restart, Shutdown, Type, Modules}. - -ssh_subsystem_child_spec(Role, Address, Port, Profile, Options) -> - Name = make_ref(), - StartFunc = {ssh_subsystem_sup, start_link, [Role, Address, Port, Profile, Options]}, - Restart = temporary, - Shutdown = infinity, - Modules = [ssh_subsystem_sup], - Type = supervisor, - {Name, StartFunc, Restart, Shutdown, Type, Modules}. - - id(Sup, Address, Port, Profile) -> {Sup, Address, Port, Profile}. @@ -168,23 +159,7 @@ fmt_host(A) when is_atom(A) -> A; fmt_host(S) when is_list(S) -> S. -ssh_subsystem_sup([{_, Child, _, [ssh_subsystem_sup]} | _]) -> - Child; -ssh_subsystem_sup([_ | Rest]) -> - ssh_subsystem_sup(Rest). - -ssh_acceptor_sup([{_, Child, _, [ssh_acceptor_sup]} | _]) -> - Child; -ssh_acceptor_sup([_ | Rest]) -> - ssh_acceptor_sup(Rest). +lookup(SupModule, SystemSup) -> + lists:keyfind([SupModule], 4, + supervisor:which_children(SystemSup)). -stop_acceptor(Sup) -> - [{Name, AcceptorSup}] = - [{SupName, ASup} || {SupName, ASup, _, [ssh_acceptor_sup]} <- - supervisor:which_children(Sup)], - case supervisor:terminate_child(AcceptorSup, Name) of - ok -> - supervisor:delete_child(AcceptorSup, Name); - Error -> - Error - end. diff --git a/lib/ssh/src/sshc_sup.erl b/lib/ssh/src/sshc_sup.erl index 9aab9d57e9..c71b81dc6d 100644 --- a/lib/ssh/src/sshc_sup.erl +++ b/lib/ssh/src/sshc_sup.erl @@ -32,18 +32,20 @@ %% Supervisor callback -export([init/1]). +-define(SSHC_SUP, ?MODULE). + %%%========================================================================= %%% API %%%========================================================================= start_link() -> - supervisor:start_link({local, ?MODULE}, ?MODULE, []). + supervisor:start_link({local,?SSHC_SUP}, ?MODULE, []). start_child(Args) -> supervisor:start_child(?MODULE, Args). stop_child(Client) -> spawn(fun() -> - ClientSup = whereis(?MODULE), + ClientSup = whereis(?SSHC_SUP), supervisor:terminate_child(ClientSup, Client) end), ok. @@ -52,19 +54,16 @@ stop_child(Client) -> %%% Supervisor callback %%%========================================================================= init(_) -> - RestartStrategy = simple_one_for_one, - MaxR = 0, - MaxT = 3600, - {ok, {{RestartStrategy, MaxR, MaxT}, [child_spec()]}}. - -%%%========================================================================= -%%% Internal functions -%%%========================================================================= -child_spec() -> - Name = undefined, % As simple_one_for_one is used. - StartFunc = {ssh_connection_handler, start_link, []}, - Restart = temporary, - Shutdown = 4000, - Modules = [ssh_connection_handler], - Type = worker, - {Name, StartFunc, Restart, Shutdown, Type, Modules}. + SupFlags = #{strategy => simple_one_for_one, + intensity => 0, + period => 3600 + }, + ChildSpecs = [#{id => undefined, % As simple_one_for_one is used. + start => {ssh_connection_handler, start_link, []}, + restart => temporary, + shutdown => 4000, + type => worker, + modules => [ssh_connection_handler] + } + ], + {ok, {SupFlags,ChildSpecs}}. diff --git a/lib/ssh/src/sshd_sup.erl b/lib/ssh/src/sshd_sup.erl index d4805e9465..449ba20d02 100644 --- a/lib/ssh/src/sshd_sup.erl +++ b/lib/ssh/src/sshd_sup.erl @@ -19,7 +19,7 @@ %% %% %%---------------------------------------------------------------------- -%% Purpose: The top supervisor for ssh servers hangs under +%% Purpose: The top supervisor for ssh servers hangs under %% ssh_sup. %%---------------------------------------------------------------------- @@ -29,72 +29,79 @@ -include("ssh.hrl"). --export([start_link/0, +-export([start_link/0, start_child/4, stop_child/1, - stop_child/3, - system_name/1]). + stop_child/3 +]). %% Supervisor callback -export([init/1]). +-define(SSHD_SUP, ?MODULE). + %%%========================================================================= %%% API %%%========================================================================= start_link() -> - supervisor:start_link({local, ?MODULE}, ?MODULE, []). + %% No children are start now. We wait until the user calls ssh:daemon + %% and uses start_child/4 to create the children + supervisor:start_link({local,?SSHD_SUP}, ?MODULE, []). start_child(Address, Port, Profile, Options) -> -io:format("~p:~p ~p:~p~n",[?MODULE,?LINE,Address, Port]), case ssh_system_sup:system_supervisor(Address, Port, Profile) of undefined -> -io:format("~p:~p undefined~n",[?MODULE,?LINE]), + %% Here we start listening on a new Host/Port/Profile Spec = child_spec(Address, Port, Profile, Options), - Reply = supervisor:start_child(?MODULE, Spec), -io:format("~p:~p Reply=~p~n",[?MODULE,?LINE,Reply]), - Reply; + supervisor:start_child(?SSHD_SUP, Spec); Pid -> -io:format("~p:~p Pid=~p~n",[?MODULE,?LINE,Pid]), + %% Here we resume listening on a new Host/Port/Profile after + %% haveing stopped listening to he same with ssh:stop_listen(Pid) AccPid = ssh_system_sup:acceptor_supervisor(Pid), ssh_acceptor_sup:start_child(AccPid, Address, Port, Profile, Options), {ok,Pid} end. -stop_child(Name) -> - supervisor:terminate_child(?MODULE, Name). +stop_child(ChildId) when is_tuple(ChildId) -> + supervisor:terminate_child(?SSHD_SUP, ChildId); +stop_child(ChildPid) when is_pid(ChildPid)-> + stop_child(system_name(ChildPid)). -stop_child(Address, Port, Profile) -> - Name = id(Address, Port, Profile), - stop_child(Name). -system_name(SysSup) -> - Children = supervisor:which_children(sshd_sup), - system_name(SysSup, Children). +stop_child(Address, Port, Profile) -> + Id = id(Address, Port, Profile), + stop_child(Id). %%%========================================================================= %%% Supervisor callback %%%========================================================================= init(_) -> - {ok, {{one_for_one, 10, 3600}, []}}. + SupFlags = #{strategy => one_for_one, + intensity => 10, + period => 3600 + }, + ChildSpecs = [ + ], + {ok, {SupFlags,ChildSpecs}}. %%%========================================================================= %%% Internal functions %%%========================================================================= child_spec(Address, Port, Profile, Options) -> - Name = id(Address, Port,Profile), - StartFunc = {ssh_system_sup, start_link, [Address, Port, Profile, Options]}, - Restart = temporary, - Shutdown = infinity, - Modules = [ssh_system_sup], - Type = supervisor, - {Name, StartFunc, Restart, Shutdown, Type, Modules}. + #{id => id(Address, Port, Profile), + start => {ssh_system_sup, start_link, [Address, Port, Profile, Options]}, + restart => temporary, + shutdown => infinity, + type => supervisor, + modules => [ssh_system_sup] + }. id(Address, Port, Profile) -> {server, ssh_system_sup, Address, Port, Profile}. -system_name([], _ ) -> - undefined; -system_name(SysSup, [{Name, SysSup, _, _} | _]) -> - Name; -system_name(SysSup, [_ | Rest]) -> - system_name(SysSup, Rest). +system_name(SysSup) -> + case lists:keyfind(SysSup, 2, supervisor:which_children(?SSHD_SUP)) of + {Name, SysSup, _, _} -> Name; + false -> undefind + end. + -- cgit v1.2.3 From 92bdf870ec6509eb958535780b8655206478f7db Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Tue, 28 Mar 2017 17:20:46 +0200 Subject: ssh: change 'brutal_kill' to timeout' --- lib/ssh/src/ssh_acceptor_sup.erl | 2 +- lib/ssh/src/ssh_sup.erl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/src/ssh_acceptor_sup.erl b/lib/ssh/src/ssh_acceptor_sup.erl index 3ad842f98c..26defcfdbd 100644 --- a/lib/ssh/src/ssh_acceptor_sup.erl +++ b/lib/ssh/src/ssh_acceptor_sup.erl @@ -87,7 +87,7 @@ child_spec(Address, Port, Profile, Options) -> #{id => id(Address, Port, Profile), start => {ssh_acceptor, start_link, [Port, Address, Options, Timeout]}, restart => transient, - shutdown => brutal_kill, + shutdown => 5500, %brutal_kill, type => worker, modules => [ssh_acceptor] }. diff --git a/lib/ssh/src/ssh_sup.erl b/lib/ssh/src/ssh_sup.erl index 6be809b1bd..26574763e4 100644 --- a/lib/ssh/src/ssh_sup.erl +++ b/lib/ssh/src/ssh_sup.erl @@ -39,7 +39,7 @@ init(_) -> ChildSpecs = [#{id => Module, start => {Module, start_link, []}, restart => permanent, - shutdown => brutal_kill, + shutdown => 4000, %brutal_kill, type => supervisor, modules => [Module] } -- cgit v1.2.3 From 3bed79615b9702f8335dbe75295c6610b097175e Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Wed, 29 Mar 2017 13:13:43 +0200 Subject: ssh: remove dead code and add comments --- lib/ssh/src/ssh.erl | 55 ++++++++--------------------------------------------- 1 file changed, 8 insertions(+), 47 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/src/ssh.erl b/lib/ssh/src/ssh.erl index 680047dffd..aff143dc26 100644 --- a/lib/ssh/src/ssh.erl +++ b/lib/ssh/src/ssh.erl @@ -416,66 +416,27 @@ handle_daemon_args(IPaddr, Opts) when is_tuple(IPaddr) -> IP -> {IPaddr, [{ip,IPaddr}|Opts--[{ip,IP}]]} %% Backward compatibility end; -handle_daemon_args(Address, Opts) when is_list(Address) ; is_atom(Address) -> +handle_daemon_args(Address, Opts) when is_list(Address) ; % IP address in string or a domain + is_atom(Address) % domains could be atoms in inet + -> IP = proplists:get_value(ip, Opts), case inet:parse_strict_address(Address) of + %% check if Address is an IP-address {ok, IP} -> {IP, Opts}; {ok, OtherIP} -> {OtherIP, [{ip,OtherIP}|Opts--[{ip,IP}]]}; _ -> + %% Not an IP-address. Check if it is a host name: case inet:getaddr(Address, family(Opts)) of {ok, IP} -> {Address, Opts}; {ok, OtherIP} -> {Address, [{ip,OtherIP}|Opts--[{ip,IP}]]}; - _ -> {Address, Opts} - end - end. - - --ifdef(hulahopp). -%% Check the Address parameter and set an ip-option in some cases. The -%% Address parameter is left unchanged because ssh:stop_listener and -%% ssh:stop_daemon needs to find the system supervisor by name - -handle_daemon_args(any, Opts) -> - %% Listen to 0.0.0.0. The caller may have set an ip-option. Trust - %% that one in such a case. - {any, Opts}; - -handle_daemon_args(loopback, Opts) -> - %% Listen to a loopback address. Let the underlying layers decide - %% in case the caller hasn't set the ip-option. - {loopback, ensure_ip_option(loopback,Opts)}; - -handle_daemon_args(IP, Opts) when is_tuple(IP) -> - %% An IP address in Erlang tuple format: - {IP, ensure_ip_option(IP,Opts)}; - -handle_daemon_args(Address, Opts) when is_list(Address) ; is_atom(Address) -> - %% This might be a host name, an FQDN, an IP address in string format ("127.1.1.1") - %% etc. It might be a string or an atom since inet:hostname() is defined in that way - case inet:parse_strict_address(Address) of - {ok, IP} -> - {Address, ensure_ip_option(IP,Opts)}; - _ -> - %% Try to lookup as a hostname: - case inet:getaddr(Address, family(Opts)) of - {ok, IP} -> - {Address, ensure_ip_option(IP,Opts)}; _ -> - %% Give up and let the underlying system handle this + %% Not a Host name and not an IP address, let + %% inet and the OS later figure out what it + %% could be {Address, Opts} end end. - -%% Add an ip-option if not already present. -ensure_ip_option(Address, Opts) -> - case proplists:get_value(ip, Opts) of - undefined -> [{ip,Address}|Opts]; - _ -> Opts - end. --endif. - - %% Has the caller indicated the address family? family(Opts) -> family(Opts, inet). -- cgit v1.2.3 From a9fc169d5d0ca63a3062800429e3a16169901ab3 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Wed, 29 Mar 2017 15:08:48 +0200 Subject: ssh: Change handling of IP addresses, 'any' and names in sup structure --- lib/ssh/src/ssh.erl | 100 ++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 70 insertions(+), 30 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/src/ssh.erl b/lib/ssh/src/ssh.erl index aff143dc26..8c802d46eb 100644 --- a/lib/ssh/src/ssh.erl +++ b/lib/ssh/src/ssh.erl @@ -26,6 +26,7 @@ -include("ssh_connect.hrl"). -include_lib("public_key/include/public_key.hrl"). -include_lib("kernel/include/file.hrl"). +-include_lib("kernel/include/inet.hrl"). -export([start/0, start/1, stop/0, connect/2, connect/3, connect/4, @@ -120,7 +121,7 @@ connect(Host, Port, UserOptions) when is_integer(Port), is_list(UserOptions) -> connect(Host, Port, UserOptions, infinity). -connect(Host, Port, UserOptions, Timeout) when is_integer(Port), +connect(Host0, Port, UserOptions, Timeout) when is_integer(Port), Port>0, is_list(UserOptions) -> case ssh_options:handle_options(client, UserOptions) of @@ -130,6 +131,7 @@ connect(Host, Port, UserOptions, Timeout) when is_integer(Port), {_, Transport, _} = TransportOpts = ?GET_OPT(transport, Options), ConnectionTimeout = ?GET_OPT(connect_timeout, Options), SocketOpts = [{active,false} | ?GET_OPT(socket_options,Options)], + Host = mangle_connect_address(Host0, SocketOpts), try Transport:connect(Host, Port, SocketOpts, ConnectionTimeout) of {ok, Socket} -> Opts = ?PUT_INTERNAL_OPT([{user_pid,self()}, {host,Host}], Options), @@ -227,7 +229,7 @@ daemon(Host0, Port0, UserOptions0) when 0 =< Port0, Port0 =< 65535 -> try {Host1, UserOptions} = handle_daemon_args(Host0, UserOptions0), #{} = Options0 = ssh_options:handle_options(server, UserOptions), - + {{Host,Port}, ListenSocket} = open_listen_socket(Host1, Port0, Options0), @@ -266,27 +268,23 @@ daemon(Host0, Port0, UserOptions0) when 0 =< Port0, Port0 =< 65535 -> daemon_info(Pid) -> case catch ssh_system_sup:acceptor_supervisor(Pid) of AsupPid when is_pid(AsupPid) -> - [{Name,Port,Profile}] = - [{Nam,Prt,Prf} + [{IP,Port,Profile}] = + [{IP,Prt,Prf} || {{ssh_acceptor_sup,Hst,Prt,Prf},_Pid,worker,[ssh_acceptor]} <- supervisor:which_children(AsupPid), - Nam <- [case inet:parse_strict_address(Hst) of - {ok,IP} -> IP; - _ when Hst=="any" -> any; - _ when Hst=="loopback" -> loopback; - _ -> Hst - end] + IP <- [case inet:parse_strict_address(Hst) of + {ok,IP} -> IP; + _ -> Hst + end] ], {ok, [{port,Port}, - {name,Name}, + {ip,IP}, {profile,Profile} ]}; _ -> {error,bad_daemon_ref} end. - - %%-------------------------------------------------------------------- -spec stop_listener(daemon_ref()) -> ok. -spec stop_listener(inet:ip_address(), inet:port_number()) -> ok. @@ -298,8 +296,14 @@ stop_listener(SysSup) -> ssh_system_sup:stop_listener(SysSup). stop_listener(Address, Port) -> stop_listener(Address, Port, ?DEFAULT_PROFILE). +stop_listener(any, Port, Profile) -> + map_ip(fun(IP) -> + ssh_system_sup:stop_listener(IP, Port, Profile) + end, [{0,0,0,0},{0,0,0,0,0,0,0,0}]); stop_listener(Address, Port, Profile) -> - ssh_system_sup:stop_listener(Address, Port, Profile). + map_ip(fun(IP) -> + ssh_system_sup:stop_listener(IP, Port, Profile) + end, {address,Address}). %%-------------------------------------------------------------------- -spec stop_daemon(daemon_ref()) -> ok. @@ -312,9 +316,15 @@ stop_listener(Address, Port, Profile) -> stop_daemon(SysSup) -> ssh_system_sup:stop_system(SysSup). stop_daemon(Address, Port) -> - ssh_system_sup:stop_system(Address, Port, ?DEFAULT_PROFILE). + stop_daemon(Address, Port, ?DEFAULT_PROFILE). +stop_daemon(any, Port, Profile) -> + map_ip(fun(IP) -> + ssh_system_sup:stop_system(IP, Port, Profile) + end, [{0,0,0,0},{0,0,0,0,0,0,0,0}]); stop_daemon(Address, Port, Profile) -> - ssh_system_sup:stop_system(Address, Port, Profile). + map_ip(fun(IP) -> + ssh_system_sup:stop_system(IP, Port, Profile) + end, {address,Address}). %%-------------------------------------------------------------------- -spec shell(inet:socket() | string()) -> _. @@ -397,6 +407,9 @@ default_algorithms() -> %% consideration %% +%% The handle_daemon_args/2 function basically only sets the ip-option in Opts +%% so that it is correctly set when opening the listening socket. + handle_daemon_args(any, Opts) -> case proplists:get_value(ip, Opts) of undefined -> {any, Opts}; @@ -477,20 +490,17 @@ is_tcp_socket(Socket) -> end. %%%---------------------------------------------------------------- -open_listen_socket(Host0, Port0, Options0) -> - case ?GET_SOCKET_OPT(fd, Options0) of - undefined -> - {ok,LSock} = ssh_acceptor:listen(Port0, Options0), - {ok,{_LHost,LPort}} = inet:sockname(LSock), - {{_LHost,LPort}, LSock}; -%% {{Host0,LPort}, LSock}; - - Fd when is_integer(Fd) -> - %% Do gen_tcp:listen with the option {fd,Fd}: - {ok,LSock} = ssh_acceptor:listen(0, Options0), - {ok,{LHost,LPort}} = inet:sockname(LSock), - {{LHost,LPort}, LSock} - end. +open_listen_socket(_Host0, Port0, Options0) -> + {ok,LSock} = + case ?GET_SOCKET_OPT(fd, Options0) of + undefined -> + ssh_acceptor:listen(Port0, Options0); + Fd when is_integer(Fd) -> + %% Do gen_tcp:listen with the option {fd,Fd}: + ssh_acceptor:listen(0, Options0) + end, + {ok,{LHost,LPort}} = inet:sockname(LSock), + {{LHost,LPort}, LSock}. %%%---------------------------------------------------------------- finalize_start(Host, Port, Profile, Options0, F) -> @@ -509,3 +519,33 @@ finalize_start(Host, Port, Profile, Options0, F) -> end. %%%---------------------------------------------------------------- +map_ip(Fun, {address,IP}) when is_tuple(IP) -> + Fun(IP); +map_ip(Fun, {address,Address}) -> + IPs = try {ok,#hostent{h_addr_list=IP0s}} = inet:gethostbyname(Address), + IP0s + catch + _:_ -> [] + end, + map_ip(Fun, IPs); +map_ip(Fun, IPs) -> + lists:map(Fun, IPs). + +%%%---------------------------------------------------------------- +mangle_connect_address(A, SockOpts) -> + mangle_connect_address1(A, proplists:get_value(inet6,SockOpts,false)). + +loopback(true) -> {0,0,0,0,0,0,0,1}; +loopback(false) -> {127,0,0,1}. + +mangle_connect_address1( loopback, V6flg) -> loopback(V6flg); +mangle_connect_address1( any, V6flg) -> loopback(V6flg); +mangle_connect_address1({0,0,0,0}, _) -> loopback(false); +mangle_connect_address1({0,0,0,0,0,0,0,0}, _) -> loopback(true); +mangle_connect_address1( IP, _) when is_tuple(IP) -> IP; +mangle_connect_address1(A, _) -> + case catch inet:parse_address(A) of + {ok, {0,0,0,0}} -> loopback(false); + {ok, {0,0,0,0,0,0,0,0}} -> loopback(true); + _ -> A + end. -- cgit v1.2.3 From 2f91341ae855b28c82024caa87c7541e94f68a18 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Wed, 29 Mar 2017 12:57:23 +0200 Subject: ssh: Make test suites pass --- lib/ssh/test/ssh_algorithms_SUITE.erl | 13 ++-- lib/ssh/test/ssh_basic_SUITE.erl | 3 +- lib/ssh/test/ssh_bench_SUITE.erl | 6 +- lib/ssh/test/ssh_connection_SUITE.erl | 64 +++++++++--------- lib/ssh/test/ssh_options_SUITE.erl | 24 +++---- lib/ssh/test/ssh_relay.erl | 3 +- lib/ssh/test/ssh_sftp_SUITE.erl | 2 +- lib/ssh/test/ssh_sftpd_SUITE.erl | 4 +- lib/ssh/test/ssh_sup_SUITE.erl | 31 +++++---- lib/ssh/test/ssh_test_lib.erl | 124 +++++++++++++++++++++++++++++++--- lib/ssh/test/ssh_to_openssh_SUITE.erl | 28 ++++---- lib/ssh/test/ssh_trpt_test_lib.erl | 3 +- 12 files changed, 210 insertions(+), 95 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/test/ssh_algorithms_SUITE.erl b/lib/ssh/test/ssh_algorithms_SUITE.erl index 6f75d83c4a..2990d1e02a 100644 --- a/lib/ssh/test/ssh_algorithms_SUITE.erl +++ b/lib/ssh/test/ssh_algorithms_SUITE.erl @@ -235,13 +235,12 @@ sshc_simple_exec_os_cmd(Config) -> Parent = self(), Client = spawn( fun() -> - Cmd = lists:concat(["ssh -p ",Port, - " -C" - " -o UserKnownHostsFile=",KnownHosts, - " -o StrictHostKeyChecking=no" - " ",Host," 1+1."]), - Result = os:cmd(Cmd), - ct:log("~p~n = ~p",[Cmd, Result]), + Result = ssh_test_lib:open_sshc(Host, Port, + [" -C" + " -o UserKnownHostsFile=",KnownHosts, + " -o StrictHostKeyChecking=no" + ], + " 1+1."), Parent ! {result, self(), Result, "2"} end), receive diff --git a/lib/ssh/test/ssh_basic_SUITE.erl b/lib/ssh/test/ssh_basic_SUITE.erl index a9b6be222e..089d191fea 100644 --- a/lib/ssh/test/ssh_basic_SUITE.erl +++ b/lib/ssh/test/ssh_basic_SUITE.erl @@ -742,7 +742,8 @@ known_hosts(Config) when is_list(Config) -> Lines = string:tokens(binary_to_list(Binary), "\n"), [Line] = Lines, [HostAndIp, Alg, _KeyData] = string:tokens(Line, " "), - [Host, _Ip] = string:tokens(HostAndIp, ","), + [StoredHost, _Ip] = string:tokens(HostAndIp, ","), + true = ssh_test_lib:match_ip(StoredHost, Host), "ssh-" ++ _ = Alg, ssh:stop_daemon(Pid). %%-------------------------------------------------------------------- diff --git a/lib/ssh/test/ssh_bench_SUITE.erl b/lib/ssh/test/ssh_bench_SUITE.erl index ac52bb7e28..317e50ed1d 100644 --- a/lib/ssh/test/ssh_bench_SUITE.erl +++ b/lib/ssh/test/ssh_bench_SUITE.erl @@ -98,7 +98,7 @@ end_per_testcase(_Func, _Conf) -> connect(Config) -> KexAlgs = proplists:get_value(kex, ssh:default_algorithms()), - ct:pal("KexAlgs = ~p",[KexAlgs]), + ct:log("KexAlgs = ~p",[KexAlgs]), lists:foreach( fun(KexAlg) -> PrefAlgs = preferred_algorithms(KexAlg), @@ -242,11 +242,11 @@ median(Data) when is_list(Data) -> 1 -> lists:nth(N div 2 + 1, SortedData) end, - ct:pal("median(~p) = ~p",[SortedData,Median]), + ct:log("median(~p) = ~p",[SortedData,Median]), Median. report(Data) -> - ct:pal("EventData = ~p",[Data]), + ct:log("EventData = ~p",[Data]), ct_event:notify(#event{name = benchmark_data, data = Data}). diff --git a/lib/ssh/test/ssh_connection_SUITE.erl b/lib/ssh/test/ssh_connection_SUITE.erl index 2819a4dbd9..b911cf0e9e 100644 --- a/lib/ssh/test/ssh_connection_SUITE.erl +++ b/lib/ssh/test/ssh_connection_SUITE.erl @@ -89,7 +89,7 @@ end_per_suite(Config) -> %%-------------------------------------------------------------------- init_per_group(openssh, Config) -> - case gen_tcp:connect("localhost", 22, []) of + case ssh_test_lib:gen_tcp_connect("localhost", 22, []) of {error,econnrefused} -> {skip,"No openssh deamon"}; {ok, Socket} -> @@ -126,7 +126,7 @@ simple_exec(Config) when is_list(Config) -> simple_exec_sock(_Config) -> - {ok, Sock} = gen_tcp:connect("localhost", ?SSH_DEFAULT_PORT, [{active,false}]), + {ok, Sock} = ssh_test_lib:gen_tcp_connect("localhost", ?SSH_DEFAULT_PORT, [{active,false}]), {ok, ConnectionRef} = ssh:connect(Sock, [{silently_accept_hosts, true}, {user_interaction, false}]), do_simple_exec(ConnectionRef). @@ -179,13 +179,13 @@ daemon_sock_not_tcp(_Config) -> %%-------------------------------------------------------------------- connect_sock_not_passive(_Config) -> - {ok,Sock} = gen_tcp:connect("localhost", ?SSH_DEFAULT_PORT, []), + {ok,Sock} = ssh_test_lib: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, []), + {ok,Sock} = ssh_test_lib:gen_tcp_connect("localhost", ?SSH_DEFAULT_PORT, []), {error, not_passive_mode} = ssh:daemon(Sock), gen_tcp:close(Sock). @@ -585,12 +585,13 @@ start_shell_sock_exec_fun(Config) when is_list(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), - {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir}, - {user_dir, UserDir}, - {password, "morot"}, - {exec, fun ssh_exec/1}]), + {Pid, HostD, Port} = ssh_test_lib:daemon([{system_dir, SysDir}, + {user_dir, UserDir}, + {password, "morot"}, + {exec, fun ssh_exec/1}]), + Host = ssh_test_lib:ntoa(ssh_test_lib:mangle_connect_address(HostD)), - {ok, Sock} = gen_tcp:connect(Host, Port, [{active,false}]), + {ok, Sock} = ssh_test_lib:gen_tcp_connect(Host, Port, [{active,false}]), {ok,ConnectionRef} = ssh:connect(Sock, [{silently_accept_hosts, true}, {user, "foo"}, {password, "morot"}, @@ -623,7 +624,7 @@ start_shell_sock_daemon_exec(Config) -> {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,Ss} = ssh_test_lib:gen_tcp_connect("localhost", Port, [{active,false}]), {ok, _Pid} = ssh:daemon(Ss, [{system_dir, SysDir}, {user_dir, UserDir}, {password, "morot"}, @@ -658,10 +659,10 @@ gracefull_invalid_version(Config) when is_list(Config) -> SysDir = proplists:get_value(data_dir, Config), {_Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir}, - {user_dir, UserDir}, - {password, "morot"}]), + {user_dir, UserDir}, + {password, "morot"}]), - {ok, S} = gen_tcp:connect(Host, Port, []), + {ok, S} = ssh_test_lib:gen_tcp_connect(Host, Port, []), ok = gen_tcp:send(S, ["SSH-8.-1","\r\n"]), receive Verstring -> @@ -680,10 +681,10 @@ gracefull_invalid_start(Config) when is_list(Config) -> file:make_dir(UserDir), SysDir = proplists:get_value(data_dir, Config), {_Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir}, - {user_dir, UserDir}, - {password, "morot"}]), + {user_dir, UserDir}, + {password, "morot"}]), - {ok, S} = gen_tcp:connect(Host, Port, []), + {ok, S} = ssh_test_lib:gen_tcp_connect(Host, Port, []), ok = gen_tcp:send(S, ["foobar","\r\n"]), receive Verstring -> @@ -702,10 +703,10 @@ gracefull_invalid_long_start(Config) when is_list(Config) -> file:make_dir(UserDir), SysDir = proplists:get_value(data_dir, Config), {_Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir}, - {user_dir, UserDir}, - {password, "morot"}]), + {user_dir, UserDir}, + {password, "morot"}]), - {ok, S} = gen_tcp:connect(Host, Port, []), + {ok, S} = ssh_test_lib:gen_tcp_connect(Host, Port, []), ok = gen_tcp:send(S, [lists:duplicate(257, $a), "\r\n"]), receive Verstring -> @@ -725,10 +726,10 @@ gracefull_invalid_long_start_no_nl(Config) when is_list(Config) -> file:make_dir(UserDir), SysDir = proplists:get_value(data_dir, Config), {_Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir}, - {user_dir, UserDir}, - {password, "morot"}]), + {user_dir, UserDir}, + {password, "morot"}]), - {ok, S} = gen_tcp:connect(Host, Port, []), + {ok, S} = ssh_test_lib:gen_tcp_connect(Host, Port, []), ok = gen_tcp:send(S, [lists:duplicate(257, $a), "\r\n"]), receive Verstring -> @@ -779,22 +780,21 @@ stop_listener(Config) when is_list(Config) -> ct:fail("Exec Timeout") end, - {ok, HostAddr} = inet:getaddr(Host, inet), - case ssh_test_lib:daemon(HostAddr, Port, [{system_dir, SysDir}, - {user_dir, UserDir}, - {password, "potatis"}, - {exec, fun ssh_exec/1}]) of - {Pid1, HostAddr, Port} -> + case ssh_test_lib:daemon(Port, [{system_dir, SysDir}, + {user_dir, UserDir}, + {password, "potatis"}, + {exec, fun ssh_exec/1}]) of + {Pid1, Host, Port} -> ConnectionRef1 = ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true}, {user, "foo"}, {password, "potatis"}, {user_interaction, true}, {user_dir, UserDir}]), {error, _} = ssh:connect(Host, Port, [{silently_accept_hosts, true}, - {user, "foo"}, - {password, "morot"}, - {user_interaction, true}, - {user_dir, UserDir}]), + {user, "foo"}, + {password, "morot"}, + {user_interaction, true}, + {user_dir, UserDir}]), ssh:close(ConnectionRef0), ssh:close(ConnectionRef1), ssh:stop_daemon(Pid0), diff --git a/lib/ssh/test/ssh_options_SUITE.erl b/lib/ssh/test/ssh_options_SUITE.erl index 758c20e2b8..344a042d79 100644 --- a/lib/ssh/test/ssh_options_SUITE.erl +++ b/lib/ssh/test/ssh_options_SUITE.erl @@ -868,13 +868,13 @@ really_do_hostkey_fingerprint_check(Config, HashAlg) -> ct:log("Fingerprints(~p) = ~p",[HashAlg,FPs]), %% Start daemon with the public keys that we got fingerprints from - {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir}, + {Pid, Host0, Port} = ssh_test_lib:daemon([{system_dir, SysDir}, {user_dir, UserDirServer}, {password, "morot"}]), - + Host = ssh_test_lib:ntoa(Host0), FP_check_fun = fun(PeerName, FP) -> - ct:pal("PeerName = ~p, FP = ~p",[PeerName,FP]), - HostCheck = (Host == PeerName), + ct:log("PeerName = ~p, FP = ~p",[PeerName,FP]), + HostCheck = ssh_test_lib:match_ip(Host, PeerName), FPCheck = if is_atom(HashAlg) -> lists:member(FP, FPs); is_list(HashAlg) -> lists:all(fun(FP1) -> lists:member(FP1,FPs) end, @@ -1052,20 +1052,20 @@ id_string_random_client(Config) -> %%-------------------------------------------------------------------- id_string_no_opt_server(Config) -> {_Server, Host, Port} = ssh_test_lib:std_daemon(Config, []), - {ok,S1}=gen_tcp:connect(Host,Port,[{active,false},{packet,line}]), + {ok,S1}=ssh_test_lib:gen_tcp_connect(Host,Port,[{active,false},{packet,line}]), {ok,"SSH-2.0-Erlang/"++Vsn} = gen_tcp:recv(S1, 0, 2000), true = expected_ssh_vsn(Vsn). %%-------------------------------------------------------------------- id_string_own_string_server(Config) -> {_Server, Host, Port} = ssh_test_lib:std_daemon(Config, [{id_string,"Olle"}]), - {ok,S1}=gen_tcp:connect(Host,Port,[{active,false},{packet,line}]), + {ok,S1}=ssh_test_lib:gen_tcp_connect(Host,Port,[{active,false},{packet,line}]), {ok,"SSH-2.0-Olle\r\n"} = gen_tcp:recv(S1, 0, 2000). %%-------------------------------------------------------------------- id_string_random_server(Config) -> {_Server, Host, Port} = ssh_test_lib:std_daemon(Config, [{id_string,random}]), - {ok,S1}=gen_tcp:connect(Host,Port,[{active,false},{packet,line}]), + {ok,S1}=ssh_test_lib:gen_tcp_connect(Host,Port,[{active,false},{packet,line}]), {ok,"SSH-2.0-"++Rnd} = gen_tcp:recv(S1, 0, 2000), case Rnd of "Erlang"++_ -> ct:log("Id=~p",[Rnd]), @@ -1086,11 +1086,11 @@ ssh_connect_negtimeout(Config, Parallel) -> ct:log("Parallel: ~p",[Parallel]), {_Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SystemDir},{user_dir, UserDir}, - {parallel_login, Parallel}, - {negotiation_timeout, NegTimeOut}, - {failfun, fun ssh_test_lib:failfun/2}]), - - {ok,Socket} = gen_tcp:connect(Host, Port, []), + {parallel_login, Parallel}, + {negotiation_timeout, NegTimeOut}, + {failfun, fun ssh_test_lib:failfun/2}]), + + {ok,Socket} = ssh_test_lib:gen_tcp_connect(Host, Port, []), Factor = 2, ct:log("And now sleeping ~p*NegTimeOut (~p ms)...", [Factor, round(Factor * NegTimeOut)]), diff --git a/lib/ssh/test/ssh_relay.erl b/lib/ssh/test/ssh_relay.erl index 28000fbb97..1e3810e9d4 100644 --- a/lib/ssh/test/ssh_relay.erl +++ b/lib/ssh/test/ssh_relay.erl @@ -131,7 +131,8 @@ init([ListenAddr, ListenPort, PeerAddr, PeerPort | _Options]) -> S = #state{local_addr = ListenAddr, local_port = ListenPort, lpid = LPid, - peer_addr = PeerAddr, + peer_addr = ssh_test_lib:ntoa( + ssh_test_lib:mangle_connect_address(PeerAddr)), peer_port = PeerPort }, {ok, S}; diff --git a/lib/ssh/test/ssh_sftp_SUITE.erl b/lib/ssh/test/ssh_sftp_SUITE.erl index acf76157a2..7efeb3a0ad 100644 --- a/lib/ssh/test/ssh_sftp_SUITE.erl +++ b/lib/ssh/test/ssh_sftp_SUITE.erl @@ -660,7 +660,7 @@ start_channel_sock(Config) -> {Host,Port} = proplists:get_value(peer, Config), %% Get a tcp socket - {ok, Sock} = gen_tcp:connect(Host, Port, [{active,false}]), + {ok, Sock} = ssh_test_lib:gen_tcp_connect(Host, Port, [{active,false}]), %% and open one channel on one new Connection {ok, ChPid1, Conn} = ssh_sftp:start_channel(Sock, Opts), diff --git a/lib/ssh/test/ssh_sftpd_SUITE.erl b/lib/ssh/test/ssh_sftpd_SUITE.erl index 673fb54a4f..379c0bcb0a 100644 --- a/lib/ssh/test/ssh_sftpd_SUITE.erl +++ b/lib/ssh/test/ssh_sftpd_SUITE.erl @@ -705,10 +705,10 @@ try_access(Path, Cm, Channel, ReqId) -> {ok, <>, <<>>} -> case Code of ?SSH_FX_FILE_IS_A_DIRECTORY -> - ct:pal("Got the expected SSH_FX_FILE_IS_A_DIRECTORY status",[]), + ct:log("Got the expected SSH_FX_FILE_IS_A_DIRECTORY status",[]), ok; ?SSH_FX_FAILURE -> - ct:pal("Got the expected SSH_FX_FAILURE status",[]), + ct:log("Got the expected SSH_FX_FAILURE status",[]), ok; _ -> case Rest of diff --git a/lib/ssh/test/ssh_sup_SUITE.erl b/lib/ssh/test/ssh_sup_SUITE.erl index fdeb8186a5..dd7c4b1473 100644 --- a/lib/ssh/test/ssh_sup_SUITE.erl +++ b/lib/ssh/test/ssh_sup_SUITE.erl @@ -137,16 +137,18 @@ sshd_subtree(Config) when is_list(Config) -> HostIP = proplists:get_value(host_ip, Config), Port = proplists:get_value(port, Config), SystemDir = proplists:get_value(data_dir, Config), - ssh:daemon(HostIP, Port, [{system_dir, SystemDir}, - {failfun, fun ssh_test_lib:failfun/2}, - {user_passwords, - [{?USER, ?PASSWD}]}]), + {ok,Daemon} = ssh:daemon(HostIP, Port, [{system_dir, SystemDir}, + {failfun, fun ssh_test_lib:failfun/2}, + {user_passwords, + [{?USER, ?PASSWD}]}]), - ?wait_match([{{server,ssh_system_sup, HostIP, Port, ?DEFAULT_PROFILE}, + ct:log("Expect HostIP=~p, Port=~p, Daemon=~p",[HostIP,Port,Daemon]), + ?wait_match([{{server,ssh_system_sup, ListenIP, Port, ?DEFAULT_PROFILE}, Daemon, supervisor, [ssh_system_sup]}], supervisor:which_children(sshd_sup), - Daemon), + [ListenIP,Daemon]), + true = ssh_test_lib:match_ip(HostIP, ListenIP), check_sshd_system_tree(Daemon, Config), ssh:stop_daemon(HostIP, Port), ct:sleep(?WAIT_FOR_SHUTDOWN), @@ -161,16 +163,18 @@ sshd_subtree_profile(Config) when is_list(Config) -> Profile = proplists:get_value(profile, Config), SystemDir = proplists:get_value(data_dir, Config), - {ok, _} = ssh:daemon(HostIP, Port, [{system_dir, SystemDir}, - {failfun, fun ssh_test_lib:failfun/2}, - {user_passwords, - [{?USER, ?PASSWD}]}, - {profile, Profile}]), - ?wait_match([{{server,ssh_system_sup, HostIP,Port,Profile}, + {ok, Daemon} = ssh:daemon(HostIP, Port, [{system_dir, SystemDir}, + {failfun, fun ssh_test_lib:failfun/2}, + {user_passwords, + [{?USER, ?PASSWD}]}, + {profile, Profile}]), + ct:log("Expect HostIP=~p, Port=~p, Profile=~p, Daemon=~p",[HostIP,Port,Profile,Daemon]), + ?wait_match([{{server,ssh_system_sup, ListenIP,Port,Profile}, Daemon, supervisor, [ssh_system_sup]}], supervisor:which_children(sshd_sup), - Daemon), + [ListenIP,Daemon]), + true = ssh_test_lib:match_ip(HostIP, ListenIP), check_sshd_system_tree(Daemon, Config), ssh:stop_daemon(HostIP, Port, Profile), ct:sleep(?WAIT_FOR_SHUTDOWN), @@ -310,3 +314,4 @@ acceptor_pid(DaemonPid) -> receive {Pid, supsearch, L} -> {ok,L} after 2000 -> timeout end. + diff --git a/lib/ssh/test/ssh_test_lib.erl b/lib/ssh/test/ssh_test_lib.erl index 0ada8233a7..6186d44890 100644 --- a/lib/ssh/test/ssh_test_lib.erl +++ b/lib/ssh/test/ssh_test_lib.erl @@ -32,15 +32,18 @@ -define(TIMEOUT, 50000). +%%%---------------------------------------------------------------- connect(Port, Options) when is_integer(Port) -> connect(hostname(), Port, Options). connect(any, Port, Options) -> connect(hostname(), Port, Options); connect(Host, Port, Options) -> + ct:log("~p:~p Calling ssh:connect(~p, ~p, ~p)",[?MODULE,?LINE,Host, Port, Options]), {ok, ConnectionRef} = ssh:connect(Host, Port, Options), ConnectionRef. +%%%---------------------------------------------------------------- daemon(Options) -> daemon(any, 0, Options). @@ -53,26 +56,57 @@ daemon(Host, Options) -> daemon(Host, Port, Options) -> ct:log("~p:~p Calling ssh:daemon(~p, ~p, ~p)",[?MODULE,?LINE,Host,Port,Options]), case ssh:daemon(Host, Port, Options) of - {ok, Pid} when Host == any -> - ct:log("ssh:daemon ok (1)",[]), - {Pid, hostname(), daemon_port(Port,Pid)}; {ok, Pid} -> - ct:log("ssh:daemon ok (2)",[]), - {Pid, Host, daemon_port(Port,Pid)}; + {ok,L} = ssh:daemon_info(Pid), + ListenPort = proplists:get_value(port, L), + ListenIP = proplists:get_value(ip, L), + {Pid, ListenIP, ListenPort}; Error -> ct:log("ssh:daemon error ~p",[Error]), Error end. +%%%---------------------------------------------------------------- daemon_port(Pid) -> daemon_port(0, Pid). daemon_port(0, Pid) -> {ok,Dinf} = ssh:daemon_info(Pid), proplists:get_value(port, Dinf); daemon_port(Port, _) -> Port. - +%%%---------------------------------------------------------------- +gen_tcp_connect(Host0, Port, Options) -> + Host = ssh_test_lib:ntoa(ssh_test_lib:mangle_connect_address(Host0)), + ct:log("~p:~p gen_tcp:connect(~p, ~p, ~p)~nHost0 = ~p", + [?MODULE,?LINE, Host, Port, Options, Host0]), + Result = gen_tcp:connect(Host, Port, Options), + ct:log("~p:~p Result = ~p", [?MODULE,?LINE, Result]), + Result. + +%%%---------------------------------------------------------------- +open_sshc(Host0, Port, OptStr) -> + open_sshc(Host0, Port, OptStr, ""). + +open_sshc(Host0, Port, OptStr, ExecStr) -> + Cmd = open_sshc_cmd(Host0, Port, OptStr, ExecStr), + Result = os:cmd(Cmd), + ct:log("~p:~p Result = ~p", [?MODULE,?LINE, Result]), + Result. + +open_sshc_cmd(Host, Port, OptStr) -> + open_sshc_cmd(Host, Port, OptStr, ""). + +open_sshc_cmd(Host0, Port, OptStr, ExecStr) -> + Host = ssh_test_lib:ntoa(ssh_test_lib:mangle_connect_address(Host0)), + Cmd = lists:flatten(["ssh -p ", integer_to_list(Port), + " ", OptStr, + " ", Host, + " ", ExecStr]), + ct:log("~p:~p OpenSSH Cmd = ~p", [?MODULE,?LINE, Cmd]), + Cmd. + +%%%---------------------------------------------------------------- std_daemon(Config, ExtraOpts) -> PrivDir = proplists:get_value(priv_dir, Config), UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth @@ -88,6 +122,7 @@ std_daemon1(Config, ExtraOpts) -> {failfun, fun ssh_test_lib:failfun/2} | ExtraOpts]). +%%%---------------------------------------------------------------- std_connect(Config, Host, Port, ExtraOpts) -> UserDir = proplists:get_value(priv_dir, Config), _ConnectionRef = @@ -98,6 +133,7 @@ std_connect(Config, Host, Port, ExtraOpts) -> {user_interaction, false} | ExtraOpts]). +%%%---------------------------------------------------------------- std_simple_sftp(Host, Port, Config) -> std_simple_sftp(Host, Port, Config, []). @@ -112,6 +148,7 @@ std_simple_sftp(Host, Port, Config, Opts) -> ok = ssh:close(ConnectionRef), Data == ReadData. +%%%---------------------------------------------------------------- std_simple_exec(Host, Port, Config) -> std_simple_exec(Host, Port, Config, []). @@ -138,6 +175,7 @@ std_simple_exec(Host, Port, Config, Opts) -> ct:fail(ExecResult) end. +%%%---------------------------------------------------------------- start_shell(Port, IOServer) -> start_shell(Port, IOServer, []). @@ -152,6 +190,7 @@ start_shell(Port, IOServer, ExtraOptions) -> end). +%%%---------------------------------------------------------------- start_io_server() -> spawn_link(?MODULE, init_io_server, [self()]). @@ -210,8 +249,7 @@ reply(TestCase, Result) -> %%ct:log("reply ~p sending ~p ! ~p",[self(), TestCase, Result]), TestCase ! Result. - - +%%%---------------------------------------------------------------- rcv_expected(Expect, SshPort, Timeout) -> receive {SshPort, Recvd} when is_function(Expect) -> @@ -865,3 +903,73 @@ create_random_dir(Config) -> %% The likelyhood of always generating an existing file name is low create_random_dir(Config) end. + +%%%---------------------------------------------------------------- +match_ip(A, B) -> + R = match_ip0(A,B) orelse match_ip0(B,A), + ct:log("match_ip(~p, ~p) -> ~p",[A, B, R]), + R. + +match_ip0(A, A) -> + true; +match_ip0(any, _) -> + true; +match_ip0(A, B) -> + case match_ip1(A, B) of + true -> + true; + false when is_list(A) -> + case inet:parse_address(A) of + {ok,IPa} -> match_ip0(IPa, B); + _ -> false + end; + false when is_list(B) -> + case inet:parse_address(B) of + {ok,IPb} -> match_ip0(A, IPb); + _ -> false + end; + false -> + false + end. + +match_ip1(any, _) -> true; +match_ip1(loopback, {127,_,_,_}) -> true; +match_ip1({0,0,0,0}, {127,_,_,_}) -> true; +match_ip1(loopback, {0,0,0,0,0,0,0,1}) -> true; +match_ip1({0,0,0,0,0,0,0,0}, {0,0,0,0,0,0,0,1}) -> true; +match_ip1(_, _) -> false. + +%%%---------------------------------------------------------------- +mangle_connect_address(A) -> + mangle_connect_address(A, []). + +mangle_connect_address(A, SockOpts) -> + mangle_connect_address1(A, proplists:get_value(inet6,SockOpts,false)). + +loopback(true) -> {0,0,0,0,0,0,0,1}; +loopback(false) -> {127,0,0,1}. + +mangle_connect_address1( loopback, V6flg) -> loopback(V6flg); +mangle_connect_address1( any, V6flg) -> loopback(V6flg); +mangle_connect_address1({0,0,0,0}, _) -> loopback(false); +mangle_connect_address1({0,0,0,0,0,0,0,0}, _) -> loopback(true); +mangle_connect_address1( IP, _) when is_tuple(IP) -> IP; +mangle_connect_address1(A, _) -> + case catch inet:parse_address(A) of + {ok, {0,0,0,0}} -> loopback(false); + {ok, {0,0,0,0,0,0,0,0}} -> loopback(true); + _ -> A + end. + +%%%---------------------------------------------------------------- +ntoa(A) -> + try inet:ntoa(A) + of + {error,_} when is_atom(A) -> atom_to_list(A); + {error,_} when is_list(A) -> A; + S when is_list(S) -> S + catch + _:_ when is_atom(A) -> atom_to_list(A); + _:_ when is_list(A) -> A + end. + diff --git a/lib/ssh/test/ssh_to_openssh_SUITE.erl b/lib/ssh/test/ssh_to_openssh_SUITE.erl index 7eda009552..35e3ee3edf 100644 --- a/lib/ssh/test/ssh_to_openssh_SUITE.erl +++ b/lib/ssh/test/ssh_to_openssh_SUITE.erl @@ -376,18 +376,18 @@ erlang_server_openssh_client_public_key_rsa(Config) when is_list(Config) -> erlang_server_openssh_client_public_key_X(Config, ssh_rsa). -erlang_server_openssh_client_public_key_X(Config, PubKeyAlg) -> +erlang_server_openssh_client_public_key_X(Config, _PubKeyAlg) -> SystemDir = proplists:get_value(data_dir, Config), PrivDir = proplists:get_value(priv_dir, Config), KnownHosts = filename:join(PrivDir, "known_hosts"), {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SystemDir}, {failfun, fun ssh_test_lib:failfun/2}]), - ct:sleep(500), - Cmd = "ssh -p " ++ integer_to_list(Port) ++ - " -o UserKnownHostsFile=" ++ KnownHosts ++ - " " ++ Host ++ " 1+1.", + Cmd = ssh_test_lib:open_sshc_cmd(Host, Port, + [" -o UserKnownHostsFile=", KnownHosts, + " -o StrictHostKeyChecking=no"], + "1+1."), OpenSsh = ssh_test_lib:open_port({spawn, Cmd}), ssh_test_lib:rcv_expected({data,<<"2\n">>}, OpenSsh, ?TIMEOUT), ssh:stop_daemon(Pid). @@ -395,13 +395,13 @@ erlang_server_openssh_client_public_key_X(Config, PubKeyAlg) -> %%-------------------------------------------------------------------- %% Test that the Erlang/OTP server can renegotiate with openSSH erlang_server_openssh_client_renegotiate(Config) -> - PubKeyAlg = ssh_rsa, + _PubKeyAlg = ssh_rsa, SystemDir = proplists:get_value(data_dir, Config), PrivDir = proplists:get_value(priv_dir, Config), KnownHosts = filename:join(PrivDir, "known_hosts"), {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SystemDir}, - {failfun, fun ssh_test_lib:failfun/2}]), + {failfun, fun ssh_test_lib:failfun/2}]), ct:sleep(500), RenegLimitK = 3, @@ -409,11 +409,13 @@ erlang_server_openssh_client_renegotiate(Config) -> Data = lists:duplicate(trunc(1.1*RenegLimitK*1024), $a), ok = file:write_file(DataFile, Data), - Cmd = "ssh -p " ++ integer_to_list(Port) ++ - " -o UserKnownHostsFile=" ++ KnownHosts ++ - " -o RekeyLimit=" ++ integer_to_list(RenegLimitK) ++"K" ++ - " " ++ Host ++ " < " ++ DataFile, - OpenSsh = ssh_test_lib:open_port({spawn, Cmd}), + Cmd = ssh_test_lib:open_sshc_cmd(Host, Port, + [" -o UserKnownHostsFile=", KnownHosts, + " -o StrictHostKeyChecking=no", + " -o RekeyLimit=",integer_to_list(RenegLimitK),"K"]), + + + OpenSsh = ssh_test_lib:open_port({spawn, Cmd++" < "++DataFile}), Expect = fun({data,R}) -> try @@ -462,7 +464,7 @@ erlang_client_openssh_server_renegotiate(_Config) -> {silently_accept_hosts,true}], group_leader(IO, self()), {ok, ConnRef} = ssh:connect(Host, ?SSH_DEFAULT_PORT, Options), - ct:pal("Parent = ~p, IO = ~p, Shell = ~p, ConnRef = ~p~n",[Parent, IO, self(), ConnRef]), + ct:log("Parent = ~p, IO = ~p, Shell = ~p, ConnRef = ~p~n",[Parent, IO, self(), ConnRef]), case ssh_connection:session_channel(ConnRef, infinity) of {ok,ChannelId} -> success = ssh_connection:ptty_alloc(ConnRef, ChannelId, []), diff --git a/lib/ssh/test/ssh_trpt_test_lib.erl b/lib/ssh/test/ssh_trpt_test_lib.erl index 261239c152..e1f4c65300 100644 --- a/lib/ssh/test/ssh_trpt_test_lib.erl +++ b/lib/ssh/test/ssh_trpt_test_lib.erl @@ -314,8 +314,7 @@ mangle_opts(Options) -> lists:keydelete(K,1,Opts) end, Options, SysOpts). -host({0,0,0,0}) -> "localhost"; -host(H) -> H. +host(H) -> ssh_test_lib:ntoa(ssh_test_lib:mangle_connect_address(H)). %%%---------------------------------------------------------------- send(S=#s{ssh=C}, hello) -> -- cgit v1.2.3 From 7ad21ca66f5a46be231fffe884ac2c3b5d97c7ae Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Tue, 4 Apr 2017 19:53:05 +0200 Subject: ssh: document what happens when ssh:daemon sets both HostAddr and ip option The idea is that the HostAddress argument takes precedence over an ip-option. However, an ip-option overrides the 'any' HostAddr. This fixes the case of dameon(Port, [{ip,IP}..] in a non-surprising way. --- lib/ssh/doc/src/ssh.xml | 22 ++++++++++++- lib/ssh/src/ssh.erl | 82 +++++-------------------------------------------- 2 files changed, 29 insertions(+), 75 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/doc/src/ssh.xml b/lib/ssh/doc/src/ssh.xml index 88d402cf38..48c9aa18e9 100644 --- a/lib/ssh/doc/src/ssh.xml +++ b/lib/ssh/doc/src/ssh.xml @@ -379,7 +379,7 @@ on the given port. Port = integer() - HostAddress = ip_address() | any + HostAddress = ip_address() | any | loopback Options = [{Option, Value}] Option = atom() Value = term() @@ -390,6 +390,26 @@

Starts a server listening for SSH connections on the given port. If the Port is 0, a random free port is selected. See daemon_info/1 about how to find the selected port number.

+ +

Please note that by historical reasons both the HostAddress argument and the inet socket option + ip set the listening address. This is a source of possible inconsistent settings.

+ +

The rules for handling the two address passing options are:

+ + if HostAddress is an ip-address, that ip-address is the listening address. + An ip-option will be discarded if present. + + if HostAddress is loopback, the listening address + is loopback and an loopback address will be choosen by the underlying layers. + An ip-option will be discarded if present. + + if HostAddress is any and no ip-option is present, the listening address is + any and the socket will listen to all addresses + + if HostAddress is any and an ip-option is present, the listening address is + set to the value of the ip-option + +

Options:

diff --git a/lib/ssh/src/ssh.erl b/lib/ssh/src/ssh.erl index 8c802d46eb..3e80a04b70 100644 --- a/lib/ssh/src/ssh.erl +++ b/lib/ssh/src/ssh.erl @@ -225,7 +225,8 @@ daemon(Port, UserOptions) when 0 =< Port, Port =< 65535 -> daemon(any, Port, UserOptions). -daemon(Host0, Port0, UserOptions0) when 0 =< Port0, Port0 =< 65535 -> +daemon(Host0, Port0, UserOptions0) when 0 =< Port0, Port0 =< 65535, + Host0 == any ; Host0 == loopback ; is_tuple(Host0) -> try {Host1, UserOptions} = handle_daemon_args(Host0, UserOptions0), #{} = Options0 = ssh_options:handle_options(server, UserOptions), @@ -259,7 +260,11 @@ daemon(Host0, Port0, UserOptions0) when 0 =< Port0, Port0 =< 65535 -> {error,Error}; _C:_E -> {error,{cannot_start_daemon,_C,_E}} - end. + end; + +daemon(_, _, _) -> + {error, badarg}. + %%-------------------------------------------------------------------- @@ -378,35 +383,6 @@ default_algorithms() -> %%-------------------------------------------------------------------- %%% Internal functions %%-------------------------------------------------------------------- - -%% - if Address is 'any' and no ip-option is present, the name is -%% 'any' and the socket will listen to all addresses -%% -%% - if Address is 'any' and an ip-option is present, the name is -%% set to the value of the ip-option and the socket will listen -%% to that address -%% -%% - if Address is 'loopback' and no ip-option is present, the name -%% is 'loopback' and an loopback address will be choosen by the -%% underlying layers -%% -%% - if Address is 'loopback' and an ip-option is present, the name -%% is set to the value of the ip-option kept and the socket will -%% listen to that address -%% -%% - if Address is an ip-address, that ip-address is the name and -%% the listening address. An ip-option will be discarded. -%% -%% - if Address is a HostName, and that resolves to an ip-address, -%% that ip-address is the name and the listening address. An -%% ip-option will be discarded. -%% -%% - if Address is a string or an atom other than thoose defined -%% above, that Address will be the name and the listening address -%% will be choosen by the lower layers taking an ip-option in -%% consideration -%% - %% The handle_daemon_args/2 function basically only sets the ip-option in Opts %% so that it is correctly set when opening the listening socket. @@ -416,53 +392,11 @@ handle_daemon_args(any, Opts) -> IP -> {IP, Opts} end; -handle_daemon_args(loopback, Opts) -> - case proplists:get_value(ip, Opts) of - undefined -> {loopback, [{ip,loopback}|Opts]}; - IP -> {IP, Opts} - end; - -handle_daemon_args(IPaddr, Opts) when is_tuple(IPaddr) -> +handle_daemon_args(IPaddr, Opts) when is_tuple(IPaddr) ; IPaddr == loopback -> case proplists:get_value(ip, Opts) of undefined -> {IPaddr, [{ip,IPaddr}|Opts]}; IPaddr -> {IPaddr, Opts}; IP -> {IPaddr, [{ip,IPaddr}|Opts--[{ip,IP}]]} %% Backward compatibility - end; - -handle_daemon_args(Address, Opts) when is_list(Address) ; % IP address in string or a domain - is_atom(Address) % domains could be atoms in inet - -> - IP = proplists:get_value(ip, Opts), - case inet:parse_strict_address(Address) of - %% check if Address is an IP-address - {ok, IP} -> {IP, Opts}; - {ok, OtherIP} -> {OtherIP, [{ip,OtherIP}|Opts--[{ip,IP}]]}; - _ -> - %% Not an IP-address. Check if it is a host name: - case inet:getaddr(Address, family(Opts)) of - {ok, IP} -> {Address, Opts}; - {ok, OtherIP} -> {Address, [{ip,OtherIP}|Opts--[{ip,IP}]]}; - _ -> - %% Not a Host name and not an IP address, let - %% inet and the OS later figure out what it - %% could be - {Address, Opts} - end - end. - -%% Has the caller indicated the address family? -family(Opts) -> - family(Opts, inet). - -family(Opts, Default) -> - case proplists:get_value(inet,Opts) of - true -> inet; - inet -> inet; - inet6 -> inet6; - _ -> case proplists:get_value(inet6,Opts) of - true -> inet6; - _ -> Default - end end. %%%---------------------------------------------------------------- -- cgit v1.2.3 From b89f06569ee24011a8535c57d6a82e336afeb5bf Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Wed, 5 Apr 2017 15:45:18 +0200 Subject: ssh: Doc-changes to make clearer IP-address and 'ip'-option --- lib/ssh/doc/src/ssh.xml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/doc/src/ssh.xml b/lib/ssh/doc/src/ssh.xml index 48c9aa18e9..368261968d 100644 --- a/lib/ssh/doc/src/ssh.xml +++ b/lib/ssh/doc/src/ssh.xml @@ -396,18 +396,18 @@

The rules for handling the two address passing options are:

- if HostAddress is an ip-address, that ip-address is the listening address. - An ip-option will be discarded if present. + if HostAddress is an IP-address, that IP-address is the listening address. + An 'ip'-option will be discarded if present. if HostAddress is loopback, the listening address is loopback and an loopback address will be choosen by the underlying layers. - An ip-option will be discarded if present. + An 'ip'-option will be discarded if present. - if HostAddress is any and no ip-option is present, the listening address is + if HostAddress is any and no 'ip'-option is present, the listening address is any and the socket will listen to all addresses - if HostAddress is any and an ip-option is present, the listening address is - set to the value of the ip-option + if HostAddress is any and an 'ip'-option is present, the listening address is + set to the value of the 'ip'-option

Options:

-- cgit v1.2.3