GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-128035: Add ssl.HAS_PHA to detect libssl PHA support by WillChilds-Klein · Pull Request #128036 · python/cpython Conversation
| ||||||||||||||||||||||||||||||
| @@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the guidance. I've updated the news and whatsnew files accordingly.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
-
👍 1 reaction
| * 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
-
👍 1 reaction
7f19054 to
6b11e12
Compare
Co-authored-by: Tomas R. <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
6b11e12 to
63df081
Compare
picnixz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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_PHAconstant 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_HANDSHAKEto 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.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
-
👍 1 reaction
| * 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * 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.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
gpshead
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
-
👍 2 reactions
…GH-128036) * Add ssl.HAS_PHA to detect libssl Post-Handshake-Auth support Co-authored-by: Tomas R. <[email protected]> Co-authored-by: Bénédikt Tran <[email protected]>
Reviewers
Uh oh!
There was an error while loading. Please reload this page.
Assignees
Labels
Projects
Milestone
No milestoneDevelopment
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
4 participants
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.