Merge branch 'ingela/ssl/error-handling-should-be-throw/OTP-15502' into maint

* ingela/ssl/error-handling-should-be-throw/OTP-15502:
  ssl: Clean up extension handling

3 files changed, 52 insertions, 69 deletions

diff --git a/lib/ssl/src/dtls_handshake.erl b/lib/ssl/src/dtls_handshake.erl
index 3f70eaec8a..1917d51c03 100644
--- a/lib/ssl/src/dtls_handshake.erl
+++ b/lib/ssl/src/dtls_handshake.erl
@@ -215,8 +215,6 @@ handle_client_hello_extensions(Version, Type, Random, CipherSuites,
 						     HelloExt, dtls_v1:corresponding_tls_version(Version),
 						     SslOpts, Session0, 
                                                      ConnectionStates0, Renegotiation) of
-	#alert{} = Alert ->
-	    Alert;
 	{Session, ConnectionStates, Protocol, ServerHelloExt} ->
 	    {Version, {Type, Session}, ConnectionStates, Protocol, ServerHelloExt, HashSign}
     catch throw:Alert ->
@@ -225,17 +223,16 @@ handle_client_hello_extensions(Version, Type, Random, CipherSuites,
 
 handle_server_hello_extensions(Version, SessionId, Random, CipherSuite,
 			       Compression, HelloExt, SslOpt, ConnectionStates0, Renegotiation) ->
-    case ssl_handshake:handle_server_hello_extensions(dtls_record, Random, CipherSuite,
-						      Compression, HelloExt,
-						      dtls_v1:corresponding_tls_version(Version),
-						      SslOpt, ConnectionStates0, Renegotiation) of
-	#alert{} = Alert ->
-	    Alert;
+    try ssl_handshake:handle_server_hello_extensions(dtls_record, Random, CipherSuite,
+                                                     Compression, HelloExt,
+                                                     dtls_v1:corresponding_tls_version(Version),
+                                                     SslOpt, ConnectionStates0, Renegotiation) of
 	{ConnectionStates, ProtoExt, Protocol} ->
 	    {Version, SessionId, ConnectionStates, ProtoExt, Protocol}
+    catch throw:Alert ->
+	    Alert
     end.
 
-
 %%--------------------------------------------------------------------
 
 enc_handshake(#hello_verify_request{protocol_version = {Major, Minor},
diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl
index 14df1d2e02..a50580e1ea 100644
--- a/lib/ssl/src/ssl_handshake.erl
+++ b/lib/ssl/src/ssl_handshake.erl
@@ -972,34 +972,30 @@ handle_client_hello_extensions(RecordCB, Random, ClientCipherSuites,
 			       #session{cipher_suite = NegotiatedCipherSuite,
 					compression_method = Compression} = Session0,
 			       ConnectionStates0, Renegotiation) ->
-    Session = handle_srp_extension(SRP, Session0),
-    ConnectionStates = handle_renegotiation_extension(server, RecordCB, Version, Info,
-						      Random, NegotiatedCipherSuite, 
+        Session = handle_srp_extension(SRP, Session0),
+        ConnectionStates = handle_renegotiation_extension(server, RecordCB, Version, Info,
+                                                          Random, NegotiatedCipherSuite, 
 						      ClientCipherSuites, Compression,
-						      ConnectionStates0, Renegotiation, SecureRenegotation),
-
-    ServerHelloExtensions =  #hello_extensions{
-				renegotiation_info = renegotiation_info(RecordCB, server,
-									ConnectionStates, Renegotiation),
-				ec_point_formats = server_ecc_extension(Version, ECCFormat)
-			       },
-
+                                                          ConnectionStates0, Renegotiation, SecureRenegotation),
+        
+        ServerHelloExtensions =  #hello_extensions{
+                                    renegotiation_info = renegotiation_info(RecordCB, server,
+                                                                            ConnectionStates, Renegotiation),
+                                    ec_point_formats = server_ecc_extension(Version, ECCFormat)
+                                   },
+        
     %% If we receive an ALPN extension and have ALPN configured for this connection,
     %% we handle it. Otherwise we check for the NPN extension.
     if
         ALPN =/= undefined, ALPNPreferredProtocols =/= undefined ->
