Fixes and improvements for DOWNLOADER_CLIENT_TLS_CIPHERS.

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
 

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)

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))
 

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
 
 

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(

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)