From: Christopher Faulet <cfaulet@haproxy.com>
Date: Fri, 13 May 2022 07:20:13 +0000 (+0200)
Subject: MINOR: mux-h1: Add global option accpet payload for any HTTP/1.0 requests
X-Git-Tag: v2.6-dev10~32
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0f9c0f5801fb3721a5b6e5efb492f942b6ffbd5e;p=thirdparty%2Fhaproxy.git

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 d5283270c2..96210d8029 100644
--- 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 f9eba51750..5085e11e05 100644
--- 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 },