ssl: Reenable padding check for TLS-1.0 and provide backwards compatible

    disable option

7 files changed, 47 insertions, 39 deletions

diff --git a/lib/ssl/src/dtls_record.erl b/lib/ssl/src/dtls_record.erl
index c0776e822b..59b3ddec5c 100644
--- a/lib/ssl/src/dtls_record.erl
+++ b/lib/ssl/src/dtls_record.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2013-2014. All Rights Reserved.
+%% Copyright Ericsson AB 2013-2015. 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
@@ -194,7 +194,7 @@ decode_cipher_text(#ssl_tls{type = Type, version = Version,
 						    compression_algorithm=CompAlg}
 					    } = ReadState0}= ConnnectionStates0) ->
     {PlainFragment, Mac, ReadState1} = ssl_record:decipher(dtls_v1:corresponding_tls_version(Version),
-							   CipherFragment, ReadState0),
+							   CipherFragment, ReadState0, true),
     MacHash = calc_mac_hash(ReadState1, Type, Version, Epoch, Seq, PlainFragment),
     case ssl_record:is_correct_mac(Mac, MacHash) of
 	true ->
diff --git a/lib/ssl/src/ssl.erl b/lib/ssl/src/ssl.erl
index b06b1aa7c7..78f3584225 100644
--- a/lib/ssl/src/ssl.erl
+++ b/lib/ssl/src/ssl.erl
@@ -656,7 +656,8 @@ handle_options(Opts0) ->
 		    log_alert = handle_option(log_alert, Opts, true),
 		    server_name_indication = handle_option(server_name_indication, Opts, undefined),
 		    honor_cipher_order = handle_option(honor_cipher_order, Opts, false),
-		    protocol = proplists:get_value(protocol, Opts, tls)
+		    protocol = proplists:get_value(protocol, Opts, tls),
+		    padding_check =  proplists:get_value(padding_check, Opts, true)
 		   },
 
     CbInfo  = proplists:get_value(cb_info, Opts, {gen_tcp, tcp, tcp_closed, tcp_error}),
@@ -669,7 +670,7 @@ handle_options(Opts0) ->
 		  cb_info, renegotiate_at, secure_renegotiate, hibernate_after,
 		  erl_dist, next_protocols_advertised,
 		  client_preferred_next_protocols, log_alert,
