ssl: Fix interop problems with older TLS versions

This commit fixes a bug that makes a client, configured to support
TLS 1.3, to select TLS 1.3 record protocol before having received
a ServerHello.

OTP-16303

5 files changed, 126 insertions, 34 deletions

diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl
index 8cc8c45946..20224f4c65 100644
--- a/lib/ssl/src/tls_connection.erl
+++ b/lib/ssl/src/tls_connection.erl
@@ -298,9 +298,12 @@ handle_protocol_record(#ssl_tls{type = ?HANDSHAKE, fragment = Data},
 		    StateName, #state{protocol_buffers =
 					  #protocol_buffers{tls_handshake_buffer = Buf0} = Buffers,
                                       connection_env = #connection_env{negotiated_version = Version},
+                                      static_env = #static_env{role = Role},
 				      ssl_options = Options} = State0) ->
     try
-	EffectiveVersion = effective_version(Version, Options),
+	%% Calculate the effective version that should be used when decoding an incoming handshake
+	%% message.
+	EffectiveVersion = effective_version(Version, Options, Role),
 	{Packets, Buf} = tls_handshake:get_tls_handshake(EffectiveVersion,Data,Buf0, Options),
 	State =
 	    State0#state{protocol_buffers =
@@ -543,7 +546,9 @@ init({call, From}, {start, Timeout},
             handshake_env = #handshake_env{renegotiation = {Renegotiation, _}} = HsEnv,
             connection_env = CEnv,
 	    ssl_options = #{log_level := LogLevel,
-                            versions := Versions,
+                            %% Use highest version in initial ClientHello.
+                            %% Versions is a descending list of supported versions.
+                            versions := [HelloVersion|_] = Versions,
                             session_tickets := SessionTickets} = SslOpts,
 	    session = NewSession,
 	    connection_states = ConnectionStates0
@@ -560,7 +565,6 @@ init({call, From}, {start, Timeout},
                                        KeyShare,
                                        TicketData),
 
-    HelloVersion = tls_record:hello_version(Versions),
     Handshake0 = ssl_handshake:init_handshake_history(),
 
     %% Update pre_shared_key extension with binders (TLS 1.3)
@@ -573,8 +577,16 @@ init({call, From}, {start, Timeout},
     ssl_logger:debug(LogLevel, outbound, 'handshake', Hello1),
     ssl_logger:debug(LogLevel, outbound, 'record', BinMsg),
 
+    %% RequestedVersion is used as the legacy record protocol version and shall be
+    %% {3,3} in case of TLS 1.2 and higher. In all other cases it defaults to the
+    %% lowest supported protocol version.
+    %%
+    %% negotiated_version is also used by the TLS 1.3 state machine and is set after
+    %% ServerHello is processed.
+    RequestedVersion = tls_record:hello_version(Versions),
     State = State1#state{connection_states = ConnectionStates,
-                         connection_env = CEnv#connection_env{negotiated_version = HelloVersion}, %% Requested version
+                         connection_env = CEnv#connection_env{
+                                            negotiated_version = RequestedVersion},
                          session = Session,
                          handshake_env = HsEnv#handshake_env{tls_handshake_history = Handshake},
                          start_or_recv_from = From,
@@ -612,7 +624,7 @@ hello(internal, #client_hello{extensions = Extensions} = Hello,
     {next_state, user_hello, State#state{start_or_recv_from = undefined,
                                          handshake_env = HsEnv#handshake_env{hello = Hello}},
      [{reply, From, {ok, Extensions}}]};
-hello(internal, #server_hello{extensions = Extensions} = Hello, 
+hello(internal, #server_hello{extensions = Extensions} = Hello,
       #state{ssl_options = #{handshake := hello},
              handshake_env = HsEnv,
              start_or_recv_from = From} = State) ->
@@ -683,9 +695,12 @@ hello(internal, #server_hello{} = Hello,
 	    ssl_connection:handle_session(Hello, 
 					  Version, NewId, ConnectionStates, ProtoExt, Protocol, State);
         %% TLS 1.3
-        {next_state, wait_sh} ->
+        {next_state, wait_sh, SelectedVersion} ->
             %% Continue in TLS 1.3 'wait_sh' state
-            {next_state, wait_sh, State, [{next_event, internal, Hello}]}
+            {next_state, wait_sh,
+             State#state{
+               connection_env = CEnv#connection_env{negotiated_version = SelectedVersion}},
+             [{next_event, internal, Hello}]}
     end;
 hello(info, Event, State) ->
     gen_info(Event, ?FUNCTION_NAME, State);
@@ -1079,12 +1094,19 @@ next_tls_record(Data, StateName,
                                                       tls_cipher_texts = CT0} = Buffers,
                                 ssl_options = SslOpts} = State0) ->
     Versions =
