ssl: Improve AEAD encode/decode

- Update calculation of nonce and additional data
- Update cipher_aead, decipher_aead
- Add test for TLS 1.3 encode/decode

Change-Id: Id0a5cc68d8746079fb42c0192c0c64405f6d7a72

4 files changed, 143 insertions, 113 deletions

diff --git a/lib/ssl/src/ssl_cipher.erl b/lib/ssl/src/ssl_cipher.erl
index bf64ed8b69..4b975d753b 100644
--- a/lib/ssl/src/ssl_cipher.erl
+++ b/lib/ssl/src/ssl_cipher.erl
@@ -44,7 +44,8 @@
 	 hash_algorithm/1, sign_algorithm/1, is_acceptable_hash/2, is_fallback/1,
 	 random_bytes/1, calc_mac_hash/4,
          is_stream_ciphersuite/1, signature_scheme/1,
-         scheme_to_components/1, hash_size/1, effective_key_bits/1]).
+         scheme_to_components/1, hash_size/1, effective_key_bits/1,
+         key_material/1]).
 
 %% RFC 8446 TLS 1.3
 -export([generate_client_shares/1, generate_server_share/1, add_zero_padding/2]).
diff --git a/lib/ssl/src/ssl_record.erl b/lib/ssl/src/ssl_record.erl
index ddc83821b4..499ba108f2 100644
--- a/lib/ssl/src/ssl_record.erl
+++ b/lib/ssl/src/ssl_record.erl
@@ -39,7 +39,8 @@
 	 set_renegotiation_flag/2,
 	 set_client_verify_data/3,
 	 set_server_verify_data/3,
-	 empty_connection_state/2, initial_connection_state/2, record_protocol_role/1]).
+	 empty_connection_state/2, initial_connection_state/2, record_protocol_role/1,
+         step_encryption_state/1]).
 
 %% Compression
 -export([compress/3, uncompress/3, compressions/0]).
@@ -118,6 +119,20 @@ activate_pending_connection_state(#{current_write := Current,
 	   }.
 
 %%--------------------------------------------------------------------
+-spec step_encryption_state(connection_states()) -> connection_states().
+%%
+%% Description: Activates the next encyrption state (e.g. handshake
+%% encryption).
+%%--------------------------------------------------------------------
+step_encryption_state(#{pending_read := PendingRead,
+                        pending_write := PendingWrite} = States) ->
+    NewRead = PendingRead#{sequence_number => 0},
+    NewWrite = PendingWrite#{sequence_number => 0},
+    States#{current_read => NewRead,
+            current_write => NewWrite}.
+
+
+%%--------------------------------------------------------------------
 -spec set_security_params(#security_parameters{}, #security_parameters{},
 			  connection_states()) -> connection_states().
 %%
