From eb92823498642b90e8060baf48ab21ac7714c2da Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko <wk@sydorenko.org.ua> Date: Wed, 24 Jan 2024 04:23:01 +0100 Subject: [PATCH] Relay TLS connection drop through `FatalSSLAlert` Specifically, this patch adds an exception interception code to the place where the socket is being first wrapped. In case of the `builtin` TLS adapter, this is also a place where a handshake attempt happens. This switches the method of relaying an unrecoverable connection error from a sentinel of a tuple with two falsy values to raising a unified exception consistently. --- cheroot/connections.py | 9 +++++++-- cheroot/ssl/builtin.py | 32 +++++++++++++++++++------------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/cheroot/connections.py b/cheroot/connections.py index 9346bc6aea..9ea6c829f2 100644 --- a/cheroot/connections.py +++ b/cheroot/connections.py @@ -292,6 +292,13 @@ 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.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: msg = ( 'The client sent a plain HTTP request, but ' @@ -311,8 +318,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'): diff --git a/cheroot/ssl/builtin.py b/cheroot/ssl/builtin.py index b128c858ef..e28e5df188 100644 --- a/cheroot/ssl/builtin.py +++ b/cheroot/ssl/builtin.py @@ -30,8 +30,6 @@ 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.""" @@ -264,7 +262,6 @@ 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, @@ -276,16 +273,25 @@ def wrap(self, sock): raise errors.FatalSSLAlert( *tls_connection_drop_error.args, ) from tls_connection_drop_error - except ssl.SSLError as ex: - if 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 - except generic_socket_error: - pass - else: - return s, self.get_environ(s) - return EMPTY_RESULT + 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): """Create WSGI environ entries to be merged into each request."""