BUG/MEDIUM: h1: drop headers whose names contain invalid chars

Originally with "option accept-invalid-http-request", we couldn't really
edit the request on the fly to remove offending headers. But since we
have HTX and the headers are indexed one at a time, it has become
trivial. A non-negligible number of violations are conditioned by the
now renamed "option accept-unsafe-violations-in-http-request", and a
controversial one could definitely be reporting and passing invalid
header names containing control chars or spaces. The option was placed
so as not to block requests/responses containing them, but there's no
point in passing them to the other side. Most of the time it will be
totally harmless since the other side will reject them. But in case
haproxy is placed in front of a non-compliant server, it would fail
to protect it.

This patch implements a name check for all headers when a parsing
error was detected. It's cheap enough (especially since only done
after an error), and will skip the header if its name is invalid.
This may also remove some possibilities of confusion in logs, or
when encoding headers names for example.

This should be backported at least till the latest LTS.

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 2585c8505e0ab2f325e24dec4d786ed7fa8cb01f..ca82a77952ba50bbd92f92b08b95b83333808344 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -9962,7 +9962,7 @@ no option accept-unsafe-violations-in-http-request
   When this option is set, the following rules are observed:
 
     * In H1 only, invalid characters, including NULL character, in header name
-      will be accepted;
+      will not be rejected; however the header will be dropped.
 
     * In H1 only, NULL character in header value will be accepted;
 
@@ -10028,7 +10028,7 @@ no option accept-unsafe-violations-in-http-response
   When this option is set, the following rules are observed:
 
     * In H1 only, invalid characters, including NULL character, in header name
-      will be accepted;
+      will not be rejected; however the header will be dropped.
 
     * In H1 only, NULL character in header value will be accepted;
 

diff --git a/src/h1.c b/src/h1.c
index 4ea75919135e4595d4afd0aeea5f3be40ade0fa2..e1df21a50a48bb493e8f08e416e999cd7a93f629 100644 (file)
--- a/src/h1.c
+++ b/src/h1.c
@@ -952,6 +952,20 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
                                        goto http_output_full;
                                }
 
+                               /* Skip headers whose names contain forbidden
+                                * chars. When any is detected, h1m->err_pos >= 0,
+                                * so we recheck the name only when an error was
+                                * detected.
+                                */
+                               if (unlikely(h1m->err_pos >= 0)) {
+                                       size_t i = 0;
+                                       while (i < n.len && HTTP_IS_TOKEN(n.ptr[i]))
+                                               i++;
+
+                                       if (i < n.len)
+                                               break;
+                               }
+
                                if (isteqi(n, ist("transfer-encoding"))) {
                                        ret = h1_parse_xfer_enc_header(h1m, v);
                                        if (ret < 0) {