diff --git a/README.rst b/README.rst index 3b03e648351..bbb938fd896 100644 --- a/README.rst +++ b/README.rst @@ -2,9 +2,7 @@ Twisted ======= |pypi|_ -|coverage|_ |travis|_ -|appveyor|_ |circleci|_ For information on what's new in Twisted 19.2.0, see the `NEWS `_ file that comes with the distribution. @@ -41,10 +39,10 @@ Additional instructions for installing this software are in `the installation in Documentation and Support ------------------------- -Twisted's documentation is available from the `Twisted Matrix website `_. +Twisted's documentation is available from the `Twisted Matrix website `_. This documentation contains how-tos, code examples, and an API reference. -Help is also available on the `Twisted mailing list `_. +Help is also available on the `Twisted mailing list `_. There is also a pair of very lively IRC channels, ``#twisted`` (for general Twisted questions) and ``#twisted.web`` (for Twisted Web), on ``chat.freenode.net``. @@ -96,17 +94,11 @@ Warranty Again, see the included `LICENSE `_ file for specific legal details. -.. |coverage| image:: https://codecov.io/github/twisted/twisted/coverage.svg?branch=trunk -.. _coverage: https://codecov.io/github/twisted/twisted - .. |pypi| image:: http://img.shields.io/pypi/v/twisted.svg .. _pypi: https://pypi.python.org/pypi/twisted .. |travis| image:: https://travis-ci.org/twisted/twisted.svg?branch=trunk .. _travis: https://travis-ci.org/twisted/twisted -.. |appveyor| image:: https://ci.appveyor.com/api/projects/status/x4oyqtl9cqc2i2l8 -.. _appveyor: https://ci.appveyor.com/project/adiroiban/twisted - .. |circleci| image:: https://circleci.com/gh/twisted/twisted.svg?style=svg .. _circleci: https://circleci.com/gh/twisted/twisted diff --git a/src/twisted/newsfragments/9643.bugfix b/src/twisted/newsfragments/9643.bugfix new file mode 100644 index 00000000000..05740570568 --- /dev/null +++ b/src/twisted/newsfragments/9643.bugfix @@ -0,0 +1 @@ +twisted.web.client.Agent.request() and twisted.web.client.ProxyAgent.request() now produce TypeError when the method argument is not bytes, rather than failing to generate the request. diff --git a/src/twisted/newsfragments/9648.feature b/src/twisted/newsfragments/9648.feature new file mode 100644 index 00000000000..113ddb2cba2 --- /dev/null +++ b/src/twisted/newsfragments/9648.feature @@ -0,0 +1 @@ +The PyPI page for Twisted has been enhanced to include more information and useful links. diff --git a/src/twisted/python/_setup.py b/src/twisted/python/_setup.py index d3e3786d02d..ae7c166656f 100644 --- a/src/twisted/python/_setup.py +++ b/src/twisted/python/_setup.py @@ -33,8 +33,10 @@ @var notPortedModules: Modules that are not yet ported to Python 3. """ +import io import os import platform +import re import sys from distutils.command import build_ext @@ -56,12 +58,13 @@ author_email="twisted-python@twistedmatrix.com", maintainer="Glyph Lefkowitz", maintainer_email="glyph@twistedmatrix.com", - url="http://twistedmatrix.com/", + url="https://twistedmatrix.com/", + project_urls={ + 'Documentation': 'https://twistedmatrix.com/documents/current/', + 'Source': 'https://github.com/twisted/twisted', + 'Issues': 'https://twistedmatrix.com/trac/report', + }, license="MIT", - long_description="""\ -An extensible framework for Python programming, with special focus -on event-based network programming and multiprotocol integration. -""", classifiers=[ "Programming Language :: Python :: 2.7", "Programming Language :: Python :: 3", @@ -206,15 +209,56 @@ def _checkPythonVersion(): -def getSetupArgs(extensions=_EXTENSIONS): +def _longDescriptionArgsFromReadme(readme): """ + Generate a PyPI long description from the readme. - @return: The keyword arguments to be used the the setup method. + @param readme: Path to the readme reStructuredText file. + @type readme: C{str} + + @return: Keyword arguments to be passed to C{setuptools.setup()}. + @rtype: C{str} + """ + with io.open(readme, encoding='utf-8') as f: + readmeRst = f.read() + + # Munge links of the form `NEWS `_ to point at the appropriate + # location on GitHub so that they function when the long description is + # displayed on PyPI. + longDesc = re.sub( + r'`([^`]+)\s+<(?!https?://)([^>]+)>`_', + r'`\1 `_', + readmeRst, + flags=re.I, + ) + + return { + 'long_description': longDesc, + 'long_description_content_type': 'text/x-rst', + } + + + +def getSetupArgs(extensions=_EXTENSIONS, readme='README.rst'): + """ + Generate arguments for C{setuptools.setup()} + + @param extensions: C extension modules to maybe build. This argument is to + be used for testing. + @type extensions: C{list} of C{ConditionalExtension} + + @param readme: Path to the readme reStructuredText file. This argument is + to be used for testing. + @type readme: C{str} + + @return: The keyword arguments to be used by the setup method. @rtype: L{dict} """ _checkPythonVersion() arguments = STATIC_PACKAGE_METADATA.copy() + if readme: + arguments.update(_longDescriptionArgsFromReadme(readme)) # This is a workaround for distutils behavior; ext_modules isn't # actually used by our custom builder. distutils deep-down checks @@ -307,7 +351,7 @@ def prepare_extensions(self): # _XOPEN_SOURCE_EXTENDED macros to build in order to gain access to # the msg_control, msg_controllen, and msg_flags members in # sendmsg.c. (according to - # http://stackoverflow.com/questions/1034587). See the documentation + # https://stackoverflow.com/questions/1034587). See the documentation # of X/Open CAE in the standards(5) man page of Solaris. if sys.platform.startswith('sunos'): self.define_macros.append(('_XOPEN_SOURCE', 1)) diff --git a/src/twisted/python/test/test_setup.py b/src/twisted/python/test/test_setup.py index 4fa01426cbc..0cad6f87288 100644 --- a/src/twisted/python/test/test_setup.py +++ b/src/twisted/python/test/test_setup.py @@ -8,24 +8,24 @@ import os - from pkg_resources import parse_requirements from setuptools.dist import Distribution import twisted -from twisted.trial.unittest import TestCase +from twisted.trial.unittest import SynchronousTestCase from twisted.python import _setup, filepath from twisted.python.compat import _PY3 from twisted.python._setup import ( BuildPy3, getSetupArgs, + _longDescriptionArgsFromReadme, ConditionalExtension, _EXTRAS_REQUIRE, ) -class SetupTests(TestCase): +class SetupTests(SynchronousTestCase): """ Tests for L{getSetupArgs}. """ @@ -38,9 +38,9 @@ def test_conditionalExtensions(self): good_ext = ConditionalExtension("whatever", ["whatever.c"], condition=lambda b: True) bad_ext = ConditionalExtension("whatever", ["whatever.c"], - condition=lambda b: False) + condition=lambda b: False) - args = getSetupArgs(extensions=[good_ext, bad_ext]) + args = getSetupArgs(extensions=[good_ext, bad_ext], readme=None) # ext_modules should be set even though it's not used. See comment # in getSetupArgs @@ -60,7 +60,7 @@ def test_win32Definition(self): ext = ConditionalExtension("whatever", ["whatever.c"], define_macros=[("whatever", 2)]) - args = getSetupArgs(extensions=[ext]) + args = getSetupArgs(extensions=[ext], readme=None) builder = args["cmdclass"]["build_ext"](Distribution()) self.patch(os, "name", "nt") @@ -69,7 +69,7 @@ def test_win32Definition(self): -class OptionalDependenciesTests(TestCase): +class OptionalDependenciesTests(SynchronousTestCase): """ Tests for L{_EXTRAS_REQUIRE} """ @@ -295,7 +295,7 @@ def __getattr__(self, name): -class WithPlatformTests(TestCase): +class WithPlatformTests(SynchronousTestCase): """ Tests for L{_checkCPython} when used with a (fake) C{platform} module. """ @@ -316,7 +316,7 @@ def test_other(self): -class BuildPy3Tests(TestCase): +class BuildPy3Tests(SynchronousTestCase): """ Tests for L{BuildPy3}. """ @@ -363,3 +363,39 @@ def test_find_package_modules(self): ]), sorted(result), ) + + + +class LongDescriptionTests(SynchronousTestCase): + """ + Tests for C{_getLongDescriptionArgs()} + + Note that the validity of the reStructuredText syntax is tested separately + using L{twine check} in L{tox.ini}. + """ + def test_generate(self): + """ + L{_longDescriptionArgsFromReadme()} outputs a L{long_description} in + reStructuredText format. Local links are transformed into absolute ones + that point at the Twisted GitHub repository. + """ + path = self.mktemp() + with open(path, 'w') as f: + f.write('\n'.join([ + 'Twisted', + '=======', + '', + 'Changes: `NEWS `_.', + "Read `the docs `_.\n", + ])) + + self.assertEqual({ + 'long_description': '''\ +Twisted +======= + +Changes: `NEWS `_. +Read `the docs `_. +''', + 'long_description_content_type': 'text/x-rst', + }, _longDescriptionArgsFromReadme(path)) diff --git a/src/twisted/web/_newclient.py b/src/twisted/web/_newclient.py index 370f47d574d..74a8a6c2d1f 100644 --- a/src/twisted/web/_newclient.py +++ b/src/twisted/web/_newclient.py @@ -29,6 +29,8 @@ from __future__ import division, absolute_import __metaclass__ = type +import re + from zope.interface import implementer from twisted.python.compat import networkString @@ -579,6 +581,74 @@ def connectionLost(self, reason): +_VALID_METHOD = re.compile( + br"\A[%s]+\Z" % ( + bytes().join( + ( + b"!", b"#", b"$", b"%", b"&", b"'", b"*", + b"+", b"-", b".", b"^", b"_", b"`", b"|", b"~", + b"\x30-\x39", + b"\x41-\x5a", + b"\x61-\x7A", + ), + ), + ), +) + + + +def _ensureValidMethod(method): + """ + An HTTP method is an HTTP token, which consists of any visible + ASCII character that is not a delimiter (i.e. one of + C{"(),/:;<=>?@[\\]{}}.) + + @param method: the method to check + @type method: L{bytes} + + @return: the method if it is valid + @rtype: L{bytes} + + @raise ValueError: if the method is not valid + + @see: U{https://tools.ietf.org/html/rfc7230#section-3.1.1}, + U{https://tools.ietf.org/html/rfc7230#section-3.2.6}, + U{https://tools.ietf.org/html/rfc5234#appendix-B.1} + """ + if _VALID_METHOD.match(method): + return method + raise ValueError("Invalid method {!r}".format(method)) + + + +_VALID_URI = re.compile(br'\A[\x21-\x7e]+\Z') + + + +def _ensureValidURI(uri): + """ + A valid URI cannot contain control characters (i.e., characters + between 0-32, inclusive and 127) or non-ASCII characters (i.e., + characters with values between 128-255, inclusive). + + @param uri: the URI to check + @type uri: L{bytes} + + @return: the URI if it is valid + @rtype: L{bytes} + + @raise ValueError: if the URI is not valid + + @see: U{https://tools.ietf.org/html/rfc3986#section-3.3}, + U{https://tools.ietf.org/html/rfc3986#appendix-A}, + U{https://tools.ietf.org/html/rfc5234#appendix-B.1} + """ + if _VALID_URI.match(uri): + return uri + raise ValueError("Invalid URI {!r}".format(uri)) + + + @implementer(IClientRequest) class Request: """ @@ -618,8 +688,8 @@ def __init__(self, method, uri, headers, bodyProducer, persistent=False): connection, defaults to C{False}. @type persistent: L{bool} """ - self.method = method - self.uri = uri + self.method = _ensureValidMethod(method) + self.uri = _ensureValidURI(uri) self.headers = headers self.bodyProducer = bodyProducer self.persistent = persistent @@ -664,8 +734,15 @@ def _writeHeaders(self, transport, TEorCL): # method would probably be good. It would be nice if this method # weren't limited to issuing HTTP/1.1 requests. requestLines = [] - requestLines.append(b' '.join([self.method, self.uri, - b'HTTP/1.1\r\n'])) + requestLines.append( + b' '.join( + [ + _ensureValidMethod(self.method), + _ensureValidURI(self.uri), + b'HTTP/1.1\r\n', + ] + ), + ) if not self.persistent: requestLines.append(b'Connection: close\r\n') if TEorCL is not None: diff --git a/src/twisted/web/client.py b/src/twisted/web/client.py index e65ac9ad259..340906f6215 100644 --- a/src/twisted/web/client.py +++ b/src/twisted/web/client.py @@ -47,6 +47,9 @@ def urlunparse(parts): from twisted.web.http_headers import Headers from twisted.logger import Logger +from twisted.web._newclient import _ensureValidURI, _ensureValidMethod + + class PartialDownloadError(error.Error): """ @@ -78,11 +81,13 @@ class HTTPPageGetter(http.HTTPClient): _completelyDone = True - _specialHeaders = set((b'host', b'user-agent', b'cookie', b'content-length')) + _specialHeaders = set( + (b'host', b'user-agent', b'cookie', b'content-length'), + ) def connectionMade(self): - method = getattr(self.factory, 'method', b'GET') - self.sendCommand(method, self.factory.path) + method = _ensureValidMethod(getattr(self.factory, 'method', b'GET')) + self.sendCommand(method, _ensureValidURI(self.factory.path)) if self.factory.scheme == b'http' and self.factory.port != 80: host = self.factory.host + b':' + intToBytes(self.factory.port) elif self.factory.scheme == b'https' and self.factory.port != 443: @@ -362,7 +367,7 @@ def __init__(self, url, method=b'GET', postdata=None, headers=None, # just in case a broken http/1.1 decides to keep connection alive self.headers.setdefault(b"connection", b"close") self.postdata = postdata - self.method = method + self.method = _ensureValidMethod(method) self.setURL(url) @@ -389,6 +394,7 @@ def __repr__(self): return "<%s: %s>" % (self.__class__.__name__, self.url) def setURL(self, url): + _ensureValidURI(url.strip()) self.url = url uri = URI.fromBytes(url) if uri.scheme and uri.host: @@ -733,7 +739,7 @@ def _makeGetterFactory(url, factoryFactory, contextFactory=None, @return: The factory created by C{factoryFactory} """ - uri = URI.fromBytes(url) + uri = URI.fromBytes(_ensureValidURI(url.strip())) factory = factoryFactory(url, *args, **kwargs) if uri.scheme == b'https': from twisted.internet import ssl @@ -1490,6 +1496,12 @@ def _requestWithEndpoint(self, key, endpoint, method, parsedURI, Issue a new request, given the endpoint and the path sent as part of the request. """ + if not isinstance(method, bytes): + raise TypeError('method={!r} is {}, but must be bytes'.format( + method, type(method))) + + method = _ensureValidMethod(method) + # Create minimal headers, if necessary: if headers is None: headers = Headers() @@ -1714,6 +1726,7 @@ def request(self, method, uri, headers=None, bodyProducer=None): @see: L{twisted.web.iweb.IAgent.request} """ + uri = _ensureValidURI(uri.strip()) parsedURI = URI.fromBytes(uri) try: endpoint = self._getEndpoint(parsedURI) @@ -1747,6 +1760,8 @@ def request(self, method, uri, headers=None, bodyProducer=None): """ Issue a new request via the configured proxy. """ + uri = _ensureValidURI(uri.strip()) + # Cache *all* connections under the same key, since we are only # connecting to a single destination, the proxy: key = ("http-proxy", self._proxyEndpoint) diff --git a/src/twisted/web/newsfragments/9647.bugfix b/src/twisted/web/newsfragments/9647.bugfix new file mode 100644 index 00000000000..b76916cdc9e --- /dev/null +++ b/src/twisted/web/newsfragments/9647.bugfix @@ -0,0 +1 @@ +All HTTP clients in twisted.web.client now raise a ValueError when called with a method and/or URL that contain invalid characters. This mitigates CVE-2019-12387. Thanks to Alex Brasetvik for reporting this vulnerability. \ No newline at end of file diff --git a/src/twisted/web/test/injectionhelpers.py b/src/twisted/web/test/injectionhelpers.py new file mode 100644 index 00000000000..ffeb8629c42 --- /dev/null +++ b/src/twisted/web/test/injectionhelpers.py @@ -0,0 +1,168 @@ +""" +Helpers for URI and method injection tests. + +@see: U{CVE-2019-12387} +""" + +import string + + +UNPRINTABLE_ASCII = ( + frozenset(range(0, 128)) - + frozenset(bytearray(string.printable, 'ascii')) +) + +NONASCII = frozenset(range(128, 256)) + + + +class MethodInjectionTestsMixin(object): + """ + A mixin that runs HTTP method injection tests. Define + L{MethodInjectionTestsMixin.attemptRequestWithMaliciousMethod} in + a L{twisted.trial.unittest.SynchronousTestCase} subclass to test + how HTTP client code behaves when presented with malicious HTTP + methods. + + @see: U{CVE-2019-12387} + """ + + def attemptRequestWithMaliciousMethod(self, method): + """ + Attempt to send a request with the given method. This should + synchronously raise a L{ValueError} if either is invalid. + + @param method: the method (e.g. C{GET\x00}) + + @param uri: the URI + + @type method: + """ + raise NotImplementedError() + + + def test_methodWithCLRFRejected(self): + """ + Issuing a request with a method that contains a carriage + return and line feed fails with a L{ValueError}. + """ + with self.assertRaises(ValueError) as cm: + method = b"GET\r\nX-Injected-Header: value" + self.attemptRequestWithMaliciousMethod(method) + self.assertRegex(str(cm.exception), "^Invalid method") + + + def test_methodWithUnprintableASCIIRejected(self): + """ + Issuing a request with a method that contains unprintable + ASCII characters fails with a L{ValueError}. + """ + for c in UNPRINTABLE_ASCII: + method = b"GET%s" % (bytearray([c]),) + with self.assertRaises(ValueError) as cm: + self.attemptRequestWithMaliciousMethod(method) + self.assertRegex(str(cm.exception), "^Invalid method") + + + def test_methodWithNonASCIIRejected(self): + """ + Issuing a request with a method that contains non-ASCII + characters fails with a L{ValueError}. + """ + for c in NONASCII: + method = b"GET%s" % (bytearray([c]),) + with self.assertRaises(ValueError) as cm: + self.attemptRequestWithMaliciousMethod(method) + self.assertRegex(str(cm.exception), "^Invalid method") + + + +class URIInjectionTestsMixin(object): + """ + A mixin that runs HTTP URI injection tests. Define + L{MethodInjectionTestsMixin.attemptRequestWithMaliciousURI} in a + L{twisted.trial.unittest.SynchronousTestCase} subclass to test how + HTTP client code behaves when presented with malicious HTTP + URIs. + """ + + def attemptRequestWithMaliciousURI(self, method): + """ + Attempt to send a request with the given URI. This should + synchronously raise a L{ValueError} if either is invalid. + + @param uri: the URI. + + @type method: + """ + raise NotImplementedError() + + + def test_hostWithCRLFRejected(self): + """ + Issuing a request with a URI whose host contains a carriage + return and line feed fails with a L{ValueError}. + """ + with self.assertRaises(ValueError) as cm: + uri = b"http://twisted\r\n.invalid/path" + self.attemptRequestWithMaliciousURI(uri) + self.assertRegex(str(cm.exception), "^Invalid URI") + + + def test_hostWithWithUnprintableASCIIRejected(self): + """ + Issuing a request with a URI whose host contains unprintable + ASCII characters fails with a L{ValueError}. + """ + for c in UNPRINTABLE_ASCII: + uri = b"http://twisted%s.invalid/OK" % (bytearray([c]),) + with self.assertRaises(ValueError) as cm: + self.attemptRequestWithMaliciousURI(uri) + self.assertRegex(str(cm.exception), "^Invalid URI") + + + def test_hostWithNonASCIIRejected(self): + """ + Issuing a request with a URI whose host contains non-ASCII + characters fails with a L{ValueError}. + """ + for c in NONASCII: + uri = b"http://twisted%s.invalid/OK" % (bytearray([c]),) + with self.assertRaises(ValueError) as cm: + self.attemptRequestWithMaliciousURI(uri) + self.assertRegex(str(cm.exception), "^Invalid URI") + + + def test_pathWithCRLFRejected(self): + """ + Issuing a request with a URI whose path contains a carriage + return and line feed fails with a L{ValueError}. + """ + with self.assertRaises(ValueError) as cm: + uri = b"http://twisted.invalid/\r\npath" + self.attemptRequestWithMaliciousURI(uri) + self.assertRegex(str(cm.exception), "^Invalid URI") + + + def test_pathWithWithUnprintableASCIIRejected(self): + """ + Issuing a request with a URI whose path contains unprintable + ASCII characters fails with a L{ValueError}. + """ + for c in UNPRINTABLE_ASCII: + uri = b"http://twisted.invalid/OK%s" % (bytearray([c]),) + with self.assertRaises(ValueError) as cm: + self.attemptRequestWithMaliciousURI(uri) + self.assertRegex(str(cm.exception), "^Invalid URI") + + + def test_pathWithNonASCIIRejected(self): + """ + Issuing a request with a URI whose path contains non-ASCII + characters fails with a L{ValueError}. + """ + for c in NONASCII: + uri = b"http://twisted.invalid/OK%s" % (bytearray([c]),) + with self.assertRaises(ValueError) as cm: + self.attemptRequestWithMaliciousURI(uri) + self.assertRegex(str(cm.exception), "^Invalid URI") diff --git a/src/twisted/web/test/test_agent.py b/src/twisted/web/test/test_agent.py index afc63044bdf..040dc9ef340 100644 --- a/src/twisted/web/test/test_agent.py +++ b/src/twisted/web/test/test_agent.py @@ -11,7 +11,7 @@ from zope.interface.verify import verifyObject -from twisted.trial.unittest import TestCase +from twisted.trial.unittest import TestCase, SynchronousTestCase from twisted.web import client, error, http_headers from twisted.web._newclient import RequestNotSent, RequestTransmissionFailed from twisted.web._newclient import ResponseNeverReceived, ResponseFailed @@ -51,6 +51,10 @@ from twisted.test.proto_helpers import AccumulatingProtocol from twisted.test.iosim import IOPump, FakeTransport from twisted.test.test_sslverify import certificatesForAuthorityAndServer +from twisted.web.test.injectionhelpers import ( + MethodInjectionTestsMixin, + URIInjectionTestsMixin, +) from twisted.web.error import SchemeNotSupported from twisted.logger import globalLogPublisher @@ -897,6 +901,7 @@ class AgentTests(TestCase, FakeReactorAndConnectMixin, AgentTestsMixin, """ Tests for the new HTTP client API provided by L{Agent}. """ + def makeAgent(self): """ @return: a new L{twisted.web.client.Agent} instance @@ -989,6 +994,15 @@ def getConnection(this, key, ep): self.assertEqual(agent._pool.connected, True) + def test_nonBytesMethod(self): + """ + L{Agent.request} raises L{TypeError} when the C{method} argument isn't + L{bytes}. + """ + self.assertRaises(TypeError, self.agent.request, + u'GET', b'http://foo.example/') + + def test_unsupportedScheme(self): """ L{Agent.request} returns a L{Deferred} which fails with @@ -1318,6 +1332,48 @@ def test_endpointFactoryPool(self): +class AgentMethodInjectionTests( + FakeReactorAndConnectMixin, + MethodInjectionTestsMixin, + SynchronousTestCase, +): + """ + Test L{client.Agent} against HTTP method injections. + """ + + def attemptRequestWithMaliciousMethod(self, method): + """ + Attempt a request with the provided method. + + @param method: see L{MethodInjectionTestsMixin} + """ + agent = client.Agent(self.createReactor()) + uri = b"http://twisted.invalid" + agent.request(method, uri, client.Headers(), None) + + + +class AgentURIInjectionTests( + FakeReactorAndConnectMixin, + URIInjectionTestsMixin, + SynchronousTestCase, +): + """ + Test L{client.Agent} against URI injections. + """ + + def attemptRequestWithMaliciousURI(self, uri): + """ + Attempt a request with the provided method. + + @param uri: see L{URIInjectionTestsMixin} + """ + agent = client.Agent(self.createReactor()) + method = b"GET" + agent.request(method, uri, client.Headers(), None) + + + class AgentHTTPSTests(TestCase, FakeReactorAndConnectMixin, IntegrationTestingMixin): """ @@ -2475,6 +2531,15 @@ def setUp(self): self.agent._proxyEndpoint = self.StubEndpoint(oldEndpoint, self) + def test_nonBytesMethod(self): + """ + L{ProxyAgent.request} raises L{TypeError} when the C{method} argument + isn't L{bytes}. + """ + self.assertRaises(TypeError, self.agent.request, + u'GET', b'http://foo.example/') + + def test_proxyRequest(self): """ L{client.ProxyAgent} issues an HTTP request against the proxy, with the @@ -3184,3 +3249,101 @@ def test_changeCacheSize(self): self.assertEquals(5, len(policy._cache)) self.assertIn(hostn, policy._cache) + + + +class RequestMethodInjectionTests( + MethodInjectionTestsMixin, + SynchronousTestCase, +): + """ + Test L{client.Request} against HTTP method injections. + """ + + def attemptRequestWithMaliciousMethod(self, method): + """ + Attempt a request with the provided method. + + @param method: see L{MethodInjectionTestsMixin} + """ + client.Request( + method=method, + uri=b"http://twisted.invalid", + headers=http_headers.Headers(), + bodyProducer=None, + ) + + + +class RequestWriteToMethodInjectionTests( + MethodInjectionTestsMixin, + SynchronousTestCase, +): + """ + Test L{client.Request.writeTo} against HTTP method injections. + """ + + def attemptRequestWithMaliciousMethod(self, method): + """ + Attempt a request with the provided method. + + @param method: see L{MethodInjectionTestsMixin} + """ + headers = http_headers.Headers({b"Host": [b"twisted.invalid"]}) + req = client.Request( + method=b"GET", + uri=b"http://twisted.invalid", + headers=headers, + bodyProducer=None, + ) + req.method = method + req.writeTo(StringTransport()) + + + +class RequestURIInjectionTests( + URIInjectionTestsMixin, + SynchronousTestCase, +): + """ + Test L{client.Request} against HTTP URI injections. + """ + + def attemptRequestWithMaliciousURI(self, uri): + """ + Attempt a request with the provided URI. + + @param method: see L{URIInjectionTestsMixin} + """ + client.Request( + method=b"GET", + uri=uri, + headers=http_headers.Headers(), + bodyProducer=None, + ) + + + +class RequestWriteToURIInjectionTests( + URIInjectionTestsMixin, + SynchronousTestCase, +): + """ + Test L{client.Request.writeTo} against HTTP method injections. + """ + + def attemptRequestWithMaliciousURI(self, uri): + """ + Attempt a request with the provided method. + + @param method: see L{URIInjectionTestsMixin} + """ + headers = http_headers.Headers({b"Host": [b"twisted.invalid"]}) + req = client.Request( + method=b"GET", + uri=b"http://twisted.invalid", + headers=headers, + bodyProducer=None, + ) + req.uri = uri + req.writeTo(StringTransport()) diff --git a/src/twisted/web/test/test_webclient.py b/src/twisted/web/test/test_webclient.py index 41cff5407a0..680e02780d6 100644 --- a/src/twisted/web/test/test_webclient.py +++ b/src/twisted/web/test/test_webclient.py @@ -7,6 +7,7 @@ from __future__ import division, absolute_import +import io import os from errno import ENOSPC @@ -20,7 +21,8 @@ from twisted.web import server, client, error, resource from twisted.web.static import Data from twisted.web.util import Redirect -from twisted.internet import reactor, defer, interfaces +from twisted.internet import address, reactor, defer, interfaces +from twisted.internet.protocol import ClientFactory from twisted.python.filepath import FilePath from twisted.protocols.policies import WrappingFactory from twisted.test.proto_helpers import ( @@ -35,6 +37,12 @@ from twisted.logger import (globalLogPublisher, FilteringLogObserver, LogLevelFilterPredicate, LogLevel, Logger) +from twisted.web.test.injectionhelpers import ( + MethodInjectionTestsMixin, + URIInjectionTestsMixin, +) + + serverPEM = FilePath(test.__file__).sibling('server.pem') serverPEMPath = serverPEM.asBytesMode().path @@ -1519,3 +1527,306 @@ def test_httpDownloaderDeprecated(self): L{client.HTTPDownloader} is deprecated. """ self._testDeprecatedClass("HTTPDownloader") + + + +class GetPageMethodInjectionTests( + MethodInjectionTestsMixin, + unittest.SynchronousTestCase, +): + """ + Test L{client.getPage} against HTTP method injections. + """ + + def attemptRequestWithMaliciousMethod(self, method): + """ + Attempt a request with the provided method. + + @param method: see L{MethodInjectionTestsMixin} + """ + uri = b'http://twisted.invalid' + client.getPage(uri, method=method) + + + +class GetPageURIInjectionTests( + URIInjectionTestsMixin, + unittest.SynchronousTestCase, +): + """ + Test L{client.getPage} against URI injections. + """ + + def attemptRequestWithMaliciousURI(self, uri): + """ + Attempt a request with the provided URI. + + @param uri: see L{URIInjectionTestsMixin} + """ + client.getPage(uri) + + + +class DownloadPageMethodInjectionTests( + MethodInjectionTestsMixin, + unittest.SynchronousTestCase, +): + """ + Test L{client.getPage} against HTTP method injections. + """ + + def attemptRequestWithMaliciousMethod(self, method): + """ + Attempt a request with the provided method. + + @param method: see L{MethodInjectionTestsMixin} + """ + uri = b'http://twisted.invalid' + client.downloadPage(uri, file=io.BytesIO(), method=method) + + + +class DownloadPageURIInjectionTests( + URIInjectionTestsMixin, + unittest.SynchronousTestCase, +): + """ + Test L{client.downloadPage} against URI injections. + """ + + def attemptRequestWithMaliciousURI(self, uri): + """ + Attempt a request with the provided URI. + + @param uri: see L{URIInjectionTestsMixin} + """ + client.downloadPage(uri, file=io.BytesIO()) + + + +def makeHTTPPageGetterFactory(protocolClass, method, host, path): + """ + Make a L{ClientFactory} that can be used with + L{client.HTTPPageGetter} and its subclasses. + + @param protocolClass: The protocol class + @type protocolClass: A subclass of L{client.HTTPPageGetter} + + @param method: the HTTP method + + @param host: the host + + @param path: The URI path + + @return: A L{ClientFactory}. + """ + factory = ClientFactory.forProtocol(protocolClass) + + factory.method = method + factory.host = host + factory.path = path + + factory.scheme = b"http" + factory.port = 0 + factory.headers = {} + factory.agent = b"User/Agent" + factory.cookies = {} + + return factory + + + +class HTTPPageGetterMethodInjectionTests( + MethodInjectionTestsMixin, + unittest.SynchronousTestCase, +): + """ + Test L{client.HTTPPageGetter} against HTTP method injections. + """ + protocolClass = client.HTTPPageGetter + + def attemptRequestWithMaliciousMethod(self, method): + """ + Attempt a request with the provided method. + + @param method: L{MethodInjectionTestsMixin} + """ + transport = StringTransport() + factory = makeHTTPPageGetterFactory( + self.protocolClass, + method=method, + host=b"twisted.invalid", + path=b"/", + ) + getter = factory.buildProtocol( + address.IPv4Address("TCP", "127.0.0.1", 0), + ) + getter.makeConnection(transport) + + + +class HTTPPageGetterURIInjectionTests( + URIInjectionTestsMixin, + unittest.SynchronousTestCase, +): + """ + Test L{client.HTTPPageGetter} against HTTP URI injections. + """ + protocolClass = client.HTTPPageGetter + + def attemptRequestWithMaliciousURI(self, uri): + """ + Attempt a request with the provided URI. + + @param uri: L{URIInjectionTestsMixin} + """ + transport = StringTransport() + # Setting the host and path to the same value is imprecise but + # doesn't require parsing an invalid URI. + factory = makeHTTPPageGetterFactory( + self.protocolClass, + method=b"GET", + host=uri, + path=uri, + ) + getter = factory.buildProtocol( + address.IPv4Address("TCP", "127.0.0.1", 0), + ) + getter.makeConnection(transport) + + + +class HTTPPageDownloaderMethodInjectionTests( + HTTPPageGetterMethodInjectionTests +): + + """ + Test L{client.HTTPPageDownloader} against HTTP method injections. + """ + protocolClass = client.HTTPPageDownloader + + + +class HTTPPageDownloaderURIInjectionTests( + HTTPPageGetterURIInjectionTests +): + """ + Test L{client.HTTPPageDownloader} against HTTP URI injections. + """ + protocolClass = client.HTTPPageDownloader + + + +class HTTPClientFactoryMethodInjectionTests( + MethodInjectionTestsMixin, + unittest.SynchronousTestCase, +): + """ + Tests L{client.HTTPClientFactory} against HTTP method injections. + """ + + def attemptRequestWithMaliciousMethod(self, method): + """ + Attempt a request with the provided method. + + @param method: L{MethodInjectionTestsMixin} + """ + client.HTTPClientFactory(b"https://twisted.invalid", method) + + + +class HTTPClientFactoryURIInjectionTests( + URIInjectionTestsMixin, + unittest.SynchronousTestCase, +): + """ + Tests L{client.HTTPClientFactory} against HTTP URI injections. + """ + + def attemptRequestWithMaliciousURI(self, uri): + """ + Attempt a request with the provided URI. + + @param uri: L{URIInjectionTestsMixin} + """ + client.HTTPClientFactory(uri) + + + +class HTTPClientFactorySetURLURIInjectionTests( + URIInjectionTestsMixin, + unittest.SynchronousTestCase, +): + """ + Tests L{client.HTTPClientFactory.setURL} against HTTP URI injections. + """ + + def attemptRequestWithMaliciousURI(self, uri): + """ + Attempt a request with the provided URI. + + @param uri: L{URIInjectionTestsMixin} + """ + client.HTTPClientFactory(b"https://twisted.invalid").setURL(uri) + + + +class HTTPDownloaderMethodInjectionTests( + MethodInjectionTestsMixin, + unittest.SynchronousTestCase, +): + """ + Tests L{client.HTTPDownloader} against HTTP method injections. + """ + + def attemptRequestWithMaliciousMethod(self, method): + """ + Attempt a request with the provided method. + + @param method: L{MethodInjectionTestsMixin} + """ + client.HTTPDownloader( + b"https://twisted.invalid", + io.BytesIO(), + method=method, + ) + + + +class HTTPDownloaderURIInjectionTests( + URIInjectionTestsMixin, + unittest.SynchronousTestCase, +): + """ + Tests L{client.HTTPDownloader} against HTTP URI injections. + """ + + def attemptRequestWithMaliciousURI(self, uri): + """ + Attempt a request with the provided URI. + + @param uri: L{URIInjectionTestsMixin} + """ + client.HTTPDownloader(uri, io.BytesIO()) + + + +class HTTPDownloaderSetURLURIInjectionTests( + URIInjectionTestsMixin, + unittest.SynchronousTestCase, +): + """ + Tests L{client.HTTPDownloader.setURL} against HTTP URI injections. + """ + + def attemptRequestWithMaliciousURI(self, uri): + """ + Attempt a request with the provided URI. + + @param uri: L{URIInjectionTestsMixin} + """ + downloader = client.HTTPDownloader( + b"https://twisted.invalid", + io.BytesIO(), + ) + downloader.setURL(uri) diff --git a/tox.ini b/tox.ini index 428005eca82..2a89cce4caf 100644 --- a/tox.ini +++ b/tox.ini @@ -28,7 +28,7 @@ minversion=2.4 skip_missing_interpreters=True toxworkdir=build/ -envlist=lint,pyflakes,apidocs,narrativedocs,newsfragment,manifest-checker,py27-alldeps-nocov,py36-alldeps-nocov +envlist=lint,pyflakes,apidocs,narrativedocs,newsfragment,manifest-checker,twine,py27-alldeps-nocov,py36-alldeps-nocov [testenv] ;; dependencies managed by extras in t.p._setup.py._EXTRAS_REQUIRE @@ -60,6 +60,8 @@ deps = ; Code quality checkers manifest-checker: check-manifest + twine: twine + lint: pyflakes lint: twistedchecker>=0.7.1 lint: diff-cover==0.9.12 @@ -131,6 +133,8 @@ commands = manifest-checker: check-manifest --ignore "docs/_build*,docs/historic/*,admin*,bin/admin*,twisted/topfiles/*.Old" + twine: twine check {distdir}/*.* + wheels: cibuildwheel --output-dir {toxinidir}/wheelhouse {toxinidir} wheels: python setup.py sdist --formats=gztar,zip --dist-dir={toxinidir}/wheelhouse wheels: ls -l {toxinidir}/wheelhouse