-
-
Notifications
You must be signed in to change notification settings - Fork 99
move ssl_handshake handling from main thread to worker threads #750
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -276,7 +276,7 @@ def wrap(self, sock): | |
try: | ||
s = self.context.wrap_socket( | ||
sock, | ||
do_handshake_on_connect=True, | ||
do_handshake_on_connect=False, | ||
server_side=True, | ||
) | ||
except ( | ||
|
@@ -306,6 +306,97 @@ def wrap(self, sock): | |
|
||
return s, self.get_environ(s) | ||
|
||
def _handle_plain_http_error(self, wfile, buf): | ||
try: | ||
wfile.write(buf) | ||
except OSError as ex: | ||
if ex.args[0] not in errors.socket_errors_to_ignore: | ||
raise | ||
|
||
def _handle_handshake_failure(self, conn): | ||
try: | ||
conn.socket.shutdown(socket.SHUT_RDWR) | ||
except Exception: | ||
# pass | ||
return | ||
|
||
def do_handshake(self, conn): | ||
"""Process SSL handshake on connection if needed. | ||
|
||
Args: | ||
conn (:py:class:`~cheroot.server.HTTPConnection`): HTTP connection | ||
""" | ||
ssl_handshake_must_be_done = ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nitpick] This Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||
conn | ||
and getattr(conn, 'socket', None) | ||
and getattr(conn.socket, 'do_handshake', None) | ||
and not getattr(conn.socket, 'cheroot_handshake_done', False) | ||
) | ||
if ssl_handshake_must_be_done: | ||
conn.socket.cheroot_handshake_done = True | ||
do_shutdown = False | ||
try: | ||
conn.socket.do_handshake() | ||
except ssl.SSLError as generic_tls_error: | ||
do_shutdown = True | ||
|
||
# Try to send HTTP 400 status for plain text queries | ||
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: | ||
msg = ( | ||
'The client %s:%s sent a plain HTTP request, but ' | ||
'this server only speaks HTTPS on this port.' | ||
) | ||
msg = msg % (conn.remote_addr, conn.remote_port) | ||
buf = [ | ||
'%s 400 Bad Request\r\n' % conn.server.protocol, | ||
'Content-Length: %s\r\n' % len(msg), | ||
'Content-Type: text/plain\r\n\r\n', | ||
msg, | ||
] | ||
conn.server.error_log(msg) | ||
|
||
# - writing directly on conn.socket attempt to use | ||
# non-initialized SSL layer and raises exception: | ||
# EOF occurred in violation of protocol (_ssl.c:2427) | ||
# - conn.socket.unwrap fails raising exception: | ||
# [SSL: SHUTDOWN_WHILE_IN_INIT] shutdown while in | ||
# init (_ssl.c:2706) | ||
# - we create a non SSL socket to send plain text reply | ||
conn.socket = socket.socket(fileno=conn.socket.detach()) | ||
wfile = StreamWriter( | ||
conn.socket, | ||
'wb', | ||
DEFAULT_BUFFER_SIZE, | ||
) | ||
buf = ''.join(buf).encode('ISO-8859-1') | ||
self._handle_plain_http_error(wfile, buf) | ||
else: | ||
msg = 'SSL handshake for %s:%s failed with SSL error:%s' | ||
msg = msg % ( | ||
conn.remote_addr, | ||
conn.remote_port, | ||
generic_tls_error, | ||
) | ||
conn.server.error_log(msg) | ||
|
||
except Exception as generic_error: | ||
do_shutdown = True | ||
|
||
msg = 'SSL handshake for %s:%s failed with error:%s' | ||
msg = msg % (conn.remote_addr, conn.remote_port, generic_error) | ||
conn.server.error_log(msg) | ||
|
||
finally: | ||
if do_shutdown: | ||
self._handle_handshake_failure(conn) | ||
|
||
def get_environ(self, sock): | ||
"""Create WSGI environ entries to be merged into each request.""" | ||
cipher = sock.cipher() | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -276,7 +276,6 @@ def __init__( | |||||
self._threads = [] | ||||||
self._queue = queue.Queue(maxsize=accepted_queue_size) | ||||||
self._queue_put_timeout = accepted_queue_timeout | ||||||
self.get = self._queue.get | ||||||
self._pending_shutdowns = collections.deque() | ||||||
|
||||||
def start(self): | ||||||
|
@@ -294,6 +293,24 @@ def idle(self): # noqa: D401; irrelevant for properties | |||||
idles = len([t for t in self._threads if t.conn is None]) | ||||||
return max(idles - len(self._pending_shutdowns), 0) | ||||||
|
||||||
def get(self): | ||||||
"""Get request from queue, and process SSL handshake is needed. | ||||||
|
||||||
Return: | ||||||
conn (:py:class:`~cheroot.server.HTTPConnection`): HTTP connection | ||||||
ready to be processed | ||||||
|
||||||
""" | ||||||
conn = self._queue.get() | ||||||
ssl_adapter = self.server.ssl_adapter | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Accessing
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
check_for_ssl_handshake = ( | ||||||
ssl_adapter is not None | ||||||
and getattr(ssl_adapter, 'do_handshake', None) is not None | ||||||
) | ||||||
if check_for_ssl_handshake: | ||||||
ssl_adapter.do_handshake(conn) | ||||||
return conn | ||||||
|
||||||
def put(self, obj): | ||||||
"""Put request into queue. | ||||||
|
||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commented-out
# pass
line can be removed to clean up the code and avoid confusion about whether this exception block is intentional.Copilot uses AI. Check for mistakes.