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.

2 files changed, 53 insertions, 23 deletions

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),