classification
Title: If you forget to call do_handshake, then everything seems to work but hostname checking is disabled
Type: behavior Stage: resolved
Components: SSL Versions: Python 3.6, Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder: Let OpenSSL verify hostname and IP address
View: 31399
Assigned To: christian.heimes Nosy List: christian.heimes, njs
Priority: normal Keywords:

Created on 2017-04-23 10:30 by njs, last changed 2018-02-26 09:43 by njs.

Files
File name Uploaded Description Edit
ssl-handshake.zip njs, 2017-04-23 10:30 demo
Messages (8)
msg292158 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2017-04-23 10:30
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.
msg292161 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2017-04-23 11:59
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
msg292199 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2017-04-24 07:32
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;
msg312884 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2018-02-26 08:22
The bug has been fixed in #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?
msg312894 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-02-26 08:43
Do you mean, the fix is in 3.7 only?
msg312896 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-02-26 08:44
...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...
msg312902 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2018-02-26 09:36
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.)
msg312904 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-02-26 09:43
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.
History
Date User Action Args
2018-02-26 09:43:20njssetmessages: + msg312904
versions: - Python 3.7
2018-02-26 09:36:44christian.heimessetstatus: pending -> open
resolution: fixed ->
messages: + msg312902

versions: + Python 3.7
2018-02-26 08:44:47njssetstatus: open -> pending

messages: + msg312896
2018-02-26 08:43:36njssetstatus: pending -> open

messages: + msg312894
2018-02-26 08:22:40christian.heimessetstatus: open -> pending

type: behavior
versions: + Python 2.7
messages: + msg312884
superseder: Let OpenSSL verify hostname and IP address
resolution: fixed
stage: resolved
2017-04-24 07:32:49christian.heimessetmessages: + msg292199
2017-04-23 12:29:38njssettitle: If you forget to call do_handshake, then everything seems to work but hostname is disabled -> If you forget to call do_handshake, then everything seems to work but hostname checking is disabled
2017-04-23 11:59:48christian.heimessetmessages: + msg292161
2017-04-23 10:30:49njscreate