diff --git a/lib/ssl/src/tls_handshake_1_3.erl b/lib/ssl/src/tls_handshake_1_3.erl
index 2c8910cefa..670c4d424d 100644
--- a/lib/ssl/src/tls_handshake_1_3.erl
+++ b/lib/ssl/src/tls_handshake_1_3.erl
@@ -388,14 +388,24 @@ do_negotiated(#{client_share := ClientKey,
         tls_connection:send(Transport, Socket, BinMsg),
         log_handshake(SslOpts, ServerHello),
         log_tls_record(SslOpts, BinMsg),
-        ConnectionStates2 = calculate_security_parameters(ClientKey, SelectedGroup, KeyShare,
-                                                          HHistory1, ConnectionStates1),
+
+        %% ConnectionStates2 = calculate_security_parameters(ClientKey, SelectedGroup, KeyShare,
+        %%                                                   HHistory1, ConnectionStates1),
+        {HandshakeSecret, ReadKey, ReadIV, WriteKey, WriteIV} =
+            calculate_security_parameters(ClientKey, SelectedGroup, KeyShare,
+                                          HHistory1, ConnectionStates1),
+        ConnectionStates2 =
+            update_pending_connection_states(ConnectionStates1, HandshakeSecret,
+                                             ReadKey, ReadIV, WriteKey, WriteIV),
+        ConnectionStates3 =
+            ssl_record:step_encryption_state(ConnectionStates2),
+
         %% Create Certificate
         Certificate = certificate(OwnCert, CertDbHandle, CertDbRef, <<>>, server),
 
         %% Encode Certificate
-        {_, _ConnectionStates3, HHistory2} =
-            tls_connection:encode_handshake(Certificate, {3,4}, ConnectionStates2, HHistory1),
+        {_, _ConnectionStates4, HHistory2} =
+            tls_connection:encode_handshake(Certificate, {3,4}, ConnectionStates3, HHistory1),
         %% log_handshake(SslOpts, Certificate),
 
         %% Create CertificateVerify
@@ -415,19 +425,19 @@ do_negotiated(#{client_share := ClientKey,
 
         %% Next record/Next event
 
-        Maybe(not_implemented())
+        Maybe(not_implemented(negotiated))
 
 
     catch
-        {Ref, {state_not_implemented, negotiated}} ->
+        {Ref, {state_not_implemented, State}} ->
             %% TODO
-            ?ALERT_REC(?FATAL, ?INTERNAL_ERROR, "state not implemented")
+            ?ALERT_REC(?FATAL, ?INTERNAL_ERROR, {state_not_implemented, State})
     end.
 
 
 %% TODO: Remove this function!
-not_implemented() ->
-    {error, negotiated}.
+not_implemented(State) ->
+    {error, {state_not_implemented, State}}.
 
 
 log_handshake(SslOpts, Message) ->
@@ -452,7 +462,6 @@ calculate_security_parameters(ClientKey, SelectedGroup, KeyShare, HHistory, Conn
 
     %% Calculate handshake_secret
     EarlySecret = tls_v1:key_schedule(early_secret, HKDFAlgo , {psk, <<>>}),
-
     PrivateKey = get_server_private_key(KeyShare),  %% #'ECPrivateKey'{}
 
     IKM = calculate_shared_secret(ClientKey, PrivateKey, SelectedGroup),
@@ -470,20 +479,22 @@ calculate_security_parameters(ClientKey, SelectedGroup, KeyShare, HHistory, Conn
     {ReadKey, ReadIV} = tls_v1:calculate_traffic_keys(HKDFAlgo, Cipher, ClientHSTrafficSecret),
     {WriteKey, WriteIV} = tls_v1:calculate_traffic_keys(HKDFAlgo, Cipher, ServerHSTrafficSecret),
 
-    %% Update pending connection state
-    PendingRead0 = ssl_record:pending_connection_state(ConnectionStates, read),
-    PendingWrite0 = ssl_record:pending_connection_state(ConnectionStates, write),
+    {HandshakeSecret, ReadKey, ReadIV, WriteKey, WriteIV}.
 
-    PendingRead = update_conn_state(PendingRead0, HandshakeSecret, ReadKey, ReadIV),
-    PendingWrite = update_conn_state(PendingWrite0, HandshakeSecret, WriteKey, WriteIV),
+    %% %% Update pending connection state
+    %% PendingRead0 = ssl_record:pending_connection_state(ConnectionStates, read),
+    %% PendingWrite0 = ssl_record:pending_connection_state(ConnectionStates, write),
 
-    %% Update pending and copy to current (activate)
-    %% All subsequent handshake messages are encrypted
-    %% ([sender]_handshake_traffic_secret)
-    #{current_read => PendingRead,
-      current_write => PendingWrite,
-      pending_read => PendingRead,
-      pending_write => PendingWrite}.
+    %% PendingRead = update_conn_state(PendingRead0, HandshakeSecret, ReadKey, ReadIV),
+    %% PendingWrite = update_conn_state(PendingWrite0, HandshakeSecret, WriteKey, WriteIV),
+
+    %% %% Update pending and copy to current (activate)
+    %% %% All subsequent handshake messages are encrypted
+    %% %% ([sender]_handshake_traffic_secret)
+    %% #{current_read => PendingRead,
+    %%   current_write => PendingWrite,
+    %%   pending_read => PendingRead,
+    %%   pending_write => PendingWrite}.
 
 
 get_server_private_key(#key_share_server_hello{server_share = ServerShare}) ->
@@ -516,8 +527,16 @@ calculate_shared_secret(OthersKey, MyKey = #'ECPrivateKey'{}, _Group)
     public_key:compute_key(Point, MyKey).
 
 
-update_conn_state(ConnectionState = #{security_parameters := SecurityParameters0},
-                  HandshakeSecret, Key, IV) ->
+update_pending_connection_states(CS = #{pending_read := PendingRead0,
+                                pending_write := PendingWrite0},
+                         HandshakeSecret, ReadKey, ReadIV, WriteKey, WriteIV) ->
+    PendingRead = update_connection_state(PendingRead0, HandshakeSecret, ReadKey, ReadIV),
+    PendingWrite = update_connection_state(PendingWrite0, HandshakeSecret, WriteKey, WriteIV),
+    CS#{pending_read => PendingRead,
+        pending_write => PendingWrite}.
+
+update_connection_state(ConnectionState = #{security_parameters := SecurityParameters0},
+                        HandshakeSecret, Key, IV) ->
     %% Store secret
     SecurityParameters = SecurityParameters0#security_parameters{
                            master_secret = HandshakeSecret},
@@ -525,6 +544,7 @@ update_conn_state(ConnectionState = #{security_parameters := SecurityParameters0
                      cipher_state => cipher_init(Key, IV)}.
 
 
+
 cipher_init(Key, IV) ->
     #cipher_state{key = Key, iv = IV, tag_len = 16}.
 
diff --git a/lib/ssl/src/tls_record_1_3.erl b/lib/ssl/src/tls_record_1_3.erl
index d424336187..1681babed9 100644
--- a/lib/ssl/src/tls_record_1_3.erl
+++ b/lib/ssl/src/tls_record_1_3.erl
@@ -76,8 +76,8 @@ encode_data(Frag, ConnectionStates) ->
 encode_plain_text(Type, Data0, #{current_write := Write0} = ConnectionStates) ->
     PadLen = 0, %% TODO where to specify PadLen?
     Data = inner_plaintext(Type, Data0, PadLen),
-    {CipherFragment, Write1} = encode_plain_text(Data, Write0),
-    {CipherText, Write} = encode_tls_cipher_text(CipherFragment, Write1),
+    CipherFragment = encode_plain_text(Data, Write0),
+    {CipherText, Write} = encode_tls_cipher_text(CipherFragment, Write0),
     {CipherText, ConnectionStates#{current_write => Write}}.
 
 encode_iolist(Type, Data, ConnectionStates0) ->
@@ -105,24 +105,23 @@ decode_cipher_text(#ssl_tls{type = ?OPAQUE_TYPE,
                             fragment = CipherFragment},
 		   #{current_read :=
 			 #{sequence_number := Seq,
-                           cipher_state := CipherS0,
+                           cipher_state := #cipher_state{key = Key,
+                                                         iv = IV,
+                                                         tag_len = TagLen},
 			   security_parameters :=
 			       #security_parameters{
 				  cipher_type = ?AEAD,
                                   bulk_cipher_algorithm =
                                       BulkCipherAlgo}
 			  } = ReadState0} = ConnectionStates0) ->
