Merge pull request #800 from julianz-/http_https_handling

Unify handling of http over https errors

Author: webknjaz

cheroot/connections.py

@@ -1,12 +1,13 @@
 """Utilities to manage open connections."""
 
-import io
+import contextlib as _cm
 import os
 import selectors
 import socket
 import threading
 import time
-from contextlib import suppress
+from http import HTTPStatus as _HTTPStatus
+from http.client import responses as _http_responses
 
 from . import errors
 from ._compat import IS_WINDOWS
@@ -112,6 +113,16 @@ def close(self):
             self._selector.close()
 
 
+@_cm.contextmanager
+def _suppress_socket_io_errors(socket, /):
+    """Suppress known socket I/O errors."""
+    try:
+        yield
+    except OSError as ex:
+        if ex.args[0] not in errors.socket_errors_to_ignore:
+            raise
+
+
 class ConnectionManager:
     """Class which manages HTTPConnection objects.
 
@@ -281,7 +292,7 @@ def _remove_invalid_sockets(self):
             # One of the reason on why a socket could cause an error
             # is that the socket is already closed, ignore the
             # socket error if we try to close it at this point.
-            with suppress(OSError):
+            with _cm.suppress(OSError):
                 conn.close()
 
     def _from_server_socket(self, server_socket):  # noqa: C901  # FIXME
@@ -297,7 +308,8 @@ def _from_server_socket(self, server_socket):  # noqa: C901  # FIXME
             ssl_env = {}
             # if ssl cert and key are set, we try to be a secure HTTP server
             if self.server.ssl_adapter is not None:
-                try:
+                # FIXME: WPS505 -- too many nested blocks
+                try:  # noqa: WPS505
                     s, ssl_env = self.server.ssl_adapter.wrap(s)
                 except errors.FatalSSLAlert as tls_connection_drop_error:
                     self.server.error_log(
@@ -313,23 +325,7 @@ def _from_server_socket(self, server_socket):  # noqa: C901  # FIXME
                         '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.'
-                    )
-                    buf = [
-                        '%s 400 Bad Request\r\n' % self.server.protocol,
-                        'Content-Length: %s\r\n' % len(msg),
-                        'Content-Type: text/plain\r\n\r\n',
-                        msg,
-                    ]
-
-                    wfile = mf(s, 'wb', io.DEFAULT_BUFFER_SIZE)
-                    try:
-                        wfile.write(''.join(buf).encode('ISO-8859-1'))
-                    except OSError as ex:
-                        if ex.args[0] not in errors.socket_errors_to_ignore:
-                            raise
+                    self._send_bad_request_plain_http_error(s)
                     return None
                 mf = self.server.ssl_adapter.makefile
                 # Re-apply our timeout since we may have a new socket object
@@ -403,3 +399,31 @@ def can_add_keepalive_connection(self):
         """Flag whether it is allowed to add a new keep-alive connection."""
         ka_limit = self.server.keep_alive_conn_limit
         return ka_limit is None or self._num_connections < ka_limit
+
+    def _send_bad_request_plain_http_error(self, raw_sock, /):
+        """Send Bad Request 400 response, and close the socket."""
+        msg = (
+            'The client sent a plain HTTP request, but this server '
+            'only speaks HTTPS on this port.'
+        )
+
+        http_response_status_line = ' '.join(
+            (
+                self.server.protocol,
+                str(_HTTPStatus.BAD_REQUEST.value),
+                _http_responses[_HTTPStatus.BAD_REQUEST],
+            ),
+        )
+        response_parts = [
+            f'{http_response_status_line}\r\n',
+            'Content-Type: text/plain\r\n',
+            f'Content-Length: {len(msg)}\r\n',
+            'Connection: close\r\n',
+            '\r\n',
+            msg,
+        ]
+        response_bytes = ''.join(response_parts).encode('ISO-8859-1')
+
+        with _suppress_socket_io_errors(raw_sock), _cm.closing(raw_sock):
+            raw_sock.sendall(response_bytes)
+            raw_sock.shutdown(socket.SHUT_WR)

cheroot/ssl/__init__.py

@@ -1,8 +1,58 @@
 """Implementation of the SSL adapter base interface."""
 
+import socket as _socket
 from abc import ABC, abstractmethod
 from warnings import warn as _warn
 
+from .. import errors as _errors
+
+
+def _ensure_peer_speaks_https(raw_socket, /) -> None:
+    """
+    Raise exception if the client sent plain HTTP.
+
+    This method probes the TCP stream for signs of the peer having
+    sent us plaintext HTTP on the HTTPS port by peeking at the
+    first bytes. If there's no data yet, the method considers the
+    guess inconclusive and does not error out. This allows the server
+    to continue until the SSL handshake is attempted, at which point
+    an error will be caught by the SSL layer if the client
+    is not speaking TLS.
+
+    :raises NoSSLError: When plaintext HTTP is detected on an HTTPS socket
+    """
+    PEEK_BYTES = 16
+    PEEK_TIMEOUT = 0.5
+
+    original_timeout = raw_socket.gettimeout()
+    raw_socket.settimeout(PEEK_TIMEOUT)
+
+    try:
+        first_bytes = raw_socket.recv(PEEK_BYTES, _socket.MSG_PEEK)
+    except (OSError, _socket.timeout):
+        return
+    finally:
+        raw_socket.settimeout(original_timeout)
+
+    if not first_bytes:
+        return
+
+    http_methods = (
+        b'GET ',
+        b'POST ',
+        b'PUT ',
+        b'DELETE ',
+        b'HEAD ',
+        b'OPTIONS ',
+        b'PATCH ',
+        b'CONNECT ',
+        b'TRACE ',
+    )
+    if first_bytes.startswith(http_methods):
+        raise _errors.NoSSLError(
+            'Expected HTTPS on the socket but got plain HTTP',
+        ) from None
+
 
 class Adapter(ABC):
     """Base class for SSL driver library adapters.
@@ -51,7 +101,8 @@ def bind(self, sock):
     @abstractmethod
     def wrap(self, sock):
         """Wrap the given socket and return WSGI environ entries."""
-        raise NotImplementedError  # pragma: no cover
+        _ensure_peer_speaks_https(sock)
+        return sock, {}
 
     @abstractmethod
     def get_environ(self):

cheroot/ssl/builtin.py

@@ -305,6 +305,10 @@ def context(self, context):
 
     def wrap(self, sock):
         """Wrap and return the given socket, plus WSGI environ entries."""
+        sock, _env = super().wrap(  # checks for plaintext http on https port
+            sock,
+        )
+
         try:
             s = self.context.wrap_socket(
                 sock,

cheroot/ssl/pyopenssl.py

@@ -339,6 +339,11 @@ def wrap(self, sock):
         # 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
+
+        sock, _env = super().wrap(  # checks for plaintext http on https port
+            sock,
+        )
+
         conn = SSLConnection(self.context, sock)
 
         conn.set_accept_state()  # Tell OpenSSL this is a server connection