From 5984409d1264871cbe61bfec875de53e51713efb Mon Sep 17 00:00:00 2001
From: Steve Vinoski
Date: Thu, 24 Nov 2011 21:03:33 -0500
Subject: honor packet_size for http packet parsing to fix OTP-9389
Allow applications to use a packet_size setting on a socket to control
acceptable HTTP header line length. This gives them the ability to
accept HTTP headers larger than the default settings allow, but also
lets them avoid DOS attacks by accepting header lines only up to
whatever length they wish to allow.
Without this change, if an HTTP request/response line or header
arrives on a socket in http, http_bin, httph, or httph_bin parsing
mode, and the request/response line or header is too long to fit into
a default inet_drv buffer of 1460 bytes, an unexpected error
occurs. These problems were described and discussed on
erlang-questions in June 2011 in this thread:
http://erlang.org/pipermail/erlang-questions/2011-June/059563.html
In the original code, no buffer reallocation occurs to enlarge the
buffer, even if packet_size or line_length are set in a way that
should allow the HTTP data to be parsed properly. The only available
workaround was to collect headers and parse them using
erlang:decode_packet, but that approach has drawbacks such as having
to collect all HTTP header data before it can be handed to
decode_packet for correct parsing, and also requiring each and every
Erlang web server developer/maintainer to add the workaround to his or
her web server.
Change the packet parser to honor the packet_size setting for HTTP
parsing. If packet_size is set, and an HTTP request/response or header
line exceeds the default 1460 byte TCP buffer limit, return an
indication to tcp_remain that it should realloc the buffer to enlarge
it to packet_size. Also fix the HTTP parsing code to properly honor
line_length by truncating any HTTP request/response or header lines
that exceed that setting.
For backward compatibility, default behavior is unchanged; if an
application wants to be able to accept long HTTP header lines, it must
set packet_size to an appropriate value. Buffer reallocation occurs
only when needed, so the original default buffer size in the code is
still the default.
Make the line mode parsing honor packet_size as well, for consistency.
Add new regression tests to the emulator decode_packet suite and also
to the kernel gen_tcp_misc suite.
The documentation for packet_size in inet:setopts/2 is already
sufficient.
Many thanks to Sverker Eriksson for his guidance on how to best fix
this bug and also for reviewing a number of patch attempts prior to
this one.
---
lib/kernel/test/gen_tcp_echo_SUITE.erl | 11 +++---
lib/kernel/test/gen_tcp_misc_SUITE.erl | 66 ++++++++++++++++++++++++++++++++--
2 files changed, 69 insertions(+), 8 deletions(-)
(limited to 'lib')
diff --git a/lib/kernel/test/gen_tcp_echo_SUITE.erl b/lib/kernel/test/gen_tcp_echo_SUITE.erl
index fffaaf4c45..5bbaeb02ad 100644
--- a/lib/kernel/test/gen_tcp_echo_SUITE.erl
+++ b/lib/kernel/test/gen_tcp_echo_SUITE.erl
@@ -167,8 +167,12 @@ echo_test_1(SockOpts, EchoFun, Config0) ->
[{type, {cdr, little}}|Config]),
?line case lists:keymember(packet_size, 1, SockOpts) of
false ->
- ?line echo_packet([{packet, line}|SockOpts],
- EchoFun, Config);
+ % This is cheating, we should test that packet_size
+ % also works for line and http.
+ echo_packet([{packet, line}|SockOpts], EchoFun, Config),
+ echo_packet([{packet, http}|SockOpts], EchoFun, Config),
+ echo_packet([{packet, http_bin}|SockOpts], EchoFun, Config);
+
true -> ok
end,
?line echo_packet([{packet, tpkt}|SockOpts], EchoFun, Config),
@@ -183,9 +187,6 @@ echo_test_1(SockOpts, EchoFun, Config0) ->
[{type, {asn1, short, LongTag}}|Config]),
?line echo_packet([{packet, asn1}|SockOpts], EchoFun,
[{type, {asn1, long, LongTag}}|Config]),
-
- ?line echo_packet([{packet, http}|SockOpts], EchoFun, Config),
- ?line echo_packet([{packet, http_bin}|SockOpts], EchoFun, Config),
ok.
echo_packet(SockOpts, EchoFun, Opts) ->
diff --git a/lib/kernel/test/gen_tcp_misc_SUITE.erl b/lib/kernel/test/gen_tcp_misc_SUITE.erl
index b1ef8826d5..c1c5ff8b81 100644
--- a/lib/kernel/test/gen_tcp_misc_SUITE.erl
+++ b/lib/kernel/test/gen_tcp_misc_SUITE.erl
@@ -40,7 +40,8 @@
accept_timeouts_in_order3/1,accept_timeouts_mixed/1,
killing_acceptor/1,killing_multi_acceptors/1,killing_multi_acceptors2/1,
several_accepts_in_one_go/1,active_once_closed/1, send_timeout/1, send_timeout_active/1,
- otp_7731/1, zombie_sockets/1, otp_7816/1, otp_8102/1]).
+ otp_7731/1, zombie_sockets/1, otp_7816/1, otp_8102/1,
+ otp_9389/1]).
%% Internal exports.
-export([sender/3, not_owner/1, passive_sockets_server/2, priority_server/1,
@@ -72,7 +73,7 @@ all() ->
killing_acceptor, killing_multi_acceptors,
killing_multi_acceptors2, several_accepts_in_one_go,
active_once_closed, send_timeout, send_timeout_active, otp_7731,
- zombie_sockets, otp_7816, otp_8102].
+ zombie_sockets, otp_7816, otp_8102, otp_9389].
groups() ->
[].
@@ -2479,4 +2480,63 @@ otp_8102_do(LSocket, PortNum, {Bin,PType}) ->
io:format("Got error msg, ok.\n",[]),
gen_tcp:close(SSocket),
gen_tcp:close(RSocket).
-
+
+otp_9389(doc) -> ["Verify packet_size handles long HTTP header lines"];
+otp_9389(suite) -> [];
+otp_9389(Config) when is_list(Config) ->
+ ?line {ok, LS} = gen_tcp:listen(0, []),
+ ?line {ok, {_, PortNum}} = inet:sockname(LS),
+ io:format("Listening on ~w with port number ~p\n", [LS, PortNum]),
+ OrigLinkHdr = "/" ++ string:chars($S, 8192),
+ _Server = spawn_link(
+ fun() ->
+ ?line {ok, S} = gen_tcp:accept(LS),
+ ?line ok = inet:setopts(S, [{packet_size, 16384}]),
+ ?line ok = otp_9389_loop(S, OrigLinkHdr),
+ ?line ok = gen_tcp:close(S)
+ end),
+ ?line {ok, S} = gen_tcp:connect("localhost", PortNum,
+ [binary, {active, false}]),
+ Req = "GET / HTTP/1.1\r\n"
+ ++ "Host: localhost\r\n"
+ ++ "Link: " ++ OrigLinkHdr ++ "\r\n\r\n",
+ ?line ok = gen_tcp:send(S, Req),
+ ?line ok = inet:setopts(S, [{packet, http}]),
+ ?line {ok, {http_response, {1,1}, 200, "OK"}} = gen_tcp:recv(S, 0),
+ ?line ok = inet:setopts(S, [{packet, httph}, {packet_size, 16384}]),
+ ?line {ok, {http_header, _, 'Content-Length', _, "0"}} = gen_tcp:recv(S, 0),
+ ?line {ok, {http_header, _, "Link", _, LinkHdr}} = gen_tcp:recv(S, 0),
+ ?line true = (LinkHdr == OrigLinkHdr),
+ ok = gen_tcp:close(S),
+ ok = gen_tcp:close(LS),
+ ok.
+
+otp_9389_loop(S, OrigLinkHdr) ->
+ ?line ok = inet:setopts(S, [{active,once},{packet,http}]),
+ receive
+ {http, S, {http_request, 'GET', _, _}} ->
+ ?line ok = otp_9389_loop(S, OrigLinkHdr, undefined)
+ after
+ 3000 ->
+ ?line error({timeout,request_line})
+ end.
+otp_9389_loop(S, OrigLinkHdr, ok) ->
+ ?line Resp = "HTTP/1.1 200 OK\r\nContent-length: 0\r\n" ++
+ "Link: " ++ OrigLinkHdr ++ "\r\n\r\n",
+ ?line ok = gen_tcp:send(S, Resp);
+otp_9389_loop(S, OrigLinkHdr, State) ->
+ ?line ok = inet:setopts(S, [{active,once}, {packet,httph}]),
+ receive
+ {http, S, http_eoh} ->
+ ?line otp_9389_loop(S, OrigLinkHdr, ok);
+ {http, S, {http_header, _, "Link", _, LinkHdr}} ->
+ ?line LinkHdr = OrigLinkHdr,
+ ?line otp_9389_loop(S, OrigLinkHdr, State);
+ {http, S, {http_header, _, _Hdr, _, _Val}} ->
+ ?line otp_9389_loop(S, OrigLinkHdr, State);
+ {http, S, {http_error, Err}} ->
+ ?line error({error, Err})
+ after
+ 3000 ->
+ ?line error({timeout,header})
+ end.
--
cgit v1.2.3
From dc5f7190f16cf4552db74fba3f4e0f2d654e2594 Mon Sep 17 00:00:00 2001
From: Sverker Eriksson
Date: Mon, 28 Nov 2011 15:14:14 +0100
Subject: erts: Remove truncation of http packet parsing and return error
instead
This is a slight modification of previous commit by Steve Vinoski
For backward compatibility of old users of decode_packet, I think it's enough
to return error instead of keeping the old line truncation behaviour.
---
lib/kernel/doc/src/inet.xml | 4 ++++
1 file changed, 4 insertions(+)
(limited to 'lib')
diff --git a/lib/kernel/doc/src/inet.xml b/lib/kernel/doc/src/inet.xml
index fad5af85bb..1a05b4ba99 100644
--- a/lib/kernel/doc/src/inet.xml
+++ b/lib/kernel/doc/src/inet.xml
@@ -573,6 +573,10 @@ fe80::204:acff:fe17:bf38
is longer than the max allowed length, the packet is
considered invalid. The same happens if the packet header
is too big for the socket receive buffer.
+ For line oriented protocols (line,http*),
+ option packet_size also guarantees that lines up to the
+ indicated length are accepted and not considered invalid due
+ to internal buffer limitations.
{read_packets, Integer}(UDP sockets)
-
--
cgit v1.2.3