[openssl-commits] [openssl] master update

[Matt Caswell]

The branch master has been updated
       via  6c61b2749634246956f8ec7adc9520e5d22dcbf4 (commit)
      from  b8a3f39b890304757058deb730c855b72c14947b (commit)


- Log -----------------------------------------------------------------
commit 6c61b2749634246956f8ec7adc9520e5d22dcbf4
Author: Matt Caswell <matt at openssl.org>
Date:   Wed Feb 14 17:29:32 2018 +0000

    Remove a spurious TLSProxy byte in TLSv1.3
    
    When the proxy re-encrypted a TLSv1.3 record it was adding a spurious
    byte onto the end. This commit removes that.
    
    The "extra" byte was intended to be the inner content type of the record.
    However, TLSProxy was actually adding the original encrypted data into the
    record (which already has the inner content type in it) and then adding
    the spurious additional content type byte on the end (and adjusting the
    record length accordingly).
    
    It is interesting to look at why this didn't cause a failure:
    
    The receiving peer first attempts to decrypt the data. Because this is
    TLSProxy we always use a GCM based ciphersuite with a 16 byte tag. When
    we decrypt this it actually gets diverted to the ossltest engine. All this
    does is go through the motions of encrypting/decrypting but just passes
    back the original data. Crucially it will never fail because of a bad tag!
    The receiving party thinks the spurious additional byte is part of the
    tag and the ossltest engine ignores it.
    
    This means the data that gets passed back to the record layer still has
    an additional spurious byte on it - but because the 16 byte tag has been
    removed, this is actually the first byte of the original tag. Again
    because we are using ossltest engine we aren't actually creating "real"
    tags - we only ever emit 16, 0 bytes for the tag. So the spurious
    additional byte always has the value 0. The TLSv1.3 spec says that records
    can have additional 0 bytes on the end of them - this is "padding". So the
    record layer interprets this 0 byte as padding and strips it off to end up
    with the originally transmitted record data - which it can now process
    successfully.
    
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/5370)

-----------------------------------------------------------------------

Summary of changes:
 util/perl/TLSProxy/Record.pm | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/util/perl/TLSProxy/Record.pm b/util/perl/TLSProxy/Record.pm
index c949287..773f759 100644
--- a/util/perl/TLSProxy/Record.pm
+++ b/util/perl/TLSProxy/Record.pm
@@ -286,15 +286,13 @@ sub reconstruct_record
     my $self = shift;
     my $server = shift;
     my $data;
-    my $tls13_enc = 0;
 
     if ($self->sslv2) {
         $data = pack('n', $self->len | 0x8000);
     } else {
         if (TLSProxy::Proxy->is_tls13() && $self->encrypted) {
             $data = pack('Cnn', $self->outer_content_type, $self->version,
-                         $self->len + 1);
-            $tls13_enc = 1;
+                         $self->len);
         } else {
             $data = pack('Cnn', $self->content_type, $self->version,
                          $self->len);
@@ -303,10 +301,6 @@ sub reconstruct_record
     }
     $data .= $self->data;
 
-    if ($tls13_enc) {
-        $data .= pack('C', $self->content_type);
-    }
-
     return $data;
 }