@glyph commented

This ticket should be for server-side support. I don't know what's required client-side yet.

	@exarkun commented

pyOpenSSL 0.13 supports SNI. For a server, what you do is call set_tlsext_servername_callback on the Context instance you're using. You pass it a callback function that will be called with a Connection instance. In the callback function, you use Connection.get_servername to see who the client wanted to talk to. Then, if appropriate, you construct a new Context configured however you like (including using a certificate/key which matches the client's expectations) and make the server use it using Connection.set_context.

While not the prettiest API (not least because it forces a lot of OpenSSL specifics onto you), it's actually all accessible already, without any particular additional support from Twisted. So from that point of view, this ticket is actually resolved.

The client-side support is actually harder, because there currently is no way in the Twisted SSL client-side APIs to specify which hostname you want to talk to.

So with pyOpenSSL 0.13 and current Twisted, server-side is ugly but possible and client-side is not possible at all.

	@glyph commented

Replying to exarkun:

Thanks for this update, and thanks even more for actually implementing the requisite SNI wrappers.

I think that we should probably have some explicit facility to do SNI so that it can get communicated down to layers like HTTP without OpenSSL specifics.

Also, anyone who takes on the client portion of this implementation should be aware of #5190.

	@wsanchez set owner to @glyph

Assigning this to Glyph while I’m still his boss.

	@hynek commented

Any plans to actually do this or are we going to leave txsni for a while and see how it evolves?

	@glyph commented

Oh yeah we should probably actually link to txsni here, shouldn't we.

	@glyph commented

Replying to hynek:

Any plans to actually do this or are we going to leave txsni for a while and see how it evolves?

I'd like to have SNI support within Twisted proper, but txsni is an extremely opinionated implementation and I'm not sure if we should go the same way. The TxSNI endpoint requires you to have a directory full of certificates, and assumes you're going to have the same factory backing each one. A fully flexible implementation would require some way to select which factory to use for each certificate, except that breaks the connectionMade-means-TCP-connection nature of our TLS APIs, because by that point you've already been through buildProtocol.

Also, txsni uses a synchronous mapping, which means certificates need to be loaded synchronously. Perhaps we would want certificates loaded asynchronously.

So, I don't know. Should we just roll txsni's endpoint in and then worry about these other issues later?

	@hynek commented

I’m starting to be rather positive about external dependencies to Twisted since it lowers the general barriers of contributions.

I would suggest the following:

• move txsni into the twisted organization to make it look more official
• add proper docs at RTFD (which explain the tradeoffs you just mentioned)
• …and a changelog
• give it a proper version that doesn’t look like a pre-alpha (add a development status that reflects that)
• link it from within the TLS docs because no-one will look here

IOW let it look less like a hack and more like an official thing to use and then let’s see what happens.

I would recommend you to just steal my general docs style from e.g. https://github.com/hynek/attrs/tree/master/docs it works very well and you just have to “fill in the blanks”.

	@mithrandi commented

This is slightly off-topic, but I just ran into an issue implementing SNI in my application that probably needs to be taken into consideration for any SNI implementation that makes its way into Twisted.

The verification settings (VERIFY_PEER, verify callback, etc.) are set on the context, but they are also per-connection state. The connection settings are initialized from the context settings when the connection is initialized, meaning that calling connection.set_context later does not update them. It is thus necessary to call connection.set_verify and possibly connection.set_verify_depth at the same time as connection.set_context, if you are intending to switch to a context with different verification settings.

	@glyph commented

Replying to mithrandi:

This is slightly off-topic, but I just ran into an issue implementing SNI in my application that probably needs to be taken into consideration for any SNI implementation that makes its way into Twisted.

The verification settings (VERIFY_PEER, verify callback, etc.) are set on the context, but they are also per-connection state. The connection settings are initialized from the context settings when the connection is initialized, meaning that calling connection.set_context later does not update them. It is thus necessary to call connection.set_verify and possibly connection.set_verify_depth at the same time as connection.set_context, if you are intending to switch to a context with different verification settings.

Oh wow. Uh, hrm. This is an interesting problem :-). Thank you very much for reporting it.

	evilalive commented

Is there any update on this ticket?

Implementing SNI in twisted would require major work or the implementation is already doable with the current codebase?

	@glyph commented

Replying to evilalive:

Is there any update on this ticket?

Nope. Would you like to contribute something? :)

Implementing SNI in twisted would require major work or the implementation is already doable with the current codebase?

It would definitely require work, but as txsni shows, you can definitely do it with the current code.

	@glyph commented

Also of interest: https://txacme.readthedocs.io/en/latest/

	@glyph commented

So, I guess the solution here really ought to be absorbing txsni, but only after txsni implements Deferred support - glyph/txsni#17