Skip to content

Commit 00bfcf8

Browse files
committed
Unified handling of http over https errors
Added logic including a new method ( _check_for_plain_http) in the Adapter base class to identify when a client attempts to speak unencrypted HTTP on an HTTPS port. When a plaintext connection is detected, the server now attempts to access the raw underlying TCP socket (if wrapped) to send a "400 Bad Request" error response before any TLS failure. This unifies the behavior of the 'builtin' and 'pyopenssl' adapters for this specific error condition.
1 parent a475500 commit 00bfcf8

7 files changed

Lines changed: 257 additions & 85 deletions

File tree

.flake8

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,11 @@ per-file-ignores =
124124
cheroot/__main__.py: WPS130
125125
cheroot/_compat.py: DAR101, DAR201, DAR301, DAR401, I003, RST304, WPS100, WPS111, WPS123, WPS226, WPS229, WPS420, WPS422, WPS432, WPS504, WPS505
126126
cheroot/cli.py: DAR101, DAR201, DAR401, I001, I004, I005, WPS100, WPS110, WPS120, WPS130, WPS202, WPS226, WPS229, WPS338, WPS420, WPS421
127-
cheroot/connections.py: DAR101, DAR201, DAR301, DAR401, I001, I003, I004, I005, RST304, S104, WPS100, WPS110, WPS111, WPS121, WPS122, WPS130, WPS201, WPS204, WPS210, WPS212, WPS214, WPS220, WPS229, WPS231, WPS301, WPS324, WPS338, WPS420, WPS421, WPS422, WPS432, WPS501, WPS504, WPS505
127+
cheroot/connections.py: DAR101, DAR201, DAR301, DAR401, I001, I003, I004, I005, RST304, S104, WPS100, WPS110, WPS111, WPS121, WPS122, WPS130, WPS201, WPS204, WPS210, WPS212, WPS214, WPS220, WPS229, WPS231, WPS237, WPS301, WPS324, WPS338, WPS420, WPS421, WPS422, WPS432, WPS501, WPS504, WPS505
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, WPS238, WPS338, WPS422, WPS501, WPS505, WPS529, WPS608, WPS612
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/connections.py

Lines changed: 50 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
"""Utilities to manage open connections."""
22

3-
import io
43
import os
54
import selectors
65
import socket
@@ -306,31 +305,9 @@ def _from_server_socket(self, server_socket): # noqa: C901 # FIXME
306305
f'{tls_connection_drop_error!s}',
307306
)
308307
return None
309-
except errors.NoSSLError as http_over_https_err:
310-
self.server.error_log(
311-
f'Client {addr!s} attempted to speak plain HTTP into '
312-
'a TCP connection configured for TLS-only traffic — '
313-
'trying to send back a plain HTTP error response: '
314-
f'{http_over_https_err!s}',
315-
)
316-
msg = (
317-
'The client sent a plain HTTP request, but '
318-
'this server only speaks HTTPS on this port.'
319-
)
320-
buf = [
321-
'%s 400 Bad Request\r\n' % self.server.protocol,
322-
'Content-Length: %s\r\n' % len(msg),
323-
'Content-Type: text/plain\r\n\r\n',
324-
msg,
325-
]
326-
327-
wfile = mf(s, 'wb', io.DEFAULT_BUFFER_SIZE)
328-
try:
329-
wfile.write(''.join(buf).encode('ISO-8859-1'))
330-
except OSError as ex:
331-
if ex.args[0] not in errors.socket_errors_to_ignore:
332-
raise
333-
return None
308+
except errors.NoSSLError:
309+
return self._send_bad_request_plain_http_error(s, addr)
310+
334311
mf = self.server.ssl_adapter.makefile
335312
# Re-apply our timeout since we may have a new socket object
336313
if hasattr(s, 'settimeout'):
@@ -403,3 +380,50 @@ def can_add_keepalive_connection(self):
403380
"""Flag whether it is allowed to add a new keep-alive connection."""
404381
ka_limit = self.server.keep_alive_conn_limit
405382
return ka_limit is None or self._num_connections < ka_limit
383+
384+
def _send_bad_request_plain_http_error(self, sock, addr):
385+
"""Send Bad Request 400 response, and close the socket."""
386+
self.server.error_log(
387+
f'Client {addr!s} attempted to speak plain HTTP into '
388+
'a TCP connection configured for TLS-only traffic — '
389+
'Sending 400 Bad Request.',
390+
)
391+
392+
msg = (
393+
'The client sent a plain HTTP request, but this server '
394+
'only speaks HTTPS on this port.'
395+
)
396+
397+
response_parts = [
398+
f'{self.server.protocol} 400 Bad Request\r\n',
399+
'Content-Type: text/plain\r\n',
400+
f'Content-Length: {len(msg)}\r\n',
401+
'Connection: close\r\n',
402+
'\r\n',
403+
msg,
404+
]
405+
response_bytes = ''.join(response_parts).encode('ISO-8859-1')
406+
407+
# The original socket (sock) is SSL-wrapped, but we must send the 400
408+
# response unencrypted over the raw TCP channel.
409+
sock_to_send = sock
410+
# Handle both raw sockets and SSL connections
411+
if hasattr(sock, 'do_handshake'):
412+
with suppress(AttributeError):
413+
# Access the raw, underlying TCP socket via the
414+
# common '._socket' attribute to bypass the SSL layer.
415+
# fall back to using the given socket directly if
416+
# the attribute does not exist.
417+
sock_to_send = sock._socket
418+
try:
419+
# Use the determined socket (raw TCP if successfully retrieved)
420+
sock_to_send.sendall(response_bytes)
421+
422+
# Shutdown the writing end of the socket used for sending.
423+
sock_to_send.shutdown(socket.SHUT_WR)
424+
425+
except OSError as ex:
426+
if ex.args[0] not in errors.socket_errors_to_ignore:
427+
raise
428+
429+
sock.close()

