SSL: check_hostname should imply CERT_REQUIRED

[tiran]

BPO	31431
Nosy	@tiran, @alex, @dstufft
PRs	bpo-31431: SSLContext.check_hostname auto-sets CERT_REQUIRED #3531

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/tiran'
closed_at = <Date 2017-09-15.18:30:20.902>
created_at = <Date 2017-09-12.15:58:47.913>
labels = ['expert-SSL', 'type-feature', '3.7']
title = 'SSL: check_hostname should imply CERT_REQUIRED'
updated_at = <Date 2017-09-15.18:30:20.901>
user = 'https://github.com/tiran'

bugs.python.org fields:

activity = <Date 2017-09-15.18:30:20.901>
actor = 'christian.heimes'
assignee = 'christian.heimes'
closed = True
closed_date = <Date 2017-09-15.18:30:20.902>
closer = 'christian.heimes'
components = ['SSL']
creation = <Date 2017-09-12.15:58:47.913>
creator = 'christian.heimes'
dependencies = []
files = []
hgrepos = []
issue_num = 31431
keywords = ['patch']
message_count = 2.0
messages = ['301967', '302288']
nosy_count = 4.0
nosy_names = ['janssen', 'christian.heimes', 'alex', 'dstufft']
pr_nums = ['3531']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue31431'
versions = ['Python 3.7']

[tiran]

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.

[tiran]

New changeset e82c034 by Christian Heimes in branch 'master':
bpo-31431: SSLContext.check_hostname auto-sets CERT_REQUIRED (bpo-3531)
e82c034