-    AAD = start_additional_data(),
-    CipherS1 = ssl_cipher:nonce_seed(<<?UINT64(Seq)>>, CipherS0),
-    case decipher_aead(BulkCipherAlgo, CipherS1, AAD, CipherFragment) of
-	{PlainFragment, CipherS1} ->
+    case decipher_aead(CipherFragment, BulkCipherAlgo, Key, Seq, IV, TagLen) of
+	#alert{} = Alert ->
+	    Alert;
+	PlainFragment ->
 	    ConnectionStates =
                 ConnectionStates0#{current_read =>
-                                        ReadState0#{cipher_state => CipherS1,
-                                                    sequence_number => Seq + 1}},
-	    decode_inner_plaintext(PlainFragment, ConnectionStates);
-	#alert{} = Alert ->
-	    Alert
+                                       ReadState0#{sequence_number => Seq + 1}},
+	    {decode_inner_plaintext(PlainFragment), ConnectionStates}
     end;
 decode_cipher_text(#ssl_tls{type = Type,
                             version = ?LEGACY_VERSION,
@@ -137,7 +136,7 @@ decode_cipher_text(#ssl_tls{type = Type,
               fragment = CipherFragment}, ConnnectionStates0};
 decode_cipher_text(#ssl_tls{type = Type}, _) ->
     %% Version mismatch is already asserted
-    ?ALERT_REC(?FATAL, ?BAD_RECORD_MAC, {record_typ_mismatch, Type}).
+    ?ALERT_REC(?FATAL, ?BAD_RECORD_MAC, {record_type_mismatch, Type}).
 
 %%--------------------------------------------------------------------
 %%% Internal functions
@@ -170,62 +169,61 @@ encode_plain_text(#inner_plaintext{
                      content = Data,
                      type = Type,
                      zeros = Zeros
-                    }, #{cipher_state := CipherS0,
+                    }, #{cipher_state := #cipher_state{key= Key,
+                                                       iv = IV,
+                                                       tag_len = TagLen},
                          sequence_number := Seq,
                          security_parameters :=
                              #security_parameters{