cheroot/ssl/__init__.py

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

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

56

@@ -50,3 +51,43 @@ def get_environ(self):
5051
def makefile(self, sock, mode='r', bufsize=-1):
5152
"""Return socket file object."""
5253
raise NotImplementedError # pragma: no cover
54+
55+
def _check_for_plain_http(self, raw_socket):
56+
"""Check if the client sent plain HTTP by peeking at first bytes.
57+
58+
This is a best-effort check to provide a helpful error message when
59+
clients accidentally use HTTP on an HTTPS port. If we can't detect
60+
plain HTTP (timeout, no data yet, etc), we return False and let the
61+
SSL handshake proceed, which will fail with its own error.
62+
63+
Returns:
64+
bool: True if plain HTTP is detected, False otherwise
65+
"""
66+
PEEK_BYTES = 16
67+
PEEK_TIMEOUT = 0.5
68+
69+
original_timeout = raw_socket.gettimeout()
70+
raw_socket.settimeout(PEEK_TIMEOUT)
71+
72+
try:
73+
first_bytes = raw_socket.recv(PEEK_BYTES, socket.MSG_PEEK)
74+
except (OSError, socket.timeout):
75+
return False
76+
finally:
77+
raw_socket.settimeout(original_timeout)
78+
79+
if not first_bytes:
80+
return False
81+
82+
http_methods = (
83+
b'GET ',
84+
b'POST ',
85+
b'PUT ',
86+
b'DELETE ',
87+
b'HEAD ',
88+
b'OPTIONS ',
89+
b'PATCH ',
90+
b'CONNECT ',
91+
b'TRACE ',
92+
)
93+
return first_bytes.startswith(http_methods)

cheroot/ssl/builtin.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,9 @@ def bind(self, sock):
309309

310310
def wrap(self, sock):
311311
"""Wrap and return the given socket, plus WSGI environ entries."""
312+
if self._check_for_plain_http(sock):
313+
raise errors.NoSSLError
314+
312315
try:
313316
s = self.context.wrap_socket(
314317
sock,

cheroot/ssl/pyopenssl.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,15 @@ def wrap(self, sock):
333333
# pyOpenSSL doesn't perform the handshake until the first read/write
334334
# forcing the handshake to complete tends to result in the connection
335335
# closing so we can't reliably access protocol/client cert for the env
336+
337+
# Get raw socket for plain HTTP check
338+
if isinstance(sock, ssl_conn_type):
339+
raw_sock = sock._socket
340+
else:
341+
raw_sock = sock
342+
343+
if self._check_for_plain_http(raw_sock):
344+
raise errors.NoSSLError
336345
return sock, self._environ.copy()
337346

338347
def _password_callback(

0 commit comments

Comments
 (0)