[issue43582] SSLContext.sni_callback docs inaccurately describe available handshake info
New submission from Andrew Dailey <steveday168@gmail.com>: Hello, The documentation for SSLContext.sni_callback [0] seems to incorrectly describe the information available at that stage of the TLS handshake. According to the docs: Due to the early negotiation phase of the TLS connection, only limited methods and attributes are usable like SSLSocket.selected_alpn_protocol() and SSLSocket.context. SSLSocket.getpeercert(), SSLSocket.getpeercert(), SSLSocket.cipher() and SSLSocket.compress() methods require that the TLS connection has progressed beyond the TLS Client Hello and therefore will not contain return meaningful values nor can they be called safely. This paragraph claims that SSLSocket.selected_alpn_protocol() should be usable within sni_callback but I think this is inaccurate. Based on the OpenSSL docs [1] and my own testing, the servername callback occurs after ClientHello but _before_ the ALPN callback. This prevents accurate ALPN information from being available until later. I believe that any call to SSLSocket.selected_alpn_protocol() within an SSLContext.sni_callback will simply return None. Excerpt from the OpenSSL docs: Several callbacks are executed during ClientHello processing, including the ClientHello, ALPN, and servername callbacks. The ClientHello callback is executed first, then the servername callback, followed by the ALPN callback. I think it'd be better to explain that the only "useful" thing you can do within sni_callback is to see what sni_name is desired an optionally swap out the context for one with a more appropriate cert chain. Any information about the selected ALPN protocol has to wait until later in the handshake. [0] https://docs.python.org/3/library/ssl.html#ssl.SSLContext.sni_callback [1] https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_tlsext_servername_cal... ---------- assignee: docs@python components: Documentation, SSL messages: 389231 nosy: docs@python, theandrew168 priority: normal severity: normal status: open title: SSLContext.sni_callback docs inaccurately describe available handshake info type: enhancement versions: Python 3.8, Python 3.9 _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue43582> _______________________________________
Change by Andrew Dailey <steveday168@gmail.com>: ---------- nosy: +christian.heimes _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue43582> _______________________________________
Christian Heimes <lists@cheimes.de> added the comment: I analysed the issue in comment https://bugs.python.org/issue43577#msg389222 ---------- nosy: +alex, dstufft, janssen, njs versions: +Python 3.10 _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue43582> _______________________________________
Christian Heimes <lists@cheimes.de> added the comment: I don't see any way to fix the issue with our current API. OpenSSL 1.1.1 provides a new API SSL_client_hello_get0_ext() to access raw extension during early stage of ClientHello phase. https://www.openssl.org/docs/man1.1.1/man3/SSL_client_hello_get1_extensions_... ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue43582> _______________________________________
Andrew Dailey <steveday168@gmail.com> added the comment: Yea, I'm still on the hunt for a better way to solve my primary problem: detect an acme-tls/1 ALPN protocol request during the TLS handshake so that I can swap out the context to one with the cert chain that Let's Encrypt is expecting to see. It seems like OpenSSL provides three primary hooks into the handshake: ClientHello, servername, and ALPN. The servername callback is the only one that can be "officially" customized by Python's SSL API. The ALPN callback seems to be used under the hood to implement SSLContext.set_alpn_protocols() but there isn't a way to specify complete control of the callback. My current "hack" is to use the SSLContext._msg_callback to check for the acme-tls/1 protocol explicitly: def msg_callback(conn, direction, version, content_type, msg_type, data): if direction == 'read' and b'acme-tls/1' in data: print('got an acme-tls/1 request') print('set a flag for sni_callback to check, etc etc') I know this probably isn't a good or safe way to solve the problem. The current docs make it sound like sni_callback would be my one-stop shop but that ended up not being the case. Maybe I could subclass SSLSocket, override do_handshake(), and then swap out the context before or after super().do_handshake()? I'm quite new to Python/OpenSSL internals so I'm not sure if that is even possible. Can a context be swapped out so late in the handshake process? The SSL_client_hello_get0_ext() function you mentioned could be a contender. The _msg_callback I'm currently using _does_ do the trick but maybe shouldn't be documented and made official? Regardless of how best to solve my current acme-tls/1 ALPN detection issue, the sni_callback won't ever be the full answer unless some internal mechanics are added to watch ClientHello and preemptively peek at the requested ALPN protocol(s). ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue43582> _______________________________________
Christian Heimes <lists@cheimes.de> added the comment: SSLContext.set_alpn_protocols() is a high level interface on top of SSL_CTX_set_alpn_select_cb(). Python doesn't directly expose the ALPN selector callback. The ssl module only provides a way to set a hard-coded callback that wraps SSL_select_next_proto(). https://github.com/python/cpython/blob/9a50ef43e42ee32450a81ce13ed5a0729d3b8... https://github.com/python/cpython/blob/9a50ef43e42ee32450a81ce13ed5a0729d3b8... To solve your problem Python would have to support something like: def alpn_cb(sslsocket, client_alpn_protocols, negotiated): return negotiated context.alpn_cb = alpn_cb ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue43582> _______________________________________
Andrew Dailey <steveday168@gmail.com> added the comment: Yea, I noticed that through some of my digging. The ALPN callback is used to implement SSLContext.set_alpn_protocols() but full control of the callback isn't exposed. Aside from adjusting how the ALPN callback used, do you know of any other way to swap contexts once the selected ALPN proto is known but not before it's too late? As I said before, I'm not super familiar with Python / OpenSSL internals but maybe overriding SSLSocket.do_handshake() would suffice? I don't want this issue to get too far off track. I'm still doing research on how I'd go about drafting and submitting a formal patch here on the issue tracker. I'm new to this process but definitely want to help out as much as I can. Here's my current idea for how to adjust the documentation given the current behavior / capabilities. CURRENT: Due to the early negotiation phase of the TLS connection, only limited methods and attributes are usable like SSLSocket.selected_alpn_protocol() and SSLSocket.context. SSLSocket.getpeercert(), SSLSocket.getpeercert(), SSLSocket.cipher() and SSLSocket.compress() methods require that the TLS connection has progressed beyond the TLS Client Hello and therefore will not contain return meaningful values nor can they be called safely. REVISED: Based on the value of `sni_name`, a new SSLContext can be created and attached to the current SSLSocket. Due to the early negotiation phase of the TLS connection, only the Client Hello will have occurred by the time this callback is called. Methods and attributes such as SSLSocket.selected_alpn_protocol(), SSLSocket.getpeercert(), SSLSocket.cipher(), and SSLSocket.compress() require that the TLS connection has progressed beyond the TLS Client Hello and therefore will not contain return meaningful values nor can they be called safely. ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue43582> _______________________________________
Christian Heimes <lists@cheimes.de> added the comment: The callback from context.alpn_callback would fire when OpenSSL handles the ALPN extension. Since the callback is triggered in the ClientHello phase of the handshake, you'll be able to replace the socket's context with another context. The OpenSSL codes work a bit like this: TLSEXT_INDEX = [ ..., TLSEXT_IDX_server_name, ..., TLSEXT_IDX_application_layer_protocol_negotiation, ] for extid in TLSEXT_INDEX: if client.has_extension(extid): ext = do_stuff(client, extid) ext.callback(client) Any of the callbacks is able to replace the context. The process for contributing to Python is explained in the devguide: https://devguide.python.org/. Please start by signing a contributor agreement. Then you can file a PR on Github. ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue43582> _______________________________________
Andrew Dailey <steveday168@gmail.com> added the comment: Okay, that makes sense. I appreciate the overview! What do you feel is the best way to "solve" this issue, then? Should the docs be updated to reflect the fact that ALPN info isn't available in the sni_callback? Or should some code be modified to make the docs correct (even though that'd have to be a bit hacky since the OpenSSL handshake callback order seems fairly set in stone). I've got the contributor agreement signed and ready to go. I can formalize my ideas for revising the docs into a patch if that would make sense and be helpful. ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue43582> _______________________________________
participants (2)
-
Andrew Dailey -
Christian Heimes