]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MEDIUM: h1: Add an option to sanitize connection headers during parsing
authorChristopher Faulet <cfaulet@haproxy.com>
Fri, 29 Mar 2019 14:03:13 +0000 (15:03 +0100)
committerChristopher Faulet <cfaulet@haproxy.com>
Fri, 12 Apr 2019 20:06:53 +0000 (22:06 +0200)
The flag H1_MF_CLEAN_CONN_HDR has been added to let the H1 parser sanitize
connection headers. It means it will remove all "close" and "keep-alive" values
during the parsing. One noticeable effect is that connection headers may be
unfolded. In practice, this is not a problem because it is not frequent to have
multiple values for the connection headers.

If this flag is set, during the parsing The function
h1_parse_next_connection_header() is called in a loop instead of
h1_parse_conection_header().

No need to backport this patch

include/common/h1.h
src/h1.c

index f0f2039310c3a9b5ce4635773a8e95a695135d26..b36f6fa041226b7f281889785e4c7c336736aa7e 100644 (file)
@@ -93,6 +93,7 @@ enum h1m_state {
 #define H1_MF_XFER_ENC          0x00000200 // transfer-encoding is present
 #define H1_MF_NO_PHDR           0x00000400 // don't add pseudo-headers in the header list
 #define H1_MF_HDRS_ONLY         0x00000800 // parse headers only
+#define H1_MF_CLEAN_CONN_HDR    0x00001000 // skip close/keep-alive values of connection headers during parsing
 
 /* Note: for a connection to be persistent, we need this for the request :
  *   - one of CLEN or CHNK
@@ -144,7 +145,7 @@ int h1_measure_trailers(const struct buffer *buf, unsigned int ofs, unsigned int
 
 int h1_parse_cont_len_header(struct h1m *h1m, struct ist *value);
 void 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_connection_header(struct h1m *h1m, struct ist *value);
 
 /* for debugging, reports the HTTP/1 message state name */
 static inline const char *h1m_state_str(enum h1m_state msg_state)
index 38d13c6c33beae02b1cf517b0af6fcd6c062d7c2..d89bcb5f7121d66fda56dbab4f63871825ec1f8b 100644 (file)
--- a/src/h1.c
+++ b/src/h1.c
@@ -132,15 +132,20 @@ void h1_parse_xfer_enc_header(struct h1m *h1m, struct ist value)
  * "keep-alive", and "upgrade" values, and updating h1m->flags according to
  * what was found there. Note that flags are only added, not removed, so the
  * function is safe for being called multiple times if multiple occurrences
- * are found.
+ * are found. If the flag H1_MF_CLEAN_CONN_HDR, the header value is cleaned
+ * up from "keep-alive" and "close" values. To do so, the header value is
+ * rewritten in place and its length is updated.
  */
-void h1_parse_connection_header(struct h1m *h1m, struct ist value)
+void h1_parse_connection_header(struct h1m *h1m, struct ist *value)
 {
-       char *e, *n;
+       char *e, *n, *p;
        struct ist word;
 
-       word.ptr = value.ptr - 1; // -1 for next loop's pre-increment
-       e = value.ptr + value.len;
+       word.ptr = value->ptr - 1; // -1 for next loop's pre-increment
+       p = value->ptr;
+       e = value->ptr + value->len;
+       if (h1m->flags & H1_MF_CLEAN_CONN_HDR)
+               value->len = 0;
 
        while (++word.ptr < e) {
                /* skip leading delimitor and blanks */
@@ -154,14 +159,33 @@ void h1_parse_connection_header(struct h1m *h1m, struct ist value)
                while (word.len && HTTP_IS_LWS(word.ptr[word.len-1]))
                        word.len--;
 
-               if (isteqi(word, ist("keep-alive")))
+               if (isteqi(word, ist("keep-alive"))) {
                        h1m->flags |= H1_MF_CONN_KAL;
-               else if (isteqi(word, ist("close")))
+                       if (h1m->flags & H1_MF_CLEAN_CONN_HDR)
+                               goto skip_val;
+               }
+               else if (isteqi(word, ist("close"))) {
                        h1m->flags |= H1_MF_CONN_CLO;
+                       if (h1m->flags & H1_MF_CLEAN_CONN_HDR)
+                               goto skip_val;
+               }
                else if (isteqi(word, ist("upgrade")))
                        h1m->flags |= H1_MF_CONN_UPG;
 
-               word.ptr = n;
+               if (h1m->flags & H1_MF_CLEAN_CONN_HDR) {
+                       if (value->ptr + value->len == p) {
+                               /* no rewrite done till now */
+                               value->len = n - value->ptr;
+                       }
+                       else {
+                               if (value->len)
+                                       value->ptr[value->len++] = ',';
+                               istcat(value, word, e - value->ptr);
+                       }
+               }
+
+         skip_val:
+               word.ptr = p = n;
        }
 }
 
@@ -802,7 +826,11 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
                                        }
                                }
                                else if (isteqi(n, ist("connection"))) {
-                                       h1_parse_connection_header(h1m, v);
+                                       h1_parse_connection_header(h1m, &v);
+                                       if (!v.len) {
+                                               /* skip it */
+                                               break;
+                                       }
                                }
 
                                http_set_hdr(&hdr[hdr_count++], n, v);