[Bug] CRS rule 920171 is not applied on chunked-encoded requests about coraza-caddy HOT 4 CLOSED

Hey ! First of all, thank you for the detailed issue. It's way easier to debug this way.

I will validate but I think chunk transfers are removed from the http request objects in golang, which means coraza will never have access to it in this connector. Probably coraza receives the full buffered request.

I think @M4tteoP has information on this

from coraza-caddy.

Hi, many thanks for taking the time on providing lots of details!

I will validate but I think chunk transfers are removed from the http request objects in golang, which means Coraza will never have access to it in this connector.

I really think this is the issue. 920171-2 and 920171-3 are two of the few CRS tests that are still failing against the go/http middleware (that is the same used by the latest commit in the caddy connector). The Go library removes the Transfer-Encoding header (see here), and this is exactly what the 920171 rule is looking for (SecRule &REQUEST_HEADERS:Transfer-Encoding "!@eq 0").

What we should maybe do is understand if it is possible from the http.Request to read the Chunked boolean. If so we can populate again the header. But I don't know if it is feasible or if the HTTP library makes the Chunked logic completely transparent.

Update: https://github.com/golang/go/blob/master/src/net/http/transfer.go#L586 this line is definitely interesting. Maybe something like the following code can fix it.

if r.TransferEncoding != nil && r.TransferEncoding[0] == "chunked" {
	r.Header.Set("Transfer-Encoding", "chunked")
}

I will give it a go next week :)

from coraza-caddy.

Thank you for your fast response!

@M4tteoP This could do the trick and you should definitively give it a shot!

In my opinion however, it is not a problem of this connector but of the go/http library which gives you a HTTP request without a header that indicates a body is available. As the RFC says, Content-Length and Transfer-Encoding are responsible for that. So a more cleaner solution would be, if the go/http library replaces the Transfer-Encoding with a appropriate Content-Length header instead of removing it completely. This should trigger CRS rule 920170 then. So maybe we should populate this problem to the devs of go/http?

from coraza-caddy.

I believe the behavior Go is going for is that, because the body that is provided doesn't have chunked headers, the header is removed - I guess the idea is to present to business logic in the same way regardless of encoding.

But even so the body is still streamed so it can't compute a content length.

There doesn't seem to be a great reason to remove the encoding header anyways but I guess they'll keep the behavior for backwards compatibility.

Probably synthesizing the header for the coraza transaction is going to be the way to go if it's possible. Otherwise maybe a phase 2 rule could be added that checks &REQUEST_BODY.

from coraza-caddy.