Skip to content

Commit

Permalink
Merge PR #518
Browse files Browse the repository at this point in the history
This patch simplifies the handshake-time TLS error processing and
introduces support for newer exceptions that OpenSSL triggers under
Python 3.8 and higher.
  • Loading branch information
webknjaz committed Jan 24, 2024
2 parents ba807ac + 93d8379 commit 0fd16f0
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 59 deletions.
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, {}
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

0 comments on commit 0fd16f0

Please sign in to comment.