CrowdHailer / raxx

Interface for HTTP webservers, frameworks and clients
https://hexdocs.pm/raxx
Apache License 2.0
402 stars 29 forks source link

Handle unexpected HTTP verb #173

Closed lasseebert closed 5 years ago

lasseebert commented 5 years ago

I have Raxx and Ace running in production and noticed some error logs with weird HTTP verbs like PROPFIND, PING and WSPB.

Someone is obviously trying to find security flaws and hoping to see certain systems like WebDAV.

How can I handle unexpected HTTP verbs? In this case I would like to just return a 404 or similar and not log an error to my log.

I can reproduce locally by starting my Ace/Raxx app and hitting it with this curl:

➤ curl localhost:4010 -X PROPFIND
curl: (52) Empty reply from server

It will give this error log, which is similar to the one on my production deploy:

➤ iex -S mix
Erlang/OTP 21 [erts-10.3] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [hipe]

21:56:53.736 [info]  Serving cleartext using HTTP/1 on port 4010
Interactive Elixir (1.8.1) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> 
21:56:55.083 [error] GenServer #PID<0.256.0> terminating
** (FunctionClauseError) no function clause matching in Raxx.request/2
    (raxx) lib/raxx.ex:133: Raxx.request("PROPFIND", %URI{authority: nil, fragment: nil, host: nil, path: "/", port: nil, query: nil, scheme: nil, userinfo: nil})
    (ace) lib/ace/http1/parser.ex:232: Ace.HTTP1.Parser.build_partial_request/2
    (ace) lib/ace/http1/parser.ex:129: Ace.HTTP1.Parser.pop_part/1
    (ace) lib/ace/http1/parser.ex:48: Ace.HTTP1.Parser.parse/3 
    (ace) lib/ace/http1/endpoint.ex:64: Ace.HTTP1.Endpoint.handle_info/2
    (stdlib) gen_server.erl:637: :gen_server.try_dispatch/4
    (stdlib) gen_server.erl:711: :gen_server.handle_msg/6
    (stdlib) gen_server.erl:661: :gen_server.try_handle_call/4
