diff --git a/h2_headers.t b/h2_headers.t index ffdecff9..ff479fdf 100644 --- a/h2_headers.t +++ b/h2_headers.t @@ -23,7 +23,7 @@ use Test::Nginx::HTTP2; select STDERR; $| = 1; select STDOUT; $| = 1; -my $t = Test::Nginx->new()->has(qw/http http_v2 proxy rewrite/)->plan(110) +my $t = Test::Nginx->new()->has(qw/http http_v2 proxy rewrite/)->plan(133) ->write_file_expand('nginx.conf', <<'EOF'); %%TEST_GLOBALS%% @@ -937,23 +937,69 @@ ok($frame, 'HPACK table boundary - header field value'); # 10.3. Intermediary Encapsulation Attacks # Any request or response that contains a character not permitted # in a header field value MUST be treated as malformed. +my $malformed_values = 0; +foreach my $malformed_value ( + "x-bar\rreferrer: baz", + "x-bar\0referrer: junk", + "x-bar\nreferrer: junk", +) { + $malformed_values += 1; + subtest "Malformed value test $malformed_values" => sub { + plan tests => 2; + + $s = Test::Nginx::HTTP2->new(); + $sid = $s->new_stream({ headers => [ + { name => ':method', value => 'GET', mode => 0 }, + { name => ':scheme', value => 'http', mode => 0 }, + { name => ':path', value => '/proxy2/', mode => 2 }, + { name => ':authority', value => 'localhost', mode => 2 }, + { name => 'x-foo', value => $malformed_value, mode => 4 }]}); + $frames = $s->read(all => [{ sid => $sid, fin => 1 }]); + + # Therefore, an intermediary cannot translate an HTTP/2 request or response + # containing an invalid field name into an HTTP/1.1 message. + + ($frame) = grep { $_->{type} eq "HEADERS" } @$frames; + is($frame->{headers}->{'x-sent-foo'}, undef, 'malformed header not forwarded'); + is($frame->{headers}->{':status'}, 400, 'newline in request header - bad request'); + } +} -$s = Test::Nginx::HTTP2->new(); -$sid = $s->new_stream({ headers => [ - { name => ':method', value => 'GET', mode => 0 }, - { name => ':scheme', value => 'http', mode => 0 }, - { name => ':path', value => '/proxy2/', mode => 1 }, - { name => ':authority', value => 'localhost', mode => 1 }, - { name => 'x-foo', value => "x-bar\r\nreferer:see-this", mode => 2 }]}); -$frames = $s->read(all => [{ sid => $sid, fin => 1 }]); - -# 10.3. Intermediary Encapsulation Attacks -# An intermediary therefore cannot translate an HTTP/2 request or response -# containing an invalid field name into an HTTP/1.1 message. - -($frame) = grep { $_->{type} eq "HEADERS" } @$frames; -isnt($frame->{headers}->{'x-referer'}, 'see-this', 'newline in request header'); -is($frame->{headers}->{':status'}, 400, 'newline in request header - bad request'); +foreach my $malformed_value ( + "okay value", + " leading space", + "\tleading tab", + "trailing space ", + "trailing tab\t", + "\tleading and trailing tab\t", + " leading space and trailing tab\t", + " \t ", + " ", + "0", + "", +) { + $s = Test::Nginx::HTTP2->new(); + $sid = $s->new_stream({ headers => [ + { name => ':method', value => 'GET', mode => 0 }, + { name => ':scheme', value => 'http', mode => 0 }, + { name => ':path', value => '/proxy2/', mode => 2 }, + { name => ':authority', value => 'localhost', mode => 2 }, + { name => 'x-foo', value => $malformed_value, mode => 2 }]}); + $frames = $s->read(all => [{ sid => $sid, fin => 1 }]); + + # Therefore, an intermediary cannot translate an HTTP/2 request or response + # containing an invalid field name into an HTTP/1.1 message. + + ($frame) = grep { $_->{type} eq "HEADERS" } @$frames; + is($frame->{headers}->{':status'}, 200, 'value stripped'); + my $stripped = $malformed_value =~ s/(\A[ \t]*)|([ \t]*\z)//rg; + if ($stripped eq '') { + undef $stripped; + } + is($frame->{headers}->{'x-sent-foo'}, + $stripped, + 'leading and trailing space stripped'); +} # invalid header name as seen with underscore should not lead to ignoring rest diff --git a/h3_headers.t b/h3_headers.t index 2d190c8b..63361378 100644 --- a/h3_headers.t +++ b/h3_headers.t @@ -24,7 +24,7 @@ select STDERR; $| = 1; select STDOUT; $| = 1; my $t = Test::Nginx->new()->has(qw/http http_v3 proxy rewrite cryptx/) - ->has_daemon('openssl')->plan(75) + ->has_daemon('openssl')->plan(80) ->write_file_expand('nginx.conf', <<'EOF'); %%TEST_GLOBALS%% @@ -44,6 +44,7 @@ http { listen 127.0.0.1:%%PORT_8980_UDP%% quic; listen 127.0.0.1:8081; server_name localhost; + reject_leading_trailing_whitespace_client on; location / { add_header X-Sent-Foo $http_x_foo; @@ -710,28 +711,38 @@ $frames = $s->read(all => [{ sid => $sid, fin => 1 }]); is($frame->{headers}->{':status'}, 400, 'header size indexed greater'); # ensure that request header field value with newline doesn't get split -# -# 10.3. Intermediary-Encapsulation Attacks -# Requests or responses containing invalid field names MUST be treated -# as malformed. - -$s = Test::Nginx::HTTP3->new(); -$sid = $s->new_stream({ headers => [ - { name => ':method', value => 'GET', mode => 0 }, - { name => ':scheme', value => 'http', mode => 0 }, - { name => ':path', value => '/proxy2/', mode => 2 }, - { name => ':authority', value => 'localhost', mode => 2 }, - { name => 'x-foo', value => "x-bar\r\nreferer:see-this", mode => 4 }]}); -$frames = $s->read(all => [{ sid => $sid, fin => 1 }]); - -# 10.3. Intermediary Encapsulation Attacks -# Therefore, an intermediary cannot translate an HTTP/3 request or response -# containing an invalid field name into an HTTP/1.1 message. - -($frame) = grep { $_->{type} eq "HEADERS" } @$frames; -isnt($frame->{headers}->{'x-referer'}, 'see-this', 'newline in request header'); - -is($frame->{headers}->{':status'}, 400, 'newline in request header - bad request'); +my $malformed_values = 0; +foreach my $malformed_value ( + "x-bar\rreferrer: baz", + "x-bar\0referrer: junk", + "x-bar\nreferrer: junk", + " leading space", + "\tleading tab", + "trailing space ", + "trailing tab\t", +) { + $malformed_values += 1; + subtest "Malformed value test $malformed_values" => sub { + plan tests => 2; + + $s = Test::Nginx::HTTP3->new(); + $sid = $s->new_stream({ headers => [ + { name => ':method', value => 'GET', mode => 0 }, + { name => ':scheme', value => 'http', mode => 0 }, + { name => ':path', value => '/proxy2/', mode => 2 }, + { name => ':authority', value => 'localhost', mode => 2 }, + { name => 'x-foo', value => $malformed_value, mode => 4 }]}); + $frames = $s->read(all => [{ sid => $sid, fin => 1 }]); + + # 10.3. Intermediary Encapsulation Attacks + # Therefore, an intermediary cannot translate an HTTP/3 request or response + # containing an invalid field name into an HTTP/1.1 message. + + ($frame) = grep { $_->{type} eq "HEADERS" } @$frames; + is($frame->{headers}->{'x-foo'}, undef, 'malformed header not forwarded'); + is($frame->{headers}->{':status'}, 400, 'bad request'); + } +} # invalid header name as seen with underscore should not lead to ignoring rest diff --git a/proxy.t b/proxy.t index 324e451d..41a58329 100644 --- a/proxy.t +++ b/proxy.t @@ -21,7 +21,7 @@ use Test::Nginx; select STDERR; $| = 1; select STDOUT; $| = 1; -my $t = Test::Nginx->new()->has(qw/http proxy/)->plan(28); +my $t = Test::Nginx->new()->has(qw/http proxy/)->plan(30); $t->write_file_expand('nginx.conf', <<'EOF'); @@ -61,6 +61,13 @@ http { proxy_connect_timeout 2s; } + location /test-tabs { + if ($http_x_forwarded_for != '192.0.2.1') { + return 444; + } + return 200 'tabs got stripped'; + } + location /var { proxy_pass http://$arg_b; proxy_read_timeout 2s; @@ -120,6 +127,10 @@ like(http_get('/vars'), qr/X-Proxy-Host:\s127\.0\.0\.1:$p0/, 'proxy_host'); like(http_get('/vars'), qr/X-Proxy-Port:\s$p0/, 'proxy_port'); like(http_xff('/vars', '192.0.2.1'), qr/X-Proxy-Forwarded:.*192\.0\.2\.1/, 'proxy_add_x_forwarded_for'); +like(http_xff('/test-tabs', " \t 192.0.2.1\t "), qr/tabs got stripped/, + 'tabs stripped'); +like(http_xff('/test-tabs', " \t 192.0.2.1 \t"), qr/tabs got stripped/, + 'tabs stripped'); ($ct, $ht) = get('/time/header'); cmp_ok($ct, '<', 1, 'connect time - slow response header'); diff --git a/proxy_h2_headers.t b/proxy_h2_headers.t index a611190a..a5009a2c 100644 --- a/proxy_h2_headers.t +++ b/proxy_h2_headers.t @@ -40,6 +40,8 @@ http { server { listen 127.0.0.1:8080; server_name localhost; + reject_leading_trailing_whitespace off; + reject_leading_trailing_whitespace_upstream on; location / { proxy_pass http://127.0.0.1:8081; @@ -54,6 +56,18 @@ http { proxy_buffers 8 16k; } + location /bad_header_name { + proxy_pass http://127.0.0.1:8081; + proxy_http_version 2; + proxy_set_header X-Bad-Name $arg_bad_name; + } + + location /bad_header_value { + proxy_pass http://127.0.0.1:8081; + proxy_http_version 2; + proxy_set_header X-Bad-Value $arg_bad_value; + } + location /continuation { proxy_pass http://127.0.0.1:8081$uri; proxy_http_version 2; @@ -70,7 +84,7 @@ EOF $t->run_daemon(\&http_daemon); $t->waitforsocket('127.0.0.1:' . port(8081)); -$t->try_run('no proxy_http_version 2')->plan(19); +$t->try_run('no proxy_http_version 2')->plan(36); ############################################################################### @@ -94,6 +108,47 @@ like(http_get('/field/8'), qr/200 OK/, 'long header field 2'); like(http_get('/field/15'), qr/200 OK/, 'long header field 3'); like(http_get('/field/16'), qr/502 Bad/, 'long header field 4'); +sub bad_name { + my ($name, $msg) = @_; + my $output = http_get('/bad_header_name?bad_name=' . unpack('H*', $name)); + if ($name =~ /[\x00-\x20:A-Z\x7F]/) { + like($output, qr/502 Bad/, $msg); + } else { + like($output, qr/200 OK/, $output); + } +} + +sub bad_value { + my ($value, $msg) = @_; + my $output = http_get('/bad_header_value?bad_value=' . unpack('H*', $value)); + if ($value =~ /[\x00\r\n]|(\A[ \t])|([ \t]\z)/) { + like($output, qr/502 Bad/, $msg); + } else { + like($output, qr/200 OK/, $msg); + } +} + +# bad header names (all hex encoded) +bad_name('ABC', 'uppercase header name'); +bad_name('a:b', 'colon in header name'); +bad_name(':b', 'bad pseudo-header'); +bad_name('a b', 'space in header name'); +bad_name("a\rb", 'cr in header name'); +bad_name("a\nb", 'lf in header name'); +bad_name("a\0b", 'nul in header name'); +bad_name("a\x7Fb", '\x7F in header name'); +bad_name("ab", 'okay'); + +# bad header values +bad_value(' abc', 'rejected leading space'); +bad_value('abc ', 'rejected trailing space'); +bad_value("\tabc", 'rejected leading tab'); +bad_value("abc\t", 'rejected trailing tab'); +bad_value("a\nb", 'rejected nl'); +bad_value("a\rb", 'rejected cr'); +bad_value("a\000b", 'rejected nul'); +bad_value("ab", 'okay'); + # padding & priority like(http_get('/padding'), qr/200 OK/, 'padding'); @@ -138,7 +193,25 @@ sub http_daemon { my $sid = $frame->{sid}; my $uri = $frame->{headers}{':path'}; - if ($uri =~ m|mode/(\d)|) { + if ($uri =~ m|newline|) { + $c->new_stream({ headers => [ + { name => ':status', value => '404' }, + { name => 'x-junk', value => "abc\ndef" }, + ]}, $sid); + } elsif ($uri =~ m|bad_header_name|) { + $c->new_stream({ headers => [ + { name => ':status', value => '200' }, + { name => pack('H*', $frame->{headers}{'x-bad-name'}), + value => 'abc', mode => 2 }, + ]}, $sid); + } elsif ($uri =~ m|bad_header_value|) { + print STDERR (pack('H*', $frame->{headers}{'x-bad-value'}) . "\n"); + $c->new_stream({ headers => [ + { name => ':status', value => '200' }, + { name => 'abc', mode => 2, + value => pack('H*', $frame->{headers}{'x-bad-value'}) }, + ]}, $sid); + } elsif ($uri =~ m|mode/(\d)|) { my $mode = $1; $c->new_stream({ headers => [