BUG/MINOR: http/hardening: don't count unregistered hdr/tlr too early

In 3.5-dev11, the ability to remove unregistered headers and trailers
(X-*) was added with commits 4b9b6044b2 ("MINOR: http/hardening: add
"delete-unregistered-header-fields"") and b5fd464f0a ("MINOR:
http/hardening: add "delete-unregistered-trailer-fields""). There is
a small mistake there, which is the the http_harden() call is made
before checking if the situation was met, resulting in the flags being
set for each header/trailer. For now this doesn't have any impact but
it will when reporting is implemented. Let's fix that by only calling
http_harden() after the header syntax was checked. No backport is
needed.

diff --git a/src/h1.c b/src/h1.c
index d174133382fa6e09a56bac333a01a2e61124cad1..41cd4d700cb100a0b5ffbc0b5e1ae8e920029941 100644 (file)
--- a/src/h1.c
+++ b/src/h1.c
@@ -1076,12 +1076,23 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
                                        break;
                                }
 
+
                                /* we may have to delete unregistered header names (X-) */
-                               if (unlikely((h1m->state != H1_MSG_TRAILERS && http_harden(HTTP_HRD_DEL_UNREG_HDR)) ||
-                                            (h1m->state == H1_MSG_TRAILERS && http_harden(HTTP_HRD_DEL_UNREG_TLR)))) {
-                                       if (n.len > 1 && n.ptr[1] == '-' &&
-                                           (n.ptr[0] == 'x' || n.ptr[0] == 'X'))
-                                               break;
+                               if (h1m->state != H1_MSG_TRAILERS &&
+                                   unlikely((http_hdng_chk.rules & HTTP_HRD_DEL_UNREG_HDR) &&
+                                            (n.len > 1 && n.ptr[1] == '-' &&
+                                             (n.ptr[0] == 'x' || n.ptr[0] == 'X'))) &&
+                                   http_harden(HTTP_HRD_DEL_UNREG_HDR)) {
+                                       /* skip it, so it won't be added */
+                                       break;
+                               }
+                               else if (h1m->state == H1_MSG_TRAILERS &&
+                                   unlikely((http_hdng_chk.rules & HTTP_HRD_DEL_UNREG_TLR) &&
+                                            (n.len > 1 && n.ptr[1] == '-' &&
+                                             (n.ptr[0] == 'x' || n.ptr[0] == 'X'))) &&
+                                   http_harden(HTTP_HRD_DEL_UNREG_TLR)) {
+                                       /* skip it, so it won't be added */
+                                       break;
                                }
 
                                if (isteqi(n, ist("transfer-encoding"))) {

diff --git a/src/h2.c b/src/h2.c
index 926cb53a1dd228438a54532e80857640628049c9..5f635e21f768da61ed93f2fa19e5f535ae81165a 100644 (file)
--- a/src/h2.c
+++ b/src/h2.c
@@ -405,11 +405,12 @@ int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *ms
                }
 
                /* we may have to delete unregistered header names (X-) */
-               if (unlikely(http_harden(HTTP_HRD_DEL_UNREG_HDR))) {
-                       if (list[idx].n.len > 1 && list[idx].n.ptr[1] == '-' &&
-                           (list[idx].n.ptr[0] == 'x' || list[idx].n.ptr[0] == 'X'))
-                               /* skip it, so it won't be added */
-                               continue;
+               if (unlikely((http_hdng_chk.rules & HTTP_HRD_DEL_UNREG_HDR) &&
+                            (list[idx].n.len > 1 && list[idx].n.ptr[1] == '-' &&
+                             (list[idx].n.ptr[0] == 'x' || list[idx].n.ptr[0] == 'X'))) &&
+                   http_harden(HTTP_HRD_DEL_UNREG_HDR)) {
+                       /* skip it, so it won't be added */
+                       continue;
                }
 
                if (unlikely(!(fields & H2_PHDR_FND_NONE))) {
@@ -747,11 +748,12 @@ int h2_make_htx_response(struct http_hdr *list, struct htx *htx, unsigned int *m
                }
 
                /* we may have to delete unregistered header names (X-) */
-               if (unlikely(http_harden(HTTP_HRD_DEL_UNREG_HDR))) {
-                       if (list[idx].n.len > 1 && list[idx].n.ptr[1] == '-' &&
-                           (list[idx].n.ptr[0] == 'x' || list[idx].n.ptr[0] == 'X'))
-                               /* skip it, so it won't be added */
-                               continue;
+               if (unlikely((http_hdng_chk.rules & HTTP_HRD_DEL_UNREG_HDR) &&
+                            (list[idx].n.len > 1 && list[idx].n.ptr[1] == '-' &&
+                             (list[idx].n.ptr[0] == 'x' || list[idx].n.ptr[0] == 'X'))) &&
+                   http_harden(HTTP_HRD_DEL_UNREG_HDR)) {
+                       /* skip it, so it won't be added */
+                       continue;
                }
 
                if (!(fields & H2_PHDR_FND_NONE)) {

diff --git a/src/h3.c b/src/h3.c
index e4ba9eb0b99f82612682772e5eab3e19fe977761..167ce9d9b875012813315fdcebee9df1d88bf9da 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -1069,13 +1069,13 @@ static ssize_t h3_req_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
                }
 
                /* we may have to delete unregistered header names (X-) */
-               if (unlikely(http_harden(HTTP_HRD_DEL_UNREG_HDR))) {
-                       if (list[hdr_idx].n.len > 1 && list[hdr_idx].n.ptr[1] == '-' &&
-                           (list[hdr_idx].n.ptr[0] == 'x' || list[hdr_idx].n.ptr[0] == 'X')) {
-                               /* skip it, so it won't be added */
-                               ++hdr_idx;
-                               continue;
-                       }
+               if (unlikely((http_hdng_chk.rules & HTTP_HRD_DEL_UNREG_HDR) &&
+                            (list[hdr_idx].n.len > 1 && list[hdr_idx].n.ptr[1] == '-' &&
+                             (list[hdr_idx].n.ptr[0] == 'x' || list[hdr_idx].n.ptr[0] == 'X'))) &&
+                   http_harden(HTTP_HRD_DEL_UNREG_HDR)) {
+                       /* skip it, so it won't be added */
+                       ++hdr_idx;
+                       continue;
                }
 
                if (isteq(list[hdr_idx].n, ist("host"))) {

diff --git a/src/http_htx.c b/src/http_htx.c
index 163508340bc3bd5024ff3544565c4c6787b7a8cc..51853655923752af3b08ad34ed5efa3b20f0973c 100644 (file)
--- a/src/http_htx.c
+++ b/src/http_htx.c
@@ -2090,11 +2090,12 @@ enum http_parser_status http_trailers_to_htx(struct http_hdr *list, struct htx *
                }
 
                /* we may have to delete unregistered header names (X-) */
-               if (unlikely(http_harden(HTTP_HRD_DEL_UNREG_TLR))) {
-                       if (list[idx].n.len > 1 && list[idx].n.ptr[1] == '-' &&
-                           (list[idx].n.ptr[0] == 'x' || list[idx].n.ptr[0] == 'X'))
-                               /* skip it, so it won't be added */
-                               continue;
+               if (unlikely(http_hdng_chk.rules & HTTP_HRD_DEL_UNREG_TLR) &&
+                   (list[idx].n.len > 1 && list[idx].n.ptr[1] == '-' &&
+                    (list[idx].n.ptr[0] == 'x' || list[idx].n.ptr[0] == 'X')) &&
+                   http_harden(HTTP_HRD_DEL_UNREG_TLR)) {
+                       /* skip it, so it won't be added */
+                       continue;
                }
 
                /* trim leading/trailing LWS as per RC9113#8.2.1 */