classification
Title: http.client.HTTPSConnection checks hostname when SSL context has check_hostname==False
Type: behavior Stage:
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: alex, benjamin.peterson, christian.heimes, desbma, python-dev
Priority: normal Keywords:

Created on 2014-11-27 19:42 by desbma, last changed 2014-12-07 23:36 by desbma. This issue is now closed.

Files
File name Uploaded Description Edit
ch-weirdness.aptch benjamin.peterson, 2014-11-30 16:15 review
Messages (10)
msg231775 - (view) Author: desbma (desbma) * Date: 2014-11-27 19:42
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)
msg231886 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2014-11-30 04:36
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.
msg231887 - (view) Author: desbma (desbma) * Date: 2014-11-30 11:51
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)
msg231890 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2014-11-30 16:15
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?
msg231892 - (view) Author: Alex Gaynor (alex) * (Python committer) Date: 2014-11-30 16:20
This will cause it to not validate in some cases where it currently is validating? That seems like a regression to me.
msg231893 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2014-11-30 16:38
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.
msg231896 - (view) Author: desbma (desbma) * Date: 2014-11-30 16:57
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.
msg232274 - (view) Author: Roundup Robot (python-dev) Date: 2014-12-07 18:47
New changeset a409a7cd908d by Benjamin Peterson in branch '3.4':
HTTPSConnection: prefer the context's check_hostname attribute over the constructor parameter (#22959)
https://hg.python.org/cpython/rev/a409a7cd908d

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

New changeset ee095a2e2b35 by Benjamin Peterson in branch 'default':
merge 3.4 (#22959)
https://hg.python.org/cpython/rev/ee095a2e2b35
msg232275 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2014-12-07 18:49
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.
msg232289 - (view) Author: desbma (desbma) * Date: 2014-12-07 23:36
Thank you
History
Date User Action Args
2014-12-07 23:36:37desbmasetmessages: + msg232289
2014-12-07 18:49:12benjamin.petersonsetstatus: open -> closed
resolution: fixed
messages: + msg232275
2014-12-07 18:47:49python-devsetnosy: + python-dev
messages: + msg232274
2014-11-30 16:57:30desbmasetmessages: + msg231896
2014-11-30 16:38:40benjamin.petersonsetmessages: + msg231893
2014-11-30 16:20:53alexsetmessages: + msg231892
2014-11-30 16:15:40benjamin.petersonsetfiles: + ch-weirdness.aptch

messages: + msg231890
2014-11-30 11:51:52desbmasetmessages: + msg231887
2014-11-30 04:36:53benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg231886
2014-11-27 23:29:04pitrousetnosy: + christian.heimes, alex
2014-11-27 19:42:39desbmacreate