diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index c042d3f43..0cb81c43e 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -442,8 +442,9 @@ or even enable client-side authentication (and various other things). If you do use a custom ContextFactory, make sure its ``__init__`` method accepts a ``method`` parameter (this is the ``OpenSSL.SSL`` method mapping -:setting:`DOWNLOADER_CLIENT_TLS_METHOD`) and a ``tls_verbose_logging`` -parameter (``bool``). +:setting:`DOWNLOADER_CLIENT_TLS_METHOD`), a ``tls_verbose_logging`` +parameter (``bool``) and a ``tls_ciphers`` parameter (see +:setting:`DOWNLOADER_CLIENT_TLS_CIPHERS`). .. setting:: DOWNLOADER_CLIENT_TLS_CIPHERS diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index e72052afc..91b45a8fc 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -54,7 +54,7 @@ class HTTP11DownloadHandler(object): ) msg = """ '%s' does not accept `method` argument (type OpenSSL.SSL method,\ - e.g. OpenSSL.SSL.SSLv23_METHOD) and/or `tls_verbose_logging` argument.\ + e.g. OpenSSL.SSL.SSLv23_METHOD) and/or `tls_verbose_logging` argument and/or `tls_ciphers` argument.\ Please upgrade your context factory class to handle them or ignore them.""" % ( settings['DOWNLOADER_CLIENTCONTEXTFACTORY'],) warnings.warn(msg) diff --git a/scrapy/utils/ssl.py b/scrapy/utils/ssl.py index 632827471..02aed60ee 100644 --- a/scrapy/utils/ssl.py +++ b/scrapy/utils/ssl.py @@ -6,6 +6,11 @@ import OpenSSL._util as pyOpenSSLutil from scrapy.utils.python import to_native_str +# The OpenSSL symbol is present since 1.1.1 but it's not currently supported in any version of pyOpenSSL. +# Using the binding directly, as this code does, requires cryptography 2.4. +SSL_OP_NO_TLSv1_3 = getattr(pyOpenSSLutil.lib, 'SSL_OP_NO_TLSv1_3', 0) + + def ffi_buf_to_string(buf): return to_native_str(pyOpenSSLutil.ffi.string(buf)) diff --git a/tests/mockserver.py b/tests/mockserver.py index 8be8a36bb..77908284b 100644 --- a/tests/mockserver.py +++ b/tests/mockserver.py @@ -4,7 +4,6 @@ from six.moves.urllib.parse import urlencode from subprocess import Popen, PIPE from OpenSSL import SSL - from twisted.web.server import Site, NOT_DONE_YET from twisted.web.resource import Resource from twisted.web.static import File @@ -15,8 +14,8 @@ from twisted.web.util import redirectTo from twisted.internet import reactor, ssl from twisted.internet.task import deferLater - from scrapy.utils.python import to_bytes, to_unicode +from scrapy.utils.ssl import SSL_OP_NO_TLSv1_3 def getarg(request, name, default=None, type=None): @@ -217,18 +216,16 @@ class MockServer(): return host + path -def ssl_context_factory(keyfile='keys/localhost.key', certfile='keys/localhost.crt'): - return ssl.DefaultOpenSSLContextFactory( +def ssl_context_factory(keyfile='keys/localhost.key', certfile='keys/localhost.crt', cipher_string=None): + factory = ssl.DefaultOpenSSLContextFactory( os.path.join(os.path.dirname(__file__), keyfile), os.path.join(os.path.dirname(__file__), certfile), ) - - -def broken_ssl_context_factory(keyfile='keys/localhost.key', certfile='keys/localhost.crt', cipher_string='DEFAULT'): - factory = ssl_context_factory(keyfile, certfile) - ctx = factory.getContext() - ctx.set_options(SSL.OP_CIPHER_SERVER_PREFERENCE | SSL.OP_NO_TLSv1_2) - ctx.set_cipher_list(cipher_string) + if cipher_string: + ctx = factory.getContext() + # disabling TLS1.2+ because it unconditionally enables some strong ciphers + ctx.set_options(SSL.OP_CIPHER_SERVER_PREFERENCE | SSL.OP_NO_TLSv1_2 | SSL_OP_NO_TLSv1_3) + ctx.set_cipher_list(to_bytes(cipher_string)) return factory diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 5df2ffe45..109469503 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -39,7 +39,7 @@ from scrapy.utils.test import get_crawler, skip_if_no_boto from scrapy.utils.python import to_bytes from scrapy.exceptions import NotConfigured -from tests.mockserver import MockServer, ssl_context_factory, Echo, broken_ssl_context_factory +from tests.mockserver import MockServer, ssl_context_factory, Echo from tests.spiders import SingleRequestSpider @@ -553,7 +553,7 @@ class Https11InvalidDNSPattern(Https11TestCase): super(Https11InvalidDNSPattern, self).setUp() -class Https11BadCiphers(unittest.TestCase): +class Https11CustomCiphers(unittest.TestCase): scheme = 'https' download_handler_cls = HTTP11DownloadHandler @@ -569,7 +569,7 @@ class Https11BadCiphers(unittest.TestCase): self.wrapper = WrappingFactory(self.site) self.host = 'localhost' self.port = reactor.listenSSL( - 0, self.wrapper, broken_ssl_context_factory(self.keyfile, self.certfile, cipher_string='CAMELLIA256-SHA'), + 0, self.wrapper, ssl_context_factory(self.keyfile, self.certfile, cipher_string='CAMELLIA256-SHA'), interface=self.host) self.portno = self.port.getHost().port self.download_handler = self.download_handler_cls( diff --git a/tests/test_webclient.py b/tests/test_webclient.py index 2ebe075ab..a81946490 100644 --- a/tests/test_webclient.py +++ b/tests/test_webclient.py @@ -6,9 +6,10 @@ import os import six import shutil +import OpenSSL.SSL from twisted.trial import unittest from twisted.web import server, static, util, resource -from twisted.internet import reactor, defer, ssl +from twisted.internet import reactor, defer from twisted.test.proto_helpers import StringTransport from twisted.python.filepath import FilePath from twisted.protocols.policies import WrappingFactory @@ -18,8 +19,9 @@ from scrapy.core.downloader import webclient as client from scrapy.core.downloader.contextfactory import ScrapyClientContextFactory from scrapy.http import Request, Headers from scrapy.settings import Settings +from scrapy.utils.misc import create_instance from scrapy.utils.python import to_bytes, to_unicode -from tests.mockserver import ssl_context_factory, broken_ssl_context_factory +from tests.mockserver import ssl_context_factory def getPage(url, contextFactory=None, response_transform=None, *args, **kwargs): @@ -402,12 +404,19 @@ class WebClientSSLTestCase(unittest.TestCase): self.assertEqual, to_bytes(s)) -class WebClientBrokenSSLTestCase(WebClientSSLTestCase): - context_factory = broken_ssl_context_factory(cipher_string='CAMELLIA256-SHA') +class WebClientCustomCiphersSSLTestCase(WebClientSSLTestCase): + # we try to use a cipher that is not enabled by default in OpenSSL + custom_ciphers = 'CAMELLIA256-SHA' + context_factory = ssl_context_factory(cipher_string=custom_ciphers) def testPayload(self): s = "0123456789" * 10 - settings = Settings({'DOWNLOADER_CLIENT_TLS_CIPHERS': 'CAMELLIA256-SHA'}) + settings = Settings({'DOWNLOADER_CLIENT_TLS_CIPHERS': self.custom_ciphers}) + client_context_factory = create_instance(ScrapyClientContextFactory, settings=settings, crawler=None) return getPage(self.getURL("payload"), body=s, - contextFactory=ScrapyClientContextFactory(settings=settings)).addCallback(self.assertEqual, - to_bytes(s)) + contextFactory=client_context_factory).addCallback(self.assertEqual, to_bytes(s)) + + def testPayloadDefaultCiphers(self): + s = "0123456789" * 10 + d = getPage(self.getURL("payload"), body=s, contextFactory=ScrapyClientContextFactory()) + return self.assertFailure(d, OpenSSL.SSL.Error)