aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSimon Cornish <[email protected]>2015-05-11 15:54:50 -0700
committerHans Nilsson <[email protected]>2015-05-21 10:40:39 +0200
commitd467173208581ee70213d34674ea841813af2e7e (patch)
tree2c4dec499dc6effc49e280e5cb4d4823cedd51c4
parent5e71fae6329a8cfa82ac5d5f9146e947fc92f542 (diff)
downloadotp-d467173208581ee70213d34674ea841813af2e7e.tar.gz
otp-d467173208581ee70213d34674ea841813af2e7e.tar.bz2
otp-d467173208581ee70213d34674ea841813af2e7e.zip
Fix protocol violations during rekeying
In RFC 4253, sections 7.1 & 9 describe rekeying with special attention to the protocol messages that may be received and may not be sent during rekeying. This patch fixes a number of problems during rekeying caused by data & requests received from the network, and/or data & requests sent by the user.
-rw-r--r--lib/ssh/src/ssh_connection_handler.erl237
1 files changed, 146 insertions, 91 deletions
diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl
index 9b11cadab6..65208ae158 100644
--- a/lib/ssh/src/ssh_connection_handler.erl
+++ b/lib/ssh/src/ssh_connection_handler.erl
@@ -71,6 +71,7 @@
key_exchange_init_msg, % #ssh_msg_kexinit{}
renegotiate = false, % boolean()
last_size_rekey = 0,
+ event_queue = [],
connection_queue,
address,
port,
@@ -83,6 +84,11 @@
{next_state, state_name(), term(), timeout()} |
{stop, term(), term()}.
+-type gen_fsm_sync_return() :: {next_state, state_name(), term()} |
+ {next_state, state_name(), term(), timeout()} |
+ {reply, term(), state_name(), term()} |
+ {stop, term(), term(), term()}.
+
%%====================================================================
%% Internal application API
%%====================================================================
@@ -433,9 +439,7 @@ key_exchange(#ssh_msg_kex_dh_gex_reply{} = Msg,
new_keys(#ssh_msg_newkeys{} = Msg, #state{ssh_params = Ssh0} = State0) ->
{ok, Ssh} = ssh_transport:handle_new_keys(Msg, Ssh0),
- {NextStateName, State} =
- after_new_keys(State0#state{ssh_params = Ssh}),
- {next_state, NextStateName, next_packet(State)}.
+ after_new_keys(next_packet(State0#state{ssh_params = Ssh})).
%%--------------------------------------------------------------------
-spec userauth(#ssh_msg_service_request{} | #ssh_msg_service_accept{} |
@@ -594,33 +598,6 @@ handle_event(#ssh_msg_debug{always_display = Display, message = DbgMsg, language
handle_event(#ssh_msg_unimplemented{}, StateName, State) ->
{next_state, StateName, next_packet(State)};
-handle_event({adjust_window, ChannelId, Bytes}, StateName,
- #state{connection_state =
- #connection{channel_cache = Cache}} = State0) ->
- State =
- case ssh_channel:cache_lookup(Cache, ChannelId) of
- #channel{recv_window_size = WinSize, remote_id = Id} = Channel ->
- ssh_channel:cache_update(Cache, Channel#channel{recv_window_size =
- WinSize + Bytes}),
- Msg = ssh_connection:channel_adjust_window_msg(Id, Bytes),
- send_replies([{connection_reply, Msg}], State0);
- undefined ->
- State0
- end,
- {next_state, StateName, next_packet(State)};
-
-handle_event({reply_request, success, ChannelId}, StateName,
- #state{connection_state =
- #connection{channel_cache = Cache}} = State0) ->
- State = case ssh_channel:cache_lookup(Cache, ChannelId) of
- #channel{remote_id = RemoteId} ->
- Msg = ssh_connection:channel_success_msg(RemoteId),
- send_replies([{connection_reply, Msg}], State0);
- undefined ->
- State0
- end,
- {next_state, StateName, State};
-
handle_event(renegotiate, connected, #state{ssh_params = Ssh0}
= State) ->
{KeyInitMsg, SshPacket, Ssh} = ssh_transport:key_exchange_init_msg(Ssh0),
@@ -632,8 +609,7 @@ handle_event(renegotiate, connected, #state{ssh_params = Ssh0}
renegotiate = true})};
handle_event(renegotiate, StateName, State) ->
- timer:apply_after(?REKEY_TIMOUT, gen_fsm, send_all_state_event, [self(), renegotiate]),
- %% Allready in keyexcahange so ignore
+ %% Already in key-exchange so safe to ignore
{next_state, StateName, State};
%% Rekey due to sent data limit reached?
@@ -655,6 +631,38 @@ handle_event(data_size, connected, #state{ssh_params = Ssh0} = State) ->
{next_state, connected, next_packet(State)}
end;
handle_event(data_size, StateName, State) ->
+ %% Already in key-exchange so safe to ignore
+ {next_state, StateName, State};
+
+handle_event(Event, StateName, State) when StateName /= connected ->
+ Events = [{event, Event} | State#state.event_queue],
+ {next_state, StateName, State#state{event_queue = Events}};
+
+handle_event({adjust_window, ChannelId, Bytes}, StateName,
+ #state{connection_state =
+ #connection{channel_cache = Cache}} = State0) ->
+ State =
+ case ssh_channel:cache_lookup(Cache, ChannelId) of
+ #channel{recv_window_size = WinSize, remote_id = Id} = Channel ->
+ ssh_channel:cache_update(Cache, Channel#channel{recv_window_size =
+ WinSize + Bytes}),
+ Msg = ssh_connection:channel_adjust_window_msg(Id, Bytes),
+ send_replies([{connection_reply, Msg}], State0);
+ undefined ->
+ State0
+ end,
+ {next_state, StateName, next_packet(State)};
+
+handle_event({reply_request, success, ChannelId}, StateName,
+ #state{connection_state =
+ #connection{channel_cache = Cache}} = State0) ->
+ State = case ssh_channel:cache_lookup(Cache, ChannelId) of
+ #channel{remote_id = RemoteId} ->
+ Msg = ssh_connection:channel_success_msg(RemoteId),
+ send_replies([{connection_reply, Msg}], State0);
+ undefined ->
+ State0
+ end,
{next_state, StateName, State};
handle_event({request, ChannelPid, ChannelId, Type, Data}, StateName, State0) ->
@@ -685,8 +693,65 @@ handle_event({unknown, Data}, StateName, State) ->
sockname]} | {channel_info, channel_id(), [recv_window |
send_window]} |
{close, channel_id()} | stop, term(), state_name(), #state{})
- -> gen_fsm_state_return().
+ -> gen_fsm_sync_return().
%%--------------------------------------------------------------------
+handle_sync_event(get_print_info, _From, StateName, State) ->
+ Reply =
+ try
+ {inet:sockname(State#state.socket),
+ inet:peername(State#state.socket)
+ }
+ of
+ {{ok,Local}, {ok,Remote}} -> {{Local,Remote},io_lib:format("statename=~p",[StateName])};
+ _ -> {{"-",0},"-"}
+ catch
+ _:_ -> {{"?",0},"?"}
+ end,
+ {reply, Reply, StateName, State};
+
+handle_sync_event({connection_info, Options}, _From, StateName, State) ->
+ Info = ssh_info(Options, State, []),
+ {reply, Info, StateName, State};
+
+handle_sync_event({channel_info, ChannelId, Options}, _From, StateName,
+ #state{connection_state = #connection{channel_cache = Cache}} = State) ->
+ case ssh_channel:cache_lookup(Cache, ChannelId) of
+ #channel{} = Channel ->
+ Info = ssh_channel_info(Options, Channel, []),
+ {reply, Info, StateName, State};
+ undefined ->
+ {reply, [], StateName, State}
+ end;
+
+handle_sync_event({info, ChannelPid}, _From, StateName,
+ #state{connection_state =
+ #connection{channel_cache = Cache}} = State) ->
+ Result = ssh_channel:cache_foldl(
+ fun(Channel, Acc) when ChannelPid == all;
+ Channel#channel.user == ChannelPid ->
+ [Channel | Acc];
+ (_, Acc) ->
+ Acc
+ end, [], Cache),
+ {reply, {ok, Result}, StateName, State};
+
+handle_sync_event(stop, _, _StateName, #state{connection_state = Connection0,
+ role = Role,
+ opts = Opts} = State0) ->
+ {disconnect, Reason, {{replies, Replies}, Connection}} =
+ ssh_connection:handle_msg(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_BY_APPLICATION,
+ description = "User closed down connection",
+ language = "en"}, Connection0, Role),
+ State = send_replies(Replies, State0),
+ SSHOpts = proplists:get_value(ssh_opts, Opts),
+ disconnect_fun(Reason, SSHOpts),
+ {stop, normal, ok, State#state{connection_state = Connection}};
+
+
+handle_sync_event(Event, From, StateName, State) when StateName /= connected ->
+ Events = [{sync, Event, From} | State#state.event_queue],
+ {next_state, StateName, State#state{event_queue = Events}};
+
handle_sync_event({request, ChannelPid, ChannelId, Type, Data, Timeout}, From, StateName, State0) ->
{{replies, Replies}, State1} = handle_request(ChannelPid,
ChannelId, Type, Data,
@@ -789,46 +854,6 @@ handle_sync_event({recv_window, ChannelId}, _From, StateName,
end,
{reply, Reply, StateName, next_packet(State)};
-handle_sync_event(get_print_info, _From, StateName, State) ->
- Reply =
- try
- {inet:sockname(State#state.socket),
- inet:peername(State#state.socket)
- }
- of
- {{ok,Local}, {ok,Remote}} -> {{Local,Remote},io_lib:format("statename=~p",[StateName])};
- _ -> {{"-",0},"-"}
- catch
- _:_ -> {{"?",0},"?"}
- end,
- {reply, Reply, StateName, State};
-
-handle_sync_event({connection_info, Options}, _From, StateName, State) ->
- Info = ssh_info(Options, State, []),
- {reply, Info, StateName, State};
-
-handle_sync_event({channel_info, ChannelId, Options}, _From, StateName,
- #state{connection_state = #connection{channel_cache = Cache}} = State) ->
- case ssh_channel:cache_lookup(Cache, ChannelId) of
- #channel{} = Channel ->
- Info = ssh_channel_info(Options, Channel, []),
- {reply, Info, StateName, State};
- undefined ->
- {reply, [], StateName, State}
- end;
-
-handle_sync_event({info, ChannelPid}, _From, StateName,
- #state{connection_state =
- #connection{channel_cache = Cache}} = State) ->
- Result = ssh_channel:cache_foldl(
- fun(Channel, Acc) when ChannelPid == all;
- Channel#channel.user == ChannelPid ->
- [Channel | Acc];
- (_, Acc) ->
- Acc
- end, [], Cache),
- {reply, {ok, Result}, StateName, State};
-
handle_sync_event({close, ChannelId}, _, StateName,
#state{connection_state =
#connection{channel_cache = Cache}} = State0) ->
@@ -843,19 +868,7 @@ handle_sync_event({close, ChannelId}, _, StateName,
undefined ->
State0
end,
- {reply, ok, StateName, next_packet(State)};
-
-handle_sync_event(stop, _, _StateName, #state{connection_state = Connection0,
- role = Role,
- opts = Opts} = State0) ->
- {disconnect, Reason, {{replies, Replies}, Connection}} =
- ssh_connection:handle_msg(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_BY_APPLICATION,
- description = "User closed down connection",
- language = "en"}, Connection0, Role),
- State = send_replies(Replies, State0),
- SSHOpts = proplists:get_value(ssh_opts, Opts),
- disconnect_fun(Reason, SSHOpts),
- {stop, normal, ok, State#state{connection_state = Connection}}.
+ {reply, ok, StateName, next_packet(State)}.
%%--------------------------------------------------------------------
-spec handle_info({atom(), port(), binary()} | {atom(), port()} |
@@ -1284,8 +1297,17 @@ generate_event(<<?BYTE(Byte), _/binary>> = Msg, StateName,
ConnectionMsg = ssh_message:decode(Msg),
State1 = generate_event_new_state(State0, EncData),
try ssh_connection:handle_msg(ConnectionMsg, Connection0, Role) of
- {{replies, Replies}, Connection} ->
- State = send_replies(Replies, State1#state{connection_state = Connection}),
+ {{replies, Replies0}, Connection} ->
+ if StateName == connected ->
+ Replies = Replies0,
+ State2 = State1;
+ true ->
+ {ConnReplies, Replies} =
+ lists:splitwith(fun not_connected_filter/1, Replies0),
+ Q = State1#state.event_queue ++ ConnReplies,
+ State2 = State1#state{ event_queue = Q }
+ end,
+ State = send_replies(Replies, State2#state{connection_state = Connection}),
{next_state, StateName, next_packet(State)};
{noreply, Connection} ->
{next_state, StateName, next_packet(State1#state{connection_state = Connection})};
@@ -1458,15 +1480,43 @@ next_packet(#state{socket = Socket} = State) ->
State.
after_new_keys(#state{renegotiate = true} = State) ->
- {connected, State#state{renegotiate = false}};
+ State1 = State#state{renegotiate = false, event_queue = []},
+ lists:foldr(fun after_new_keys_events/2, {next_state, connected, State1}, State#state.event_queue);
after_new_keys(#state{renegotiate = false,
ssh_params = #ssh{role = client} = Ssh0} = State) ->
{Msg, Ssh} = ssh_auth:service_request_msg(Ssh0),
send_msg(Msg, State),
- {userauth, State#state{ssh_params = Ssh}};
+ {next_state, userauth, State#state{ssh_params = Ssh}};
after_new_keys(#state{renegotiate = false,
ssh_params = #ssh{role = server}} = State) ->
- {userauth, State}.
+ {next_state, userauth, State}.
+
+after_new_keys_events({sync, _Event, From}, {stop, _Reason, _StateData}=Terminator) ->
+ gen_fsm:reply(From, {error, closed}),
+ Terminator;
+after_new_keys_events(_, {stop, _Reason, _StateData}=Terminator) ->
+ Terminator;
+after_new_keys_events({sync, Event, From}, {next_state, StateName, StateData}) ->
+ case handle_sync_event(Event, From, StateName, StateData) of
+ {reply, Reply, NextStateName, NewStateData} ->
+ gen_fsm:reply(From, Reply),
+ {next_state, NextStateName, NewStateData};
+ {next_state, NextStateName, NewStateData}->
+ {next_state, NextStateName, NewStateData};
+ {stop, Reason, Reply, NewStateData} ->
+ gen_fsm:reply(From, Reply),
+ {stop, Reason, NewStateData}
+ end;
+after_new_keys_events({event, Event}, {next_state, StateName, StateData}) ->
+ case handle_event(Event, StateName, StateData) of
+ {next_state, NextStateName, NewStateData}->
+ {next_state, NextStateName, NewStateData};
+ {stop, Reason, NewStateData} ->
+ {stop, Reason, NewStateData}
+ end;
+after_new_keys_events({connection_reply, _Data} = Reply, {StateName, State}) ->
+ NewState = send_replies([Reply], State),
+ {next_state, StateName, NewState}.
handle_ssh_packet_data(RemainingSshPacketLen, DecData, EncData, StateName,
State) ->
@@ -1627,6 +1677,11 @@ log_error(Reason) ->
error_logger:error_report(Report),
"Internal error".
+not_connected_filter({connection_reply, _Data}) ->
+ true;
+not_connected_filter(_) ->
+ false.
+
send_replies([], State) ->
State;
send_replies([{connection_reply, Data} | Rest], #state{ssh_params = Ssh0} = State) ->