Issue8109
Created on 2010-03-10 15:14 by jcea, last changed 2022-04-11 14:56 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| issue8109_server_side_sni.patch | daniel-black, 2012-09-14 13:16 | review | ||
| issue-8109-sni-serverside.patch | daniel-black, 2012-12-10 03:30 | review | ||
| issue-8109-sni-serverside.patch | daniel-black, 2012-12-16 03:42 | review | ||
| sni.patch | pitrou, 2013-01-04 21:21 | review | ||
| sni2.patch | pitrou, 2013-01-04 23:01 | review | ||
| issue-8109.patch | kyoshida, 2013-04-11 03:14 | review | ||
| issue-8109.patch | kyoshida, 2013-04-11 10:06 | review | ||
| Messages (27) | |||
|---|---|---|---|
| msg100787 - (view) | Author: Jesús Cea Avión (jcea) * ![]() |
Date: 2010-03-10 15:14 | |
SSL sockets should support SNI, both as servers and clients: http://en.wikipedia.org/wiki/Server_Name_Indication After that, libraries that support SSL/TLS should be upgraded to take advantage of it. Any interest in supporting this?. |
|||
| msg103751 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2010-04-20 20:47 | |
Duplicate of issue5639. |
|||
| msg125613 - (view) | Author: Daniel Black (grooverdan) * | Date: 2011-01-07 01:37 | |
issue #5639 only has functionality for client side SNI. Server side SNI is still missing. For server side SNI to be supported a server program should be able to retrieve the server name provided by the client call and alter the server certificate/key before the server completes the TLS/SSL connection. |
|||
| msg125645 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2011-01-07 13:43 | |
> Server side SNI is still missing. Right, re-opening. |
|||
| msg168793 - (view) | Author: danblack (daniel-black) | Date: 2012-08-21 16:52 | |
test_sni not working. getpeercert() not returning a certificate. |
|||
| msg170233 - (view) | Author: Jesús Cea Avión (jcea) * ![]() |
Date: 2012-09-10 20:33 | |
Daniel, your patch looks quite interesting. Please, send a contributor agreement to the PSF: http://www.python.org/psf/contrib/contrib-form-python/ . Let me know when you status have changed. Why are you changing "Lib/test/keycert2.pem"? Please, provide also a documentation patch. This is a feature enhancement. Would be applied to 3.4, it is too late for 3.3 :-(. Too bad! :( |
|||
| msg170474 - (view) | Author: danblack (daniel-black) | Date: 2012-09-14 13:16 | |
> Daniel, your patch looks quite interesting. Please, send a contributor agreement to the PSF: http://www.python.org/psf/contrib/contrib-form-python/ . Let me know when you status have changed. Already done. Has been accepted and I've got an acknowledgement email. > Why are you changing "Lib/test/keycert2.pem"? I was mistakely assuming that this was the only test that used it. Fixed now. Also added a CA key and server for validating key chains. I didn't end up using it however thought it would be handy. > Please, provide also a documentation patch. Done. Also improved error checking and reference counting. > This is a feature enhancement. Would be applied to 3.4, it is too late for 3.3 :-(. Too bad! :( Was expected. Its been 2.5 years since the bug opened. A little more won't hurt. I've also changed SSLSocket.context to be a property. Its not quite working. The current test case as is working however using an assignment as per line 1958 of Lib/test/test_ssl.py. |
|||
| msg172167 - (view) | Author: danblack (daniel-black) | Date: 2012-10-06 04:30 | |
happy with this? I'm not sure what i've done to make s._set_context(newctx) work but s.context = newctx fail. I though the code here http://bugs.python.org/review/8109/diff2/5815:5989/Lib/ssl.py effectively maps them. |
|||
| msg172195 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2012-10-06 13:10 | |
Daniel, I'll take a look. |
|||
| msg176776 - (view) | Author: danblack (daniel-black) | Date: 2012-12-02 07:49 | |
> Antoine Pitrou (pitrou) * Date: 2012-10-06 13:10 > Daniel, I'll take a look. minor nag :-) |
|||
| msg177258 - (view) | Author: danblack (daniel-black) | Date: 2012-12-10 03:30 | |
I've added a full set of alert descriptions and cleaned up the doco some more. The reference counting when the SNI callback comes in is my greatest worry. |
|||
| msg177544 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2012-12-15 17:57 | |
I've posted a few more comments. As for cyclic garbage collection, it's explained a bit there: http://docs.python.org/dev/extending/newtypes.html#supporting-cyclic-garbage-collection If it isn't very clear to you, I can still handle it myself, though. Those docs aren't the best. |
|||
| msg177581 - (view) | Author: danblack (daniel-black) | Date: 2012-12-16 08:34 | |
> If it isn't very clear to you, I can still handle it myself, though. Those docs aren't the best. Not clear enough. Yes I'd appreciate you handling it. Thanks. |
|||
| msg179079 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2013-01-04 21:08 | |
Here is an updated patch with cyclic GC support, and other small things. |
|||
| msg179093 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2013-01-04 23:01 | |
Updated patch after Daniel's comments. |
|||
| msg179141 - (view) | Author: Roundup Robot (python-dev) ![]() |
Date: 2013-01-05 20:22 | |
New changeset 927afb7bca2a by Antoine Pitrou in branch 'default': Issue #8109: The ssl module now has support for server-side SNI, thanks to a :meth:`SSLContext.set_servername_callback` method. http://hg.python.org/cpython/rev/927afb7bca2a |
|||
| msg179143 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2013-01-05 20:24 | |
I've committed the latest patch. Thank you very much! |
|||
| msg179145 - (view) | Author: danblack (daniel-black) | Date: 2013-01-05 21:27 | |
> I've committed the latest patch. Thank you very much! much appreciate your help. |
|||
| msg179158 - (view) | Author: Christian Heimes (christian.heimes) * ![]() |
Date: 2013-01-06 00:36 | |
Coverity reports an issue in the callback function: /Modules/_ssl.c: 2403 ( uninit_use) 2400 /* remove race condition in this the call back while if removing the 2401 * callback is in progress */ 2402 PyGILState_Release(gstate); >>> CID 966640: Uninitialized scalar variable (UNINIT) >>> Using uninitialized value "ret". 2403 return ret; 2404 } 2405 2406 ssl = SSL_get_app_data(s); 2407 assert(PySSLSocket_Check(ssl)); I don't know which error code should be returned in this case. |
|||
| msg179173 - (view) | Author: danblack (daniel-black) | Date: 2013-01-06 06:18 | |
> I don't know which error code should be returned in this case. Thanks Christian. My fault - asked Antoine to remove the default value for it and didn't see this like. make line 2403: return SSL_TLSEXT_ERR_OK; |
|||
| msg179182 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2013-01-06 14:30 | |
Fixed in 52b4d9bfc9ea (Roundup e-mail gateway seems broken). |
|||
| msg179194 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2013-01-06 15:51 | |
(testing Roundup mail gateway, please ignore) |
|||
| msg186539 - (view) | Author: Kazuhiro Yoshida (kyoshida) | Date: 2013-04-11 03:14 | |
I am trying to use SSLContext.set_servername_callback in my program but when a callback is set, it seems that connecting to the server without providing a server name causes a segmentation fault. (e.g. 'openssl s_client -connect localhost:443 -servername foo' is OK but 'openssl s_client -connect localhost:443' crashes the server. A simple test that causes the same error is included in the patch.) My expectation was to get None as the second argument of the callback in such cases so I modified Modules/_ssl.c (as in the patch) to make it behave as I expected. The modification seems to work fine as far as I've tested, but I'd appreciate if an official fix is available. |
|||
| msg186541 - (view) | Author: Daniel Black (grooverdan) * | Date: 2013-04-11 03:50 | |
nice patch. Thanks for finding the bug. I like the solution with test case. Just needs a small enhancement of documention to ensure other users expect this behaviour. |
|||
| msg186555 - (view) | Author: Kazuhiro Yoshida (kyoshida) | Date: 2013-04-11 10:06 | |
Thanks for a comment. I've made a version that adds a line to the document. |
|||
| msg186578 - (view) | Author: Roundup Robot (python-dev) ![]() |
Date: 2013-04-11 18:49 | |
New changeset 4ae6095b4638 by Antoine Pitrou in branch 'default': Fix a crash when setting a servername callback on a SSL server socket and the client doesn't send a server name. http://hg.python.org/cpython/rev/4ae6095b4638 |
|||
| msg186579 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2013-04-11 18:49 | |
Thank you for finding this! The patch is now committed. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:56:58 | admin | set | github: 52356 |
| 2013-06-24 19:12:59 | mpb | set | nosy:
+ mpb |
| 2013-04-11 18:49:53 | pitrou | set | messages: + msg186579 |
| 2013-04-11 18:49:20 | python-dev | set | messages: + msg186578 |
| 2013-04-11 10:06:45 | kyoshida | set | files:
+ issue-8109.patch messages: + msg186555 |
| 2013-04-11 03:50:27 | grooverdan | set | messages: + msg186541 |
| 2013-04-11 03:14:55 | kyoshida | set | files:
+ issue-8109.patch nosy: + kyoshida messages: + msg186539 |
| 2013-01-06 15:51:14 | pitrou | set | messages: + msg179194 |
| 2013-01-06 14:30:49 | pitrou | set | status: open -> closed resolution: fixed messages: + msg179182 stage: needs patch -> resolved |
| 2013-01-06 06:18:37 | daniel-black | set | messages: + msg179173 |
| 2013-01-06 00:36:25 | christian.heimes | set | status: closed -> open nosy:
+ christian.heimes resolution: fixed -> (no value) |
| 2013-01-05 21:27:27 | daniel-black | set | messages: + msg179145 |
| 2013-01-05 20:24:42 | pitrou | set | status: open -> closed resolution: fixed messages: + msg179143 stage: patch review -> resolved |
| 2013-01-05 20:22:49 | python-dev | set | nosy:
+ python-dev messages: + msg179141 |
| 2013-01-04 23:01:51 | pitrou | set | files:
+ sni2.patch messages: + msg179093 |
| 2013-01-04 21:21:32 | pitrou | set | files: + sni.patch |
| 2013-01-04 21:21:24 | pitrou | set | files: - sni.patch |
| 2013-01-04 21:08:51 | pitrou | set | files:
+ sni.patch messages:
+ msg179079 |
| 2012-12-16 08:34:36 | daniel-black | set | messages: + msg177581 |
| 2012-12-16 03:42:51 | daniel-black | set | files: + issue-8109-sni-serverside.patch |
| 2012-12-15 17:57:39 | pitrou | set | messages: + msg177544 |
| 2012-12-10 03:30:25 | daniel-black | set | files:
+ issue-8109-sni-serverside.patch messages: + msg177258 |
| 2012-12-02 07:49:06 | daniel-black | set | messages: + msg176776 |
| 2012-11-23 20:00:53 | piotr.dobrogost | set | nosy:
+ piotr.dobrogost |
| 2012-10-06 13:10:49 | pitrou | set | messages: + msg172195 |
| 2012-10-06 04:30:16 | daniel-black | set | messages: + msg172167 |
| 2012-09-14 13:16:58 | daniel-black | set | files: - issue8109_server_side_sni.patch |
| 2012-09-14 13:16:25 | daniel-black | set | files:
+ issue8109_server_side_sni.patch messages: + msg170474 |
| 2012-09-10 20:34:02 | jcea | set | versions: + Python 3.4, - Python 3.3 |
| 2012-09-10 20:33:49 | jcea | set | messages:
+ msg170233 stage: needs patch -> patch review |
| 2012-08-21 16:52:54 | daniel-black | set | files:
+ issue8109_server_side_sni.patch nosy:
+ daniel-black keywords: + patch |
| 2011-01-07 13:43:16 | pitrou | set | status: closed -> open title: Support for TLS Server Name Indication extension -> Server-side support for TLS Server Name Indication extension |
| 2011-01-07 01:37:36 | grooverdan | set | nosy:
+ grooverdan messages: + msg125613 |
| 2010-04-20 20:47:50 | pitrou | set | status: open -> closed nosy:
+ pitrou superseder: Support TLS SNI extension in ssl module |
| 2010-03-10 15:14:05 | jcea | create | |

