From b798412444aeace740c574486e810735688c02c8 Mon Sep 17 00:00:00 2001 From: Emile Fugulin Date: Mon, 18 Nov 2019 19:44:01 -0500 Subject: [PATCH 1/5] Remove default strip of header name --- gunicorn/config.py | 17 +++++++++++++++++ gunicorn/http/message.py | 5 ++++- tests/requests/invalid/020.http | 4 ++++ tests/requests/invalid/020.py | 5 +++++ tests/requests/valid/028.http | 4 ++++ tests/requests/valid/028.py | 14 ++++++++++++++ 6 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 tests/requests/invalid/020.http create mode 100644 tests/requests/invalid/020.py create mode 100644 tests/requests/valid/028.http create mode 100644 tests/requests/valid/028.py diff --git a/gunicorn/config.py b/gunicorn/config.py index e8e0f926..d165f256 100644 --- a/gunicorn/config.py +++ b/gunicorn/config.py @@ -2010,3 +2010,20 @@ class PasteGlobalConf(Setting): .. versionadded:: 19.7 """ + + +class StripHeaderSpaces(Setting): + name = "strip_header_spaces" + section = "Server Mechanics" + cli = ["--strip-header-spaces"] + validator = validate_bool + action = "store_true" + default = False + desc = """\ + Strip spaces present between the header name and the the ``:``. + + This is known to induce vulnerabilities and is not compliant with the HTTP/1.1 standard. + See https://portswigger.net/research/http-desync-attacks-request-smuggling-reborn. + + Use with care and only if necessary. + """ diff --git a/gunicorn/http/message.py b/gunicorn/http/message.py index 4040c7ae..5807a464 100644 --- a/gunicorn/http/message.py +++ b/gunicorn/http/message.py @@ -90,7 +90,10 @@ class Message(object): if curr.find(":") < 0: raise InvalidHeader(curr.strip()) name, value = curr.split(":", 1) - name = name.rstrip(" \t").upper() + if self.cfg.strip_header_spaces: + name = name.rstrip(" \t").upper() + else: + name = name.upper() if HEADER_RE.search(name): raise InvalidHeaderName(name) diff --git a/tests/requests/invalid/020.http b/tests/requests/invalid/020.http new file mode 100644 index 00000000..a699e848 --- /dev/null +++ b/tests/requests/invalid/020.http @@ -0,0 +1,4 @@ +GET /stuff/here?foo=bar HTTP/1.1\r\n +Content-Length : 3\r\n +\r\n +xyz diff --git a/tests/requests/invalid/020.py b/tests/requests/invalid/020.py new file mode 100644 index 00000000..d336fbc8 --- /dev/null +++ b/tests/requests/invalid/020.py @@ -0,0 +1,5 @@ +from gunicorn.config import Config +from gunicorn.http.errors import InvalidHeaderName + +cfg = Config() +request = InvalidHeaderName diff --git a/tests/requests/valid/028.http b/tests/requests/valid/028.http new file mode 100644 index 00000000..9db5ecfb --- /dev/null +++ b/tests/requests/valid/028.http @@ -0,0 +1,4 @@ +GET /stuff/here?foo=bar HTTP/1.1\r\n +Content-Length : 3\r\n +\r\n +xyz \ No newline at end of file diff --git a/tests/requests/valid/028.py b/tests/requests/valid/028.py new file mode 100644 index 00000000..d8254683 --- /dev/null +++ b/tests/requests/valid/028.py @@ -0,0 +1,14 @@ +from gunicorn.config import Config + +cfg = Config() +cfg.set("strip_header_spaces", True) + +request = { + "method": "GET", + "uri": uri("/stuff/here?foo=bar"), + "version": (1, 1), + "headers": [ + ("CONTENT-LENGTH", "3"), + ], + "body": b"xyz" +} \ No newline at end of file From bd8670b4db321727de22d934dee6dc1c2d41f704 Mon Sep 17 00:00:00 2001 From: Emile Fugulin Date: Mon, 18 Nov 2019 20:49:22 -0500 Subject: [PATCH 2/5] Handle duplicate content-length --- gunicorn/http/message.py | 2 ++ tests/requests/invalid/021.http | 5 +++++ tests/requests/invalid/021.py | 5 +++++ 3 files changed, 12 insertions(+) create mode 100644 tests/requests/invalid/021.http create mode 100644 tests/requests/invalid/021.py diff --git a/gunicorn/http/message.py b/gunicorn/http/message.py index 5807a464..cbfbd11c 100644 --- a/gunicorn/http/message.py +++ b/gunicorn/http/message.py @@ -131,6 +131,8 @@ class Message(object): content_length = None for (name, value) in self.headers: if name == "CONTENT-LENGTH": + if content_length is not None: + raise InvalidHeader("CONTENT-LENGTH", req=self) content_length = value elif name == "TRANSFER-ENCODING": chunked = value.lower() == "chunked" diff --git a/tests/requests/invalid/021.http b/tests/requests/invalid/021.http new file mode 100644 index 00000000..90e77dd1 --- /dev/null +++ b/tests/requests/invalid/021.http @@ -0,0 +1,5 @@ +GET /stuff/here?foo=bar HTTP/1.1\r\n +Content-Length: 3\r\n +Content-Length: 2\r\n +\r\n +xyz diff --git a/tests/requests/invalid/021.py b/tests/requests/invalid/021.py new file mode 100644 index 00000000..95b0581a --- /dev/null +++ b/tests/requests/invalid/021.py @@ -0,0 +1,5 @@ +from gunicorn.config import Config +from gunicorn.http.errors import InvalidHeader + +cfg = Config() +request = InvalidHeader From f74324bd750265a3c1f47b4c837f6fd7ce74db54 Mon Sep 17 00:00:00 2001 From: Emile Fugulin Date: Mon, 18 Nov 2019 22:29:02 -0500 Subject: [PATCH 3/5] Handle multiple transfer-encoding --- gunicorn/http/errors.py | 8 ++++++++ gunicorn/http/message.py | 10 ++++++++-- gunicorn/workers/base.py | 7 ++++++- tests/requests/invalid/022.http | 5 +++++ tests/requests/invalid/022.py | 5 +++++ tests/requests/valid/029.http | 7 +++++++ tests/requests/valid/029.py | 14 ++++++++++++++ tests/requests/valid/030.http | 7 +++++++ tests/requests/valid/030.py | 14 ++++++++++++++ 9 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 tests/requests/invalid/022.http create mode 100644 tests/requests/invalid/022.py create mode 100644 tests/requests/valid/029.http create mode 100644 tests/requests/valid/029.py create mode 100644 tests/requests/valid/030.http create mode 100644 tests/requests/valid/030.py diff --git a/gunicorn/http/errors.py b/gunicorn/http/errors.py index 7839ef05..ea5b4826 100644 --- a/gunicorn/http/errors.py +++ b/gunicorn/http/errors.py @@ -118,3 +118,11 @@ class ForbiddenProxyRequest(ParseException): class InvalidSchemeHeaders(ParseException): def __str__(self): return "Contradictory scheme headers" + + +class UnsupportedTransferEncoding(ParseException): + def __init__(self, te): + self.te = te + + def __str__(self): + return "Unsupported Transfer-Encoding: %s" % self.te diff --git a/gunicorn/http/message.py b/gunicorn/http/message.py index cbfbd11c..59f50d7e 100644 --- a/gunicorn/http/message.py +++ b/gunicorn/http/message.py @@ -12,7 +12,7 @@ from gunicorn.http.unreader import SocketUnreader from gunicorn.http.body import ChunkedReader, LengthReader, EOFReader, Body from gunicorn.http.errors import (InvalidHeader, InvalidHeaderName, NoMoreData, InvalidRequestLine, InvalidRequestMethod, InvalidHTTPVersion, - LimitRequestLine, LimitRequestHeaders) + LimitRequestLine, LimitRequestHeaders, UnsupportedTransferEncoding) from gunicorn.http.errors import InvalidProxyLine, ForbiddenProxyRequest from gunicorn.http.errors import InvalidSchemeHeaders from gunicorn.util import bytes_to_str, split_request_uri @@ -135,7 +135,13 @@ class Message(object): raise InvalidHeader("CONTENT-LENGTH", req=self) content_length = value elif name == "TRANSFER-ENCODING": - chunked = value.lower() == "chunked" + normalized_value = value.lower() + if normalized_value == "identity": + pass + elif normalized_value == "chunked": + chunked = True + else: + raise UnsupportedTransferEncoding(normalized_value) elif name == "SEC-WEBSOCKET-KEY1": content_length = 8 diff --git a/gunicorn/workers/base.py b/gunicorn/workers/base.py index f95994bc..7689c61f 100644 --- a/gunicorn/workers/base.py +++ b/gunicorn/workers/base.py @@ -20,6 +20,7 @@ from gunicorn.http.errors import ( InvalidProxyLine, InvalidRequestLine, InvalidRequestMethod, InvalidSchemeHeaders, LimitRequestHeaders, LimitRequestLine, + UnsupportedTransferEncoding ) from gunicorn.http.wsgi import Response, default_environ from gunicorn.reloader import reloader_engines @@ -206,7 +207,7 @@ class Worker(object): LimitRequestLine, LimitRequestHeaders, InvalidProxyLine, ForbiddenProxyRequest, InvalidSchemeHeaders, - SSLError)): + SSLError, UnsupportedTransferEncoding)): status_int = 400 reason = "Bad Request" @@ -237,6 +238,10 @@ class Worker(object): reason = "Forbidden" mesg = "'%s'" % str(exc) status_int = 403 + elif isinstance(exc, UnsupportedTransferEncoding): + reason = "Not implemented" + mesg = "'%s'" % str(exc) + status_int = 501 msg = "Invalid request from ip={ip}: {error}" self.log.debug(msg.format(ip=addr[0], error=str(exc))) diff --git a/tests/requests/invalid/022.http b/tests/requests/invalid/022.http new file mode 100644 index 00000000..784504be --- /dev/null +++ b/tests/requests/invalid/022.http @@ -0,0 +1,5 @@ +GET /stuff/here?foo=bar HTTP/1.1\r\n +Transfer-Encoding: chunked\r\n +Transfer-Encoding: compress\r\n +\r\n +xyz diff --git a/tests/requests/invalid/022.py b/tests/requests/invalid/022.py new file mode 100644 index 00000000..db5c9f38 --- /dev/null +++ b/tests/requests/invalid/022.py @@ -0,0 +1,5 @@ +from gunicorn.config import Config +from gunicorn.http.errors import UnsupportedTransferEncoding + +cfg = Config() +request = UnsupportedTransferEncoding diff --git a/tests/requests/valid/029.http b/tests/requests/valid/029.http new file mode 100644 index 00000000..c8611dbd --- /dev/null +++ b/tests/requests/valid/029.http @@ -0,0 +1,7 @@ +GET /stuff/here?foo=bar HTTP/1.1\r\n +Transfer-Encoding: chunked\r\n +Transfer-Encoding: identity\r\n +\r\n +5\r\n +hello\r\n +000\r\n diff --git a/tests/requests/valid/029.py b/tests/requests/valid/029.py new file mode 100644 index 00000000..f25449d1 --- /dev/null +++ b/tests/requests/valid/029.py @@ -0,0 +1,14 @@ +from gunicorn.config import Config + +cfg = Config() + +request = { + "method": "GET", + "uri": uri("/stuff/here?foo=bar"), + "version": (1, 1), + "headers": [ + ('TRANSFER-ENCODING', 'chunked'), + ('TRANSFER-ENCODING', 'identity') + ], + "body": b"hello" +} diff --git a/tests/requests/valid/030.http b/tests/requests/valid/030.http new file mode 100644 index 00000000..5d029dd9 --- /dev/null +++ b/tests/requests/valid/030.http @@ -0,0 +1,7 @@ +GET /stuff/here?foo=bar HTTP/1.1\r\n +Transfer-Encoding: identity\r\n +Transfer-Encoding: chunked\r\n +\r\n +5\r\n +hello\r\n +000\r\n diff --git a/tests/requests/valid/030.py b/tests/requests/valid/030.py new file mode 100644 index 00000000..3e98467b --- /dev/null +++ b/tests/requests/valid/030.py @@ -0,0 +1,14 @@ +from gunicorn.config import Config + +cfg = Config() + +request = { + "method": "GET", + "uri": uri("/stuff/here?foo=bar"), + "version": (1, 1), + "headers": [ + ('TRANSFER-ENCODING', 'identity'), + ('TRANSFER-ENCODING', 'chunked') + ], + "body": b"hello" +} From ddf5e66ac864c62d5426d96247f9156a18134597 Mon Sep 17 00:00:00 2001 From: Emile Fugulin Date: Wed, 20 Nov 2019 12:24:52 -0500 Subject: [PATCH 4/5] Remove strict check of Transfer-Encoding --- gunicorn/http/errors.py | 8 -------- gunicorn/http/message.py | 9 ++------- gunicorn/workers/base.py | 7 +------ tests/requests/invalid/022.http | 5 ----- tests/requests/invalid/022.py | 5 ----- 5 files changed, 3 insertions(+), 31 deletions(-) delete mode 100644 tests/requests/invalid/022.http delete mode 100644 tests/requests/invalid/022.py diff --git a/gunicorn/http/errors.py b/gunicorn/http/errors.py index ea5b4826..7839ef05 100644 --- a/gunicorn/http/errors.py +++ b/gunicorn/http/errors.py @@ -118,11 +118,3 @@ class ForbiddenProxyRequest(ParseException): class InvalidSchemeHeaders(ParseException): def __str__(self): return "Contradictory scheme headers" - - -class UnsupportedTransferEncoding(ParseException): - def __init__(self, te): - self.te = te - - def __str__(self): - return "Unsupported Transfer-Encoding: %s" % self.te diff --git a/gunicorn/http/message.py b/gunicorn/http/message.py index 59f50d7e..e5ce4a68 100644 --- a/gunicorn/http/message.py +++ b/gunicorn/http/message.py @@ -12,7 +12,7 @@ from gunicorn.http.unreader import SocketUnreader from gunicorn.http.body import ChunkedReader, LengthReader, EOFReader, Body from gunicorn.http.errors import (InvalidHeader, InvalidHeaderName, NoMoreData, InvalidRequestLine, InvalidRequestMethod, InvalidHTTPVersion, - LimitRequestLine, LimitRequestHeaders, UnsupportedTransferEncoding) + LimitRequestLine, LimitRequestHeaders) from gunicorn.http.errors import InvalidProxyLine, ForbiddenProxyRequest from gunicorn.http.errors import InvalidSchemeHeaders from gunicorn.util import bytes_to_str, split_request_uri @@ -135,13 +135,8 @@ class Message(object): raise InvalidHeader("CONTENT-LENGTH", req=self) content_length = value elif name == "TRANSFER-ENCODING": - normalized_value = value.lower() - if normalized_value == "identity": - pass - elif normalized_value == "chunked": + if value.lower() == "chunked": chunked = True - else: - raise UnsupportedTransferEncoding(normalized_value) elif name == "SEC-WEBSOCKET-KEY1": content_length = 8 diff --git a/gunicorn/workers/base.py b/gunicorn/workers/base.py index 7689c61f..f95994bc 100644 --- a/gunicorn/workers/base.py +++ b/gunicorn/workers/base.py @@ -20,7 +20,6 @@ from gunicorn.http.errors import ( InvalidProxyLine, InvalidRequestLine, InvalidRequestMethod, InvalidSchemeHeaders, LimitRequestHeaders, LimitRequestLine, - UnsupportedTransferEncoding ) from gunicorn.http.wsgi import Response, default_environ from gunicorn.reloader import reloader_engines @@ -207,7 +206,7 @@ class Worker(object): LimitRequestLine, LimitRequestHeaders, InvalidProxyLine, ForbiddenProxyRequest, InvalidSchemeHeaders, - SSLError, UnsupportedTransferEncoding)): + SSLError)): status_int = 400 reason = "Bad Request" @@ -238,10 +237,6 @@ class Worker(object): reason = "Forbidden" mesg = "'%s'" % str(exc) status_int = 403 - elif isinstance(exc, UnsupportedTransferEncoding): - reason = "Not implemented" - mesg = "'%s'" % str(exc) - status_int = 501 msg = "Invalid request from ip={ip}: {error}" self.log.debug(msg.format(ip=addr[0], error=str(exc))) diff --git a/tests/requests/invalid/022.http b/tests/requests/invalid/022.http deleted file mode 100644 index 784504be..00000000 --- a/tests/requests/invalid/022.http +++ /dev/null @@ -1,5 +0,0 @@ -GET /stuff/here?foo=bar HTTP/1.1\r\n -Transfer-Encoding: chunked\r\n -Transfer-Encoding: compress\r\n -\r\n -xyz diff --git a/tests/requests/invalid/022.py b/tests/requests/invalid/022.py deleted file mode 100644 index db5c9f38..00000000 --- a/tests/requests/invalid/022.py +++ /dev/null @@ -1,5 +0,0 @@ -from gunicorn.config import Config -from gunicorn.http.errors import UnsupportedTransferEncoding - -cfg = Config() -request = UnsupportedTransferEncoding From be513237cc4bcabc5b5988504294f8eb1da5f745 Mon Sep 17 00:00:00 2001 From: Emile Fugulin Date: Wed, 20 Nov 2019 12:57:46 -0500 Subject: [PATCH 5/5] Add syttent to THANKS --- THANKS | 1 + 1 file changed, 1 insertion(+) diff --git a/THANKS b/THANKS index 2c2a6de5..09a15fad 100644 --- a/THANKS +++ b/THANKS @@ -178,3 +178,4 @@ WooParadog Xie Shi Yue Du zakdances +Emile Fugulin