From 3ac86f1c1a6d39235b07a7ef499903342dda8a44 Mon Sep 17 00:00:00 2001 From: julianz- <6255571+julianz-@users.noreply.github.com> Date: Mon, 24 Nov 2025 18:37:17 -0800 Subject: [PATCH] Align SSL adapters by removing `bind()` Relocated `SSLConnection` instantiation from `bind()` to `wrap()` to align with the adapter interface where `wrap()` is responsible for converting raw sockets to SSL connections. With this chenge `bind()` serves no purpose and so is deprecated. This change also fixes the pyOpenSSL adapter so that it doesn't prematurely wrap the raw socket, an issue in the old `bind()` implementation. --- .flake8 | 2 +- cheroot/server.py | 27 ++++-- cheroot/server.pyi | 2 +- cheroot/ssl/__init__.py | 18 +++- cheroot/ssl/__init__.pyi | 1 - cheroot/ssl/builtin.py | 4 - cheroot/ssl/builtin.pyi | 1 - cheroot/ssl/pyopenssl.py | 30 ++++--- cheroot/ssl/pyopenssl.pyi | 1 - cheroot/test/test_ssl.py | 86 +++++++++++++++---- .../changelog-fragments.d/801.deprecation.rst | 9 ++ stubtest_allowlist.txt | 1 + 12 files changed, 134 insertions(+), 48 deletions(-) create mode 100644 docs/changelog-fragments.d/801.deprecation.rst diff --git a/.flake8 b/.flake8 index aca9b2fffe..a82f62819f 100644 --- a/.flake8 +++ b/.flake8 @@ -128,7 +128,7 @@ per-file-ignores = cheroot/errors.py: DAR101, DAR201, I003, RST304, WPS111, WPS121, WPS422 cheroot/makefile.py: DAR101, DAR201, DAR401, E800, I003, I004, N801, N802, S101, WPS100, WPS110, WPS111, WPS117, WPS120, WPS121, WPS122, WPS123, WPS130, WPS204, WPS210, WPS212, WPS213, WPS220, WPS229, WPS231, WPS232, WPS338, WPS420, WPS422, WPS429, WPS431, WPS504, WPS604, WPS606 cheroot/server.py: DAR003, DAR101, DAR201, DAR202, DAR301, DAR401, E800, I001, I003, I004, I005, N806, RST201, RST301, RST303, RST304, WPS100, WPS110, WPS111, WPS115, WPS120, WPS121, WPS122, WPS130, WPS132, WPS201, WPS202, WPS204, WPS210, WPS211, WPS212, WPS213, WPS214, WPS220, WPS221, WPS225, WPS226, WPS229, WPS230, WPS231, WPS236, WPS237, WPS238, WPS301, WPS338, WPS342, WPS410, WPS420, WPS421, WPS422, WPS429, WPS432, WPS504, WPS505, WPS601, WPS602, WPS608, WPS617 - cheroot/ssl/builtin.py: DAR101, DAR201, DAR401, I001, I003, N806, RST304, WPS110, WPS111, WPS115, WPS117, WPS120, WPS121, WPS122, WPS130, WPS201, WPS210, WPS214, WPS229, WPS231, WPS338, WPS422, WPS501, WPS505, WPS529, WPS608, WPS612 + cheroot/ssl/builtin.py: DAR101, DAR201, DAR401, I001, I003, N806, RST304, WPS110, WPS111, WPS115, WPS117, WPS120, WPS121, WPS122, WPS130, WPS201, WPS210, WPS214, WPS229, WPS231, WPS338, WPS422, WPS501, WPS505, WPS529, WPS608 cheroot/ssl/pyopenssl.py: C815, DAR101, DAR201, DAR401, I001, I003, I005, N801, N804, RST304, WPS100, WPS110, WPS111, WPS117, WPS120, WPS121, WPS130, WPS210, WPS220, WPS221, WPS225, WPS229, WPS231, WPS238, WPS301, WPS335, WPS338, WPS420, WPS422, WPS430, WPS432, WPS501, WPS504, WPS505, WPS601, WPS608, WPS615 cheroot/test/conftest.py: DAR101, DAR201, DAR301, I001, I003, I005, WPS100, WPS130, WPS325, WPS354, WPS420, WPS422, WPS430, WPS457 cheroot/test/helper.py: DAR101, DAR201, DAR401, I001, I003, I004, N802, WPS110, WPS111, WPS121, WPS201, WPS220, WPS231, WPS301, WPS414, WPS421, WPS422, WPS505 diff --git a/cheroot/server.py b/cheroot/server.py index 9a288539c7..27e9173b15 100644 --- a/cheroot/server.py +++ b/cheroot/server.py @@ -80,6 +80,7 @@ import traceback as traceback_ import urllib.parse from functools import lru_cache +from warnings import warn as _warn from . import __version__, connections, errors from ._compat import IS_PPC, bton @@ -1987,8 +1988,7 @@ def bind(self, family, type, proto=0): type, proto, self.nodelay, - self.ssl_adapter, - self.reuse_port, + reuse_port=self.reuse_port, ) sock = self.socket = self.bind_socket(sock, self.bind_addr) self.bind_addr = self.resolve_real_bind_addr(sock) @@ -2112,10 +2112,26 @@ def prepare_socket( # pylint: disable=too-many-positional-arguments type, proto, nodelay, - ssl_adapter, + ssl_adapter=None, reuse_port=False, ): - """Create and prepare the socket object.""" + """ + Create and prepare the socket object. + + :param ssl_adapter: Legacy SSL adapter parameter. + This argument is now ignored internally. It is now deprecated + and will be removed in a future release. Pass the adapter to + the :class:`HTTPServer` constructor instead. + """ + if ssl_adapter is not None: + _warn( + 'The `ssl_adapter` parameter in `prepare_socket` is deprecated ' + 'and will be removed in a future version. Pass the adapter' + ' to the `HTTPServer` constructor instead.', + DeprecationWarning, + stacklevel=2, + ) + sock = socket.socket(family, type, proto) connections.prevent_socket_inheritance(sock) @@ -2140,9 +2156,6 @@ def prepare_socket( # pylint: disable=too-many-positional-arguments if nodelay and not isinstance(bind_addr, (str, bytes)): sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1) - if ssl_adapter is not None: - sock = ssl_adapter.bind(sock) - # If listening on the IPV6 any address ('::' = IN6ADDR_ANY), # activate dual-stack. See # https://github.com/cherrypy/cherrypy/issues/871. diff --git a/cheroot/server.pyi b/cheroot/server.pyi index c5c0f517f6..d18b251875 100644 --- a/cheroot/server.pyi +++ b/cheroot/server.pyi @@ -196,7 +196,7 @@ class HTTPServer: type, proto, nodelay, - ssl_adapter, + ssl_adapter=None, reuse_port: bool = ..., ): ... @staticmethod diff --git a/cheroot/ssl/__init__.py b/cheroot/ssl/__init__.py index c0072c0557..94c6792c26 100644 --- a/cheroot/ssl/__init__.py +++ b/cheroot/ssl/__init__.py @@ -1,6 +1,7 @@ """Implementation of the SSL adapter base interface.""" from abc import ABC, abstractmethod +from warnings import warn as _warn class Adapter(ABC): @@ -31,14 +32,25 @@ def __init__( self.private_key_password = private_key_password self.context = None - @abstractmethod def bind(self, sock): - """Wrap and return the given socket.""" + """ + Return the given socket. + + Deprecated: + This method no longer performs any SSL-specific operations. + SSL wrapping now happens in :meth:`.wrap`. :meth:`.bind` will be + removed in a future version. + """ + _warn( + 'SSLAdapter.bind() is deprecated and will be removed in a future version.', + DeprecationWarning, + stacklevel=2, + ) return sock @abstractmethod def wrap(self, sock): - """Wrap and return the given socket, plus WSGI environ entries.""" + """Wrap the given socket and return WSGI environ entries.""" raise NotImplementedError # pragma: no cover @abstractmethod diff --git a/cheroot/ssl/__init__.pyi b/cheroot/ssl/__init__.pyi index c595121546..13d8848a5f 100644 --- a/cheroot/ssl/__init__.pyi +++ b/cheroot/ssl/__init__.pyi @@ -18,7 +18,6 @@ class Adapter(ABC): *, private_key_password: str | bytes | None = ..., ): ... - @abstractmethod def bind(self, sock): ... @abstractmethod def wrap(self, sock): ... diff --git a/cheroot/ssl/builtin.py b/cheroot/ssl/builtin.py index ed747ab6e3..881e09db68 100644 --- a/cheroot/ssl/builtin.py +++ b/cheroot/ssl/builtin.py @@ -303,10 +303,6 @@ def context(self, context): if ssl.HAS_SNI and context.sni_callback is None: context.sni_callback = _sni_callback - def bind(self, sock): - """Wrap and return the given socket.""" - return super(BuiltinSSLAdapter, self).bind(sock) - def wrap(self, sock): """Wrap and return the given socket, plus WSGI environ entries.""" try: diff --git a/cheroot/ssl/builtin.pyi b/cheroot/ssl/builtin.pyi index b05aaf5ad7..dbd402ce2f 100644 --- a/cheroot/ssl/builtin.pyi +++ b/cheroot/ssl/builtin.pyi @@ -20,7 +20,6 @@ class BuiltinSSLAdapter(Adapter): def context(self): ... @context.setter def context(self, context) -> None: ... - def bind(self, sock): ... def wrap(self, sock): ... def get_environ(self, sock): ... def makefile(self, sock, mode: str = ..., bufsize: int = ...): ... diff --git a/cheroot/ssl/pyopenssl.py b/cheroot/ssl/pyopenssl.py index 8a041a79b8..194fd223fd 100644 --- a/cheroot/ssl/pyopenssl.py +++ b/cheroot/ssl/pyopenssl.py @@ -216,6 +216,8 @@ def __new__(mcl, name, bases, nmspc): 'settimeout', 'gettimeout', 'shutdown', + 'recv_into', + '_decref_socketios', ) proxy_methods_no_args = ('shutdown',) @@ -270,6 +272,17 @@ def __init__(self, *args): self._ssl_conn = SSL.Connection(*args) self._lock = threading.RLock() + @property + def _socket(self): + """ + Expose underlying raw socket. + + This is needed for times when the cheroot server needs access to the + original socket object, e.g. in response to a client attempting + to speak plain HTTP on an HTTPS port. + """ + return self._ssl_conn._socket + class pyOpenSSLAdapter(Adapter): """A wrapper for integrating :doc:`pyOpenSSL `.""" @@ -318,22 +331,18 @@ def __init__( private_key_password=private_key_password, ) - self._environ = None - - def bind(self, sock): - """Wrap and return the given socket.""" - if self.context is None: - self.context = self.get_context() - conn = SSLConnection(self.context, sock) + self.context = self.get_context() self._environ = self.get_environ() - return conn def wrap(self, sock): """Wrap and return the given socket, plus WSGI environ entries.""" # pyOpenSSL doesn't perform the handshake until the first read/write # forcing the handshake to complete tends to result in the connection # closing so we can't reliably access protocol/client cert for the env - return sock, self._environ.copy() + conn = SSLConnection(self.context, sock) + + conn.set_accept_state() # Tell OpenSSL this is a server connection + return conn, self._environ.copy() def _password_callback( self, @@ -442,7 +451,8 @@ def makefile(self, sock, mode='r', bufsize=-1): if 'r' in mode else SSLFileobjectStreamWriter ) - if SSL and isinstance(sock, ssl_conn_type): + # sock is an pyopenSSL.SSLConnection instance here + if SSL and isinstance(sock, SSLConnection): wrapped_socket = cls(sock, mode, bufsize) wrapped_socket.ssl_timeout = sock.gettimeout() return wrapped_socket diff --git a/cheroot/ssl/pyopenssl.pyi b/cheroot/ssl/pyopenssl.pyi index 59dae05ae7..4d0b092f53 100644 --- a/cheroot/ssl/pyopenssl.pyi +++ b/cheroot/ssl/pyopenssl.pyi @@ -34,7 +34,6 @@ class pyOpenSSLAdapter(Adapter): *, private_key_password: str | bytes | None = ..., ) -> None: ... - def bind(self, sock): ... def wrap(self, sock): ... def _password_callback( self, diff --git a/cheroot/test/test_ssl.py b/cheroot/test/test_ssl.py index 115237f4f7..881b5ae990 100644 --- a/cheroot/test/test_ssl.py +++ b/cheroot/test/test_ssl.py @@ -5,6 +5,7 @@ import http.client import json import os +import socket import ssl import subprocess import sys @@ -25,6 +26,8 @@ load_pem_private_key, ) +from cheroot.ssl import Adapter + from .._compat import ( IS_ABOVE_OPENSSL10, IS_ABOVE_OPENSSL31, @@ -39,7 +42,7 @@ ntob, ntou, ) -from ..server import Gateway, HTTPServer, get_ssl_adapter_class +from ..server import HTTPServer, get_ssl_adapter_class from ..testing import ( ANY_INTERFACE_IPV4, ANY_INTERFACE_IPV6, @@ -758,8 +761,6 @@ def test_http_over_https_error( tls_certificate_chain_pem_path, tls_certificate_private_key_pem_path, ) - if adapter_type == 'pyopenssl': - tls_adapter.context = tls_adapter.get_context() tls_certificate.configure_cert(tls_adapter.context) @@ -908,20 +909,7 @@ def test_openssl_adapter_with_false_key_password( expected_warn, ): """Check that server init fails when wrong private key password given.""" - httpserver = HTTPServer( - bind_addr=(ANY_INTERFACE_IPV4, EPHEMERAL_PORT), - gateway=Gateway, - ) - tls_adapter_cls = get_ssl_adapter_class(name=adapter_type) - tls_adapter = tls_adapter_cls( - certificate=tls_certificate_chain_pem_path, - private_key=tls_certificate_passwd_private_key_pem_path, - private_key_password=false_password, - ) - - httpserver.ssl_adapter = tls_adapter - with expected_warn, pytest.raises( OpenSSL.SSL.Error, # Decode error has happened very rarely with Python 3.9 in MacOS. @@ -929,7 +917,67 @@ def test_openssl_adapter_with_false_key_password( # to interpretation of garbage characters in certificates. match=r'.+\'(bad decrypt|decode error)\'.+', ): - httpserver.prepare() + tls_adapter_cls( + certificate=tls_certificate_chain_pem_path, + private_key=tls_certificate_passwd_private_key_pem_path, + private_key_password=false_password, + ) + + +@pytest.fixture +def dummy_adapter(monkeypatch): + """Provide a dummy SSL adapter instance.""" + # hide abstract methods so we can instantiate Adapter + monkeypatch.setattr(Adapter, '__abstractmethods__', set()) + # pylint: disable=abstract-class-instantiated + return Adapter( + certificate='cert.pem', + private_key='key.pem', + ) + + +def test_bind_deprecated_call(dummy_adapter): + """Test deprecated ``bind()`` method issues warning and returns socket.""" + sock = socket.socket() + + with pytest.deprecated_call(): + result = dummy_adapter.bind(sock) + + assert result is sock + + sock.close() + + +def test_prepare_socket_emits_deprecation_warning( + dummy_adapter, +): + """ + Test ``prepare_socket()`` deprecated argument triggers a warning. + + ``ssl_adapter`` has been deprecated in ``prepare_socket()``. + """ + # Required parameters for prepare_socket (standard IPv4 TCP config) + bind_addr = ('127.0.0.1', 8080) + family = socket.AF_INET + sock_type = socket.SOCK_STREAM + proto = socket.IPPROTO_TCP + nodelay = True + + expected_message = r'ssl_adapter.*deprecated' # regex pattern + + with pytest.deprecated_call(match=expected_message): + sock = HTTPServer.prepare_socket( + bind_addr=bind_addr, + family=family, + type=sock_type, + proto=proto, + nodelay=nodelay, + ssl_adapter=dummy_adapter, + ) + + # Check that the returned object is indeed a socket + assert isinstance(sock, socket.socket) + # Check we have a socket configured with file descriptor + assert sock.fileno() > 0 - assert not httpserver.requests._threads - assert not httpserver.ready + sock.close() diff --git a/docs/changelog-fragments.d/801.deprecation.rst b/docs/changelog-fragments.d/801.deprecation.rst new file mode 100644 index 0000000000..ed190438e3 --- /dev/null +++ b/docs/changelog-fragments.d/801.deprecation.rst @@ -0,0 +1,9 @@ +Deprecated :py:meth:`~cheroot.ssl.Adapter.bind` method from the :py:class:`~cheroot.ssl.Adapter` +interface and respective implementations. Previously :py:meth:`~cheroot.ssl.Adapter.bind` was +doing nothing in the :py:class:`builtin TLS adapter ` +but was being used in the :py:class:`~cheroot.ssl.pyopenssl.pyOpenSSLAdapter` to wrap the socket. +Socket wrapping is now done exclusively in the :py:meth:`~cheroot.ssl.Adapter.wrap` +method subclass implementations. A side-effect of this change is that the ``ssl_adapter`` argument +of :py:meth:`~cheroot.server.HTTPServer.prepare_socket()` is also deprecated. + +-- by :user:`julianz-` diff --git a/stubtest_allowlist.txt b/stubtest_allowlist.txt index cc6e94a5a5..efc794c108 100644 --- a/stubtest_allowlist.txt +++ b/stubtest_allowlist.txt @@ -19,6 +19,7 @@ cheroot.ssl.pyopenssl.SSLConnection.makefile cheroot.ssl.pyopenssl.SSLConnection.pending cheroot.ssl.pyopenssl.SSLConnection.read cheroot.ssl.pyopenssl.SSLConnection.recv +cheroot.ssl.pyopenssl.SSLConnection.recv_into cheroot.ssl.pyopenssl.SSLConnection.renegotiate cheroot.ssl.pyopenssl.SSLConnection.send cheroot.ssl.pyopenssl.SSLConnection.sendall