From 20b65c7bdd9ca34c497624d1d07edd433be88a83 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 15 Nov 2016 11:09:25 +0000 Subject: Fix some TLSProxy warnings After the client processes the server's initial flight in TLS1.3 it may respond with either an encrypted, or an unencrypted alert. We needed to teach TLSProxy about this so that it didn't issue spurious warnings. Reviewed-by: Rich Salz --- util/TLSProxy/Proxy.pm | 12 +++++++++++- util/TLSProxy/Record.pm | 16 ++++++++++++++-- util/TLSProxy/ServerHello.pm | 1 + 3 files changed, 26 insertions(+), 3 deletions(-) (limited to 'util/TLSProxy') diff --git a/util/TLSProxy/Proxy.pm b/util/TLSProxy/Proxy.pm index 16fd09463f..be9f8f88a0 100644 --- a/util/TLSProxy/Proxy.pm +++ b/util/TLSProxy/Proxy.pm @@ -23,6 +23,8 @@ use TLSProxy::NewSessionTicket; my $have_IPv6 = 0; my $IP_factory; +my $is_tls13 = 0; + sub new { my $class = shift; @@ -103,6 +105,7 @@ sub clearClient $self->{record_list} = []; $self->{message_list} = []; $self->{clientflags} = ""; + $is_tls13 = 0; TLSProxy::Message->clear(); TLSProxy::Record->clear(); @@ -503,5 +506,12 @@ sub fill_known_data } return $ret; } - +sub is_tls13 +{ + my $class = shift; + if (@_) { + $is_tls13 = shift; + } + return $is_tls13; +} 1; diff --git a/util/TLSProxy/Record.pm b/util/TLSProxy/Record.pm index 6d35f08bed..7189035fb4 100644 --- a/util/TLSProxy/Record.pm +++ b/util/TLSProxy/Record.pm @@ -111,7 +111,7 @@ sub get_records if (($server && $server_encrypting) || (!$server && $client_encrypting)) { - if ($version != VERS_TLS_1_3() && $etm) { + if (!TLSProxy::Proxy->is_tls13() && $etm) { $record->decryptETM(); } else { $record->decrypt(); @@ -229,7 +229,19 @@ sub decrypt() my $data = $self->data; #Throw away any IVs - if ($self->version >= VERS_TLS_1_3()) { + if (TLSProxy::Proxy->is_tls13()) { + #A TLS1.3 client, when processing the server's initial flight, could + #respond with either an encrypted or an unencrypted alert. + if ($self->content_type() == RT_ALERT) { + #TODO(TLS1.3): Eventually it is sufficient just to check the record + #content type. If an alert is encrypted it will have a record + #content type of application data. However we haven't done the + #record layer changes yet, so it's a bit more complicated. For now + #we will additionally check if the data length is 2 (1 byte for + #alert level, 1 byte for alert description). If it is, then this is + #an unecrypted alert, so don't try to decrypt + return $data if (length($data) == 2); + } #8 bytes for a GCM IV $data = substr($data, 8); $mactaglen = 16; diff --git a/util/TLSProxy/ServerHello.pm b/util/TLSProxy/ServerHello.pm index 9d6ad385bb..a1bc7b3d48 100644 --- a/util/TLSProxy/ServerHello.pm +++ b/util/TLSProxy/ServerHello.pm @@ -98,6 +98,7 @@ sub parse if ($server_version == TLSProxy::Record::VERS_TLS_1_3_DRAFT) { TLSProxy::Record->server_encrypting(1); TLSProxy::Record->client_encrypting(1); + TLSProxy::Proxy->is_tls13(1); } print " Server Version:".$server_version."\n"; -- cgit v1.2.3