classification
Title: Expose OpenSSL verification results in SSLError
Type: enhancement Stage: resolved
Components: SSL Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: christian.heimes Nosy List: Chi Hsuan Yen, alex, christian.heimes, dstufft, janssen
Priority: normal Keywords: patch

Created on 2016-09-16 15:06 by Chi Hsuan Yen, last changed 2017-09-08 21:48 by christian.heimes. This issue is now closed.

Files
File name Uploaded Description Edit
expose-x509-verify-result.patch Chi Hsuan Yen, 2016-09-18 16:52 review
ssl_certverifyerror.patch christian.heimes, 2016-09-18 19:09 review
Pull Requests
URL Status Linked Edit
PR 3412 merged christian.heimes, 2017-09-07 01:51
PR 3464 merged christian.heimes, 2017-09-08 21:36
Messages (15)
msg276724 - (view) Author: Chi Hsuan Yen (Chi Hsuan Yen) * 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: Chi Hsuan Yen (Chi Hsuan Yen) * 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) * (Python committer) 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: Chi Hsuan Yen (Chi Hsuan Yen) * 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) * (Python committer) 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) * (Python committer) 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: Chi Hsuan Yen (Chi Hsuan Yen) * 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) * (Python committer) 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: Chi Hsuan Yen (Chi Hsuan Yen) * 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) * (Python committer) 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: Chi Hsuan Yen (Chi Hsuan Yen) * 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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
History
Date User Action Args
2017-09-08 21:48:01christian.heimessetmessages: + msg301741
2017-09-08 21:36:36christian.heimessetpull_requests: + pull_request3456
2017-09-08 19:03:23christian.heimessetstatus: open -> closed
resolution: fixed
messages: + msg301719

stage: resolved
2017-09-08 19:00:21christian.heimessetmessages: + msg301718
2017-09-07 01:51:36christian.heimessetpull_requests: + pull_request3413
2016-09-26 18:38:49christian.heimessetmessages: + msg277449
2016-09-20 16:50:03Chi Hsuan Yensetmessages: + msg277042
2016-09-20 15:52:19christian.heimessetmessages: + msg277039
2016-09-20 15:46:41Chi Hsuan Yensetmessages: + msg277037
2016-09-20 15:12:55christian.heimessetmessages: + msg277033
2016-09-20 14:54:13Chi Hsuan Yensetmessages: + msg277030
2016-09-19 07:01:50christian.heimessetmessages: + msg276951
2016-09-18 21:08:24christian.heimessetmessages: + msg276914
2016-09-18 19:42:41Chi Hsuan Yensetmessages: + msg276906
2016-09-18 19:09:19christian.heimessetfiles: + ssl_certverifyerror.patch

messages: + msg276904
2016-09-18 16:52:38Chi Hsuan Yensetfiles: + expose-x509-verify-result.patch
keywords: + patch
messages: + msg276898
2016-09-16 15:06:43Chi Hsuan Yencreate