diff --git a/docs/news.rst b/docs/news.rst index fd9786433..b8b976df9 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -254,7 +254,7 @@ Documentation - Added a ``CITATION.cff`` file. (:issue:`7502`, :issue:`7519`) -- Mentioned :setting:`DOWNLOADER_CLIENT_TLS_METHOD` in :ref:`bans`. +- Mentioned ``DOWNLOADER_CLIENT_TLS_METHOD`` in :ref:`bans`. (:issue:`5232`, :issue:`7518`) - Other documentation improvements and fixes. @@ -7464,7 +7464,7 @@ This 1.1 release brings a lot of interesting features and bug fixes: selectors engine without needing to upgrade Scrapy. - HTTPS downloader now does TLS protocol negotiation by default, instead of forcing TLS 1.0. You can also set the SSL/TLS method - using the new :setting:`DOWNLOADER_CLIENT_TLS_METHOD`. + using the new ``DOWNLOADER_CLIENT_TLS_METHOD`` setting. - These bug fixes may require your attention: diff --git a/docs/topics/download-handlers.rst b/docs/topics/download-handlers.rst index 272ff0dcd..84711e5af 100644 --- a/docs/topics/download-handlers.rst +++ b/docs/topics/download-handlers.rst @@ -283,8 +283,6 @@ If you want to use this handler you need to replace the default ones for the The global :setting:`DOWNLOAD_BIND_ADDRESS` setting is supported but the port number, if specified, will be ignored. - - The :setting:`DOWNLOADER_CLIENT_TLS_METHOD` setting. - - Settings specific to the Twisted networking or HTTP implementation, like :setting:`DNS_RESOLVER`. diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index 2a8f5b4c1..11c2656da 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -410,9 +410,9 @@ Here are some tips to keep in mind when dealing with these kinds of sites: services like `ProxyMesh`_. An open source alternative is `scrapoxy`_, a super proxy that you can attach your own proxies to. * for HTTPS websites, if blocking appears related to TLS behavior, consider - adjusting the :setting:`DOWNLOADER_CLIENT_TLS_METHOD` setting, since some - websites may respond differently depending on the TLS method used by the - client. + adjusting the :setting:`DOWNLOAD_TLS_MIN_VERSION` and + :setting:`DOWNLOAD_TLS_MAX_VERSION` settings, since some websites may respond + differently depending on the TLS method used by the client. * use a ban avoidance service, such as `Zyte API`_, which provides a `Scrapy plugin `__ and additional features, like `AI web scraping `__ diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index cf2658d60..06de33e6f 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -737,32 +737,50 @@ specific cipher that is not included in ``DEFAULT`` if a website requires it. by all 3rd-party handlers. It's currently unsupported by :class:`~scrapy.core.downloader.handlers._httpx.HttpxDownloadHandler`. -.. setting:: DOWNLOADER_CLIENT_TLS_METHOD +.. setting:: DOWNLOAD_TLS_MAX_VERSION -DOWNLOADER_CLIENT_TLS_METHOD ----------------------------- +DOWNLOAD_TLS_MAX_VERSION +------------------------ -Default: ``'TLS'`` +Default: ``None`` -Use this setting to customize the TLS/SSL method used by the HTTPS download -handler. +Use this setting to change the maximum version of the TLS protocol allowed to +be used by Scrapy. -This setting must be one of these string values: +This setting must be either ``None``, in which case it doesn't affect the +version selection, or one of these string values: -- ``'TLS'``: maps to OpenSSL's ``TLS_method()`` (a.k.a ``SSLv23_method()``), - which allows protocol negotiation, starting from the highest supported - by the platform; **default, recommended** -- ``'TLSv1.0'``: this value forces HTTPS connections to use TLS version 1.0 ; - set this if you want the behavior of Scrapy<1.1 -- ``'TLSv1.1'``: forces TLS version 1.1 -- ``'TLSv1.2'``: forces TLS version 1.2 +- ``'TLSv1.0'`` +- ``'TLSv1.1'`` +- ``'TLSv1.2'`` +- ``'TLSv1.3'`` + +The range of allowed TLS versions advertised by Scrapy when making TLS +connections will depend on the TLS implementation defaults and the values of +:setting:`DOWNLOAD_TLS_MIN_VERSION` and :setting:`DOWNLOAD_TLS_MAX_VERSION`. +It's possible to re-enable versions that are supported by the TLS +implementation but disabled by default by adjusting these settings, but it's +impossible to enable unsupported ones, such as any versions below 1.2 in many +modern environments. .. note:: Handling of this setting needs to be implemented inside the :ref:`download handler `, so it's not guaranteed to be supported - by all 3rd-party handlers. It's currently unsupported by - :class:`~scrapy.core.downloader.handlers._httpx.HttpxDownloadHandler`. + by all 3rd-party handlers. Additionally, the set of supported TLS versions + depends on the TLS implementation being used by the handler. + +.. setting:: DOWNLOAD_TLS_MIN_VERSION + +DOWNLOAD_TLS_MIN_VERSION +------------------------ + +Default: ``None`` + +Use this setting to change the minimum version of the TLS protocol allowed to +be used by Scrapy. + +See :setting:`DOWNLOAD_TLS_MAX_VERSION` for the details and limitations. .. setting:: DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING diff --git a/scrapy/core/downloader/contextfactory.py b/scrapy/core/downloader/contextfactory.py index d2626fd9d..40f09ffb2 100644 --- a/scrapy/core/downloader/contextfactory.py +++ b/scrapy/core/downloader/contextfactory.py @@ -1,13 +1,13 @@ from __future__ import annotations import warnings -from contextlib import contextmanager from typing import TYPE_CHECKING, Any, cast from OpenSSL import SSL from twisted.internet.ssl import ( AcceptableCiphers, CertificateOptions, + TLSVersion, optionsForClientTLS, ) from twisted.web.client import BrowserLikePolicyForHTTPS @@ -16,19 +16,19 @@ from zope.interface.declarations import implementer from zope.interface.verify import verifyObject from scrapy.core.downloader.tls import ( + _TWISTED_VERSION_MAP, DEFAULT_CIPHERS, + _openssl_methods, _ScrapyClientTLSOptions, _ScrapyClientTLSOptions26, - openssl_methods, ) from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.utils._deps_compat import TWISTED_TLS_NEW_IMPL from scrapy.utils.deprecate import create_deprecated_class from scrapy.utils.misc import build_from_crawler, load_object +from scrapy.utils.ssl import _get_cert_options_version_kwargs, _get_tls_version_limits if TYPE_CHECKING: - from collections.abc import Generator - from twisted.internet._sslverify import ClientTLSOptions # typing.Self requires Python 3.11 @@ -38,24 +38,14 @@ if TYPE_CHECKING: from scrapy.settings import BaseSettings -@contextmanager -def _filter_method_warning() -> Generator[None]: - with warnings.catch_warnings(): - # Twisted deprecation, https://github.com/scrapy/scrapy/issues/3288 - warnings.filterwarnings( - "ignore", - message=r"Passing method to twisted\.internet\.ssl\.CertificateOptions", - category=DeprecationWarning, - ) - yield - - @implementer(IPolicyForHTTPS) class _ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): """Non-peer-certificate verifying HTTPS context factory. - Default OpenSSL method is ``TLS_METHOD`` (also called ``SSLv23_METHOD``) - which allows TLS protocol negotiation. + Uses :setting:`DOWNLOADER_CLIENT_TLS_CIPHERS`, + :setting:`DOWNLOAD_TLS_MIN_VERSION` and :setting:`DOWNLOAD_TLS_MAX_VERSION` + to configure the :class:`~twisted.internet.ssl.CertificateOptions` + instance. The purpose of this custom class is to provide a ``creatorForNetloc()`` method that returns a ``_ScrapyClientTLSOptions`` instance configured based @@ -64,15 +54,19 @@ class _ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): def __init__( self, - method: int = SSL.SSLv23_METHOD, # noqa: S503 + method: int | None = SSL.SSLv23_METHOD, # noqa: S503 tls_verbose_logging: bool = False, tls_ciphers: str | None = None, *args: Any, verify_certificates: bool = False, + tls_min_version: TLSVersion | None = None, + tls_max_version: TLSVersion | None = None, **kwargs: Any, ): super().__init__(*args, **kwargs) # type: ignore[no-untyped-call] - self._ssl_method: int = method + self._ssl_method: int | None = method + 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: @@ -85,7 +79,7 @@ class _ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): def from_crawler( cls, crawler: Crawler, - method: int = SSL.SSLv23_METHOD, # noqa: S503 + method: int | None = SSL.SSLv23_METHOD, # noqa: S503 *args: Any, **kwargs: Any, ) -> Self: @@ -93,12 +87,21 @@ class _ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): "DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING" ) tls_ciphers: str | None = crawler.settings["DOWNLOADER_CLIENT_TLS_CIPHERS"] + # DOWNLOADER_CLIENT_TLS_METHOD reading and handling should be also moved here + # when the deprecated load_context_factory_from_settings() is removed + tls_min_ver, tls_max_ver = _get_tls_version_limits( + crawler.settings, _TWISTED_VERSION_MAP.__getitem__ + ) + if tls_min_ver or tls_max_ver: + method = None verify_certificates = crawler.settings.getbool("DOWNLOAD_VERIFY_CERTIFICATES") return cls( # type: ignore[misc] *args, method=method, tls_verbose_logging=tls_verbose_logging, tls_ciphers=tls_ciphers, + tls_min_version=tls_min_ver, + tls_max_version=tls_max_ver, verify_certificates=verify_certificates, **kwargs, ) @@ -108,12 +111,23 @@ class _ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): return self._get_cert_options() def _get_cert_options(self) -> CertificateOptions: - with _filter_method_warning(): - return _ScrapyCertificateOptions( - method=self._ssl_method, - fixBrokenPeers=True, - acceptableCiphers=self.tls_ciphers, + return _ScrapyCertificateOptions(**self._get_cert_options_kwargs()) + + def _get_cert_options_kwargs(self) -> dict[str, Any]: + kwargs: dict[str, Any] = { + "fixBrokenPeers": True, + "acceptableCiphers": self.tls_ciphers, + } + if self.tls_min_version or self.tls_max_version: + kwargs.update( + _get_cert_options_version_kwargs( + self.tls_min_version, self.tls_max_version + ) ) + # when ScrapyClientContextFactory is removed self._ssl_method can just be None by default + elif self._ssl_method != SSL.SSLv23_METHOD: + kwargs["method"] = self._ssl_method + return kwargs # should be removed together with ScrapyClientContextFactory def getContext( @@ -137,15 +151,10 @@ class _ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): self._get_context(), # type: ignore[arg-type] ) # Otherwise use the normal Twisted function. - # Note that this doesn't use self._get_context(). - with _filter_method_warning(): - return optionsForClientTLS( # type: ignore[no-any-return] - hostname=hostname.decode("ascii"), - extraCertificateOptions={ - "method": self._ssl_method, - "acceptableCiphers": self.tls_ciphers, - }, - ) + return optionsForClientTLS( # type: ignore[no-any-return] + hostname=hostname.decode("ascii"), + extraCertificateOptions=self._get_cert_options_kwargs(), + ) ScrapyClientContextFactory = create_deprecated_class( @@ -170,12 +179,6 @@ class BrowserLikeContextFactory(_ScrapyClientContextFactory): :meth:`creatorForNetloc` is the same as :class:`~twisted.web.client.BrowserLikePolicyForHTTPS` except this context factory allows setting the TLS/SSL method to use. - - The default OpenSSL method is ``TLS_METHOD`` (also called - ``SSLv23_METHOD``) which allows TLS protocol negotiation. - - As this overrides the parent ``creatorForNetloc()`` method, only - ``self._ssl_method`` is used from the parent class. """ def __init__(self, *args: Any, **kwargs: Any): @@ -189,11 +192,10 @@ class BrowserLikeContextFactory(_ScrapyClientContextFactory): super().__init__(*args, **kwargs) def creatorForNetloc(self, hostname: bytes, port: int) -> ClientTLSOptions: - with _filter_method_warning(): - return optionsForClientTLS( # type: ignore[no-any-return] - hostname=hostname.decode("ascii"), - extraCertificateOptions={"method": self._ssl_method}, - ) + return optionsForClientTLS( # type: ignore[no-any-return] + hostname=hostname.decode("ascii"), + extraCertificateOptions=self._get_cert_options_kwargs(), + ) @implementer(IPolicyForHTTPS) @@ -268,6 +270,16 @@ def _load_context_factory_from_settings(crawler: Crawler) -> IPolicyForHTTPS: Also passes values of other relevant settings to the factory class. """ + tls_method_setting: str = crawler.settings["DOWNLOADER_CLIENT_TLS_METHOD"] + if tls_method_setting != "TLS": + warnings.warn( + "Setting DOWNLOADER_CLIENT_TLS_METHOD to a non-default value is" + " deprecated, please use DOWNLOAD_TLS_MIN_VERSION and/or" + " DOWNLOAD_TLS_MAX_VERSION instead.", + ScrapyDeprecationWarning, + stacklevel=2, + ) + tls_method = _openssl_methods[tls_method_setting] if crawler.settings["DOWNLOADER_CLIENTCONTEXTFACTORY"] == "SENTINEL": context_factory_cls = _ScrapyClientContextFactory else: # pragma: no cover @@ -279,13 +291,12 @@ def _load_context_factory_from_settings(crawler: Crawler) -> IPolicyForHTTPS: context_factory_cls = load_object( crawler.settings["DOWNLOADER_CLIENTCONTEXTFACTORY"] ) - ssl_method = openssl_methods[crawler.settings.get("DOWNLOADER_CLIENT_TLS_METHOD")] return cast( "IPolicyForHTTPS", build_from_crawler( context_factory_cls, crawler, - method=ssl_method, + method=tls_method, ), ) diff --git a/scrapy/core/downloader/tls.py b/scrapy/core/downloader/tls.py index cb06f81df..3b903b39d 100644 --- a/scrapy/core/downloader/tls.py +++ b/scrapy/core/downloader/tls.py @@ -1,6 +1,7 @@ from __future__ import annotations import logging +import warnings from typing import TYPE_CHECKING, Any from OpenSSL import SSL @@ -18,8 +19,9 @@ from service_identity.pyopenssl import ( verify_ip_address, ) from twisted.internet._sslverify import ClientTLSOptions -from twisted.internet.ssl import AcceptableCiphers +from twisted.internet.ssl import AcceptableCiphers, TLSVersion +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.utils.deprecate import create_deprecated_class if TYPE_CHECKING: @@ -32,17 +34,37 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) -METHOD_TLS = "TLS" -METHOD_TLSv10 = "TLSv1.0" -METHOD_TLSv11 = "TLSv1.1" -METHOD_TLSv12 = "TLSv1.2" +_openssl_methods: dict[str, int] = { + "TLS": SSL.SSLv23_METHOD, # protocol negotiation (recommended) + "TLSv1.0": SSL.TLSv1_METHOD, # TLS 1.0 only + "TLSv1.1": SSL.TLSv1_1_METHOD, # TLS 1.1 only + "TLSv1.2": SSL.TLSv1_2_METHOD, # TLS 1.2 only +} -openssl_methods: dict[str, int] = { - METHOD_TLS: SSL.SSLv23_METHOD, # protocol negotiation (recommended) - METHOD_TLSv10: SSL.TLSv1_METHOD, # TLS 1.0 only - METHOD_TLSv11: SSL.TLSv1_1_METHOD, # TLS 1.1 only - METHOD_TLSv12: SSL.TLSv1_2_METHOD, # TLS 1.2 only +def __getattr__(name: str) -> Any: + deprecated = { + "METHOD_TLS": "TLS", + "METHOD_TLSv10": "TLSv1.0", + "METHOD_TLSv11": "TLSv1.1", + "METHOD_TLSv12": "TLSv1.2", + "openssl_methods": _openssl_methods, + } + if name in deprecated: + warnings.warn( + f"scrapy.core.downloader.tls.{name} is deprecated.", + ScrapyDeprecationWarning, + stacklevel=2, + ) + return deprecated[name] + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") + + +_TWISTED_VERSION_MAP: dict[str, TLSVersion] = { + "TLSv1.0": TLSVersion.TLSv1_0, + "TLSv1.1": TLSVersion.TLSv1_1, + "TLSv1.2": TLSVersion.TLSv1_2, + "TLSv1.3": TLSVersion.TLSv1_3, } diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 5d671feb3..de03c0107 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -16,6 +16,7 @@ Scrapy developers, if you add a setting here remember to: import sys from importlib import import_module from pathlib import Path +from typing import Any __all__ = [ "ADDONS", @@ -64,6 +65,8 @@ __all__ = [ "DOWNLOAD_HANDLERS_BASE", "DOWNLOAD_MAXSIZE", "DOWNLOAD_TIMEOUT", + "DOWNLOAD_TLS_MAX_VERSION", + "DOWNLOAD_TLS_MIN_VERSION", "DOWNLOAD_WARNSIZE", "DUPEFILTER_CLASS", "EDITOR", @@ -264,13 +267,15 @@ DOWNLOAD_WARNSIZE = 32 * 1024 * 1024 # 32m DOWNLOAD_TIMEOUT = 180 # 3mins +DOWNLOAD_TLS_MAX_VERSION = None +DOWNLOAD_TLS_MIN_VERSION = None + DOWNLOAD_VERIFY_CERTIFICATES = False DOWNLOADER = "scrapy.core.downloader.Downloader" DOWNLOADER_CLIENTCONTEXTFACTORY = "SENTINEL" DOWNLOADER_CLIENT_TLS_CIPHERS = "DEFAULT" -# Use highest TLS/SSL protocol version supported by the platform, also allowing negotiation: DOWNLOADER_CLIENT_TLS_METHOD = "TLS" DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING = False @@ -532,7 +537,7 @@ USER_AGENT = f"Scrapy/{import_module('scrapy').__version__} (+https://scrapy.org WARN_ON_GENERATOR_RETURN_VALUE = True -def __getattr__(name: str): +def __getattr__(name: str) -> Any: if name == "CONCURRENT_REQUESTS_PER_IP": import warnings # noqa: PLC0415 @@ -545,4 +550,4 @@ def __getattr__(name: str): ) return 0 - raise AttributeError + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") diff --git a/scrapy/utils/_deps_compat.py b/scrapy/utils/_deps_compat.py index cb5424476..fad7e6f6b 100644 --- a/scrapy/utils/_deps_compat.py +++ b/scrapy/utils/_deps_compat.py @@ -6,6 +6,8 @@ from twisted.python.versions import Version as TxVersion TWISTED_FAILURE_HAS_STACK = TWISTED_VERSION < TxVersion("twisted", 24, 10, 0) # changes to private _sslverify code, https://github.com/twisted/twisted/pull/12506 TWISTED_TLS_NEW_IMPL = TWISTED_VERSION >= TxVersion("twisted", 26, 4, 0) +# lowerMaximumSecurityTo off-by-1, https://github.com/twisted/twisted/issues/10232 +TWISTED_TLS_LIMITS_OFFBY1 = TWISTED_VERSION < TxVersion("twisted", 26, 4, 0) PYOPENSSL_VERSION = Version(PYOPENSSL_VERSION_STRING) # SSL.Context.use_certificate() wants an X509 object, SSL.Context.use_privatekey() wants a PKey object diff --git a/scrapy/utils/ssl.py b/scrapy/utils/ssl.py index 3fa2c77ba..22e9414b8 100644 --- a/scrapy/utils/ssl.py +++ b/scrapy/utils/ssl.py @@ -2,30 +2,59 @@ from __future__ import annotations import logging import ssl -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, TypedDict, TypeVar import OpenSSL._util as pyOpenSSLutil import OpenSSL.SSL import OpenSSL.version +from twisted.internet.ssl import CertificateOptions, TLSVersion +from scrapy.utils._deps_compat import TWISTED_TLS_LIMITS_OFFBY1 from scrapy.utils.python import to_unicode if TYPE_CHECKING: + from collections.abc import Callable + from OpenSSL.crypto import X509Name from scrapy.settings import BaseSettings logger = logging.getLogger(__name__) +_T = TypeVar("_T") + + +# common + + +def _get_tls_version_limit( + settings: BaseSettings, setting_name: str, converter: Callable[[str], _T] +) -> _T | None: + setting: str | None = settings[setting_name] + if setting is None: + return None + try: + return converter(setting) + except Exception as ex: + raise ValueError(f"Unknown {setting_name} value: {setting}") from ex + + +def _get_tls_version_limits( + settings: BaseSettings, converter: Callable[[str], _T] +) -> tuple[_T | None, _T | None]: + return ( + _get_tls_version_limit(settings, "DOWNLOAD_TLS_MIN_VERSION", converter), + _get_tls_version_limit(settings, "DOWNLOAD_TLS_MAX_VERSION", converter), + ) + # stdlib ssl module utils -# possible documented values for DOWNLOADER_CLIENT_TLS_METHOD -_STDLIB_PROTOCOL_MAP = { - "TLS": ssl.PROTOCOL_TLS_CLIENT, - "TLSv1.0": ssl.PROTOCOL_TLSv1, - "TLSv1.1": ssl.PROTOCOL_TLSv1_1, - "TLSv1.2": ssl.PROTOCOL_TLSv1_2, +_STDLIB_VERSION_MAP: dict[str, ssl.TLSVersion] = { + "TLSv1.0": ssl.TLSVersion.TLSv1, + "TLSv1.1": ssl.TLSVersion.TLSv1_1, + "TLSv1.2": ssl.TLSVersion.TLSv1_2, + "TLSv1.3": ssl.TLSVersion.TLSv1_3, } @@ -35,13 +64,13 @@ def _make_ssl_context(settings: BaseSettings) -> ssl.SSLContext: It's intended to be used in an HTTPS download handler. """ - method_setting: str = settings["DOWNLOADER_CLIENT_TLS_METHOD"] - if method_setting not in _STDLIB_PROTOCOL_MAP: - raise ValueError(f"Unsupported TLS method: {method_setting}") + tls_min_ver, tls_max_ver = _get_tls_version_limits( + settings, _STDLIB_VERSION_MAP.__getitem__ + ) ciphers_setting: str | None = settings["DOWNLOADER_CLIENT_TLS_CIPHERS"] verify_setting = settings.getbool("DOWNLOAD_VERIFY_CERTIFICATES") - ctx = ssl.SSLContext(_STDLIB_PROTOCOL_MAP[method_setting]) + ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) if verify_setting: ctx.check_hostname = True ctx.verify_mode = ssl.CERT_REQUIRED @@ -49,6 +78,10 @@ def _make_ssl_context(settings: BaseSettings) -> ssl.SSLContext: else: ctx.check_hostname = False ctx.verify_mode = ssl.CERT_NONE + if tls_min_ver is not None: + ctx.minimum_version = tls_min_ver + if tls_max_ver is not None: + ctx.maximum_version = tls_max_ver if ciphers_setting: ctx.set_ciphers(ciphers_setting) return ctx @@ -154,3 +187,42 @@ def _log_ssl_conn_debug_info(hostname: str, connection: OpenSSL.SSL.Connection) key_info = get_temp_key_info(connection._ssl) if key_info: logger.debug("SSL temp key: %s", key_info) + + +# Twisted-specific + + +class _CertificateOptionsVersionKwargs(TypedDict, total=False): + lowerMaximumSecurityTo: TLSVersion + insecurelyLowerMinimumTo: TLSVersion + raiseMinimumTo: TLSVersion + + +def _get_cert_options_version_kwargs( + min_version: TLSVersion | None, max_version: TLSVersion | None +) -> _CertificateOptionsVersionKwargs: + """Get TLS version kwargs for + :class:`~twisted.internet.ssl.CertificateOptions` for the given limits.""" + result: _CertificateOptionsVersionKwargs = {} + if max_version: + if TWISTED_TLS_LIMITS_OFFBY1: + # lowerMaximumSecurityTo is treated as 1 version lower than the passed one + versions = list(TLSVersion.iterconstants()) + max_index = versions.index(max_version) + if max_index + 1 >= len(versions): + raise ValueError( + f"Due to an error in Twisted < 26.4.0 cannot set the maximum TLS version to {max_version.name}" + ) + max_version = versions[max_index + 1] + result["lowerMaximumSecurityTo"] = max_version + if min_version: + # We cannot pass both insecurelyLowerMinimumTo and raiseMinimumTo, + # so we need to know the direction. + + # 1.0 in Twisted 22.8.0 and older, 1.2 in Twisted 22.10.0 and newer + default_min = CertificateOptions._defaultMinimumTLSVersion + if min_version < default_min: + result["insecurelyLowerMinimumTo"] = min_version + elif min_version > default_min: + result["raiseMinimumTo"] = min_version + return result diff --git a/tests/mockserver/http_base.py b/tests/mockserver/http_base.py index ffd11c2dd..7b38409ff 100644 --- a/tests/mockserver/http_base.py +++ b/tests/mockserver/http_base.py @@ -106,6 +106,16 @@ def main_factory( default=None, help="SSL cipher string (optional)", ) + parser.add_argument( + "--tls-min-version", + default=None, + help="Minimum accepted TLS version (optional)", + ) + parser.add_argument( + "--tls-max-version", + default=None, + help="Maximum accepted TLS version (optional)", + ) args = parser.parse_args() context_factory_kw = {} if args.keyfile: @@ -114,6 +124,10 @@ def main_factory( context_factory_kw["certfile"] = args.certfile if args.cipher_string: context_factory_kw["cipher_string"] = args.cipher_string + if args.tls_min_version: + context_factory_kw["tls_min_version"] = args.tls_min_version + if args.tls_max_version: + context_factory_kw["tls_max_version"] = args.tls_max_version context_factory = ssl_context_factory(**context_factory_kw) https_port = reactor.listenSSL(0, factory, context_factory) diff --git a/tests/mockserver/simple_https.py b/tests/mockserver/simple_https.py index a8f483ee1..943775fa5 100644 --- a/tests/mockserver/simple_https.py +++ b/tests/mockserver/simple_https.py @@ -21,11 +21,21 @@ class SimpleMockServer(BaseMockServer): listen_http = False module_name = "tests.mockserver.simple_https" - def __init__(self, keyfile: str, certfile: str, cipher_string: str | None): + def __init__( + self, + keyfile: str, + certfile: str, + *, + cipher_string: str | None = None, + tls_min_version: str | None = None, + tls_max_version: str | None = None, + ): super().__init__() self.keyfile = keyfile self.certfile = certfile self.cipher_string = cipher_string or "" + self.tls_min_version = tls_min_version + self.tls_max_version = tls_max_version def get_additional_args(self) -> list[str]: args = [ @@ -36,6 +46,10 @@ class SimpleMockServer(BaseMockServer): ] if self.cipher_string is not None: args.extend(["--cipher-string", self.cipher_string]) + if self.tls_min_version is not None: + args.extend(["--tls-min-version", self.tls_min_version]) + if self.tls_max_version is not None: + args.extend(["--tls-max-version", self.tls_max_version]) return args diff --git a/tests/mockserver/utils.py b/tests/mockserver/utils.py index 17d78ecdd..7aa656780 100644 --- a/tests/mockserver/utils.py +++ b/tests/mockserver/utils.py @@ -9,8 +9,10 @@ from OpenSSL import SSL from OpenSSL.crypto import FILETYPE_PEM, load_certificate, load_privatekey from twisted.internet.ssl import CertificateOptions, ContextFactory +from scrapy.core.downloader.tls import _TWISTED_VERSION_MAP from scrapy.utils._deps_compat import PYOPENSSL_WANTS_X509_PKEY from scrapy.utils.python import to_bytes +from scrapy.utils.ssl import _get_cert_options_version_kwargs if TYPE_CHECKING: from twisted.internet.interfaces import IOpenSSLContextFactory @@ -19,7 +21,10 @@ if TYPE_CHECKING: def ssl_context_factory( keyfile: str = "keys/localhost.key", certfile: str = "keys/localhost.crt", + *, cipher_string: str | None = None, + tls_min_version: str | None = None, + tls_max_version: str | None = None, ) -> IOpenSSLContextFactory: keyfile_path = Path(__file__).parent.parent / keyfile certfile_path = Path(__file__).parent.parent / certfile @@ -31,10 +36,14 @@ def ssl_context_factory( cert = load_certificate(FILETYPE_PEM, certfile_path.read_bytes()) # type: ignore[assignment] key = load_privatekey(FILETYPE_PEM, keyfile_path.read_bytes()) # type: ignore[assignment] + tls_min = _TWISTED_VERSION_MAP.get(tls_min_version) if tls_min_version else None + tls_max = _TWISTED_VERSION_MAP.get(tls_max_version) if tls_max_version else None + tls_version_kwargs = _get_cert_options_version_kwargs(tls_min, tls_max) # https://github.com/twisted/twisted/issues/12638 factory: CertificateOptions = CertificateOptions( privateKey=key, # type: ignore[arg-type] certificate=cert, # type: ignore[arg-type] + **tls_version_kwargs, ) if cipher_string: ctx = factory.getContext() diff --git a/tests/test_core_downloader.py b/tests/test_core_downloader.py index 43edf1774..abeaa2f65 100644 --- a/tests/test_core_downloader.py +++ b/tests/test_core_downloader.py @@ -14,7 +14,7 @@ from twisted.web import server, static from twisted.web.client import Agent, BrowserLikePolicyForHTTPS, readBody from twisted.web.client import Response as TxResponse -from scrapy.core.downloader import Downloader, Slot +from scrapy.core.downloader import Downloader, Slot, tls from scrapy.core.downloader.contextfactory import ( _load_context_factory_from_settings, _ScrapyClientContextFactory, @@ -202,18 +202,37 @@ class TestContextFactoryTLSMethod(TestContextFactoryBase): def test_setting_none(self): crawler = get_crawler(settings_dict={"DOWNLOADER_CLIENT_TLS_METHOD": None}) - with pytest.raises(KeyError): + with ( + pytest.warns( + ScrapyDeprecationWarning, + match="Setting DOWNLOADER_CLIENT_TLS_METHOD to a non-default value is deprecated", + ), + pytest.raises(KeyError), + ): _load_context_factory_from_settings(crawler) def test_setting_bad(self): crawler = get_crawler(settings_dict={"DOWNLOADER_CLIENT_TLS_METHOD": "bad"}) - with pytest.raises(KeyError): + with ( + pytest.warns( + ScrapyDeprecationWarning, + match="Setting DOWNLOADER_CLIENT_TLS_METHOD to a non-default value is deprecated", + ), + pytest.raises(KeyError), + ): _load_context_factory_from_settings(crawler) + @pytest.mark.filterwarnings( + r"ignore:Passing method to twisted\.internet\.ssl\.CertificateOptions:DeprecationWarning" + ) @coroutine_test async def test_setting_explicit(self, server_url: str) -> None: crawler = get_crawler(settings_dict={"DOWNLOADER_CLIENT_TLS_METHOD": "TLSv1.2"}) - client_context_factory = _load_context_factory_from_settings(crawler) + with pytest.warns( + ScrapyDeprecationWarning, + match="Setting DOWNLOADER_CLIENT_TLS_METHOD to a non-default value is deprecated", + ): + client_context_factory = _load_context_factory_from_settings(crawler) assert client_context_factory._ssl_method == OpenSSL.SSL.TLSv1_2_METHOD await self._assert_factory_works(server_url, client_context_factory) @@ -227,6 +246,9 @@ class TestContextFactoryTLSMethod(TestContextFactoryBase): assert client_context_factory._ssl_method == OpenSSL.SSL.SSLv23_METHOD await self._assert_factory_works(server_url, client_context_factory) + @pytest.mark.filterwarnings( + r"ignore:Passing method to twisted\.internet\.ssl\.CertificateOptions:DeprecationWarning" + ) @coroutine_test async def test_direct_init(self, server_url: str) -> None: client_context_factory = _ScrapyClientContextFactory(OpenSSL.SSL.TLSv1_2_METHOD) @@ -246,3 +268,16 @@ async def test_fetch_deprecated_spider_arg(): match=r"The fetch\(\) method of .+\.CustomDownloader requires a spider argument", ): await crawler.crawl_async() + + +def test_deprecated_tls_module_names() -> None: + with pytest.warns( + ScrapyDeprecationWarning, + match="scrapy.core.downloader.tls.METHOD_TLS is deprecated", + ): + assert tls.METHOD_TLS == "TLS" + with pytest.warns( + ScrapyDeprecationWarning, + match="scrapy.core.downloader.tls.openssl_methods is deprecated", + ): + assert isinstance(tls.openssl_methods, dict) diff --git a/tests/test_downloader_handler_httpx.py b/tests/test_downloader_handler_httpx.py index f26a17f02..e7a2ac0ba 100644 --- a/tests/test_downloader_handler_httpx.py +++ b/tests/test_downloader_handler_httpx.py @@ -15,6 +15,7 @@ from tests.test_downloader_handlers_http_base import ( TestHttpsCustomCiphersBase, TestHttpsInvalidDNSIdBase, TestHttpsInvalidDNSPatternBase, + TestHttpsTLSVersionBase, TestHttpsWrongHostnameBase, TestHttpWithCrawlerBase, TestMitmProxyBase, @@ -104,6 +105,10 @@ class TestHttpsCustomCiphers(HttpxDownloadHandlerMixin, TestHttpsCustomCiphersBa pass +class TestHttpsTLSVersion(HttpxDownloadHandlerMixin, TestHttpsTLSVersionBase): + pass + + class TestHttpWithCrawler(HttpxDownloadHandlerMixin, TestHttpWithCrawlerBase): pass diff --git a/tests/test_downloader_handler_twisted_http11.py b/tests/test_downloader_handler_twisted_http11.py index fb3305945..79750a136 100644 --- a/tests/test_downloader_handler_twisted_http11.py +++ b/tests/test_downloader_handler_twisted_http11.py @@ -18,6 +18,7 @@ from tests.test_downloader_handlers_http_base import ( TestHttpsCustomCiphersBase, TestHttpsInvalidDNSIdBase, TestHttpsInvalidDNSPatternBase, + TestHttpsTLSVersionBase, TestHttpsWrongHostnameBase, TestHttpWithCrawlerBase, TestMitmProxyBase, @@ -83,6 +84,10 @@ class TestHttpsCustomCiphers(HTTP11DownloadHandlerMixin, TestHttpsCustomCiphersB pass +class TestHttpsTLSVersion(HTTP11DownloadHandlerMixin, TestHttpsTLSVersionBase): + pass + + class TestHttpWithCrawler(HTTP11DownloadHandlerMixin, TestHttpWithCrawlerBase): pass diff --git a/tests/test_downloader_handler_twisted_http2.py b/tests/test_downloader_handler_twisted_http2.py index 24cb9b1aa..5f79a5453 100644 --- a/tests/test_downloader_handler_twisted_http2.py +++ b/tests/test_downloader_handler_twisted_http2.py @@ -19,6 +19,7 @@ from tests.test_downloader_handlers_http_base import ( TestHttpsCustomCiphersBase, TestHttpsInvalidDNSIdBase, TestHttpsInvalidDNSPatternBase, + TestHttpsTLSVersionBase, TestHttpsWrongHostnameBase, TestHttpWithCrawlerBase, TestMitmProxyBase, @@ -173,6 +174,10 @@ class TestHttp2CustomCiphers(H2DownloadHandlerMixin, TestHttpsCustomCiphersBase) pass +class TestHttp2TLSVersion(H2DownloadHandlerMixin, TestHttpsTLSVersionBase): + pass + + class TestHttp2WithCrawler(H2DownloadHandlerMixin, TestHttpWithCrawlerBase): is_secure = True diff --git a/tests/test_downloader_handlers_http_base.py b/tests/test_downloader_handlers_http_base.py index 3b60fa555..e67129abf 100644 --- a/tests/test_downloader_handlers_http_base.py +++ b/tests/test_downloader_handlers_http_base.py @@ -33,6 +33,7 @@ from scrapy.exceptions import ( UnsupportedURLSchemeError, ) from scrapy.http import Headers, HtmlResponse, Request, Response, TextResponse +from scrapy.utils._deps_compat import TWISTED_TLS_LIMITS_OFFBY1 from scrapy.utils.defer import deferred_from_coro, maybe_deferred_to_future from scrapy.utils.misc import build_from_crawler from scrapy.utils.spider import DefaultSpider @@ -894,7 +895,7 @@ class TestSimpleHttpsBase(ABC): @pytest.fixture(scope="class") def simple_mockserver(self) -> Generator[SimpleMockServer]: with SimpleMockServer( - self.keyfile, self.certfile, self.cipher_string + self.keyfile, self.certfile, cipher_string=self.cipher_string ) as simple_mockserver: yield simple_mockserver @@ -957,6 +958,143 @@ class TestHttpsCustomCiphersBase(TestSimpleHttpsBase): cipher_string = "CAMELLIA256-SHA" +class TestHttpsTLSVersionBase(ABC): + keyfile = "keys/localhost.key" + certfile = "keys/localhost.crt" + + @property + @abstractmethod + def download_handler_cls(self) -> type[DownloadHandlerProtocol]: + raise NotImplementedError + + @asynccontextmanager + async def get_dh( + self, client_tls_min: str | None, client_tls_max: str | None + ) -> AsyncGenerator[DownloadHandlerProtocol]: + settings = {} + if client_tls_min is not None: + settings["DOWNLOAD_TLS_MIN_VERSION"] = client_tls_min + if client_tls_max is not None: + settings["DOWNLOAD_TLS_MAX_VERSION"] = client_tls_max + crawler = get_crawler(DefaultSpider, settings_dict=settings) + crawler.spider = crawler._create_spider() + dh = build_from_crawler(self.download_handler_cls, crawler) + try: + yield dh + finally: + await dh.close() + + @pytest.mark.parametrize( + ( + "server_tls_min", + "server_tls_max", + "client_tls_min", + "client_tls_max", + "expect_success", + ), + [ + pytest.param(None, None, None, None, True, id="no-limits"), + pytest.param(None, None, None, "TLSv1.2", True, id="client-max-tls1.2"), + pytest.param(None, None, "TLSv1.3", None, True, id="client-min-tls1.3"), + pytest.param( + "TLSv1.3", + None, + None, + "TLSv1.2", + False, + id="client-max-below-server-min", + ), + pytest.param( + None, + "TLSv1.2", + "TLSv1.3", + None, + False, + id="client-min-above-server-max", + ), + pytest.param(None, "TLSv1.2", None, "TLSv1.2", True, id="both-tls1.2"), + pytest.param("TLSv1.3", None, "TLSv1.3", None, True, id="both-tls1.3"), + pytest.param( + None, + None, + "TLSv1.0", + None, + True, + id="client-min-tls1.0", + marks=pytest.mark.filterwarnings( + r"ignore:ssl\.TLSVersion\.TLSv1 is deprecated:DeprecationWarning" + ), + ), + pytest.param( + "TLSv1.0", + None, + "TLSv1.0", + None, + True, + id="both-min-tls1.0", + marks=pytest.mark.filterwarnings( + r"ignore:ssl\.TLSVersion\.TLSv1 is deprecated:DeprecationWarning" + ), + ), + pytest.param( + "TLSv1.2", + "TLSv1.3", + "TLSv1.2", + "TLSv1.3", + True, + id="both-tls1.2-1.3", + marks=pytest.mark.xfail( + TWISTED_TLS_LIMITS_OFFBY1, + reason="Can't set max to 1.3 on this Twisted version", + strict=True, + ), + ), + ], + ) + @coroutine_test + async def test_download( + self, + server_tls_min: str | None, + server_tls_max: str | None, + client_tls_min: str | None, + client_tls_max: str | None, + expect_success: bool, + ) -> None: + with SimpleMockServer( + self.keyfile, + self.certfile, + tls_min_version=server_tls_min, + tls_max_version=server_tls_max, + ) as simple_mockserver: + url = f"https://localhost:{simple_mockserver.port(is_secure=True)}/file" + request = Request(url) + async with self.get_dh(client_tls_min, client_tls_max) as dh: + if expect_success: + response = await dh.download_request(request) + assert response.body == b"0123456789" + else: + with pytest.raises( + (DownloadConnectionRefusedError, DownloadFailedError) + ): + await dh.download_request(request) + + @coroutine_test + async def test_invalid_min_version_setting(self) -> None: + with pytest.raises( + ValueError, match="Unknown DOWNLOAD_TLS_MIN_VERSION value: invalid" + ): + async with self.get_dh(client_tls_min="invalid", client_tls_max=None): + pass + + @coroutine_test + async def test_invalid_max_version_setting(self) -> None: + with pytest.raises( + ValueError, match="Unknown DOWNLOAD_TLS_MAX_VERSION value: invalid" + ): + async with self.get_dh(client_tls_min=None, client_tls_max="invalid"): + pass + + class TestHttpWithCrawlerBase(ABC): @property @abstractmethod