aboutsummaryrefslogtreecommitdiffstats
path: root/lib/ssl/src
diff options
context:
space:
mode:
authorIngela Anderton Andin <[email protected]>2014-05-09 12:16:30 +0200
committerIngela Anderton Andin <[email protected]>2014-05-09 12:16:30 +0200
commit726a6beccce6ff77d9c3910396b9680147accd12 (patch)
treedc94e5911e4c868c314e180ecd12425e37e42f9f /lib/ssl/src
parent17a4fefd66da7e34775c5ddb9ac146816d5abd42 (diff)
parent6b483da9c102b1650ab103f78f57f9bd7c707781 (diff)
downloadotp-726a6beccce6ff77d9c3910396b9680147accd12.tar.gz
otp-726a6beccce6ff77d9c3910396b9680147accd12.tar.bz2
otp-726a6beccce6ff77d9c3910396b9680147accd12.zip
Merge branch 'ia/ssl/false-alerts/OTP-11890' into maint
* ia/ssl/false-alerts/OTP-11890: ssl: Add checks to avoid processing of illegal alerts
Diffstat (limited to 'lib/ssl/src')
-rw-r--r--lib/ssl/src/ssl_alert.erl29
-rw-r--r--lib/ssl/src/ssl_alert.hrl2
-rw-r--r--lib/ssl/src/tls_connection.erl22
3 files changed, 37 insertions, 16 deletions
diff --git a/lib/ssl/src/ssl_alert.erl b/lib/ssl/src/ssl_alert.erl
index db1535b5ec..78dc98bc25 100644
--- a/lib/ssl/src/ssl_alert.erl
+++ b/lib/ssl/src/ssl_alert.erl
@@ -31,7 +31,7 @@
-include("ssl_record.hrl").
-include("ssl_internal.hrl").
--export([encode/3, alert_txt/1, reason_code/2]).
+-export([encode/3, decode/1, alert_txt/1, reason_code/2]).
%%====================================================================
%% Internal application API
@@ -41,12 +41,21 @@
-spec encode(#alert{}, ssl_record:ssl_version(), #connection_states{}) ->
{iolist(), #connection_states{}}.
%%
-%% Description:
+%% Description: Encodes an alert
%%--------------------------------------------------------------------
encode(#alert{} = Alert, Version, ConnectionStates) ->
ssl_record:encode_alert_record(Alert, Version, ConnectionStates).
%%--------------------------------------------------------------------
+-spec decode(binary()) -> [#alert{}] | #alert{}.
+%%
+%% Description: Decode alert(s), will return a singel own alert if peer
+%% sends garbage or too many warning alerts.
+%%--------------------------------------------------------------------
+decode(Bin) ->
+ decode(Bin, [], 0).
+
+%%--------------------------------------------------------------------
-spec reason_code(#alert{}, client | server) -> closed | {essl, string()}.
%%
%% Description: Returns the error reason that will be returned to the
@@ -71,6 +80,22 @@ alert_txt(#alert{level = Level, description = Description, where = {Mod,Line}})
%%--------------------------------------------------------------------
%%% Internal functions
%%--------------------------------------------------------------------
+
+%% It is very unlikely that an correct implementation will send more than one alert at the time
+%% So it there is more than 10 warning alerts we consider it an error
+decode(<<?BYTE(Level), ?BYTE(_), _/binary>>, _, N) when Level == ?WARNING, N > ?MAX_ALERTS ->
+ ?ALERT_REC(?FATAL, ?DECODE_ERROR);
+decode(<<?BYTE(Level), ?BYTE(Description), Rest/binary>>, Acc, N) when Level == ?WARNING ->
+ Alert = ?ALERT_REC(Level, Description),
+ decode(Rest, [Alert | Acc], N + 1);
+decode(<<?BYTE(Level), ?BYTE(Description), _Rest/binary>>, Acc, _) when Level == ?FATAL->
+ Alert = ?ALERT_REC(Level, Description),
+ lists:reverse([Alert | Acc]); %% No need to decode rest fatal alert will end the connection
+decode(<<?BYTE(_Level), _/binary>>, _, _) ->
+ ?ALERT_REC(?FATAL, ?ILLEGAL_PARAMETER);
+decode(<<>>, Acc, _) ->
+ lists:reverse(Acc, []).
+
level_txt(?WARNING) ->
"Warning:";
level_txt(?FATAL) ->
diff --git a/lib/ssl/src/ssl_alert.hrl b/lib/ssl/src/ssl_alert.hrl
index 2d1f323085..f4f1d74264 100644
--- a/lib/ssl/src/ssl_alert.hrl
+++ b/lib/ssl/src/ssl_alert.hrl
@@ -104,6 +104,8 @@
-define(ALERT_REC(Level,Desc), #alert{level=Level,description=Desc,where={?FILE, ?LINE}}).
+-define(MAX_ALERTS, 10).
+
%% Alert
-record(alert, {
level,
diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl
index 930706cde6..32086ff6ce 100644
--- a/lib/ssl/src/tls_connection.erl
+++ b/lib/ssl/src/tls_connection.erl
@@ -362,20 +362,11 @@ encode_handshake(Handshake, Version, ConnectionStates0, Hist0) ->
ssl_record:encode_handshake(Frag, Version, ConnectionStates0),
{Encoded, ConnectionStates, Hist}.
-
encode_change_cipher(#change_cipher_spec{}, Version, ConnectionStates) ->
ssl_record:encode_change_cipher_spec(Version, ConnectionStates).
-
-
decode_alerts(Bin) ->
- decode_alerts(Bin, []).
-
-decode_alerts(<<?BYTE(Level), ?BYTE(Description), Rest/binary>>, Acc) ->
- A = ?ALERT_REC(Level, Description),
- decode_alerts(Rest, [A | Acc]);
-decode_alerts(<<>>, Acc) ->
- lists:reverse(Acc, []).
+ ssl_alert:decode(Bin).
initial_state(Role, Host, Port, Socket, {SSLOptions, SocketOptions}, User,
{CbModule, DataTag, CloseTag, ErrorTag}) ->
@@ -420,10 +411,13 @@ next_state(Current,_, #alert{} = Alert, #state{negotiated_version = Version} = S
next_state(_,Next, no_record, State) ->
{next_state, Next, State, get_timeout(State)};
-next_state(_,Next, #ssl_tls{type = ?ALERT, fragment = EncAlerts}, State) ->
- Alerts = decode_alerts(EncAlerts),
- handle_alerts(Alerts, {next_state, Next, State, get_timeout(State)});
-
+next_state(Current, Next, #ssl_tls{type = ?ALERT, fragment = EncAlerts}, #state{negotiated_version = Version} = State) ->
+ case decode_alerts(EncAlerts) of
+ Alerts = [_|_] ->
+ handle_alerts(Alerts, {next_state, Next, State, get_timeout(State)});
+ #alert{} = Alert ->
+ handle_own_alert(Alert, Version, Current, State)
+ end;
next_state(Current, Next, #ssl_tls{type = ?HANDSHAKE, fragment = Data},
State0 = #state{protocol_buffers =
#protocol_buffers{tls_handshake_buffer = Buf0} = Buffers,