[openssl-commits] [openssl] master update

Andy Polyakov appro at openssl.org
Sun Apr 8 09:43:36 UTC 2018


The branch master has been updated
       via  ceaa389445f9f6b99244bd45041580883b4e8502 (commit)
       via  c53c2fec82b6026331c98642ce4ad32ff7fe4fef (commit)
       via  f69d050ee344e931aea4102f09bb0134a4f4c12a (commit)
       via  dcf3d83faf3542b984d5586bdb1d50c90137a29d (commit)
       via  55fd5d3fc5f7df2bbbdc11caa14a33da383cf65b (commit)
      from  6e301900503f43564029754c799976c89950d33e (commit)


- Log -----------------------------------------------------------------
commit ceaa389445f9f6b99244bd45041580883b4e8502
Author: Andy Polyakov <appro at openssl.org>
Date:   Fri Apr 6 11:44:38 2018 +0200

    TLSProxy/Record.pm: remove dead condition and improve readability.
    
    Reviewed-by: Richard Levitte <levitte at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5887)

commit c53c2fec82b6026331c98642ce4ad32ff7fe4fef
Author: Andy Polyakov <appro at openssl.org>
Date:   Fri Apr 6 11:33:16 2018 +0200

    TLSProxy/Proxy.pm: refine partial packet handling.
    
    Original logic was "if no records found *or* last one is truncated, then
    leave complete records in queue." Trouble is that if we don't pass on
    complete records and get complete packet in opposite direction, then
    queued records will go back to sender. In other words complete records
    should always be passed on. [Possible alternative would be to match
    direction in reconstruct_record.]
    
    Reviewed-by: Richard Levitte <levitte at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5887)

commit f69d050ee344e931aea4102f09bb0134a4f4c12a
Author: Andy Polyakov <appro at openssl.org>
Date:   Thu Apr 5 19:19:35 2018 +0200

    apps/{s_client.c|s_socket}.c: omit usleep calls.
    
    Even though removed calls were oiriginally added on Windows, problem
    they tried to mitigate is not Windows-specific.
    
    Reviewed-by: Richard Levitte <levitte at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5887)

commit dcf3d83faf3542b984d5586bdb1d50c90137a29d
Author: Andy Polyakov <appro at openssl.org>
Date:   Thu Apr 5 18:59:36 2018 +0200

    apps/s_socket.c: disable the Nagle algorithm.
    
    Without TCP_NODELAY alerts risk to be dropped between shutdown and close.
    
    Reviewed-by: Richard Levitte <levitte at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5887)

commit 55fd5d3fc5f7df2bbbdc11caa14a33da383cf65b
Author: Andy Polyakov <appro at openssl.org>
Date:   Thu Apr 5 18:56:52 2018 +0200

    TLSProxy/Proxy.pm: harmonize inner loop with the way sockets are.
    
    Reviewed-by: Richard Levitte <levitte at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5887)

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

Summary of changes:
 apps/s_client.c              |  26 +++++------
 apps/s_socket.c              |  16 +------
 util/perl/TLSProxy/Proxy.pm  |  34 ++++++++++-----
 util/perl/TLSProxy/Record.pm | 102 +++++++++++++++++++------------------------
 4 files changed, 83 insertions(+), 95 deletions(-)

diff --git a/apps/s_client.c b/apps/s_client.c
index ce7366f..89cddb3 100644
--- a/apps/s_client.c
+++ b/apps/s_client.c
@@ -3051,19 +3051,6 @@ int s_client_main(int argc, char **argv)
     do_ssl_shutdown(con);
 
     /*
-     * Give the socket time to send its last data before we close it.
-     * No amount of setting SO_LINGER etc on the socket seems to persuade
-     * Windows to send the data before closing the socket...but sleeping
-     * for a short time seems to do it (units in ms)
-     * TODO: Find a better way to do this
-     */
-#if defined(OPENSSL_SYS_WINDOWS)
-    Sleep(50);
-#elif defined(OPENSSL_SYS_CYGWIN)
-    usleep(50000);
-#endif
-
-    /*
      * If we ended with an alert being sent, but still with data in the
      * network buffer to be read, then calling BIO_closesocket() will
      * result in a TCP-RST being sent. On some platforms (notably
@@ -3074,6 +3061,19 @@ int s_client_main(int argc, char **argv)
      * TCP-RST. This seems to allow the peer to read the alert data.
      */
     shutdown(SSL_get_fd(con), 1); /* SHUT_WR */
