Opened 7 years ago

Last modified 7 years ago

#6782 defect new

— at TLS handshake code is wrongInitial Version

Reported by: Andy Lutomirski Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: habnabit Branch:
Author:

Description

Quoting from the manpage for SSL_do_handshake:

 If the underlying BIO is non-blocking, SSL_do_handshake() will also return when the underlying BIO could not satisfy the needs of SSL_do_handshake() to
       continue the handshake. In this case a call to SSL_get_error() with the return value of SSL_do_handshake() will yield SSL_ERROR_WANT_READ or
       SSL_ERROR_WANT_WRITE. The calling process then must repeat the call after taking appropriate action to satisfy the needs of SSL_do_handshake().

The code in twisted.protocols.tls is completely wrong, and I suspect that it only works at all because OpenSSL is buggy. If we fix it, then it becomes possible to fix #6204 (because we'll actually know when the handshake is done) and we can actually report handshake errors correctly (although getting them logged in, say, a web server will require forwarding them to the underlying protocol).

Here's a partial fix. It appears to work. I don't know how to write test cases for it without resorting to some kind of end-to-end test.

_flushReceiveBIO is still wrong, but it's no worse than before. (It should handle WantWriteError.) Simiarly, _write is busted (and this may be a real-world problem if programs write more than 215 bytes in one go after the handshake is done).

If something like this is applied, it's probably worth calling pauseProducing at startup and then resuming when the handshake is done.

Change History (1)

Changed 7 years ago by Andy Lutomirski

Attachment: tls.patch added

Partial fix

Note: See TracTickets for help on using tickets.