The Wayback Machine - https://web.archive.org/web/20221231113050/https://github.com/python/cpython/issues/95341
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

Jump to bottom

ssl module incorrectly supports tls-unique channel binding for TLS 1.3 #95341

Open
davidben opened this issue Jul 27, 2022 · 6 comments
Open

ssl module incorrectly supports tls-unique channel binding for TLS 1.3 #95341

davidben opened this issue Jul 27, 2022 · 6 comments
Labels
expert-SSL type-bug An unexpected behavior, bug, or error

Comments

@davidben
Copy link
Contributor

davidben commented Jul 27, 2022

CPython's get_channel_binding method implements the tls-unique channel binding for TLS 1.3:
https://github.com/python/cpython/blob/main/Lib/test/test_ssl.py#L671-L681
https://github.com/python/cpython/blob/main/Modules/_ssl.c#L2705

But this is incorrect. tls-unique is vulnerable to a couple of attacks (3SHAKE, SLOTH), so it was left undefined in TLS 1.3. RFC 9266 defines a replacement tls-exporter binding, built with Export Keying Material instead.
@davidben davidben added the type-bug An unexpected behavior, bug, or error label Jul 27, 2022
@Neustradamus
Copy link

Neustradamus commented Jul 27, 2022 •
edited

There are two possibilities for TLS Binding:
TLS =< 1.2: RFC5929: Channel Bindings for TLS: https://tools.ietf.org/html/rfc5929
TLS = 1.3: RFC9266: Channel Bindings for TLS 1.3: https://tools.ietf.org/html/rfc9266

Details:

  • tls-unique for TLS =< 1.2
  • tls-exporter for TLS = 1.3

Linked to:

@Neustradamus Neustradamus mentioned this issue Jul 27, 2022
Closed
tiran added a commit to tiran/cpython that referenced this issue Jul 28, 2022
@tiran
pythongh-95341: Implement tls-exporter channel bindings and export ke…
c84c553 
…y materials
@bedevere-bot bedevere-bot mentioned this issue Jul 28, 2022
Draft
@tiran
Copy link
Member

tiran commented Jul 28, 2022

Meh, what a mess. Thanks for notifying us, David.

I have started a PR to implement tls-exporter on top of export keying materials API. Of course OpenSSL makes it annoyingly complicated to check for presence of ext master secret for TLS 1.2 connections. I had to fall back to SSL_ctrl. Did I get the RFC right regarding context value? If I understand the RFC correctly, then it wants an empty context, not no context.

@Neustradamus Neustradamus mentioned this issue Jul 28, 2022
Closed
@davidben
Copy link
Contributor Author

davidben commented Jul 28, 2022

Of course OpenSSL makes it annoyingly complicated to check for presence of ext master secret for TLS 1.2 connections. I had to fall back to SSL_ctrl.

Hmm. Does SSL_get_extms_support not work?

@tiran
Copy link
Member

tiran commented Jul 28, 2022

Hmm. Does SSL_get_extms_support not work?

Oh, there is a function for that? I somehow missed the fact that OpenSSL has an API function for extms...

@tiran
Copy link
Member

tiran commented Jul 28, 2022

Ah, it is implemented as a macro! That's why I could neither find a symbol in libssl nor a function in ssl/*.c.

@davidben
Copy link
Contributor Author

davidben commented Jul 28, 2022

Yeah all the SSL_ctrl nonsense is macros. I tried to convince upstream to make them real functions, but no dice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
expert-SSL type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Neustradamus @davidben @tiran @kumaraditya303