-			case handle_alpn_extension(ALPNPreferredProtocols, decode_alpn(ALPN)) of
-                #alert{} = Alert ->
-                    Alert;
-                Protocol ->
-                    {Session, ConnectionStates, Protocol,
-                        ServerHelloExtensions#hello_extensions{alpn=encode_alpn([Protocol], Renegotiation)}}
-            end;
+            Protocol = handle_alpn_extension(ALPNPreferredProtocols, decode_alpn(ALPN)),
+            {Session, ConnectionStates, Protocol,
+             ServerHelloExtensions#hello_extensions{alpn=encode_alpn([Protocol], Renegotiation)}};
         true ->
-            ProtocolsToAdvertise = handle_next_protocol_extension(NextProtocolNegotiation, Renegotiation, Opts),
+                ProtocolsToAdvertise = handle_next_protocol_extension(NextProtocolNegotiation, Renegotiation, Opts),
             {Session, ConnectionStates, undefined,
-				ServerHelloExtensions#hello_extensions{next_protocol_negotiation=
-                	encode_protocols_advertised_on_server(ProtocolsToAdvertise)}}
+             ServerHelloExtensions#hello_extensions{next_protocol_negotiation=
+                                                        encode_protocols_advertised_on_server(ProtocolsToAdvertise)}}
     end.
 
 handle_server_hello_extensions(RecordCB, Random, CipherSuite, Compression,
@@ -1022,12 +1018,8 @@ handle_server_hello_extensions(RecordCB, Random, CipherSuite, Compression,
         [Protocol] when not Renegotiation ->
             {ConnectionStates, alpn, Protocol};
         undefined ->
-            case handle_next_protocol(NextProtocolNegotiation, NextProtoSelector, Renegotiation) of
-                #alert{} = Alert ->
-                    Alert;
-                Protocol ->
-                    {ConnectionStates, npn, Protocol}
-            end;
+            Protocol = handle_next_protocol(NextProtocolNegotiation, NextProtoSelector, Renegotiation),
+            {ConnectionStates, npn, Protocol};
         {error, Reason} ->
             ?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, Reason);
         [] ->
@@ -2189,30 +2181,26 @@ filter_unavailable_ecc_suites(_, Suites) ->
 handle_renegotiation_extension(Role, RecordCB, Version, Info, Random, NegotiatedCipherSuite, 
 			       ClientCipherSuites, Compression,
 			       ConnectionStates0, Renegotiation, SecureRenegotation) ->
-    case handle_renegotiation_info(RecordCB, Role, Info, ConnectionStates0,
-				   Renegotiation, SecureRenegotation,
-				   ClientCipherSuites) of
-	{ok, ConnectionStates} ->
-	    hello_pending_connection_states(RecordCB, Role,
-					    Version,
-					    NegotiatedCipherSuite,
-					    Random,
-					    Compression,
-					    ConnectionStates);
-	#alert{} = Alert ->
-	    throw(Alert)
-    end.
+    {ok, ConnectionStates} = handle_renegotiation_info(RecordCB, Role, Info, ConnectionStates0,
+                                                       Renegotiation, SecureRenegotation,
+                                                       ClientCipherSuites),
+    hello_pending_connection_states(RecordCB, Role,
+                                    Version,
+                                    NegotiatedCipherSuite,
+                                    Random,
+                                    Compression,
+                                    ConnectionStates).
 
 %% Receive protocols, choose one from the list, return it.
 handle_alpn_extension(_, {error, Reason}) ->
-    ?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, Reason);
+    throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, Reason));
 handle_alpn_extension([], _) ->
-	?ALERT_REC(?FATAL, ?NO_APPLICATION_PROTOCOL);
+    throw(?ALERT_REC(?FATAL, ?NO_APPLICATION_PROTOCOL));
 handle_alpn_extension([ServerProtocol|Tail], ClientProtocols) ->
-	case lists:member(ServerProtocol, ClientProtocols) of
-		true -> ServerProtocol;
-		false -> handle_alpn_extension(Tail, ClientProtocols)
-	end.
+    case lists:member(ServerProtocol, ClientProtocols) of
+        true -> ServerProtocol;
+            false -> handle_alpn_extension(Tail, ClientProtocols)
+    end.
 
 handle_next_protocol(undefined,
 		     _NextProtocolSelector, _Renegotiating) ->
@@ -2225,14 +2213,14 @@ handle_next_protocol(#next_protocol_negotiation{} = NextProtocols,
         true ->
             select_next_protocol(decode_next_protocols(NextProtocols), NextProtocolSelector);
         false ->
-            ?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, unexpected_next_protocol_extension)
+            throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, unexpected_next_protocol_extension))
     end.
 
 
 handle_next_protocol_extension(NextProtocolNegotiation, Renegotiation, SslOpts)->
     case handle_next_protocol_on_server(NextProtocolNegotiation, Renegotiation, SslOpts) of
 	#alert{} = Alert ->
