aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHans Nilsson <[email protected]>2015-11-19 17:16:05 +0100
committerHans Nilsson <[email protected]>2015-11-23 14:14:05 +0100
commit226e77ef7162b0fc043d99a5f68f5dcc891fb093 (patch)
treefccfcb33805b931bdd3bbe28e78f98fc933c050e
parentd4a3296ba3117315343057715ee428490e992ef0 (diff)
downloadotp-226e77ef7162b0fc043d99a5f68f5dcc891fb093.tar.gz
otp-226e77ef7162b0fc043d99a5f68f5dcc891fb093.tar.bz2
otp-226e77ef7162b0fc043d99a5f68f5dcc891fb093.zip
ssh: refactor packet reception
There was an assymetric relationship between receiving a ssh-packet (decrypting-mac-decompress) and sending one. When sending, most of the work was defined in the ssh_transport module, while at reception the ssh_connection_handler was the one knowing what to do. This commit moves the reception down to the ssh_transport module where it belongs.
-rw-r--r--lib/ssh/src/ssh_connection_handler.erl129
-rw-r--r--lib/ssh/src/ssh_transport.erl94
2 files changed, 94 insertions, 129 deletions
diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl
index 8448218d91..505c6eb181 100644
--- a/lib/ssh/src/ssh_connection_handler.erl
+++ b/lib/ssh/src/ssh_connection_handler.erl
@@ -970,57 +970,39 @@ handle_info({Protocol, Socket, Info}, hello,
transport_protocol = Protocol} = State) ->
event({info_line, Info}, hello, State);
-handle_info({Protocol, Socket, Data}, Statename,
+handle_info({Protocol, Socket, Data}, StateName,
#state{socket = Socket,
transport_protocol = Protocol,
- ssh_params = #ssh{decrypt_block_size = BlockSize,
- recv_mac_size = MacSize} = Ssh0,
- decoded_data_buffer = <<>>,
- encoded_data_buffer = EncData0} = State0) ->
-
- %% Implementations SHOULD decrypt the length after receiving the
- %% first 8 (or cipher block size, whichever is larger) bytes of a
- %% packet. (RFC 4253: Section 6 - Binary Packet Protocol)
- case size(EncData0) + size(Data) >= erlang:max(8, BlockSize) of
- true ->
- {Ssh, SshPacketLen, DecData, EncData} =
-
- ssh_transport:decrypt_first_block(<<EncData0/binary,
- Data/binary>>, Ssh0),
- case SshPacketLen > ?SSH_MAX_PACKET_SIZE of
- true ->
- DisconnectMsg =
- #ssh_msg_disconnect{code =
- ?SSH_DISCONNECT_PROTOCOL_ERROR,
- description = "Bad packet length "
- ++ integer_to_list(SshPacketLen),
- language = "en"},
- handle_disconnect(DisconnectMsg, State0);
- false ->
- RemainingSshPacketLen =
- (SshPacketLen + ?SSH_LENGHT_INDICATOR_SIZE) -
- BlockSize + MacSize,
- State = State0#state{ssh_params = Ssh},
- handle_ssh_packet_data(RemainingSshPacketLen,
- DecData, EncData, Statename,
- State)
- end;
- false ->
- {next_state, Statename,
- next_packet(State0#state{encoded_data_buffer =
- <<EncData0/binary, Data/binary>>})}
+ ssh_params = Ssh0,
+ decoded_data_buffer = DecData0,
+ encoded_data_buffer = EncData0,
+ undecoded_packet_length = RemainingSshPacketLen0} = State0) ->
+ Encoded = <<EncData0/binary, Data/binary>>,
+ case ssh_transport:handle_packet_part(DecData0, Encoded, RemainingSshPacketLen0, Ssh0) of
+ {get_more, DecBytes, EncDataRest, RemainingSshPacketLen, Ssh1} ->
+ {next_state, StateName,
+ next_packet(State0#state{encoded_data_buffer = EncDataRest,
+ decoded_data_buffer = DecBytes,
+ undecoded_packet_length = RemainingSshPacketLen,
+ ssh_params = Ssh1})};
+ {decoded, MsgBytes, EncDataRest, Ssh1} ->
+ generate_event(MsgBytes, StateName,
+ State0#state{ssh_params = Ssh1,
+ %% Important to be set for
+ %% next_packet
+%%% FIXME: the following three seem to always be set in generate_event!
+ decoded_data_buffer = <<>>,
+ undecoded_packet_length = undefined,
+ encoded_data_buffer = EncDataRest},
+ EncDataRest);
+ {bad_mac, Ssh1} ->
+ DisconnectMsg =
+ #ssh_msg_disconnect{code = ?SSH_DISCONNECT_PROTOCOL_ERROR,
+ description = "Bad mac",
+ language = ""},
+ handle_disconnect(DisconnectMsg, State0#state{ssh_params=Ssh1})
end;
-
-handle_info({Protocol, Socket, Data}, Statename,
- #state{socket = Socket,
- transport_protocol = Protocol,
- decoded_data_buffer = DecData,
- encoded_data_buffer = EncData,
- undecoded_packet_length = Len} =
- State) when is_integer(Len) ->
- handle_ssh_packet_data(Len, DecData, <<EncData/binary, Data/binary>>,
- Statename, State);
-
+
handle_info({CloseTag, _Socket}, _StateName,
#state{transport_close_tag = CloseTag,
ssh_params = #ssh{role = _Role, opts = _Opts}} = State) ->
@@ -1631,57 +1613,6 @@ 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) ->
- EncSize = size(EncData),
- case RemainingSshPacketLen > EncSize of
- true ->
- {next_state, StateName,
- next_packet(State#state{decoded_data_buffer = DecData,
- encoded_data_buffer = EncData,
- undecoded_packet_length =
- RemainingSshPacketLen})};
- false ->
- handle_ssh_packet(RemainingSshPacketLen, StateName,
- State#state{decoded_data_buffer = DecData,
- encoded_data_buffer = EncData})
-
- end.
-
-handle_ssh_packet(Length, StateName, #state{decoded_data_buffer = DecData0,
- encoded_data_buffer = EncData0,
- ssh_params = Ssh0,
- transport_protocol = _Protocol,
- socket = _Socket} = State0) ->
- try
- {Ssh1, DecData, EncData, Mac} =
- ssh_transport:unpack(EncData0, Length, Ssh0),
- SshPacket = <<DecData0/binary, DecData/binary>>,
- case ssh_transport:is_valid_mac(Mac, SshPacket, Ssh1) of
- true ->
- PacketData = ssh_transport:msg_data(SshPacket),
- {Ssh1, Msg} = ssh_transport:decompress(Ssh1, PacketData),
- generate_event(Msg, StateName,
- State0#state{ssh_params = Ssh1,
- %% Important to be set for
- %% next_packet
- decoded_data_buffer = <<>>},
- EncData);
- false ->
- DisconnectMsg =
- #ssh_msg_disconnect{code = ?SSH_DISCONNECT_PROTOCOL_ERROR,
- description = "Bad mac",
- language = "en"},
- handle_disconnect(DisconnectMsg, State0)
- end
- catch _:_ ->
- Disconnect =
- #ssh_msg_disconnect{code = ?SSH_DISCONNECT_PROTOCOL_ERROR,
- description = "Bad input",
- language = "en"},
- handle_disconnect(Disconnect, State0)
- end.
-
handle_disconnect(DisconnectMsg, State) ->
handle_disconnect(own, DisconnectMsg, State).
diff --git a/lib/ssh/src/ssh_transport.erl b/lib/ssh/src/ssh_transport.erl
index 0c999b96cc..f18e4b4d01 100644
--- a/lib/ssh/src/ssh_transport.erl
+++ b/lib/ssh/src/ssh_transport.erl
@@ -31,10 +31,10 @@
-include("ssh.hrl").
-export([versions/2, hello_version_msg/1]).
--export([next_seqnum/1, decrypt_first_block/2, decrypt_blocks/3,
+-export([next_seqnum/1,
supported_algorithms/0, supported_algorithms/1,
default_algorithms/0, default_algorithms/1,
- is_valid_mac/3,
+ handle_packet_part/4,
handle_hello_version/1,
key_exchange_init_msg/1,
key_init/3, new_keys_message/1,
@@ -45,9 +45,13 @@
handle_kex_ecdh_init/2,
handle_kex_ecdh_reply/2,
extract_public_key/1,
- unpack/3, decompress/2, ssh_packet/2, pack/2, pack/3, msg_data/1,
+ ssh_packet/2, pack/2,
sign/3, verify/4]).
+%%% For test suites
+-export([pack/3]).
+-export([decompress/2, decrypt_blocks/3, is_valid_mac/3 ]). % FIXME: remove
+
%%%----------------------------------------------------------------------------
%%%
%%% There is a difference between supported and default algorithms. The
@@ -196,12 +200,6 @@ hello_version_msg(Data) ->
next_seqnum(SeqNum) ->
(SeqNum + 1) band 16#ffffffff.
-decrypt_first_block(Bin, #ssh{decrypt_block_size = BlockSize} = Ssh0) ->
- <<EncBlock:BlockSize/binary, EncData/binary>> = Bin,
- {Ssh, <<?UINT32(PacketLen), _/binary>> = DecData} =
- decrypt(Ssh0, EncBlock),
- {Ssh, PacketLen, DecData, EncData}.
-
decrypt_blocks(Bin, Length, Ssh0) ->
<<EncBlocks:Length/binary, EncData/binary>> = Bin,
{Ssh, DecData} = decrypt(Ssh0, EncBlocks),
@@ -938,27 +936,61 @@ pack(Data0, #ssh{encrypt_block_size = BlockSize,
Ssh = Ssh2#ssh{send_sequence = (SeqNum+1) band 16#ffffffff},
{Packet, Ssh}.
-unpack(EncodedSoFar, ReminingLenght, #ssh{recv_mac_size = MacSize} = Ssh0) ->
- SshLength = ReminingLenght - MacSize,
- {NoMac, Mac, Rest} = case MacSize of
- 0 ->
- <<NoMac0:SshLength/binary,
- Rest0/binary>> = EncodedSoFar,
- {NoMac0, <<>>, Rest0};
- _ ->
- <<NoMac0:SshLength/binary,
- Mac0:MacSize/binary,
- Rest0/binary>> = EncodedSoFar,
- {NoMac0, Mac0, Rest0}
- end,
- {Ssh1, DecData, <<>>} =
- case SshLength of
- 0 ->
- {Ssh0, <<>>, <<>>};
- _ ->
- decrypt_blocks(NoMac, SshLength, Ssh0)
- end,
- {Ssh1, DecData, Rest, Mac}.
+
+handle_packet_part(<<>>, Encoded0, undefined, Ssh0) ->
+ %% New ssh packet
+ case get_length(Encoded0, Ssh0) of
+ get_more ->
+ %% too short to get the length
+ {get_more, <<>>, Encoded0, undefined, Ssh0};
+
+ {ok, PacketLen, _DecData, _Encoded1, _Ssh1} when PacketLen > ?SSH_MAX_PACKET_SIZE ->
+ %% far too long message than expected
+ throw(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_PROTOCOL_ERROR,
+ description = "Bad packet length "
+ ++ integer_to_list(PacketLen),
+ language = ""});
+
+ {ok, PacketLen, DecData, Encoded1,
+ #ssh{decrypt_block_size = BlockSize,
+ recv_mac_size = MacSize} = Ssh1} ->
+ %% enough bytes so we got the length and can calculate how many
+ %% more bytes to expect for a full packet
+ Remaining = (PacketLen + ?SSH_LENGHT_INDICATOR_SIZE) - BlockSize + MacSize,
+ handle_packet_part(DecData, Encoded1, Remaining, Ssh1)
+ end;
+handle_packet_part(Decoded0, Encoded0, Remaining, Ssh0)
+ when size(Encoded0) < Remaining ->
+ %% need more bytes to finalize the packet
+ {get_more, Decoded0, Encoded0, Remaining, Ssh0};
+handle_packet_part(Decoded0, Encoded0, Remaining,
+ #ssh{recv_mac_size = MacSize} = Ssh0) ->
+ %% enough bytes to decode the packet.
+ SshLengthNotDecoded = Remaining - MacSize,
+ <<PktT:SshLengthNotDecoded/binary, Mac:MacSize/binary, EncRest0/binary>> = Encoded0,
+ {Ssh1, DecData} = decrypt(Ssh0, PktT),
+ MsgBytes = <<Decoded0/binary,DecData/binary>>,
+ case is_valid_mac(Mac, MsgBytes, Ssh1) of
+ false ->
+ {bad_mac, Ssh1};
+ true ->
+ {Ssh, DecompressedMsgBytes} = decompress(Ssh1, msg_data(MsgBytes)),
+ {decoded, DecompressedMsgBytes, EncRest0, Ssh}
+ end.
+
+
+get_length(Encoded0, #ssh{decrypt_block_size = BlockSize} = Ssh0) ->
+ case size(Encoded0) >= erlang:max(8, BlockSize) of
+ true ->
+ <<EncBlock:BlockSize/binary, EncodedRest/binary>> = Encoded0,
+ {Ssh, Decoded} = decrypt(Ssh0, EncBlock),
+ <<?UINT32(PacketLen), _/binary>> = Decoded,
+ {ok, PacketLen, Decoded, EncodedRest, Ssh};
+ false ->
+ get_more
+ end.
+
+
msg_data(PacketData) ->
<<Len:32, PaddingLen:8, _/binary>> = PacketData,
@@ -1181,6 +1213,8 @@ decrypt_final(Ssh) ->
decrypt_ctx = undefined,
decrypt_block_size = 8}}.
+decrypt(Ssh, <<>>) ->
+ {Ssh, <<>>};
decrypt(#ssh{decrypt = none} = Ssh, Data) ->
{Ssh, Data};
decrypt(#ssh{decrypt = '3des-cbc', decrypt_keys = Keys,