From 5e48b34f684c837bb93980c96740c16663813fdb Mon Sep 17 00:00:00 2001
From: Ingela Anderton Andin <ingela@erlang.org>
Date: Thu, 21 Apr 2022 13:06:15 +0200
Subject: ssl: Fix version mismatch

The special case handled by the tls_gen_connection:effective_version
function is only valid in the hello and wait_sh state.

Closes #5835
---
 lib/ssl/src/ssl_handshake.erl      | 11 +++++------
 lib/ssl/src/tls_gen_connection.erl | 29 +++++++++++++++++------------
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl
index 837f7c0249..ee102077af 100644
--- a/lib/ssl/src/ssl_handshake.erl
+++ b/lib/ssl/src/ssl_handshake.erl
@@ -863,14 +863,13 @@ decode_handshake(_Version, ?CERTIFICATE_STATUS, <<?BYTE(?CERTIFICATE_STATUS_TYPE
         response = ASN1OcspResponse};
 decode_handshake(_Version, ?SERVER_KEY_EXCHANGE, Keys) ->
     #server_key_exchange{exchange_keys = Keys};
-decode_handshake({Major, Minor} = Version, ?CERTIFICATE_REQUEST,
-       <<?BYTE(CertTypesLen), CertTypes:CertTypesLen/binary,
-	?UINT16(HashSignsLen), HashSigns:HashSignsLen/binary,
-         ?UINT16(CertAuthsLen), CertAuths:CertAuthsLen/binary>>)
-  when Major >= 3, Minor >= 3 ->
+decode_handshake({3, 3} = Version, ?CERTIFICATE_REQUEST,
+                 <<?BYTE(CertTypesLen), CertTypes:CertTypesLen/binary,
+                   ?UINT16(HashSignsLen), HashSigns:HashSignsLen/binary,
+                   ?UINT16(CertAuthsLen), CertAuths:CertAuthsLen/binary>>) ->
     HashSignAlgos = decode_sign_alg(Version, HashSigns),
     #certificate_request{certificate_types = CertTypes,
-			 hashsign_algorithms = #hash_sign_algos{hash_sign_algos = HashSignAlgos},
+                         hashsign_algorithms = #hash_sign_algos{hash_sign_algos = HashSignAlgos},
 			 certificate_authorities = CertAuths};
 decode_handshake(_Version, ?CERTIFICATE_REQUEST,
        <<?BYTE(CertTypesLen), CertTypes:CertTypesLen/binary,
diff --git a/lib/ssl/src/tls_gen_connection.erl b/lib/ssl/src/tls_gen_connection.erl
index 89cc945e21..f5ec0832ad 100644
--- a/lib/ssl/src/tls_gen_connection.erl
+++ b/lib/ssl/src/tls_gen_connection.erl
@@ -398,7 +398,7 @@ handle_protocol_record(#ssl_tls{type = ?HANDSHAKE, fragment = Data},
     try
 	%% Calculate the effective version that should be used when decoding an incoming handshake
 	%% message.
-	EffectiveVersion = effective_version(Version, Options, Role),
+	EffectiveVersion = effective_version(Version, Options, Role, StateName),
 	{Packets, Buf} = tls_handshake:get_tls_handshake(EffectiveVersion,Data,Buf0, Options),
 	State =
 	    State0#state{protocol_buffers =
@@ -751,19 +751,24 @@ next_record_done(#state{protocol_buffers = Buffers} = State, CipherTexts, Connec
      State#state{protocol_buffers = Buffers#protocol_buffers{tls_cipher_texts = CipherTexts},
                  connection_states = ConnectionStates}}.
 
-%% 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} ->
+
+%% Pre TLS-1.3, on the client side, the connection state variable negotiated_version will initialy be
+%% the requested version. On the server side the the variable is intially undefined.
+%% When the client can support TLS-1.3 and one or more prior versions and we are waiting
+%% for the server hello (with or without a RetryRequest, that is in state hello or in state wait_sh),
+%% the "initial requested version" keeped in the connection state variable negotiated_version
+%% (before the versions is actually negotiated) will always be the value of TLS-1.2 (which is a legacy
+%% filed in TLS-1.3 client hello and versions are negotiated with an hello extension), but when
+%% decoding the server_hello messages we want to go through TLS-1.3 decode functions to be able
+%% to handle TLS-1.3 extensions if TLS-1.3 will be the negotiated version.
+effective_version({3,3} , #{versions := [{3,4} = Version |_]}, client, StateName) when StateName == hello;
+                                                                                       StateName == wait_sh ->
     Version;
-%% Use highest supported version during startup (TLS server, all versions).
-effective_version(undefined, #{versions := [Version|_]}, _) ->
+%% When the negotiated_version variable is not yet set use the highest supported version.
+effective_version(undefined, #{versions := [Version|_]}, _, _) ->
     Version;
-%% Use negotiated version in all other cases.
-effective_version(Version, _, _) ->
+%% In all other cases use version saved in the connection state variable negotiated_version
+effective_version(Version, _, _, _) ->
     Version.
 
 assert_buffer_sanity(<<?BYTE(_Type), ?UINT24(Length), Rest/binary>>, 
-- 
cgit v1.2.1