Fix a TLSProxy race condition

TLSProxy starts s_server and specifies the number of client connects
it should expect. After that s_server is supposed to close down
automatically. However, if another test is then run then TLSProxy
will start a new instance of s_server. If the previous instance
hasn't closed down yet then the new instance can fail to bind to
the socket.

Reviewed-by: Richard Levitte <levitte@openssl.org>

diff --git a/test/recipes/70-test_sslsessiontick.t b/test/recipes/70-test_sslsessiontick.t
index f13c7ba5860d431bdd6b54aee2a7d217e92ae838..56ae4c0cba2fb7341fef98e1ac3fa1dad39352ae 100755 (executable)
--- a/test/recipes/70-test_sslsessiontick.t
+++ b/test/recipes/70-test_sslsessiontick.t
@@ -71,6 +71,7 @@ $ENV{OPENSSL_ENGINES} = bldtop_dir("engines");
 $ENV{OPENSSL_ia32cap} = '~0x200000200000000';
 
 sub checkmessages($$$$$$);
+sub clearclient();
 sub clearall();
 
 my $chellotickext = 0;
@@ -119,7 +120,7 @@ clearall();
 $proxy->serverconnects(2);
 $proxy->clientflags("-sess_out ".$session);
 $proxy->start();
-$proxy->clear();
+$proxy->clearClient();
 $proxy->clientflags("-sess_in ".$session);
 $proxy->clientstart();
 checkmessages(4, "Session resumption session ticket test", 1, 0, 0, 0);
@@ -132,7 +133,7 @@ clearall();
 $proxy->serverconnects(2);
 $proxy->clientflags("-sess_out ".$session." -no_ticket");
 $proxy->start();
-$proxy->clear();
+$proxy->clearClient();
 $proxy->clientflags("-sess_in ".$session);
 $proxy->clientstart();
 checkmessages(5, "Session resumption with ticket capable client without a "
@@ -153,14 +154,14 @@ $proxy->serverconnects(3);
 $proxy->filter(undef);
 $proxy->clientflags("-sess_out ".$session);
 $proxy->start();
-$proxy->clear();
+$proxy->clearClient();
 $proxy->clientflags("-sess_in ".$session." -sess_out ".$session);
 $proxy->filter(\&inject_empty_ticket_filter);
 $proxy->clientstart();
 #Expected result: ClientHello extension seen; ServerHello extension seen;
 #                 NewSessionTicket message seen; Abbreviated handshake.
 checkmessages(7, "Empty ticket resumption test",  1, 1, 1, 0);
-clearall();
+clearclient();
 $proxy->clientflags("-sess_in ".$session);
 $proxy->filter(undef);
 $proxy->clientstart();
@@ -252,11 +253,18 @@ sub checkmessages($$$$$$)
     }
 }
 
-sub clearall()
+
+sub clearclient()
 {
     $chellotickext = 0;
     $shellotickext = 0;
     $fullhand = 0;
     $ticketseen = 0;
+    $proxy->clearClient();
+}
+
+sub clearall()
+{
+    clearclient();
     $proxy->clear();
 }

diff --git a/test/recipes/70-test_sslvertol.t b/test/recipes/70-test_sslvertol.t
index a35eab960b4005f65643c7e45ecd0e7990875355..d436b5aba7963b98efea1460b729e98d4e69e77d 100755 (executable)
--- a/test/recipes/70-test_sslvertol.t
+++ b/test/recipes/70-test_sslvertol.t
@@ -84,7 +84,8 @@ ok(TLSProxy::Message->success(), "Version tolerance test, TLS 1.3");
 
 #Test 2: Testing something below SSLv3 should fail
 $client_version = TLSProxy::Record::VERS_SSL_3_0 - 1;
-$proxy->restart();
+$proxy->clear();
+$proxy->start();
 ok(TLSProxy::Message->fail(), "Version tolerance test, SSL < 3.0");
 
 sub vers_tolerance_filter

