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

@@ -540,8 +540,17 @@ parse_header_colon(Buffer, State=#state{opts=Opts, in_state=PS}, Headers) ->
 			error_terminate(431, State#state{in_state=PS#ps_header{headers=Headers}},
 				{connection_error, limit_reached,
 					'A header name is larger than configuration allows. (RFC7230 3.2.5, RFC6585 5)'});
+		nomatch ->
+			%% 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.

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