Commit 5427976d authored by Matt Caswell's avatar Matt Caswell
Browse files

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: default avatarRichard Levitte <levitte@openssl.org>
parent 2460c7f1
Loading
Loading
Loading
Loading
+13 −5
Original line number Diff line number Diff line
@@ -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();
}
+2 −1
Original line number Diff line number Diff line
@@ -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
+5 −5
Original line number Diff line number Diff line
@@ -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();
+30 −4
Original line number Diff line number Diff line
@@ -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;