From 6228b1dae265bbe6c46457d82b2d14d672af5f46 Mon Sep 17 00:00:00 2001 From: Andy Polyakov Date: Mon, 2 Apr 2018 23:26:25 +0200 Subject: [PATCH] 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 (Merged from https://github.com/openssl/openssl/pull/5843) --- util/perl/TLSProxy/Proxy.pm | 318 ++++++++++++++++++++---------------- 1 file changed, 179 insertions(+), 139 deletions(-) diff --git a/util/perl/TLSProxy/Proxy.pm b/util/perl/TLSProxy/Proxy.pm index 0b90159811..c20b556c02 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 { -- 2.25.1