python-Twisted/CVE-2023-46137-HTTP-pipeline-response.patch

175 lines
6.8 KiB
Diff

---
src/twisted/web/http.py | 32 +++++++++--
src/twisted/web/newsfragments/11976.bugfix | 7 ++
src/twisted/web/test/test_web.py | 81 ++++++++++++++++++++++++++++-
3 files changed, 114 insertions(+), 6 deletions(-)
--- a/src/twisted/web/http.py
+++ b/src/twisted/web/http.py
@@ -2419,14 +2419,38 @@ class HTTPChannel(basic.LineReceiver, po
self._handlingRequest = True
+ # We go into raw mode here even though we will be receiving lines next
+ # in the protocol; however, this data will be buffered and then passed
+ # back to line mode in the setLineMode call in requestDone.
+ self.setRawMode()
+
req = self.requests[-1]
req.requestReceived(command, path, version)
- def dataReceived(self, data):
+ def rawDataReceived(self, data: bytes) -> None:
"""
- Data was received from the network. Process it.
+ This is called when this HTTP/1.1 parser is in raw mode rather than
+ line mode.
+
+ It may be in raw mode for one of two reasons:
+
+ 1. All the headers of a request have been received and this
+ L{HTTPChannel} is currently receiving its body.
+
+ 2. The full content of a request has been received and is currently
+ being processed asynchronously, and this L{HTTPChannel} is
+ buffering the data of all subsequent requests to be parsed
+ later.
+
+ In the second state, the data will be played back later.
+
+ @note: This isn't really a public API, and should be invoked only by
+ L{LineReceiver}'s line parsing logic. If you wish to drive an
+ L{HTTPChannel} from a custom data source, call C{dataReceived} on
+ it directly.
+
+ @see: L{LineReceive.rawDataReceived}
"""
- # If we're currently handling a request, buffer this data.
if self._handlingRequest:
self._dataBuffer.append(data)
if (
@@ -2438,9 +2462,7 @@ class HTTPChannel(basic.LineReceiver, po
# ready. See docstring for _optimisticEagerReadSize above.
self._networkProducer.pauseProducing()
return
- return basic.LineReceiver.dataReceived(self, data)
- def rawDataReceived(self, data):
self.resetTimeout()
try:
--- /dev/null
+++ b/src/twisted/web/newsfragments/11976.bugfix
@@ -0,0 +1,7 @@
+In Twisted 16.3.0, we changed twisted.web to stop dispatching HTTP/1.1
+pipelined requests to application code. There was a bug in this change which
+still allowed clients which could send multiple full HTTP requests in a single
+TCP segment to trigger asynchronous processing of later requests, which could
+lead to out-of-order responses. This has now been corrected and twisted.web
+should never process a pipelined request over HTTP/1.1 until the previous
+request has fully completed.
--- a/src/twisted/web/test/test_web.py
+++ b/src/twisted/web/test/test_web.py
@@ -8,6 +8,7 @@ Tests for various parts of L{twisted.web
import os
import zlib
from io import BytesIO
+from typing import List
from zope.interface import implementer
from zope.interface.verify import verifyObject
@@ -17,10 +18,13 @@ from twisted.internet.address import IPv
from twisted.internet.task import Clock
from twisted.logger import LogLevel, globalLogPublisher
from twisted.python import failure, reflect
+from twisted.python.compat import iterbytes
from twisted.python.filepath import FilePath
-from twisted.test.proto_helpers import EventLoggingObserver
+from twisted.test.proto_helpers import EventLoggingObserver, StringTransport
from twisted.trial import unittest
from twisted.web import error, http, iweb, resource, server
+from twisted.web.resource import Resource
+from twisted.web.server import NOT_DONE_YET, Request, Site
from twisted.web.static import Data
from twisted.web.test.requesthelper import DummyChannel, DummyRequest
from ._util import assertIsFilesystemTemporary
@@ -1849,3 +1853,78 @@ class ExplicitHTTPFactoryReactor(unittes
factory = http.HTTPFactory()
self.assertIs(factory.reactor, reactor)
+
+
+class QueueResource(Resource):
+ """
+ Add all requests to an internal queue,
+ without responding to the requests.
+ You can access the requests from the queue and handle their response.
+ """
+
+ isLeaf = True
+
+ def __init__(self) -> None:
+ super().__init__()
+ self.dispatchedRequests: List[Request] = []
+
+ def render_GET(self, request: Request) -> int:
+ self.dispatchedRequests.append(request)
+ return NOT_DONE_YET
+
+
+class TestRFC9112Section932(unittest.TestCase):
+ """
+ Verify that HTTP/1.1 request ordering is preserved.
+ """
+
+ def test_multipleRequestsInOneSegment(self) -> None:
+ """
+ Twisted MUST NOT respond to a second HTTP/1.1 request while the first
+ is still pending.
+ """
+ qr = QueueResource()
+ site = Site(qr)
+ proto = site.buildProtocol(None)
+ serverTransport = StringTransport()
+ proto.makeConnection(serverTransport)
+ proto.dataReceived(
+ b"GET /first HTTP/1.1\r\nHost: a\r\n\r\n"
+ b"GET /second HTTP/1.1\r\nHost: a\r\n\r\n"
+ )
+ # The TCP data contains 2 requests,
+ # but only 1 request was dispatched,
+ # as the first request was not yet finalized.
+ self.assertEqual(len(qr.dispatchedRequests), 1)
+ # The first request is finalized and the
+ # second request is dispatched right away.
+ qr.dispatchedRequests[0].finish()
+ self.assertEqual(len(qr.dispatchedRequests), 2)
+
+ def test_multipleRequestsInDifferentSegments(self) -> None:
+ """
+ Twisted MUST NOT respond to a second HTTP/1.1 request while the first
+ is still pending, even if the second request is received in a separate
+ TCP package.
+ """
+ qr = QueueResource()
+ site = Site(qr)
+ proto = site.buildProtocol(None)
+ serverTransport = StringTransport()
+ proto.makeConnection(serverTransport)
+ raw_data = (
+ b"GET /first HTTP/1.1\r\nHost: a\r\n\r\n"
+ b"GET /second HTTP/1.1\r\nHost: a\r\n\r\n"
+ )
+ # Just go byte by byte for the extreme case in which each byte is
+ # received in a separate TCP package.
+ for chunk in iterbytes(raw_data):
+ proto.dataReceived(chunk)
+ # The TCP data contains 2 requests,
+ # but only 1 request was dispatched,
+ # as the first request was not yet finalized.
+ self.assertEqual(len(qr.dispatchedRequests), 1)
+ # The first request is finalized and the
+ # second request is dispatched right away.
+ qr.dispatchedRequests[0].finish()
+ self.assertEqual(len(qr.dispatchedRequests), 2)