From b98334b677be774d746037ab81e7e724bfeca5eb Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Wed, 17 Sep 2014 17:09:24 +0200 Subject: ssh: Gracefully handle incorrect versions Conflicts: lib/ssh/test/ssh_connection_SUITE.erl --- lib/ssh/src/ssh_connect.hrl | 3 +- lib/ssh/src/ssh_connection_handler.erl | 30 ++++++--- lib/ssh/src/ssh_transport.erl | 27 ++++---- lib/ssh/test/ssh_connection_SUITE.erl | 109 ++++++++++++++++++++++++++++++--- 4 files changed, 138 insertions(+), 31 deletions(-) (limited to 'lib') diff --git a/lib/ssh/src/ssh_connect.hrl b/lib/ssh/src/ssh_connect.hrl index 8421b07167..9307dbbad0 100644 --- a/lib/ssh/src/ssh_connect.hrl +++ b/lib/ssh/src/ssh_connect.hrl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2005-2013. All Rights Reserved. +%% Copyright Ericsson AB 2005-2014. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -26,6 +26,7 @@ -define(DEFAULT_PACKET_SIZE, 32768). -define(DEFAULT_WINDOW_SIZE, 2*?DEFAULT_PACKET_SIZE). -define(DEFAULT_TIMEOUT, 5000). +-define(MAX_PROTO_VERSION, 255). %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %% diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl index 86804c4436..68736c3365 100644 --- a/lib/ssh/src/ssh_connection_handler.erl +++ b/lib/ssh/src/ssh_connection_handler.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2008-2013. All Rights Reserved. +%% Copyright Ericsson AB 2008-2014. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -71,7 +71,8 @@ connection_queue, address, port, - opts + opts, + recbuf }). -type state_name() :: hello | kexinit | key_exchange | new_keys | userauth | connection. @@ -293,28 +294,39 @@ info(ConnectionHandler, ChannelProcess) -> hello(socket_control, #state{socket = Socket, ssh_params = Ssh} = State) -> VsnMsg = ssh_transport:hello_version_msg(string_version(Ssh)), send_msg(VsnMsg, State), - inet:setopts(Socket, [{packet, line}, {active, once}]), - {next_state, hello, State}; + {ok, [{recbuf, Size}]} = inet:getopts(Socket, [recbuf]), + inet:setopts(Socket, [{packet, line}, {active, once}, {recbuf, ?MAX_PROTO_VERSION}]), + {next_state, hello, State#state{recbuf = Size}}; -hello({info_line, _Line},#state{socket = Socket} = State) -> +hello({info_line, _Line},#state{role = client, socket = Socket} = State) -> + %% The server may send info lines before the version_exchange inet:setopts(Socket, [{active, once}]), {next_state, hello, State}; +hello({info_line, _Line},#state{role = server} = State) -> + DisconnectMsg = + #ssh_msg_disconnect{code = + ?SSH_DISCONNECT_PROTOCOL_ERROR, + description = "Did not receive expected protocol version exchange", + language = "en"}, + handle_disconnect(DisconnectMsg, State); + hello({version_exchange, Version}, #state{ssh_params = Ssh0, - socket = Socket} = State) -> + socket = Socket, + recbuf = Size} = State) -> {NumVsn, StrVsn} = ssh_transport:handle_hello_version(Version), case handle_version(NumVsn, StrVsn, Ssh0) of {ok, Ssh1} -> - inet:setopts(Socket, [{packet,0}, {mode,binary}, {active, once}]), + inet:setopts(Socket, [{packet,0}, {mode,binary}, {active, once}, {recbuf, Size}]), {KeyInitMsg, SshPacket, Ssh} = ssh_transport:key_exchange_init_msg(Ssh1), send_msg(SshPacket, State), {next_state, kexinit, next_packet(State#state{ssh_params = Ssh, key_exchange_init_msg = KeyInitMsg})}; not_supported -> - DisconnectMsg = + DisconnectMsg = #ssh_msg_disconnect{code = - ?SSH_DISCONNECT_PROTOCOL_VERSION_NOT_SUPPORTED, + ?SSH_DISCONNECT_PROTOCOL_VERSION_NOT_SUPPORTED, description = "Protocol version " ++ StrVsn ++ " not supported", language = "en"}, diff --git a/lib/ssh/src/ssh_transport.erl b/lib/ssh/src/ssh_transport.erl index ea05c849b7..76fa776113 100644 --- a/lib/ssh/src/ssh_transport.erl +++ b/lib/ssh/src/ssh_transport.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2004-2013. All Rights Reserved. +%% Copyright Ericsson AB 2004-2014. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -82,16 +82,21 @@ format_version({Major,Minor}) -> integer_to_list(Minor) ++ "-Erlang". handle_hello_version(Version) -> - StrVersion = trim_tail(Version), - case string:tokens(Version, "-") of - [_, "2.0" | _] -> - {{2,0}, StrVersion}; - [_, "1.99" | _] -> - {{2,0}, StrVersion}; - [_, "1.3" | _] -> - {{1,3}, StrVersion}; - [_, "1.5" | _] -> - {{1,5}, StrVersion} + try + StrVersion = trim_tail(Version), + case string:tokens(Version, "-") of + [_, "2.0" | _] -> + {{2,0}, StrVersion}; + [_, "1.99" | _] -> + {{2,0}, StrVersion}; + [_, "1.3" | _] -> + {{1,3}, StrVersion}; + [_, "1.5" | _] -> + {{1,5}, StrVersion} + end + catch + error:_ -> + {undefined, "unknown version"} end. key_exchange_init_msg(Ssh0) -> diff --git a/lib/ssh/test/ssh_connection_SUITE.erl b/lib/ssh/test/ssh_connection_SUITE.erl index c115ccee5f..d63b3f2a75 100644 --- a/lib/ssh/test/ssh_connection_SUITE.erl +++ b/lib/ssh/test/ssh_connection_SUITE.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2008-2013. All Rights Reserved. +%% Copyright Ericsson AB 2008-2014. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -31,8 +31,8 @@ %% Common Test interface functions ----------------------------------- %%-------------------------------------------------------------------- -suite() -> - [{ct_hooks,[ts_install_cth]}]. +%% suite() -> +%% [{ct_hooks,[ts_install_cth]}]. all() -> [ @@ -40,7 +40,11 @@ all() -> interrupted_send, start_shell, start_shell_exec, - start_shell_exec_fun + start_shell_exec_fun, + gracefull_invalid_version, + gracefull_invalid_start, + gracefull_invalid_long_start, + gracefull_invalid_long_start_no_nl ]. groups() -> [{openssh_payload, [], [simple_exec, @@ -302,7 +306,7 @@ start_shell(Config) when is_list(Config) -> ok = ssh_connection:shell(ConnectionRef,ChannelId0), receive - {ssh_cm,ConnectionRef, {data, ChannelId, 0, <<"Enter command\r\n">>}} -> + {ssh_cm,ConnectionRef, {data, ChannelId0, 0, <<"Enter command\r\n">>}} -> ok after 5000 -> ct:fail("CLI Timeout") @@ -333,10 +337,10 @@ start_shell_exec(Config) when is_list(Config) -> {ok, ChannelId0} = ssh_connection:session_channel(ConnectionRef, infinity), success = ssh_connection:exec(ConnectionRef, ChannelId0, - "testing", infinity), + "testing", infinity), receive - {ssh_cm,ConnectionRef, {data, ChannelId, 0, <<"testing\r\n">>}} -> - ok + {ssh_cm,ConnectionRef, {data, ChannelId0, 0, <<"testing\r\n">>}} -> + ok after 5000 -> ct:fail("Exec Timeout") end, @@ -370,14 +374,99 @@ start_shell_exec_fun(Config) when is_list(Config) -> "testing", infinity), receive - {ssh_cm,ConnectionRef, {data, ChannelId, 0, <<"testing\r\n">>}} -> - ok + {ssh_cm,ConnectionRef, {data, ChannelId0, 0, <<"testing\r\n">>}} -> + ok after 5000 -> ct:fail("Exec Timeout") end, ssh:close(ConnectionRef), ssh:stop_daemon(Pid). + +%%-------------------------------------------------------------------- + +gracefull_invalid_version(Config) when is_list(Config) -> + PrivDir = ?config(priv_dir, Config), + UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth + file:make_dir(UserDir), + SysDir = ?config(data_dir, Config), + {_Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir}, + {user_dir, UserDir}, + {password, "morot"}]), + + {ok, S} = gen_tcp:connect(Host, Port, []), + ok = gen_tcp:send(S, ["SSH-8.-1","\r\n"]), + receive + Verstring -> + ct:pal("Server version: ~p~n", [Verstring]), + receive + {tcp_closed, S} -> + ok + end + end. + +gracefull_invalid_start(Config) when is_list(Config) -> + PrivDir = ?config(priv_dir, Config), + UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth + file:make_dir(UserDir), + SysDir = ?config(data_dir, Config), + {_Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir}, + {user_dir, UserDir}, + {password, "morot"}]), + + {ok, S} = gen_tcp:connect(Host, Port, []), + ok = gen_tcp:send(S, ["foobar","\r\n"]), + receive + Verstring -> + ct:pal("Server version: ~p~n", [Verstring]), + receive + {tcp_closed, S} -> + ok + end + end. + +gracefull_invalid_long_start(Config) when is_list(Config) -> + PrivDir = ?config(priv_dir, Config), + UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth + file:make_dir(UserDir), + SysDir = ?config(data_dir, Config), + {_Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir}, + {user_dir, UserDir}, + {password, "morot"}]), + + {ok, S} = gen_tcp:connect(Host, Port, []), + ok = gen_tcp:send(S, [lists:duplicate(257, $a), "\r\n"]), + receive + Verstring -> + ct:pal("Server version: ~p~n", [Verstring]), + receive + {tcp_closed, S} -> + ok + end + end. + + +gracefull_invalid_long_start_no_nl(Config) when is_list(Config) -> + PrivDir = ?config(priv_dir, Config), + UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth + file:make_dir(UserDir), + SysDir = ?config(data_dir, Config), + {_Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir}, + {user_dir, UserDir}, + {password, "morot"}]), + + {ok, S} = gen_tcp:connect(Host, Port, []), + ok = gen_tcp:send(S, [lists:duplicate(257, $a), "\r\n"]), + receive + Verstring -> + ct:pal("Server version: ~p~n", [Verstring]), + receive + {tcp_closed, S} -> + ok + end + end. + + %%-------------------------------------------------------------------- %% Internal functions ------------------------------------------------ %%-------------------------------------------------------------------- -- cgit v1.2.3 From 0fba2ec56dd3557a6ef74a211581dce9827be1a6 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Mon, 22 Sep 2014 10:12:39 +0200 Subject: ssh: Add format_status/2 so sensitive data will not be present in logs --- lib/ssh/src/ssh_connection_handler.erl | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl index 68736c3365..fba278aba7 100644 --- a/lib/ssh/src/ssh_connection_handler.erl +++ b/lib/ssh/src/ssh_connection_handler.erl @@ -48,7 +48,7 @@ userauth/2, connected/2]). -export([init/1, handle_event/3, - handle_sync_event/4, handle_info/3, terminate/3, code_change/4]). + handle_sync_event/4, handle_info/3, terminate/3, format_status/2, code_change/4]). -record(state, { role, @@ -963,6 +963,36 @@ terminate_subsytem(#connection{system_supervisor = SysSup, terminate_subsytem(_) -> ok. +format_status(normal, [_, State]) -> + [{data, [{"StateData", State}]}]; +format_status(terminate, [_, State]) -> + SshParams0 = (State#state.ssh_params), + SshParams = SshParams0#ssh{c_keyinit = "***", + s_keyinit = "***", + send_mac_key = "***", + send_mac_size = "***", + recv_mac_key = "***", + recv_mac_size = "***", + encrypt_keys = "***", + encrypt_ctx = "***", + decrypt_keys = "***", + decrypt_ctx = "***", + compress_ctx = "***", + decompress_ctx = "***", + shared_secret = "***", + exchanged_hash = "***", + session_id = "***", + keyex_key = "***", + keyex_info = "***", + available_host_keys = "***"}, + [{data, [{"StateData", State#state{decoded_data_buffer = "***", + encoded_data_buffer = "***", + key_exchange_init_msg = "***", + opts = "***", + recbuf = "***", + ssh_params = SshParams + }}]}]. + %%-------------------------------------------------------------------- -spec code_change(OldVsn::term(), state_name(), Oldstate::term(), Extra::term()) -> {ok, state_name(), #state{}}. -- cgit v1.2.3