Skip to content
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

BaseIOStream.read_bytes may raise incorrect exception type with OpenSSL 3.4.0 #3443

Open
fjetter opened this issue Nov 14, 2024 · 3 comments

Comments

@fjetter
Copy link

fjetter commented Nov 14, 2024

In dask/distributed we're seeing a wrong exception being raised in one of our tests, see dask/distributed#8936

The test is closing the stream server side just after the connection has been established. We then assert that the client side notices this and is raising an appropriate error.

This issue seems to be linked to the new openssl version 3.4.0. With the new version we're seeing a ssl.SSLError: [SYS] unknown error (_ssl.c:2580) instead of a StreamClosedError

I don't have a minimal (tornado only) reproducer, yet. We can deal with this by loosening our exception handling but I wondered if there is something tornado can / wants to do

@bdarnell
Copy link
Member

This sounds similar to #3357 (fixed in Tornado 6.4.1, can you confirm if you're already using that version?)

We fixed that issue by altering one of our except: blocks in _do_ssl_handshake. Sounds like we need to do that somewhere else too. It's not immediately obvious where that should be; do you have a full stack trace?

Or maybe the cpython _ssl.c needs to be updated to handle some new error code instead of saying "unknown error".

@jrbourbeau
Copy link
Contributor

jrbourbeau commented Nov 22, 2024

FWIW distributed/comm/tests/test_comms.py::test_tls_comm_closed_implicit[tornado] is the specific test over in distributed that's failing. Here's an example CI build with the failure https://github.com/dask/distributed/actions/runs/11938506353/job/33276952764 and here is the full traceback

Traceback:
____________________ test_tls_comm_closed_implicit[tornado] ____________________

tcp = <module 'distributed.comm.tcp' from '/home/runner/work/distributed/distributed/distributed/comm/tcp.py'>

    @gen_test()
    async def test_tls_comm_closed_implicit(tcp):
>       await check_comm_closed_implicit("tls://127.0.0.1", **tls_kwargs)

distributed/comm/tests/test_comms.py:777: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
distributed/comm/tests/test_comms.py:763: in check_comm_closed_implicit
    await comm.read()
distributed/comm/tcp.py:225: in read
    frames_nosplit_nbytes_bin = await stream.read_bytes(fmt_size)
../../../miniconda3/envs/dask-distributed/lib/python3.11/site-packages/tornado/iostream.py:422: in read_bytes
    self._try_inline_read()
../../../miniconda3/envs/dask-distributed/lib/python3.11/site-packages/tornado/iostream.py:836: in _try_inline_read
    pos = self._read_to_buffer_loop()
../../../miniconda3/envs/dask-distributed/lib/python3.11/site-packages/tornado/iostream.py:750: in _read_to_buffer_loop
    if self._read_to_buffer() == 0:
../../../miniconda3/envs/dask-distributed/lib/python3.11/site-packages/tornado/iostream.py:861: in _read_to_buffer
    bytes_read = self.read_from_fd(buf)
../../../miniconda3/envs/dask-distributed/lib/python3.11/site-packages/tornado/iostream.py:1552: in read_from_fd
    return self.socket.recv_into(buf, len(buf))
../../../miniconda3/envs/dask-distributed/lib/python3.11/ssl.py:1314: in recv_into
    return self.read(nbytes, buffer)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <ssl.SSLSocket [closed] fd=-1, family=2, type=1, proto=0>, len = 65536
buffer = bytearray(b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x...0\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00')

    def read(self, len=1024, buffer=None):
        """Read up to LEN bytes and return them.
        Return zero-length string on EOF."""
    
        self._checkClosed()
        if self._sslobj is None:
            raise ValueError("Read on closed or unwrapped SSL socket.")
        try:
            if buffer is not None:
>               return self._sslobj.read(len, buffer)
E               ssl.SSLError: [SYS] unknown error (_ssl.c:2580)

../../../miniconda3/envs/dask-distributed/lib/python3.11/ssl.py:1166: SSLError

@jrbourbeau
Copy link
Contributor

Oh, also can confirm that tornado=6.4.1 is being used in that CI build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants