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.

Author: julianz-

.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

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
@@ -1369,6 +1370,7 @@ def _handle_no_ssl(self, req):
             'this server only speaks HTTPS on this port.'
         )
         req.simple_response('400 Bad Request', msg)
+        self.wfile.flush()
         self.linger = True
 
     def _conditional_error(self, req, response):
@@ -1987,7 +1989,9 @@ def bind(self, family, type, proto=0):
             type,
             proto,
             self.nodelay,
-            self.ssl_adapter,
+            # ssl_adapter is passed to HTTPServer, not needed here
+            # this parameter is deprecated and will be removed in future
+            None,
             self.reuse_port,
         )
         sock = self.socket = self.bind_socket(sock, self.bind_addr)
@@ -2115,7 +2119,25 @@ def prepare_socket(  # pylint: disable=too-many-positional-arguments
         ssl_adapter,
         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.
+
+        :deprecated ssl_adapter: This parameter is now deprecated and will be
+            removed in a future release. Pass the adapter to the '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 +2162,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.

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

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

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:

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 = ...): ...

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 <pyopenssl:index>`."""
@@ -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

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,

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,28 +909,108 @@ 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.
         # Might be caused by a random issue in file handling leading
         # 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
+
+    with pytest.deprecated_call():
+        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
+
+    sock.close()
+
+
+def test_prepare_socket_no_warning(recwarn):
+    """Test no warning is emitted when 'ssl_adapter' is correctly omitted."""
+    # 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
+
+    # The ptyest 'recwarn' fixture automatically
+    # collects warnings during this block.
+    # with contextlib.suppress(socket.error):
+    sock = HTTPServer.prepare_socket(
+        bind_addr=bind_addr,
+        family=family,
+        type=sock_type,
+        proto=proto,
+        nodelay=nodelay,
+        ssl_adapter=None,
+    )
+
+    # Assert that no DeprecationWarning was recorded by checking the list
+    # collected by the recwarn fixture.
+    deprecation_warnings = [
+        w for w in recwarn.list if issubclass(w.category, DeprecationWarning)
+    ]
+    assert len(deprecation_warnings) == 0
 
-    assert not httpserver.requests._threads
-    assert not httpserver.ready
+    # 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
+    sock.close()

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 <cheroot.ssl.builtin.BuiltinSSLAdapter>`
+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-`