From 5427976d9eddacc87c7e079976bc7738e133dbdc Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 15 Mar 2016 16:44:26 +0000 Subject: [PATCH] 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 --- test/recipes/70-test_sslsessiontick.t | 18 ++++++++++---- test/recipes/70-test_sslvertol.t | 3 ++- test/recipes/70-test_tlsextms.t | 10 ++++---- util/TLSProxy/Proxy.pm | 34 +++++++++++++++++++++++---- 4 files changed, 50 insertions(+), 15 deletions(-) diff --git a/test/recipes/70-test_sslsessiontick.t b/test/recipes/70-test_sslsessiontick.t index f13c7ba586..56ae4c0cba 100755 --- 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 a35eab960b..d436b5aba7 100755 --- 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 06b4d9ee3f..47a03213c8 100644 --- 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 96e368189e..fcbe2483c4 100644 --- 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; -- 2.25.1