Skip to content

Test handling of leading and trailing spaces and tabs #37

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DemiMarie wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Test handling of leading and trailing spaces and tabs #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 63 additions & 17 deletions h2_headers.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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%%
Expand Down Expand Up @@ -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

Expand Down
57 changes: 34 additions & 23 deletions h3_headers.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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%%
Expand All @@ -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;
Expand Down Expand Up @@ -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

Expand Down
13 changes: 12 additions & 1 deletion proxy.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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');
Expand Down
77 changes: 75 additions & 2 deletions proxy_h2_headers.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);

###############################################################################

Expand All @@ -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');
Expand Down Expand Up @@ -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 => [
Expand Down