MINOR: h1: Change T-E header parsing to fail if chunked encoding is found twice

According to the RFC7230, "chunked" encoding must not be applied more than
once to a message body. To handle this case, h1_parse_xfer_enc_header() is
now responsible to fail when a parsing error is found. It also fails if the
"chunked" encoding is not the last one for a request.

To help the parsing, two H1 parser flags have been added: H1_MF_TE_CHUNKED
and H1_MF_TE_OTHER. These flags are set, respectively, when "chunked"
encoding and any other encoding are found. H1_MF_CHNK flag is used when
"chunked" encoding is the last one.

diff --git a/include/haproxy/h1.h b/include/haproxy/h1.h
index 858db7d02f1f1e0e3110beeb72b42abbbce2201f..7ebd9c84bc02a496f0a76a9ff21b73677fc45273 100644 (file)
--- a/include/haproxy/h1.h
+++ b/include/haproxy/h1.h
@@ -81,7 +81,7 @@ enum h1m_state {
 /* HTTP/1 message flags (32 bit), for use in h1m->flags only */
 #define H1_MF_NONE              0x00000000
 #define H1_MF_CLEN              0x00000001 // content-length present
-#define H1_MF_CHNK              0x00000002 // chunk present, exclusive with c-l
+#define H1_MF_CHNK              0x00000002 // chunk present (as last encoding), exclusive with c-l
 #define H1_MF_RESP              0x00000004 // this message is the response message
 #define H1_MF_TOLOWER           0x00000008 // turn the header names to lower case
 #define H1_MF_VER_11            0x00000010 // message indicates version 1.1 or above
@@ -96,6 +96,8 @@ enum h1m_state {
 #define H1_MF_METH_CONNECT      0x00002000 // Set for a response to a CONNECT request
 #define H1_MF_METH_HEAD         0x00004000 // Set for a response to a HEAD request
 #define H1_MF_UPG_WEBSOCKET     0x00008000 // Set for a Websocket upgrade handshake
+#define H1_MF_TE_CHUNKED        0x00010000 // T-E "chunked"
+#define H1_MF_TE_OTHER          0x00020000 // T-E other than supported ones found (only "chunked" is supported for now)
 
 /* Note: for a connection to be persistent, we need this for the request :
  *   - one of CLEN or CHNK
@@ -146,7 +148,7 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
 int h1_measure_trailers(const struct buffer *buf, unsigned int ofs, unsigned int max);
 
 int h1_parse_cont_len_header(struct h1m *h1m, struct ist *value);
-void h1_parse_xfer_enc_header(struct h1m *h1m, struct ist value);
+int h1_parse_xfer_enc_header(struct h1m *h1m, struct ist value);
 void h1_parse_connection_header(struct h1m *h1m, struct ist *value);
 void h1_parse_upgrade_header(struct h1m *h1m, struct ist value);
 

diff --git a/src/h1.c b/src/h1.c
index e7334fac3048daee4f2bb2b1820f08dda2e0d150..e0ba8d768bd4362246888acb889a1d60b0770eb8 100644 (file)
--- a/src/h1.c
+++ b/src/h1.c
@@ -93,19 +93,21 @@ int h1_parse_cont_len_header(struct h1m *h1m, struct ist *value)
 }
 
 /* Parse the Transfer-Encoding: header field of an HTTP/1 request, looking for
- * "chunked" being the last value, and setting H1_MF_CHNK in h1m->flags only in
- * this case. Any other token found or any empty header field found will reset
- * this flag, so that it accurately represents the token's presence at the last
- * position. The H1_MF_XFER_ENC flag is always set. Note that transfer codings
- * are case-insensitive (cf RFC7230#4).
+ * "chunked" encoding to perform some checks (it must be the last encoding for
+ * the request and must not be performed twice for any message). The
+ * H1_MF_TE_CHUNKED is set if a valid "chunked" encoding is found. The
+ * H1_MF_TE_OTHER flag is set if any other encoding is found. The H1_MF_XFER_ENC
+ * flag is always set. The H1_MF_CHNK is set when "chunked" encoding is the last
+ * one. Note that transfer codings are case-insensitive (cf RFC7230#4). This
+ * function returns <0 if a error is found, 0 if the whole header can be dropped
+ * (not used yet), or >0 if the value can be indexed.
  */
-void h1_parse_xfer_enc_header(struct h1m *h1m, struct ist value)
+int h1_parse_xfer_enc_header(struct h1m *h1m, struct ist value)
 {
        char *e, *n;
        struct ist word;
 
        h1m->flags |= H1_MF_XFER_ENC;
-       h1m->flags &= ~H1_MF_CHNK;
 
        word.ptr = value.ptr - 1; // -1 for next loop's pre-increment
        e = value.ptr + value.len;
@@ -123,11 +125,36 @@ void h1_parse_xfer_enc_header(struct h1m *h1m, struct ist value)
                        word.len--;
 
                h1m->flags &= ~H1_MF_CHNK;
-               if (isteqi(word, ist("chunked")))
-                       h1m->flags |= H1_MF_CHNK;
+               if (isteqi(word, ist("chunked"))) {
+                       if (h1m->flags & H1_MF_TE_CHUNKED) {
+                               /* cf RFC7230#3.3.1 : A sender MUST NOT apply
+                                * chunked more than once to a message body
+                                * (i.e., chunking an already chunked message is
+                                * not allowed)
+                                */
+                               goto fail;
+                       }
+                       h1m->flags |= (H1_MF_TE_CHUNKED|H1_MF_CHNK);
+               }
+               else {
+                       if ((h1m->flags & (H1_MF_RESP|H1_MF_TE_CHUNKED)) == H1_MF_TE_CHUNKED) {
+                               /* cf RFC7230#3.3.1 : If any transfer coding
+                                * other than chunked is applied to a request
+                                * payload body, the sender MUST apply chunked
+                                * as the final transfer coding to ensure that
+                                * the message is properly framed.
+                                */
+                               goto fail;
+                       }
+                       h1m->flags |= H1_MF_TE_OTHER;
+               }
 
                word.ptr = n;
        }
+
+       return 1;
+  fail:
+       return -1;
 }
 
 /* Parse the Connection: header of an HTTP/1 request, looking for "close",
@@ -843,7 +870,16 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
                                }
 
                                if (isteqi(n, ist("transfer-encoding"))) {
-                                       h1_parse_xfer_enc_header(h1m, v);
+                                       ret = h1_parse_xfer_enc_header(h1m, v);
+                                       if (ret < 0) {
+                                               state = H1_MSG_HDR_L2_LWS;
+                                               ptr = v.ptr; /* Set ptr on the error */
+                                               goto http_msg_invalid;
+                                       }
+                                       else if (ret == 0) {
+                                               /* skip it */
+                                               break;
+                                       }
                                }
                                else if (isteqi(n, ist("content-length"))) {
                                        ret = h1_parse_cont_len_header(h1m, &v);

diff --git a/src/hlua.c b/src/hlua.c
index e997964895f200f9bf73469aa1f5634cfa94eb2f..4ccd8c57b1641e80b5f830f8c728d51176fc83e1 100644 (file)
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -5355,8 +5355,19 @@ __LJMP static int hlua_applet_http_send_response(lua_State *L)
                        value = lua_tolstring(L, -1, &vlen);
 
                        /* Simple Protocol checks. */
-                       if (isteqi(ist2(name, nlen), ist("transfer-encoding")))
-                               h1_parse_xfer_enc_header(&h1m, ist2(value, vlen));
+                       if (isteqi(ist2(name, nlen), ist("transfer-encoding"))) {
+                               int ret;
+
+                               ret = h1_parse_xfer_enc_header(&h1m, ist2(value, vlen));
+                               if (ret < 0) {
+                                       hlua_pusherror(L, "Lua applet http '%s': Invalid '%s' header.\n",
+                                                      luactx->appctx->rule->arg.hlua_rule->fcn->name,
+                                                      name);
+                                       WILL_LJMP(lua_error(L));
+                               }
+                               else if (ret == 0)
+                                       goto next; /* Skip it */
+                       }
                        else if (isteqi(ist2(name, nlen), ist("content-length"))) {
                                struct ist v = ist2(value, vlen);
                                int ret;