Subtle error in do_handshake_on_connect handling can potentially compromise curio.ssl's security

[njsmith]

When curio.ssl creates an ssl-wrapped socket, it always passes do_handshake_on_connect=False to the underlying ssl module, and also makes a record of what the user actually requested, and uses this to attempt to reimplement the semantics on top. Specifically, the Socket.connect method checks whether the user requested do_handshake_on_connect=True, and if so then it calls do_handshake at that time.

However, two things:

• The real do_handshake_on_connect is somewhat misnamed: if you read Lib/ssl.py you'll see that it actually triggers do_handshake in two circumstances: (a) on connect, (b) when an already-connected socket is wrapped. AFAICT curio only implements the first case.

• If you use an ssl-wrapped socket that has do_handshake_on_connect=False, and don't call do_handshake, then everything will appear to work but SSL's security is silently compromised (see bpo-30141). Yeah, it's pretty awesome.

These two things together mean that if you perform the following sequence of actions:

• create a socket
• connect it, THEN wrap it with do_handshake_on_connect=True (the default)
• never call do_handshake

then with the stdlib ssl module you're safe, but with curio now you're silently vulnerable to MITM attacks.

Fortunately it looks like curio's open_connection and open_unix_connection utility functions do unconditionally call do_handshake, so I don't think they're affected. But probably curio's wrap_socket function and method should grow some logic for calling do_handshake if do_handshake_on_connect=True and they're passed a socket that's already connected. (I guess this also means wrap_socket needs to become async.)

[dabeaz]

Ah. Good catch. I'll take a look at it.

[dabeaz]

Pushed a fix that changes wrap_socket to an async method and patches the network related functions accordingly. Still needs a unit test for the specific case mentioned here.