File 4671-ssl-Add-more-unexpected-alerts.patch of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 4671-ssl-Add-more-unexpected-alerts.patch of Package erlang (Revision 36aa626aaad1ccbc23c5b81d2d7e5890)

Currently displaying revision 36aa626aaad1ccbc23c5b81d2d7e5890 , Show latest

From 00f93ff0a43c0ee24465243b5c574d89d0d81e6f Mon Sep 17 00:00:00 2001
From: Ingela Anderton Andin <ingela@erlang.org>
Date: Mon, 31 Jan 2022 20:13:03 +0100
Subject: [PATCH] ssl: Add more unexpected alerts

Malicious clients might try to DOS attack by keeping connections alive
by sending warning alerts or application data packages instead of
client hello messages. Although the server timeout will mitigate this
we can catch this early and send an unexpected message alert.

Also make some document clarifications.
---
 lib/ssl/doc/src/ssl.xml            | 17 +++++-
 lib/ssl/doc/src/using_ssl.xml      | 12 +++-
 lib/ssl/src/ssl_gen_statem.erl     | 40 ++++++++++---
 lib/ssl/src/tls_gen_connection.erl | 15 ++++-
 lib/ssl/test/tls_api_SUITE.erl     | 92 +++++++++++++++++++++++++++++-
 5 files changed, 163 insertions(+), 13 deletions(-)

diff --git a/lib/ssl/doc/src/ssl.xml b/lib/ssl/doc/src/ssl.xml
index 846e42f0ee..66c44fec1b 100644
--- a/lib/ssl/doc/src/ssl.xml
+++ b/lib/ssl/doc/src/ssl.xml
@@ -1737,6 +1737,11 @@ fun(srp, Username :: binary(), UserState :: term()) ->
 	the process owning the sslsocket will receive messages of type 
 	<seetype marker="#active_msgs"> active_msgs() </seetype>
 		
+
+	<warning>Not setting the timeout makes the server more vulnerable to
+	DoS attacks.
+	</warning>
+	
 </desc>
 </func>
 
@@ -1749,9 +1754,11 @@ fun(srp, Username :: binary(), UserState :: term()) ->
 	or equivalent, socket to an SSL socket, that is, performs
 the TLS server-side handshake and returns a TLS socket.
 
-	<warning>The <c>Socket</c> shall be in passive mode ({active,
-	false}) before calling this function or else the behavior of this function
-	is undefined.
+	<warning>The ordinary <c>Socket</c> shall be in passive mode ({active,
+	false}) before calling this function, and before the client tries
+	to connect with TLS, or else the behavior of this function
+	is undefined. The best way to ensure this is to create the ordinary listen socket
+	in passive mode.
 	</warning>
 
 	If <c>Socket</c> is an 
@@ -1760,6 +1767,10 @@ fun(srp, Username :: binary(), UserState :: term()) ->
 	<seemfa marker="#listen/2">listen/2</seemfa> and then performs
 	the TLS/DTLS handshake. Returns a new TLS/DTLS socket if the handshake is successful.
 
+	<warning>Not setting the timeout makes the server more vulnerable to
+	DoS attacks.
+	</warning>
+	
 	
 	 If option <c>{handshake, hello}</c> is specified the handshake is
 	 paused after receiving the client hello message and the
