From a14ecf19c68ba5b9eb828a41356b1adbc1c5739c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Wed, 2 Oct 2019 13:31:13 +0200 Subject: Add more HTTP/1.1 header parsing tests Fix a case where Cowboy was waiting for more data that simply did not come. Now Cowboy will generate an error immediately when a header line has no colon separator. These test cases come from known request smuggling attack vectors. Cowboy was not vulnerable to any of them. --- src/cowboy_http.erl | 11 ++++++- test/rfc7230_SUITE.erl | 82 +++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 84 insertions(+), 9 deletions(-) diff --git a/src/cowboy_http.erl b/src/cowboy_http.erl index 5136a3b..a6c640a 100644 --- a/src/cowboy_http.erl +++ b/src/cowboy_http.erl @@ -541,7 +541,16 @@ parse_header_colon(Buffer, State=#state{opts=Opts, in_state=PS}, Headers) -> {connection_error, limit_reached, 'A header name is larger than configuration allows. (RFC7230 3.2.5, RFC6585 5)'}); nomatch -> - {more, State#state{in_state=PS#ps_header{headers=Headers}}, Buffer}; + %% We don't have a colon but we might have an invalid header line, + %% so check if we have an LF and abort with an error if we do. + case match_eol(Buffer, 0) of + nomatch -> + {more, State#state{in_state=PS#ps_header{headers=Headers}}, Buffer}; + _ -> + error_terminate(400, State#state{in_state=PS#ps_header{headers=Headers}}, + {connection_error, protocol_error, + 'A header line is missing a colon separator. (RFC7230 3.2.4)'}) + end; _ -> parse_hd_name(Buffer, State, Headers, <<>>) end. diff --git a/test/rfc7230_SUITE.erl b/test/rfc7230_SUITE.erl index 6aefd8e..6752c30 100644 --- a/test/rfc7230_SUITE.erl +++ b/test/rfc7230_SUITE.erl @@ -673,6 +673,14 @@ reject_two_sp_between_request_target_and_version(Config) -> %% Request version. +reject_invalid_version_http09(Config) -> + doc("Any version number other than HTTP/1.0 or HTTP/1.1 must be " + "rejected by a server or intermediary with a 505 status code. (RFC7230 2.6, RFC7230 A.2)"), + #{code := 505} = do_raw(Config, + "GET / HTTP/0.9\r\n" + "Host: localhost\r\n" + "\r\n"). + reject_invalid_version_http100(Config) -> doc("Any version number other than HTTP/1.0 or HTTP/1.1 must be " "rejected by a server or intermediary with a 505 status code. (RFC7230 2.6, RFC7230 A.2)"), @@ -766,21 +774,52 @@ reject_whitespace_before_header_name(Config) -> doc("Messages that contain whitespace before the header name must " "be rejected with a 400 status code and the closing of the " "connection. (RFC7230 3.2.4)"), - #{code := 400, client := Client} = do_raw(Config, [ + #{code := 400, client := Client1} = do_raw(Config, [ "GET / HTTP/1.1\r\n" " Host: localhost\r\n" "\r\n"]), - {error, closed} = raw_recv(Client, 0, 1000). + {error, closed} = raw_recv(Client1, 0, 1000), + #{code := 400, client := Client2} = do_raw(Config, [ + "GET / HTTP/1.1\r\n" + "\tHost: localhost\r\n" + "\r\n"]), + {error, closed} = raw_recv(Client2, 0, 1000). reject_whitespace_between_header_name_and_colon(Config) -> doc("Messages that contain whitespace between the header name and " "colon must be rejected with a 400 status code and the closing " "of the connection. (RFC7230 3.2.4)"), - #{code := 400, client := Client} = do_raw(Config, [ + #{code := 400, client := Client1} = do_raw(Config, [ "GET / HTTP/1.1\r\n" "Host : localhost\r\n" "\r\n"]), - {error, closed} = raw_recv(Client, 0, 1000). + {error, closed} = raw_recv(Client1, 0, 1000), + #{code := 400, client := Client2} = do_raw(Config, [ + "GET / HTTP/1.1\r\n" + "Host\t: localhost\r\n" + "\r\n"]), + {error, closed} = raw_recv(Client2, 0, 1000). + +reject_header_name_without_colon(Config) -> + doc("Messages that contain a header name that is not followed by a " + "colon must be rejected with a 400 status code and the closing " + "of the connection. (RFC7230 3.2.4)"), + #{code := 400, client := Client1} = do_raw(Config, [ + "GET / HTTP/1.1\r\n" + "Host\r\n" + "\r\n"]), + {error, closed} = raw_recv(Client1, 0, 1000), + #{code := 400, client := Client2} = do_raw(Config, [ + "GET / HTTP/1.1\r\n" + "Host localhost\r\n" + "\r\n"]), + {error, closed} = raw_recv(Client2, 0, 1000), + #{code := 400, client := Client3} = do_raw(Config, [ + "GET / HTTP/1.1\r\n" + "Host\r\n" + " : localhost\r\n" + "\r\n"]), + {error, closed} = raw_recv(Client3, 0, 1000). limit_header_name(Config) -> doc("The header name must be subject to a configurable limit. A " @@ -828,6 +867,17 @@ drop_whitespace_after_header_value(Config) -> "\r\n" "Hello world!"]). +reject_lf_line_breaks(Config) -> + doc("A server may accept header names separated by a single LF, instead of " + "CRLF. Cowboy rejects all requests that use LF as separator. (RFC7230 3.5)"), + #{code := 400, client := Client} = do_raw(Config, [ + "POST /echo/read_body HTTP/1.1\r\n" + "Host: localhost\n" + "Transfer-encoding: chunked\r\n" + "\r\n" + "6\r\nHello \r\n5\r\nworld\r\n1\r\n!\r\n0\r\n\r\n"]), + {error, closed} = raw_recv(Client, 0, 1000). + %@todo %The order of header fields with differing names is not significant. (RFC7230 3.2.2) % @@ -1023,13 +1073,21 @@ reject_non_terminal_chunked(Config) -> doc("Messages where chunked, when present, is not the last " "transfer-encoding must be rejected with a 400 status code " "and the closing of the connection. (RFC7230 3.3.3)"), - #{code := 400, client := Client} = do_raw(Config, [ + #{code := 400, client := Client1} = do_raw(Config, [ "POST / HTTP/1.1\r\n" "Host: localhost\r\n" "Transfer-encoding: chunked, gzip\r\n" "\r\n", zlib:gzip(<<"6\r\nHello \r\n5\r\nworld\r\n1\r\n!\r\n0\r\n\r\n">>)]), - {error, closed} = raw_recv(Client, 0, 1000). + {error, closed} = raw_recv(Client1, 0, 1000), + #{code := 400, client := Client2} = do_raw(Config, [ + "POST / HTTP/1.1\r\n" + "Host: localhost\r\n" + "Transfer-encoding: chunked\r\n" + "Transfer-encoding: gzip\r\n" + "\r\n", + zlib:gzip(<<"6\r\nHello \r\n5\r\nworld\r\n1\r\n!\r\n0\r\n\r\n">>)]), + {error, closed} = raw_recv(Client2, 0, 1000). %@todo %Some non-conformant implementations send the "deflate" compressed @@ -1039,13 +1097,21 @@ reject_unknown_transfer_encoding(Config) -> doc("Messages encoded with a transfer-encoding the server does not " "understand must be rejected with a 501 status code and the " "closing of the connection. (RFC7230 3.3.1)"), - #{code := 400, client := Client} = do_raw(Config, [ + #{code := 400, client := Client1} = do_raw(Config, [ "POST / HTTP/1.1\r\n" "Host: localhost\r\n" "Transfer-encoding: unknown, chunked\r\n" "\r\n", "6\r\nHello \r\n5\r\nworld\r\n1\r\n!\r\n0\r\n\r\n"]), - {error, closed} = raw_recv(Client, 0, 1000). + {error, closed} = raw_recv(Client1, 0, 1000), + #{code := 400, client := Client2} = do_raw(Config, [ + "POST / HTTP/1.1\r\n" + "Host: localhost\r\n" + "Transfer-encoding: unknown\r\n" + "Transfer-encoding: chunked\r\n" + "\r\n", + "6\r\nHello \r\n5\r\nworld\r\n1\r\n!\r\n0\r\n\r\n"]), + {error, closed} = raw_recv(Client2, 0, 1000). %@todo %A server may reject requests with a body and no content-length -- cgit v1.2.3