-                                cipher_type = ?AEAD}
-                        } = WriteState0) ->
-    PlainText = <<Data/binary, ?BYTE(Type), Zeros/binary>>,
-    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{opaque_type = Type,
-                      legacy_version = {3,3},
-                      encoded_record = Encoded}, WriteState};
+                                cipher_type = ?AEAD,
+                                bulk_cipher_algorithm = BulkCipherAlgo}
+                        }) ->
+    PlainText = [Data, Type, Zeros],
+    Encoded = cipher_aead(PlainText, BulkCipherAlgo, Key, Seq, IV, TagLen),
+    #tls_cipher_text{opaque_type = 23,  %% 23 (application_data) for outward compatibility
+                     legacy_version = {3,3},
+                     encoded_record = Encoded};
 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,
+    #tls_cipher_text{opaque_type = Type,
                       legacy_version = {3,3},
-                      encoded_record = Data}, WriteState0};
+                      encoded_record = Data};
 
 encode_plain_text(_, CS) ->
     exit({cs, CS}).
 
-start_additional_data() ->
-    {MajVer, MinVer} = ?LEGACY_VERSION,
-    <<?BYTE(?OPAQUE_TYPE), ?BYTE(MajVer), ?BYTE(MinVer)>>.
-
-end_additional_data(AAD, Len) ->
-    <<AAD/binary, ?UINT16(Len)>>.
-
-nonce(#cipher_state{nonce = Nonce, iv = IV}) ->
-    Len = size(IV),
-    crypto:exor(<<Nonce:Len/bytes>>, IV).
+additional_data(Length) ->
+    <<?BYTE(?OPAQUE_TYPE), ?BYTE(3), ?BYTE(3),?UINT16(Length)>>.
 
-cipher_aead(Fragment,
-	    #{cipher_state := CipherS0,
-	      security_parameters :=
-		  #security_parameters{bulk_cipher_algorithm =
-					   BulkCipherAlgo}
-	     } = WriteState0, AAD) ->
-    {CipherFragment, CipherS1} =
-	cipher_aead(BulkCipherAlgo, CipherS0, AAD, Fragment),
-    {CipherFragment,  WriteState0#{cipher_state => CipherS1}}.
+%% The per-record nonce for the AEAD construction is formed as
+%% follows:
+%%
+%% 1.  The 64-bit record sequence number is encoded in network byte
+%%     order and padded to the left with zeros to iv_length.
+%%
+%% 2.  The padded sequence number is XORed with either the static
+%%     client_write_iv or server_write_iv (depending on the role).
+%%
+%% The resulting quantity (of length iv_length) is used as the
+%% per-record nonce.
+nonce(Seq, IV) ->
+    Padding = binary:copy(<<0>>, byte_size(IV) - 8),
+    crypto:exor(<<Padding/binary,?UINT64(Seq)>>, IV).
 
-cipher_aead(Type, #cipher_state{key=Key} = CipherState, AAD0, Fragment) ->
-    AAD = end_additional_data(AAD0, erlang:iolist_size(Fragment)),
-    Nonce = nonce(CipherState),
-    {Content, CipherTag} = ssl_cipher:aead_encrypt(Type, Key, Nonce, Fragment, AAD),
-    {<<Content/binary, CipherTag/binary>>, CipherState}.
+cipher_aead(Fragment, BulkCipherAlgo, Key, Seq, IV, TagLen) ->
+    AAD = additional_data(erlang:iolist_size(Fragment) + TagLen),
+    Nonce = nonce(Seq, IV),
+    {Content, CipherTag} =
+        ssl_cipher:aead_encrypt(BulkCipherAlgo, Key, Nonce, Fragment, AAD),
+    <<Content/binary, CipherTag/binary>>.
 
 encode_tls_cipher_text(#tls_cipher_text{opaque_type = Type,
                                         legacy_version = {MajVer, MinVer},
@@ -234,13 +232,14 @@ encode_tls_cipher_text(#tls_cipher_text{opaque_type = Type,
     {[<<?BYTE(Type), ?BYTE(MajVer), ?BYTE(MinVer), ?UINT16(Length)>>, Encoded],
      Write#{sequence_number => Seq +1}}.
 
-decipher_aead(Type, #cipher_state{key = Key} = CipherState, AAD0, CipherFragment) ->
+decipher_aead(CipherFragment, BulkCipherAlgo, Key, Seq, IV, TagLen) ->
     try
-        Nonce = nonce(CipherState),
-        {AAD, CipherText, CipherTag} = aead_ciphertext_split(CipherState, CipherFragment, AAD0),
-	case ssl_cipher:aead_decrypt(Type, Key, Nonce, CipherText, CipherTag, AAD) of
+        AAD = additional_data(erlang:iolist_size(CipherFragment)),
+        Nonce = nonce(Seq, IV),
+        {CipherText, CipherTag} = aead_ciphertext_split(CipherFragment, TagLen),
+	case ssl_cipher:aead_decrypt(BulkCipherAlgo, Key, Nonce, CipherText, CipherTag, AAD) of
 	    Content when is_binary(Content) ->
-		{Content, CipherState};
+		Content;
 	    _ ->
                 ?ALERT_REC(?FATAL, ?BAD_RECORD_MAC, decryption_failed)
 	end
@@ -249,39 +248,34 @@ decipher_aead(Type, #cipher_state{key = Key} = CipherState, AAD0, CipherFragment
             ?ALERT_REC(?FATAL, ?BAD_RECORD_MAC, decryption_failed)
     end.
 
-aead_ciphertext_split(#cipher_state{tag_len = Len}, CipherTextFragment, AAD) ->
-    CipherLen = size(CipherTextFragment) - Len,
-    <<CipherText:CipherLen/bytes, CipherTag:Len/bytes>> = CipherTextFragment,
-    {end_additional_data(AAD, CipherLen), CipherText, CipherTag}.
 
-decode_inner_plaintext(PlainText, ConnnectionStates) ->
-    case remove_padding(PlainText) of
-        #alert{} = Alert ->
-            Alert;
-        {Data, Type} ->
-            {#ssl_tls{type = Type,
-                      version = {3,4}, %% Internally use real version
-                      fragment = Data}, ConnnectionStates}
-    end.
+aead_ciphertext_split(CipherTextFragment, TagLen)
+  when is_binary(CipherTextFragment) ->
+    CipherLen = erlang:byte_size(CipherTextFragment) - TagLen,
+    <<CipherText:CipherLen/bytes, CipherTag:TagLen/bytes>> = CipherTextFragment,
+    {CipherText, CipherTag};
+aead_ciphertext_split(CipherTextFragment, TagLen)
+  when is_list(CipherTextFragment) ->
+    CipherLen = erlang:iolist_size(CipherTextFragment) - TagLen,
+    <<CipherText:CipherLen/bytes, CipherTag:TagLen/bytes>> =
+        erlang:iolist_to_binary(CipherTextFragment),
+    {CipherText, CipherTag}.
 
-remove_padding(PlainText)->
-    case binary:split(PlainText, <<0>>,  [global, trim]) of
-        [] ->
-            ?ALERT_REC(?FATAL, ?UNEXPECTED_MESSAGE, padding_error);
-        [Content] ->
-            Type = binary:last(Content),
-            split_content(Type, Content, erlang:byte_size(Content) - 1)
+decode_inner_plaintext(PlainText) ->
+    case binary:last(PlainText) of
+        0 ->
+            decode_inner_plaintext(init_binary(PlainText));
+        Type when Type =:= ?APPLICATION_DATA orelse
+                  Type =:= ?HANDSHAKE orelse
+                  Type =:= ?ALERT ->
+            #ssl_tls{type = Type,
+                     version = {3,4}, %% Internally use real version
+                     fragment = init_binary(PlainText)};
+        _Else ->
+            ?ALERT_REC(?FATAL, ?UNEXPECTED_MESSAGE, empty_alert)
     end.
 
-split_content(?HANDSHAKE, _, 0) ->
-    ?ALERT_REC(?FATAL, ?UNEXPECTED_MESSAGE, empty_handshake);
-split_content(?ALERT, _, 0) ->
-    ?ALERT_REC(?FATAL, ?UNEXPECTED_MESSAGE, empty_alert);
-%% For special middlebox compatible case!
-split_content(?CHANGE_CIPHER_SPEC, _, 0) ->
-    ?ALERT_REC(?FATAL, ?UNEXPECTED_MESSAGE, empty_change_cipher_spec);
-split_content(?APPLICATION_DATA = Type, _, 0) ->
-    {Type, <<>>};
-split_content(Type, Content, N) ->
-    <<Data:N/bytes, ?BYTE(Type)>> = Content, 
-    {Type, Data}.
+init_binary(B) ->
+    {Init, _} =
+        split_binary(B, byte_size(B) - 1),
+    Init.