From abb5c21e25343139e47559dbf9a22d099f97154f Mon Sep 17 00:00:00 2001
From: Danil Zagoskin <z@gosk.in>
Date: Sun, 20 Apr 2014 02:25:42 +0400
Subject: ssl: Fix crash on garbage during handshake

If a client sends some garbage in ssl record instead of
valid fragment, server crashes with function_clause while
receiving next record from client.
This patch makes server raise handshake failure instead of
crashing and exposing internal state to user code.
---
 lib/ssl/src/tls_connection.erl   |  6 +++++-
 lib/ssl/test/ssl_basic_SUITE.erl | 46 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 2 deletions(-)

(limited to 'lib')

diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl
index 8142a18c37..930706cde6 100644
--- a/lib/ssl/src/tls_connection.erl
+++ b/lib/ssl/src/tls_connection.erl
@@ -751,7 +751,11 @@ handle_tls_handshake(Handle, StateName,
 	    handle_tls_handshake(Handle, NextStateName, State);
 	{stop, _,_} = Stop ->
 	    Stop
-    end.
+    end;
+
+handle_tls_handshake(_Handle, _StateName, #state{}) ->
+    throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE)).
+
 write_application_data(Data0, From, 
 		       #state{socket = Socket,
 			      negotiated_version = Version,
diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl
index 3d711021f3..406be65c3b 100644
--- a/lib/ssl/test/ssl_basic_SUITE.erl
+++ b/lib/ssl/test/ssl_basic_SUITE.erl
@@ -190,7 +190,8 @@ error_handling_tests()->
      tcp_connect_big,
      close_transport_accept,
      recv_active,
-     recv_active_once
+     recv_active_once,
+     dont_crash_on_handshake_garbage
     ].
 
 rizzo_tests() ->
@@ -2646,6 +2647,49 @@ ciphersuite_vs_version(Config) when is_list(Config) ->
 										
 %%--------------------------------------------------------------------
 
+dont_crash_on_handshake_garbage() ->
+    [{doc, "Ensure SSL server worker thows an alert on garbage during handshake "
+      "instead of crashing and exposing state to user code"}].
+
+dont_crash_on_handshake_garbage(Config) ->
+    ServerOpts = ?config(server_opts, Config),
+
+    {_ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
+
+    Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
+					{from, self()},
+					{mfa, {ssl_test_lib, send_recv_result_active, []}},
+					{options, ServerOpts}]),
+    unlink(Server), monitor(process, Server),
+    Port = ssl_test_lib:inet_port(Server),
+
+    {ok, Socket} = gen_tcp:connect(Hostname, Port, [binary, {active, false}]),
+
+    % Send hello and garbage record
+    ok = gen_tcp:send(Socket,
+                      [<<22, 3,3, 49:16, 1, 45:24, 3,3, % client_hello
+                         16#deadbeef:256, % 32 'random' bytes = 256 bits
+                         0, 6:16, 0,255, 0,61, 0,57, 1, 0 >>, % some hello values
+
+                       <<22, 3,3, 5:16, 92,64,37,228,209>> % garbage
+                      ]),
+    % Send unexpected change_cipher_spec
+    ok = gen_tcp:send(Socket, <<20, 0,0,12, 111,40,244,7,137,224,16,109,197,110,249,152>>),
+
+    % Ensure we receive an alert, not sudden disconnect
+    {ok, <<21, _/binary>>} = drop_handshakes(Socket, 1000).
+
+drop_handshakes(Socket, Timeout) ->
+    {ok, <<RecType:8, _RecMajor:8, _RecMinor:8, RecLen:16>> = Header} = gen_tcp:recv(Socket, 5, Timeout),
+    {ok, <<Frag:RecLen/binary>>} = gen_tcp:recv(Socket, RecLen, Timeout),
+    case RecType of
+        22 -> drop_handshakes(Socket, Timeout);
+        _ -> {ok, <<Header/binary, Frag/binary>>}
+    end.
+
+
+%%--------------------------------------------------------------------
+
 hibernate() ->
     [{doc,"Check that an SSL connection that is started with option "
       "{hibernate_after, 1000} indeed hibernates after 1000ms of "
-- 
cgit v1.2.3