Server-side support for TLS Server Name Indication extension

[jcea]

BPO	8109
Nosy	@jcea, @pitrou, @tiran
Files	issue8109_server_side_sni.patch issue-8109-sni-serverside.patch issue-8109-sni-serverside.patch sni.patch sni2.patch issue-8109.patch issue-8109.patch

^{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 2013-01-06.14:30:49.204>
created_at = <Date 2010-03-10.15:14:05.562>
labels = ['type-feature', 'library']
title = 'Server-side support for TLS Server Name Indication extension'
updated_at = <Date 2013-06-24.19:12:59.900>
user = 'https://github.com/jcea'

bugs.python.org fields:

activity = <Date 2013-06-24.19:12:59.900>
actor = 'mpb'
assignee = 'none'
closed = True
closed_date = <Date 2013-01-06.14:30:49.204>
closer = 'pitrou'
components = ['Library (Lib)']
creation = <Date 2010-03-10.15:14:05.562>
creator = 'jcea'
dependencies = []
files = ['27190', '28271', '28326', '28566', '28568', '29770', '29776']
hgrepos = []
issue_num = 8109
keywords = ['patch']
message_count = 27.0
messages = ['100787', '103751', '125613', '125645', '168793', '170233', '170474', '172167', '172195', '176776', '177258', '177544', '177581', '179079', '179093', '179141', '179143', '179145', '179158', '179173', '179182', '179194', '186539', '186541', '186555', '186578', '186579']
nosy_count = 9.0
nosy_names = ['jcea', 'pitrou', 'christian.heimes', 'grooverdan', 'python-dev', 'piotr.dobrogost', 'daniel-black', 'kyoshida', 'mpb']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue8109'
versions = ['Python 3.4']

[jcea]

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?.

[pitrou]

Duplicate of bpo-5639.

[grooverdan]

issue bpo-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.

[pitrou]

Server side SNI is still missing.

Right, re-opening.

[daniel-black]

test_sni not working. getpeercert() not returning a certificate.

[jcea]

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! :(

[daniel-black]

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.

[daniel-black]

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.

[pitrou]

Daniel, I'll take a look.

[daniel-black]

Antoine Pitrou (pitrou) * Date: 2012-10-06 13:10
Daniel, I'll take a look.

minor nag :-)

[daniel-black]

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.

[pitrou]

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.

[daniel-black]

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.

[pitrou]

Here is an updated patch with cyclic GC support, and other small things.

[pitrou]

Updated patch after Daniel's comments.

[python-dev]

New changeset 927afb7bca2a by Antoine Pitrou in branch 'default':
Issue bpo-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

[pitrou]

I've committed the latest patch. Thank you very much!

[daniel-black]

I've committed the latest patch. Thank you very much!

much appreciate your help.

[tiran]

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.

[daniel-black]

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;

[pitrou]

Fixed in 52b4d9bfc9ea (Roundup e-mail gateway seems broken).

[pitrou]

(testing Roundup mail gateway, please ignore)

[kyoshida]

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.

[grooverdan]

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.

[kyoshida]

Thanks for a comment.
I've made a version that adds a line to the document.

[python-dev]

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

[pitrou]

Thank you for finding this! The patch is now committed.