diff --git a/test/recipes/70-test_tlsextms.t b/test/recipes/70-test_tlsextms.t
index 06b4d9ee3f64599fa71f035b51950908694b2154..47a03213c82df9814a14001f026fbb3ae3bb731b 100644 (file)
--- a/test/recipes/70-test_tlsextms.t
+++ b/test/recipes/70-test_tlsextms.t
@@ -136,7 +136,7 @@ setrmextms(0, 0);
 $proxy->serverconnects(2);
 $proxy->clientflags("-sess_out ".$session);
 $proxy->start();
-$proxy->clear();
+$proxy->clearClient();
 $proxy->clientflags("-sess_in ".$session);
 $proxy->clientstart();
 checkmessages(5, "Session resumption extended master secret test", 1, 1, 0);
@@ -152,7 +152,7 @@ setrmextms(1, 0);
 $proxy->serverconnects(2);
 $proxy->clientflags("-sess_out ".$session);
 $proxy->start();
-$proxy->clear();
+$proxy->clearClient();
 $proxy->clientflags("-sess_in ".$session);
 setrmextms(0, 0);
 $proxy->clientstart();
@@ -168,7 +168,7 @@ setrmextms(0, 0);
 $proxy->serverconnects(2);
 $proxy->clientflags("-sess_out ".$session);
 $proxy->start();
-$proxy->clear();
+$proxy->clearClient();
 $proxy->clientflags("-sess_in ".$session);
 setrmextms(1, 0);
 $proxy->clientstart();
@@ -184,7 +184,7 @@ setrmextms(0, 0);
 $proxy->serverconnects(2);
 $proxy->clientflags("-sess_out ".$session);
 $proxy->start();
-$proxy->clear();
+$proxy->clearClient();
 $proxy->clientflags("-sess_in ".$session);
 setrmextms(0, 1);
 $proxy->clientstart();
@@ -200,7 +200,7 @@ setrmextms(0, 1);
 $proxy->serverconnects(2);
 $proxy->clientflags("-sess_out ".$session);
 $proxy->start();
-$proxy->clear();
+$proxy->clearClient();
 $proxy->clientflags("-sess_in ".$session);
 setrmextms(0, 0);
 $proxy->clientstart();

diff --git a/util/TLSProxy/Proxy.pm b/util/TLSProxy/Proxy.pm
index 96e368189ef34f89a13078edf5abf75b39f89bf7..fcbe2483c4afb3fcddaf4ee3a21f5a7899eb935d 100644 (file)
--- a/util/TLSProxy/Proxy.pm
+++ b/util/TLSProxy/Proxy.pm
@@ -52,6 +52,7 @@
 # Hudson (tjh@cryptsoft.com).
 
 use strict;
+use POSIX ":sys_wait_h";
 
 package TLSProxy::Proxy;
 
@@ -86,6 +87,7 @@ sub new
         serverflags => "",
         clientflags => "",
         serverconnects => 1,
+        serverpid => 0,
 
         #Public read
         execute => $execute,
@@ -138,23 +140,31 @@ sub new
     return bless $self, $class;
 }
 
-sub clear
+sub clearClient
 {
     my $self = shift;
 
     $self->{cipherc} = "";
-    $self->{ciphers} = "AES128-SHA";
     $self->{flight} = 0;
     $self->{record_list} = [];
     $self->{message_list} = [];
-    $self->{serverflags} = "";
     $self->{clientflags} = "";
-    $self->{serverconnects} = 1;
 
     TLSProxy::Message->clear();
     TLSProxy::Record->clear();
 }
 
+sub clear
+{
+    my $self = shift;
+
+    $self->clearClient;
+    $self->{ciphers} = "AES128-SHA";
+    $self->{serverflags} = "";
+    $self->{serverconnects} = 1;
+    $self->{serverpid} = 0;
+}
+
 sub restart
 {
     my $self = shift;
@@ -195,6 +205,7 @@ sub start
         }
         exec($execcmd);
     }
+    $self->serverpid($pid);
 
     $self->clientstart;
 }
@@ -319,6 +330,13 @@ sub clientstart
     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);
+    }
 }
 
 sub process_packet
@@ -503,4 +521,12 @@ sub message_list
     }
     return $self->{message_list};
 }
+sub serverpid
+{
+    my $self = shift;
+    if (@_) {
+      $self->{serverpid} = shift;
+    }
+    return $self->{serverpid};
+}
 1;