
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2014-11-27 19:42 bydesbma, last changed2022-04-11 14:58 byadmin. This issue is nowclosed.
| 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.clientimport sslssl_context = ssl.create_default_context() ssl_context.check_hostname = Falsehttps = 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_NONEShould be changed to: will_verify = (context.verify_mode != ssl.CERT_NONE) and (context.check_hostname) | |||
| msg231886 -(view) | Author: Benjamin Peterson (benjamin.peterson)*![]() | 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.clientimport sslssl_context = ssl.SSLContext(ssl.PROTOCOL_TLSv1_2)ssl_context.verify_mode = ssl.CERT_REQUIREDssl_context.check_hostname = Falsehttps = http.client.HTTPSConnection("localhost", context=ssl_context)print(https._check_hostname) | |||
| msg231890 -(view) | Author: Benjamin Peterson (benjamin.peterson)*![]() | 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)*![]() | 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)*![]() | 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. Theproblem is it's impossible to have cert validation w/o hostname checkingby passing a context to some higher level API than HTTPSConnection (likexmlrpclib) because HTTPSConnection tries to be clever. Ideally, thecheck_hostname argument wouldn't exist, and everything would come fromthe 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 changeseta409a7cd908d 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/a409a7cd908dNew changeset41021c771510 by Benjamin Peterson in branch '2.7':remove HTTPSConnection's check_hostname parameter (#22959)https://hg.python.org/cpython/rev/41021c771510New changesetee095a2e2b35 by Benjamin Peterson in branch 'default':merge 3.4 (#22959)https://hg.python.org/cpython/rev/ee095a2e2b35 | |||
| msg232275 -(view) | Author: Benjamin Peterson (benjamin.peterson)*![]() | 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 |
| 2022-04-11 14:58:10 | admin | set | github: 67148 |
| 2014-12-07 23:36:37 | desbma | set | messages: +msg232289 |
| 2014-12-07 18:49:12 | benjamin.peterson | set | status: open -> closed resolution: fixed messages: +msg232275 |
| 2014-12-07 18:47:49 | python-dev | set | nosy: +python-dev messages: +msg232274 |
| 2014-11-30 16:57:30 | desbma | set | messages: +msg231896 |
| 2014-11-30 16:38:40 | benjamin.peterson | set | messages: +msg231893 |
| 2014-11-30 16:20:53 | alex | set | messages: +msg231892 |
| 2014-11-30 16:15:40 | benjamin.peterson | set | files: +ch-weirdness.aptch messages: +msg231890 |
| 2014-11-30 11:51:52 | desbma | set | messages: +msg231887 |
| 2014-11-30 04:36:53 | benjamin.peterson | set | nosy: +benjamin.peterson messages: +msg231886 |
| 2014-11-27 23:29:04 | pitrou | set | nosy: +christian.heimes,alex |
| 2014-11-27 19:42:39 | desbma | create | |