Skip to content

Commit

Permalink
Fix deadlocks due to invalid handling of SSL sockets and select
Browse files Browse the repository at this point in the history
  • Loading branch information
curdbecker committed Jan 10, 2022
1 parent dc34581 commit 0cf2d9b
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 4 deletions.
34 changes: 30 additions & 4 deletions websockify/websocketproxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,11 @@ def do_proxy(self, target):
else:
self.heartbeat = None

# see comment below - an assertion to deal with the
# intrinsics of non-blocking SSL sockets and select.
assert not isinstance(target, ssl.SSLSocket) \
or (self.buffer_size >= 16 * 1024 and not target.getblocking())

while True:
wlist = []

Expand Down Expand Up @@ -228,22 +233,43 @@ def do_proxy(self, target):
self.server.target_host, self.server.target_port)
raise self.CClose(closed['code'], closed['reason'])


if target in outs:
# Send queued client data to the target
dat = tqueue.pop(0)
sent = target.send(dat)
try:
sent = target.send(dat)
except ssl.SSLWantWriteError:
# nothing was sent - sending needs to be retried later
sent = 0

if sent == len(dat):
self.print_traffic(">")
else:
# requeue the remaining data
tqueue.insert(0, dat[sent:])
self.print_traffic(".>")


if target in ins:
# Receive target data, encode it and queue for client
buf = target.recv(self.buffer_size)
try:
# It is strictly required that buffer size is more than 16kb, so that
# all possible data can be received from a SSL socket in a single call.
# Otherwise, there could be still data available for reading, but select
# wouldn't report the socket as readable again, since all data has already
# been read by the SSL socket from the OS.
# see also: https://docs.python.org/3/library/ssl.html#notes-on-non-blocking-sockets

# The maximum size of a single SSL record is 16kb. And OpenSSL's SSL_read()
# will only return data from the current record - until it has been fully read.
# see also: https://www.openssl.org/docs/man1.1.1/man3/SSL_read.html
buf = target.recv(self.buffer_size)
except ssl.SSLWantReadError:
# The underlying OS socket had data, but there isn't any data to
# receive from the SSL socket yet (SSL record not yet fully received,
# only SSL control data received, e.g. re-handshake, session tickets, ...)
# Let's retry later.
continue

if len(buf) == 0:
if self.verbose:
self.log_message("%s:%s: Target closed connection",
Expand Down
4 changes: 4 additions & 0 deletions websockify/websockifyserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,10 @@ def socket(host, port=None, connect=False, prefer_ipv6=False,
sock.connect(addrs[0][4])
if use_ssl:
sock = ssl.wrap_socket(sock)
# SSL socket must be non-blocking in order to properly
# handle receiving and sending data using select.
# See also the comment(s) in ProxyRequestHandler.do_proxy()
sock.setblocking(0)
else:
sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
sock.bind(addrs[0][4])
Expand Down

0 comments on commit 0cf2d9b

Please sign in to comment.