-		  server_name_indication, honor_cipher_order],
+		  server_name_indication, honor_cipher_order, padding_check],
 
     SockOpts = lists:foldl(fun(Key, PropList) ->
 				   proplists:delete(Key, PropList)
@@ -847,6 +848,8 @@ validate_option(server_name_indication, undefined) ->
     undefined;
 validate_option(honor_cipher_order, Value) when is_boolean(Value) ->
     Value;
+validate_option(padding_check, Value) when is_boolean(Value) ->
+    Value;
 validate_option(Opt, Value) ->
     throw({error, {options, {Opt, Value}}}).
 
diff --git a/lib/ssl/src/ssl_cipher.erl b/lib/ssl/src/ssl_cipher.erl
index e1d89c149e..939778dd40 100644
--- a/lib/ssl/src/ssl_cipher.erl
+++ b/lib/ssl/src/ssl_cipher.erl
@@ -33,7 +33,7 @@
 -include_lib("public_key/include/public_key.hrl").
 
 -export([security_parameters/2, security_parameters/3, suite_definition/1,
-	 cipher_init/3, decipher/5, cipher/5, decipher_aead/6, cipher_aead/6,
+	 cipher_init/3, decipher/6, cipher/5, decipher_aead/6, cipher_aead/6,
 	 suite/1, suites/1, all_suites/1, 
 	 ec_keyed_suites/0, anonymous_suites/1, psk_suites/1, srp_suites/0,
 	 openssl_suite/1, openssl_suite_name/1, filter/2, filter_suites/1,
@@ -182,7 +182,8 @@ block_cipher(Fun, BlockSz, #cipher_state{key=Key, iv=IV} = CS0,
     {T, CS0#cipher_state{iv=NextIV}}.
 
 %%--------------------------------------------------------------------
--spec decipher(cipher_enum(), integer(), #cipher_state{}, binary(), ssl_record:ssl_version()) ->
+-spec decipher(cipher_enum(), integer(), #cipher_state{}, binary(), 
+	       ssl_record:ssl_version(), boolean()) ->
 		      {binary(), binary(), #cipher_state{}} | #alert{}.
 %%
 %% Description: Decrypts the data and the MAC using cipher described
@@ -190,9 +191,9 @@ block_cipher(Fun, BlockSz, #cipher_state{key=Key, iv=IV} = CS0,
 %% Used for "MAC then Cipher" suites where first the data is decrypted
 %% and the an HMAC of the decrypted data is checked
 %%-------------------------------------------------------------------
-decipher(?NULL, _HashSz, CipherState, Fragment, _) ->
+decipher(?NULL, _HashSz, CipherState, Fragment, _, _) ->
     {Fragment, <<>>, CipherState};
-decipher(?RC4, HashSz, CipherState = #cipher_state{state = State0}, Fragment, _) ->
+decipher(?RC4, HashSz, CipherState = #cipher_state{state = State0}, Fragment, _, _) ->
     try crypto:stream_decrypt(State0, Fragment) of
 	{State, Text} ->
 	    GSC = generic_stream_cipher_from_bin(Text, HashSz),
@@ -208,20 +209,20 @@ decipher(?RC4, HashSz, CipherState = #cipher_state{state = State0}, Fragment, _)
 	    ?ALERT_REC(?FATAL, ?BAD_RECORD_MAC)
     end;
 
-decipher(?DES, HashSz, CipherState, Fragment, Version) ->
+decipher(?DES, HashSz, CipherState, Fragment, Version, PaddingCheck) ->
     block_decipher(fun(Key, IV, T) ->
 			   crypto:block_decrypt(des_cbc, Key, IV, T)
-		   end, CipherState, HashSz, Fragment, Version);
-decipher(?'3DES', HashSz, CipherState, Fragment, Version) ->
+		   end, CipherState, HashSz, Fragment, Version, PaddingCheck);
+decipher(?'3DES', HashSz, CipherState, Fragment, Version, PaddingCheck) ->
     block_decipher(fun(<<K1:8/binary, K2:8/binary, K3:8/binary>>, IV, T) ->
 			   crypto:block_decrypt(des3_cbc, [K1, K2, K3], IV, T)
-		   end, CipherState, HashSz, Fragment, Version);
-decipher(?AES_CBC, HashSz, CipherState, Fragment, Version) ->
+		   end, CipherState, HashSz, Fragment, Version, PaddingCheck);
+decipher(?AES_CBC, HashSz, CipherState, Fragment, Version, PaddingCheck) ->
     block_decipher(fun(Key, IV, T) when byte_size(Key) =:= 16 ->
 			   crypto:block_decrypt(aes_cbc128, Key, IV, T);
 		      (Key, IV, T) when byte_size(Key) =:= 32 ->
 			   crypto:block_decrypt(aes_cbc256, Key, IV, T)
-		   end, CipherState, HashSz, Fragment, Version).
+		   end, CipherState, HashSz, Fragment, Version, PaddingCheck).
 
 %%--------------------------------------------------------------------
 -spec decipher_aead(cipher_enum(),  #cipher_state{}, integer(), binary(), binary(), ssl_record:ssl_version()) ->
@@ -237,7 +238,7 @@ decipher_aead(?CHACHA20_POLY1305, CipherState, SeqNo, AAD, Fragment, Version) ->
     aead_decipher(chacha20_poly1305, CipherState, SeqNo, AAD, Fragment, Version).
 
 block_decipher(Fun, #cipher_state{key=Key, iv=IV} = CipherState0, 
-	       HashSz, Fragment, Version) ->
+	       HashSz, Fragment, Version, PaddingCheck) ->
     try 
 	Text = Fun(Key, IV, Fragment),
 	NextIV = next_iv(Fragment, IV),
@@ -245,7 +246,7 @@ block_decipher(Fun, #cipher_state{key=Key, iv=IV} = CipherState0,
 	Content = GBC#generic_block_cipher.content,
 	Mac = GBC#generic_block_cipher.mac,
 	CipherState1 = CipherState0#cipher_state{iv=GBC#generic_block_cipher.next_iv},
-	case is_correct_padding(GBC, Version) of
+	case is_correct_padding(GBC, Version, PaddingCheck) of
 	    true ->
 		{Content, Mac, CipherState1};
 	    false ->
@@ -1632,16 +1633,18 @@ generic_stream_cipher_from_bin(T, HashSz) ->
     #generic_stream_cipher{content=Content,
 			   mac=Mac}.
 
-%% For interoperability reasons we do not check the padding content in
-%% SSL 3.0 and TLS 1.0 as it is not strictly required and breaks
-%% interopability with for instance Google. 
 is_correct_padding(#generic_block_cipher{padding_length = Len,
-					 padding = Padding}, {3, N})
-  when N == 0; N == 1 ->
-    Len == byte_size(Padding); 
-%% Padding must be check in TLS 1.1 and after  
+					 padding = Padding}, {3, 0}, _) ->
+    Len == byte_size(Padding); %% Only length check is done in SSL 3.0 spec
+%% For interoperability reasons it is possible to disable
+%% the padding check when using TLS 1.0, as it is not strictly required 
+%% in the spec (only recommended), howerver this makes TLS 1.0 vunrable to the Poodle attack 
+%% so by default this clause will not match
+is_correct_padding(GenBlockCipher, {3, 1}, false) ->
+    is_correct_padding(GenBlockCipher, {3, 0}, false);
+%% Padding must be checked in TLS 1.1 and after  
 is_correct_padding(#generic_block_cipher{padding_length = Len,
-					 padding = Padding}, _) ->
+					 padding = Padding}, _, _) ->
     Len == byte_size(Padding) andalso
 		list_to_binary(lists:duplicate(Len, Len)) == Padding.
 
diff --git a/lib/ssl/src/ssl_internal.hrl b/lib/ssl/src/ssl_internal.hrl
index 4c2fdf74f1..3cf6020169 100644
--- a/lib/ssl/src/ssl_internal.hrl
+++ b/lib/ssl/src/ssl_internal.hrl
@@ -120,7 +120,8 @@
 	  server_name_indication = undefined,
 	  %% Should the server prefer its own cipher order over the one provided by
 	  %% the client?
-	  honor_cipher_order = false
+	  honor_cipher_order = false,
+	  padding_check = true
 	  }).
 
 -record(socket_options,
diff --git a/lib/ssl/src/ssl_record.erl b/lib/ssl/src/ssl_record.erl
index 63fc57edad..a02375a947 100644
--- a/lib/ssl/src/ssl_record.erl
+++ b/lib/ssl/src/ssl_record.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2013-2014. All Rights Reserved.
+%% Copyright Ericsson AB 2013-2015. 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
@@ -48,7 +48,7 @@
 -export([compress/3, uncompress/3, compressions/0]).
 
 %% Payload encryption/decryption
--export([cipher/4, decipher/3, is_correct_mac/2,
+-export([cipher/4, decipher/4, is_correct_mac/2,
 	 cipher_aead/4, decipher_aead/4]).
 
 -export_type([ssl_version/0, ssl_atom_version/0]).
@@ -396,7 +396,7 @@ cipher_aead(Version, Fragment,
     {CipherFragment,  WriteState0#connection_state{cipher_state = CipherS1}}.
 
 %%--------------------------------------------------------------------
--spec decipher(ssl_version(), binary(), #connection_state{}) -> {binary(), binary(), #connection_state{}} | #alert{}.
+-spec decipher(ssl_version(), binary(), #connection_state{}, boolean()) -> {binary(), binary(), #connection_state{}} | #alert{}.
 %%
 %% Description: Payload decryption
 %%--------------------------------------------------------------------
@@ -406,8 +406,8 @@ decipher(Version, CipherFragment,
 							BulkCipherAlgo,
 						    hash_size = HashSz},
 			   cipher_state = CipherS0
-			  } = ReadState) ->
-    case ssl_cipher:decipher(BulkCipherAlgo, HashSz, CipherS0, CipherFragment, Version) of
+			  } = ReadState, PaddingCheck) ->
+    case ssl_cipher:decipher(BulkCipherAlgo, HashSz, CipherS0, CipherFragment, Version, PaddingCheck) of
 	{PlainFragment, Mac, CipherS1} ->
 	    CS1 = ReadState#connection_state{cipher_state = CipherS1},
 	    {PlainFragment, Mac, CS1};
diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl
index 7df73fb581..77d3aa7889 100644
--- a/lib/ssl/src/tls_connection.erl
+++ b/lib/ssl/src/tls_connection.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2007-2014. All Rights Reserved.
+%% Copyright Ericsson AB 2007-2015. 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
@@ -482,8 +482,9 @@ next_record(#state{protocol_buffers = #protocol_buffers{tls_packets = [], tls_ci
 next_record(#state{protocol_buffers =
 		       #protocol_buffers{tls_packets = [], tls_cipher_texts = [CT | Rest]}
 		   = Buffers,
-		   connection_states = ConnStates0} = State) ->
-    case tls_record:decode_cipher_text(CT, ConnStates0) of
+		   connection_states = ConnStates0,
+		   ssl_options = #ssl_options{padding_check = Check}} = State) ->
+    case tls_record:decode_cipher_text(CT, ConnStates0, Check) of
 	{Plain, ConnStates} ->		      
 	    {Plain, State#state{protocol_buffers =
 				    Buffers#protocol_buffers{tls_cipher_texts = Rest},
diff --git a/lib/ssl/src/tls_record.erl b/lib/ssl/src/tls_record.erl
index 37929c928d..3d5c5c0da3 100644
--- a/lib/ssl/src/tls_record.erl
+++ b/lib/ssl/src/tls_record.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2007-2014. All Rights Reserved.
+%% Copyright Ericsson AB 2007-2015. 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
@@ -34,7 +34,7 @@
 -export([get_tls_records/2]).
 
 %% Decoding
--export([decode_cipher_text/2]).
+-export([decode_cipher_text/3]).
 
 %% Encoding
 -export([encode_plain_text/4]).
@@ -159,7 +159,7 @@ encode_plain_text(Type, Version, Data,
     {CipherText, ConnectionStates#connection_states{current_write = WriteState#connection_state{sequence_number = Seq +1}}}.
 
 %%--------------------------------------------------------------------
--spec decode_cipher_text(#ssl_tls{}, #connection_states{}) ->
+-spec decode_cipher_text(#ssl_tls{}, #connection_states{}, boolean()) ->
 				{#ssl_tls{}, #connection_states{}}| #alert{}.
 %%
 %% Description: Decode cipher text
@@ -174,7 +174,7 @@ decode_cipher_text(#ssl_tls{type = Type, version = Version,
 						 #security_parameters{
 						    cipher_type = ?AEAD,
 						    compression_algorithm=CompAlg}
-					    } = ReadState0} = ConnnectionStates0) ->
+					    } = ReadState0} = ConnnectionStates0, _) ->
     AAD = calc_aad(Type, Version, ReadState0),
     case ssl_record:decipher_aead(Version, CipherFragment, ReadState0, AAD) of
 	{PlainFragment, ReadState1} ->
@@ -197,8 +197,8 @@ decode_cipher_text(#ssl_tls{type = Type, version = Version,
 					     sequence_number = Seq,
 					     security_parameters=
 						 #security_parameters{compression_algorithm=CompAlg}
-					    } = ReadState0} = ConnnectionStates0) ->
-    case ssl_record:decipher(Version, CipherFragment, ReadState0) of
+					    } = ReadState0} = ConnnectionStates0, PaddingCheck) ->
+    case ssl_record:decipher(Version, CipherFragment, ReadState0, PaddingCheck) of
 	{PlainFragment, Mac, ReadState1} ->
 	    MacHash = calc_mac_hash(Type, Version, PlainFragment, ReadState1),
 	    case ssl_record:is_correct_mac(Mac, MacHash) of