-        %% TLS 1.3 Client/Server
-        %% - Ignore TLSPlaintext.legacy_record_version
-        %% - Verify that TLSCiphertext.legacy_record_version is set to  0x0303 for all records
-        %%   other than an initial ClientHello, where it MAY also be 0x0301.
+        %% TLSPlaintext.legacy_record_version is ignored in TLS 1.3 and thus all
+        %% record version are accepted when receiving initial ClientHello and
+        %% ServerHello. This can happen in state 'hello' in case of all TLS
+        %% versions and also in state 'start' when TLS 1.3 is negotiated.
+        %% After the version is negotiated all subsequent TLS records shall have
+        %% the proper legacy_record_version (= negotiated_version).
+        %% Note: TLS record version {3,4} is used internally in TLS 1.3 and at this
+        %% point it is the same as the negotiated protocol version.
+        %% TODO: Refactor state machine and introduce a record_protocol_version beside
+        %% the negotiated_version.
         case StateName of
-            hello ->
+            State when State =:= hello orelse
+                       State =:= start ->
                 [tls_record:protocol_version(Vsn) || Vsn <- ?ALL_AVAILABLE_VERSIONS];
             _ ->
                 State0#state.connection_env#connection_env.negotiated_version
@@ -1337,9 +1359,19 @@ choose_tls_version(_, _) ->
     'tls_v1.2'.
 
 
