| msg276724 - (view) |
Author: (yan12125) * |
Date: 2016-09-16 15:06 |
| This was originally a post at python-ideas. Now I reformat it to be more like a feature request. Currently, Python raises SSLError with reason=CERTIFICATE_VERIFY_FAILED for all kinds of certificate verification failures. This results in difficulties in debugging SSL errors for others. (Some downstream bug reports: [1][2]) In OpenSSL, such errors are further divided into several kinds. For example, expired certificates result in X509_V_ERR_CERT_HAS_EXPIRED, and typical self-signed certificates fall into X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT. The error code can be retrieved via `SSL_get_verify_result` and human readable messages are available from `X509_verify_cert_error_string`. I hope I can get error messages like this: (Omit URLError to avoid verbose messages) $ ./python -c 'import urllib.request; urllib.request.urlopen("https://self-signed.badssl.com/")' Traceback (most recent call last): File "/home/yen/Projects/cpython/Lib/urllib/request.py", line 1318, in do_open encode_chunked=req.has_header('Transfer-encoding')) File "/home/yen/Projects/cpython/Lib/http/client.py", line 1239, in request self._send_request(method, url, body, headers, encode_chunked) File "/home/yen/Projects/cpython/Lib/http/client.py", line 1285, in _send_request self.endheaders(body, encode_chunked=encode_chunked) File "/home/yen/Projects/cpython/Lib/http/client.py", line 1234, in endheaders self._send_output(message_body, encode_chunked=encode_chunked) File "/home/yen/Projects/cpython/Lib/http/client.py", line 1026, in _send_output self.send(msg) File "/home/yen/Projects/cpython/Lib/http/client.py", line 964, in send self.connect() File "/home/yen/Projects/cpython/Lib/http/client.py", line 1400, in connect server_hostname=server_hostname) File "/home/yen/Projects/cpython/Lib/ssl.py", line 401, in wrap_socket _context=self, _session=session) File "/home/yen/Projects/cpython/Lib/ssl.py", line 808, in __init__ self.do_handshake() File "/home/yen/Projects/cpython/Lib/ssl.py", line 1061, in do_handshake self._sslobj.do_handshake() File "/home/yen/Projects/cpython/Lib/ssl.py", line 683, in do_handshake self._sslobj.do_handshake() ssl.SSLError: [SSL: CERTIFICATE_VERIFY_FAILED: DEPTH_ZERO_SELF_SIGNED_CERT] certificate verify failed: self signed certificate (_ssl.c:752) And for expired certificates: $ ./python -c 'import urllib.request; urllib.request.urlopen("https://expired.badssl.com/")' Traceback (most recent call last): File "/home/yen/Projects/cpython/Lib/urllib/request.py", line 1318, in do_open encode_chunked=req.has_header('Transfer-encoding')) File "/home/yen/Projects/cpython/Lib/http/client.py", line 1239, in request self._send_request(method, url, body, headers, encode_chunked) File "/home/yen/Projects/cpython/Lib/http/client.py", line 1285, in _send_request self.endheaders(body, encode_chunked=encode_chunked) File "/home/yen/Projects/cpython/Lib/http/client.py", line 1234, in endheaders self._send_output(message_body, encode_chunked=encode_chunked) File "/home/yen/Projects/cpython/Lib/http/client.py", line 1026, in _send_output self.send(msg) File "/home/yen/Projects/cpython/Lib/http/client.py", line 964, in send self.connect() File "/home/yen/Projects/cpython/Lib/http/client.py", line 1400, in connect server_hostname=server_hostname) File "/home/yen/Projects/cpython/Lib/ssl.py", line 401, in wrap_socket _context=self, _session=session) File "/home/yen/Projects/cpython/Lib/ssl.py", line 808, in __init__ self.do_handshake() File "/home/yen/Projects/cpython/Lib/ssl.py", line 1061, in do_handshake self._sslobj.do_handshake() File "/home/yen/Projects/cpython/Lib/ssl.py", line 683, in do_handshake self._sslobj.do_handshake() ssl.SSLError: [SSL: CERTIFICATE_VERIFY_FAILED: CERT_HAS_EXPIRED] certificate verify failed: certificate has expired (_ssl.c:752) I've once tried to achieve it, but my CPython knowledge is way too limited to give a good enough patch. [1] https://github.com/rg3/youtube-dl/issues/10574 [2] https://github.com/rg3/youtube-dl/issues/7309 |
|
|
| msg276898 - (view) |
Author: (yan12125) * |
Date: 2016-09-18 16:52 |
| Here's a quick try. I didn't add tests and update docs as it's my first serious patch to CPython and I'm not sure whether my approach is OK or not. |
|
|
| msg276904 - (view) |
Author: Christian Heimes (christian.heimes) *  |
Date: 2016-09-18 19:09 |
| Good work! I completely forgot that the SSL object holds the last verification error in its struct. This allows the ssl module to print some information when cert verification fails. It's still not perfect, because it is missing information about the the failing certificate. It's much better than no reason at all. I took your patch and simplified it a bit. I removed the sub reason attribute, too. We can add it later and use an enum.IntEnum instead of the old hack. ssl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:766) |
|
|
| msg276906 - (view) |
Author: (yan12125) * |
Date: 2016-09-18 19:42 |
| That looks much better. I should have create a subclass of SSLError. Here's a minor concern: fill_and_set_sslerror adds a new argument for verification errors. If someone else wants to support more errors, this function would have more arguments, which sounds bad for me - or we can postpone discussions until there's really such a need? |
|
|
| msg276914 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2016-09-18 21:08 |
| You don't have to be concerned about additional arguments. fill_and_set_sslerror() is an internal helper function. In fact it's a helper function for two other helper functions. Let's postpone the discussion until the argument sizes grows out of proportion. |
|
|
| msg276951 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2016-09-19 07:01 |
| What do we do about ssl.CertificateError? It's not a subclass of SSLError and raised by match_hostname(). |
|
|
| msg277030 - (view) |
Author: (yan12125) * |
Date: 2016-09-20 14:54 |
| With this change: (tested with OpenSSL git-master) @@ -632,20 +651,22 @@ newPySSLSocket(PySSLContext *sslctx, PyS SSL_set_bio(self->ssl, inbio->bio, outbio->bio); } mode = SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER; #ifdef SSL_MODE_AUTO_RETRY mode |= SSL_MODE_AUTO_RETRY; #endif SSL_set_mode(self->ssl, mode); + if (server_hostname != NULL) { #if HAVE_SNI - if (server_hostname != NULL) SSL_set_tlsext_host_name(self->ssl, server_hostname); #endif + SSL_set1_host(self->ssl, server_hostname); + } /* If the socket is in non-blocking mode or timeout mode, set the BIO * to non-blocking mode (blocking is the default) */ if (sock && sock->sock_timeout >= 0) { BIO_set_nbio(SSL_get_rbio(self->ssl), 1); BIO_set_nbio(SSL_get_wbio(self->ssl), 1); } When connecting to https://wrong.host.badssl.com/, the error is: ssl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: Hostname mismatch (_ssl.c:768) With this change in mind, an idea is to drop the Python implementation of match_hostname and rely on OpenSSL's checking mechanism (`do_x509_check`). As a result: * ssl.CertificateError can be either an alias of ssl.SSLCertVerificationError or a subclass of it * When verify_result is X509_V_ERR_HOSTNAME_MISMATCH, the error message is formatted with more information following the current approach in `match_hostname` ("hostname XXX doesn't match YYY...") |
|
|
| msg277033 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2016-09-20 15:12 |
| Yes, I'm planning to use the feature in 3.7. First I have to finish my PEP and get consents that I can drop support for OpenSSL 1.0.1 and earlier. We still support older versions but the feature is only available in 1.0.2+. I also need to come up with a solution to check IP addresses, too. It requires a different function. |
|
|
| msg277037 - (view) |
Author: (yan12125) * |
Date: 2016-09-20 15:46 |
| That's great. OpenSSL plans to drop 1.0.1 branch support after 2016/12/31. [1] I guess it's OK to drop 1.0.1 support in 3.7. Thanks for constantly improving SSL/TLS support in CPython! [1] https://www.openssl.org/source/ |
|
|
| msg277039 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2016-09-20 15:52 |
| I'm familiar with the release cycles of OpenSSL. In fact I want to tie support for OpenSSL versions to the release cycle of OpenSSL. Python core dev is a bit ... special. :) I can't just drop support. Some developers are opposing my plans and want to keep support for OpenSSL 1.0.1 and earlier. Enjoy this mail thread: https://mail.python.org/pipermail/python-dev/2016-August/145907.html |
|
|
| msg277042 - (view) |
Author: (yan12125) * |
Date: 2016-09-20 16:50 |
| > I'm familiar with the release cycles of OpenSSL. Oh I shouldn't say something trivial :) I know that thread. Hope I can help something on persuading others. |
|
|
| msg277449 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2016-09-26 18:38 |
| For hostname verification it might be a good idea to add a replacement for ssl.CertificateError. |
|
|
| msg301718 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2017-09-08 19:00 |
| New changeset b3ad0e5127bdeb6e506301e0d65403fa23c4177b by Christian Heimes in branch 'master': bpo-28182: Expose OpenSSL verification results (#3412) https://github.com/python/cpython/commit/b3ad0e5127bdeb6e506301e0d65403fa23c4177b |
|
|
| msg301719 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2017-09-08 19:03 |
| The ssl module now reports cause of validation failure: >>> import ssl >>> import ssl, socket >>> ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) >>> sock = ctx.wrap_socket(socket.socket(), server_hostname='www.python.org') >>> sock.connect(('www.python.org', 443)) Traceback (most recent call last): ... ssl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:825) |
|
|
| msg301741 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2017-09-08 21:48 |
| New changeset 0915360b9ef765bf84d4471a8a079f48c49bad68 by Christian Heimes in branch 'master': bpo-28182: restore backwards compatibility (#3464) https://github.com/python/cpython/commit/0915360b9ef765bf84d4471a8a079f48c49bad68 |
|
|