From df0c5663dd944a3dd06936105d0696a704c20e4e Mon Sep 17 00:00:00 2001 From: Kenneth Lakin Date: Sat, 30 Apr 2016 02:31:51 -0700 Subject: ssl: Add BEAST mitigation selection option Some legacy TLS 1.0 software does not tolerate the 1/n-1 content split BEAST mitigation technique. This commit adds a beast_mitigation SSL option (defaulting to one_n_minus_one) to select or disable the BEAST mitigation technique. Valid option values are (one_n_minus_one | zero_n | disabled). --- lib/ssl/doc/src/ssl.xml | 18 +++++++++-- lib/ssl/src/dtls_connection.erl | 3 +- lib/ssl/src/ssl.erl | 7 ++++- lib/ssl/src/ssl_internal.hrl | 3 ++ lib/ssl/src/ssl_record.erl | 43 +++++++++++++++++---------- lib/ssl/src/ssl_record.hrl | 4 ++- lib/ssl/src/tls_connection.erl | 3 +- lib/ssl/test/ssl_basic_SUITE.erl | 64 +++++++++++++++++++++++++++++++++++++++- 8 files changed, 123 insertions(+), 22 deletions(-) (limited to 'lib') diff --git a/lib/ssl/doc/src/ssl.xml b/lib/ssl/doc/src/ssl.xml index 154664d855..33ece8f769 100644 --- a/lib/ssl/doc/src/ssl.xml +++ b/lib/ssl/doc/src/ssl.xml @@ -417,10 +417,24 @@ fun(srp, Username :: string(), UserState :: term()) -> If set to false, it disables the block cipher padding check to be able to interoperate with legacy software.

- -

Using {padding_check, boolean()} makes TLS vulnerable to the Poodle attack.

 + + {beast_mitigation, one_n_minus_one | zero_n | disabled} +

Affects SSL-3.0 and TLS-1.0 connections only. Used to change the BEAST + mitigation strategy to interoperate with legacy software. + Defaults to one_n_minus_one

. + +

one_n_minus_one - Perform 1/n-1 BEAST mitigation.

+ +

zero_n - Perform 0/n BEAST mitigation.

+ +

disabled - Disable BEAST mitigation.

 + +

Using {beast_mitigation, disabled} makes SSL or TLS + vulnerable to the BEAST attack.

 + +