Last message: {:tcp, #Port<0.8>, "PROPFIND / HTTP/1.1\r\nHost: localhost:4010\r\nUser-Agent: curl/7.58.0\r\nAccept: */*\r\n\r\n"}
State: %Ace.HTTP1.Endpoint{channel: %Ace.HTTP.Channel{endpoint: #PID<0.256.0>, id: 1, socket: {:tcp, #Port<0.8>}}, keep_alive: false, monitor: #Reference<0.69145361.3102212101.20885>, pending_ack_count: 0, receive_state: {:start_line, "", %{max_line_length: 2048}}, socket: {:tcp, #Port<0.8>}, status: {:request, :response}, worker: #PID<0.457.0>}

21:56:55.085 [error] GenServer #PID<0.256.0> terminating
** (FunctionClauseError) no function clause matching in Raxx.request/2
    (raxx) lib/raxx.ex:133: Raxx.request("PROPFIND", %URI{authority: nil, fragment: nil, host: nil, path: "/", port: nil, query: nil, scheme: nil, userinfo: nil})
    (ace) lib/ace/http1/parser.ex:232: Ace.HTTP1.Parser.build_partial_request/2
    (ace) lib/ace/http1/parser.ex:129: Ace.HTTP1.Parser.pop_part/1
    (ace) lib/ace/http1/parser.ex:48: Ace.HTTP1.Parser.parse/3 
    (ace) lib/ace/http1/endpoint.ex:64: Ace.HTTP1.Endpoint.handle_info/2
    (stdlib) gen_server.erl:637: :gen_server.try_dispatch/4
    (stdlib) gen_server.erl:711: :gen_server.handle_msg/6
    (stdlib) gen_server.erl:661: :gen_server.try_handle_call/4
Last message (from #PID<0.255.0>): {:accept, {:tcp, #Port<0.7>}}
State: %Ace.HTTP.Server{settings: [port: 4010, cleartext: true], socket: nil, worker_supervisor: #PID<0.252.0>}
Client #PID<0.255.0> is alive

    (stdlib) gen_server.erl:403: :gen_server.loop/7
    (stdlib) proc_lib.erl:249: :proc_lib.init_p_do_apply/3

21:56:55.085 [error] GenServer #PID<0.457.0> terminating
** (FunctionClauseError) no function clause matching in Raxx.request/2
    (raxx) lib/raxx.ex:133: Raxx.request("PROPFIND", %URI{authority: nil, fragment: nil, host: nil, path: "/", port: nil, query: nil, scheme: nil, userinfo: nil})
    (ace) lib/ace/http1/parser.ex:232: Ace.HTTP1.Parser.build_partial_request/2
    (ace) lib/ace/http1/parser.ex:129: Ace.HTTP1.Parser.pop_part/1
    (ace) lib/ace/http1/parser.ex:48: Ace.HTTP1.Parser.parse/3 
    (ace) lib/ace/http1/endpoint.ex:64: Ace.HTTP1.Endpoint.handle_info/2
    (stdlib) gen_server.erl:637: :gen_server.try_dispatch/4
    (stdlib) gen_server.erl:711: :gen_server.handle_msg/6
    (stdlib) gen_server.erl:661: :gen_server.try_handle_call/4
Last message: {:DOWN, #Reference<0.69145361.3102212101.20884>, :process, #PID<0.256.0>, {:function_clause, [{Raxx, :request, ["PROPFIND", %URI{authority: nil, fragment: nil, host: nil, path: "/", port: nil, query: nil, scheme: nil, userinfo: nil}], [file: 'lib/raxx.ex', line: 133]}, {Ace.HTTP1.Parser, :build_partial_request, 2, [file: 'lib/ace/http1/parser.ex', line: 232]}, {Ace.HTTP1.Parser, :pop_part, 1, [file: 'lib/ace/http1/parser.ex', line: 129]}, {Ace.HTTP1.Parser, :parse, 3, [file: 'lib/ace/http1/parser.ex', line: 48]}, {Ace.HTTP1.Endpoint, :handle_info, 2, [file: 'lib/ace/http1/endpoint.ex', line: 64]}, {:gen_server, :try_dispatch, 4, [file: 'gen_server.erl', line: 637]}, {:gen_server, :handle_msg, 6, [file: 'gen_server.erl', line: 711]}, {:gen_server, :try_handle_call, 4, [file: 'gen_server.erl', line: 661]}]}}
State: %Ace.HTTP.Worker{app_module: LurtzWeb.WebServer, app_state: %{}, channel: %Ace.HTTP.Channel{endpoint: #PID<0.256.0>, id: 1, socket: {:tcp, #Port<0.8>}}, channel_monitor: #Reference<0.69145361.3102212101.20884>}
CrowdHailer commented 5 years ago

I don't have an answer ready for this issue. These logs come from the parser in Ace, because each connection is isolated there is no harm in these logs but in that case they should not keep spamming your logs.

In my opinion there are two options here.

  1. have Ace automatically return a default 400 response for any parser failure, it currently does this for several other kinds of "bad request"
  2. expose these methods to the Raxx app and let user handle them as they want. This has problems because turning unknown methods to atoms opens up an attack vector to flood the atom table.

My preference is for 1 because it is simpler, and it handles the case you need right now.

lasseebert commented 5 years ago

@CrowdHailer 1. works for me too. I have not yet looked at Ace source code, but please let me know if you would like me to take a go on it ;)

CrowdHailer commented 5 years ago

It would be great if there were more eyes on the ace code. 👍

I'm hoping not too hard an issue to fix, hopefully just one extra case clause somewhere.

On Thu, 28 Mar 2019, 05:05 Lasse Skindstad Ebert, notifications@github.com wrote:

@CrowdHailer https://github.com/CrowdHailer 1. works for me too. I have not yet looked at Ace source code, but please let me know if you would like me to take a go on it ;)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CrowdHailer/raxx/issues/173#issuecomment-477449723, or mute the thread https://github.com/notifications/unsubscribe-auth/AFlznqpdzAYvqsZoj212nI6NBs8jR7Siks5vbE2xgaJpZM4cOxKG .

CrowdHailer commented 5 years ago

I think it should just send a default 501 response

The 501 (Not Implemented) status code indicates that the server does not support the functionality required to fulfill the request. This is the appropriate response when the server does not recognize the request method and is not capable of supporting it for any resource. A 501 response is cacheable by default; i.e., unless otherwise indicated by the method definition or explicit cache controls (see Section 4.2.2 of [RFC7234]).

CrowdHailer commented 5 years ago

@lasseebert Fixed in Ace with this commit https://github.com/CrowdHailer/Ace/commit/6a456447d5562385124f56d95e1b1f6dc2367c8f

Will release 0.18.7 soon

lasseebert commented 5 years ago

Awesome, thanks!