diff --git a/lib/ssl/doc/src/using_ssl.xml b/lib/ssl/doc/src/using_ssl.xml
index 0079e3d7d8..6b7fc62328 100644
--- a/lib/ssl/doc/src/using_ssl.xml
+++ b/lib/ssl/doc/src/using_ssl.xml
@@ -74,6 +74,12 @@ ssl:listen(9999, [{certfile, "cert.pem"}, {keyfile, "key.pem"},{reuseaddr, true}
 <code type="erl">3 server> {ok, TLSTransportSocket} = ssl:transport_accept(ListenSocket).
 {ok,{sslsocket, [...]}}</code>
 
+
+<note> ssl:transport_accept/1 and ssl:handshake/2 are separate functions so that
+the handshake part can be called in a new erlang process dedicated to handling the
+connection
+</note>
+
 Step 4: Start the client side: 
 <code type="erl">1 client> ssl:start().
 ok</code>
@@ -84,7 +90,11 @@ ok</code>
 Step 5: Do the TLS handshake:
 <code type="erl">4 server> {ok, Socket} = ssl:handshake(TLSTransportSocket).
 {ok,{sslsocket, [...]}}</code>
- 
+
+<note> A real server should use ssl:handshake/2 that has a timeout
+to avoid DoS attacks. In the example the timeout defaults to infinty.
+</note>
+
 Step 6: Send a message over TLS:
 <code type="erl">5 server> ssl:send(Socket, "foo").
 ok</code>
diff --git a/lib/ssl/src/ssl_gen_statem.erl b/lib/ssl/src/ssl_gen_statem.erl
index 292e490e8d..43c6b4ec3b 100644
--- a/lib/ssl/src/ssl_gen_statem.erl
+++ b/lib/ssl/src/ssl_gen_statem.erl
@@ -1000,7 +1000,6 @@ handle_alert(#alert{level = ?FATAL} = Alert0, StateName,
 Pids = Connection:pids(State),
 alert_user(Pids, Transport, Trackers, Socket, StateName, Opts, Pid, From, Alert, Role, StateName, Connection),
 {stop, {shutdown, normal}, State};
-
 handle_alert(#alert{level = ?WARNING, description = ?CLOSE_NOTIFY} = Alert,
 	 downgrade= StateName, State) ->
 {next_state, StateName, State, [{next_event, internal, Alert}]};
@@ -1009,6 +1008,31 @@ handle_alert(#alert{level = ?WARNING, description = ?CLOSE_NOTIFY} = Alert0,
 Alert = Alert0#alert{role = opposite_role(Role)},
 handle_normal_shutdown(Alert, StateName, State),
 {stop,{shutdown, peer_close}, State};
+handle_alert(#alert{level = ?WARNING, description = ?NO_RENEGOTIATION} = Alert, StateName,
+	 #state{static_env = #static_env{role = server = Role,
+ protocol_cb = Connection},
+ handshake_env = #handshake_env{renegotiation = {false, first}},
+ ssl_options = #{log_level := LogLevel}
+		 } = State) when StateName == intial_hello;
+ StateName == hello;
+ StateName == certify;
+ StateName == abbreviated;
+ StateName == cipher ->
+ log_alert(LogLevel, Role,
+ Connection:protocol_name(), StateName, Alert#alert{role = opposite_role(Role)}),
+ OwnAlert = ?ALERT_REC(?FATAL, ?UNEXPECTED_MESSAGE, unexpected_renegotiate_alert_during_initial_handshake),
+ handle_own_alert(OwnAlert, StateName, State);
+handle_alert(#alert{} = Alert, StateName,
+	 #state{static_env = #static_env{role = server = Role,
+ protocol_cb = Connection},
+ handshake_env = #handshake_env{renegotiation = {false, first}},
+ ssl_options = #{log_level := LogLevel}} = State) when StateName == start;
+ StateName == intial_hello;
+ StateName == hello ->
+ log_alert(LogLevel, Role,
+ Connection:protocol_name(), StateName, Alert#alert{role = opposite_role(Role)}),
+ OwnAlert = ?ALERT_REC(?FATAL, ?UNEXPECTED_MESSAGE, unexpected_alert),
+ handle_own_alert(OwnAlert, StateName, State);
 handle_alert(#alert{level = ?WARNING, description = ?NO_RENEGOTIATION} = Alert0, StateName,
 	 #state{static_env = #static_env{role = Role,
 protocol_cb = Connection},
@@ -1019,7 +1043,6 @@ handle_alert(#alert{level = ?WARNING, description = ?NO_RENEGOTIATION} = Alert0,
 Connection:protocol_name(), StateName, Alert),
 handle_normal_shutdown(Alert, StateName, State),
 {stop,{shutdown, peer_close}, State};
-
 handle_alert(#alert{level = ?WARNING, description = ?NO_RENEGOTIATION} = Alert, connection = StateName,
 	 #state{static_env = #static_env{role = Role,
 protocol_cb = Connection},
@@ -1031,7 +1054,6 @@ handle_alert(#alert{level = ?WARNING, description = ?NO_RENEGOTIATION} = Alert,
 gen_statem:reply(From, {error, renegotiation_rejected}),
 State = Connection:reinit_handshake_data(State0),
 Connection:next_event(connection, no_record, State#state{handshake_env = HsEnv#handshake_env{renegotiation = undefined}});
-
 handle_alert(#alert{level = ?WARNING, description = ?NO_RENEGOTIATION} = Alert, StateName,
 	 #state{static_env = #static_env{role = Role,
 protocol_cb = Connection},
@@ -1044,16 +1066,20 @@ handle_alert(#alert{level = ?WARNING, description = ?NO_RENEGOTIATION} = Alert,
 %% Go back to connection!
 State = Connection:reinit(State0#state{handshake_env = HsEnv#handshake_env{renegotiation = undefined}}),
 Connection:next_event(connection, no_record, State);
-
-%% Gracefully log and ignore all other warning alerts
+%% Gracefully log and ignore all other warning alerts pre TLS-1.3
 handle_alert(#alert{level = ?WARNING} = Alert, StateName,
 	 #state{static_env = #static_env{role = Role,
 protocol_cb = Connection},
- ssl_options = #{log_level := LogLevel}} = State) ->
+ connection_env = #connection_env{negotiated_version = Version},
+ ssl_options = #{log_level := LogLevel}} = State) when Version < {3,4} ->
 log_alert(LogLevel, Role,
 Connection:protocol_name(), StateName,
 Alert#alert{role = opposite_role(Role)}),
- Connection:next_event(StateName, no_record, State).
+ Connection:next_event(StateName, no_record, State);
+handle_alert(Alert0, StateName, State) ->
+ %% In TLS-1.3 all error alerts are fatal not matter of legacy level
+ handle_alert(Alert0#alert{level = ?FATAL}, StateName, State).
+
 handle_trusted_certs_db(#state{ssl_options =
 				 #{cacertfile := <<>>, cacerts := []}}) ->
 %% No trusted certs specified
diff --git a/lib/ssl/test/tls_api_SUITE.erl b/lib/ssl/test/tls_api_SUITE.erl
index a5f4fe3709..5048fd9ff1 100644
--- a/lib/ssl/test/tls_api_SUITE.erl
+++ b/lib/ssl/test/tls_api_SUITE.erl
@@ -26,6 +26,7 @@
 -include_lib("ssl/src/ssl_internal.hrl").
 -include_lib("ssl/src/ssl_api.hrl").
 -include_lib("ssl/src/tls_handshake.hrl").
+-include_lib("ssl/src/ssl_alert.hrl").
 
 %% Common test
 -export([all/0,
@@ -71,6 +72,12 @@
 tls_dont_crash_on_handshake_garbage/1,
 tls_tcp_error_propagation_in_active_mode/0,
 tls_tcp_error_propagation_in_active_mode/1,
+ tls_reject_warning_alert_in_initial_hs/0,
+ tls_reject_warning_alert_in_initial_hs/1,
+ tls_reject_fake_warning_alert_in_initial_hs/0,
+ tls_reject_fake_warning_alert_in_initial_hs/1,
+ tls_app_data_in_initial_hs_state/0,
+ tls_app_data_in_initial_hs_state/1,
 peername/0,
 peername/1,
 sockname/0,
@@ -116,7 +123,7 @@ all() ->
 
 groups() ->
 [
- {'tlsv1.3', [], api_tests() -- [sockname]},
+ {'tlsv1.3', [], api_tests() -- [sockname]},
 {'tlsv1.2', [], api_tests()},
 {'tlsv1.1', [], api_tests()},
 {'tlsv1', [], api_tests()}
@@ -140,6 +147,9 @@ api_tests() ->
 tls_tcp_msg_big,
 tls_dont_crash_on_handshake_garbage,
 tls_tcp_error_propagation_in_active_mode,
+ tls_reject_warning_alert_in_initial_hs,
+ tls_reject_fake_warning_alert_in_initial_hs,
+ tls_app_data_in_initial_hs_state,
 peername,
 sockname,
 tls_server_handshake_timeout,
@@ -671,6 +681,86 @@ tls_tcp_error_propagation_in_active_mode(Config) when is_list(Config) ->
 
 ssl_test_lib:check_result(Client, {ssl_closed, SslSocket}).
 
+%%--------------------------------------------------------------------
+tls_reject_warning_alert_in_initial_hs() ->
+ [{doc,"Test sending warning ALERT instead of client hello"}].
+tls_reject_warning_alert_in_initial_hs(Config) when is_list(Config) ->
+ ServerOpts = ssl_test_lib:ssl_options(server_rsa_opts, Config),
+ {_ClientNode, ServerNode, _Hostname} = ssl_test_lib:run_where(Config),
+ {Major, Minor} = case ssl_test_lib:protocol_version(Config, tuple) of
+ {3,4} ->
+ {3,3};
+ Other ->
+ Other
+ end,
+ Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
+					 {from, self()},
+					 {mfa, {ssl_test_lib, no_result, []}},
+					 {options, [{versions, [ssl_test_lib:protocol_version(Config)]} | ServerOpts]}]),
+ Port = ssl_test_lib:inet_port(Server),
+ {ok, Socket} = gen_tcp:connect("localhost", Port, [{active, false}, binary]),
+ NoRenegotiateAlert = <<?BYTE(?ALERT), ?BYTE(Major), ?BYTE(Minor), ?UINT16(2), ?BYTE(?WARNING), ?BYTE(?NO_RENEGOTIATION)>>,
+ gen_tcp:send(Socket, NoRenegotiateAlert),
+ UnexpectedMsgAlert = <<?BYTE(?ALERT), ?BYTE(Major), ?BYTE(Minor), ?UINT16(2), ?BYTE(?FATAL), ?BYTE(?UNEXPECTED_MESSAGE)>>,
+ {ok, UnexpectedMsgAlert} = gen_tcp:recv(Socket, 7),
+ {error, closed} = gen_tcp:recv(Socket, 0).
+
+%%--------------------------------------------------------------------
+tls_reject_fake_warning_alert_in_initial_hs() ->
+ [{doc,"Test sending 'fake' warning ALERT covers different function clause pre TLS-1.3"}].
+tls_reject_fake_warning_alert_in_initial_hs(Config) when is_list(Config) ->
+ ServerOpts = ssl_test_lib:ssl_options(server_rsa_opts, Config),
+ {_ClientNode, ServerNode, _Hostname} = ssl_test_lib:run_where(Config),
+ {Major, Minor} = case ssl_test_lib:protocol_version(Config, tuple) of
+ {3,4} ->
+ {3,3};
+ Other ->
+ Other
+ end,
+ Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
+					 {from, self()},
+					 {mfa, {ssl_test_lib, no_result, []}},
+					 {options, [{versions, [ssl_test_lib:protocol_version(Config)]} | ServerOpts]}]),
+ Port = ssl_test_lib:inet_port(Server),
+ {ok, Socket} = gen_tcp:connect("localhost", Port, [{active, false}, binary]),
+ NoRenegotiateAlert = <<?BYTE(?ALERT), ?BYTE(Major), ?BYTE(Minor), ?UINT16(2), ?BYTE(?WARNING), ?BYTE(?UNEXPECTED_MESSAGE)>>,
+ gen_tcp:send(Socket, NoRenegotiateAlert),
+ UnexpectedMsgAlert = <<?BYTE(?ALERT), ?BYTE(Major), ?BYTE(Minor), ?UINT16(2), ?BYTE(?FATAL), ?BYTE(?UNEXPECTED_MESSAGE)>>,
+ {ok, UnexpectedMsgAlert} = gen_tcp:recv(Socket, 7),
+ {error, closed} = gen_tcp:recv(Socket, 0).
+
+%%--------------------------------------------------------------------
+tls_app_data_in_initial_hs_state() ->
+ [{doc,"Test sending application data instead of initial client hello. In TLS-1.3 application data can be sent",
+ "in first round trip but not before client hello."}].
+tls_app_data_in_initial_hs_state(Config) when is_list(Config) ->
+ ServerOpts = ssl_test_lib:ssl_options(server_rsa_opts, Config),
+ {_ClientNode, ServerNode, _Hostname} = ssl_test_lib:run_where(Config),
+ Version = ssl_test_lib:protocol_version(Config, tuple),
+ {Major, Minor} = case Version of
+ {3,4} ->
+ {3,3};
+ Other ->
+ Other
+ end,
+ Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
+					 {from, self()},
+					 {mfa, {ssl_test_lib, no_result, []}},
+					 {options, [{versions, [ssl_test_lib:protocol_version(Config)]} | ServerOpts]}]),
+ Port = ssl_test_lib:inet_port(Server),
+ {ok, Socket} = gen_tcp:connect("localhost", Port, [{active, false}, binary]),
+ AppData = <<?BYTE(?APPLICATION_DATA), ?BYTE(Major), ?BYTE(Minor), ?UINT16(3), ?BYTE($F), ?BYTE($O), ?BYTE($O)>>,
+ gen_tcp:send(Socket, AppData),
+ UnexpectedMsgAlert =
+ case Version of
+ {_, 4} ->
+ <<?BYTE(?ALERT), ?BYTE(Major), ?BYTE(Minor), ?UINT16(2), ?BYTE(?FATAL), ?BYTE(?DECODE_ERROR)>>;
+ _ ->
+ <<?BYTE(?ALERT), ?BYTE(Major), ?BYTE(Minor), ?UINT16(2), ?BYTE(?FATAL), ?BYTE(?UNEXPECTED_MESSAGE)>>
+ end,
+ {ok, UnexpectedMsgAlert} = gen_tcp:recv(Socket, 7),
+ {error, closed} = gen_tcp:recv(Socket, 0).
+
 %%--------------------------------------------------------------------
 peername() ->
 [{doc,"Test API function peername/1"}].
-- 
2.34.1

Places

File 4671-ssl-Add-more-unexpected-alerts.patch of Package erlang (Revision 36aa626aaad1ccbc23c5b81d2d7e5890)

Places