From db845bde0a37d4b28e5112270b4de502a54925c8 Mon Sep 17 00:00:00 2001
From: Ingela Anderton Andin <ingela@erlang.org>
Date: Wed, 1 Feb 2017 11:04:43 +0100
Subject: ssl: Avoid SSL/TLS hello format confusion

Valid SSL 3.0 or TLS hellos might accidentally match SSL 2.0 format
(and sometimes the other way around before inspecting data)
so we need to match SSL 3.0 and TLS first and only match SSL 2.0
hellos when flag to support it is set.
---
 lib/ssl/src/tls_handshake.erl        | 67 +++++++++++++++++++++++-------------
 lib/ssl/test/ssl_handshake_SUITE.erl |  9 +++++
 2 files changed, 53 insertions(+), 23 deletions(-)

(limited to 'lib/ssl')

diff --git a/lib/ssl/src/tls_handshake.erl b/lib/ssl/src/tls_handshake.erl
index 2800ee6537..f4803ce19f 100644
--- a/lib/ssl/src/tls_handshake.erl
+++ b/lib/ssl/src/tls_handshake.erl
@@ -192,7 +192,8 @@ handle_client_hello(Version, #client_hello{session_id = SugesstedId,
     end.
 
 get_tls_handshake_aux(Version, <<?BYTE(Type), ?UINT24(Length),
-				 Body:Length/binary,Rest/binary>>, #ssl_options{v2_hello_compatible = V2Hello} = Opts,  Acc) ->
+				 Body:Length/binary,Rest/binary>>, 
+                      #ssl_options{v2_hello_compatible = V2Hello} = Opts,  Acc) ->
     Raw = <<?BYTE(Type), ?UINT24(Length), Body/binary>>,
     try decode_handshake(Version, Type, Body, V2Hello) of
 	Handshake ->
@@ -207,27 +208,17 @@ get_tls_handshake_aux(_Version, Data, _, Acc) ->
 decode_handshake(_, ?HELLO_REQUEST, <<>>, _) ->
     #hello_request{};
 
-%% Client hello v2.
-%% The server must be able to receive such messages, from clients that
-%% are willing to use ssl v3 or higher, but have ssl v2 compatibility.
-decode_handshake(_Version, ?CLIENT_HELLO, <<?BYTE(Major), ?BYTE(Minor),
-					    ?UINT16(CSLength), ?UINT16(0),
-					    ?UINT16(CDLength),
-					    CipherSuites:CSLength/binary,
-					    ChallengeData:CDLength/binary>>, true) ->
-    #client_hello{client_version = {Major, Minor},
-		  random = ssl_v2:client_random(ChallengeData, CDLength),
-		  session_id = 0,
-		  cipher_suites =  ssl_handshake:decode_suites('3_bytes', CipherSuites),
-		  compression_methods = [?NULL],
-		  extensions = #hello_extensions{}
-		 };
-decode_handshake(_Version, ?CLIENT_HELLO, <<?BYTE(_), ?BYTE(_),
-					    ?UINT16(CSLength), ?UINT16(0),
-					    ?UINT16(CDLength),
-					    _CipherSuites:CSLength/binary,
-					    _ChallengeData:CDLength/binary>>, false) ->
-    throw(?ALERT_REC(?FATAL, ?PROTOCOL_VERSION, ssl_v2_client_hello_no_supported));
+decode_handshake(_Version, ?CLIENT_HELLO, Bin, true) ->
+    try decode_hello(Bin) of
+        Hello ->
+            Hello
+    catch
+        _:_ ->
+            decode_v2_hello(Bin)
+    end;
+decode_handshake(_Version, ?CLIENT_HELLO, Bin, false) ->
+    decode_hello(Bin);
+
 decode_handshake(_Version, ?CLIENT_HELLO, <<?BYTE(Major), ?BYTE(Minor), Random:32/binary,
 					    ?BYTE(SID_length), Session_ID:SID_length/binary,
 					    ?UINT16(Cs_length), CipherSuites:Cs_length/binary,
@@ -244,10 +235,40 @@ decode_handshake(_Version, ?CLIENT_HELLO, <<?BYTE(Major), ?BYTE(Minor), Random:3
        compression_methods = Comp_methods,
        extensions = DecodedExtensions
       };
-
 decode_handshake(Version, Tag, Msg, _) ->
     ssl_handshake:decode_handshake(Version, Tag, Msg).
 
+
+decode_hello(<<?BYTE(Major), ?BYTE(Minor), Random:32/binary,
+               ?BYTE(SID_length), Session_ID:SID_length/binary,
+               ?UINT16(Cs_length), CipherSuites:Cs_length/binary,
+               ?BYTE(Cm_length), Comp_methods:Cm_length/binary,
+               Extensions/binary>>) ->
+    DecodedExtensions = ssl_handshake:decode_hello_extensions({client, Extensions}),
+    
+    #client_hello{
+       client_version = {Major,Minor},
+       random = Random,
+       session_id = Session_ID,
+       cipher_suites = ssl_handshake:decode_suites('2_bytes', CipherSuites),
+       compression_methods = Comp_methods,
+       extensions = DecodedExtensions
+      }.
+%% The server must be able to receive such messages, from clients that
+%% are willing to use ssl v3 or higher, but have ssl v2 compatibility.
+decode_v2_hello(<<?BYTE(Major), ?BYTE(Minor),
+                  ?UINT16(CSLength), ?UINT16(0),
+                  ?UINT16(CDLength),
+                  CipherSuites:CSLength/binary,
+                  ChallengeData:CDLength/binary>>) ->
+    #client_hello{client_version = {Major, Minor},
+		  random = ssl_v2:client_random(ChallengeData, CDLength),
+		  session_id = 0,
+		  cipher_suites =  ssl_handshake:decode_suites('3_bytes', CipherSuites),
+		  compression_methods = [?NULL],
+		  extensions = #hello_extensions{}
+		 }.
+
 enc_handshake(#hello_request{}, _Version) ->
     {?HELLO_REQUEST, <<>>};
 enc_handshake(#client_hello{client_version = {Major, Minor},
diff --git a/lib/ssl/test/ssl_handshake_SUITE.erl b/lib/ssl/test/ssl_handshake_SUITE.erl
index 74b14145dd..0a50c98a28 100644
--- a/lib/ssl/test/ssl_handshake_SUITE.erl
+++ b/lib/ssl/test/ssl_handshake_SUITE.erl
@@ -33,6 +33,7 @@
 %% Common Test interface functions -----------------------------------
 %%--------------------------------------------------------------------
 all() -> [decode_hello_handshake,
+          decode_hello_handshake_version_confusion,
 	  decode_single_hello_extension_correctly,
 	  decode_supported_elliptic_curves_hello_extension_correctly,
 	  decode_unknown_hello_extension_correctly,
@@ -106,6 +107,14 @@ decode_hello_handshake(_Config) ->
     #renegotiation_info{renegotiated_connection = <<0>>}
 	= (Hello#server_hello.extensions)#hello_extensions.renegotiation_info.
 
+
+decode_hello_handshake_version_confusion(_) -> 
+    HelloPacket = <<3,3,0,0,0,0,0,63,210,235,149,6,244,140,108,13,177,74,16,218,33,108,219,41,73,228,3,82,132,123,73,144,118,100,0,0,32,192,4,0,10,192,45,192,38,0,47,192,18,0,163,0,22,0,165,192,29,192,18,192,30,0,103,0,57,192,48,0,47,1,0>>,
+    Version = {3,3},
+    ClientHello = 1, 
+    Hello = tls_handshake:decode_handshake({3,3}, ClientHello, HelloPacket, false),
+    Hello = tls_handshake:decode_handshake({3,3}, ClientHello, HelloPacket, true).
+
 decode_single_hello_extension_correctly(_Config) -> 
     Renegotiation = <<?UINT16(?RENEGOTIATION_EXT), ?UINT16(1), 0>>,
     Extensions = ssl_handshake:decode_hello_extensions(Renegotiation),
-- 
cgit v1.2.3