[PATCH 00 of 10] read hardening
Maxim Dounin
mdounin at mdounin.ru
Sun Mar 17 23:04:27 UTC 2024
Hello!
On Fri, Mar 15, 2024 at 09:14:14PM +0300, Maxim Dounin wrote:
> Here is a patch series which introduces various hardening in data
> reading code paths. Review and testing appreciated.
And below are tests for the patch series.
# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1710640481 -10800
# Sun Mar 17 04:54:41 2024 +0300
# Node ID 3b364d68e7fb254920b10c0aec4b3c24b0240f92
# Parent 1867428f1673214f6e2bb647a2bba0ae3a77bcb7
Tests: test for long commands with SMTP pipelining.
diff --git a/mail_smtp.t b/mail_smtp.t
--- a/mail_smtp.t
+++ b/mail_smtp.t
@@ -98,7 +98,7 @@ http {
EOF
$t->run_daemon(\&Test::Nginx::SMTP::smtp_test_daemon);
-$t->run()->plan(41);
+$t->run()->plan(43);
$t->waitforsocket('127.0.0.1:' . port(8026));
@@ -284,6 +284,28 @@ my $s = Test::Nginx::SMTP->new();
$s->ok('long pipelined rcpt to 4');
$s->ok('long pipelined rset');
+# Pipelining longer than smtp_client_buffer, with
+# extra pipelined commands to be processed by nginx itself
+
+$s = Test::Nginx::SMTP->new(PeerAddr => '127.0.0.1:' . port(8027));
+$s->read();
+$s->send('EHLO example.com');
+$s->read();
+
+$s->send('MAIL FROM:<test at example.com> FOO=' . ('X' x 90) . CRLF
+ . 'RCPT TO:<test at example.com>' . CRLF
+ . 'RSET');
+
+$s->read();
+
+TODO: {
+local $TODO = 'not yet';
+
+$s->ok('pipelined long rcpt to');
+$s->ok('pipelined long rset');
+
+}
+
# Connection must stay even if error returned to rcpt to command
$s = Test::Nginx::SMTP->new();
# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1710640483 -10800
# Sun Mar 17 04:54:43 2024 +0300
# Node ID d81108e6e92bded9f5c6ba4f66268ef5ca17a443
# Parent 3b364d68e7fb254920b10c0aec4b3c24b0240f92
Tests: mail max_commands tests.
diff --git a/mail_max_commands.t b/mail_max_commands.t
new file mode 100644
--- /dev/null
+++ b/mail_max_commands.t
@@ -0,0 +1,127 @@
+#!/usr/bin/perl
+
+# (C) Maxim Dounin
+
+# Tests for mail max_commands.
+
+###############################################################################
+
+use warnings;
+use strict;
+
+use Test::More;
+use Socket qw/ CRLF /;
+
+BEGIN { use FindBin; chdir($FindBin::Bin); }
+
+use lib 'lib';
+use Test::Nginx;
+use Test::Nginx::IMAP;
+use Test::Nginx::POP3;
+use Test::Nginx::SMTP;
+
+###############################################################################
+
+select STDERR; $| = 1;
+select STDOUT; $| = 1;
+
+local $SIG{PIPE} = 'IGNORE';
+
+my $t = Test::Nginx->new()->has(qw/mail imap pop3 smtp/)
+ ->write_file_expand('nginx.conf', <<'EOF');
+
+%%TEST_GLOBALS%%
+
+daemon off;
+
+events {
+}
+
+mail {
+ auth_http http://127.0.0.1:8080; # unused
+
+ max_commands 1;
+
+ server {
+ listen 127.0.0.1:8143;
+ protocol imap;
+ }
+
+ server {
+ listen 127.0.0.1:8110;
+ protocol pop3;
+ }
+
+ server {
+ listen 127.0.0.1:8025;
+ protocol smtp;
+ }
+}
+
+EOF
+
+$t->try_run('no max_commands')->plan(18);
+
+###############################################################################
+
+# imap
+
+my $s = Test::Nginx::IMAP->new();
+$s->read();
+
+$s->send('a01 NOOP');
+$s->check(qr/^a01 OK/, 'imap first noop');
+$s->send('a02 NOOP');
+$s->check(qr/^a02 BAD/, 'imap second noop rejected');
+$s->send('a03 NOOP');
+$s->check(qr/^$/, 'imap max commands');
+
+$s = Test::Nginx::IMAP->new();
+$s->read();
+
+$s->send('a01 NOOP' . CRLF . 'a02 NOOP' . CRLF . 'a03 NOOP');
+$s->check(qr/^a01 OK/, 'imap pipelined first noop');
+$s->check(qr/^a02 BAD/, 'imap pipelined second noop rejected');
+$s->check(qr/^$/, 'imap pipelined max commands');
+
+# pop3
+
+$s = Test::Nginx::POP3->new();
+$s->read();
+
+$s->send('NOOP');
+$s->check(qr/^\+OK/, 'pop3 first noop');
+$s->send('NOOP');
+$s->check(qr/^-ERR/, 'pop3 second noop');
+$s->send('NOOP');
+$s->check(qr/^$/, 'pop3 max commands');
+
+$s = Test::Nginx::POP3->new();
+$s->read();
+
+$s->send('NOOP' . CRLF . 'NOOP' . CRLF . 'NOOP');
+$s->check(qr/^\+OK/, 'pop3 pipelined first noop');
+$s->check(qr/^-ERR/, 'pop3 pipelined second noop rejected');
+$s->check(qr/^$/, 'pop3 pipelined max commands');
+
+# smtp
+
+$s = Test::Nginx::SMTP->new();
+$s->read();
+
+$s->send('RSET');
+$s->check(qr/^2.. /, 'smtp first rset');
+$s->send('RSET');
+$s->check(qr/^5.. /, 'smtp second rset rejected');
+$s->send('RSET');
+$s->check(qr/^$/, 'smtp max commands');
+
+$s = Test::Nginx::SMTP->new();
+$s->read();
+
+$s->send('RSET' . CRLF . 'RSET' . CRLF . 'RSET');
+$s->check(qr/^2.. /, 'smtp pipelined first rset');
+$s->check(qr/^5.. /, 'smtp pipelined second rset rejected');
+$s->check(qr/^$/, 'smtp pipelined max commands');
+
+###############################################################################
# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1710640487 -10800
# Sun Mar 17 04:54:47 2024 +0300
# Node ID c8100b4fc4597fc0c1c59e3751cbfbe5415d159a
# Parent d81108e6e92bded9f5c6ba4f66268ef5ca17a443
Tests: tests for request body chunked extensions and trailers.
diff --git a/body_chunked.t b/body_chunked.t
--- a/body_chunked.t
+++ b/body_chunked.t
@@ -22,7 +22,7 @@ use Test::Nginx;
select STDERR; $| = 1;
select STDOUT; $| = 1;
-my $t = Test::Nginx->new()->has(qw/http proxy rewrite/)->plan(18);
+my $t = Test::Nginx->new()->has(qw/http proxy rewrite/)->plan(24);
$t->write_file_expand('nginx.conf', <<'EOF');
@@ -119,6 +119,8 @@ like(http_get_body('/single', '012345678
qr/X-Body: (0123456789){128}\x0d?$/ms, 'body in single buffer');
like(http_get_body('/large', '0123456789' x 128), qr/ 413 /, 'body too large');
+like(http_get_body('/large', 'X' x 1024), qr/ 200 /, 'body exact limit');
+like(http_get_body('/large', 'X' x 1025), qr/ 413 /, 'body just above limit');
# pipelined requests
@@ -162,6 +164,66 @@ like(
qr/400 Bad/, 'runaway chunk discard'
);
+# chunk extensions and trailers
+
+like(
+ http(
+ 'GET /large HTTP/1.1' . CRLF
+ . 'Host: localhost' . CRLF
+ . 'Connection: close' . CRLF
+ . 'Transfer-Encoding: chunked' . CRLF . CRLF
+ . ('1; foo' . CRLF . 'X' . CRLF) x 16
+ . '0' . CRLF . CRLF
+ ),
+ qr/ 200 /, 'chunk extensions'
+);
+
+TODO: {
+local $TODO = 'not yet';
+
+like(
+ http(
+ 'GET /large HTTP/1.1' . CRLF
+ . 'Host: localhost' . CRLF
+ . 'Connection: close' . CRLF
+ . 'Transfer-Encoding: chunked' . CRLF . CRLF
+ . ('1; foo' . CRLF . 'X' . CRLF) x 512
+ . '0' . CRLF . CRLF
+ ),
+ qr/ 413 /, 'too many chunk extensions'
+);
+
+}
+
+like(
+ http(
+ 'GET /large HTTP/1.1' . CRLF
+ . 'Host: localhost' . CRLF
+ . 'Connection: close' . CRLF
+ . 'Transfer-Encoding: chunked' . CRLF . CRLF
+ . '1' . CRLF . 'X' . CRLF
+ . '0' . CRLF . ('X-Trailer: foo' . CRLF) x 16 . CRLF
+ ),
+ qr/ 200 /, 'trailers'
+);
+
+TODO: {
+local $TODO = 'not yet';
+
+like(
+ http(
+ 'GET /large HTTP/1.1' . CRLF
+ . 'Host: localhost' . CRLF
+ . 'Connection: close' . CRLF
+ . 'Transfer-Encoding: chunked' . CRLF . CRLF
+ . '1' . CRLF . 'X' . CRLF
+ . '0' . CRLF . ('X-Trailer: foo' . CRLF) x 512 . CRLF
+ ),
+ qr/ 413 /, 'too many trailers'
+);
+
+}
+
# proxy_next_upstream
like(http_get_body('/next', '0123456789'),
# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1710640490 -10800
# Sun Mar 17 04:54:50 2024 +0300
# Node ID 7ea29608aae02f2df98d5918d0bca4e2d5737129
# Parent c8100b4fc4597fc0c1c59e3751cbfbe5415d159a
Tests: basic request parsing tests.
diff --git a/http_request.t b/http_request.t
new file mode 100644
--- /dev/null
+++ b/http_request.t
@@ -0,0 +1,190 @@
+#!/usr/bin/perl
+
+# (C) Maxim Dounin
+
+# Tests for basic HTTP request parsing.
+
+###############################################################################
+
+use warnings;
+use strict;
+
+use Test::More;
+
+use Socket qw/ CRLF CR LF /;
+
+BEGIN { use FindBin; chdir($FindBin::Bin); }
+
+use lib 'lib';
+use Test::Nginx;
+
+###############################################################################
+
+select STDERR; $| = 1;
+select STDOUT; $| = 1;
+
+my $t = Test::Nginx->new()->has(qw/http rewrite/)->plan(40)
+ ->write_file_expand('nginx.conf', <<'EOF');
+
+%%TEST_GLOBALS%%
+
+daemon off;
+
+events {
+}
+
+http {
+ %%TEST_GLOBALS_HTTP%%
+
+ server {
+ listen 127.0.0.1:8080;
+ return 200 ok\n;
+ }
+}
+
+EOF
+
+$t->run();
+
+###############################################################################
+
+# some basic HTTP/0.9, HTTP/1.0, and HTTP/1.1 requests
+
+like(http(
+ "GET /" . CRLF
+), qr/^ok/s, 'http/0.9 request');
+
+like(http(
+ "GET / HTTP/1.0" . CRLF .
+ CRLF
+), qr/ 200 /, 'http/1.0 request');
+
+like(http(
+ "GET / HTTP/1.0" . CRLF .
+ "Host: foo" . CRLF .
+ CRLF
+), qr/ 200 /, 'http/1.0 request with host');
+
+like(http(
+ "GET / HTTP/1.1" . CRLF .
+ "Host: foo" . CRLF .
+ "Connection: close" . CRLF .
+ CRLF
+), qr/ 200 /, 'http/1.1 request');
+
+like(http(
+ "GET / HTTP/1.1" . CRLF .
+ "Connection: close" . CRLF .
+ CRLF
+), qr/ 400 /, 'http/1.1 request rejected without host');
+
+like(http(
+ "GET http://foo/ HTTP/1.1" . CRLF .
+ "Host: foo" . CRLF .
+ "Connection: close" . CRLF .
+ CRLF
+), qr/ 200 /, 'http/1.1 request absolute form');
+
+# ensure an empty line is ignored before the request
+
+like(http(CRLF . "GET / HTTP/1.0" . CRLF . CRLF), qr/ 200 /,
+ 'empty line ignored');
+like(http(LF . "GET / HTTP/1.0" . CRLF . CRLF), qr/ 200 /,
+ 'empty line with just LF ignored');
+
+TODO: {
+local $TODO = 'not yet';
+
+like(http(CR . "GET / HTTP/1.0" . CRLF . CRLF), qr/ 400 /,
+ 'empty line with just CR rejected');
+like(http(CRLF . CRLF . "GET / HTTP/1.0" . CRLF . CRLF), qr/ 400 /,
+ 'multiple empty lines rejected');
+like(http(LF . LF . "GET / HTTP/1.0" . CRLF . CRLF), qr/ 400 /,
+ 'multiple LFs rejected');
+like(http(CR . CR . "GET / HTTP/1.0" . CRLF . CRLF), qr/ 400 /,
+ 'multiple CRs rejected');
+
+}
+
+# method
+
+like(http("FOO / HTTP/1.0" . CRLF . CRLF), qr/ 200 /, 'method');
+like(http("FOO-BAR / HTTP/1.0" . CRLF . CRLF), qr/ 200 /,
+ 'method with dash');
+like(http("FOO_BAR / HTTP/1.0" . CRLF . CRLF), qr/ 200 /,
+ 'method with underscore');
+like(http("FOO.BAR / HTTP/1.0" . CRLF . CRLF), qr/ 400 /,
+ 'method with dot rejected');
+like(http("get / HTTP/1.0" . CRLF . CRLF), qr/ 400 /,
+ 'method in lowercase rejected');
+
+# URI
+
+like(http("GET /foo12.bar HTTP/1.0" . CRLF . CRLF), qr/ 200 /, 'uri');
+like(http("GET /control\x0d HTTP/1.0" . CRLF . CRLF), qr/ 400 /,
+ 'uri with CR');
+like(http("GET /control\x01 HTTP/1.0" . CRLF . CRLF), qr/ 400 /,
+ 'uri with control');
+like(http("GET /control\t HTTP/1.0" . CRLF . CRLF), qr/ 400 /,
+ 'uri with tab');
+
+# version
+
+like(http(
+ "GET / HTTP/1.2" . CRLF .
+ "Host: foo" . CRLF .
+ "Connection: close" . CRLF .
+ CRLF
+), qr/ 200 /, 'version 1.2');
+
+like(http(
+ "GET / HTTP/1.99" . CRLF .
+ "Host: foo" . CRLF .
+ "Connection: close" . CRLF .
+ CRLF
+), qr/ 200 /, 'version 1.99');
+
+like(http("GET / HTTP/1.000" . CRLF . CRLF), qr/ 200 /,
+ 'version leading zeros');
+like(http("GET / HTTP/2.0" . CRLF . CRLF), qr/ 505 /,
+ 'version too high rejected');
+like(http("GET / HTTP/1.x" . CRLF . CRLF), qr/ 400 /,
+ 'version non-numeric rejected');
+like(http("GET / HTTP/1.100" . CRLF . CRLF), qr/ 400 /,
+ 'version too high minor rejected');
+
+like(http("GET / http/1.0" . CRLF . CRLF), qr/ 400 /,
+ 'lowercase protocol rejected');
+
+# spaces in request line
+
+like(http("GET / HTTP/1.0 " . CRLF . CRLF), qr/ 200 /,
+ 'spaces after version');
+like(http("GET / HTTP/1.0" . CRLF . CRLF), qr/ 200 /,
+ 'spaces after uri');
+like(http("GET / HTTP/1.0" . CRLF . CRLF), qr/ 200 /,
+ 'spaces before uri');
+
+like(http("GET / HTTP/ 1.0" . CRLF . CRLF), qr/ 400 /,
+ 'spaces before version rejected');
+like(http("GET / HTTP /1.0" . CRLF . CRLF), qr/ 400 /,
+ 'spaces after protocol rejected');
+like(http("GET / HT TP/1.0" . CRLF . CRLF), qr/ 400 /,
+ 'spaces within protocol rejected');
+like(http(" GET / HTTP/ 1.0" . CRLF . CRLF), qr/ 400 /,
+ 'spaces before method rejected');
+
+# headers
+
+like(http("GET / HTTP/1.0" . CRLF . "Foo: bar" . CRLF . CRLF), qr/ 200 /,
+ 'header');
+like(http("GET / HTTP/1.0" . CRLF . "Foo : bar" . CRLF . CRLF), qr/ 400 /,
+ 'header with space rejected');
+like(http("GET / HTTP/1.0" . CRLF . " Foo: bar" . CRLF . CRLF), qr/ 400 /,
+ 'header with leading space rejected');
+like(http("GET / HTTP/1.0" . CRLF . "Foo\x01: bar" . CRLF . CRLF), qr/ 400 /,
+ 'header with control rejected');
+like(http("GET / HTTP/1.0" . CRLF . "Foo\t: bar" . CRLF . CRLF), qr/ 400 /,
+ 'header with tab rejected');
+
+###############################################################################
--
Maxim Dounin
http://mdounin.ru/
More information about the nginx-devel
mailing list