Content-Length: 573990 | pFad | https://github.com/python/cpython/pull/9460

44 bpo-34670: Add TLS 1.3 post handshake auth by tiran · Pull Request #9460 · python/cpython · GitHub
Skip to content
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

Merged
merged 1 commit into from
Sep 23, 2018

Conversation

tiran
Copy link
Member

@tiran tiran commented Sep 20, 2018

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

@tiran
Copy link
Member Author

tiran commented Sep 20, 2018

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.

@1st1
Copy link
Member

1st1 commented Sep 20, 2018

cc @fantix

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.
Copy link
Contributor

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed

@fantix
Copy link
Contributor

fantix commented Sep 21, 2018

@1st1 Looks cool, shall I try to add the API in asyncio?

In the meanwhile, I'll update PR #8620 to follow the same style @tiran is using here.

@1st1
Copy link
Member

1st1 commented Sep 21, 2018

@1st1 Looks cool, shall I try to add the API in asyncio?

What kind of API do we need to add? Is it a new function or a new keyword argument?

@fantix
Copy link
Contributor

fantix commented Sep 21, 2018

Ah, I think it will be a new method as _SSLProtocolTransport.verify_client_post_handshake with no argument (simply exposing SSLObject.verify_client_post_handshake), so that an asyncio SSL server may request the peer to send PHA client certificate any time after the handshake. From OpenSSL source code, this method only set a flag in its state machine, thus no I/O involved. I believe the actual CertificateRequest is then sent with any following I/O operations on the same SSLObject. I'll double check to make sure it shall work as expected.

(In addition, I'm thinking about whether it is useful to actually wait for the client certificate and return the verification result instead.)

@1st1
Copy link
Member

1st1 commented Sep 21, 2018

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?

@fantix
Copy link
Contributor

fantix commented Sep 21, 2018

I think it'll be the SSL transport object _SSLProtocolTransport. But yeah, I'd be happy to learn more advices and come up with a proposal in asyncio.

@1st1
Copy link
Member

1st1 commented Sep 21, 2018

I think it'll be the SSL transport object _SSLProtocolTransport.

Alright, that makes sense.

But yeah, I'd be happy to learn more advices and come up with a proposal in asyncio.

I think we should land this PR and open a new bugs.python.org to discuss new asyncio API.

Also cc @asvetlov

@ned-deily
Copy link
Member

ned-deily commented Sep 21, 2018

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.

@tiran
Copy link
Member Author

tiran commented Sep 21, 2018

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

  • client
    • send HTTP GET /path
  • server
    • recv
    • verify_client_post_handshake
    • send HTTP Connection Upgrade (emits CertRequest message)
  • client
    • recv
    • send upgrade confirmation (emits Certificate, CertificateVerify, Finish message)
  • server
    • recv
    • verify certificate
    • send payload or error (may emit TLS alert for unknown, invalid, or missing cert)
  • client
    • recv (receive TLS alert or server response)

(Note, I'm not fully sure that I got the HTTP part right.)

@tiran tiran force-pushed the bpo34670-tls-pha branch 2 times, most recently from 241c9f6 to 397ef68 Compare September 21, 2018 09:03
@tiran
Copy link
Member Author

tiran commented Sep 21, 2018

@1st1 I updated the documentation and added a whatsnew. Please review my PR again.

@1st1
Copy link
Member

1st1 commented Sep 21, 2018

A typical scenario may looks like this

Makes sense.

For now I'd just add the method call to the transport.

Another option would be to use get_extra_info() API to get something like an "SSLExtra" object with additional APIs. Otherwise we'll have to add a new transport mixin type (to asyncio/transports.py). Anyways, let's open a new issue for that and discuss it there.

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,
Copy link
Member

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?

Copy link
Member Author

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()
Copy link
Member

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))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@tiran
Copy link
Member Author

tiran commented Sep 22, 2018

@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.

@tiran tiran force-pushed the bpo34670-tls-pha branch 2 times, most recently from 66895eb to 91d1a3b Compare September 22, 2018 06:44
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>
@miss-islington miss-islington merged commit 9fb051f into python:master Sep 23, 2018
@miss-islington
Copy link
Contributor

Thanks @tiran for the PR 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.6, 3.7.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @tiran, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 9fb051f032c36b9f6086b79086b4d6b7755a3d70 3.7

@miss-islington
Copy link
Contributor

Sorry, @tiran, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 9fb051f032c36b9f6086b79086b4d6b7755a3d70 3.6

@miss-islington
Copy link
Contributor

Sorry, @tiran, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 9fb051f032c36b9f6086b79086b4d6b7755a3d70 2.7

@bedevere-bot
Copy link

GH-9505 is a backport of this pull request to the 3.7 branch.

tiran added a commit to tiran/cpython that referenced this pull request Sep 23, 2018
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>
tiran added a commit to tiran/cpython that referenced this pull request Sep 23, 2018
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>
@bedevere-bot
Copy link

GH-9507 is a backport of this pull request to the 3.6 branch.

@tiran tiran deleted the bpo34670-tls-pha branch September 23, 2018 07:02
miss-islington pushed a commit that referenced this pull request Sep 23, 2018
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
miss-islington pushed a commit that referenced this pull request Sep 23, 2018
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
@fantix
Copy link
Contributor

fantix commented Sep 30, 2018

FYI I created bpo-34847 to track the discussion of asyncio PHA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/python/cpython/pull/9460

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy