From c236ed09726a4283a597f662307c9ac07e4f3b7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Thu, 27 Feb 2020 12:19:07 +0100 Subject: Detect invalid HTTP/2 preface errors And make sure all HTTP/2 connection_error(s) result in a gun_down message containing the error. In the preface case we do not send a gun_error message (because there's no stream open yet) and gun_down was always saying normal. Also make sure the human readable reason is included in the gun_error message, if any. --- src/gun_http2.erl | 37 ++++++++++++++++++++--- test/rfc7540_SUITE.erl | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 5 deletions(-) diff --git a/src/gun_http2.erl b/src/gun_http2.erl index 99776c6..990d08c 100644 --- a/src/gun_http2.erl +++ b/src/gun_http2.erl @@ -75,7 +75,7 @@ %% is established and continues normally. An exception is when a HEADERS %% frame is sent followed by CONTINUATION frames: no other frame can be %% sent in between. - parse_state = undefined :: normal + parse_state = undefined :: preface | normal | {continuation, cowboy_stream:streamid(), cowboy_stream:fin(), binary()}, %% HPACK decoding and encoding state. @@ -107,7 +107,7 @@ init(Owner, Socket, Transport, Opts) -> Handlers = maps:get(content_handlers, Opts, [gun_data_h]), State = #http2_state{owner=Owner, socket=Socket, transport=Transport, opts=Opts, content_handlers=Handlers, - parse_state=normal}, %% @todo Have a special parse state for preface. + parse_state=preface}, #http2_state{local_settings=Settings} = State, %% Send the HTTP/2 preface. Transport:send(Socket, [ @@ -119,8 +119,32 @@ init(Owner, Socket, Transport, Opts) -> handle(Data, State=#http2_state{buffer=Buffer}) -> parse(<< Buffer/binary, Data/binary >>, State#http2_state{buffer= <<>>}). +parse(Data0, State0=#http2_state{buffer=Buffer, parse_state=preface}) -> + Data = << Buffer/binary, Data0/binary >>, + case cow_http2:parse(Data) of + {ok, Frame, Rest} when element(1, Frame) =:= settings -> + case frame(Frame, State0#http2_state{parse_state=normal}) of + close -> close; + Error = {error, _} -> Error; + State -> parse(Rest, State) + end; + more -> + case Data of + %% Maybe we have a proper SETTINGS frame. + <<_:24,4:8,_/bits>> -> + {state, State0#http2_state{buffer=Data}}; + %% Not a SETTINGS frame, this is an invalid preface. + _ -> + terminate(State0, {connection_error, protocol_error, + 'Invalid connection preface received. (RFC7540 3.5)'}) + end; + %% Any error in the preface is converted to this specific error + %% to make debugging the problem easier (it's the server's fault). + _ -> + terminate(State0, {connection_error, protocol_error, + 'Invalid connection preface received. (RFC7540 3.5)'}) + end; parse(Data0, State0=#http2_state{buffer=Buffer, parse_state=PS}) -> - %% @todo Parse states: Preface. Continuation. Data = << Buffer/binary, Data0/binary >>, case cow_http2:parse(Data) of {ok, Frame, Rest} when PS =:= normal -> @@ -524,7 +548,7 @@ terminate(#http2_state{socket=Socket, transport=Transport, streams=Streams}, Rea _ = [ReplyTo ! {gun_error, self(), Reason} || #stream{reply_to=ReplyTo} <- Streams], %% @todo LastGoodStreamID Transport:send(Socket, cow_http2:goaway(0, terminate_reason(Reason), <<>>)), - close. + terminate_ret(Reason). terminate(State=#http2_state{socket=Socket, transport=Transport}, StreamID, Reason) -> case get_stream_by_id(StreamID, State) of @@ -532,7 +556,7 @@ terminate(State=#http2_state{socket=Socket, transport=Transport}, StreamID, Reas ReplyTo ! {gun_error, self(), Reason}, %% @todo LastGoodStreamID Transport:send(Socket, cow_http2:goaway(0, terminate_reason(Reason), <<>>)), - close; + terminate_ret(Reason); _ -> terminate(State, Reason) end. @@ -540,6 +564,9 @@ terminate(State=#http2_state{socket=Socket, transport=Transport}, StreamID, Reas terminate_reason({connection_error, Reason, _}) -> Reason; terminate_reason({stop, _, _}) -> no_error. +terminate_ret(Reason={connection_error, _, _}) -> {error, Reason}; +terminate_ret(_) -> close. + %% Stream functions. stream_decode_init(State=#http2_state{decode_state=DecodeState0}, StreamID, IsFin, HeaderBlock) -> diff --git a/test/rfc7540_SUITE.erl b/test/rfc7540_SUITE.erl index 9398318..047e4bb 100644 --- a/test/rfc7540_SUITE.erl +++ b/test/rfc7540_SUITE.erl @@ -63,8 +63,90 @@ do_receive(Pid, Timeout) -> error(timeout) end. +do_init_origin(tcp, http, Fun) -> + Self = self(), + Pid = spawn_link(fun() -> do_init_origin_tcp(Self, Fun) end), + Port = do_receive(Pid), + {ok, Pid, Port}. + +do_init_origin_tcp(Parent, Fun) -> + {ok, ListenSocket} = gen_tcp:listen(0, [binary, {active, false}]), + {ok, {_, Port}} = inet:sockname(ListenSocket), + Parent ! {self(), Port}, + {ok, ClientSocket} = gen_tcp:accept(ListenSocket, 5000), + %% No handshake. + Fun(Parent, ClientSocket, gen_tcp). + %% Tests. +prior_knowledge_preface_garbage(_) -> + doc("A PROTOCOL_ERROR connection error must result from the server sending " + "an invalid preface in the form of garbage when connecting " + "using the prior knowledge method. (RFC7540 3.4, RFC7540 3.5)"), + %% We are going to do the handshake manually. + {ok, _, Port} = do_init_origin(tcp, http, fun(_, Socket, Transport) -> + ok = Transport:send(Socket, <<0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15>>), + timer:sleep(100) + end), + {ok, ConnPid} = gun:open("localhost", Port, #{protocols => [http2]}), + {ok, http2} = gun:await_up(ConnPid), + receive + {gun_down, ConnPid, http2, {error, {connection_error, protocol_error, + 'Invalid connection preface received. (RFC7540 3.5)'}}, [], []} -> + gun:close(ConnPid); + Msg -> + error({unexpected_msg, Msg}) + after 1000 -> + error(timeout) + end. + +prior_knowledge_preface_http1(_) -> + doc("A PROTOCOL_ERROR connection error must result from the server sending " + "an invalid preface in the form of an HTTP/1.1 response when connecting " + "using the prior knowledge method. (RFC7540 3.4, RFC7540 3.5)"), + %% We are going to do the handshake manually. + {ok, _, Port} = do_init_origin(tcp, http, fun(_, Socket, Transport) -> + ok = Transport:send(Socket, << + "HTTP/1.1 400 Bad Request\r\n" + "Connection: close\r\n" + "Content-Length: 0\r\n" + "Date: Thu, 27 Feb 2020 09:32:17 GMT\r\n" + "\r\n">>), + timer:sleep(100) + end), + {ok, ConnPid} = gun:open("localhost", Port, #{protocols => [http2]}), + {ok, http2} = gun:await_up(ConnPid), + receive + {gun_down, ConnPid, http2, {error, {connection_error, protocol_error, + 'Invalid connection preface received. (RFC7540 3.5)'}}, [], []} -> + gun:close(ConnPid); + Msg -> + error({unexpected_msg, Msg}) + after 1000 -> + error(timeout) + end. + +prior_knowledge_preface_other_frame(_) -> + doc("A PROTOCOL_ERROR connection error must result from the server sending " + "an invalid preface in the form of a non-SETTINGS frame when connecting " + "using the prior knowledge method. (RFC7540 3.4, RFC7540 3.5)"), + %% We are going to do the handshake manually. + {ok, _, Port} = do_init_origin(tcp, http, fun(_, Socket, Transport) -> + ok = Transport:send(Socket, cow_http2:window_update(1)), + timer:sleep(100) + end), + {ok, ConnPid} = gun:open("localhost", Port, #{protocols => [http2]}), + {ok, http2} = gun:await_up(ConnPid), + receive + {gun_down, ConnPid, http2, {error, {connection_error, protocol_error, + 'Invalid connection preface received. (RFC7540 3.5)'}}, [], []} -> + gun:close(ConnPid); + Msg -> + error({unexpected_msg, Msg}) + after 1000 -> + error(timeout) + end. + headers_priority_flag(_) -> doc("HEADERS frames may include a PRIORITY flag indicating " "that stream dependency information is attached. (RFC7540 6.2)"), -- cgit v1.2.3