From 59dad3e9bfe0a3d724484d93ad09a7b41de8dab4 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Wed, 23 Apr 2014 22:00:06 +0200 Subject: ssh: remove confusing info in some reports --- lib/ssh/src/ssh_connection_handler.erl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl index 322da50f21..06866392da 100644 --- a/lib/ssh/src/ssh_connection_handler.erl +++ b/lib/ssh/src/ssh_connection_handler.erl @@ -1482,8 +1482,7 @@ ssh_channel_info([ _ | Rest], Channel, Acc) -> log_error(Reason) -> Report = io_lib:format("Erlang ssh connection handler failed with reason: " - "~p ~n, Stacktace: ~p ~n" - "please report this to erlang-bugs@erlang.org \n", + "~p ~n, Stacktrace: ~p ~n", [Reason, erlang:get_stacktrace()]), error_logger:error_report(Report), "Internal error". -- cgit v1.2.3 From 1700332e03168d577eb64b93fcae876a6ad9db3d Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Wed, 23 Apr 2014 21:45:27 +0200 Subject: ssh: Add max_session parameter to ssh:daemon --- lib/ssh/doc/src/ssh.xml | 21 ++++++++++++--- lib/ssh/src/ssh.erl | 4 +++ lib/ssh/src/ssh_acceptor.erl | 47 +++++++++++++++++++++++++-------- lib/ssh/test/ssh_basic_SUITE.erl | 56 ++++++++++++++++++++++++++++++++++++++-- 4 files changed, 111 insertions(+), 17 deletions(-) diff --git a/lib/ssh/doc/src/ssh.xml b/lib/ssh/doc/src/ssh.xml index 7fbd70c87e..57aab09cc6 100644 --- a/lib/ssh/doc/src/ssh.xml +++ b/lib/ssh/doc/src/ssh.xml @@ -307,18 +307,31 @@ -

Max time in milliseconds for the authentication negotiation. The default value is 2 minutes. +

Max time in milliseconds for the authentication negotiation. The default value is 2 minutes. If the client fails to login within this time, the connection is closed. +

+ + + + +

The maximum number of simultaneous sessions that are accepted at any time for this daemon. This includes sessions that are being authorized. So if set to N, and N clients have connected but not started the login process, the N+1 connection attempt will be aborted. If N connections are authenticated and still logged in, no more loggins will be accepted until one of the existing ones log out. +

+

The counter is per listening port, so if two daemons are started, one with {max_sessions,N} and the other with {max_sessions,M} there will be in total N+M connections accepted for the whole ssh server. +

+

Note that if parallel_login is false, only one client at a time may be in the authentication phase. +

+

As default, the option is not set. This means that the number is not limited.

-

If set to false (the default value), only one login is handled a time. If set to true, an unlimited logins will be allowed simultanously. Note that this affects only the connections with authentication in progress, not the already authenticated connections. +

If set to false (the default value), only one login is handled a time. If set to true, an unlimited number of login attempts will be allowed simultanously. +

+

If the max_sessions option is set to N and parallel_login is set to true, the max number of simultaneous login attempts at any time is limited to N-K where K is the number of authenticated connections present at this daemon.

-

Do not enable parallel_logins without protecting the server by other means like a firewall. If set to true, there is no protection against dos attacs.

+

