Projects
Eulaceura:Mainline:GA
python-waitress
_service:obs_scm:backport-CVE-2024-49768.patch
Sign Up
Log In
Username
Password
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File _service:obs_scm:backport-CVE-2024-49768.patch of Package python-waitress
From 6943dcf556610ece2ff3cddb39e59a05ef110661 Mon Sep 17 00:00:00 2001 From: Delta Regeer <bertjw@regeer.org> Date: Sat, 26 Oct 2024 22:10:36 -0600 Subject: [PATCH 1/4] Make DummySock() look more like an actual socket This forces DummySock() to look like a properly connected socket where there is a buffer that is read from by the remote, and a buffer that is written to by the remote. The local side does the opposite, this way data written by the local side can be read by the remote without operating on the same buffer. --- tests/test_channel.py | 57 +++++++++++++++++++++++++++++++++---------- 1 file changed, 44 insertions(+), 13 deletions(-) diff --git a/tests/test_channel.py b/tests/test_channel.py index 8467ae7..7d677e9 100644 --- a/tests/test_channel.py +++ b/tests/test_channel.py @@ -18,7 +18,7 @@ class TestHTTPChannel(unittest.TestCase): map = {} inst = self._makeOne(sock, "127.0.0.1", adj, map=map) inst.outbuf_lock = DummyLock() - return inst, sock, map + return inst, sock.local(), map def test_ctor(self): inst, _, map = self._makeOneWithMap() @@ -218,7 +218,7 @@ class TestHTTPChannel(unittest.TestCase): def send(_): return 0 - sock.send = send + sock.remote.send = send wrote = inst.write_soon(b"a") self.assertEqual(wrote, 1) @@ -236,7 +236,7 @@ class TestHTTPChannel(unittest.TestCase): def send(_): return 0 - sock.send = send + sock.remote.send = send outbufs = inst.outbufs wrote = inst.write_soon(wrapper) @@ -270,7 +270,7 @@ class TestHTTPChannel(unittest.TestCase): def send(_): return 0 - sock.send = send + sock.remote.send = send inst.adj.outbuf_high_watermark = 3 inst.current_outbuf_count = 4 @@ -286,7 +286,7 @@ class TestHTTPChannel(unittest.TestCase): def send(_): return 0 - sock.send = send + sock.remote.send = send inst.adj.outbuf_high_watermark = 3 inst.total_outbufs_len = 4 @@ -315,7 +315,7 @@ class TestHTTPChannel(unittest.TestCase): inst.connected = False raise Exception() - sock.send = send + sock.remote.send = send inst.adj.outbuf_high_watermark = 3 inst.total_outbufs_len = 4 @@ -345,7 +345,7 @@ class TestHTTPChannel(unittest.TestCase): inst.connected = False raise Exception() - sock.send = send + sock.remote.send = send wrote = inst.write_soon(b"xyz") self.assertEqual(wrote, 3) @@ -376,7 +376,7 @@ class TestHTTPChannel(unittest.TestCase): inst.total_outbufs_len = len(inst.outbufs[0]) inst.adj.send_bytes = 1 inst.adj.outbuf_high_watermark = 2 - sock.send = lambda x, do_close=True: False + sock.remote.send = lambda x, do_close=True: False inst.will_close = False inst.last_activity = 0 result = inst.handle_write() @@ -400,7 +400,7 @@ class TestHTTPChannel(unittest.TestCase): def test__flush_some_full_outbuf_socket_returns_zero(self): inst, sock, map = self._makeOneWithMap() - sock.send = lambda x: False + sock.remote.send = lambda x: False inst.outbufs[0].append(b"abc") inst.total_outbufs_len = sum(len(x) for x in inst.outbufs) result = inst._flush_some() @@ -907,7 +907,8 @@ class DummySock: closed = False def __init__(self): - self.sent = b"" + self.local_sent = b"" + self.remote_sent = b"" def setblocking(self, *arg): self.blocking = True @@ -925,14 +926,44 @@ class DummySock: self.closed = True def send(self, data): - self.sent += data + self.remote_sent += data return len(data) def recv(self, buffer_size): - result = self.sent[:buffer_size] - self.sent = self.sent[buffer_size:] + result = self.local_sent[:buffer_size] + self.local_sent = self.local_sent[buffer_size:] return result + def local(self): + outer = self + + class LocalDummySock: + def send(self, data): + outer.local_sent += data + return len(data) + + def recv(self, buffer_size): + result = outer.remote_sent[:buffer_size] + outer.remote_sent = outer.remote_sent[buffer_size:] + return result + + def close(self): + outer.closed = True + + @property + def sent(self): + return outer.remote_sent + + @property + def closed(self): + return outer.closed + + @property + def remote(self): + return outer + + return LocalDummySock() + class DummyLock: notified = False -- Gitee From 7e7f11e61d358ab1cb853fcadf2b46b1f00f5993 Mon Sep 17 00:00:00 2001 From: Delta Regeer <bertjw@regeer.org> Date: Sat, 26 Oct 2024 22:12:14 -0600 Subject: [PATCH 2/4] Add a new test to validate the lookahead race condition --- tests/test_channel.py | 55 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/tests/test_channel.py b/tests/test_channel.py index 7d677e9..d798091 100644 --- a/tests/test_channel.py +++ b/tests/test_channel.py @@ -805,11 +805,12 @@ class TestHTTPChannelLookahead(TestHTTPChannel): ) return [body] - def _make_app_with_lookahead(self): + def _make_app_with_lookahead(self, recv_bytes=8192): """ Setup a channel with lookahead and store it and the socket in self """ adj = DummyAdjustments() + adj.recv_bytes = recv_bytes adj.channel_request_lookahead = 5 channel, sock, map = self._makeOneWithMap(adj=adj) channel.server.application = self.app_check_disconnect @@ -901,6 +902,58 @@ class TestHTTPChannelLookahead(TestHTTPChannel): self.assertEqual(data.split("\r\n")[-1], "finished") self.assertEqual(self.request_body, b"x") + def test_lookahead_bad_request_drop_extra_data(self): + """ + Send two requests, the first one being bad, split on the recv_bytes + limit, then emulate a race that could happen whereby we read data from + the socket while the service thread is cleaning up due to an error + processing the request. + """ + + invalid_request = [ + "GET / HTTP/1.1", + "Host: localhost:8080", + "Content-length: -1", + "", + ] + + invalid_request_len = len("".join([x + "\r\n" for x in invalid_request])) + + second_request = [ + "POST / HTTP/1.1", + "Host: localhost:8080", + "Content-Length: 1", + "", + "x", + ] + + full_request = invalid_request + second_request + + self._make_app_with_lookahead(recv_bytes=invalid_request_len) + self._send(*full_request) + self.channel.handle_read() + self.assertEqual(len(self.channel.requests), 1) + self.channel.server.tasks[0].service() + self.assertTrue(self.channel.close_when_flushed) + # Read all of the next request + self.channel.handle_read() + self.channel.handle_read() + # Validate that there is no more data to be read + self.assertEqual(self.sock.remote.local_sent, b"") + # Validate that we dropped the data from the second read, and did not + # create a new request + self.assertEqual(len(self.channel.requests), 0) + data = self.sock.recv(256).decode("ascii") + self.assertFalse(self.channel.readable()) + self.assertTrue(self.channel.writable()) + + # Handle the write, which will close the socket + self.channel.handle_write() + self.assertTrue(self.sock.closed) + + data = self.sock.recv(256) + self.assertEqual(len(data), 0) + class DummySock: blocking = False -- Gitee From f4ba1c260cf17156b582c6252496213ddc96b591 Mon Sep 17 00:00:00 2001 From: Delta Regeer <bertjw@regeer.org> Date: Sat, 26 Oct 2024 22:13:08 -0600 Subject: [PATCH 3/4] Fix a race condition on recv_bytes boundary when request is invalid A remote client may send a request that is exactly recv_bytes long, followed by a secondary request using HTTP pipelining. When request lookahead is disabled (default) we won't read any more requests, and when the first request fails due to a parsing error, we simply close the connection. However when request lookahead is enabled, it is possible to process and receive the first request, start sending the error message back to the client while we read the next request and queue it. This will allow the secondar request to be serviced by the worker thread while the connection should be closed. The fix here checks if we should not have read the data in the first place (because the conection is going to be torn down) while we hold the `requests_lock` which means the service thread can't be in the middle of flipping the `close_when_flushed` flag. --- src/waitress/channel.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/waitress/channel.py b/src/waitress/channel.py index 3860ed5..f4d9677 100644 --- a/src/waitress/channel.py +++ b/src/waitress/channel.py @@ -140,7 +140,7 @@ class HTTPChannel(wasyncore.dispatcher): # 1. We're not already about to close the connection. # 2. We're not waiting to flush remaining data before closing the # connection - # 3. There are not too many tasks already queued + # 3. There are not too many tasks already queued (if lookahead is enabled) # 4. There's no data in the output buffer that needs to be sent # before we potentially create a new task. @@ -196,6 +196,15 @@ class HTTPChannel(wasyncore.dispatcher): return False with self.requests_lock: + # Don't bother processing anymore data if this connection is about + # to close. This may happen if readable() returned True, on the + # main thread before the service thread set the close_when_flushed + # flag, and we read data but our service thread is attempting to + # shut down the connection due to an error. We want to make sure we + # do this while holding the request_lock so that we can't race + if self.will_close or self.close_when_flushed: + return False + while data: if self.request is None: self.request = self.parser_class(self.adj) -- Gitee From 810a435f9e9e293bd3446a5ce2df86f59c4e7b1b Mon Sep 17 00:00:00 2001 From: Delta Regeer <bertjw@regeer.org> Date: Sat, 26 Oct 2024 22:22:32 -0600 Subject: [PATCH 4/4] Add documentation for channel_request_lookahead --- docs/arguments.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/arguments.rst b/docs/arguments.rst index 0b6ca45..b8a856a 100644 --- a/docs/arguments.rst +++ b/docs/arguments.rst @@ -314,3 +314,17 @@ url_prefix be stripped of the prefix. Default: ``''`` + +channel_request_lookahead + Sets the amount of requests we can continue to read from the socket, while + we are processing current requests. The default value won't allow any + lookahead, increase it above ``0`` to enable. + + When enabled this inserts a callable ``waitress.client_disconnected`` into + the environment that allows the task to check if the client disconnected + while waiting for the response at strategic points in the execution and to + cancel the operation. + + Default: ``0`` + + .. versionadded:: 2.0.0 -- Gitee
Locations
Projects
Search
Status Monitor
Help
Open Build Service
OBS Manuals
API Documentation
OBS Portal
Reporting a Bug
Contact
Mailing List
Forums
Chat (IRC)
Twitter
Open Build Service (OBS)
is an
openSUSE project
.
浙ICP备2022010568号-2