Skip to content

Commit 8322acf

Browse files
julianz-julianz-
committed
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 deleted. This change also fixes the pyOpenSSL adapter so that it doesn't prematurely wrap the raw socket, an issue in the old `bind()` implementation.
1 parent a475500 commit 8322acf

File tree

12 files changed

+48
-30
lines changed

12 files changed

+48
-30
lines changed

‎.flake8‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ per-file-ignores =
128128
cheroot/errors.py: DAR101, DAR201, I003, RST304, WPS111, WPS121, WPS422
129129
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
130130
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
131-
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
131+
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
132132
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
133133
cheroot/test/conftest.py: DAR101, DAR201, DAR301, I001, I003, I005, WPS100, WPS130, WPS325, WPS354, WPS420, WPS422, WPS430, WPS457
134134
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‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1987,7 +1987,6 @@ def bind(self, family, type, proto=0):
19871987
type,
19881988
proto,
19891989
self.nodelay,
1990-
self.ssl_adapter,
19911990
self.reuse_port,
19921991
)
19931992
sock = self.socket = self.bind_socket(sock, self.bind_addr)
@@ -2037,7 +2036,6 @@ def bind_unix_socket(self, bind_addr): # noqa: C901 # FIXME
20372036
type=socket.SOCK_STREAM,
20382037
proto=0,
20392038
nodelay=self.nodelay,
2040-
ssl_adapter=self.ssl_adapter,
20412039
reuse_port=self.reuse_port,
20422040
)
20432041

@@ -2112,7 +2110,6 @@ def prepare_socket( # pylint: disable=too-many-positional-arguments
21122110
type,
21132111
proto,
21142112
nodelay,
2115-
ssl_adapter,
21162113
reuse_port=False,
21172114
):
21182115
"""Create and prepare the socket object."""
@@ -2140,9 +2137,6 @@ def prepare_socket( # pylint: disable=too-many-positional-arguments
21402137
if nodelay and not isinstance(bind_addr, (str, bytes)):
21412138
sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)
21422139

2143-
if ssl_adapter is not None:
2144-
sock = ssl_adapter.bind(sock)
2145-
21462140
# If listening on the IPV6 any address ('::' = IN6ADDR_ANY),
21472141
# activate dual-stack. See
21482142
# https://github.com/cherrypy/cherrypy/issues/871.

‎cheroot/server.pyi‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,6 @@ class HTTPServer:
196196
type,
197197
proto,
198198
nodelay,
199-
ssl_adapter,
200199
reuse_port: bool = ...,
201200
): ...
202201
@staticmethod