+    /*
+     * We just said we have nothing else to say, but it doesn't mean that
+     * the other side has nothing. It's even recommended to consume incoming
+     * data. [In testing context this ensures that alerts are passed on...]
+     */
+    timeout.tv_sec = 0;
+    timeout.tv_usec = 500000;  /* some extreme round-trip */
+    do {
+        FD_ZERO(&readfds);
+        openssl_fdset(s, &readfds);
+    } while (select(s + 1, &readfds, NULL, NULL, &timeout) > 0
+             && BIO_read(sbio, sbuf, BUFSIZZ) > 0);
+
     BIO_closesocket(SSL_get_fd(con));
  end:
     if (con != NULL) {
diff --git a/apps/s_socket.c b/apps/s_socket.c
index e3cfda9..ae62a13 100644
--- a/apps/s_socket.c
+++ b/apps/s_socket.c
@@ -146,7 +146,7 @@ int init_client(int *sock, const char *host, const char *port,
         }
 #endif
 
-        if (!BIO_connect(*sock, BIO_ADDRINFO_address(ai), 0)) {
+        if (!BIO_connect(*sock, BIO_ADDRINFO_address(ai), BIO_SOCK_NODELAY)) {
             BIO_closesocket(*sock);
             *sock = INVALID_SOCKET;
             continue;
@@ -330,22 +330,10 @@ int do_server(int *accept_sock, const char *host, const char *port,
                 BIO_closesocket(asock);
                 break;
             }
+            BIO_set_tcp_ndelay(sock, 1);
             i = (*cb)(sock, type, protocol, context);
 
             /*
-             * Give the socket time to send its last data before we close it.
-             * No amount of setting SO_LINGER etc on the socket seems to
-             * persuade Windows to send the data before closing the socket...
-             * but sleeping for a short time seems to do it (units in ms)
-             * TODO: Find a better way to do this
-             */
-#if defined(OPENSSL_SYS_WINDOWS)
-            Sleep(50);
-#elif defined(OPENSSL_SYS_CYGWIN)
-            usleep(50000);
-#endif
-
-            /*
              * If we ended with an alert being sent, but still with data in the
              * network buffer to be read, then calling BIO_closesocket() will
              * result in a TCP-RST being sent. On some platforms (notably
diff --git a/util/perl/TLSProxy/Proxy.pm b/util/perl/TLSProxy/Proxy.pm
index c20b556..b3b2fec 100644
--- a/util/perl/TLSProxy/Proxy.pm
+++ b/util/perl/TLSProxy/Proxy.pm
@@ -356,7 +356,8 @@ sub clientstart
     my @ready;
     my $ctr = 0;
     local $SIG{PIPE} = "IGNORE";
-    while(     (!(TLSProxy::Message->end)
+    while($fdset->count
+            && (!(TLSProxy::Message->end)
                 || (defined $self->sessionfile()
                     && (-s $self->sessionfile()) == 0))
             && $ctr < 10) {
@@ -366,15 +367,25 @@ sub clientstart
         }
         foreach my $hand (@ready) {
             if ($hand == $server_sock) {
-                $server_sock->sysread($indata, 16384) or goto END;
-                $indata = $self->process_packet(1, $indata);
-                $client_sock->syswrite($indata);
-                $ctr = 0;
+                if ($server_sock->sysread($indata, 16384)) {
+                    if ($indata = $self->process_packet(1, $indata)) {
+                        $client_sock->syswrite($indata) or goto END;
+                    }
+                    $ctr = 0;
+                } else {
+                    $fdset->remove($server_sock);
+                    $client_sock->shutdown(SHUT_WR);
+                }
             } elsif ($hand == $client_sock) {
-                $client_sock->sysread($indata, 16384) or goto END;
-                $indata = $self->process_packet(0, $indata);
-                $server_sock->syswrite($indata);
-                $ctr = 0;
+                if ($client_sock->sysread($indata, 16384)) {
+                    if ($indata = $self->process_packet(0, $indata)) {
+                        $server_sock->syswrite($indata) or goto END;
+                    }
+                    $ctr = 0;
+                } else {
+                    $fdset->remove($client_sock);
+                    $server_sock->shutdown(SHUT_WR);
+                }
             } else {
                 kill(3, $self->{real_serverpid});
                 die "Unexpected handle";
@@ -445,14 +456,15 @@ sub process_packet
 
     #Return contains the list of record found in the packet followed by the
     #list of messages in those records and any partial message
-    my @ret = TLSProxy::Record->get_records($server, $self->flight, $self->{partial}[$server].$packet);
+    my @ret = TLSProxy::Record->get_records($server, $self->flight,
+                                            $self->{partial}[$server].$packet);
     $self->{partial}[$server] = $ret[2];
     push @{$self->{record_list}}, @{$ret[0]};
     push @{$self->{message_list}}, @{$ret[1]};
 
     print "\n";
 
-    if (scalar(@{$ret[0]}) == 0 or length($ret[2]) != 0) {
+    if (scalar(@{$ret[0]}) == 0) {
         return "";
     }
 
diff --git a/util/perl/TLSProxy/Record.pm b/util/perl/TLSProxy/Record.pm
index 624d31c..acace36 100644
--- a/util/perl/TLSProxy/Record.pm
+++ b/util/perl/TLSProxy/Record.pm
@@ -64,12 +64,6 @@ sub get_records
     my $partial = "";
     my @record_list = ();
     my @message_list = ();
-    my $data;
-    my $content_type;
-    my $version;
-    my $len;
-    my $len_real;
-    my $decrypt_len;
 
     my $recnum = 1;
     while (length ($packet) > 0) {
@@ -79,65 +73,59 @@ sub get_records
         } else {
             print " (client -> server)\n";
         }
-        #Get the record header
-        if (length($packet) < TLS_RECORD_HEADER_LENGTH
-                || length($packet) < 5 + unpack("n", substr($packet, 3, 2))) {
+
+        #Get the record header (unpack can't fail if $packet is too short)
+        my ($content_type, $version, $len) = unpack('Cnn', $packet);
+
+        if (length($packet) < TLS_RECORD_HEADER_LENGTH + $len) {
             print "Partial data : ".length($packet)." bytes\n";
             $partial = $packet;
-            $packet = "";
-        } else {
-            ($content_type, $version, $len) = unpack('CnnC*', $packet);
-            $data = substr($packet, 5, $len);
-
-            print "  Content type: ".$record_type{$content_type}."\n";
-            print "  Version: $tls_version{$version}\n";
-            print "  Length: $len";
-            if ($len == length($data)) {
-                print "\n";
-                $decrypt_len = $len_real = $len;
-            } else {
-                print " (expected), ".length($data)." (actual)\n";
-                $decrypt_len = $len_real = length($data);
-            }
+            last;
+        }
+
+        my $data = substr($packet, TLS_RECORD_HEADER_LENGTH, $len);
+
+        print "  Content type: ".$record_type{$content_type}."\n";
+        print "  Version: $tls_version{$version}\n";
+        print "  Length: $len\n";
+
+        my $record = TLSProxy::Record->new(
+            $flight,
+            $content_type,
+            $version,
+            $len,
+            0,
+            $len,       # len_real
+            $len,       # decrypt_len
+            $data,      # data
+            $data       # decrypt_data
+        );
+
+        if ($content_type != RT_CCS) {
+            if (($server && $server_encrypting)
+                     || (!$server && $client_encrypting)) {
+                if (!TLSProxy::Proxy->is_tls13() && $etm) {
+                    $record->decryptETM();
+                } else {
+                    $record->decrypt();
+                }
+                $record->encrypted(1);
 
-            my $record = TLSProxy::Record->new(
-                $flight,
-                $content_type,
-                $version,
-                $len,
-                0,
-                $len_real,
-                $decrypt_len,
-                substr($packet, TLS_RECORD_HEADER_LENGTH, $len_real),
-                substr($packet, TLS_RECORD_HEADER_LENGTH, $len_real)
-            );
-
-            if ($content_type != RT_CCS) {
-                if (($server && $server_encrypting)
-                         || (!$server && $client_encrypting)) {
-                    if (!TLSProxy::Proxy->is_tls13() && $etm) {
-                        $record->decryptETM();
-                    } else {
-                        $record->decrypt();
-                    }
-                    $record->encrypted(1);
-
-                    if (TLSProxy::Proxy->is_tls13()) {
-                        print "  Inner content type: "
-                              .$record_type{$record->content_type()}."\n";
-                    }
+                if (TLSProxy::Proxy->is_tls13()) {
+                    print "  Inner content type: "
+                          .$record_type{$record->content_type()}."\n";
                 }
             }
+        }
 
-            push @record_list, $record;
+        push @record_list, $record;
 
-            #Now figure out what messages are contained within this record
-            my @messages = TLSProxy::Message->get_messages($server, $record);
-            push @message_list, @messages;
+        #Now figure out what messages are contained within this record
+        my @messages = TLSProxy::Message->get_messages($server, $record);
+        push @message_list, @messages;
 
-            $packet = substr($packet, TLS_RECORD_HEADER_LENGTH + $len_real);
-            $recnum++;
-        }
+        $packet = substr($packet, TLS_RECORD_HEADER_LENGTH + $len);
+        $recnum++;
     }
 
     return (\@record_list, \@message_list, $partial);


More information about the openssl-commits mailing list