fix unknown ssl extension parsing by changing length from bits to bytes

3 files changed, 75 insertions, 3 deletions

diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl
index 453ea20f99..0ac1ba9d66 100644
--- a/lib/ssl/src/ssl_handshake.erl
+++ b/lib/ssl/src/ssl_handshake.erl
@@ -39,6 +39,8 @@
 	 encode_handshake/2, init_hashes/0, update_hashes/2,
 	 decrypt_premaster_secret/2]).
 
+-export([dec_hello_extensions/2]).
+
 -type tls_handshake() :: #client_hello{} | #server_hello{} |
 			 #server_hello_done{} | #certificate{} | #certificate_request{} |
 			 #client_key_exchange{} | #finished{} | #certificate_verify{} |
@@ -912,9 +914,11 @@ dec_hello_extensions(<<?UINT16(?RENEGOTIATION_EXT), ?UINT16(Len), Info:Len/binar
 		      end,	    
     dec_hello_extensions(Rest, [{renegotiation_info, 
 			   #renegotiation_info{renegotiated_connection = RenegotiateInfo}} | Acc]);
-dec_hello_extensions(<<?UINT16(_), ?UINT16(Len), _Unknown:Len, Rest/binary>>, Acc) ->
+dec_hello_extensions(<<?UINT16(_), ?UINT16(Len), _Unknown:Len/binary, Rest/binary>>, Acc) ->
     dec_hello_extensions(Rest, Acc);
 %% Need this clause?
+%% I don't think we need this clause anymore. It was previously catching parsing errors caused by the missing /binary.
+%% Maybe we should be logging an error somewhere because we really should not be entering this clause.
 dec_hello_extensions(_, Acc) ->
     Acc.
 
diff --git a/lib/ssl/test/Makefile b/lib/ssl/test/Makefile
index 5be07cad2c..922abea41b 100644
--- a/lib/ssl/test/Makefile
+++ b/lib/ssl/test/Makefile
@@ -35,8 +35,9 @@ VSN=$(GS_VSN)
 # ----------------------------------------------------
 
 MODULES = \
-        ssl_test_lib \
+	ssl_test_lib \
 	ssl_basic_SUITE \
+	ssl_handshake_SUITE \
 	ssl_packet_SUITE \
 	ssl_payload_SUITE \
 	ssl_to_openssl_SUITE \
@@ -45,7 +46,7 @@ MODULES = \
 	old_ssl_active_SUITE \
 	old_ssl_active_once_SUITE \
 	old_ssl_passive_SUITE \
-        old_ssl_verify_SUITE \
+	old_ssl_verify_SUITE \
 	old_ssl_peer_cert_SUITE \
 	old_ssl_misc_SUITE \
 	old_ssl_protocol_SUITE \
diff --git a/lib/ssl/test/ssl_handshake_SUITE.erl b/lib/ssl/test/ssl_handshake_SUITE.erl
new file mode 100644
index 0000000000..08c23b2d47
--- /dev/null
+++ b/lib/ssl/test/ssl_handshake_SUITE.erl
@@ -0,0 +1,67 @@
+%%
+%% %CopyrightBegin%
+%%
+%% Copyright Ericsson AB 2008-2011. All Rights Reserved.
+%%
+%% The contents of this file are subject to the Erlang Public License,
+%% Version 1.1, (the "License"); you may not use this file except in
+%% compliance with the License. You should have received a copy of the
+%% Erlang Public License along with this software. If not, it can be
+%% retrieved online at http://www.erlang.org/.
+%%
+%% Software distributed under the License is distributed on an "AS IS"
+%% basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See
+%% the License for the specific language governing rights and limitations
+%% under the License.
+%%
+%% %CopyrightEnd%
+%%
+
+%%
+
+-module(ssl_handshake_SUITE).
+
+-compile(export_all).
+
+-include_lib("common_test/include/ct.hrl").
+-include("ssl_internal.hrl").
+-include("ssl_handshake.hrl").
+
+suite() -> [{ct_hooks,[ts_install_cth]}].
+
+all() -> [
+	decode_hello_handshake,
+	decode_single_hello_extension_correctly,
+	decode_unknown_hello_extension_correctly].
+
+decode_hello_handshake(_Config) ->
+	HelloPacket = <<16#02, 16#00, 16#00,
+	16#44, 16#03, 16#03, 16#4e, 16#7f, 16#c1, 16#03, 16#35,
+	16#c2, 16#07, 16#b9, 16#4a, 16#58, 16#af, 16#34, 16#07,
+	16#a6, 16#7e, 16#ef, 16#52, 16#cb, 16#e0, 16#ea, 16#b7,
+	16#aa, 16#47, 16#c8, 16#c2, 16#2c, 16#66, 16#fa, 16#f8,
+	16#09, 16#42, 16#cf, 16#00, 16#c0, 16#30, 16#00, 16#00,
+	16#1c, 
+	16#00, 16#0b, 16#00, 16#04, 16#03, 16#00, 16#01, 16#02, % ec_point_formats
+	16#ff, 16#01, 16#00, 16#01, 16#00, %% renegotiate 
+	16#00, 16#23,
+	16#00, 16#00, 16#33, 16#74, 16#00, 16#07, 16#06, 16#73,
+	16#70, 16#64, 16#79, 16#2f, 16#32>>,
+	
+	{Records, _Buffer} = ssl_handshake:get_tls_handshake(HelloPacket, <<>>),
+	
+	{Hello, _Data} = hd(Records),
+	#renegotiation_info{renegotiated_connection = <<0>>} = Hello#server_hello.renegotiation_info.
+	
+decode_single_hello_extension_correctly(_Config) -> 
+	Renegotiation = <<?UINT16(?RENEGOTIATION_EXT), ?UINT16(1), 0>>,
+	Extensions = ssl_handshake:dec_hello_extensions(Renegotiation, []),
+	[{renegotiation_info,#renegotiation_info{renegotiated_connection = <<0>>}}] = Extensions.
+	
+
+decode_unknown_hello_extension_correctly(_Config) ->
+	FourByteUnknown = <<16#CA,16#FE, ?UINT16(4), 3, 0, 1, 2>>,
+	Renegotiation = <<?UINT16(?RENEGOTIATION_EXT), ?UINT16(1), 0>>,
+	Extensions = ssl_handshake:dec_hello_extensions(<<FourByteUnknown/binary, Renegotiation/binary>>, []),
+	[{renegotiation_info,#renegotiation_info{renegotiated_connection = <<0>>}}] = Extensions.
+