Basically what it says in the title... if you create an SSL object via wrap_socket with do_handshake_on_connect=False, or via wrap_bio, and then forget to call do_handshake and just go straight to sending and receiving data, then the encrypted connection is successfully established and everything seems to work. However, in this mode the hostname is silently *not* checked, so the connection is vulnerable to MITM attacks.

(I guess from reading the SSL_read and SSL_write manpages that openssl is just silently doing the handshake automatically – very helpfully! – but it's only Python's do_handshake code that knows to check the hostname?)

This doesn't affect correctly written programs that follow the documentation and either use do_handshake_on_connect=True (the default for wrap_socket) or explicitly call do_handshake, so it's not a super-scary bug. But IMHO it definitely shouldn't be this easy to silently fail-open.

The attached test script sets up a TLS echo server that has a certificate for the host "trio-test-1.example.org" that's signed by a locally trusted CA, and then checks:

• connecting to it with do_handshake and expecting the correct hostname: works, as expected
• connecting to it with do_handshake and expecting a different hostname: correctly raises an error due to the mismatched hostnames
• connecting to it withOUT do_handshake and expecting a different hostname: incorrectly succeeds at connecting, sending data, receiving data, etc., without any error

and it checks using both ctx.wrap_socket(..., do_handshake_on_connect=False) and a little custom socket wrapper class defined using ctx.wrap_bio(...).

I've only marked 3.5 and 3.6 as affected because those are the only versions I've tested, but I suspect other versions are affected as well.

Sigh, this is the seventh or eight security issue related to Python's hostname verification, maybe more. I know for years that Python's current approach is buggy and a collection of bad ideas. That's it, I'm going to rip out ssl.match_hostname() and let OpenSSL handle all verification internally. I've been working on another PEP that features the change for quite some time. I'll to finish my SSL PEP before PyCon and language summit.

Here is a quick proof-of-concept implementation (requires OpenSSL >= 1.0.2 and libressl >= 2.5).

https://github.com/tiran/cpython/tree/openssl_check_hostname

The PR doesn't fix all bugs with the current approach. In the auto-handshake case, the struct members peer_cert and handshake_done are not set correctly. I'll look into the matter. Perhaps I can set them in the handshake or verify callback.

if (self->peer_cert)
    X509_free (self->peer_cert);
self->peer_cert = SSL_get_peer_certificate(self->ssl);
self->handshake_done = 1;

The bug has been fixed in bpo-31399. Since 3.7, Python uses OpenSSL's X509_VERIFY_PARAM_set1_host() to verify the host name during the handshake. Unfortunately the fix is in OpenSSL 1.0.2 only. Backport would break compatibility with OpenSSL 1.0.1 and all currently released LibreSSL versions.

Are you ok with closing the bug?

Do you mean, the fix is in 3.7 only?

...huh, not sure why that shows me changing the status. I just typed something in the text box, didn't touch any of the dropdowns...

Correct, the fix is in 3.7 and 3.8 only. I don't see a realistic way to address the problem in 3.6 and 2.7 without breaking people on old LTS releases and BSD.

(The bug tracker changes status automatically when you reply to a pending ticket.)

I agree that backporting X509_VERIFY_PARAM_set1_host is unreasonable, at least until the openssl ecosystem has moved forward a bit. But in earlier versions, would it be easy to detect that do_handshake() hasn't been called and raise an error?

The docs say you have to call do_handshake(), so if you don't that's already a bug and breaking that case should be OK, especially since it's never worked correctly.

I'm not very stressed about this myself because my code doesn't trigger the error -- only buggy code does :-). But it would be nice if the buggy code could fail closed.

3.6 will be out of support very soon. I'm closing this old bug as wontfix. Thanks for your investigation! :)