Error SSL: BAD_LENGTH, setting enable_multithread would fix

[agavignet]

Hi,

We are often seeing the following SSL error with our application that uses socketio. It happens when stressing out (with parallel processes) the connection on the same server, unfortunately error can not be easily catched from the application.

Exception in thread Thread-6:
Traceback (most recent call last):
  File "/usr/lib/python3.6/threading.py", line 916, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.6/threading.py", line 864, in run
    self._target(*self._args, **self._kwargs)
  File "/home/foo/.local/lib/python3.6/site-packages/engineio/client.py", line 665, in _write_loop
    self.ws.send(encoded_packet)
  File "/home/foo/.local/lib/python3.6/site-packages/websocket/_core.py", line 253, in send
    return self.send_frame(frame)
  File "/home/foo/.local/lib/python3.6/site-packages/websocket/_core.py", line 278, in send_frame
    l = self._send(data)
  File "/home/foo/.local/lib/python3.6/site-packages/websocket/_core.py", line 448, in _send
    return send(self.sock, data)
  File "/home/foo/.local/lib/python3.6/site-packages/websocket/_socket.py", line 157, in send
    return _send()
  File "/home/foo/.local/lib/python3.6/site-packages/websocket/_socket.py", line 139, in _send
    return sock.send(data)
  File "/usr/lib/python3.6/ssl.py", line 944, in send
    return self._sslobj.write(data)
  File "/usr/lib/python3.6/ssl.py", line 642, in write
    return self._sslobj.write(data)
ssl.SSLError: [SSL: BAD_LENGTH] bad length (_ssl.c:2162)

It appears websocket.create_connection has an option enable_multithread. This ensures multithreading is correctly handled. See https://github.com/websocket-client/websocket-client/blob/master/websocket/_core.py#L462

So problem is solved when adding enable_multithread=True here https://github.com/miguelgrinberg/python-engineio/blob/master/engineio/client.py#L377

Which is called by socket.io for example here https://github.com/miguelgrinberg/python-socketio/blob/master/socketio/client.py#L276

One way of handling this would be to expose in eio a ws_options so we could set it:

self.client = socketio.Client(...)
self.client.eio.ws_options = {"enable_multithread": True}

Then engineio could create the websocket connection this way:

ws = websocket.create_connection(
    websocket_url + self._get_url_timestamp(), header=headers,
    cookie=cookies, **self.ws_options)

What do you think? Thanks

[miguelgrinberg]

This is only a partial solution, it will not work for events that carry binary data in their payloads.

The emit function is not thread-safe, to use it in a multithreaded application I recommend you create a critical section around all usages in your application.

[agavignet]

Thanks for you answer,

The emit function is not thread-safe, to use it in a multithreaded application I recommend you create a critical section around all usages in your application.

The thing is the application is not multithreaded, but several instances of it can run at the same time, so unless I'm wrong using critical sections would not help here. I might need to catch the exception and retry the connection in that case but I'm not sure which part I should surround with a try/except. Do you have any clue?

[miguelgrinberg]

Several instances (i.e. different processes) should not be a problem, because each client will have its own private connection to the server.

So let me ask you this. How did you end up finding that the multithreading option solves your issue if you are not doing multithreading?

[agavignet]

So let me ask you this. How did you end up finding that the multithreading option solves your issue if you are not doing multithreading?

I googled and read a bit about SSL BAD LENGTH errors and found this workaround in this issue websocket-client/websocket-client#591.

Even if the app is not multithreaded, actually I think problem is the same with multi processes. All processes connect to the same server (and often at the same time), so I think (but this is a hypothesis) that at a lower layer, TLS client does not handle multiple clients connection to one server and packets are not transmitted/received/queued safely.

[miguelgrinberg]

I still don't understand how this multithreading option helps in your case.

Network connections from different processes are completely independent, there is no collision there unless the problem is in some shared component, which I doubt. Even if it was, the multithreading option uses a lock, which prevents multiple calls from within a single process, there is no protection against multiple processes, so I don't understand how that option helps at all if you are not using multiple threads.

Can you provide a complete client and server example that I can use to test this problem?

[agavignet]

Can you provide a complete client and server example that I can use to test this problem?

Sure, I'll will be able to do it next week, thank you

[agavignet]

@miguelgrinberg Here we go, I pushed a minimal sample that reproduces the issue, the README explains how to run this sample https://github.com/agavignet/sample-socketio
Don't hesitate to ping me if something is not clear.
Thanks!

[miguelgrinberg]

I've made a small change to your client:

diff --git a/client.py b/client.py
index abf6a22..7e98dbd 100644
--- a/client.py
+++ b/client.py
@@ -54,4 +54,5 @@ class Client:
 if __name__ == "__main__":
     with Client() as sio:
         sio.wait_for_status_ok()
+    sio.client.wait()
     print("Done")

This is so that it waits until the client's internal state is clean before exiting.

With this change, the only error I occasionally see is the bad file descriptor. I do not see any other errors. This is probably a race condition during exit that I need to figure out. I see this error with or without the multithreading option, so I do not believe it is related.

[agavignet]

With this change, the only error I occasionally see is the bad file descriptor.

So without this change, do you confirm you see SSL BAD LENGTH or SSLEOFError or any others?
If so, that is a really good news, I will try your change.

Thank you

[agavignet]

Unfortunately after few tries I got a BAD_LENGHT error even with the change 😕

[agavignet]

@miguelgrinberg Few more points about the issue:

• I reproduce the issue even with only one client and as you know, there is no multithreading in my sample.
• websocket lib instanciates a lock in WebSocket constructor:

        if enable_multithread:
            self.lock = threading.Lock()
            self.readlock = threading.Lock()
        else:
            self.lock = NoLock()
            self.readlock = NoLock()

Those locks are used in relevant methods so there should not be edge cases activating it.

• I still don't understand why such errors happen. Reading websocket should default to enable_multithread=True websocket-client/websocket-client#591 enable_multithread helps a lot. I think my suggestion of having a way for engineio's user to set extra websocket creation arguments is fair. It does not change the current behavior of the library and add a way to handle this issue. Regarding the code, passing a generic **self.ws_options to create_connection can also factorize the code regarding the ssl_verify logic.

[miguelgrinberg]

The problem with adding this option is that if I add it, I need to implement it for the 7 or 8 or so platforms that I currently support. I cannot add this on the public interface and throw these options out on the floor on all other platforms.

The websocket client has an issue open for this, so I was hoping they would address it there.

[agavignet]

Thanks for your time and feeback @miguelgrinberg, I understand your point of view. I'm going to fork engineio and see what I can do about it. Closing this.