Skip to content

Commit

Permalink
Fix some test failures on openssl 1.1.1 / Ubuntu 18.10
Browse files Browse the repository at this point in the history
See python-trio#817

We don't have CI for this yet, but at least on my laptop it helps.
However, it doesn't fix everything.

- Add OP_ENABLE_MIDDLEBOX_COMPAT to the list of static ssl constants
- Handle SSLZeroReturnError in more places
- Fix some -- but not all -- of the places where session tickets cause
  failures. See python-trio#819
  • Loading branch information
njsmith committed Dec 23, 2018
1 parent 5a03bee commit 03bfa3b
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 13 deletions.
17 changes: 14 additions & 3 deletions trio/_ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,9 @@ async def receive_some(self, max_bytes):
# For some reason, EOF before handshake sometimes raises
# SSLSyscallError instead of SSLEOFError (e.g. on my linux
# laptop, but not on appveyor). Thanks openssl.
if (
if isinstance(exc.__cause__, _stdlib_ssl.SSLZeroReturnError):
return b""
elif (
self._https_compatible and isinstance(
exc.__cause__,
(_stdlib_ssl.SSLEOFError, _stdlib_ssl.SSLSyscallError)
Expand All @@ -625,9 +627,18 @@ async def receive_some(self, max_bytes):
# BROKEN. But that's actually fine, because after getting an
# EOF on TLS then the only thing you can do is close the
# stream, and closing doesn't care about the state.
#
# SSLZeroReturnError means the TLS layer shut down cleanly, so
# that's always an application-level EOF.
# SSLEOFError means that the transport closed without the TLS
# layer shutting down cleanly, so that's an application-level
# EOF iff we're in https-compatible mode.
if (
self._https_compatible
and isinstance(exc.__cause__, _stdlib_ssl.SSLEOFError)
isinstance(exc.__cause__, _stdlib_ssl.SSLZeroReturnError)
or (
self._https_compatible
and isinstance(exc.__cause__, _stdlib_ssl.SSLEOFError)
)
):
return b""
else:
Expand Down
3 changes: 2 additions & 1 deletion trio/ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@
SSL_ERROR_SYSCALL, SSL_ERROR_WANT_CONNECT, SSL_ERROR_WANT_READ,
SSL_ERROR_WANT_WRITE, SSL_ERROR_WANT_X509_LOOKUP,
SSL_ERROR_ZERO_RETURN, VERIFY_CRL_CHECK_CHAIN, VERIFY_CRL_CHECK_LEAF,
VERIFY_DEFAULT, VERIFY_X509_STRICT, VERIFY_X509_TRUSTED_FIRST
VERIFY_DEFAULT, VERIFY_X509_STRICT, VERIFY_X509_TRUSTED_FIRST,
OP_ENABLE_MIDDLEBOX_COMPAT
)
except ImportError:
pass
Expand Down
40 changes: 31 additions & 9 deletions trio/tests/test_ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def ssl_echo_serve_sync(sock, *, expect_fail=False):
# other side has initiated a graceful shutdown; we try to
# respond in kind but it's legal for them to have already gone
# away.
exceptions = (BrokenPipeError,)
exceptions = (BrokenPipeError, stdlib_ssl.SSLZeroReturnError)
# Under unclear conditions, CPython sometimes raises
# SSLWantWriteError here. This is a bug (bpo-32219), but it's
# not our bug, so ignore it.
Expand All @@ -86,6 +86,7 @@ def ssl_echo_serve_sync(sock, *, expect_fail=False):
if expect_fail:
print("ssl_echo_serve_sync got error as expected:", exc)
else: # pragma: no cover
print("ssl_echo_serve_sync got unexpected error:", exc)
raise
else:
if expect_fail: # pragma: no cover
Expand Down Expand Up @@ -760,17 +761,34 @@ async def stream_maker():

async def clogged_stream_maker():
client, server = ssl_lockstep_stream_pair()

# If we don't do handshakes up front, then we run into a problem in
# the following situation:
# - server does wait_send_all_might_not_block
# - client does receive_some to unclog it
# Then the client's receive_some will actually send some data to start
# the handshake, and itself get stuck.
#
# And then we push a bit of application-level data from the
# server->client, in order to clear out any TLS 1.3 session tickets
# that could otherwise cause things to hang:
# https://github.com/python-trio/trio/issues/819
async def client_handshake():
await client.do_handshake()
assert await client.receive_some(1) == b"x"

async def server_handshake():
await server.do_handshake()
await server.send_all(b"x")

async with _core.open_nursery() as nursery:
nursery.start_soon(client.do_handshake)
nursery.start_soon(server.do_handshake)
nursery.start_soon(client_handshake)
nursery.start_soon(server_handshake)
return client, server

with trio.fail_after(2):
await clogged_stream_maker()

await check_two_way_stream(stream_maker, clogged_stream_maker)


Expand Down Expand Up @@ -880,18 +898,22 @@ async def server_closer():
await client_ssl.do_handshake()

# Check that a graceful close *before* handshaking gives a clean EOF on
# the other side
# the other side.
# Unfortunately with openssl 1.1.1 this can't work reliably if the client
# calls aclose() and the server calls receive_some():
# https://github.com/python-trio/trio/issues/819
# so, we do it the other way around.
client_ssl, server_ssl = ssl_memory_stream_pair()

async def expect_eof_server():
async def expect_eof_client():
with assert_checkpoints():
assert await server_ssl.receive_some(10) == b""
assert await client_ssl.receive_some(10) == b""
with assert_checkpoints():
await server_ssl.aclose()
await client_ssl.aclose()

async with _core.open_nursery() as nursery:
nursery.start_soon(client_ssl.aclose)
nursery.start_soon(expect_eof_server)
nursery.start_soon(server_ssl.aclose)
nursery.start_soon(expect_eof_client)


async def test_send_all_fails_in_the_middle():
Expand Down

0 comments on commit 03bfa3b

Please sign in to comment.