[openssl-commits] [openssl] master update

Andy Polyakov appro at openssl.org
Wed Apr 4 18:24:50 UTC 2018


The branch master has been updated
       via  6228b1dae265bbe6c46457d82b2d14d672af5f46 (commit)
       via  5540eb7040839b0075a2b7651b6a95264d025e15 (commit)
      from  8e2bec9b8aaba602af6fda2523a15238aa49aade (commit)


- Log -----------------------------------------------------------------
commit 6228b1dae265bbe6c46457d82b2d14d672af5f46
Author: Andy Polyakov <appro at openssl.org>
Date:   Mon Apr 2 23:26:25 2018 +0200

    TLSProxy/Proxy.pm: switch to dynamic ports and overhaul.
    
    By asking for port 0, you get a free port dynamically assigned by OS.
    TLSProxy::Proxy now asks for 0 and asks s_server to do the same. The
    s_server's port is reported in "ACCEPT" line, which TLSProxy::Proxy
    parses and uses.
    
    Because the server port is now a random affair in TLSProxy::Proxy,
    it's no longer possible to change it with the method 'server_port',
    and it has become an accessor only. For the sake of orthogonality, so
    has the method 'server_addr'.
    
    Remove all fork calls on Windows, as fork is not to be trusted there.
    This naturally minimized amount of fork calls on POSIX systems, to 1.
    
    Sink s_server's output to 'perl -ne print' which ensures that output
    is written strictly in lines. This keeps TAP parser happy.
    
    Improve synchronization in -naccept +n cases by establishing next
    connection to s_server *after* s_client finishes instead of before it
    starts.
    
    Improve error handling and clean up some methods.
    
    Reviewed-by: Richard Levitte <levitte at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5843)

commit 5540eb7040839b0075a2b7651b6a95264d025e15
Author: Richard Levitte <levitte at openssl.org>
Date:   Fri Mar 30 21:13:25 2018 +0200

    openssl s_server: print the accepting address and socket
    
    The line saying ACCEPT is extended with a space followed by the the
    address and port combination on which s_server accepts connections.
    The address is written in such a way that s_client should be able to
    accepts as argument for the '-connect' option.
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5843)

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

Summary of changes:
 apps/s_apps.h               |   5 +-
 apps/s_server.c             |   9 +-
 apps/s_socket.c             |  30 ++++-
 util/perl/TLSProxy/Proxy.pm | 318 +++++++++++++++++++++++++-------------------
 4 files changed, 211 insertions(+), 151 deletions(-)

diff --git a/apps/s_apps.h b/apps/s_apps.h
index 2454161..0a3bc96 100644
--- a/apps/s_apps.h
+++ b/apps/s_apps.h
@@ -22,9 +22,8 @@
 
 typedef int (*do_server_cb)(int s, int stype, int prot, unsigned char *context);
 int do_server(int *accept_sock, const char *host, const char *port,
-              int family, int type, int protocol,
-              do_server_cb cb,
-              unsigned char *context, int naccept);
+              int family, int type, int protocol, do_server_cb cb,
+              unsigned char *context, int naccept, BIO *bio_s_out);
 #ifdef HEADER_X509_H
 int verify_callback(int ok, X509_STORE_CTX *ctx);
 #endif
diff --git a/apps/s_server.c b/apps/s_server.c
index 9b5106d..be1564a 100644
--- a/apps/s_server.c
+++ b/apps/s_server.c
@@ -2095,8 +2095,6 @@ int s_server_main(int argc, char *argv[])
     if (max_early_data >= 0)
         SSL_CTX_set_max_early_data(ctx, max_early_data);
 
-    BIO_printf(bio_s_out, "ACCEPT\n");
-    (void)BIO_flush(bio_s_out);
     if (rev)
         server_cb = rev_body;
     else if (www)
@@ -2109,7 +2107,7 @@ int s_server_main(int argc, char *argv[])
         unlink(host);
 #endif
     do_server(&accept_socket, host, port, socket_family, socket_type, protocol,
-              server_cb, context, naccept);
+              server_cb, context, naccept, bio_s_out);
     print_stats(bio_s_out, ctx);
     ret = 0;
  end:
@@ -2673,9 +2671,6 @@ static int sv_body(int s, int stype, int prot, unsigned char *context)
     }
     BIO_printf(bio_s_out, "CONNECTION CLOSED\n");
     OPENSSL_clear_free(buf, bufsize);
