From 9284bc84039794cb732c8fe593b129b4623d79c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Dimitrov?= Date: Wed, 7 Nov 2018 09:51:01 +0100 Subject: 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 --- lib/ssl/src/tls_connection.erl | 7 +++---- lib/ssl/src/tls_record.erl | 10 +++++----- lib/ssl/src/tls_record_1_3.erl | 39 +++++++++++++++++++++++++++++++++------ lib/ssl/src/tls_v1.erl | 2 +- 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(<>, 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(<>, 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, -- cgit v1.2.3