-	    Alert;
+	    throw(Alert);
 	ProtocolsToAdvertise ->
 	    ProtocolsToAdvertise
     end.
@@ -2428,14 +2416,14 @@ handle_renegotiation_info(_RecordCB, client, #renegotiation_info{renegotiated_co
 	true ->
 	    {ok, ConnectionStates};
 	false ->
-            ?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, client_renegotiation)
+            throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, client_renegotiation))
     end;
 handle_renegotiation_info(_RecordCB, server, #renegotiation_info{renegotiated_connection = ClientVerify},
 			  ConnectionStates, true, _, CipherSuites) ->
 
       case is_member(?TLS_EMPTY_RENEGOTIATION_INFO_SCSV, CipherSuites) of
 	  true ->
-              ?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, {server_renegotiation, empty_renegotiation_info_scsv});
+              throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, {server_renegotiation, empty_renegotiation_info_scsv}));
 	  false ->
 	      ConnectionState = ssl_record:current_connection_state(ConnectionStates, read),
 	      Data =  maps:get(client_verify_data, ConnectionState),
@@ -2443,7 +2431,7 @@ handle_renegotiation_info(_RecordCB, server, #renegotiation_info{renegotiated_co
 		  true ->
 		      {ok, ConnectionStates};
 		  false ->
-                      ?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, server_renegotiation)
+                      throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, server_renegotiation))
 	      end
       end;
 
@@ -2453,7 +2441,7 @@ handle_renegotiation_info(RecordCB, client, undefined, ConnectionStates, true, S
 handle_renegotiation_info(RecordCB, server, undefined, ConnectionStates, true, SecureRenegotation, CipherSuites) ->
      case is_member(?TLS_EMPTY_RENEGOTIATION_INFO_SCSV, CipherSuites) of
 	  true ->
-             ?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, {server_renegotiation, empty_renegotiation_info_scsv});
+             throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, {server_renegotiation, empty_renegotiation_info_scsv}));
 	 false ->
 	     handle_renegotiation_info(RecordCB, ConnectionStates, SecureRenegotation)
      end.
@@ -2462,9 +2450,9 @@ handle_renegotiation_info(_RecordCB, ConnectionStates, SecureRenegotation) ->
     ConnectionState = ssl_record:current_connection_state(ConnectionStates, read),
     case {SecureRenegotation, maps:get(secure_renegotiation, ConnectionState)} of
 	{_, true} ->
-            ?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, already_secure);
+            throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, already_secure));
 	{true, false} ->
-	    ?ALERT_REC(?FATAL, ?NO_RENEGOTIATION);
+	    throw(?ALERT_REC(?FATAL, ?NO_RENEGOTIATION));
 	{false, false} ->
 	    {ok, ConnectionStates}
     end.
diff --git a/lib/ssl/src/tls_handshake.erl b/lib/ssl/src/tls_handshake.erl
index 19a5eb0348..65217ad68e 100644
--- a/lib/ssl/src/tls_handshake.erl
+++ b/lib/ssl/src/tls_handshake.erl
@@ -223,8 +223,6 @@ handle_client_hello_extensions(Version, Type, Random, CipherSuites,
 						     HelloExt, Version, SslOpts,
 						     Session0, ConnectionStates0, 
                                                      Renegotiation) of
-	#alert{} = Alert ->
-	    Alert;
 	{Session, ConnectionStates, Protocol, ServerHelloExt} ->
 	    {Version, {Type, Session}, ConnectionStates, Protocol, 
              ServerHelloExt, HashSign}
@@ -235,14 +233,14 @@ handle_client_hello_extensions(Version, Type, Random, CipherSuites,
 
 handle_server_hello_extensions(Version, SessionId, Random, CipherSuite,
 			Compression, HelloExt, SslOpt, ConnectionStates0, Renegotiation) ->
-    case ssl_handshake:handle_server_hello_extensions(tls_record, Random, CipherSuite,
+    try ssl_handshake:handle_server_hello_extensions(tls_record, Random, CipherSuite,
 						      Compression, HelloExt, Version,
 						      SslOpt, ConnectionStates0, 
-                                                      Renegotiation) of
-	#alert{} = Alert ->
-	    Alert;
+                                                     Renegotiation) of
 	{ConnectionStates, ProtoExt, Protocol} ->
 	    {Version, SessionId, ConnectionStates, ProtoExt, Protocol}
+    catch throw:Alert ->
+	    Alert
     end.
 %%--------------------------------------------------------------------
 enc_handshake(#hello_request{}, _Version) ->