aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBen Murphy <[email protected]>2011-09-26 08:39:18 +0100
committerBen Murphy <[email protected]>2011-09-27 00:50:24 +0100
commit5f7725dc581c7891cb41e725db50076d654511ba (patch)
treeb87b8b3409f7af12432fd5a00dad6e71d9c36794
parent53cf0b70c705e0bf6c09f83f2ce2709d79593ce6 (diff)
downloadotp-5f7725dc581c7891cb41e725db50076d654511ba.tar.gz
otp-5f7725dc581c7891cb41e725db50076d654511ba.tar.bz2
otp-5f7725dc581c7891cb41e725db50076d654511ba.zip
fix unknown ssl extension parsing by changing length from bits to bytes
-rw-r--r--lib/ssl/src/ssl_handshake.erl6
-rw-r--r--lib/ssl/test/Makefile5
-rw-r--r--lib/ssl/test/ssl_handshake_SUITE.erl67
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.
+