ssl: Fix alert handling (TLS 1.3)

Server and client use different secrets when sending certificate related
alerts. This is due to a change to the TLS protocol where clients send
their 'certificate' message after they have received the server's 'finished'
message.

4 files changed, 57 insertions, 28 deletions

diff --git a/lib/ssl/src/tls_connection_1_3.erl b/lib/ssl/src/tls_connection_1_3.erl
index 821b7000cc..117e4f059d 100644
--- a/lib/ssl/src/tls_connection_1_3.erl
+++ b/lib/ssl/src/tls_connection_1_3.erl
@@ -228,8 +228,8 @@ wait_cert_cr(internal, #change_cipher_spec{}, State, _Module) ->
     tls_connection:next_event(?FUNCTION_NAME, no_record, State);
 wait_cert_cr(internal, #certificate_1_3{} = Certificate, State0, _Module) ->
     case tls_handshake_1_3:do_wait_cert_cr(Certificate, State0) of
-        #alert{} = Alert ->
-            ssl_connection:handle_own_alert(Alert, {3,4}, wait_cert_cr, State0);
+        {#alert{} = Alert, State} ->
+            ssl_connection:handle_own_alert(Alert, {3,4}, wait_cert_cr, State);
         {State1, NextState} ->
             tls_connection:next_event(NextState, no_record, State1)
     end;
diff --git a/lib/ssl/src/tls_handshake_1_3.erl b/lib/ssl/src/tls_handshake_1_3.erl
index e16d0c761b..4de51c9a35 100644
--- a/lib/ssl/src/tls_handshake_1_3.erl
+++ b/lib/ssl/src/tls_handshake_1_3.erl
@@ -733,6 +733,8 @@ do_wait_cert(#certificate_1_3{} = Certificate, State0) ->
             {?ALERT_REC(?FATAL, ?INTERNAL_ERROR, Reason), State};
         {Ref, {{handshake_failure, Reason}, State}} ->
             {?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, Reason), State};
+        {Ref, {#alert{} = Alert, State}} ->
+            {Alert, State};
         {#alert{} = Alert, State} ->
             {Alert, State}
     end.
@@ -893,7 +895,9 @@ do_wait_cert_cr(#certificate_1_3{} = Certificate, State0) ->
         {Ref, {{internal_error, Reason}, _State}} ->
             ?ALERT_REC(?FATAL, ?INTERNAL_ERROR, Reason);
         {Ref, {{handshake_failure, Reason}, _State}} ->
-            ?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, Reason)
+            ?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, Reason);
+        {Ref, {#alert{} = Alert, State}} ->
+            {Alert, State}
     end;
 do_wait_cert_cr(#certificate_request_1_3{} = CertificateRequest, State0) ->
     {Ref,Maybe} = maybe(),
@@ -1112,13 +1116,11 @@ process_certificate(#certificate_1_3{certificate_list = Certs0},
                     State = store_peer_cert(State0, PeerCert, PublicKeyInfo),
                     {ok, {State, wait_cv}};
                 {error, Reason} ->
-                    State1 = calculate_traffic_secrets(State0),
-                    State = ssl_record:step_encryption_state(State1),
+                    State = update_encryption_state(Role, State0),
                     {error, {Reason, State}};
-                #alert{} = Alert ->
-                    State1 = calculate_traffic_secrets(State0),
-                    State = ssl_record:step_encryption_state(State1),
-                    {Alert, State}
+                {ok, #alert{} = Alert} ->
+                    State = update_encryption_state(Role, State0),
+                    {error, {Alert, State}}
             end;
         false ->
             State1 = calculate_traffic_secrets(State0),
@@ -1142,6 +1144,17 @@ is_supported_signature_algorithm([BinCert|_], SignAlgs0) ->
     lists:member(Scheme, SignAlgs).
 
 
+%% Sets correct encryption state when sending Alerts in shared states that use different secrets.
+%% - If client: use handshake secrets.
+%% - If server: use traffic secrets as by this time the client's state machine
+%%              already stepped into the 'connection' state.
+update_encryption_state(server, State0) ->
+    State1 = calculate_traffic_secrets(State0),
+    ssl_record:step_encryption_state(State1);
+update_encryption_state(client, State) ->
+    State.
+
+
 validate_certificate_chain(Certs, CertDbHandle, CertDbRef, SslOptions, CRLDbHandle, Role, Host) ->
     ServerName = ssl_handshake:server_name(SslOptions#ssl_options.server_name_indication, Host, Role),
     [PeerCert | ChainCerts ] = Certs,
@@ -1162,9 +1175,9 @@ validate_certificate_chain(Certs, CertDbHandle, CertDbRef, SslOptions, CRLDbHand
             {ok, {PublicKeyInfo,_}} ->
                 {ok, {PeerCert, PublicKeyInfo}};
             {error, Reason} ->
-                ssl_handshake:handle_path_validation_error(Reason, PeerCert, ChainCerts,
-                                                           SslOptions, Options,
-                                                           CertDbHandle, CertDbRef)
+                {ok, ssl_handshake:handle_path_validation_error(Reason, PeerCert, ChainCerts,
+                                                                SslOptions, Options,
+                                                                CertDbHandle, CertDbRef)}
         end
     catch
         error:{badmatch,{asn1, Asn1Reason}} ->
diff --git a/lib/ssl/test/ssl_certificate_verify_SUITE.erl b/lib/ssl/test/ssl_certificate_verify_SUITE.erl
index 358e9f8f77..c6982bb928 100644
--- a/lib/ssl/test/ssl_certificate_verify_SUITE.erl
+++ b/lib/ssl/test/ssl_certificate_verify_SUITE.erl
@@ -302,7 +302,13 @@ server_require_peer_cert_fail(Config) when is_list(Config) ->
 					      {from, self()},
 					      {options, [{active, Active} | BadClientOpts]}]),
 
-    ssl_test_lib:check_server_alert(Server, Client, handshake_failure).
+    Version = proplists:get_value(version,Config),
+    case Version of
+        'tlsv1.3' ->
+            ssl_test_lib:check_server_alert(Server, Client, certificate_required);
+        _ ->
+            ssl_test_lib:check_server_alert(Server, Client, handshake_failure)
+    end.
 
 %%--------------------------------------------------------------------
 server_require_peer_cert_empty_ok() ->
@@ -855,6 +861,7 @@ invalid_signature_server(Config) when is_list(Config) ->
 					      {from, self()},
 					      {options, [{verify, verify_peer} | ClientOpts]}]),
     ssl_test_lib:check_server_alert(Server, Client, unknown_ca).
+
 %%--------------------------------------------------------------------
 
 invalid_signature_client() ->
diff --git a/lib/ssl/test/ssl_test_lib.erl b/lib/ssl/test/ssl_test_lib.erl
index c706c68d3a..0318cc81e3 100644
--- a/lib/ssl/test/ssl_test_lib.erl
+++ b/lib/ssl/test/ssl_test_lib.erl
@@ -428,41 +428,42 @@ check_result(Pid, Msg) ->
 		      {got, Unexpected}},
 	    ct:fail(Reason)
     end.
+
 check_server_alert(Pid, Alert) ->
     receive
 	{Pid, {error, {tls_alert, {Alert, STxt}}}} ->
             check_server_txt(STxt),
+            ok;
+        {Pid, {error, closed}} ->
             ok
     end.
 check_server_alert(Server, Client, Alert) ->
     receive
 	{Server, {error, {tls_alert, {Alert, STxt}}}} ->
             check_server_txt(STxt),
-	    receive
-		{Client, {error, {tls_alert, {Alert, CTxt}}}} ->
-                    check_client_txt(CTxt),
-		    ok;
-		{Client, {error, closed}} ->
-		    ok
-	    end
+            check_client_alert(Client, Alert)
     end.
 check_client_alert(Pid, Alert) ->
     receive
 	{Pid, {error, {tls_alert, {Alert, CTxt}}}} ->
             check_client_txt(CTxt),
+            ok;
+        {Pid, {ssl_error, _, {tls_alert, {Alert, CTxt}}}} ->
+            check_client_txt(CTxt),
+            ok;
+        {Pid, {error, closed}} ->
             ok
     end.
 check_client_alert(Server, Client, Alert) ->
     receive
 	{Client, {error, {tls_alert, {Alert, CTxt}}}} ->
             check_client_txt(CTxt),
-	    receive
-		{Server, {error, {tls_alert, {Alert, STxt}}}} ->
-                      check_server_txt(STxt),
-		    ok;
-		{Server, {error, closed}} ->
-		    ok
-	    end
+            check_server_alert(Server, Alert);
+        {Client, {ssl_error, _, {tls_alert, {Alert, CTxt}}}} ->
+            check_client_txt(CTxt),
+            ok;
+        {Client, {error, closed}} ->
+            ok
     end.
 check_server_txt("TLS server" ++ _) ->
     ok;
@@ -1103,7 +1104,15 @@ run_client_error(Opts) ->
     Options = proplists:get_value(options, Opts),
     ct:log("~p:~p~nssl:connect(~p, ~p, ~p)~n", [?MODULE,?LINE, Host, Port, Options]),
     Error = Transport:connect(Host, Port, Options),
-    Pid ! {self(), Error}.
+    case Error of
+        {error, {tls_alert, _}} ->
+            Pid ! {self(), Error};
+        {ok, _Socket} ->
+            receive
+                {ssl_error, _, {tls_alert, _}} = SslError ->
+                                Pid ! {self(), SslError}
+            end
+    end.
 
 accepters(N) ->
     accepters([], N).