Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle SSLZeroReturnError exceptions in stdlib TLS adapter #518

Merged
5 commits merged into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions cheroot/connections.py
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,20 @@ def _from_server_socket(self, server_socket): # noqa: C901 # FIXME
if self.server.ssl_adapter is not None:
try:
s, ssl_env = self.server.ssl_adapter.wrap(s)
except errors.NoSSLError:
except errors.FatalSSLAlert as tls_connection_drop_error:
self.server.error_log(
f'Client {addr !s} lost β€” peer dropped the TLS '
'connection suddenly, during handshake: '
f'{tls_connection_drop_error !s}',
)
return
except errors.NoSSLError as http_over_https_err:
self.server.error_log(
f'Client {addr !s} attempted to speak plain HTTP into '
'a TCP connection configured for TLS-only traffic β€” '
'trying to send back a plain HTTP error response: '
f'{http_over_https_err !s}',
)
msg = (
'The client sent a plain HTTP request, but '
'this server only speaks HTTPS on this port.'
Expand All @@ -311,8 +324,6 @@ def _from_server_socket(self, server_socket): # noqa: C901 # FIXME
if ex.args[0] not in errors.socket_errors_to_ignore:
raise
return
if not s:
return
mf = self.server.ssl_adapter.makefile
# Re-apply our timeout since we may have a new socket object
if hasattr(s, 'settimeout'):
Expand Down
80 changes: 25 additions & 55 deletions cheroot/ssl/builtin.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,9 @@

from . import Adapter
from .. import errors
from .._compat import IS_ABOVE_OPENSSL10
from ..makefile import StreamReader, StreamWriter
from ..server import HTTPServer

generic_socket_error = OSError


def _assert_ssl_exc_contains(exc, *msgs):
"""Check whether SSL exception contains either of messages provided."""
Expand Down Expand Up @@ -265,62 +262,35 @@ def bind(self, sock):

def wrap(self, sock):
"""Wrap and return the given socket, plus WSGI environ entries."""
EMPTY_RESULT = None, {}
toppk marked this conversation as resolved.
Show resolved Hide resolved
try:
s = self.context.wrap_socket(
sock, do_handshake_on_connect=True, server_side=True,
)
except ssl.SSLError as ex:
if ex.errno == ssl.SSL_ERROR_EOF:
# This is almost certainly due to the cherrypy engine
# 'pinging' the socket to assert it's connectable;
# the 'ping' isn't SSL.
return EMPTY_RESULT
elif ex.errno == ssl.SSL_ERROR_SSL:
if _assert_ssl_exc_contains(ex, 'http request'):
# The client is speaking HTTP to an HTTPS server.
raise errors.NoSSLError

# Check if it's one of the known errors
# Errors that are caught by PyOpenSSL, but thrown by
# built-in ssl
_block_errors = (
'unknown protocol', 'unknown ca', 'unknown_ca',
'unknown error',
'https proxy request', 'inappropriate fallback',
'wrong version number',
'no shared cipher', 'certificate unknown',
'ccs received early',
'certificate verify failed', # client cert w/o trusted CA
'version too low', # caused by SSL3 connections
'unsupported protocol', # caused by TLS1 connections
)
if _assert_ssl_exc_contains(ex, *_block_errors):
# Accepted error, let's pass
return EMPTY_RESULT
elif _assert_ssl_exc_contains(ex, 'handshake operation timed out'):
# This error is thrown by builtin SSL after a timeout
# when client is speaking HTTP to an HTTPS server.
# The connection can safely be dropped.
return EMPTY_RESULT
raise
except generic_socket_error as exc:
"""It is unclear why exactly this happens.

It's reproducible only with openssl>1.0 and stdlib
:py:mod:`ssl` wrapper.
In CherryPy it's triggered by Checker plugin, which connects
to the app listening to the socket port in TLS mode via plain
HTTP during startup (from the same process).


Ref: https://github.com/cherrypy/cherrypy/issues/1618
"""
is_error0 = exc.args == (0, 'Error')

if is_error0 and IS_ABOVE_OPENSSL10:
return EMPTY_RESULT
raise
except (
ssl.SSLEOFError,
ssl.SSLZeroReturnError,
) as tls_connection_drop_error:
raise errors.FatalSSLAlert(
*tls_connection_drop_error.args,
) from tls_connection_drop_error
except ssl.SSLError as generic_tls_error:
peer_speaks_plain_http_over_https = (
generic_tls_error.errno == ssl.SSL_ERROR_SSL and
_assert_ssl_exc_contains(generic_tls_error, 'http request')
)
if peer_speaks_plain_http_over_https:
reraised_connection_drop_exc_cls = errors.NoSSLError
else:
reraised_connection_drop_exc_cls = errors.FatalSSLAlert

raise reraised_connection_drop_exc_cls(
*generic_tls_error.args,
) from generic_tls_error
except OSError as tcp_connection_drop_error:
raise errors.FatalSSLAlert(
*tcp_connection_drop_error.args,
) from tcp_connection_drop_error

return s, self.get_environ(s)

def get_environ(self, sock):
Expand Down
1 change: 0 additions & 1 deletion stubtest_allowlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ cheroot.ssl.pyopenssl.SSLConnection.write

# false positives (https://github.com/python/mypy/issues/11843)
cheroot.connections.IS_WINDOWS
cheroot.ssl.builtin.IS_ABOVE_OPENSSL10

# suppress is both a function and class
cheroot._compat.suppress
Expand Down