Do not enable parallel_logins without protecting the server by other means, for example the max_sessions option or a firewall configuration. If set to true, there is no protection against DOS attacks.

 - diff --git a/lib/ssh/src/ssh.erl b/lib/ssh/src/ssh.erl index de6e8cc421..75081b7a61 100644 --- a/lib/ssh/src/ssh.erl +++ b/lib/ssh/src/ssh.erl @@ -332,6 +332,8 @@ handle_option([{idle_time, _} = Opt | Rest], SocketOptions, SshOptions) -> handle_option(Rest, SocketOptions, [handle_ssh_option(Opt) | SshOptions]); handle_option([{rekey_limit, _} = Opt|Rest], SocketOptions, SshOptions) -> handle_option(Rest, SocketOptions, [handle_ssh_option(Opt) | SshOptions]); +handle_option([{max_sessions, _} = Opt|Rest], SocketOptions, SshOptions) -> + handle_option(Rest, SocketOptions, [handle_ssh_option(Opt) | SshOptions]); handle_option([{negotiation_timeout, _} = Opt|Rest], SocketOptions, SshOptions) -> handle_option(Rest, SocketOptions, [handle_ssh_option(Opt) | SshOptions]); handle_option([{parallel_login, _} = Opt|Rest], SocketOptions, SshOptions) -> @@ -366,6 +368,8 @@ handle_ssh_option({pref_public_key_algs, Value} = Opt) when is_list(Value), leng end; handle_ssh_option({connect_timeout, Value} = Opt) when is_integer(Value); Value == infinity -> Opt; +handle_ssh_option({max_sessions, Value} = Opt) when is_integer(Value), Value>0 -> + Opt; handle_ssh_option({negotiation_timeout, Value} = Opt) when is_integer(Value); Value == infinity -> Opt; handle_ssh_option({parallel_login, Value} = Opt) when Value==true ; Value==false -> diff --git a/lib/ssh/src/ssh_acceptor.erl b/lib/ssh/src/ssh_acceptor.erl index e57b07cee8..7302196674 100644 --- a/lib/ssh/src/ssh_acceptor.erl +++ b/lib/ssh/src/ssh_acceptor.erl @@ -80,18 +80,36 @@ acceptor_loop(Callback, Port, Address, Opts, ListenSocket, AcceptTimeout) -> ListenSocket, AcceptTimeout) end. -handle_connection(_Callback, Address, Port, Options, Socket) -> +handle_connection(Callback, Address, Port, Options, Socket) -> SystemSup = ssh_system_sup:system_supervisor(Address, Port), - {ok, SubSysSup} = ssh_system_sup:start_subsystem(SystemSup, Options), - ConnectionSup = ssh_subsystem_sup:connection_supervisor(SubSysSup), - Timeout = proplists:get_value(negotiation_timeout, - proplists:get_value(ssh_opts, Options, []), - 2*60*1000), - ssh_connection_handler:start_connection(server, Socket, - [{supervisors, [{system_sup, SystemSup}, - {subsystem_sup, SubSysSup}, - {connection_sup, ConnectionSup}]} - | Options], Timeout). + SSHopts = proplists:get_value(ssh_opts, Options, []), + MaxSessions = proplists:get_value(max_sessions,SSHopts,infinity), + case number_of_connections(SystemSup) < MaxSessions of + true -> + {ok, SubSysSup} = ssh_system_sup:start_subsystem(SystemSup, Options), + ConnectionSup = ssh_subsystem_sup:connection_supervisor(SubSysSup), + Timeout = proplists:get_value(negotiation_timeout, SSHopts, 2*60*1000), + ssh_connection_handler:start_connection(server, Socket, + [{supervisors, [{system_sup, SystemSup}, + {subsystem_sup, SubSysSup}, + {connection_sup, ConnectionSup}]} + | Options], Timeout); + false -> + Callback:close(Socket), + IPstr = if is_tuple(Address) -> inet:ntoa(Address); + true -> Address + end, + Str = try io_lib:format('~s:~p',[IPstr,Port]) + catch _:_ -> "port "++integer_to_list(Port) + end, + error_logger:info_report("Ssh login attempt to "++Str++" denied due to option " + "max_sessions limits to "++ io_lib:write(MaxSessions) ++ + " sessions." + ), + {error,max_sessions} + end. + + handle_error(timeout) -> ok; @@ -117,3 +135,10 @@ handle_error(Reason) -> String = lists:flatten(io_lib:format("Accept error: ~p", [Reason])), error_logger:error_report(String), exit({accept_failed, String}). + + +number_of_connections(SystemSup) -> + length([X || + {R,X,supervisor,[ssh_subsystem_sup]} <- supervisor:which_children(SystemSup), + is_reference(R) + ]). diff --git a/lib/ssh/test/ssh_basic_SUITE.erl b/lib/ssh/test/ssh_basic_SUITE.erl index d2e52379fa..a8b64b1425 100644 --- a/lib/ssh/test/ssh_basic_SUITE.erl +++ b/lib/ssh/test/ssh_basic_SUITE.erl @@ -47,21 +47,26 @@ all() -> daemon_already_started, server_password_option, server_userpassword_option, - double_close]. + double_close, + {group, hardening_tests} + ]. groups() -> [{dsa_key, [], basic_tests()}, {rsa_key, [], basic_tests()}, {dsa_pass_key, [], [pass_phrase]}, {rsa_pass_key, [], [pass_phrase]}, - {internal_error, [], [internal_error]} + {internal_error, [], [internal_error]}, + {hardening_tests, [], [max_sessions]} ]. + basic_tests() -> [send, close, peername_sockname, exec, exec_compressed, shell, cli, known_hosts, idle_time, rekey, openssh_zlib_basic_test]. + %%-------------------------------------------------------------------- init_per_suite(Config) -> case catch crypto:start() of @@ -74,6 +79,8 @@ end_per_suite(_Config) -> ssh:stop(), crypto:stop(). %%-------------------------------------------------------------------- +init_per_group(hardening_tests, Config) -> + init_per_group(dsa_key, Config); init_per_group(dsa_key, Config) -> DataDir = ?config(data_dir, Config), PrivDir = ?config(priv_dir, Config), @@ -103,6 +110,8 @@ init_per_group(internal_error, Config) -> init_per_group(_, Config) -> Config. +end_per_group(hardening_tests, Config) -> + end_per_group(dsa_key, Config); end_per_group(dsa_key, Config) -> PrivDir = ?config(priv_dir, Config), ssh_test_lib:clean_dsa(PrivDir), @@ -638,6 +647,49 @@ openssh_zlib_basic_test(Config) -> ok = ssh:close(ConnectionRef), ssh:stop_daemon(Pid). +%%-------------------------------------------------------------------- + +max_sessions(Config) -> + SystemDir = filename:join(?config(priv_dir, Config), system), + UserDir = ?config(priv_dir, Config), + MaxSessions = 2, + {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SystemDir}, + {user_dir, UserDir}, + {user_passwords, [{"carni", "meat"}]}, + {parallel_login, true}, + {max_sessions, MaxSessions} + ]), + + Connect = fun() -> + R=ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true}, + {user_dir, UserDir}, + {user_interaction, false}, + {user, "carni"}, + {password, "meat"} + ]), + ct:log("Connection ~p up",[R]) + end, + + try [Connect() || _ <- lists:seq(1,MaxSessions)] + of + _ -> + ct:pal("Expect Info Report:",[]), + try Connect() + of + _ConnectionRef -> + ssh:stop_daemon(Pid), + {fail,"Too many connections accepted"} + catch + error:{badmatch,{error,"Connection closed"}} -> + ssh:stop_daemon(Pid), + ok + end + catch + error:{badmatch,{error,"Connection closed"}} -> + ssh:stop_daemon(Pid), + {fail,"Too few connections accepted"} + end. + %%-------------------------------------------------------------------- %% Internal functions ------------------------------------------------ %%-------------------------------------------------------------------- -- cgit v1.2.3 From 3af70a78b6b84ed1e503d4b8d249ae9e8147eba2 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Thu, 24 Apr 2014 15:21:55 +0200 Subject: ssh: Doc change on max_session param --- lib/ssh/doc/src/ssh.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ssh/doc/src/ssh.xml b/lib/ssh/doc/src/ssh.xml index 57aab09cc6..5a141ced3c 100644 --- a/lib/ssh/doc/src/ssh.xml +++ b/lib/ssh/doc/src/ssh.xml @@ -315,7 +315,7 @@

The maximum number of simultaneous sessions that are accepted at any time for this daemon. This includes sessions that are being authorized. So if set to N, and N clients have connected but not started the login process, the N+1 connection attempt will be aborted. If N connections are authenticated and still logged in, no more loggins will be accepted until one of the existing ones log out.

-

The counter is per listening port, so if two daemons are started, one with {max_sessions,N} and the other with {max_sessions,M} there will be in total N+M connections accepted for the whole ssh server. +

The counter is per listening port, so if two daemons are started, one with {max_sessions,N} and the other with {max_sessions,M} there will be in total N+M connections accepted for the whole ssh application.

Note that if parallel_login is false, only one client at a time may be in the authentication phase.

-- cgit v1.2.3