diff --git a/pyproject.toml b/pyproject.toml index f13e67020..45e18ccc9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,8 +7,7 @@ name = "Scrapy" dynamic = ["version"] description = "A high-level Web Crawling and Web Scraping framework" dependencies = [ - # Twisted pinned until Scrapy is updated for its internal TLS API changes - "Twisted>=21.7.0,<=25.5.0", + "Twisted>=21.7.0", "cryptography>=37.0.0", "cssselect>=0.9.1", "defusedxml>=0.7.1", @@ -18,10 +17,9 @@ dependencies = [ "packaging", "parsel>=1.5.0", "protego>=0.1.15", - # pyOpenSSL pinned until Twisted with immutable contexts is released and supported in Scrapy - "pyOpenSSL>=22.0.0,<26.2.0", + "pyOpenSSL>=22.0.0", "queuelib>=1.4.2", - "service_identity>=18.1.0", + "service_identity>=23.1.0", "tldextract", "w3lib>=1.17.0", "zope.interface>=5.1.0", diff --git a/scrapy/core/downloader/contextfactory.py b/scrapy/core/downloader/contextfactory.py index 6575d59c1..7f7d34c21 100644 --- a/scrapy/core/downloader/contextfactory.py +++ b/scrapy/core/downloader/contextfactory.py @@ -5,7 +5,6 @@ from contextlib import contextmanager from typing import TYPE_CHECKING, Any, cast from OpenSSL import SSL -from twisted.internet._sslverify import _setAcceptableProtocols from twisted.internet.ssl import ( AcceptableCiphers, CertificateOptions, @@ -19,9 +18,11 @@ from zope.interface.verify import verifyObject from scrapy.core.downloader.tls import ( DEFAULT_CIPHERS, _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 @@ -108,7 +109,7 @@ class _ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): def _get_cert_options(self) -> CertificateOptions: with _filter_method_warning(): - return CertificateOptions( # type: ignore[no-any-return] + return _ScrapyCertificateOptions( method=self._ssl_method, fixBrokenPeers=True, acceptableCiphers=self.tls_ciphers, @@ -121,17 +122,21 @@ class _ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): return self._get_context() def _get_context(self) -> SSL.Context: - cert_options = self._get_cert_options() - ctx: SSL.Context = cert_options.getContext() - ctx.set_options(0x4) # OP_LEGACY_SERVER_CONNECT - return ctx + return self._get_cert_options().getContext() def creatorForNetloc(self, hostname: bytes, port: int) -> ClientTLSOptions: if not self._verify_certificates: - # _ScrapyClientTLSOptions is needed to skip verification errors + # Our options class is needed to skip verification errors + if TWISTED_TLS_NEW_IMPL: + return _ScrapyClientTLSOptions26( + self._get_cert_options()._makeTLSConnection, + hostname.decode("ascii"), + verbose_logging=self.tls_verbose_logging, + ) return _ScrapyClientTLSOptions( - hostname.decode("ascii"), self._get_context() - ) # type: ignore[no-untyped-call] + hostname.decode("ascii"), # type: ignore[arg-type] + 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(): @@ -202,8 +207,25 @@ class _AcceptableProtocolsContextFactory: the acceptable protocols on the :class:`.ClientTLSOptions` instance returned by it. It's only needed because we support custom factories via :setting:`DOWNLOADER_CLIENTCONTEXTFACTORY`. + + It's a no-op on Twisted 26.4.0+, though using it with custom + factories on those Twisted versions may be not enough for HTTP/2 support. """ + # Something needs to call set_alpn_protos() for ALPN to work. + # + # Twisted < 26.4.0 does it in OpenSSLCertificateOptions._makeContext() + # (requires passing acceptableProtocols from the factory to + # OpenSSLCertificateOptions) and in TLSMemoryBIOFactory._createConnection() + # based on H2ClientFactory.acceptableProtocols (too late, it seems). + # + # Newer Twisted does it in OpenSSLCertificateOptions._makeContext() as + # well, and in OpenSSLCertificateOptions._makeTLSConnection() based on + # H2ClientFactory.acceptableProtocols (which now works). + # + # When we drop DOWNLOADER_CLIENTCONTEXTFACTORY it looks like we can replace + # all of this with _ScrapyClientContextFactory.acceptableProtocols. + def __init__(self, context_factory: Any, acceptable_protocols: list[bytes]): verifyObject(IPolicyForHTTPS, context_factory) self._wrapped_context_factory: Any = context_factory @@ -213,7 +235,12 @@ class _AcceptableProtocolsContextFactory: options: ClientTLSOptions = self._wrapped_context_factory.creatorForNetloc( hostname, port ) - _setAcceptableProtocols(options._ctx, self._acceptable_protocols) + if not TWISTED_TLS_NEW_IMPL: + from twisted.internet._sslverify import ( # type: ignore[attr-defined] # noqa: PLC0415 # pylint: disable=no-name-in-module + _setAcceptableProtocols, + ) + + _setAcceptableProtocols(options._ctx, self._acceptable_protocols) # type: ignore[attr-defined] return options @@ -225,6 +252,18 @@ AcceptableProtocolsContextFactory = create_deprecated_class( ) +class _ScrapyCertificateOptions(CertificateOptions): + """A wrapper needed to add flags to the SSL context before it's used.""" + + def _makeContext(self, skipCiphers: bool = False) -> SSL.Context: + if TWISTED_TLS_NEW_IMPL: + ctx = super()._makeContext(skipCiphers) + else: + ctx = super()._makeContext() + ctx.set_options(0x4) # OP_LEGACY_SERVER_CONNECT + return ctx + + def _load_context_factory_from_settings(crawler: Crawler) -> IPolicyForHTTPS: """Create an instance of :setting:`DOWNLOADER_CLIENTCONTEXTFACTORY`. diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 1127c533f..e6451cdd7 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -225,7 +225,8 @@ class TunnelingTCP4ClientEndpoint(TCP4ClientEndpoint): if respm and int(respm.group("status")) == 200: # set proper Server Name Indication extension sslOptions = self._contextFactory.creatorForNetloc( # type: ignore[call-arg,misc] - self._tunneledHost, self._tunneledPort + self._tunneledHost, # type: ignore[arg-type] + self._tunneledPort, ) self._protocol.transport.startTLS(sslOptions, self._protocolFactory) self._tunnelReadyDeferred.callback(self._protocol) diff --git a/scrapy/core/downloader/tls.py b/scrapy/core/downloader/tls.py index 9ec5b8c48..779c05daa 100644 --- a/scrapy/core/downloader/tls.py +++ b/scrapy/core/downloader/tls.py @@ -1,15 +1,34 @@ +from __future__ import annotations + import logging -from typing import Any +from typing import TYPE_CHECKING, Any from OpenSSL import SSL from service_identity import VerificationError from service_identity.exceptions import CertificateError -from service_identity.pyopenssl import verify_hostname, verify_ip_address +from service_identity.hazmat import ( + DNS_ID, + IPAddress_ID, + ServiceID, + verify_service_identity, +) +from service_identity.pyopenssl import ( + extract_patterns, + verify_hostname, + verify_ip_address, +) from twisted.internet._sslverify import ClientTLSOptions from twisted.internet.ssl import AcceptableCiphers from scrapy.utils.deprecate import create_deprecated_class +if TYPE_CHECKING: + from collections.abc import Callable + + from OpenSSL.crypto import X509 + from twisted.protocols.tls import TLSMemoryBIOProtocol + + logger = logging.getLogger(__name__) @@ -39,6 +58,8 @@ class _ScrapyClientTLSOptions(ClientTLSOptions): Instances of this class are returned from :class:`._ScrapyClientContextFactory`. + + This class is used on Twisted older than 26.4.0. """ def _identityVerifyingInfoCallback( @@ -64,7 +85,7 @@ class _ScrapyClientTLSOptions(ClientTLSOptions): e, ) else: - super()._identityVerifyingInfoCallback(connection, where, ret) # type: ignore[no-untyped-call] + super()._identityVerifyingInfoCallback(connection, where, ret) # type: ignore[misc] ScrapyClientTLSOptions = create_deprecated_class( @@ -75,6 +96,78 @@ ScrapyClientTLSOptions = create_deprecated_class( ) +class _ScrapyClientTLSOptions26(ClientTLSOptions): + """ + SSL Client connection creator ignoring certificate verification errors + (for genuinely invalid certificates or bugs in verification code). + + Same as Twisted's private _sslverify.ClientTLSOptions, + except that VerificationError, CertificateError and ValueError + exceptions are caught, so that the connection is not closed, only + logging warnings. + + Instances of this class are returned from + :class:`.ScrapyClientContextFactory`. + + This class is used on Twisted 26.4.0 and newer. + """ + + def __init__( + self, + createConnection: Callable[[TLSMemoryBIOProtocol], SSL.Connection], + hostname: str, + verbose_logging: bool = False, + ): + super().__init__(createConnection, hostname) + self.verbose_logging: bool = verbose_logging + + def clientConnectionForTLS( + self, tlsProtocol: TLSMemoryBIOProtocol + ) -> SSL.Connection: + """This method is needed to override the verify callback.""" + conn = super().clientConnectionForTLS(tlsProtocol) + callback = self._verifyCB( + self._hostnameIsDnsName, self._hostnameASCII, self.verbose_logging + ) + conn.set_verify(SSL.VERIFY_PEER | SSL.VERIFY_FAIL_IF_NO_PEER_CERT, callback) + return conn + + @staticmethod + def _verifyCB( + hostIsDNS: bool, hostnameASCII: str, verbose_logging: bool + ) -> Callable[[SSL.Connection, X509, int, int, int], bool]: + svcid: ServiceID = ( + DNS_ID(hostnameASCII) if hostIsDNS else IPAddress_ID(hostnameASCII) + ) + + def verifyCallback( + conn: SSL.Connection, cert: X509, err: int, depth: int, ok: int + ) -> bool: + if depth != 0: + # We are only verifying the leaf certificate. + return bool(ok) + + try: + verify_service_identity(extract_patterns(cert), [svcid], []) + except (CertificateError, VerificationError) as e: + logger.warning( + 'Remote certificate is not valid for hostname "%s"; %s', + hostnameASCII, + e, + ) + except ValueError as e: + logger.warning( + "Ignoring error while verifying certificate " + 'from host "%s" (exception: %r)', + hostnameASCII, + e, + ) + + return True + + return verifyCallback + + DEFAULT_CIPHERS: AcceptableCiphers = AcceptableCiphers.fromOpenSSLCipherString( "DEFAULT" ) diff --git a/scrapy/shell.py b/scrapy/shell.py index 0966d9f55..44e542470 100644 --- a/scrapy/shell.py +++ b/scrapy/shell.py @@ -241,7 +241,10 @@ class Shell: with contextlib.suppress(IgnoreRequest): response = threads.blockingCallFromThread( - reactor, deferred_f_from_coro_f(self._schedule), request, spider + reactor, + deferred_f_from_coro_f(self._schedule), # type: ignore[arg-type] + request, + spider, ) else: assert self._loop diff --git a/scrapy/utils/_deps_compat.py b/scrapy/utils/_deps_compat.py index 2ca6f657c..b7086bc98 100644 --- a/scrapy/utils/_deps_compat.py +++ b/scrapy/utils/_deps_compat.py @@ -4,6 +4,10 @@ from twisted import version as TWISTED_VERSION 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) +# AsyncioSelectorReactor no longer calls get_event_loop(), https://github.com/twisted/twisted/pull/12508 +TWISTED_LOOP_314_CHANGES = 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/tests/mockserver/utils.py b/tests/mockserver/utils.py index 188eef61d..17d78ecdd 100644 --- a/tests/mockserver/utils.py +++ b/tests/mockserver/utils.py @@ -1,13 +1,13 @@ from __future__ import annotations from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, cast from cryptography.hazmat.primitives.serialization import load_pem_private_key from cryptography.x509 import load_pem_x509_certificate from OpenSSL import SSL from OpenSSL.crypto import FILETYPE_PEM, load_certificate, load_privatekey -from twisted.internet.ssl import CertificateOptions +from twisted.internet.ssl import CertificateOptions, ContextFactory from scrapy.utils._deps_compat import PYOPENSSL_WANTS_X509_PKEY from scrapy.utils.python import to_bytes @@ -31,13 +31,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] + # https://github.com/twisted/twisted/issues/12638 factory: CertificateOptions = CertificateOptions( - privateKey=key, - certificate=cert, + privateKey=key, # type: ignore[arg-type] + certificate=cert, # type: ignore[arg-type] ) if cipher_string: ctx = factory.getContext() # disabling TLS1.3 because it unconditionally enables some strong ciphers ctx.set_options(SSL.OP_CIPHER_SERVER_PREFERENCE | SSL.OP_NO_TLSv1_3) ctx.set_cipher_list(to_bytes(cipher_string)) - return factory + return cast("ContextFactory", factory) diff --git a/tests/test_core_downloader.py b/tests/test_core_downloader.py index f9c5abf8a..43edf1774 100644 --- a/tests/test_core_downloader.py +++ b/tests/test_core_downloader.py @@ -6,6 +6,10 @@ from typing import TYPE_CHECKING, cast import OpenSSL.SSL 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.protocols.tls import TLSMemoryBIOFactory, TLSMemoryBIOProtocol from twisted.web import server, static from twisted.web.client import Agent, BrowserLikePolicyForHTTPS, readBody from twisted.web.client import Response as TxResponse @@ -17,7 +21,10 @@ from scrapy.core.downloader.contextfactory import ( ) from scrapy.core.downloader.handlers.http11 import _RequestBodyProducer from scrapy.exceptions import ScrapyDeprecationWarning -from scrapy.utils._deps_compat import PYOPENSSL_SET_CIPHER_LIST_TMP_CONN +from scrapy.utils._deps_compat import ( + PYOPENSSL_SET_CIPHER_LIST_TMP_CONN, + TWISTED_TLS_NEW_IMPL, +) from scrapy.utils.defer import maybe_deferred_to_future from scrapy.utils.misc import build_from_crawler from scrapy.utils.python import to_bytes @@ -98,50 +105,82 @@ class TestContextFactoryBase: class TestContextFactory(TestContextFactoryBase): - @coroutine_test - async def test_payload(self, server_url: str) -> None: - s = "0123456789" * 10 + @pytest.fixture + def factory(self) -> _ScrapyClientContextFactory: crawler = get_crawler() - client_context_factory = _load_context_factory_from_settings(crawler) - body = await self.get_page( - server_url + "payload", client_context_factory, body=s + return _load_context_factory_from_settings(crawler) + + @staticmethod + def _get_dummy_protocol() -> TLSMemoryBIOProtocol: + # from Twisted src/twisted/web/test/test_agent.py::dummyTLSProtocol() + factory = TLSMemoryBIOFactory( + optionsForClientTLS("example.com"), True, Factory.forProtocol(TxProtocol) ) + return factory.buildProtocol(None) + + @coroutine_test + async def test_payload( + self, factory: _ScrapyClientContextFactory, server_url: str + ) -> None: + s = "0123456789" * 10 + body = await self.get_page(server_url + "payload", factory, body=s) assert body == to_bytes(s) - def test_no_context_sharing(self) -> None: + @pytest.mark.skipif( + TWISTED_TLS_NEW_IMPL, + reason="The context is not stored on this Twisted version", + ) + def test_no_context_sharing(self, factory: _ScrapyClientContextFactory) -> None: """Every call to creatorForNetloc() should give a fresh context.""" - crawler = get_crawler() - client_context_factory: _ScrapyClientContextFactory = ( - _load_context_factory_from_settings(crawler) - ) - creator1 = client_context_factory.creatorForNetloc(b"website1.tld", 443) + creator1 = factory.creatorForNetloc(b"website1.tld", 443) assert creator1._hostnameBytes == b"website1.tld" - creator2 = client_context_factory.creatorForNetloc(b"website2.tld", 443) + creator2 = factory.creatorForNetloc(b"website2.tld", 443) assert creator2._hostnameBytes == b"website2.tld" - assert creator1._ctx is not creator2._ctx + assert creator1._ctx is not creator2._ctx # type: ignore[attr-defined] + + def test_no_context_sharing_with_conn( + self, factory: _ScrapyClientContextFactory + ) -> None: + """Like test_no_context_sharing() but get the context from a connection.""" + creator1 = factory.creatorForNetloc(b"website1.tld", 443) + assert creator1._hostnameBytes == b"website1.tld" + conn1 = creator1.clientConnectionForTLS(self._get_dummy_protocol()) + + creator2 = factory.creatorForNetloc(b"website2.tld", 443) + assert creator2._hostnameBytes == b"website2.tld" + conn2 = creator2.clientConnectionForTLS(self._get_dummy_protocol()) + + assert conn1.get_context() is not conn2.get_context() @pytest.mark.skipif( PYOPENSSL_SET_CIPHER_LIST_TMP_CONN, reason="Fails or doesn't make sense on this pyOpenSSL version", ) - def test_no_immutable_ctx_warning(self) -> None: + def test_no_immutable_ctx_warning( + self, factory: _ScrapyClientContextFactory + ) -> None: """There should be no pyOpenSSL context modification warning. pyOpenSSL < 25.1.0 doesn't produce this warning, and on 25.1.0 it's always produced due to https://github.com/scrapy/scrapy/issues/6859#issuecomment-4294917851. """ - crawler = get_crawler() - client_context_factory: _ScrapyClientContextFactory = ( - _load_context_factory_from_settings(crawler) - ) with warnings.catch_warnings(): warnings.filterwarnings( "error", category=DeprecationWarning, message="Attempting to mutate a Context after a Connection was created", ) - client_context_factory.creatorForNetloc(b"website.tld", 443) + factory.creatorForNetloc(b"website.tld", 443) + + def test_ctx_flags(self, factory: _ScrapyClientContextFactory) -> None: + """The context should have the expected flags set.""" + creator = factory.creatorForNetloc(b"website.tld", 443) + conn = creator.clientConnectionForTLS(self._get_dummy_protocol()) + ctx = conn.get_context() + # fragile but pyOpenSSL doesn't have Context.get_options() + options = OpenSSL.SSL._lib.SSL_CTX_get_options(ctx._context) # type: ignore[attr-defined] + assert options & 0x4 # OP_LEGACY_SERVER_CONNECT class TestContextFactoryTLSMethod(TestContextFactoryBase): diff --git a/tests/test_crawler_subprocess.py b/tests/test_crawler_subprocess.py index beae4f277..dc2fdda4c 100644 --- a/tests/test_crawler_subprocess.py +++ b/tests/test_crawler_subprocess.py @@ -14,6 +14,7 @@ from packaging.version import parse as parse_version from pexpect.popen_spawn import PopenSpawn from w3lib import __version__ as w3lib_version +from scrapy.utils._deps_compat import TWISTED_LOOP_314_CHANGES from tests.utils import async_sleep, get_script_run_env from tests.utils.decorators import coroutine_test @@ -171,6 +172,9 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin): assert "Using asyncio event loop: uvloop.Loop" in log assert "async pipeline opened!" in log + @pytest.mark.xfail( + TWISTED_LOOP_314_CHANGES, reason="Breaks with Twisted changes for Python 3.14" + ) @pytest.mark.requires_uvloop def test_asyncio_enabled_reactor_same_loop(self): log = self.run_script("asyncio_enabled_reactor_same_loop.py") diff --git a/tox.ini b/tox.ini index e0e95c194..72686d3d0 100644 --- a/tox.ini +++ b/tox.ini @@ -48,7 +48,7 @@ deps = typing-extensions==4.15.0 Pillow==12.2.0 Protego==0.6.0 - Twisted==25.5.0 + Twisted==26.4.0 attrs==26.1.0 boto3-stubs[s3]==1.43.2 botocore-stubs==1.42.41 @@ -119,7 +119,7 @@ deps = parsel==1.5.0 pyOpenSSL==22.0.0 queuelib==1.4.2 - service_identity==18.1.0 + service_identity==23.1.0 w3lib==1.17.0 zope.interface==5.1.0 {[test-requirements]deps} @@ -159,6 +159,7 @@ basepython = {[pinned]basepython} deps = {[pinned]deps} Pillow==8.3.2 + Twisted[http2]==21.7.0 boto3==1.20.0 bpython==0.7.1 brotli==1.2.0; implementation_name != "pypy" @@ -234,7 +235,7 @@ deps = parsel==1.5.0 pyOpenSSL==24.3.0 queuelib==1.4.2 - service_identity==18.1.0 + service_identity==23.1.0 w3lib==1.20.0 zope.interface==5.1.0 commands =