[#4887] add tls: server endpoint that supports SNI, PEM parsing, and supporting code that changes OpenSSL connection/context building
#11861
Conversation
as silly as 'IOpenSSLServerConnectionCreatorFactory' sounds as an interface name, there is a real problem with the previous structure: Context objects are configured by *every connection* with new ALPN/NPN parameters, which is not the way that contexts are supposed to work. The context configuration should happen once per-context per-*factory*, and the connection configuration should happen once per *connection*. However, 'per-context' is important, because an implementation of SNI requires that the context configuration happen on the creation of new contexts for other hostnames that may not have been loaded yet, not just the first context. so it is being called both too often and not often enough, and at the wrong time. client-side it's still worth doing this for symmetry, although the practical implications are somewhat less pronounced, since you just call the context methods a bit too often.
glyph
changed the title
tls: endpoint that supports SNI, PEM parsing, and supporting code that changes OpenSSL connection/context building
glyph
changed the title
tls: endpoint that supports SNI, PEM parsing, and supporting code that changes OpenSSL connection/context buildingtls: server endpoint that supports SNI, PEM parsing, and supporting code that changes OpenSSL connection/context building
supporting wildcards and multi-SAN certs, what was I thinking
There was a problem hiding this comment.
I left a few commnents.
I am not very familiar with the serverFromString API.
I was expecting to see more documentation in this branch, covering how or why to use it.
I don't know when I will have time to fully review this.
I think that as long as it has good documentation and code coverage, we can merge it.
If anyone else is interested in this API, we can update it later.
Thanks again.
| contextSetupHook: Callable[[SSL.Context], None], | ||
| ) -> IOpenSSLClientConnectionCreator: | ||
| """ | ||
| Create a client creator. |
There was a problem hiding this comment.
This is private ... so not a big name
but maybe we can have a better docstring.
To me, this docstring doesn't add any more info that I alreay got by reading the function signature def createClientCreator
Why not call it "factory" ? like getClientOptionsFactory ?
just asking :)
There was a problem hiding this comment.
This code really suffers from a bunch of iterations of evolving an interface, where they all kind of do the same thing but progressively more correctly. OptionsFactory2, OptionsFactory3 looks ugly and doesn't express the differences between them, so I tried to find synonyms ("creator" rather than "factory") to help draw the distinction
src/twisted/internet/_sslverify.py
Outdated
| # Note: remember client options for testing. See | ||
| # twisted.internet.test.test_endpoints.tlsHostnameFromEndpoint | ||
| self._ctx._clientOptions = self |
There was a problem hiding this comment.
Maybe something like this
"remember client for testing" seemls like a task for the developers
| # Note: remember client options for testing. See | |
| # twisted.internet.test.test_endpoints.tlsHostnameFromEndpoint | |
| self._ctx._clientOptions = self | |
| # Note: To help with testing, we keep a reference to the TLS options. | |
| $ See | |
| # twisted.internet.test.test_endpoints.tlsHostnameFromEndpoint | |
| self._ctx._clientOptions = self |
|
I had to deal with Twisted context factory about 10 years ago. @glyph if you have time, maybe we can do a 15-30 minutes voice/screen sharing meeting and look into giving some feedack. I am a bit lost into all the SSL context wrapping API. |
That would be great. |
clean up implementation considerably, using verify callback like we are supposed to, not overwriting app_data, not overwrititng info_callback on the context
…ble to IOpenSSLTrustRoot
Scope and purpose
Fixes #4887
This also makes it possible to do
twist web --path whatever --listen=tls:path/to/certbot/config/liveifpath/to/certbot/config/liveis a directory containing.pemfiles contining private keys, certificates, and chains in any layout (as, for example, the certbot live config dir does.