-effective_version(undefined, #{versions := [Version|_]}) ->
+%% Special version handling for TLS 1.3 clients:
+%% In the shared state 'init' negotiated_version is set to requested version and
+%% that is expected by the legacy part of the state machine. However, in order to
+%% be able to process new TLS 1.3 extensions, the effective version shall be set
+%% {3,4}.
+%% When highest supported version is {3,4} the negotiated version is set to {3,3}.
+effective_version({3,3} , #{versions := [Version|_]}, client) when Version >= {3,4} ->
     Version;
-effective_version(Version, _) ->
+%% Use highest supported version during startup (TLS server, all versions).
+effective_version(undefined, #{versions := [Version|_]}, _) ->
+    Version;
+%% Use negotiated version in all other cases.
+effective_version(Version, _, _) ->
     Version.
 
 
diff --git a/lib/ssl/src/tls_handshake.erl b/lib/ssl/src/tls_handshake.erl
index c6383be1d0..474cbd621b 100644
--- a/lib/ssl/src/tls_handshake.erl
+++ b/lib/ssl/src/tls_handshake.erl
@@ -105,7 +105,7 @@ client_hello(_Host, _Port, ConnectionStates,
 		   {tls_record:tls_version(), {resumed | new, #session{}}, 
 		    ssl_record:connection_states(), binary() | undefined, 
                     HelloExt::map(), {ssl:hash(), ssl:sign_algo()} | 
-                    undefined} | {atom(), atom()} |#alert{}.
+                    undefined} | {atom(), atom()} | {atom(), atom(), tuple()} | #alert{}.
 %%
 %% Description: Handles a received hello message
 %%--------------------------------------------------------------------
@@ -175,9 +175,9 @@ hello(#server_hello{server_version = LegacyVersion,
                             handle_server_hello_extensions(Version, SessionId, Random, CipherSuite,
                                                            Compression, HelloExt, SslOpt,
                                                            ConnectionStates0, Renegotiation);
-                        {3,4} ->
+                        SelectedVersion ->
                             %% TLS 1.3
-                            {next_state, wait_sh}
+                            {next_state, wait_sh, SelectedVersion}
                     end;
                 false ->
                     ?ALERT_REC(?FATAL, ?ILLEGAL_PARAMETER)
diff --git a/lib/ssl/src/tls_handshake_1_3.erl b/lib/ssl/src/tls_handshake_1_3.erl
index aa74d11d69..4cede4f99f 100644
--- a/lib/ssl/src/tls_handshake_1_3.erl
+++ b/lib/ssl/src/tls_handshake_1_3.erl
@@ -609,18 +609,16 @@ do_start(#client_hello{cipher_suites = ClientCiphers,
 %% TLS Client
 do_start(#server_hello{cipher_suite = SelectedCipherSuite,
                        session_id = SessionId,
-                       extensions = Extensions} = _ServerHello,
+                       extensions = Extensions},
          #state{static_env = #static_env{role = client,
                                          host = Host,
                                          port = Port,
                                          transport_cb = Transport,
                                          socket = Socket},
-                handshake_env = #handshake_env{renegotiation = {Renegotiation, _},
-                                               tls_handshake_history = _HHistory} = HsEnv,
-                connection_env = CEnv,
+                handshake_env = #handshake_env{renegotiation = {Renegotiation, _}} = HsEnv,
+                connection_env = #connection_env{negotiated_version = NegotiatedVersion},
                 ssl_options = #{ciphers := ClientCiphers,
                                 supported_groups := ClientGroups0,
-                                versions := Versions,
                                 use_ticket := UseTicket,
                                 session_tickets := SessionTickets,
                                 log_level := LogLevel} = SslOpts,
@@ -656,8 +654,6 @@ do_start(#server_hello{cipher_suite = SelectedCipherSuite,
                                            SessionId, Renegotiation, Cert, ClientKeyShare,
                                            TicketData),
 
-        HelloVersion = tls_record:hello_version(Versions),
-
         %% Update state
         State1 = update_start_state(State0,
                                     #{cipher => SelectedCipherSuite,
@@ -667,22 +663,21 @@ do_start(#server_hello{cipher_suite = SelectedCipherSuite,
 
         %% Replace ClientHello1 with a special synthetic handshake message
         State2 = replace_ch1_with_message_hash(State1),
-        #state{handshake_env = #handshake_env{tls_handshake_history = HHistory}} = State2,
+        #state{handshake_env = #handshake_env{tls_handshake_history = HHistory0}} = State2,
 
         %% Update pre_shared_key extension with binders (TLS 1.3)
-        Hello = tls_handshake_1_3:maybe_add_binders(Hello0, HHistory, TicketData, HelloVersion),
+        Hello = tls_handshake_1_3:maybe_add_binders(Hello0, HHistory0, TicketData, NegotiatedVersion),
 
-        {BinMsg, ConnectionStates, Handshake} =
-            tls_connection:encode_handshake(Hello,  HelloVersion, ConnectionStates0, HHistory),
+        {BinMsg, ConnectionStates, HHistory} =
+            tls_connection:encode_handshake(Hello,  NegotiatedVersion, ConnectionStates0, HHistory0),
         tls_socket:send(Transport, Socket, BinMsg),
         ssl_logger:debug(LogLevel, outbound, 'handshake', Hello),
         ssl_logger:debug(LogLevel, outbound, 'record', BinMsg),
 
         State = State2#state{
                   connection_states = ConnectionStates,
-                  connection_env = CEnv#connection_env{negotiated_version = HelloVersion}, %% Requested version
                   session = Session0#session{session_id = Hello#client_hello.session_id},
-                  handshake_env = HsEnv#handshake_env{tls_handshake_history = Handshake},
+                  handshake_env = HsEnv#handshake_env{tls_handshake_history = HHistory},
                   key_share = ClientKeyShare},
 
         {State, wait_sh}
diff --git a/lib/ssl/src/tls_record.erl b/lib/ssl/src/tls_record.erl
index cfd75076b3..e8040b461d 100644
--- a/lib/ssl/src/tls_record.erl
+++ b/lib/ssl/src/tls_record.erl
@@ -391,7 +391,7 @@ is_acceptable_version(_,_) ->
 
 -spec hello_version([tls_version()]) -> tls_version().
 hello_version([Highest|_]) when Highest >= {3,3} ->
-    Highest;
+    {3,3};
 hello_version(Versions) ->
     lowest_protocol_version(Versions).
 
diff --git a/lib/ssl/test/tls_1_3_version_SUITE.erl b/lib/ssl/test/tls_1_3_version_SUITE.erl
index f0b224d4e5..e0ac53e0f9 100644
--- a/lib/ssl/test/tls_1_3_version_SUITE.erl
+++ b/lib/ssl/test/tls_1_3_version_SUITE.erl
@@ -50,8 +50,14 @@ cert_groups() ->
 tests() ->
     [tls13_client_tls12_server,
      tls13_client_with_ext_tls12_server,
-     tls12_client_tls13_server].
-    
+     tls12_client_tls13_server,
+     tls_client_tls10_server,
+     tls_client_tls11_server,
+     tls_client_tls12_server,
+     tls10_client_tls_server,
+     tls11_client_tls_server,
+     tls12_client_tls_server].
+
 init_per_suite(Config) ->
     catch crypto:stop(),
     try crypto:start() of
@@ -150,4 +156,63 @@ tls12_client_tls13_server(Config) when is_list(Config) ->
     ServerOpts =  [{versions,
                    ['tlsv1.3', 'tlsv1.2']} | ssl_test_lib:ssl_options(server_cert_opts, Config)],
     ssl_test_lib:basic_test(ClientOpts, ServerOpts, Config).
-   
+
+tls_client_tls10_server() ->
+    [{doc,"Test that a TLS 1.0-1.3 client can connect to a TLS 1.0 server."}].
+tls_client_tls10_server(Config) when is_list(Config) ->
+    ClientOpts = [{versions,
+                   ['tlsv1', 'tlsv1.1', 'tlsv1.2', 'tlsv1.3']} |
+                  ssl_test_lib:ssl_options(client_cert_opts, Config)],
+    ServerOpts =  [{versions,
+                   ['tlsv1']} | ssl_test_lib:ssl_options(server_cert_opts, Config)],
+    ssl_test_lib:basic_test(ClientOpts, ServerOpts, Config).
+
+tls_client_tls11_server() ->
+    [{doc,"Test that a TLS 1.0-1.3 client can connect to a TLS 1.1 server."}].
+tls_client_tls11_server(Config) when is_list(Config) ->
+    ClientOpts = [{versions,
+                   ['tlsv1', 'tlsv1.1', 'tlsv1.2', 'tlsv1.3']} |
+                  ssl_test_lib:ssl_options(client_cert_opts, Config)],
+    ServerOpts =  [{versions,
+                   ['tlsv1.1']} | ssl_test_lib:ssl_options(server_cert_opts, Config)],
+    ssl_test_lib:basic_test(ClientOpts, ServerOpts, Config).
+
+tls_client_tls12_server() ->
+    [{doc,"Test that a TLS 1.0-1.3 client can connect to a TLS 1.2 server."}].
+tls_client_tls12_server(Config) when is_list(Config) ->
+    ClientOpts = [{versions,
+                   ['tlsv1', 'tlsv1.1', 'tlsv1.2', 'tlsv1.3']} |
+                  ssl_test_lib:ssl_options(client_cert_opts, Config)],
+    ServerOpts =  [{versions,
+                   ['tlsv1.2']} | ssl_test_lib:ssl_options(server_cert_opts, Config)],
+    ssl_test_lib:basic_test(ClientOpts, ServerOpts, Config).
+
+tls10_client_tls_server() ->
+    [{doc,"Test that a TLS 1.0 client can connect to a TLS 1.0-1.3 server."}].
+tls10_client_tls_server(Config) when is_list(Config) ->
+    ClientOpts = [{versions,
+                   ['tlsv1']} | ssl_test_lib:ssl_options(client_cert_opts, Config)],
+    ServerOpts =  [{versions,
+                   ['tlsv1','tlsv1.1', 'tlsv1.2', 'tlsv1.3']} |
+                   ssl_test_lib:ssl_options(server_cert_opts, Config)],
+    ssl_test_lib:basic_test(ClientOpts, ServerOpts, Config).
+
+tls11_client_tls_server() ->
+    [{doc,"Test that a TLS 1.1 client can connect to a TLS 1.0-1.3 server."}].
+tls11_client_tls_server(Config) when is_list(Config) ->
+    ClientOpts = [{versions,
+                   ['tlsv1.1']} | ssl_test_lib:ssl_options(client_cert_opts, Config)],
+    ServerOpts =  [{versions,
+                   ['tlsv1','tlsv1.1', 'tlsv1.2', 'tlsv1.3']} |
+                   ssl_test_lib:ssl_options(server_cert_opts, Config)],
+    ssl_test_lib:basic_test(ClientOpts, ServerOpts, Config).
+
+tls12_client_tls_server() ->
+    [{doc,"Test that a TLS 1.2 client can connect to a TLS 1.0-1.3 server."}].
+tls12_client_tls_server(Config) when is_list(Config) ->
+    ClientOpts = [{versions,
+                   ['tlsv1.2']} | ssl_test_lib:ssl_options(client_cert_opts, Config)],
+    ServerOpts =  [{versions,
+                   ['tlsv1','tlsv1.1', 'tlsv1.2', 'tlsv1.3']} |
+                   ssl_test_lib:ssl_options(server_cert_opts, Config)],
+    ssl_test_lib:basic_test(ClientOpts, ServerOpts, Config).