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. --- erts/emulator/test/decode_packet_SUITE.erl | 62 +++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 2 deletions(-) (limited to 'erts/emulator/test') diff --git a/erts/emulator/test/decode_packet_SUITE.erl b/erts/emulator/test/decode_packet_SUITE.erl index c0499554eb..55ef05079f 100644 --- a/erts/emulator/test/decode_packet_SUITE.erl +++ b/erts/emulator/test/decode_packet_SUITE.erl @@ -26,12 +26,14 @@ -export([all/0, suite/0,groups/0,init_per_suite/1, end_per_suite/1, init_per_group/2,end_per_group/2, init_per_testcase/2,end_per_testcase/2, - basic/1, packet_size/1, neg/1, http/1, line/1, ssl/1, otp_8536/1]). + basic/1, packet_size/1, neg/1, http/1, line/1, ssl/1, otp_8536/1, + otp_9389/1, otp_9389_line/1]). suite() -> [{ct_hooks,[ts_install_cth]}]. all() -> - [basic, packet_size, neg, http, line, ssl, otp_8536]. + [basic, packet_size, neg, http, line, ssl, otp_8536, + otp_9389, otp_9389_line]. groups() -> []. @@ -251,6 +253,28 @@ packet_size(Config) when is_list(Config) -> ?line {error,_} = decode_pkt(4,<>) end, lists:seq(-10,-1)), + + %% Test OTP-9389, long HTTP header lines. + Opts = [{packet_size, 128}], + Pkt = list_to_binary(["GET / HTTP/1.1\r\nHost: localhost\r\nLink: /", + string:chars($Y, 64), "\r\n\r\n"]), + <> = Pkt, + ?line {ok, {http_request,'GET',{abs_path,"/"},{1,1}}, Rest1} = + erlang:decode_packet(http, Pkt1, Opts), + ?line {ok, {http_header,_,'Host',_,"localhost"}, Rest2} = + erlang:decode_packet(httph, Rest1, Opts), + ?line {more, undefined} = erlang:decode_packet(httph, Rest2, Opts), + ?line {ok, {http_header,_,"Link",_,_}, _} = + erlang:decode_packet(httph, list_to_binary([Rest2, Pkt2]), Opts), + + Pkt3 = list_to_binary(["GET / HTTP/1.1\r\nHost: localhost\r\nLink: /", + string:chars($Y, 129), "\r\n\r\n"]), + ?line {ok, {http_request,'GET',{abs_path,"/"},{1,1}}, Rest3} = + erlang:decode_packet(http, Pkt3, Opts), + ?line {ok, {http_header,_,'Host',_,"localhost"}, Rest4} = + erlang:decode_packet(httph, Rest3, Opts), + ?line {error, invalid} = erlang:decode_packet(httph, Rest4, Opts), + ok. @@ -557,3 +581,37 @@ decode_pkt(Type,Bin,Opts) -> %%io:format(" -> ~p\n",[Res]), Res. +otp_9389(doc) -> ["Verify line_length works correctly for HTTP headers"]; +otp_9389(suite) -> []; +otp_9389(Config) when is_list(Config) -> + Opts = [{packet_size, 16384}, {line_length, 3000}], + Pkt = list_to_binary(["GET / HTTP/1.1\r\nHost: localhost\r\nLink: /", + string:chars($X, 8192), + "\r\nContent-Length: 0\r\n\r\n"]), + <> = Pkt, + {ok, {http_request,'GET',{abs_path,"/"},{1,1}}, Rest1} = + erlang:decode_packet(http, Pkt1, Opts), + {ok, {http_header,_,'Host',_,"localhost"}, Rest2} = + erlang:decode_packet(httph, Rest1, Opts), + {more, undefined} = erlang:decode_packet(httph, Rest2, Opts), + {ok, {http_header,_,"Link",_,Link}, Rest3} = + erlang:decode_packet(httph, list_to_binary([Rest2, Pkt2]), Opts), + true = (length(Link) =< 3000), + {ok, {http_error, _}, Rest4} = erlang:decode_packet(httph, Rest3, Opts), + {ok, {http_error, _}, Rest5} = erlang:decode_packet(httph, Rest4, Opts), + {ok, {http_header,_,'Content-Length',_,"0"}, <<"\r\n">>} = + erlang:decode_packet(httph, Rest5, Opts), + ok. + +otp_9389_line(doc) -> ["Verify packet_size works correctly for line mode"]; +otp_9389_line(suite) -> []; +otp_9389_line(Config) when is_list(Config) -> + Opts = [{packet_size, 20}], + Line1 = <<"0123456789012345678\n">>, + Line2 = <<"0123456789\n">>, + Line3 = <<"01234567890123456789\n">>, + Pkt = list_to_binary([Line1, Line2, Line3]), + ?line {ok, Line1, Rest1} = erlang:decode_packet(line, Pkt, Opts), + ?line {ok, Line2, Rest2} = erlang:decode_packet(line, Rest1, Opts), + ?line {error, invalid} = erlang:decode_packet(line, Rest2, Opts), + ok. -- 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. --- erts/emulator/test/decode_packet_SUITE.erl | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'erts/emulator/test') diff --git a/erts/emulator/test/decode_packet_SUITE.erl b/erts/emulator/test/decode_packet_SUITE.erl index 55ef05079f..4acbe8c6e0 100644 --- a/erts/emulator/test/decode_packet_SUITE.erl +++ b/erts/emulator/test/decode_packet_SUITE.erl @@ -596,11 +596,9 @@ otp_9389(Config) when is_list(Config) -> {more, undefined} = erlang:decode_packet(httph, Rest2, Opts), {ok, {http_header,_,"Link",_,Link}, Rest3} = erlang:decode_packet(httph, list_to_binary([Rest2, Pkt2]), Opts), - true = (length(Link) =< 3000), - {ok, {http_error, _}, Rest4} = erlang:decode_packet(httph, Rest3, Opts), - {ok, {http_error, _}, Rest5} = erlang:decode_packet(httph, Rest4, Opts), + true = (length(Link) > 8000), {ok, {http_header,_,'Content-Length',_,"0"}, <<"\r\n">>} = - erlang:decode_packet(httph, Rest5, Opts), + erlang:decode_packet(httph, Rest3, Opts), ok. otp_9389_line(doc) -> ["Verify packet_size works correctly for line mode"]; -- cgit v1.2.3