Issue31431
Created on 2017-09-12 15:58 by christian.heimes, last changed 2022-04-11 14:58 by admin. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 3531 | merged | christian.heimes, 2017-09-13 05:39 | |
| Messages (2) | |||
|---|---|---|---|
| msg301967 - (view) | Author: Christian Heimes (christian.heimes) * ![]() |
Date: 2017-09-12 15:58 | |
Hostname verification makes not much sense without verifying the certificate chain first. At the moment one has to set verify_mode to CERT_REQUIRED first: >>> import ssl >>> ctx = ssl.SSLContext(ssl.PROTOCOL_TLS) >>> ctx.check_hostname, ctx.verify_mode (False, <VerifyMode.CERT_NONE: 0>) >>> ctx.check_hostname = True Traceback (most recent call last): File "<stdin>", line 1, in <module> ValueError: check_hostname needs a SSL context with either CERT_OPTIONAL or CERT_REQUIRED >>> ctx.verify_mode = ssl.CERT_REQUIRED >>> ctx.check_hostname = True On the other hand verify mode cannot be set to CERT_NONE without disabling check_hostname first. One has to remember to set the values in opposite order! >>> ctx.verify_mode = ssl.CERT_NONE Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/home/heimes/dev/python/cpython/Lib/ssl.py", line 485, in verify_mode super(SSLContext, SSLContext).verify_mode.__set__(self, value) ValueError: Cannot set verify_mode to CERT_NONE when check_hostname is enabled. >>> ctx.check_hostname = False >>> ctx.verify_mode = ssl.CERT_NONE I find this confusing. In order to support PROTOCOL_TLS_CLIENT with _create_unverified_context(), I had to modify the code to this abomination: if not check_hostname: context.check_hostname = False if cert_reqs is not None: context.verify_mode = cert_reqs if check_hostname: context.check_hostname = True Rather than making our users to jump through additional hoops, check_hostname = True should just set CERT_REQUIRED. This is a sane and safe default. On the other hand, ssl.CERT_NONE shall *not* disable check_hostname and still fail with a ValueError if check_hostname is enabled. By the way we should not suggest CERT_OPTIONAL here, too. For TLS client connections, CERT_OPTIONAL is not really optional. CERT_OPTIONAL: SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, verify_cb), CERT_REQUIRED: SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, verify_cb). According to https://www.openssl.org/docs/man1.0.2/ssl/SSL_CTX_set_verify.html SSL_VERIFY_FAIL_IF_NO_PEER_CERT is ignored in client mode. I'll open a new bug report. |
|||
| msg302288 - (view) | Author: Christian Heimes (christian.heimes) * ![]() |
Date: 2017-09-15 18:29 | |
New changeset e82c034496512139e9ea3f68ceda86c04bc7baab by Christian Heimes in branch 'master': bpo-31431: SSLContext.check_hostname auto-sets CERT_REQUIRED (#3531) https://github.com/python/cpython/commit/e82c034496512139e9ea3f68ceda86c04bc7baab |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:52 | admin | set | github: 75612 |
| 2017-09-15 18:30:20 | christian.heimes | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
| 2017-09-15 18:29:59 | christian.heimes | set | messages: + msg302288 |
| 2017-09-13 05:39:07 | christian.heimes | set | keywords:
+ patch stage: needs patch -> patch review pull_requests: + pull_request3528 |
| 2017-09-12 15:59:00 | christian.heimes | set | nosy:
+ janssen, alex, dstufft |
| 2017-09-12 15:58:47 | christian.heimes | create | |
