[erlang-bugs] Bugs in ssh regnotiation

[Simon Cornish]

Hi,
There are two bugs surrounding the OTP ssh implementation of key rengotiation.

The first is the handling of the sent data statistics. The port stats
are not accumulated so that once rekey_limit bytes (by default, 1GB)
have been transmitted the connection will be rekeyed every minute, not
after the next 1GB.

The attached patch, rekey_limit.diff, provides a simple solution.

In the second bug, ssh_connection_handler processes channel data
requests from either the application or the network during
renegotiation. This is in violation of the RFC and causes the peer to
disconnect (observed with sshd on Solaris 10 and SLES 11).

The solution is to queue those events for processing until after
renegotiation is complete. The second attached patch is in that
direction. It's not particularly complete hence the reason this is an
email to erlang-bugs and not a pull request.

This gist can recreate the fault fairly dependably by ensuring a
constant flow of data.

https://gist.github.com/dotsimon/ef41296db307561d8f94

/Simon
-------------- next part --------------
diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl
index b7ccb02..59e80e6 100644
--- a/lib/ssh/src/ssh_connection_handler.erl
+++ b/lib/ssh/src/ssh_connection_handler.erl
@@ -68,6 +68,7 @@
 	  undecoded_packet_length, %  integer()
 	  key_exchange_init_msg,   %  #ssh_msg_kexinit{}
 	  renegotiate = false,     %  boolean() 
+	  last_size_rekey = 0,
 	  connection_queue,
 	  address,
 	  port,
@@ -596,7 +597,8 @@ handle_event(renegotiate, StateName, State) ->
 
 %% Rekey due to sent data limit reached?
 handle_event(data_size, connected, #state{ssh_params = Ssh0} = State) ->
-    {ok, [{send_oct,Sent}]} = inet:getstat(State#state.socket, [send_oct]),
+    {ok, [{send_oct,Sent0}]} = inet:getstat(State#state.socket, [send_oct]),
+    Sent = Sent0 - State#state.last_size_rekey,
     MaxSent = proplists:get_value(rekey_limit, State#state.opts, 1024000000),
     timer:apply_after(?REKEY_DATA_TIMOUT, gen_fsm, send_all_state_event, [self(), data_size]),
     case Sent >= MaxSent of
@@ -606,7 +608,8 @@ handle_event(data_size, connected, #state{ssh_params = Ssh0} = State) ->
 	    {next_state, kexinit,
 	     next_packet(State#state{ssh_params = Ssh,
 				     key_exchange_init_msg = KeyInitMsg,
-				     renegotiate = true})};
+				     renegotiate = true,
+				     last_size_rekey = Sent0})};
 	_ ->
 	    {next_state, connected, next_packet(State)}
     end;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: queue_data.patch
Type: application/octet-stream
Size: 4269 bytes
Desc: not available
URL: <http://erlang.org/pipermail/erlang-bugs/attachments/20150428/39e52dc5/attachment.obj>