firecracker icon indicating copy to clipboard operation
firecracker copied to clipboard

micro_http: firecracker hangs if content-length > request body

Open lauralt opened this issue 4 years ago • 4 comments

If the user specifies a content-length that is bigger than the request body size, Firecracker hangs waiting for more input to achieve the content-length target.

Example:

curl --unix-socket /run/firecracker.socket -i \
    -X GET 'http://localhost/machine-config' \
    -H  'Accept: application/json'          \
    -H  'Content-Length: 20' \
    -d 'some body'

lauralt avatar Jul 07 '20 11:07 lauralt

My thought is that we can work around the issue with something like the following:

  • Content-Length % 1024 == 0, expect to receive the body in Content-Length / 1024 rounds of EPOLLIN. If received less than 1024 in any round, raise error.
  • Content-Length % 1024 == X, expect to receive the body in (Content-Length / 1024) + 1 rounds of EPOLLIN. If received less than 1024 in any of the first Content-Length / 1024, then raise error. If received less or more than X in the last round, raise error.

Later edit: This works if in a round of EPOLLIN the server will read always 1024 bytes, assuming they are available. Not sure if the connection can be driven in other ways, like sending 1000 bytes at a time. If this would happen, we must understand by rounds the moment when the buffer of 1024 gets full. However, this can not be easily determined, so maybe here is were the issue lies.

iulianbarbu avatar Jul 10 '20 07:07 iulianbarbu

Just leaving it here too: the awkward partial fix from https://github.com/firecracker-microvm/firecracker/pull/2018 didn't work for request body sizes % 1024 == 0 and with Content-Length passed by user > body size. This scenario should be carefully tested when we will come with other fix.

lauralt avatar Jul 10 '20 07:07 lauralt

Firecracker API server thread does not hang when it receives deceiving requests, like the one from the issue description. It can handle other requests in parallel. The client hangs, waiting for a response.

This GET request should be treated more carefully by the API server. GETs do not need bodies. If we match the content-length to the body length, then Firecracker returns with: {"fault_message":"GET request cannot have a body."}. Maybe we can fail fast for this specific case. However, for requests that involves bodies sending, we can not do much to help the client, if the request is malformed.

Later edit: If more than 10 such hanging connections are opened, than for a new connection, firecracker API server will return a response with { "error": "Too many open connections" }.

Later edit2: Regarding However, for requests that involves bodies sending, we can not do much to help the client, if the request is malformed.: we can add a default timeout mechanism for idle connections. HTTP offers 408 status code, for Request Time Out. Nginx, for example, has an http core module directive, client_body_timeout which defines a timeout for waiting the client request body. It returns with 408 in case the timeout limit exceeds.

iulianbarbu avatar Jul 10 '20 09:07 iulianbarbu

Yep, sorry, the title was not the most accurate, it makes sense to me, when having json bodies too, to return some error if Content-Length is > body size, but since micro_http doesn't care (from what I remember) what is the body type, it will probably be weird to check this.

lauralt avatar Jul 10 '20 09:07 lauralt

This behavior can be triggered only from the host (the guest is not affected - we are ensuring through this integration test). Given that the host user is trusted in Firecracker's threat model, we do not plan to implement additional checks.

dianpopa avatar Sep 27 '22 13:09 dianpopa