Repository navigation
gh-128035: Add ssl.HAS_PHA to detect libssl PHA support - #128036
gpshead merged 9 commits into
Conversation
Sorry, something went wrong.
tomasr8
left a comment
There was a problem hiding this comment.
This will also need a news entry :)
Sorry, something went wrong.
| @@ -0,0 +1 @@ | |||
| TLSv1.3 post-handshake client authentication (PHA), often referred to as "mutual TLS" or "mTLS", allows TLS servers to authenticate client identities using digital certificates. This commit exposes a boolean property ``ssl.HAS_PHA`` to indicate whether the crypto library CPython is built against supports PHA, allowing python's test suite and consuming modules to branch accordingly. | |||
There was a problem hiding this comment.
A shorter NEWS should be written. The NEWS is a message that users will see (it's in the changelog). Some suggestion:
Indicate through :data:`ssl.HAS_PHA` whether the :mod:`ssl` module supports TLSv1.3
post-handshake client authentication (PHA). Patch by YOURNAME.In addition, you should add a What's New entry in Doc/whatsnew/3.14.rst indicating the additional constant. Usually, the same message as for the NEWS entry can be reused (check the other entries for the formatting).
Sorry, something went wrong.
There was a problem hiding this comment.
Thank you for the guidance. I've updated the news and whatsnew files accordingly.
Sorry, something went wrong.
| * Indicate through :data:`ssl.HAS_PHA` whether the :mod:`ssl` module supports TLSv1.3 | ||
| post-handshake client authentication (PHA). (Contributed by Will Childs-Klein in | ||
| :gh:`128036`.) |
There was a problem hiding this comment.
This should go under Improved modules. You can make a new section for ssl:
ssl
---
* Indicate through :data:`ssl.HAS_PHA` whether the :mod:`ssl` module supports TLSv1.3
post-handshake client authentication (PHA). (Contributed by Will Childs-Klein in
:gh:`128036`.)
Sorry, something went wrong.
Co-authored-by: Tomas R. <tomas.roun8@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
picnixz
left a comment
There was a problem hiding this comment.
Considering the precedent on HAS_PSK and conditional blake2 detection for OpenSSL-like implementations that do not support it, this an ok change.
Some questions:
- is there an equivalent of OPENSSL_NO_PHA constant or not? (AFAIK, no, but are there some implementations that offer TLSv1.3 without PHA and would define OPENSSL_NO_PHA?)
- is there a better constant than SSL_VERIFY_POST_HANDSHAKE to check if PHA is supported or not? namely, is there an implementation which supports PHA (whether by default or not) but uses another constant?
cc @gpshead for a second look as well
Sorry, something went wrong.
| * Indicate through :data:`ssl.HAS_PHA` whether the :mod:`ssl` module supports | ||
| TLSv1.3 post-handshake client authentication (PHA). |
There was a problem hiding this comment.
| * Indicate through :data:`ssl.HAS_PHA` whether the :mod:`ssl` module supports | |
| TLSv1.3 post-handshake client authentication (PHA). | |
| * Indicate through :data:`ssl.HAS_PHA` whether the :mod:`ssl` module | |
| supports TLSv1.3 post-handshake client authentication (PHA). |
(just for reducing the line length and make it look more uniform, but this is entirely optional and a matter of taste; my previous suggestion may even have been incorrect in that regard).
Sorry, something went wrong.
gpshead
left a comment
There was a problem hiding this comment.
This makes sense to me. If there are better ways to determine this in the future, we can adapt to those. As with a lot of things libssl related... I feel like it can be configured via the environment in such ways that regardless of compile time checks capabilities can be changed at runtime via its configs. This constant only covers if the library could support it, but isn't able to express if it happens to be enabled or not within the current process. That is fine and is the expected simple thing to do.
Sorry, something went wrong.
…GH-128036) * Add ssl.HAS_PHA to detect libssl Post-Handshake-Auth support Co-authored-by: Tomas R. <tomas.roun8@gmail.com> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Notes
Please see #128035's description.
Testing
📚 Documentation preview 📚: https://cpython-previews--128036.org.readthedocs.build/