diff --git a/lib/ssl/src/dtls_connection.erl b/lib/ssl/src/dtls_connection.erl index 82d6faee42..60a61bc901 100644 --- a/lib/ssl/src/dtls_connection.erl +++ b/lib/ssl/src/dtls_connection.erl @@ -416,7 +416,8 @@ encode_change_cipher(#change_cipher_spec{}, Version, ConnectionStates) -> initial_state(Role, Host, Port, Socket, {SSLOptions, SocketOptions}, User, {CbModule, DataTag, CloseTag, ErrorTag}) -> - ConnectionStates = ssl_record:init_connection_states(Role), + #ssl_options{beast_mitigation = BeastMitigation} = SSLOptions, + ConnectionStates = ssl_record:init_connection_states(Role, BeastMitigation), SessionCacheCb = case application:get_env(ssl, session_cb) of {ok, Cb} when is_atom(Cb) -> diff --git a/lib/ssl/src/ssl.erl b/lib/ssl/src/ssl.erl index 51732b4a59..5dc2e583a5 100644 --- a/lib/ssl/src/ssl.erl +++ b/lib/ssl/src/ssl.erl @@ -725,6 +725,7 @@ handle_options(Opts0, Role) -> server, Role), protocol = proplists:get_value(protocol, Opts, tls), padding_check = proplists:get_value(padding_check, Opts, true), + beast_mitigation = handle_option(beast_mitigation, Opts, one_n_minus_one), fallback = handle_option(fallback, Opts, proplists:get_value(fallback, Opts, default_option_role(client, @@ -746,7 +747,7 @@ handle_options(Opts0, Role) -> alpn_preferred_protocols, next_protocols_advertised, client_preferred_next_protocols, log_alert, server_name_indication, honor_cipher_order, padding_check, crl_check, crl_cache, - fallback, signature_algs], + fallback, signature_algs, beast_mitigation], SockOpts = lists:foldl(fun(Key, PropList) -> proplists:delete(Key, PropList) @@ -986,6 +987,10 @@ validate_option(crl_check, Value) when (Value == best_effort) or (Value == peer) Value; validate_option(crl_cache, {Cb, {_Handle, Options}} = Value) when is_atom(Cb) and is_list(Options) -> Value; +validate_option(beast_mitigation, Value) when Value == one_n_minus_one orelse + Value == zero_n orelse + Value == disabled -> + Value; validate_option(Opt, Value) -> throw({error, {options, {Opt, Value}}}). diff --git a/lib/ssl/src/ssl_internal.hrl b/lib/ssl/src/ssl_internal.hrl index 076e663cd4..dddcbdeeda 100644 --- a/lib/ssl/src/ssl_internal.hrl +++ b/lib/ssl/src/ssl_internal.hrl @@ -133,6 +133,9 @@ %% the client? honor_cipher_order = false :: boolean(), padding_check = true :: boolean(), + %%Should we use 1/n-1 or 0/n splitting to mitigate BEAST, or disable + %%mitigation entirely? + beast_mitigation = one_n_minus_one :: one_n_minus_one | zero_n | disabled, fallback = false :: boolean(), crl_check :: boolean() | peer | best_effort, crl_cache, diff --git a/lib/ssl/src/ssl_record.erl b/lib/ssl/src/ssl_record.erl index 866bfcef7e..0a086f5eeb 100644 --- a/lib/ssl/src/ssl_record.erl +++ b/lib/ssl/src/ssl_record.erl @@ -30,7 +30,7 @@ -include("ssl_alert.hrl"). %% Connection state handling --export([init_connection_states/1, +-export([init_connection_states/2, current_connection_state/2, pending_connection_state/2, activate_pending_connection_state/2, set_security_params/3, @@ -62,15 +62,16 @@ %%==================================================================== %%-------------------------------------------------------------------- --spec init_connection_states(client | server) -> #connection_states{}. +-spec init_connection_states(client | server, one_n_minus_one | zero_n | disabled ) -> + #connection_states{}. %% %% Description: Creates a connection_states record with appropriate %% values for the initial SSL connection setup. %%-------------------------------------------------------------------- -init_connection_states(Role) -> +init_connection_states(Role, BeastMitigation) -> ConnectionEnd = record_protocol_role(Role), - Current = initial_connection_state(ConnectionEnd), - Pending = empty_connection_state(ConnectionEnd), + Current = initial_connection_state(ConnectionEnd, BeastMitigation), + Pending = empty_connection_state(ConnectionEnd, BeastMitigation), #connection_states{current_read = Current, pending_read = Pending, current_write = Current, @@ -119,9 +120,10 @@ activate_pending_connection_state(States = read) -> NewCurrent = Pending#connection_state{epoch = dtls_next_epoch(Current), sequence_number = 0}, + BeastMitigation = Pending#connection_state.beast_mitigation, SecParams = Pending#connection_state.security_parameters, ConnectionEnd = SecParams#security_parameters.connection_end, - EmptyPending = empty_connection_state(ConnectionEnd), + EmptyPending = empty_connection_state(ConnectionEnd, BeastMitigation), SecureRenegotation = NewCurrent#connection_state.secure_renegotiation, NewPending = EmptyPending#connection_state{secure_renegotiation = SecureRenegotation}, States#connection_states{current_read = NewCurrent, @@ -134,9 +136,10 @@ activate_pending_connection_state(States = write) -> NewCurrent = Pending#connection_state{epoch = dtls_next_epoch(Current), sequence_number = 0}, + BeastMitigation = Pending#connection_state.beast_mitigation, SecParams = Pending#connection_state.security_parameters, ConnectionEnd = SecParams#security_parameters.connection_end, - EmptyPending = empty_connection_state(ConnectionEnd), + EmptyPending = empty_connection_state(ConnectionEnd, BeastMitigation), SecureRenegotation = NewCurrent#connection_state.secure_renegotiation, NewPending = EmptyPending#connection_state{secure_renegotiation = SecureRenegotation}, States#connection_states{current_write = NewCurrent, @@ -314,12 +317,13 @@ set_pending_cipher_state(#connection_states{pending_read = Read, encode_handshake(Frag, Version, #connection_states{current_write = #connection_state{ + beast_mitigation = BeastMitigation, security_parameters = #security_parameters{bulk_cipher_algorithm = BCA}}} = ConnectionStates) -> case iolist_size(Frag) of N when N > ?MAX_PLAIN_TEXT_LENGTH -> - Data = split_bin(iolist_to_binary(Frag), ?MAX_PLAIN_TEXT_LENGTH, Version, BCA), + Data = split_bin(iolist_to_binary(Frag), ?MAX_PLAIN_TEXT_LENGTH, Version, BCA, BeastMitigation), encode_iolist(?HANDSHAKE, Data, Version, ConnectionStates); _ -> encode_plain_text(?HANDSHAKE, Version, Frag, ConnectionStates) @@ -352,10 +356,11 @@ encode_change_cipher_spec(Version, ConnectionStates) -> %%-------------------------------------------------------------------- encode_data(Frag, Version, #connection_states{current_write = #connection_state{ + beast_mitigation = BeastMitigation, security_parameters = #security_parameters{bulk_cipher_algorithm = BCA}}} = ConnectionStates) -> - Data = split_bin(Frag, ?MAX_PLAIN_TEXT_LENGTH, Version, BCA), + Data = split_bin(Frag, ?MAX_PLAIN_TEXT_LENGTH, Version, BCA, BeastMitigation), encode_iolist(?APPLICATION_DATA, Data, Version, ConnectionStates). uncompress(?NULL, Data, CS) -> @@ -447,9 +452,10 @@ decipher_aead(Version, CipherFragment, %%-------------------------------------------------------------------- %%% Internal functions %%-------------------------------------------------------------------- -empty_connection_state(ConnectionEnd) -> +empty_connection_state(ConnectionEnd, BeastMitigation) -> SecParams = empty_security_params(ConnectionEnd), - #connection_state{security_parameters = SecParams}. + #connection_state{security_parameters = SecParams, + beast_mitigation = BeastMitigation}. empty_security_params(ConnectionEnd = ?CLIENT) -> #security_parameters{connection_end = ConnectionEnd, @@ -478,10 +484,11 @@ record_protocol_role(client) -> record_protocol_role(server) -> ?SERVER. -initial_connection_state(ConnectionEnd) -> +initial_connection_state(ConnectionEnd, BeastMitigation) -> #connection_state{security_parameters = initial_security_params(ConnectionEnd), - sequence_number = 0 + sequence_number = 0, + beast_mitigation = BeastMitigation }. initial_security_params(ConnectionEnd) -> @@ -506,11 +513,17 @@ encode_iolist(Type, Data, Version, ConnectionStates0) -> %% 1/n-1 splitting countermeasure Rizzo/Duong-Beast, RC4 chiphers are %% not vulnerable to this attack. -split_bin(<>, ChunkSize, Version, BCA) when +split_bin(<>, ChunkSize, Version, BCA, one_n_minus_one) when BCA =/= ?RC4 andalso ({3, 1} == Version orelse {3, 0} == Version) -> do_split_bin(Rest, ChunkSize, [[FirstByte]]); -split_bin(Bin, ChunkSize, _, _) -> +%% 0/n splitting countermeasure for clients that are incompatible with 1/n-1 +%% splitting. +split_bin(Bin, ChunkSize, Version, BCA, zero_n) when + BCA =/= ?RC4 andalso ({3, 1} == Version orelse + {3, 0} == Version) -> + do_split_bin(Bin, ChunkSize, [[<<>>]]); +split_bin(Bin, ChunkSize, _, _, _) -> do_split_bin(Bin, ChunkSize, []). do_split_bin(<<>>, _, Acc) -> diff --git a/lib/ssl/src/ssl_record.hrl b/lib/ssl/src/ssl_record.hrl index d34d144343..87fde35258 100644 --- a/lib/ssl/src/ssl_record.hrl +++ b/lib/ssl/src/ssl_record.hrl @@ -40,7 +40,9 @@ %% RFC 5746 secure_renegotiation, client_verify_data, - server_verify_data + server_verify_data, + %% How to do BEAST mitigation? + beast_mitigation }). -record(connection_states, { diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl index 40f3eea527..91903b4a1f 100644 --- a/lib/ssl/src/tls_connection.erl +++ b/lib/ssl/src/tls_connection.erl @@ -496,7 +496,8 @@ decode_alerts(Bin) -> initial_state(Role, Host, Port, Socket, {SSLOptions, SocketOptions, Tracker}, User, {CbModule, DataTag, CloseTag, ErrorTag}) -> - ConnectionStates = ssl_record:init_connection_states(Role), + #ssl_options{beast_mitigation = BeastMitigation} = SSLOptions, + ConnectionStates = ssl_record:init_connection_states(Role, BeastMitigation), SessionCacheCb = case application:get_env(ssl, session_cb) of {ok, Cb} when is_atom(Cb) -> diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl index d1162ab4a5..cd06b97ab2 100644 --- a/lib/ssl/test/ssl_basic_SUITE.erl +++ b/lib/ssl/test/ssl_basic_SUITE.erl @@ -236,7 +236,10 @@ error_handling_tests_tls()-> rizzo_tests() -> [rizzo, - no_rizzo_rc4]. + no_rizzo_rc4, + rizzo_one_n_minus_one, + rizzo_zero_n, + rizzo_disabled]. %%-------------------------------------------------------------------- init_per_suite(Config0) -> @@ -346,10 +349,27 @@ init_per_testcase(TestCase, Config) when TestCase == psk_cipher_suites; ssl_test_lib:ct_log_supported_protocol_versions(Config), ct:timetrap({seconds, 30}), Config; + init_per_testcase(rizzo, Config) -> ssl_test_lib:ct_log_supported_protocol_versions(Config), ct:timetrap({seconds, 40}), Config; + +init_per_testcase(rizzo_one_n_minus_one, Config) -> + ct:log("TLS/SSL version ~p~n ", [tls_record:supported_protocol_versions()]), + ct:timetrap({seconds, 40}), + rizzo_add_mitigation_option(one_n_minus_one, Config); + +init_per_testcase(rizzo_zero_n, Config) -> + ct:log("TLS/SSL version ~p~n ", [tls_record:supported_protocol_versions()]), + ct:timetrap({seconds, 40}), + rizzo_add_mitigation_option(zero_n, Config); + +init_per_testcase(rizzo_disabled, Config) -> + ct:log("TLS/SSL version ~p~n ", [tls_record:supported_protocol_versions()]), + ct:timetrap({seconds, 40}), + rizzo_add_mitigation_option(disabled, Config); + init_per_testcase(prf, Config) -> ct:log("TLS/SSL version ~p~n ", [tls_record:supported_protocol_versions()]), ct:timetrap({seconds, 40}), @@ -3488,6 +3508,36 @@ no_rizzo_rc4(Config) when is_list(Config) -> run_send_recv_rizzo(Ciphers, Config, Version, {?MODULE, send_recv_result_active_no_rizzo, []}). +rizzo_one_n_minus_one() -> + [{doc,"Test that the 1/n-1-split mitigation of Rizzo/Dungon attack can be explicitly selected"}]. + +rizzo_one_n_minus_one(Config) when is_list(Config) -> + Ciphers = [X || X ={_,Y,_} <- ssl:cipher_suites(), Y =/= rc4_128], + Prop = proplists:get_value(tc_group_properties, Config), + Version = proplists:get_value(name, Prop), + run_send_recv_rizzo(Ciphers, Config, Version, + {?MODULE, send_recv_result_active_rizzo, []}). + +rizzo_zero_n() -> + [{doc,"Test that the 0/n-split mitigation of Rizzo/Dungon attack can be explicitly selected"}]. + +rizzo_zero_n(Config) when is_list(Config) -> + Ciphers = [X || X ={_,Y,_} <- ssl:cipher_suites(), Y =/= rc4_128], + Prop = proplists:get_value(tc_group_properties, Config), + Version = proplists:get_value(name, Prop), + run_send_recv_rizzo(Ciphers, Config, Version, + {?MODULE, send_recv_result_active_no_rizzo, []}). + +rizzo_disabled() -> + [{doc,"Test that the mitigation of Rizzo/Dungon attack can be explicitly disabled"}]. + +rizzo_disabled(Config) when is_list(Config) -> + Ciphers = [X || X ={_,Y,_} <- ssl:cipher_suites(), Y =/= rc4_128], + Prop = proplists:get_value(tc_group_properties, Config), + Version = proplists:get_value(name, Prop), + run_send_recv_rizzo(Ciphers, Config, Version, + {?MODULE, send_recv_result_active_no_rizzo, []}). + %%-------------------------------------------------------------------- new_server_wants_peer_cert() -> [{doc, "Test that server configured to do client certification does" @@ -3960,6 +4010,18 @@ renegotiate_rejected(Socket) -> ssl:send(Socket, "Hello world"), ok. +rizzo_add_mitigation_option(Value, Config) -> + lists:foldl(fun(Opt, Acc) -> + case proplists:get_value(Opt, Acc) of + undefined -> Acc; + C -> + N = lists:keystore(beast_mitigation, 1, C, + {beast_mitigation, Value}), + lists:keystore(Opt, 1, Acc, {Opt, N}) + end + end, Config, + [client_opts, client_dsa_opts, server_opts, server_dsa_opts, + server_ecdsa_opts, server_ecdh_rsa_opts]). new_config(PrivDir, ServerOpts0) -> CaCertFile = proplists:get_value(cacertfile, ServerOpts0), -- cgit v1.2.3