http.client.HTTPSConnection checks hostname when SSL context has check_hostname==False

[desbma]

BPO	22959
Nosy	@tiran, @benjaminp, @alex, @desbma
Files	ch-weirdness.aptch

^{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 = None
closed_at = <Date 2014-12-07.18:49:12.781>
created_at = <Date 2014-11-27.19:42:39.785>
labels = ['type-bug', 'library']
title = 'http.client.HTTPSConnection checks hostname when SSL context has check_hostname==False'
updated_at = <Date 2014-12-07.23:36:37.519>
user = 'https://github.com/desbma'

bugs.python.org fields:

activity = <Date 2014-12-07.23:36:37.519>
actor = 'desbma'
assignee = 'none'
closed = True
closed_date = <Date 2014-12-07.18:49:12.781>
closer = 'benjamin.peterson'
components = ['Library (Lib)']
creation = <Date 2014-11-27.19:42:39.785>
creator = 'desbma'
dependencies = []
files = ['37325']
hgrepos = []
issue_num = 22959
keywords = []
message_count = 10.0
messages = ['231775', '231886', '231887', '231890', '231892', '231893', '231896', '232274', '232275', '232289']
nosy_count = 5.0
nosy_names = ['christian.heimes', 'benjamin.peterson', 'alex', 'python-dev', 'desbma']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue22959'
versions = ['Python 3.4']

[desbma]

http.client.HTTPSConnection has both a check_hostname parameter, and a context parameter to pass an already setup SSL context.
When check_hostname is not set and thus is None, and when passing a SSL context set to NOT check hostnames, ie:

import http.client
import ssl

ssl_context = ssl.create_default_context() 
ssl_context.check_hostname = False
https = http.client.HTTPSConnection(..., context=ssl_context)

The https object WILL check hostname.

In my opinion the line :
https://hg.python.org/cpython/file/3.4/Lib/http/client.py#l1207
will_verify = context.verify_mode != ssl.CERT_NONE

Should be changed to:
will_verify = (context.verify_mode != ssl.CERT_NONE) and (context.check_hostname)

[benjaminp]

Why do you think it still verifies the hostname? It will certainly check if the certificate has a valid trust chain, but it won't do matching on the hostname.

[desbma]

I think it does, when passing a context with ssl_context.verify_mode != ss.CERT_NONE, and when not setting the check_hostname parameter:

1. will_verify will be True (https://hg.python.org/cpython/file/3.4/Lib/http/client.py#l1207)
2. check_hostname will be True too (https://hg.python.org/cpython/file/3.4/Lib/http/client.py#l1209)
3. ssl.match_hostname will be called after the handshake in wrap_socket (https://hg.python.org/cpython/file/3.4/Lib/http/client.py#l1230)

The following code shows the problem:

import http.client
import ssl

ssl_context = ssl.SSLContext(ssl.PROTOCOL_TLSv1_2)
ssl_context.verify_mode = ssl.CERT_REQUIRED
ssl_context.check_hostname = False
https = http.client.HTTPSConnection("localhost", context=ssl_context)
print(https._check_hostname)

[benjaminp]

As the documentation says "If context is specified and has a verify_mode of either CERT_OPTIONAL or CERT_REQUIRED, then by default host is matched against the host name(s) allowed by the server’s certificate. If you want to change that behaviour, you can explicitly set check_hostname to False."

It is rather weird that HTTPSConnection has its own "check_hostname" parameter independent of the one on the SSL context.

Alex, what do you think of this patch?

[alex]

This will cause it to not validate in some cases where it currently is validating? That seems like a regression to me.

[benjaminp]

On Sun, Nov 30, 2014, at 11:20, Alex Gaynor wrote:

Alex Gaynor added the comment:

This will cause it to not validate in some cases where it currently is
validating? That seems like a regression to me.

I suppose. Certainly, none of the "default" cases are affected. The
problem is it's impossible to have cert validation w/o hostname checking
by passing a context to some higher level API than HTTPSConnection (like
xmlrpclib) because HTTPSConnection tries to be clever. Ideally, the
check_hostname argument wouldn't exist, and everything would come from
the context.

[desbma]

I agree that changing a default to something less secure is not something to do lightly, however I think forcing a check that is explicitly disabled is a bug and can be counter productive security wise.

People who don't have time to look at the stdlib code, and file bug like this are likely to react with "fuck it, I'll use plain HTTP".

By the way, my use case is precisely xmlrpc.

[python-dev]

New changeset a409a7cd908d by Benjamin Peterson in branch '3.4':
HTTPSConnection: prefer the context's check_hostname attribute over the constructor parameter (bpo-22959)
https://hg.python.org/cpython/rev/a409a7cd908d

New changeset 41021c771510 by Benjamin Peterson in branch '2.7':
remove HTTPSConnection's check_hostname parameter (bpo-22959)
https://hg.python.org/cpython/rev/41021c771510

New changeset ee095a2e2b35 by Benjamin Peterson in branch 'default':
merge 3.4 (bpo-22959)
https://hg.python.org/cpython/rev/ee095a2e2b35

[benjaminp]

Okay, I basically applied my patch to 3.4/3.5. I simply removed the check_hostname parameter from 2.7, since it was to be added in 2.7.9.

[desbma]

Thank you