BUG/MEDIUM: http: remove content-length from chunked messages

RFC7230 clarified the behaviour to adopt when facing both a
content-length and a transfer-encoding: chunked in a message. While
haproxy already complied with the method for getting the message
length right, and used to detect improper content-length duplicates,
it still did not remove the content-length header when facing a
transfer-encoding: chunked. Usually it is not a problem since other
agents (clients and servers) are required to parse the message
according to the rules that have been in place since RFC2616 in
1999.

However Régis Leroy reported the existence of at least one such
non-compliant agent so haproxy could be abused to get out of sync
with it on pipelined requests (HTTP request smuggling attack),
it consider part of a payload as a subsequent request.

The best thing to do is then to remove the content-length according
to RFC7230. It used to be in the todo list with a fixme in the code
while waiting for the standard to stabilize, let's apply it now that
it's published.

Thanks to Régis for bringing that subject to our attention.

This fix must be backported to 1.5 and 1.4.

diff --git a/src/proto_http.c b/src/proto_http.c
index 6bb0745eecdd4a780d105928d71f0f2ddf0514d1..ce05a837c1f3545130b3c90b27f4f11b7a47ac25 100644 (file)
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -3028,8 +3028,13 @@ int http_wait_for_request(struct stream *s, struct channel *req, int an_bit)
                }
        }
 
+       /* Chunked requests must have their content-length removed */
        ctx.idx = 0;
-       while (!(msg->flags & HTTP_MSGF_TE_CHNK) && !use_close_only &&
+       if (msg->flags & HTTP_MSGF_TE_CHNK) {
+               while (http_find_header2("Content-Length", 14, req->buf->p, &txn->hdr_idx, &ctx))
+                       http_remove_header2(msg, &txn->hdr_idx, &ctx);
+       }
+       else while (!use_close_only &&
               http_find_header2("Content-Length", 14, req->buf->p, &txn->hdr_idx, &ctx)) {
                signed long long cl;
 
@@ -6176,9 +6181,13 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit)
                }
        }
 
-       /* FIXME: below we should remove the content-length header(s) in case of chunked encoding */
+       /* Chunked responses must have their content-length removed */
        ctx.idx = 0;
-       while (!(msg->flags & HTTP_MSGF_TE_CHNK) && !use_close_only &&
+       if (msg->flags & HTTP_MSGF_TE_CHNK) {
+               while (http_find_header2("Content-Length", 14, rep->buf->p, &txn->hdr_idx, &ctx))
+                       http_remove_header2(msg, &txn->hdr_idx, &ctx);
+       }
+       else while (!use_close_only &&
               http_find_header2("Content-Length", 14, rep->buf->p, &txn->hdr_idx, &ctx)) {
                signed long long cl;