From 1b3c3ba0b914ab28a35c0ee0a65472bef46451e1 Mon Sep 17 00:00:00 2001 From: bluhm Date: Sun, 17 May 2015 22:49:03 +0000 Subject: [PATCH] Add test cases for a crash reported by Bertrand PROVOST. When a HTTP client writes multiple requests or chunks in a single transfer, relayd invokes the libevent callback manually for the next data. If the callback closes the session, this results in an use after free. Test an invalid second request method, test an invalid header line in the second PUT request, test an invalid second chunked length for a PUT request. Also test multiple valid HTTP 1.1 PUT requests with chunked body. To detect crashes of relayd, start it with "prefork 1" and grep for "lost child" log messages. Unfortunately only the first child is monitored by the parent. --- regress/usr.sbin/relayd/LICENSE | 2 +- regress/usr.sbin/relayd/Relayd.pm | 3 +- regress/usr.sbin/relayd/args-http-callback.pl | 54 ++++++++++++ .../relayd/args-http-chunked-callback.pl | 54 ++++++++++++ .../usr.sbin/relayd/args-http-chunked-put.pl | 38 +++++++++ .../relayd/args-http-headline-callback.pl | 54 ++++++++++++ .../usr.sbin/relayd/args-https-callback.pl | 58 +++++++++++++ .../relayd/args-https-chunked-callback.pl | 59 +++++++++++++ .../usr.sbin/relayd/args-https-chunked-put.pl | 42 ++++++++++ .../relayd/args-https-headline-callback.pl | 58 +++++++++++++ regress/usr.sbin/relayd/funcs.pl | 82 +++++++++++++------ 11 files changed, 478 insertions(+), 26 deletions(-) create mode 100644 regress/usr.sbin/relayd/args-http-callback.pl create mode 100644 regress/usr.sbin/relayd/args-http-chunked-callback.pl create mode 100644 regress/usr.sbin/relayd/args-http-chunked-put.pl create mode 100644 regress/usr.sbin/relayd/args-http-headline-callback.pl create mode 100644 regress/usr.sbin/relayd/args-https-callback.pl create mode 100644 regress/usr.sbin/relayd/args-https-chunked-callback.pl create mode 100644 regress/usr.sbin/relayd/args-https-chunked-put.pl create mode 100644 regress/usr.sbin/relayd/args-https-headline-callback.pl diff --git a/regress/usr.sbin/relayd/LICENSE b/regress/usr.sbin/relayd/LICENSE index 0bd073f9f6a..c8293e10125 100644 --- a/regress/usr.sbin/relayd/LICENSE +++ b/regress/usr.sbin/relayd/LICENSE @@ -1,4 +1,4 @@ -# Copyright (c) 2010-2014 Alexander Bluhm +# Copyright (c) 2010-2015 Alexander Bluhm # Copyright (c) 2014 Reyk Floeter # # Permission to use, copy, modify, and distribute this software for any diff --git a/regress/usr.sbin/relayd/Relayd.pm b/regress/usr.sbin/relayd/Relayd.pm index d678e02ab1f..1328978471e 100644 --- a/regress/usr.sbin/relayd/Relayd.pm +++ b/regress/usr.sbin/relayd/Relayd.pm @@ -1,4 +1,4 @@ -# $OpenBSD: Relayd.pm,v 1.13 2014/12/14 20:30:51 bluhm Exp $ +# $OpenBSD: Relayd.pm,v 1.14 2015/05/17 22:49:03 bluhm Exp $ # Copyright (c) 2010-2014 Alexander Bluhm # @@ -52,6 +52,7 @@ sub new { open(my $fh, '>', $self->{conffile}) or die ref($self), " conf file $self->{conffile} create failed: $!"; print $fh "log all\n"; + print $fh "prefork 1\n"; # only crashes of first child are observed print $fh "table { $self->{connectaddr} }\n" if defined($self->{table}); diff --git a/regress/usr.sbin/relayd/args-http-callback.pl b/regress/usr.sbin/relayd/args-http-callback.pl new file mode 100644 index 00000000000..0339d897a23 --- /dev/null +++ b/regress/usr.sbin/relayd/args-http-callback.pl @@ -0,0 +1,54 @@ +# test http connection over http relay invoking the callback. +# The client uses a bad method in the second request. +# Check that the relay handles the input after the error correctly. + +use strict; +use warnings; + +my @lengths = (4, 3); +our %args = ( + client => { + func => sub { + my $self = shift; + print <<'EOF'; +PUT /4 HTTP/1.1 +Host: foo.bar +Content-Length: 4 + +123 +XXX +PUT /3 HTTP/1.1 +Host: foo.bar +Content-Length: 3 + +12 +EOF + print STDERR "LEN: 4\n"; + print STDERR "LEN: 3\n"; + # relayd does not forward the first request if the second one + # is invalid. So do not expect any response. + #http_response($self, "without len"); + }, + http_vers => ["1.1"], + lengths => \@lengths, + method => "PUT", + }, + relayd => { + protocol => [ "http", + "match request header log foo", + "match response header log bar", + ], + loggrep => { + qr/, malformed, PUT/ => 1, + }, + }, + server => { + func => \&http_server, + # The server does not get any connection. + noserver => 1, + nocheck => 1, + }, + lengths => \@lengths, +); + +1; diff --git a/regress/usr.sbin/relayd/args-http-chunked-callback.pl b/regress/usr.sbin/relayd/args-http-chunked-callback.pl new file mode 100644 index 00000000000..68af1c835f3 --- /dev/null +++ b/regress/usr.sbin/relayd/args-http-chunked-callback.pl @@ -0,0 +1,54 @@ +# test chunked http connection over http relay invoking the callback +# The client writes a bad chunk length in the second chunk. +# Check that the relay handles the input after the error correctly. + +use strict; +use warnings; + +my @lengths = ([4, 3]); +our %args = ( + client => { + func => sub { + my $self = shift; + print <<'EOF'; +PUT /4/3 HTTP/1.1 +Host: foo.bar +Transfer-Encoding: chunked + +4 +123 + +XXX +3 +12 + +0 + +EOF + print STDERR "LEN: 4\n"; + print STDERR "LEN: 3\n"; + # relayd does not forward the first chunk if the second one + # is invalid. So do not expect any response. + #http_response($self, "without len"); + }, + http_vers => ["1.1"], + lengths => \@lengths, + method => "PUT", + }, + relayd => { + protocol => [ "http", + "match request header log foo", + "match response header log bar", + ], + loggrep => { + qr/, invalid chunk size, PUT/ => 1, + }, + }, + server => { + func => \&http_server, + nocheck => 1, + }, + lengths => \@lengths, +); + +1; diff --git a/regress/usr.sbin/relayd/args-http-chunked-put.pl b/regress/usr.sbin/relayd/args-http-chunked-put.pl new file mode 100644 index 00000000000..9a7e36dc84a --- /dev/null +++ b/regress/usr.sbin/relayd/args-http-chunked-put.pl @@ -0,0 +1,38 @@ +# test chunked http request over http relay + +use strict; +use warnings; + +my @lengths = ([ 251, 10000, 10 ], 1, [2, 3]); +our %args = ( + client => { + func => \&http_client, + lengths => \@lengths, + http_vers => ["1.1"], + method => "PUT", + }, + relayd => { + protocol => [ "http", + "match request header log Transfer-Encoding", + "match response header log bar", + ], + loggrep => { + qr/\[Transfer-Encoding: chunked\]/ => 1, + qr/\[\(null\)\]/ => 0, + }, + }, + server => { + func => \&http_server, + }, + lengths => \@lengths, + md5 => [ + "bc3a3f39af35fe5b1687903da2b00c7f", + "fccd8d69acceb0cc35f2fd4e2f6938d3", + "c47658d102d5b989e0da09ce403f7463", + "68b329da9893e34099c7d8ad5cb9c940", + "897316929176464ebc9ad085f31e7284", + "0ade138937c4b9cb36a28e2edb6485fc", + ], +); + +1; diff --git a/regress/usr.sbin/relayd/args-http-headline-callback.pl b/regress/usr.sbin/relayd/args-http-headline-callback.pl new file mode 100644 index 00000000000..cfc27e341b2 --- /dev/null +++ b/regress/usr.sbin/relayd/args-http-headline-callback.pl @@ -0,0 +1,54 @@ +# test persistent http connection over http relay invoking the callback +# The client writes a bad header line in the second request. +# Check that the relay handles the input after the error correctly. + +use strict; +use warnings; + +my @lengths = (4, 3); +our %args = ( + client => { + func => sub { + my $self = shift; + print <<'EOF'; +PUT /4 HTTP/1.1 +Host: foo.bar +Content-Length: 4 + +123 +PUT /3 HTTP/1.1 +XXX +Host: foo.bar +Content-Length: 3 + +12 +EOF + print STDERR "LEN: 4\n"; + print STDERR "LEN: 3\n"; + # relayd does not forward the first request if the second one + # is invalid. So do not expect any response. + #http_response($self, "without len"); + }, + http_vers => ["1.1"], + lengths => \@lengths, + method => "PUT", + }, + relayd => { + protocol => [ "http", + "match request header log foo", + "match response header log bar", + ], + loggrep => { + qr/, malformed, PUT/ => 1, + }, + }, + server => { + func => \&http_server, + # The server does not get any connection. + noserver => 1, + nocheck => 1, + }, + lengths => \@lengths, +); + +1; diff --git a/regress/usr.sbin/relayd/args-https-callback.pl b/regress/usr.sbin/relayd/args-https-callback.pl new file mode 100644 index 00000000000..f017f7c5519 --- /dev/null +++ b/regress/usr.sbin/relayd/args-https-callback.pl @@ -0,0 +1,58 @@ +# test https connection over http relay invoking the callback. +# The client uses a bad method in the second request. +# Check that the relay handles the input after the error correctly. + +use strict; +use warnings; + +my @lengths = (4, 3); +our %args = ( + client => { + func => sub { + my $self = shift; + print <<'EOF'; +PUT /4 HTTP/1.1 +Host: foo.bar +Content-Length: 4 + +123 +XXX +PUT /3 HTTP/1.1 +Host: foo.bar +Content-Length: 3 + +12 +EOF + print STDERR "LEN: 4\n"; + print STDERR "LEN: 3\n"; + # relayd does not forward the first request if the second one + # is invalid. So do not expect any response. + #http_response($self, "without len"); + }, + ssl => 1, + http_vers => ["1.1"], + lengths => \@lengths, + method => "PUT", + }, + relayd => { + protocol => [ "http", + "match request header log foo", + "match response header log bar", + ], + forwardssl => 1, + listenssl => 1, + loggrep => { + qr/, malformed, PUT/ => 1, + }, + }, + server => { + func => \&http_server, + ssl => 1, + # The server does not get any connection. + noserver => 1, + nocheck => 1, + }, + lengths => \@lengths, +); + +1; diff --git a/regress/usr.sbin/relayd/args-https-chunked-callback.pl b/regress/usr.sbin/relayd/args-https-chunked-callback.pl new file mode 100644 index 00000000000..a4fe57a33b9 --- /dev/null +++ b/regress/usr.sbin/relayd/args-https-chunked-callback.pl @@ -0,0 +1,59 @@ +# test chunked https connection over http relay invoking the callback +# The client writes a bad chunk length in the second chunk. +# Check that the relay handles the input after the error correctly. + +use strict; +use warnings; + +my @lengths = ([4, 3]); +our %args = ( + client => { + func => sub { + my $self = shift; + print <<'EOF'; +PUT /4/3 HTTP/1.1 +Host: foo.bar +Transfer-Encoding: chunked + +4 +123 + +XXX +3 +12 + +0 + +EOF + print STDERR "LEN: 4\n"; + print STDERR "LEN: 3\n"; + # relayd does not forward the first chunk if the second one + # is invalid. So do not expect any response. + #http_response($self, "without len"); + }, + ssl => 1, + http_vers => ["1.1"], + lengths => \@lengths, + method => "PUT", + }, + relayd => { + protocol => [ "http", + "match request header log foo", + "match response header log bar", + ], + forwardssl => 1, + listenssl => 1, + loggrep => { + qr/, invalid chunk size, PUT/ => 1, + }, + }, + server => { + func => \&http_server, + # relayd only connects but does no ssl handshake + #ssl => 1, + nocheck => 1, + }, + lengths => \@lengths, +); + +1; diff --git a/regress/usr.sbin/relayd/args-https-chunked-put.pl b/regress/usr.sbin/relayd/args-https-chunked-put.pl new file mode 100644 index 00000000000..32c950b10d4 --- /dev/null +++ b/regress/usr.sbin/relayd/args-https-chunked-put.pl @@ -0,0 +1,42 @@ +# test chunked https request over http relay + +use strict; +use warnings; + +my @lengths = ([ 251, 10000, 10 ], 1, [2, 3]); +our %args = ( + client => { + func => \&http_client, + ssl => 1, + lengths => \@lengths, + http_vers => ["1.1"], + method => "PUT", + }, + relayd => { + protocol => [ "http", + "match request header log Transfer-Encoding", + "match response header log bar", + ], + forwardssl => 1, + listenssl => 1, + loggrep => { + qr/\[Transfer-Encoding: chunked\]/ => 1, + qr/\[\(null\)\]/ => 0, + }, + }, + server => { + func => \&http_server, + ssl => 1, + }, + lengths => \@lengths, + md5 => [ + "bc3a3f39af35fe5b1687903da2b00c7f", + "fccd8d69acceb0cc35f2fd4e2f6938d3", + "c47658d102d5b989e0da09ce403f7463", + "68b329da9893e34099c7d8ad5cb9c940", + "897316929176464ebc9ad085f31e7284", + "0ade138937c4b9cb36a28e2edb6485fc", + ], +); + +1; diff --git a/regress/usr.sbin/relayd/args-https-headline-callback.pl b/regress/usr.sbin/relayd/args-https-headline-callback.pl new file mode 100644 index 00000000000..0cd1600dc53 --- /dev/null +++ b/regress/usr.sbin/relayd/args-https-headline-callback.pl @@ -0,0 +1,58 @@ +# test persistent https connection over http relay invoking the callback +# The client writes a bad header line in the second request. +# Check that the relay handles the input after the error correctly. + +use strict; +use warnings; + +my @lengths = (4, 3); +our %args = ( + client => { + func => sub { + my $self = shift; + print <<'EOF'; +PUT /4 HTTP/1.1 +Host: foo.bar +Content-Length: 4 + +123 +PUT /3 HTTP/1.1 +XXX +Host: foo.bar +Content-Length: 3 + +12 +EOF + print STDERR "LEN: 4\n"; + print STDERR "LEN: 3\n"; + # relayd does not forward the first request if the second one + # is invalid. So do not expect any response. + #http_response($self, "without len"); + }, + ssl => 1, + http_vers => ["1.1"], + lengths => \@lengths, + method => "PUT", + }, + relayd => { + protocol => [ "http", + "match request header log foo", + "match response header log bar", + ], + forwardssl => 1, + listenssl => 1, + loggrep => { + qr/, malformed, PUT/ => 1, + }, + }, + server => { + func => \&http_server, + ssl => 1, + # The server does not get any connection. + noserver => 1, + nocheck => 1, + }, + lengths => \@lengths, +); + +1; diff --git a/regress/usr.sbin/relayd/funcs.pl b/regress/usr.sbin/relayd/funcs.pl index 98aee4390ab..bd86178098d 100644 --- a/regress/usr.sbin/relayd/funcs.pl +++ b/regress/usr.sbin/relayd/funcs.pl @@ -1,6 +1,6 @@ -# $OpenBSD: funcs.pl,v 1.18 2015/01/05 22:41:37 bluhm Exp $ +# $OpenBSD: funcs.pl,v 1.19 2015/05/17 22:49:03 bluhm Exp $ -# Copyright (c) 2010-2014 Alexander Bluhm +# Copyright (c) 2010-2015 Alexander Bluhm # # Permission to use, copy, modify, and distribute this software for any # purpose with or without fee is hereby granted, provided that the above @@ -92,6 +92,7 @@ sub http_client { my $len = shift // $self->{len} // 251; my $cookie = $self->{cookie}; http_request($self, $len, "1.0", $cookie); + http_response($self, $len); return; } @@ -101,7 +102,10 @@ sub http_client { my @cookies = @{$self->{redo}{cookies} || $self->{cookies} || []}; while (defined (my $len = shift @lengths)) { my $cookie = shift @cookies || $self->{cookie}; - eval { http_request($self, $len, $vers, $cookie) }; + eval { + http_request($self, $len, $vers, $cookie); + http_response($self, $len); + }; warn $@ if $@; if (@lengths && ($@ || $vers eq "1.0")) { # reconnect and redo the outstanding requests @@ -136,9 +140,15 @@ sub http_request { } my @request = ("$method /$path HTTP/$vers"); push @request, "Host: foo.bar" unless defined $header{Host}; - push @request, "Content-Length: $len" - if $vers eq "1.1" && $method eq "PUT" && - !defined $header{'Content-Length'}; + if ($vers eq "1.1" && $method eq "PUT") { + if (ref($len) eq 'ARRAY') { + push @request, "Transfer-Encoding: chunked" + if !defined $header{'Transfer-Encoding'}; + } else { + push @request, "Content-Length: $len" + if !defined $header{'Content-Length'}; + } + } foreach my $key (sort keys %header) { my $val = $header{$key}; if (ref($val) eq 'ARRAY') { @@ -152,13 +162,29 @@ sub http_request { push @request, ""; print STDERR map { ">>> $_\n" } @request; print map { "$_\r\n" } @request; - write_char($self, $len) if $method eq "PUT"; + if ($method eq "PUT") { + if (ref($len) eq 'ARRAY') { + if ($vers eq "1.1") { + write_chunked($self, @$len); + } else { + write_char($self, $_) foreach (@$len); + } + } else { + write_char($self, $len); + } + } IO::Handle::flush(\*STDOUT); # XXX client shutdown seems to be broken in relayd #shutdown(\*STDOUT, SHUT_WR) # or die ref($self), " shutdown write failed: $!" # if $vers ne "1.1"; +} +sub http_response { + my ($self, $len) = @_; + my $method = $self->{method} || "GET"; + + my $vers; my $chunked = 0; { local $/ = "\r\n"; @@ -167,9 +193,10 @@ sub http_request { or die ref($self), " missing http $len response"; chomp; print STDERR "<<< $_\n"; - m{^HTTP/$vers 200 OK$} + m{^HTTP/(\d\.\d) 200 OK$} or die ref($self), " http response not ok" unless $self->{httpnok}; + $vers = $1; while () { chomp; print STDERR "<<< $_\n"; @@ -304,19 +331,22 @@ sub http_server { $cookie ||= $1 if /^Cookie: (.*)/; } } - # XXX reading to EOF does not work with relayd - #read_char($self, $vers eq "1.1" ? $len : undef) - read_char($self, $len) - if $method eq "PUT"; + if ($method eq "PUT" ) { + if (ref($len) eq 'ARRAY') { + read_chunked($self); + } else { + read_char($self, $len); + } + } my @response = ("HTTP/$vers 200 OK"); $len = defined($len) ? $len : scalar(split /|/,$url); - if (ref($len) eq 'ARRAY') { - push @response, "Transfer-Encoding: chunked" - if $vers eq "1.1"; - } else { - push @response, "Content-Length: $len" - if $vers eq "1.1" && $method eq "GET"; + if ($vers eq "1.1" && $method eq "GET") { + if (ref($len) eq 'ARRAY') { + push @response, "Transfer-Encoding: chunked"; + } else { + push @response, "Content-Length: $len"; + } } foreach my $key (sort keys %header) { my $val = $header{$key}; @@ -333,14 +363,16 @@ sub http_server { print STDERR map { ">>> $_\n" } @response; print map { "$_\r\n" } @response; - if (ref($len) eq 'ARRAY') { - if ($vers eq "1.1") { - write_chunked($self, @$len); + if ($method eq "GET") { + if (ref($len) eq 'ARRAY') { + if ($vers eq "1.1") { + write_chunked($self, @$len); + } else { + write_char($self, $_) foreach (@$len); + } } else { - write_char($self, $_) foreach (@$len); + write_char($self, $len); } - } else { - write_char($self, $len) if $method eq "GET"; } IO::Handle::flush(\*STDOUT); } while ($vers eq "1.1"); @@ -375,6 +407,8 @@ sub check_logs { check_len($c, $r, $s, %args); check_md5($c, $r, $s, %args); check_loggrep($c, $r, $s, %args); + $r->loggrep("lost child") + and die "relayd lost child"; } sub check_len { -- 2.20.1