-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
gh-63284: Add support for TLS-PSK (pre-shared key) to the ssl module #103181
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
Conversation
7fab649
to
ad6c893
Compare
I figured out why it wasn't working with TLS 1.3. The error on the client-side is:
When initialising the SSLContext there is a call to
The openssl man pages state this is a "server side only" operation:
Calling this on a client-side socket seems to result in unexpected behavior, where the client thinks it's trying to resume a non-existent session. It looks like others have run into the same issue: maovidal/paho_sslpsk2_demo#2 (comment) Disabling the call to |
@jackjansen, @dstufft, @alex (as ssl module experts) |
ad6c893
to
b481191
Compare
SSL_CTX_set_session_id_context() is a server-side only operation. Using this on the client-side is causing authentication errors
b481191
to
cfddf7b
Compare
Modified the documentation for TLS 1.3. However this ticket explains that it is a change in TLS 1.3: openssl/openssl#8894 (comment)
|
Hey just giving this a bump |
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
RFC4279 states these are UTF-8. Add unit test using non-ASCII chars
The PR has missed the 3.12 merge window
I have made the requested changes; please review again |
I have made the requested changes; please review again |
Thanks for making the requested changes! @gpshead: please review the changes made to this pull request. |
@gpshead is there anything that still need to be done to see this PR moving forward? |
Hi @gpshead, it has been a while but I'm still interested in getting this merged. I have made the requested changes; please review again |
I'm also very interested in this feature. Feel free to tag me if you need any help |
Demonstrate usage in a safer way that rejects unknown connection attempts
Thanks for the contributon! FWIW the timestamp on the NEWS.d/next/ filename is irrelevant, it is only being used as a uniqueness collision avoidance mechanism. All of those get merged into one file during releases. |
Awesome!! Thanks for taking the time to review! |
…odule (python#103181) Add support for TLS-PSK (pre-shared key) to the ssl module. --------- Co-authored-by: Oleg Iarygin <oleg@arhadthedev.net> Co-authored-by: Gregory P. Smith <greg@krypto.org>
…odule (python#103181) Add support for TLS-PSK (pre-shared key) to the ssl module. --------- Co-authored-by: Oleg Iarygin <oleg@arhadthedev.net> Co-authored-by: Gregory P. Smith <greg@krypto.org>
Add support for TLS-PSK (pre-shared key) to the ssl module (plus documentation and unit-tests).
The referenced issue (#63284) is ~10 years old, but the information is still valid.
I've tested this by:
openssl s_server -accept localhost:12345 -psk deadbeef -cipher PSK -verify_return_error -nocert
openssl s_client -connect localhost:12345 -psk deadbeef -cipher PSK -verify_return_error
Things I'm still uncertain on:
It's not yet working with TLS 1.3 (I'm currently looking into this).[Fixed]Do I need to call[Fixed]PyErr_WriteUnraisable()
if there's a python exception during a C callback?This is my first python contribution.
I have probably got a few things wrong, but I'm happy to fix them up.
Thanks!