MINOR: mux-h1: Add global option accpet payload for any HTTP/1.0 requests

Since the 2.5, for security reason, HTTP/1.0 GET/HEAD/DELETE requests with a
payload are rejected (See e136bd12a "MEDIUM: mux-h1: Reject HTTP/1.0
GET/HEAD/DELETE requests with a payload" for details). However it may be an
issue for old clients.

To avoid any compatibility issue with such clients,
"h1-accept-payload-with-any-method" global option was added. It must only be
set if there is a good reason to do so because it may lead to a request
smuggling attack on some servers or intermediaries.

This patch should solve the issue #1691. it may be backported to 2.5.

diff --git a/doc/configuration.txt b/doc/configuration.txt
index d5283270c25b74202df5a96a96e7c5f0bd93be00..96210d80295b6bf43e589e82471ce2e2da082300 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -1013,6 +1013,7 @@ The following keywords are supported in the "global" section :
    - httpclient.resolvers.prefer
    - httpclient.ssl.ca-file
    - httpclient.ssl.verify
+   - h1-accept-payload-with-any-method
    - h1-case-adjust
    - h1-case-adjust-file
    - insecure-fork-wanted
@@ -1450,6 +1451,20 @@ hard-stop-after <time>
 
   See also: grace
 
+h1-accept-payload-with-any-method
+  Does not reject HTTP/1.0 GET/HEAD/DELETE requests with a payload.
+
+  While It is explicitly allowed in HTTP/1.1, HTTP/1.0 is not clear on this
+  point and some old servers don't expect any payload and never look for body
+  length (via Content-Length or Transfer-Encoding headers). It means that some
+  intermediaries may properly handle the payload for HTTP/1.0 GET/HEAD/DELETE
+  requests, while some others may totally ignore it. That may lead to security
+  issues because a request smuggling attack is possible. Thus, by default,
+  HAProxy rejects HTTP/1.0 GET/HEAD/DELETE requests with a payload.
+
+  However, it may be an issue with some old clients. In this case, this global
+  option may be set.
+
 h1-case-adjust <from> <to>
   Defines the case adjustment to apply, when enabled, to the header name
   <from>, to change it to <to> before sending it to HTTP/1 clients or

diff --git a/src/mux_h1.c b/src/mux_h1.c
index f9eba517506c707c33cbe3be7eb463d9e5a37424..5085e11e05521952ed28de0658aabcf0186a0930 100644 (file)
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -149,7 +149,7 @@ struct h1_hdr_entry  {
 
 /* Declare the headers map */
 static struct h1_hdrs_map hdrs_map = { .name = NULL, .map  = EB_ROOT };
-
+static int accept_payload_with_any_method = 0;
 
 /* trace source and events */
 static void h1_trace(enum trace_level level, uint64_t mask,
@@ -1602,12 +1602,14 @@ static size_t h1_handle_headers(struct h1s *h1s, struct h1m *h1m, struct htx *ht
        }
 
 
-       /* Reject HTTP/1.0 GET/HEAD/DELETE requests with a payload. There is a
-        * payload if the c-l is not null or the the payload is chunk-encoded.
-        * A parsing error is reported but a A 413-Payload-Too-Large is returned
-        * instead of a 400-Bad-Request.
+       /* Reject HTTP/1.0 GET/HEAD/DELETE requests with a payload except if
+        * accept_payload_with_any_method global option is set.
+        *There is a payload if the c-l is not null or the the payload is
+        * chunk-encoded.  A parsing error is reported but a A
+        * 413-Payload-Too-Large is returned instead of a 400-Bad-Request.
         */
-       if (!(h1m->flags & (H1_MF_RESP|H1_MF_VER_11)) &&
+       if (!accept_payload_with_any_method &&
+           !(h1m->flags & (H1_MF_RESP|H1_MF_VER_11)) &&
            (((h1m->flags & H1_MF_CLEN) && h1m->body_len) || (h1m->flags & H1_MF_CHNK)) &&
            (h1sl.rq.meth == HTTP_METH_GET || h1sl.rq.meth == HTTP_METH_HEAD || h1sl.rq.meth == HTTP_METH_DELETE)) {
                h1s->flags |= H1S_F_PARSING_ERROR;
@@ -4137,6 +4139,17 @@ static int cfg_h1_headers_case_adjust_postparser()
        return err_code;
 }
 
+/* config parser for global "h1-accept-payload_=-with-any-method" */
+static int cfg_parse_h1_accept_payload_with_any_method(char **args, int section_type, struct proxy *curpx,
+                                                      const struct proxy *defpx, const char *file, int line,
+                                                      char **err)
+{
+        if (too_many_args(0, args, err, NULL))
+                return -1;
+       accept_payload_with_any_method = 1;
+       return 0;
+}
+
 
 /* config parser for global "h1-outgoing-header-case-adjust" */
 static int cfg_parse_h1_header_case_adjust(char **args, int section_type, struct proxy *curpx,
@@ -4168,9 +4181,9 @@ static int cfg_parse_h1_headers_case_adjust_file(char **args, int section_type,
         return 0;
 }
 
-
 /* config keyword parsers */
 static struct cfg_kw_list cfg_kws = {{ }, {
+               { CFG_GLOBAL, "h1-accept-payload-with-any-method", cfg_parse_h1_accept_payload_with_any_method },
                { CFG_GLOBAL, "h1-case-adjust", cfg_parse_h1_header_case_adjust },
                { CFG_GLOBAL, "h1-case-adjust-file", cfg_parse_h1_headers_case_adjust_file },
                { 0, NULL, NULL },