Make DOWNLOADER_CLIENT_TLS_CIPHERS=None enable Twisted defaults (#7665)

* Make DOWNLOADER_CLIENT_TLS_CIPHERS=None enable Twisted defaults

* Remove pointless comment

* Remove unnecessary mypy comments

docs/topics/settings.rst

@@ -763,6 +763,9 @@ necessary to access certain HTTPS websites: for example, you may need to use
 ``'DEFAULT:!DH'`` for a website with weak DH parameters or enable a
 specific cipher that is not included in ``DEFAULT`` if a website requires it.
 
+Set this setting to ``None`` to use the default ciphers of the underlying TLS
+implementation.
+
 .. _OpenSSL cipher list format: https://docs.openssl.org/master/man1/openssl-ciphers/#cipher-list-format
 
 .. note::

scrapy/core/downloader/contextfactory.py

@@ -17,7 +17,6 @@ from zope.interface.verify import verifyObject
 
 from scrapy.core.downloader.tls import (
     _TWISTED_VERSION_MAP,
-    DEFAULT_CIPHERS,
     _openssl_methods,
     _ScrapyClientTLSOptions,
     _ScrapyClientTLSOptions26,
@@ -68,11 +67,11 @@ class _ScrapyClientContextFactory(BrowserLikePolicyForHTTPS):
         self.tls_min_version: TLSVersion | None = tls_min_version
         self.tls_max_version: TLSVersion | None = tls_max_version
         self.tls_verbose_logging: bool = tls_verbose_logging  # unused
-        self.tls_ciphers: AcceptableCiphers
-        if tls_ciphers:
-            self.tls_ciphers = AcceptableCiphers.fromOpenSSLCipherString(tls_ciphers)
-        else:
-            self.tls_ciphers = DEFAULT_CIPHERS
+        self.tls_ciphers: AcceptableCiphers | None = (
+            AcceptableCiphers.fromOpenSSLCipherString(tls_ciphers)
+            if tls_ciphers
+            else None
+        )
         self._verify_certificates = verify_certificates
 
     @classmethod

scrapy/core/downloader/tls.py

@@ -43,6 +43,13 @@ _openssl_methods: dict[str, int] = {
 
 
 def __getattr__(name: str) -> Any:
+    if name == "DEFAULT_CIPHERS":
+        warnings.warn(
+            "scrapy.core.downloader.tls.DEFAULT_CIPHERS is deprecated.",
+            ScrapyDeprecationWarning,
+            stacklevel=2,
+        )
+        return AcceptableCiphers.fromOpenSSLCipherString("DEFAULT")
     deprecated = {
         "METHOD_TLS": "TLS",
         "METHOD_TLSv10": "TLSv1.0",
@@ -177,8 +184,3 @@ class _ScrapyClientTLSOptions26(ClientTLSOptions):
             return True
 
         return verifyCallback
-
-
-DEFAULT_CIPHERS: AcceptableCiphers = AcceptableCiphers.fromOpenSSLCipherString(
-    "DEFAULT"
-)

tests/test_core_downloader.py

@@ -8,7 +8,7 @@ import pytest
 from pytest_twisted import async_yield_fixture
 from twisted.internet.protocol import Factory
 from twisted.internet.protocol import Protocol as TxProtocol
-from twisted.internet.ssl import optionsForClientTLS
+from twisted.internet.ssl import AcceptableCiphers, optionsForClientTLS
 from twisted.protocols.tls import TLSMemoryBIOFactory, TLSMemoryBIOProtocol
 from twisted.web import server, static
 from twisted.web.client import Agent, BrowserLikePolicyForHTTPS, readBody
@@ -180,6 +180,49 @@ class TestContextFactory(TestContextFactoryBase):
         assert options & 0x4  # OP_LEGACY_SERVER_CONNECT
 
 
+class TestContextFactoryCiphers(TestContextFactoryBase):
+    async def _assert_factory_works(
+        self, server_url: str, client_context_factory: _ScrapyClientContextFactory
+    ) -> None:
+        s = "0123456789" * 10
+        body = await self.get_page(
+            server_url + "payload", client_context_factory, body=s
+        )
+        assert body == to_bytes(s)
+
+    def test_default(self) -> None:
+        """The default 'DEFAULT' value is passed to Twisted as is."""
+        crawler = get_crawler()
+        factory = build_from_crawler(_ScrapyClientContextFactory, crawler)
+        assert factory.tls_ciphers is not None
+        # OpenSSLAcceptableCiphers has no __eq__, so compare the parsed ciphers.
+        assert (
+            factory.tls_ciphers._ciphers
+            == AcceptableCiphers.fromOpenSSLCipherString("DEFAULT")._ciphers
+        )
+        assert factory._get_cert_options_kwargs()["acceptableCiphers"] is not None
+
+    def test_custom(self) -> None:
+        crawler = get_crawler(
+            settings_dict={"DOWNLOADER_CLIENT_TLS_CIPHERS": "CAMELLIA256-SHA"}
+        )
+        factory = build_from_crawler(_ScrapyClientContextFactory, crawler)
+        assert factory.tls_ciphers is not None
+        assert (
+            factory.tls_ciphers._ciphers
+            == AcceptableCiphers.fromOpenSSLCipherString("CAMELLIA256-SHA")._ciphers
+        )
+
+    @coroutine_test
+    async def test_none(self, server_url: str) -> None:
+        """A None value enables the Twisted default ciphers."""
+        crawler = get_crawler(settings_dict={"DOWNLOADER_CLIENT_TLS_CIPHERS": None})
+        factory = build_from_crawler(_ScrapyClientContextFactory, crawler)
+        assert factory.tls_ciphers is None
+        assert factory._get_cert_options_kwargs()["acceptableCiphers"] is None
+        await self._assert_factory_works(server_url, factory)
+
+
 class TestContextFactoryTLSMethod(TestContextFactoryBase):
     async def _assert_factory_works(
         self, server_url: str, client_context_factory: _ScrapyClientContextFactory
@@ -278,3 +321,10 @@ def test_deprecated_tls_module_names() -> None:
         match="scrapy.core.downloader.tls.openssl_methods is deprecated",
     ):
         assert isinstance(tls.openssl_methods, dict)
+    with pytest.warns(
+        ScrapyDeprecationWarning,
+        match="scrapy.core.downloader.tls.DEFAULT_CIPHERS is deprecated",
+    ):
+        assert tls.DEFAULT_CIPHERS._ciphers == (
+            AcceptableCiphers.fromOpenSSLCipherString("DEFAULT")._ciphers
+        )