MEDIUM: http/hardening: implement hardening "reject-ctl-in-header-trailer"

Rejects requests and responses which contain control characters in
their header or trailer field values (codes 0-8,10-31,127). These ones
are forbidden but have long experienced a certain tolerance among
implementations, which has encouraged some rare applications to use
them as record delimiters for example. No standard component produces
them on the public internet, except attacks. By default, HAProxy only
blocks the strictly required ones (0,10,13) and lets other ones pass,
which can make certain backend applications trigger connection errors.
As such, it is both safe and recommended to block them on public facing
listeners. See also the has_ctl() converter used for detecting them.

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 7509e057a5dbe955bd2d8c51ea5ec89e07fc4252..81955ff1b2a9bf327a5db0e95deb3dbf6e5af7ae 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -34578,6 +34578,18 @@ http-hardening <name> [from <name2>]
         safe and even recommended to delete such trailers from inbound internet
         requests.
 
+  - reject-ctl-in-header-trailer (inbound request and response)
+        Rejects requests and responses which contain control characters in
+        their header or trailer field values (codes 0-8,10-31,127). These ones
+        are forbidden but have long experienced a certain tolerance among
+        implementations, which has encouraged some rare applications to use
+        them as record delimiters for example. No standard component produces
+        them on the public internet, except attacks. By default, HAProxy only
+        blocks the strictly required ones (0,10,13) and lets other ones pass,
+        which can make certain backend applications trigger connection errors.
+        As such, it is both safe and recommended to block them on public facing
+        listeners. See also the has_ctl() converter used for detecting them.
+
   - reject-duplicate-host-header (inbound request)
         Rejects requests containing more than one "Host" header field, even if
         valid and identical. This is recommended on untrusted listeners. Note that

diff --git a/include/haproxy/http-t.h b/include/haproxy/http-t.h
index 2504b94102e67bb91660cbf05d608b1f1cf6e129..89db7d57377e377a85e9ad48744a5f94b0de6c4f 100644 (file)
--- a/include/haproxy/http-t.h
+++ b/include/haproxy/http-t.h
@@ -250,6 +250,7 @@ struct http_violation {
 #define HTTP_HRD_DEL_UNREG_TLR              0x00000080
 #define HTTP_HRD_DEL_ALL_TLR                0x00000100
 #define HTTP_HRD_REJ_HDR_FOLD               0x00000200
+#define HTTP_HRD_REJ_CTL_IN_HDR             0x00000400
 /* insert new masks above this line */
 #define HTTP_HRD_ALL                        0xFFFFFFFF
 

diff --git a/include/haproxy/http.h b/include/haproxy/http.h
index 4c269c477f0c2117fa88950f63f4d0465bd21ab3..9b862780005946466980705218469fcbbd3fd552 100644 (file)
--- a/include/haproxy/http.h
+++ b/include/haproxy/http.h
@@ -244,9 +244,24 @@ static inline int http_header_has_forbidden_char(const struct ist ist, const cha
         * remains is HT (rare), CTL, CR/LF (forbidden), or obs-text.
         */
        for (; start < istend(ist); start++) {
-               if ((uint8_t)*start <= 0x0d &&
+               /* speed-up: SP, VCHAR, obs-text are always OK,
+                * i.e. anything not CTL nor DEL.
+                */
+               if ((uint8_t)*start >= 0x20 && *start != 0x7f)
+                       continue;
+
+               /* CR/LF/NUL are always forbidden */
+               if ((uint8_t)*start < 0x20 &&
                    (1U << (uint8_t)*start) & ((1<<13) | (1<<10) | (1<<0)))
                        return 1;
+
+               /* the rest (CTL, DEL) is OK in default mode */
+               if (!(http_hdng_chk.rules & HTTP_HRD_REJ_CTL_IN_HDR))
+                       continue;
+
+               /* strict mode: DEL and all CTL but HT are forbidden */
+               if (*start != 0x9 && http_harden(HTTP_HRD_REJ_CTL_IN_HDR))
+                       return 1;
        }
        return 0;
 }

diff --git a/src/h1.c b/src/h1.c
index a7455c296bc8f9ff4babedee4850912fcbe8c4d5..587af284f86cc2ed806213109502b96080e05cbc 100644 (file)
--- a/src/h1.c
+++ b/src/h1.c
@@ -985,6 +985,15 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
                                        EAT_AND_JUMP_OR_RETURN(ptr, end, http_msg_hdr_l2_lf, http_msg_ood, state, H1_MSG_HDR_L2_LF);
                                goto http_msg_hdr_l2_lf;
                        }
+
+                       /* CR/LF/NUL have already been deal with, only remain
+                        * CTL/HT/DEL, OK in default mode, but only HT is
+                        * allowed in strict mode.
+                        */
+                       if (*ptr != 9 && http_harden(HTTP_HRD_REJ_CTL_IN_HDR)) {
+                               state = H1_MSG_HDR_VAL;
+                               goto http_msg_invalid;
+                       }
                }
 
                /* OK, not < 0x20 or a valid one (e.g. tab) */

diff --git a/src/http.c b/src/http.c
index 647041c19c37cc9dc9ac2468b52fab540e471178..cff8e568078b8d1ce5c3a45283e49be5a9651ff0 100644 (file)
--- a/src/http.c
+++ b/src/http.c
@@ -1785,6 +1785,7 @@ const struct http_hardening_kw http_hardening_kws[] = {
        { "delete-unregistered-trailer-fields",HTTP_HRD_DEL_UNREG_TLR, HTTP_DIR_BOTH },
        { "delete-all-trailer-fields",         HTTP_HRD_DEL_ALL_TLR, HTTP_DIR_BOTH },
        { "reject-header-trailer-folding",     HTTP_HRD_REJ_HDR_FOLD, HTTP_DIR_BOTH },
+       { "reject-ctl-in-header-trailer",      HTTP_HRD_REJ_CTL_IN_HDR, HTTP_DIR_BOTH },
        /* insert new hardening options above this line */
        { "all",                            HTTP_HRD_ALL,              HTTP_DIR_BOTH },
        { NULL, 0 }