MINOR: http/harderning: implement "delete-underscore-in-trailer-name"

This is the same as for the headers, but this time for the trailers. Better
keep them separated as trailers are used even worse than headers these days.

Note that in h1 we delete them when we detect them since we already have
the code for this, no need to index them and delete them later.

diff --git a/doc/configuration.txt b/doc/configuration.txt
index ef5800df4b7d0de89703091b615df77dbde8a00c..88384bb60ee87df306e6dfe04575c707913b0104 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -34536,6 +34536,16 @@ http-hardening <name> [from <name2>]
         so it is recommended to enable on untrusted listeners. Note that this
         applies only after "reject-underscore-in-header-name" was accepted.
 
+  - delete-underscore-in-trailer-name (inbound, request and response)
+        Deletes from requests and responses any trailer field whose name
+        contains an underscore character ('_' or \x5C). While this character is
+        perfectly permitted, some application frameworks which create automatic
+        variables already turn the dash ('-') into an underscore in variable
+        names, leading to an ambiguity with the underscore which can result in
+        the same variable name. No standard HTTP trailer uses this, so it is
+        recommended to enable on untrusted listeners. Note that this applies
+        only after "reject-underscore-in-trailer-name" was accepted.
+
   - 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
@@ -34567,7 +34577,8 @@ http-hardening <name> [from <name2>]
         variables tend to already turn the dash ('-') into an underscore in
         variable names, leading to an ambiguity with the underscore which can
         result in the same variable name. No standard HTTP trailer uses this,
-        so it is safe to enable on untrusted listeners.
+        so it is safe to enable on untrusted listeners. See also the
+        "delete-underscore-in-trailer-name" variant.
 
 /*
  * Local variables:

diff --git a/include/haproxy/http-t.h b/include/haproxy/http-t.h
index baf99e9eaae780c7116d6b5d1c8d851b7e3b8f29..39b83c771b15801de952fc19afd47750d0ec5809 100644 (file)
--- a/include/haproxy/http-t.h
+++ b/include/haproxy/http-t.h
@@ -245,6 +245,7 @@ struct http_violation {
 #define HTTP_HRD_REJ_USCORE_HDR_NAM         0x00000004
 #define HTTP_HRD_REJ_USCORE_TLR_NAM         0x00000008
 #define HTTP_HRD_DEL_USCORE_HDR_NAM         0x00000010
+#define HTTP_HRD_DEL_USCORE_TLR_NAM         0x00000020
 /* insert new masks above this line */
 #define HTTP_HRD_ALL                        0xFFFFFFFF
 

diff --git a/src/h1.c b/src/h1.c
index 459e4cd20e491e0fcbaf0470305e73b2a73b9b28..fa5b0c6bf37435e0213402335ab467e317ac2c77 100644 (file)
--- a/src/h1.c
+++ b/src/h1.c
@@ -1045,7 +1045,9 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
                                 */
                                if (unlikely(http_hdng_chk.found & HTTP_HRD_REJ_USCORE_HDR_NAM) &&
                                    ((h1m->state != H1_MSG_TRAILERS && istlen(istfind(n, '_')) &&
-                                     http_harden(HTTP_HRD_DEL_USCORE_HDR_NAM)))) {
+                                     http_harden(HTTP_HRD_DEL_USCORE_HDR_NAM)) ||
+                                    (h1m->state == H1_MSG_TRAILERS && istlen(istfind(n, '_')) &&
+                                     http_harden(HTTP_HRD_DEL_USCORE_TLR_NAM)))) {
                                        /* skip it, so it won't be added */
                                        break;
                                }

diff --git a/src/http.c b/src/http.c
index 341dee01f8337bc5c096d2ddbf30a8acee5effb8..18f0eb9a7bc671527135d5ead8fa84547d2c2716 100644 (file)
--- a/src/http.c
+++ b/src/http.c
@@ -1780,6 +1780,7 @@ const struct http_hardening_kw http_hardening_kws[] = {
        { "reject-underscore-in-header-name", HTTP_HRD_REJ_USCORE_HDR_NAM, HTTP_DIR_BOTH },
        { "reject-underscore-in-trailer-name", HTTP_HRD_REJ_USCORE_TLR_NAM, HTTP_DIR_BOTH },
        { "delete-underscore-in-header-name", HTTP_HRD_DEL_USCORE_HDR_NAM, HTTP_DIR_BOTH },
+       { "delete-underscore-in-trailer-name", HTTP_HRD_DEL_USCORE_TLR_NAM, HTTP_DIR_BOTH },
        /* insert new hardening options above this line */
        { "all",                            HTTP_HRD_ALL,              HTTP_DIR_BOTH },
        { NULL, 0 }

diff --git a/src/http_htx.c b/src/http_htx.c
index 89ceeb0906dc1530231c4a0b5ff7ddb31f092c53..b465b218b1f3349bca662d7955076e56e728b640 100644 (file)
--- a/src/http_htx.c
+++ b/src/http_htx.c
@@ -2074,6 +2074,17 @@ enum http_parser_status http_trailers_to_htx(struct http_hdr *list, struct htx *
                        goto fail;
                }
 
+               /* we may have to delete trailers with underscores
+                * in their names. These ones are already marked
+                * as found when not rejected so we can avoid a
+                * costly lookup in normal times.
+                */
+               if (unlikely(http_hdng_chk.found & HTTP_HRD_REJ_USCORE_TLR_NAM) &&
+                   istlen(istfind(list[idx].n, '_')) && http_harden(HTTP_HRD_DEL_USCORE_TLR_NAM)) {
+                       /* skip it, so it won't be added */
+                       continue;
+               }
+
                /* trim leading/trailing LWS as per RC9113#8.2.1 */
                for (v = list[idx].v; v.len; v.len--) {
                        if (unlikely(HTTP_IS_LWS(*v.ptr)))