From 0e4ae2a2b5d291b5e918f3ab9e675c4137d1888b Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Fri, 17 Feb 2012 14:30:19 +0100 Subject: Improved error handling --- lib/ssh/src/ssh_auth.erl | 2 +- lib/ssh/src/ssh_connection_handler.erl | 160 ++++++++++++++++++++++----------- lib/ssh/test/ssh_basic_SUITE.erl | 30 +++---- lib/ssh/test/ssh_test_lib.erl | 8 +- lib/ssh/test/ssh_to_openssh_SUITE.erl | 14 +-- 5 files changed, 134 insertions(+), 80 deletions(-) diff --git a/lib/ssh/src/ssh_auth.erl b/lib/ssh/src/ssh_auth.erl index 62d684f4dc..1a4517c689 100644 --- a/lib/ssh/src/ssh_auth.erl +++ b/lib/ssh/src/ssh_auth.erl @@ -340,7 +340,7 @@ verify_sig(SessionId, User, Service, Alg, KeyBlob, SigWLen, Opts) -> ?UINT32(SigLen), Sig:SigLen/binary>> = AlgSig, ssh_transport:verify(PlainText, sha, Sig, Key); false -> - {error, key_unacceptable} + false end. build_sig_data(SessionId, User, Service, KeyBlob, Alg) -> diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl index 9079089d5d..670250730d 100644 --- a/lib/ssh/src/ssh_connection_handler.erl +++ b/lib/ssh/src/ssh_connection_handler.erl @@ -41,7 +41,7 @@ -export([hello/2, kexinit/2, key_exchange/2, new_keys/2, userauth/2, connected/2]). --export([init/1, state_name/3, handle_event/3, +-export([init/1, handle_event/3, handle_sync_event/4, handle_info/3, terminate/3, code_change/4]). %% spawn export @@ -110,18 +110,23 @@ init([Role, Manager, Socket, SshOpts]) -> ssh_bits:install_messages(ssh_transport:transport_messages(NumVsn)), {Protocol, Callback, CloseTag} = proplists:get_value(transport, SshOpts, {tcp, gen_tcp, tcp_closed}), - Ssh = init_ssh(Role, NumVsn, StrVsn, SshOpts, Socket), - {ok, hello, #state{ssh_params = - Ssh#ssh{send_sequence = 0, recv_sequence = 0}, - socket = Socket, - decoded_data_buffer = <<>>, - encoded_data_buffer = <<>>, - transport_protocol = Protocol, - transport_cb = Callback, - transport_close_tag = CloseTag, - manager = Manager, - opts = SshOpts - }}. + try init_ssh(Role, NumVsn, StrVsn, SshOpts, Socket) of + Ssh -> + {ok, hello, #state{ssh_params = + Ssh#ssh{send_sequence = 0, recv_sequence = 0}, + socket = Socket, + decoded_data_buffer = <<>>, + encoded_data_buffer = <<>>, + transport_protocol = Protocol, + transport_cb = Callback, + transport_close_tag = CloseTag, + manager = Manager, + opts = SshOpts + }} + catch + exit:Reason -> + {stop, {shutdown, Reason}} + end. %%-------------------------------------------------------------------- %% Function: %% state_name(Event, State) -> {next_state, NextStateName, NextState}| @@ -179,7 +184,12 @@ kexinit({#ssh_msg_kexinit{} = Kex, Payload}, next_packet(State#state{ssh_params = Ssh})} catch #ssh_msg_disconnect{} = DisconnectMsg -> - handle_disconnect(DisconnectMsg, State) + handle_disconnect(DisconnectMsg, State); + _:Error -> + Desc = log_error(Error), + handle_disconnect(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_KEY_EXCHANGE_FAILED, + description = Desc, + language = "en"}, State) end. key_exchange(#ssh_msg_kexdh_init{} = Msg, @@ -192,7 +202,12 @@ key_exchange(#ssh_msg_kexdh_init{} = Msg, {next_state, new_keys, next_packet(State#state{ssh_params = Ssh})} catch #ssh_msg_disconnect{} = DisconnectMsg -> - handle_disconnect(DisconnectMsg, State) + handle_disconnect(DisconnectMsg, State); + _:Error -> + Desc = log_error(Error), + handle_disconnect(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_KEY_EXCHANGE_FAILED, + description = Desc, + language = "en"}, State) end; key_exchange(#ssh_msg_kexdh_reply{} = Msg, @@ -203,7 +218,12 @@ key_exchange(#ssh_msg_kexdh_reply{} = Msg, {next_state, new_keys, next_packet(State#state{ssh_params = Ssh})} catch #ssh_msg_disconnect{} = DisconnectMsg -> - handle_disconnect(DisconnectMsg, State) + handle_disconnect(DisconnectMsg, State); + _:Error -> + Desc = log_error(Error), + handle_disconnect(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_KEY_EXCHANGE_FAILED, + description = Desc, + language = "en"}, State) end; key_exchange(#ssh_msg_kex_dh_gex_group{} = Msg, @@ -216,7 +236,12 @@ key_exchange(#ssh_msg_kex_dh_gex_group{} = Msg, {next_state, new_keys, next_packet(State#state{ssh_params = Ssh})} catch #ssh_msg_disconnect{} = DisconnectMsg -> - handle_disconnect(DisconnectMsg, State) + handle_disconnect(DisconnectMsg, State); + _:Error -> + Desc = log_error(Error), + handle_disconnect(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_KEY_EXCHANGE_FAILED, + description = Desc, + language = "en"}, State) end; key_exchange(#ssh_msg_kex_dh_gex_request{} = Msg, @@ -227,7 +252,12 @@ key_exchange(#ssh_msg_kex_dh_gex_request{} = Msg, {next_state, new_keys, next_packet(State#state{ssh_params = Ssh})} catch #ssh_msg_disconnect{} = DisconnectMsg -> - handle_disconnect(DisconnectMsg, State) + handle_disconnect(DisconnectMsg, State); + _:Error -> + Desc = log_error(Error), + handle_disconnect(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_KEY_EXCHANGE_FAILED, + description = Desc, + language = "en"}, State) end; key_exchange(#ssh_msg_kex_dh_gex_reply{} = Msg, #state{ssh_params = #ssh{role = client} = Ssh0} = State) -> @@ -237,7 +267,12 @@ key_exchange(#ssh_msg_kex_dh_gex_reply{} = Msg, {next_state, new_keys, next_packet(State#state{ssh_params = Ssh})} catch #ssh_msg_disconnect{} = DisconnectMsg -> - handle_disconnect(DisconnectMsg, State) + handle_disconnect(DisconnectMsg, State); + _:Error -> + Desc = log_error(Error), + handle_disconnect(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_KEY_EXCHANGE_FAILED, + description = Desc, + language = "en"}, State) end. new_keys(#ssh_msg_newkeys{} = Msg, #state{ssh_params = Ssh0} = State0) -> @@ -249,7 +284,12 @@ new_keys(#ssh_msg_newkeys{} = Msg, #state{ssh_params = Ssh0} = State0) -> catch #ssh_msg_disconnect{} = DisconnectMsg -> handle_disconnect(DisconnectMsg, State0), - {stop, normal, State0} + {stop, normal, State0}; + _:Error -> + Desc = log_error(Error), + handle_disconnect(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_KEY_EXCHANGE_FAILED, + description = Desc, + language = "en"}, State0) end. userauth(#ssh_msg_service_request{name = "ssh-userauth"} = Msg, @@ -262,7 +302,12 @@ userauth(#ssh_msg_service_request{name = "ssh-userauth"} = Msg, {next_state, userauth, next_packet(State#state{ssh_params = Ssh})} catch #ssh_msg_disconnect{} = DisconnectMsg -> - handle_disconnect(DisconnectMsg, State) + handle_disconnect(DisconnectMsg, State); + _:Error -> + Desc = log_error(Error), + handle_disconnect(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_SERVICE_NOT_AVAILABLE, + description = Desc, + language = "en"}, State) end; userauth(#ssh_msg_service_accept{name = "ssh-userauth"}, @@ -284,7 +329,12 @@ userauth(#ssh_msg_userauth_request{service = "ssh-connection", {next_state, userauth, next_packet(State#state{ssh_params = Ssh})} catch #ssh_msg_disconnect{} = DisconnectMsg -> - handle_disconnect(DisconnectMsg, State) + handle_disconnect(DisconnectMsg, State); + _:Error -> + Desc = log_error(Error), + handle_disconnect(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_SERVICE_NOT_AVAILABLE, + description = Desc, + language = "en"}, State) end; userauth(#ssh_msg_userauth_request{service = "ssh-connection", @@ -307,7 +357,12 @@ userauth(#ssh_msg_userauth_request{service = "ssh-connection", {next_state, userauth, next_packet(State#state{ssh_params = Ssh})} catch #ssh_msg_disconnect{} = DisconnectMsg -> - handle_disconnect(DisconnectMsg, State) + handle_disconnect(DisconnectMsg, State); + _:Error -> + Desc = log_error(Error), + handle_disconnect(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_SERVICE_NOT_AVAILABLE, + description = Desc, + language = "en"}, State) end; userauth(#ssh_msg_userauth_info_request{} = Msg, @@ -319,7 +374,12 @@ userauth(#ssh_msg_userauth_info_request{} = Msg, {next_state, userauth, next_packet(State#state{ssh_params = Ssh})} catch #ssh_msg_disconnect{} = DisconnectMsg -> - handle_disconnect(DisconnectMsg, State) + handle_disconnect(DisconnectMsg, State); + _:Error -> + Desc = log_error(Error), + handle_disconnect(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_SERVICE_NOT_AVAILABLE, + description = Desc, + language = "en"}, State) end; userauth(#ssh_msg_userauth_info_response{} = Msg, @@ -330,7 +390,12 @@ userauth(#ssh_msg_userauth_info_response{} = Msg, {next_state, userauth, next_packet(State#state{ssh_params = Ssh})} catch #ssh_msg_disconnect{} = DisconnectMsg -> - handle_disconnect(DisconnectMsg, State) + handle_disconnect(DisconnectMsg, State); + _:Error -> + Desc = log_error(Error), + handle_disconnect(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_SERVICE_NOT_AVAILABLE, + description = Desc, + language = "en"}, State) end; userauth(#ssh_msg_userauth_success{}, #state{ssh_params = #ssh{role = client}, @@ -397,25 +462,6 @@ userauth(#ssh_msg_userauth_banner{message = Msg}, connected({#ssh_msg_kexinit{}, _Payload} = Event, State) -> kexinit(Event, State#state{renegotiate = true}). -%%-------------------------------------------------------------------- -%% Function: -%% state_name(Event, From, State) -> {next_state, NextStateName, NextState} | -%% {next_state, NextStateName, -%% NextState, Timeout} | -%% {reply, Reply, NextStateName, NextState}| -%% {reply, Reply, NextStateName, -%% NextState, Timeout} | -%% {stop, Reason, NewState}| -%% {stop, Reason, Reply, NewState} -%% Description: There should be one instance of this function for each -%% possible state name. Whenever a gen_fsm receives an event sent using -%% gen_fsm:sync_send_event/2,3, the instance of this function with the same -%% name as the current state name StateName is called to handle the event. -%%-------------------------------------------------------------------- -state_name(_Event, _From, State) -> - Reply = ok, - {reply, Reply, state_name, State}. - %%-------------------------------------------------------------------- %% Function: %% handle_event(Event, StateName, State) -> {next_state, NextStateName, @@ -566,10 +612,9 @@ handle_info({Protocol, Socket, Data}, Statename, Statename, State); handle_info({CloseTag, _Socket}, _StateName, - #state{transport_close_tag = CloseTag, %%manager = Pid, + #state{transport_close_tag = CloseTag, ssh_params = #ssh{role = _Role, opts = _Opts}} = State) -> - %%ok = ssh_connection_manager:delivered(Pid), - {stop, normal, State}; + {stop, {shutdown, connection_lost}, State}; handle_info(UnexpectedMessage, StateName, #state{ssh_params = SshParams} = State) -> Msg = lists:flatten(io_lib:format( "Unexpected message '~p' received in state '~p'\n" @@ -603,12 +648,18 @@ terminate(shutdown, _, State) -> language = "en"}, handle_disconnect(DisconnectMsg, State); -terminate(Reason, _, State) -> - Desc = io_lib:format("Erlang ssh connection handler failed with reason: " - "~p , please report this to erlang-bugs@erlang.org \n", - [Reason]), + +terminate({shutdown, connection_lost}, _, State) -> DisconnectMsg = #ssh_msg_disconnect{code = ?SSH_DISCONNECT_CONNECTION_LOST, + description = "Connection Lost", + language = "en"}, + handle_disconnect(DisconnectMsg, State); + +terminate(Reason, _, State) -> + Desc = log_error(Reason), + DisconnectMsg = + #ssh_msg_disconnect{code = ?SSH_DISCONNECT_SERVICE_NOT_AVAILABLE, description = Desc, language = "en"}, handle_disconnect(DisconnectMsg, State). @@ -928,3 +979,10 @@ ssh_info([peer | Rest], #ssh{peer = Peer} = SshParams, Acc) -> ssh_info([ _ | Rest], SshParams, Acc) -> ssh_info(Rest, SshParams, Acc). + +log_error(Reason) -> + Report = io_lib:format("Erlang ssh connection handler failed with reason: " + "~p , please report this to erlang-bugs@erlang.org \n", + [Reason]), + error_logger:error_report(Report), + "Internal error". diff --git a/lib/ssh/test/ssh_basic_SUITE.erl b/lib/ssh/test/ssh_basic_SUITE.erl index 9c13180159..8ec037dd8f 100644 --- a/lib/ssh/test/ssh_basic_SUITE.erl +++ b/lib/ssh/test/ssh_basic_SUITE.erl @@ -362,11 +362,11 @@ server_password_option(Config) when is_list(Config) -> {user_interaction, false}, {user_dir, UserDir}]), {error, Reason} = - ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true}, - {user, "vego"}, - {password, "foo"}, - {user_interaction, false}, - {user_dir, UserDir}]), + ssh:connect(Host, Port, [{silently_accept_hosts, true}, + {user, "vego"}, + {password, "foo"}, + {user_interaction, false}, + {user_dir, UserDir}]), test_server:format("Test of wrong password: Error msg: ~p ~n", [Reason]), @@ -397,21 +397,21 @@ server_userpassword_option(Config) when is_list(Config) -> ssh:close(ConnectionRef), {error, Reason0} = - ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true}, - {user, "foo"}, - {password, "morot"}, - {user_interaction, false}, - {user_dir, UserDir}]), + ssh:connect(Host, Port, [{silently_accept_hosts, true}, + {user, "foo"}, + {password, "morot"}, + {user_interaction, false}, + {user_dir, UserDir}]), test_server:format("Test of user foo that does not exist. " "Error msg: ~p ~n", [Reason0]), {error, Reason1} = - ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true}, - {user, "vego"}, - {password, "foo"}, - {user_interaction, false}, - {user_dir, UserDir}]), + ssh:connect(Host, Port, [{silently_accept_hosts, true}, + {user, "vego"}, + {password, "foo"}, + {user_interaction, false}, + {user_dir, UserDir}]), test_server:format("Test of wrong Password. " "Error msg: ~p ~n", [Reason1]), diff --git a/lib/ssh/test/ssh_test_lib.erl b/lib/ssh/test/ssh_test_lib.erl index 26bbdf5c5c..c8a4fe1c62 100644 --- a/lib/ssh/test/ssh_test_lib.erl +++ b/lib/ssh/test/ssh_test_lib.erl @@ -43,12 +43,8 @@ connect(Host, Options) -> connect(any, Port, Options) -> connect(hostname(), Port, Options); connect(Host, Port, Options) -> - case ssh:connect(Host, Port, Options) of - {ok, ConnectionRef} -> - ConnectionRef; - Error -> - Error - end. + {ok, ConnectionRef} = ssh:connect(Host, Port, Options), + ConnectionRef. daemon(Options) -> daemon(any, inet_port(), Options). diff --git a/lib/ssh/test/ssh_to_openssh_SUITE.erl b/lib/ssh/test/ssh_to_openssh_SUITE.erl index dfe526564d..c337617ee4 100644 --- a/lib/ssh/test/ssh_to_openssh_SUITE.erl +++ b/lib/ssh/test/ssh_to_openssh_SUITE.erl @@ -445,7 +445,7 @@ erlang_client_openssh_server_password(Config) when is_list(Config) -> %% to make sure we don't public-key-auth UserDir = ?config(data_dir, Config), {error, Reason0} = - ssh_test_lib:connect(?SSH_DEFAULT_PORT, [{silently_accept_hosts, true}, + ssh:connect(any, ?SSH_DEFAULT_PORT, [{silently_accept_hosts, true}, {user, "foo"}, {password, "morot"}, {user_interaction, false}, @@ -459,12 +459,12 @@ erlang_client_openssh_server_password(Config) when is_list(Config) -> case length(string:tokens(User, " ")) of 1 -> {error, Reason1} = - ssh_test_lib:connect(?SSH_DEFAULT_PORT, - [{silently_accept_hosts, true}, - {user, User}, - {password, "foo"}, - {user_interaction, false}, - {user_dir, UserDir}]), + ssh:connect(any, ?SSH_DEFAULT_PORT, + [{silently_accept_hosts, true}, + {user, User}, + {password, "foo"}, + {user_interaction, false}, + {user_dir, UserDir}]), test_server:format("Test of wrong Pasword. " "Error msg: ~p~n", [Reason1]); _ -> -- cgit v1.2.3