‎cheroot/ssl/__init__.py‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Implementation of the SSL adapter base interface."""
22

3+
import warnings
34
from abc import ABC, abstractmethod
45

56

@@ -31,9 +32,20 @@ def __init__(
3132
self.private_key_password = private_key_password
3233
self.context = None
3334

34-
@abstractmethod
3535
def bind(self, sock):
36-
"""Wrap and return the given socket."""
36+
"""
37+
Wrap and return the given socket.
38+
39+
Deprecated:
40+
This method no longer performs any SSL-specific operations.
41+
SSL wrapping now happens in wrap(). This method will be
42+
removed in a future version.
43+
"""
44+
warnings.warn(
45+
'SSLAdapter.bind() is deprecated and will be removed in a future version.',
46+
DeprecationWarning,
47+
stacklevel=2,
48+
)
3749
return sock
3850

3951
@abstractmethod

‎cheroot/ssl/__init__.pyi‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ class Adapter(ABC):
1818
*,
1919
private_key_password: str | bytes | None = ...,
2020
): ...
21-
@abstractmethod
2221
def bind(self, sock): ...
2322
@abstractmethod
2423
def wrap(self, sock): ...

‎cheroot/ssl/builtin.py‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -303,10 +303,6 @@ def context(self, context):
303303
if ssl.HAS_SNI and context.sni_callback is None:
304304
context.sni_callback = _sni_callback
305305

306-
def bind(self, sock):
307-
"""Wrap and return the given socket."""
308-
return super(BuiltinSSLAdapter, self).bind(sock)
309-
310306
def wrap(self, sock):
311307
"""Wrap and return the given socket, plus WSGI environ entries."""
312308
try:

‎cheroot/ssl/builtin.pyi‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ class BuiltinSSLAdapter(Adapter):
2020
def context(self): ...
2121
@context.setter
2222
def context(self, context) -> None: ...
23-
def bind(self, sock): ...
2423
def wrap(self, sock): ...
2524
def get_environ(self, sock): ...
2625
def makefile(self, sock, mode: str = ..., bufsize: int = ...): ...

‎cheroot/ssl/pyopenssl.py‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,8 @@ def __new__(mcl, name, bases, nmspc):
216216
'settimeout',
217217
'gettimeout',
218218
'shutdown',
219+
'recv_into',
220+
'_decref_socketios',
219221
)
220222
proxy_methods_no_args = ('shutdown',)
221223

@@ -270,6 +272,17 @@ def __init__(self, *args):
270272
self._ssl_conn = SSL.Connection(*args)
271273
self._lock = threading.RLock()
272274

275+
@property
276+
def _socket(self):
277+
"""
278+
Expose underlying raw socket.
279+
280+
This is needed for times when the cheroot server needs access to the
281+
original socket object, e.g. in replying to a client attempting
282+
to speak plain HTTP on an HTTPS port.
283+
"""
284+
return self._ssl_conn._socket
285+
273286

274287
class pyOpenSSLAdapter(Adapter):
275288
"""A wrapper for integrating :doc:`pyOpenSSL <pyopenssl:index>`."""
@@ -320,20 +333,18 @@ def __init__(
320333

321334
self._environ = None
322335

323-
def bind(self, sock):
324-
"""Wrap and return the given socket."""
325-
if self.context is None:
326-
self.context = self.get_context()
327-
conn = SSLConnection(self.context, sock)
328-
self._environ = self.get_environ()
329-
return conn
330-
331336
def wrap(self, sock):
332337
"""Wrap and return the given socket, plus WSGI environ entries."""
333338
# pyOpenSSL doesn't perform the handshake until the first read/write
334339
# forcing the handshake to complete tends to result in the connection
335340
# closing so we can't reliably access protocol/client cert for the env
336-
return sock, self._environ.copy()
341+
if self.context is None:
342+
self.context = self.get_context()
343+
conn = SSLConnection(self.context, sock)
344+
345+
conn.set_accept_state() # Tell OpenSSL this is a server connection
346+
self._environ = self.get_environ()
347+
return conn, self._environ
337348

338349
def _password_callback(
339350
self,
@@ -442,7 +453,9 @@ def makefile(self, sock, mode='r', bufsize=-1):
442453
if 'r' in mode
443454
else SSLFileobjectStreamWriter
444455
)
445-
if SSL and isinstance(sock, ssl_conn_type):
456+
# sock is an pyopenSSL.SSLConnection instance here
457+
# so checking against ssl_conn_type doesn't work
458+
if SSL and isinstance(sock, (ssl_conn_type, SSLConnection)):
446459
wrapped_socket = cls(sock, mode, bufsize)
447460
wrapped_socket.ssl_timeout = sock.gettimeout()
448461
return wrapped_socket

‎cheroot/ssl/pyopenssl.pyi‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ class pyOpenSSLAdapter(Adapter):
3434
*,
3535
private_key_password: str | bytes | None = ...,
3636
) -> None: ...
37-
def bind(self, sock): ...
3837
def wrap(self, sock): ...
3938
def _password_callback(
4039
self,

‎cheroot/test/test_ssl.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -920,16 +920,14 @@ def test_openssl_adapter_with_false_key_password(
920920
private_key_password=false_password,
921921
)
922922

923-
httpserver.ssl_adapter = tls_adapter
924-
925923
with expected_warn, pytest.raises(
926924
OpenSSL.SSL.Error,
927925
# Decode error has happened very rarely with Python 3.9 in MacOS.
928926
# Might be caused by a random issue in file handling leading
929927
# to interpretation of garbage characters in certificates.
930928
match=r'.+\'(bad decrypt|decode error)\'.+',
931929
):
932-
httpserver.prepare()
930+
tls_adapter.get_context()
933931

934932
assert not httpserver.requests._threads
935933
assert not httpserver.ready

0 commit comments

Comments
 (0)