MEDIUM: h1: Force close mode for invalid uses of T-E header

Transfer-Encoding header is not supported in HTTP/1.0. However, softwares
dealing with HTTP/1.0 and HTTP/1.1 messages may accept it and transfer
it. When a Content-Length header is also provided, it must be
ignored. Unfortunately, this may lead to vulnerabilities (request smuggling
or response splitting) if an intermediary is only implementing
HTTP/1.0. Because it may ignore Transfer-Encoding header and only handle
Content-Length one.

To avoid any security issues, when Transfer-Encoding and Content-Length
headers are found in a message, the close mode is forced. The same is
performed for HTTP/1.0 message with a Transfer-Encoding header only. This
change is conform to what it is described in the last HTTP/1.1 draft. See
also httpwg/http-core#879.

Note that Content-Length header is also removed from any incoming messages
if a Transfer-Encoding header is found. However it is not true (not yet) for
responses generated by HAProxy.

diff --git a/src/h1.c b/src/h1.c
index 4b13cab86ddbb296dc477997d445dbfd6e137559..e7334fac3048daee4f2bb2b1820f08dda2e0d150 100644 (file)
--- a/src/h1.c
+++ b/src/h1.c
@@ -938,9 +938,15 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
                state = H1_MSG_DATA;
                if (h1m->flags & H1_MF_XFER_ENC) {
                        if (h1m->flags & H1_MF_CLEN) {
+                               /* T-E + C-L: force close and remove C-L */
+                               h1m->flags |= H1_MF_CONN_CLO;
                                h1m->flags &= ~H1_MF_CLEN;
                                hdr_count = http_del_hdr(hdr, ist("content-length"));
                        }
+                       else if (!(h1m->flags & H1_MF_VER_11)) {
+                               /* T-E + HTTP/1.0: force close */
+                               h1m->flags |= H1_MF_CONN_CLO;
+                       }
 
                        if (h1m->flags & H1_MF_CHNK)
                                state = H1_MSG_CHUNK_SIZE;

diff --git a/src/mux_h1.c b/src/mux_h1.c
index a391ab12840e14e06d6b9c6ecd560a764c4aefb8..a627f19859bf580817960487dfb3e09ead303c97 100644 (file)
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -1995,12 +1995,22 @@ static size_t h1_process_mux(struct h1c *h1c, struct buffer *buf, size_t count)
                          last_lf:
                                h1m->state = H1_MSG_LAST_LF;
                                if (!(h1s->flags & H1S_F_HAVE_O_CONN)) {
-                                       /* If the reply comes from haproxy while the request is
-                                        * not finished, we force the connection close. */
                                        if ((chn_htx->flags & HTX_FL_PROXY_RESP) && h1s->req.state != H1_MSG_DONE) {
+                                               /* If the reply comes from haproxy while the request is
+                                                * not finished, we force the connection close. */
                                                h1s->flags = (h1s->flags & ~H1S_F_WANT_MSK) | H1S_F_WANT_CLO;
                                                TRACE_STATE("force close mode (resp)", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1s->h1c->conn, h1s);
                                        }
+                                       else if ((h1m->flags & (H1_MF_XFER_ENC|H1_MF_CLEN)) == (H1_MF_XFER_ENC|H1_MF_CLEN)) {
+                                               /* T-E + C-L: force close */
+                                               h1s->flags = (h1s->flags & ~H1S_F_WANT_MSK) | H1S_F_WANT_CLO;
+                                               TRACE_STATE("force close mode (T-E + C-L)", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1s->h1c->conn, h1s);
+                                       }
+                                       else if ((h1m->flags & (H1_MF_VER_11|H1_MF_XFER_ENC)) == H1_MF_XFER_ENC) {
+                                               /* T-E + HTTP/1.0: force close */
+                                               h1s->flags = (h1s->flags & ~H1S_F_WANT_MSK) | H1S_F_WANT_CLO;
+                                               TRACE_STATE("force close mode (T-E + HTTP/1.0)", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1s->h1c->conn, h1s);
+                                       }
 
                                        /* the conn_mode must be processed. So do it */
                                        n = ist("connection");