ssl: Fix encode/decode of ClientHello (TLS 1.3)

- Fix handling of hello versions. TLS 1.3 ClientHello will use
  TLS 1.3 encoding.

- Fix encoding/decoding of TLS records when record protection
  has not yet been engaged (NULL cipher).

Change-Id: I7511d1a7751f1ec8c2f2f2fb3d21ddf80a3f428b

4 files changed, 42 insertions, 16 deletions

diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl
index 5de1424414..9f98572691 100644
--- a/lib/ssl/src/tls_connection.erl
+++ b/lib/ssl/src/tls_connection.erl
@@ -502,9 +502,8 @@ init({call, From}, {start, Timeout},
     Timer = ssl_connection:start_or_recv_cancel_timer(Timeout, From),
     Hello = tls_handshake:client_hello(Host, Port, ConnectionStates0, SslOpts,
 				       Cache, CacheCb, Renegotiation, Cert, KeyShare),
-    
-    Version = Hello#client_hello.client_version,
-    HelloVersion = tls_record:hello_version(Version, SslOpts#ssl_options.versions),
+
+    HelloVersion = tls_record:hello_version(SslOpts#ssl_options.versions),
     Handshake0 = ssl_handshake:init_handshake_history(),
     {BinMsg, ConnectionStates, Handshake} =
         encode_handshake(Hello,  HelloVersion, ConnectionStates0, Handshake0),
@@ -518,7 +517,7 @@ init({call, From}, {start, Timeout},
     ssl_logger:debug(SslOpts#ssl_options.log_level, HelloMsg, #{domain => [otp,ssl,handshake]}),
     ssl_logger:debug(SslOpts#ssl_options.log_level, Report, #{domain => [otp,ssl,tls_record]}),
     State1 = State0#state{connection_states = ConnectionStates,
-			  negotiated_version = Version, %% Requested version
+			  negotiated_version = HelloVersion, %% Requested version
 			  session =
 			      Session0#session{session_id = Hello#client_hello.session_id},
 			  tls_handshake_history = Handshake,
diff --git a/lib/ssl/src/tls_record.erl b/lib/ssl/src/tls_record.erl
index 7debac7d37..50fad2e680 100644
--- a/lib/ssl/src/tls_record.erl
+++ b/lib/ssl/src/tls_record.erl
@@ -47,7 +47,7 @@
 -export([protocol_version/1,  lowest_protocol_version/1, lowest_protocol_version/2,
 	 highest_protocol_version/1, highest_protocol_version/2,
 	 is_higher/2, supported_protocol_versions/0,
-	 is_acceptable_version/1, is_acceptable_version/2, hello_version/2]).
+	 is_acceptable_version/1, is_acceptable_version/2, hello_version/1]).
 
 -export_type([tls_version/0, tls_atom_version/0]).
 
@@ -386,10 +386,10 @@ is_acceptable_version({N,_} = Version, Versions)
 is_acceptable_version(_,_) ->
     false.
 
--spec hello_version(tls_version(), [tls_version()]) -> tls_version().
-hello_version(Version, _) when Version >= {3, 3} ->
-    Version;
-hello_version(_, Versions) ->
+-spec hello_version([tls_version()]) -> tls_version().
+hello_version([Highest|_]) when Highest >= {3,3} ->
+    Highest;
+hello_version(Versions) ->
     lowest_protocol_version(Versions).
 
 %%--------------------------------------------------------------------
diff --git a/lib/ssl/src/tls_record_1_3.erl b/lib/ssl/src/tls_record_1_3.erl
index ff198a09bf..d424336187 100644
--- a/lib/ssl/src/tls_record_1_3.erl
+++ b/lib/ssl/src/tls_record_1_3.erl
@@ -111,21 +111,32 @@ decode_cipher_text(#ssl_tls{type = ?OPAQUE_TYPE,
 				  cipher_type = ?AEAD,
                                   bulk_cipher_algorithm =
                                       BulkCipherAlgo}
-			  } = ReadState0} = ConnnectionStates0) ->
+			  } = ReadState0} = ConnectionStates0) ->
     AAD = start_additional_data(),
     CipherS1 = ssl_cipher:nonce_seed(<<?UINT64(Seq)>>, CipherS0),
     case decipher_aead(BulkCipherAlgo, CipherS1, AAD, CipherFragment) of
 	{PlainFragment, CipherS1} ->
-	    ConnnectionStates = 
-                ConnnectionStates0#{current_read => 
+	    ConnectionStates =
+                ConnectionStates0#{current_read =>
                                         ReadState0#{cipher_state => CipherS1,
                                                     sequence_number => Seq + 1}},
-	    decode_inner_plaintext(PlainFragment, ConnnectionStates);
+	    decode_inner_plaintext(PlainFragment, ConnectionStates);
 	#alert{} = Alert ->
 	    Alert
     end;
+decode_cipher_text(#ssl_tls{type = Type,
+                            version = ?LEGACY_VERSION,
+                            fragment = CipherFragment},
+		   #{current_read :=
+			 #{security_parameters :=
+			       #security_parameters{
+                                  cipher_suite = ?TLS_NULL_WITH_NULL_NULL}
+			  }} = ConnnectionStates0) ->
+    {#ssl_tls{type = Type,
+              version = {3,4}, %% Internally use real version
+              fragment = CipherFragment}, ConnnectionStates0};
 decode_cipher_text(#ssl_tls{type = Type}, _) ->
-    %% Version mismatch is already asserted  
+    %% Version mismatch is already asserted
     ?ALERT_REC(?FATAL, ?BAD_RECORD_MAC, {record_typ_mismatch, Type}).
 
 %%--------------------------------------------------------------------
@@ -169,7 +180,23 @@ encode_plain_text(#inner_plaintext{
     AAD = start_additional_data(),
     CipherS1 = ssl_cipher:nonce_seed(<<?UINT64(Seq)>>, CipherS0),
     {Encoded, WriteState} = cipher_aead(PlainText, WriteState0#{cipher_state => CipherS1}, AAD),
-    {#tls_cipher_text{encoded_record = Encoded}, WriteState};
+    {#tls_cipher_text{opaque_type = Type,
+                      legacy_version = {3,3},
+                      encoded_record = Encoded}, WriteState};
+encode_plain_text(#inner_plaintext{
+                     content = Data,
+                     type = Type
+                    }, #{security_parameters :=
+                             #security_parameters{
+                                cipher_suite = ?TLS_NULL_WITH_NULL_NULL}
+                        } = WriteState0) ->
+    %% RFC8446 - 5.1.  Record Layer
+    %% When record protection has not yet been engaged, TLSPlaintext
+    %% structures are written directly onto the wire.
+    {#tls_cipher_text{opaque_type = Type,
+                      legacy_version = {3,3},
+                      encoded_record = Data}, WriteState0};
+
 encode_plain_text(_, CS) ->
     exit({cs, CS}).
 
diff --git a/lib/ssl/src/tls_v1.erl b/lib/ssl/src/tls_v1.erl
index 5665f5310e..8618355089 100644
--- a/lib/ssl/src/tls_v1.erl
+++ b/lib/ssl/src/tls_v1.erl
@@ -551,7 +551,7 @@ ecc_curves(_Minor, TLSCurves) ->
 			end
 		end, [], TLSCurves).
 
--spec groups(4 | all) -> [group()].
+-spec groups(4 | all | default) -> [group()].
 groups(all) ->
     [secp256r1,
      secp384r1,