-    if (ret >= 0)
-        BIO_printf(bio_s_out, "ACCEPT\n");
-    (void)BIO_flush(bio_s_out);
     return ret;
 }
 
@@ -3284,8 +3279,6 @@ static int www_body(int s, int stype, int prot, unsigned char *context)
     SSL_set_shutdown(con, SSL_SENT_SHUTDOWN | SSL_RECEIVED_SHUTDOWN);
 
  err:
-    if (ret >= 0)
-        BIO_printf(bio_s_out, "ACCEPT\n");
     OPENSSL_free(buf);
     BIO_free_all(io);
     return ret;
diff --git a/apps/s_socket.c b/apps/s_socket.c
index 4b82011..e3cfda9 100644
--- a/apps/s_socket.c
+++ b/apps/s_socket.c
@@ -204,7 +204,7 @@ out:
  */
 int do_server(int *accept_sock, const char *host, const char *port,
               int family, int type, int protocol, do_server_cb cb,
-              unsigned char *context, int naccept)
+              unsigned char *context, int naccept, BIO *bio_s_out)
 {
     int asock = 0;
     int sock;
@@ -283,6 +283,34 @@ int do_server(int *accept_sock, const char *host, const char *port,
     BIO_ADDRINFO_free(res);
     res = NULL;
 
+    {
+        union BIO_sock_info_u info;
+        char *hostname = NULL;
+        char *service = NULL;
+        int success = 0;
+
+        if ((info.addr = BIO_ADDR_new()) != NULL
+            && BIO_sock_info(asock, BIO_SOCK_INFO_ADDRESS, &info)
+            && (hostname = BIO_ADDR_hostname_string(info.addr, 1)) != NULL
+            && (service = BIO_ADDR_service_string(info.addr, 1)) != NULL
+            && BIO_printf(bio_s_out,
+                          strchr(hostname, ':') == NULL
+                          ? /* IPv4 */ "ACCEPT %s:%s\n"
+                          : /* IPv6 */ "ACCEPT [%s]:%s\n",
+                          hostname, service) > 0)
+            success = 1;
+
+        (void)BIO_flush(bio_s_out);
+        OPENSSL_free(hostname);
+        OPENSSL_free(service);
+        BIO_ADDR_free(info.addr);
+        if (!success) {
+            BIO_closesocket(asock);
+            ERR_print_errors(bio_err);
+            goto end;
+        }
+    }
+
     if (accept_sock != NULL)
         *accept_sock = asock;
     for (;;) {
diff --git a/util/perl/TLSProxy/Proxy.pm b/util/perl/TLSProxy/Proxy.pm
index 0b90159..c20b556 100644
--- a/util/perl/TLSProxy/Proxy.pm
+++ b/util/perl/TLSProxy/Proxy.pm
@@ -22,7 +22,6 @@ use TLSProxy::Certificate;
 use TLSProxy::CertificateVerify;
 use TLSProxy::ServerKeyExchange;
 use TLSProxy::NewSessionTicket;
-use Time::HiRes qw/usleep/;
 
 my $have_IPv6 = 0;
 my $IP_factory;
@@ -41,19 +40,19 @@ sub new
     my $self = {
         #Public read/write
         proxy_addr => "localhost",
-        proxy_port => 4453,
         server_addr => "localhost",
-        server_port => 4443,
         filter => $filter,
         serverflags => "",
         clientflags => "",
         serverconnects => 1,
-        serverpid => 0,
-        clientpid => 0,
         reneg => 0,
         sessionfile => undef,
 
         #Public read
+        proxy_port => 0,
+        server_port => 0,
+        serverpid => 0,
+        clientpid => 0,
         execute => $execute,
         cert => $cert,
         debug => $debug,
@@ -110,18 +109,17 @@ sub new
     $proxaddr =~ s/[\[\]]//g; # Remove [ and ]
     my @proxyargs = (
         LocalHost   => $proxaddr,
-        LocalPort   => $self->{proxy_port},
+        LocalPort   => 0,
         Proto       => "tcp",
         Listen      => SOMAXCONN,
        );
-    push @proxyargs, ReuseAddr => 1
-        unless $^O eq "MSWin32";
     $self->{proxy_sock} = $IP_factory->(@proxyargs);
 
     if ($self->{proxy_sock}) {
+        $self->{proxy_port} = $self->{proxy_sock}->sockport();
         print "Proxy started on port ".$self->{proxy_port}."\n";
     } else {
-        warn "Failed creating proxy socket (".$proxaddr.",".$self->{proxy_port}."): $!\n";
+        warn "Failed creating proxy socket (".$proxaddr.",0): $!\n";
     }
 
     return bless $self, $class;
@@ -184,6 +182,19 @@ sub clientrestart
     $self->clientstart;
 }
 
+sub connect_to_server
+{
+    my $self = shift;
+    my $servaddr = $self->{server_addr};
+
+    $servaddr =~ s/[\[\]]//g; # Remove [ and ]
+
+    $self->{server_sock} = $IP_factory->(PeerAddr => $servaddr,
+                                         PeerPort => $self->{server_port},
+                                         Proto => 'tcp')
+                           or die "unable to connect: $!\n";
+}
+
 sub start
 {
     my ($self) = shift;
@@ -193,31 +204,84 @@ sub start
         return 0;
     }
 
-    $pid = fork();
-    if ($pid == 0) {
-        my $execcmd = $self->execute
-            ." s_server -max_protocol TLSv1.3 -no_comp -rev -engine ossltest -accept "
-            .($self->server_port)
-            ." -cert ".$self->cert." -cert2 ".$self->cert
-            ." -naccept ".$self->serverconnects;
-        unless ($self->supports_IPv6) {
-            $execcmd .= " -4";
-        }
-        if ($self->ciphers ne "") {
-            $execcmd .= " -cipher ".$self->ciphers;
-        }
-        if ($self->ciphersuitess ne "") {
-            $execcmd .= " -ciphersuites ".$self->ciphersuitess;
-        }
-        if ($self->serverflags ne "") {
-            $execcmd .= " ".$self->serverflags;
+    my $execcmd = $self->execute
+        ." s_server -max_protocol TLSv1.3 -no_comp -rev -engine ossltest"
+        ." -accept 0 -cert ".$self->cert." -cert2 ".$self->cert
+        ." -naccept ".$self->serverconnects;
+    unless ($self->supports_IPv6) {
+        $execcmd .= " -4";
+    }
+    if ($self->ciphers ne "") {
+        $execcmd .= " -cipher ".$self->ciphers;
+    }
+    if ($self->ciphersuitess ne "") {
+        $execcmd .= " -ciphersuites ".$self->ciphersuitess;
+    }
+    if ($self->serverflags ne "") {
+        $execcmd .= " ".$self->serverflags;
+    }
+    if ($self->debug) {
+        print STDERR "Server command: $execcmd\n";
+    }
+
+    open(my $savedin, "<&STDIN");
+
+    # Temporarily replace STDIN so that sink process can inherit it...
+    $pid = open(STDIN, "$execcmd |") or die "Failed to $execcmd: $!\n";
+    $self->{real_serverpid} = $pid;
+
+    # Process the output from s_server until we find the ACCEPT line, which
+    # tells us what the accepting address and port are.
+    while (<>) {
+        print;
+        s/\R$//;                # Better chomp
+        next unless (/^ACCEPT\s.*:(\d+)$/);
+        $self->{server_port} = $1;
+        last;
+    }
+
+    if ($self->{server_port} == 0) {
+        # This actually means that s_server exited, because otherwise
+        # we would still searching for ACCEPT...
+        die "no ACCEPT detected in '$execcmd' output\n";
+    }
+
+    # Just make sure everything else is simply printed [as separate lines].
+    # The sub process simply inherits our STD* and will keep consuming
+    # server's output and printing it as long as there is anything there,
+    # out of our way.
+    my $error;
+    $pid = undef;
+    if (eval { require Win32::Process; 1; }) {
+        if (Win32::Process::Create(my $h, $^X, "perl -ne print", 0, 0, ".")) {
+            $pid = $h->GetProcessID();
+        } else {
+            $error = Win32::FormatMessage(Win32::GetLastError());
         }
-        if ($self->debug) {
-            print STDERR "Server command: $execcmd\n";
+    } else {
+        if (defined($pid = fork)) {
+            $pid or exec("$^X -ne print") or exit($!);
+        } else {
+            $error = $!;
         }
-        exec($execcmd);
     }
-    $self->serverpid($pid);
+
+    # Change back to original stdin
+    open(STDIN, "<&", $savedin);
+    close($savedin);
+
+    if (!defined($pid)) {
+        kill(3, $self->{real_serverpid});
+        die "Failed to capture s_server's output: $error\n";
+    }
+
+    $self->{serverpid} = $pid;
+
+    print STDERR "Server responds on ",
+        $self->{server_addr}, ":", $self->{server_port}, "\n";
+
+    # Connect right away...
+    $self->connect_to_server();
 
     return $self->clientstart;
 }
@@ -225,44 +289,57 @@ sub start
 sub clientstart
 {
     my ($self) = shift;
-    my $oldstdout;
 
     if ($self->execute) {
-        my $pid = fork();
-        if ($pid == 0) {
-            my $echostr;
-            if ($self->reneg()) {
-                $echostr = "R";
-            } else {
-                $echostr = "test";
-            }
-            my $execcmd = "echo ".$echostr." | ".$self->execute
-                 ." s_client -max_protocol TLSv1.3 -engine ossltest -connect "
-                 .($self->proxy_addr).":".($self->proxy_port);
-            unless ($self->supports_IPv6) {
-                $execcmd .= " -4";
-            }
-            if ($self->cipherc ne "") {
-                $execcmd .= " -cipher ".$self->cipherc;
-            }
-            if ($self->ciphersuitesc ne "") {
-                $execcmd .= " -ciphersuites ".$self->ciphersuitesc;
-            }
-            if ($self->clientflags ne "") {
-                $execcmd .= " ".$self->clientflags;
-            }
-            if (defined $self->sessionfile) {
-                $execcmd .= " -ign_eof";
-            }
-            if ($self->debug) {
-                print STDERR "Client command: $execcmd\n";
-            }
-            exec($execcmd);
+        my $pid;
+        my $execcmd = $self->execute
+             ." s_client -max_protocol TLSv1.3 -engine ossltest -connect "
+             .($self->proxy_addr).":".($self->proxy_port);
+        unless ($self->supports_IPv6) {
+            $execcmd .= " -4";
+        }
+        if ($self->cipherc ne "") {
+            $execcmd .= " -cipher ".$self->cipherc;
+        }
+        if ($self->ciphersuitesc ne "") {
+            $execcmd .= " -ciphersuites ".$self->ciphersuitesc;
+        }
+        if ($self->clientflags ne "") {
+            $execcmd .= " ".$self->clientflags;
+        }
+        if (defined $self->sessionfile) {
+            $execcmd .= " -ign_eof";
         }
-        $self->clientpid($pid);
+        if ($self->debug) {
+            print STDERR "Client command: $execcmd\n";
+        }
+
+        open(my $savedout, ">&STDOUT");
+        # If we open pipe with new descriptor, attempt to close it,
+        # explicitly or implicitly, would incur waitpid and effectively
+        # dead-lock...
+        if (!($pid = open(STDOUT, "| $execcmd"))) {
+            my $err = $!;
+            kill(3, $self->{real_serverpid});
+            die "Failed to $execcmd: $err\n";
+        }
+        $self->{clientpid} = $pid;
+
+        # queue [magic] input
+        print $self->reneg ? "R" : "test";
+
+        # this closes client's stdin without waiting for its pid
+        open(STDOUT, ">&", $savedout);
+        close($savedout);
     }
 
     # Wait for incoming connection from client
+    my $fdset = IO::Select->new($self->{proxy_sock});
+    if (!$fdset->can_read(1)) {
+        kill(3, $self->{real_serverpid});
+        die "s_client didn't try to connect\n";
+    }
+
     my $client_sock;
     if(!($client_sock = $self->{proxy_sock}->accept())) {
         warn "Failed accepting incoming connection: $!\n";
@@ -271,44 +348,11 @@ sub clientstart
 
     print "Connection opened\n";
 
-    # Now connect to the server
-    my $retry = 50;
-    my $server_sock;
-    #We loop over this a few times because sometimes s_server can take a while
-    #to start up
-    do {
-        my $servaddr = $self->server_addr;
-        $servaddr =~ s/[\[\]]//g; # Remove [ and ]
-        eval {
-            $server_sock = $IP_factory->(
-                PeerAddr => $servaddr,
-                PeerPort => $self->server_port,
-                MultiHomed => 1,
-                Proto => 'tcp'
-            );
-        };
-
-        $retry--;
-        #Some buggy IP factories can return a defined server_sock that hasn't
-        #actually connected, so we check peerport too
-        if ($@ || !defined($server_sock) || !defined($server_sock->peerport)) {
-            $server_sock->close() if defined($server_sock);
-            undef $server_sock;
-            if ($retry) {
-                #Sleep for a short while
-                select(undef, undef, undef, 0.1);
-            } else {
-                warn "Failed to start up server (".$servaddr.",".$self->server_port."): $!\n";
-                return 0;
-            }
-        }
-    } while (!$server_sock);
-
-    my $sel = IO::Select->new($server_sock, $client_sock);
+    my $server_sock = $self->{server_sock};
     my $indata;
-    my @handles = ($server_sock, $client_sock);
 
     #Wait for either the server socket or the client socket to become readable
+    $fdset = IO::Select->new($server_sock, $client_sock);
     my @ready;
     my $ctr = 0;
     local $SIG{PIPE} = "IGNORE";
@@ -316,7 +360,7 @@ sub clientstart
                 || (defined $self->sessionfile()
                     && (-s $self->sessionfile()) == 0))
             && $ctr < 10) {
-        if (!(@ready = $sel->can_read(1))) {
+        if (!(@ready = $fdset->can_read(1))) {
             $ctr++;
             next;
         }
@@ -332,39 +376,47 @@ sub clientstart
                 $server_sock->syswrite($indata);
                 $ctr = 0;
             } else {
+                kill(3, $self->{real_serverpid});
                 die "Unexpected handle";
             }
         }
     }
 
-    die "No progress made" if $ctr >= 10;
+    if ($ctr >= 10) {
+        kill(3, $self->{real_serverpid});
+        die "No progress made";
+    }
 
     END:
     print "Connection closed\n";
     if($server_sock) {
         $server_sock->close();
+        $self->{server_sock} = undef;
     }
     if($client_sock) {
         #Closing this also kills the child process
         $client_sock->close();
     }
-    if(!$self->debug) {
-        select($oldstdout);
-    }
-    $self->serverconnects($self->serverconnects - 1);
-    if ($self->serverconnects == 0) {
-        die "serverpid is zero\n" if $self->serverpid == 0;
-        print "Waiting for server process to close: "
-              .$self->serverpid."\n";
-        waitpid( $self->serverpid, 0);
+
+    my $pid;
+    if (--$self->{serverconnects} == 0) {
+        $pid = $self->{serverpid};
+        die "serverpid is zero\n" if $pid == 0;
+        print "Waiting for server process to close: $pid...\n";
+        # recall that we wait on process that buffers server's output
+        waitpid($pid, 0);
         die "exit code $? from server process\n" if $? != 0;
     } else {
-        # Give s_server sufficient time to finish what it was doing
-        usleep(250000);
+        # It's a bit counter-intuitive spot to make next connection to
+        # the s_server. Rationale is that established connection works
+        # as syncronization point, in sense that this way we know that
+        # s_server is actually done with current session...
+        $self->connect_to_server();
     }
-    die "clientpid is zero\n" if $self->clientpid == 0;
-    print "Waiting for client process to close: ".$self->clientpid."\n";
-    waitpid($self->clientpid, 0);
+    $pid = $self->{clientpid};
+    die "clientpid is zero\n" if $pid == 0;
+    print "Waiting for client process to close: $pid...\n";
+    waitpid($pid, 0);
 
     return 1;
 }
@@ -395,7 +447,7 @@ sub process_packet
     #list of messages in those records and any partial message
     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->{record_list}}, @{$ret[0]};
     push @{$self->{message_list}}, @{$ret[1]};
 
     print "\n";
@@ -471,24 +523,28 @@ sub proxy_port
     my $self = shift;
     return $self->{proxy_port};
 }
-
-#Read/write accessors
 sub server_addr
 {
     my $self = shift;
-    if (@_) {
-        $self->{server_addr} = shift;
-    }
     return $self->{server_addr};
 }
 sub server_port
 {
     my $self = shift;
-    if (@_) {
-        $self->{server_port} = shift;
-    }
     return $self->{server_port};
 }
+sub serverpid
+{
+    my $self = shift;
+    return $self->{serverpid};
+}
+sub clientpid
+{
+    my $self = shift;
+    return $self->{clientpid};
+}
+
+#Read/write accessors
 sub filter
 {
     my $self = shift;
@@ -565,22 +621,6 @@ sub message_list
     }
     return $self->{message_list};
 }
-sub serverpid
-{
-    my $self = shift;
-    if (@_) {
-        $self->{serverpid} = shift;
-    }
-    return $self->{serverpid};
-}
-sub clientpid
-{
-    my $self = shift;
-    if (@_) {
-        $self->{clientpid} = shift;
-    }
-    return $self->{clientpid};
-}
 
 sub fill_known_data
 {


More information about the openssl-commits mailing list