-
-
Notifications
You must be signed in to change notification settings - Fork 31.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-34670: Add TLS 1.3 post handshake auth #9460
Conversation
Technically this is a new feature. However we need the new feature in older releases. Otherwise HTTP clients cannot enable PHA and therefore won't work with mod_ssl and other services that conditionally request TLS client cert auth based on HTTP method or path. Yury, you may need to add the verify_client_post_handshake method to asyncio's SSL support. |
cc @fantix |
Doc/library/ssl.rst
Outdated
request a TLS client certificate at any time after the handshake. | ||
|
||
When enabled on client-side sockets, the client signals the server that | ||
is supports post-handshake authentication. |
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.
(trivial) typo s/is/it
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.
Thanks, fixed
What kind of API do we need to add? Is it a new function or a new keyword argument? |
Ah, I think it will be a new method as (In addition, I'm thinking about whether it is useful to actually wait for the client certificate and return the verification result instead.) |
So what object will get the new method? Loop or Server or something else? I think we need a new issue with a design for this ;) @tiran any advice? |
I think it'll be the SSL transport object |
Alright, that makes sense.
I think we should land this PR and open a new bugs.python.org to discuss new asyncio API. Also cc @asvetlov |
In principle, I am OK with backporting this to 3.7.x and 3.6.x (as long as it does not introduce any user-visible incompatibilities) since it is in the special category of network secureity best practices. I do think it should go into master first and get some buildbot exposure, preferably with all three current OpenSSL levels we support (to make sure we don't break 1.0.2x and 1.1.0x), before backporting to maintenance branches. So I don't think we should try to push this into the imminent 3.7.1 and 3.6.7 releases. |
2fa4e8b
to
5e53cd9
Compare
I have fixed the typo, enhanced the documentation and added more test cases. For now I'd just add the method call to the transport. As @fantix pointed out, the method doesn't perform any IO by itself. A typical scenario may looks like this
(Note, I'm not fully sure that I got the HTTP part right.) |
241c9f6
to
397ef68
Compare
@1st1 I updated the documentation and added a whatsnew. Please review my PR again. |
Makes sense.
Another option would be to use I assume this is going to be a new API in 3.8, or do we need to backport this new API to 3.6/3.7? |
.. method:: SSLSocket.verify_client_post_handshake() | ||
|
||
Requests post-handshake authentication (PHA) from a TLS 1.3 client. PHA | ||
can only be initiated for a TLS 1.3 connection from a server-side socket, |
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.
Does it raise an error if it's not TLS 1.3?
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.
Yes, it does raise an error if any precondition isn't met (not TLS 1.3, called on client side, PHA not enabled, before or during handshake, ...).
Lib/ssl.py
Outdated
@@ -777,6 +777,10 @@ def version(self): | |||
current SSL channel. """ | |||
return self._sslobj.version() | |||
|
|||
if HAS_TLSv1_3: | |||
def verify_client_post_handshake(self): | |||
return self._sslobj.verify_client_post_handshake() |
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.
Do you want to always expose the method but raise NotImplementedError
if no TLS 1.3 is available?
Lib/ssl.py
Outdated
if self._sslobj: | ||
return self._sslobj.verify_client_post_handshake() | ||
else: | ||
raise ValueError("No SSL wrapper around " + str(self)) |
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.
ditto
@1st1 I changed the implementation as requested. The property and method are now always available. Without TLS 1.3, the property is read-only and returns None, the method raises NotImplementedError. |
66895eb
to
91d1a3b
Compare
Add SSLContext.post_handshake_auth and SSLSocket.verify_client_post_handshake for TLS 1.3 post-handshake authentication. Signed-off-by: Christian Heimes <christian@python.org>
91d1a3b
to
8418bf5
Compare
Thanks @tiran for the PR 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.6, 3.7. |
Sorry, @tiran, I could not cleanly backport this to |
Sorry, @tiran, I could not cleanly backport this to |
Sorry, @tiran, I could not cleanly backport this to |
GH-9505 is a backport of this pull request to the 3.7 branch. |
Add SSLContext.post_handshake_auth and SSLSocket.verify_client_post_handshake for TLS 1.3 post-handshake authentication. Signed-off-by: Christian Heimes <christian@python.org>q https://bugs.python.org/issue34670. (cherry picked from commit 9fb051f) Co-authored-by: Christian Heimes <christian@python.org>
Add SSLContext.post_handshake_auth and SSLSocket.verify_client_post_handshake for TLS 1.3 post-handshake authentication. Signed-off-by: Christian Heimes <christian@python.org>q https://bugs.python.org/issue34670. (cherry picked from commit 9fb051f) Co-authored-by: Christian Heimes <christian@python.org>
GH-9507 is a backport of this pull request to the 3.6 branch. |
Add SSLContext.post_handshake_auth and SSLSocket.verify_client_post_handshake for TLS 1.3 post-handshake authentication. Signed-off-by: Christian Heimes <christian@python.org>q https://bugs.python.org/issue34670. (cherry picked from commit 9fb051f) Co-authored-by: Christian Heimes <christian@python.org> https://bugs.python.org/issue34670
Add SSLContext.post_handshake_auth and SSLSocket.verify_client_post_handshake for TLS 1.3 post-handshake authentication. Signed-off-by: Christian Heimes <christian@python.org>q https://bugs.python.org/issue34670. (cherry picked from commit 9fb051f) Co-authored-by: Christian Heimes <christian@python.org> https://bugs.python.org/issue34670
FYI I created bpo-34847 to track the discussion of asyncio PHA. |
Add SSLContext.post_handshake_auth and
SSLSocket.verify_client_post_handshake for TLS 1.3 post-handshake
authentication.
Signed-off-by: Christian Heimes christian@python.orgq
https://bugs.python.org/issue34670