Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reverse_proxy: problems with fastcgi requests with unknown body length #6637

Open
WeidiDeng opened this issue Oct 17, 2024 · 8 comments
Open

Comments

@WeidiDeng
Copy link
Member

Right now, caddy has problems handling fastcgi requests when the client sends a body with unknown body length, for http1.1 it's usually chunked encoded body. Users have to buffer the request manually and the buffer doesn't work right now because Content-length is not set in this case, and it will leads to some easy attacks.

Here, an attacker doesn't need to do much to block all processes spawned by php-fpm.

Caddy should handle this type of requests with a sane default just like nginx does, i.e. buffer requests with unknown body length in this case, and reject requests whose body is too large to buffer.

@WeidiDeng
Copy link
Member Author

With 6638, that specific issue should be gone. However, for request bodies larger than whatever is defined will still cause problems, unless -1 is defined which means all the request body is buffered in memory, which can be used in attacks as well.

I checked nginx configurations, request and response buffering is on by default. But let's just buffer those requests with unknown content length for now.

@WeidiDeng
Copy link
Member Author

Done, it's now available in this patch. Though the maximum request body size can't be limited there, it's available in request_body. Set that if some clients send some arbitrary large body to mount a ddos attack.

@cottonthread
Copy link

I'm experiencing the same issue, and it seems that many others are also affected by this. I'm not a development software engineer, so I'm not sure if I can ask you to show me how to temporarily mitigate this type of error. Currently, I have the following configuration in my Caddyfile, but it doesn't seem to work. I'm using Caddy through Docker, pulling the image directly from Docker Hub. Thank you very much!

(blockphpbug) {
	@blockChunkedEncoding {
		header Transfer-Encoding chunked
	}
	respond @blockChunkedEncoding 411
	request_body {
		max_size 1MB
	}
}
(passtophp) {
	php_fastcgi php:9000 {
		request_buffers 4k
		response_buffers 4k
	}
}

@dallyger
Copy link

dallyger commented Nov 16, 2024

@cottonthread this is currently another bug in Caddy. See: #6628

AFAIK there are 3 possible workarounds right now.

  1. Match against a missing Content-Length header (not sure if this actually works, as it was only a theory and not tested) as described at [bug] cannot deny transfer-encoding requests #6628 (comment). This is because the Transfer-Encoding header is meant to be peer-to-peer, not client to server. So it is correctly dropped by Caddy, but incorrectly handled.
  2. Build your own docker image, which extends the upstream one but applies additional patches. Some patches are available, but not released yet. See: Transfer-Encoding: chunked causing denial of service shopware/docker#107 (comment) and Transfer-Encoding: chunked causing denial of service shopware/docker#107 (comment).
  3. Put Caddy behind a reverse-proxy, that can handle chunked requests like Nginx or Varnish. This is the one I did. See the "Temporary solution" section at Transfer-Encoding: chunked causing denial of service shopware/docker#107.

Or migrate away to something like Nginx. This is the solution Shopware now recommends, after I've raised this issue with them and they were unable to fix it. See the warning at https://github.com/shopware/docker?tab=readme-ov-file#readme

@cottonthread
Copy link

@cottonthread this is currently another bug in Caddy. See: #6628

AFAIK there are 3 possible workarounds right now.

  1. Match against a missing Content-Length header (not sure if this actually works, as it was only a theory and not tested) as described at [bug] cannot deny transfer-encoding requests #6628 (comment). This is because the Transfer-Encoding header is meant to be peer-to-peer, not client to server. So it is correctly dropped by Caddy, but incorrectly handled.
  2. Build your own docker image, which extends the upstream one but applies additional patches. Some patches are available, but not released yet. See: Transfer-Encoding: chunked causing denial of service shopware/docker#107 (comment) and Transfer-Encoding: chunked causing denial of service shopware/docker#107 (comment).
  3. Put Caddy behind a reverse-proxy, that can handle chunked requests like Nginx or Varnish. This is the one I did. See the "Temporary solution" section at Transfer-Encoding: chunked causing denial of service shopware/docker#107.

Or migrate away to something like Nginx. This is the solution Shopware now recommends, after I've raised this issue with them and they were unable to fix it. See the warning at https://github.com/shopware/docker?tab=readme-ov-file#readme

Thank you very much for replying. I’m trying to test the first method you mentioned, but I’m not sure if I’m doing it correctly. From what I understand, every time the condition is met, it should respond with a 411 code, right? However, I’ve noticed that there’s no impact in the logs.

The relevant code appears to be this:

(blockphpbug) {
	@blockChunkedEncoding {
		header Transfer-Encoding chunked
	}
	respond @blockChunkedEncoding 411
	request_body {
		max_size 1MB
	}
	@noContentLength {
		header Transfer-Encoding chunked
		header !Content-Length
	}
	respond @noContentLength 411
}

http://example.com, http://www.example.com { 
	import blockphpbug
	request_body {
		max_size 1MB
	}
	php_fastcgi php:9000 {
		index index.html
		request_buffers 4k
		response_buffers 4k
	}
	root * ./example.com 
	encode gzip
	file_server
	log
}

I understand the important part is here:

header Transfer-Encoding chunked
header !Content-Length

It essentially indicates that if the Transfer-Encoding header is set to chunked, the request must also have a Content-Length header; otherwise, it returns a 411 response, right?

@dallyger
Copy link

You have to drop the header Transfer-Encoding chunked line. Caddy does not export that header to match against it in the config.
The missing Content-Length check is a workaround. This header should only be missing if Transfer-Encoding: chunked is set. Therefore matching it indirectly (or someone is sending an invalid http request).

@cottonthread
Copy link

You have to drop the header Transfer-Encoding chunked line. Caddy does not export that header to match against it in the config. The missing Content-Length check is a workaround. This header should only be missing if Transfer-Encoding: chunked is set. Therefore matching it indirectly (or someone is sending an invalid http request).

Thank you very much for your response. I wanted to comment that if I write the rule as follows:

@noContentLength {
	header !Content-Length
}
respond @noContentLength 411

Every time I visit the website, it directly reports a 411 error because my website uses index.php. Therefore, I understand that Caddy should first determine that the request involves Transfer-Encoding: chunked before checking whether there is a Content-Length header, right? Something like this:

@noContentLength {
	header Transfer-Encoding chunked
	header !Content-Length
}
respond @noContentLength 411

But if you’re saying that Transfer-Encoding: chunked is NOT something Caddy checks, then I assume this rule wouldn’t work, right? I suppose there might be other ways to handle this, perhaps by using something like:

request_body {
	max_size 1MB
}

Or even:

php_fastcgi php:9000 {
	request_buffers 4k
	response_buffers 4k
}

Or is it simply not possible to fix this using any of these methods?

Thank you very much for your patience.

@dallyger
Copy link

I don't think, that the index.php file is the issue here, as the request would most likely not even reach it. The request is routed like so:
client (Browser) -> Caddy -> PHP. Caddy accepts the request from the client and sends the 411 before reaching out to PHP. But it is interresting, that it always fails. Maybe your client is sending invalid requests? What client are you using?

I'm sorry, but I don't think I can help you with that. I don't know Caddy or how to configure it. I am just using a project, that once shipped with Caddy by default and therefore reported this issue. Our solution was to deploy another reverse-proxy infront to prepare the requests and that project migrated to nginx.

AFAIK your other examples are not yet working in caddy due to this transfer-encoding issue. Some patches were tested but not merged yet. Maybe @WeidiDeng, who authored those patches, can help and tell us what the status is?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants