BUG/MEDIUM: h1-htx: Sanitize parsing to properly handle upgrade requests

Thanks to previous patches, the request messages are now sanitized to
properly handle Upgrade requests. Now, if a 'connection: upgrade' header
value was found while no 'Upgrade' header, the 'upgrade' values is removed
from the 'connection' header. Conversely the opposite is also performed. If
'Upgrade' header was found, but no "conneciotn: upgrade" header value, all
occurrences of 'Upgrade' header are refused.

This patch depends on following ones:
  * MINOR: h1: Add  a H1M flag to specify a non-empty 'Upgrade:' header was parsed
  * MINOR: http: Add function to remove all occurrences of a value in a header

It should fix the issue 3397. But the H2 part should be reviewed too, and
probably the H1 response parsing, to be consistent with this change.

The series should be backported as far as 2.4.

diff --git a/include/haproxy/htx-t.h b/include/haproxy/htx-t.h
index 9f950b2882ede680d9c28cd91173fce598487e51..0a12ce200fdca7c350abcf2d489838009120b21c 100644 (file)
--- a/include/haproxy/htx-t.h
+++ b/include/haproxy/htx-t.h
@@ -141,6 +141,7 @@
 #define HTX_SL_F_CONN_UPG       0x00001000 /* The message contains "connection: upgrade" header */
 #define HTX_SL_F_BODYLESS_RESP  0x00002000 /* The response to this message is bodyless (only for request) */
 #define HTX_SL_F_NOT_HTTP       0x00004000 /* Not an HTTP message (e.g "RTSP", only possible if invalid message are accepted) */
+#define HTX_SL_F_UPG_HDR        0x00008000 /* non-empty Upgrapde header found */
 
 /* This function is used to report flags in debugging tools. Please reflect
  * below any single-bit flag addition above in the same order via the
@@ -157,7 +158,8 @@ static forceinline char *hsl_show_flags(char *buf, size_t len, const char *delim
        _(HTX_SL_F_CLEN, _(HTX_SL_F_CHNK, _(HTX_SL_F_VER_11,
        _(HTX_SL_F_BODYLESS, _(HTX_SL_F_HAS_SCHM, _(HTX_SL_F_SCHM_HTTP,
        _(HTX_SL_F_SCHM_HTTPS, _(HTX_SL_F_HAS_AUTHORITY,
-       _(HTX_SL_F_NORMALIZED_URI, _(HTX_SL_F_CONN_UPG)))))))))))));
+       _(HTX_SL_F_NORMALIZED_URI, _(HTX_SL_F_CONN_UPG, _(HTX_SL_F_BODYLESS_RESP,
+       _(HTX_SL_F_NOT_HTTP, _(HTX_SL_F_UPG_HDR))))))))))))))));
        /* epilogue */
        _(~0U);
        return buf;

diff --git a/src/h1_htx.c b/src/h1_htx.c
index d0837abbcc7e2bfb1bbf32ee249c4ef8b30a1ba9..c226c0007ed0e9bd15abe2545d95dc5e65e9fdf4 100644 (file)
--- a/src/h1_htx.c
+++ b/src/h1_htx.c
@@ -162,6 +162,8 @@ static unsigned int h1m_htx_sl_flags(struct h1m *h1m)
        }
        if (h1m->flags & H1_MF_CONN_UPG)
                flags |= HTX_SL_F_CONN_UPG;
+       if (h1m->flags & H1_MF_UPG_HDR)
+               flags |= HTX_SL_F_UPG_HDR;
        return flags;
 }
 
@@ -213,6 +215,31 @@ static int h1_postparse_req_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx
                }
        }
 
+       /* Remove Upgrade header if no 'connection: upgrade' found */
+       if ((h1m->flags & (H1_MF_CONN_UPG|H1_MF_UPG_HDR)) == H1_MF_UPG_HDR) {
+               int i;
+
+               for (i = 0; hdrs[i].n.len; i++) {
+                       if (isteqi(hdrs[i].n, ist("upgrade")))
+                               hdrs[i].v = IST_NULL;
+               }
+               h1m->flags &=~ (H1_MF_CONN_UPG|H1_MF_UPG_HDR);
+       }
+
+       /* Remove 'Upgrade' value from connection header if not Upgrade header found */
+       if ((h1m->flags & (H1_MF_CONN_UPG|H1_MF_UPG_HDR)) == H1_MF_CONN_UPG) {
+               int i;
+
+               for (i = 0; hdrs[i].n.len; i++) {
+                       if (isteqi(hdrs[i].n, ist("connection"))) {
+                               http_remove_header_value(&hdrs[i].v, ist("upgrade"));
+                               if (!istlen(hdrs[i].v))
+                                       hdrs[i].v = IST_NULL;
+                       }
+               }
+               h1m->flags &=~ (H1_MF_CONN_UPG|H1_MF_UPG_HDR);
+       }
+
        flags |= h1m_htx_sl_flags(h1m);
 
        sl = htx_add_stline(htx, HTX_BLK_REQ_SL, flags, meth, uri, vsn);