Document configuring TLS ciphers and log a link to it on raised handshake error #297
Conversation
consideRatio
commented
Nov 5, 2024
•
consideRatio
commented
- fixes TLJH upgrade causes SSLV3_ALERT_HANDSHAKE_FAILURE until ciphers explicitly configured #293
|
@manics I figured this made sense to include as well, it turns out that people can experience a breaking change with LDAPAuthenticator when they upgrade Python to 3.10 (TLJH 1->2 upgrade) and work against an LDAP server that only accepts older TLS ciphers. |
README.md
Outdated
| # default ciphers accepted with LDAPAuthenticator in Python < 3.10 | ||
| pre_python310_ciphers = "AES128-SHA:AES256-SHA:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA:ECDHE-ECDSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES128-SHA:ECDHE-RSA-AES256-SHA:AES128-GCM-SHA256:AES128-SHA256:AES256-GCM-SHA384:AES256-SHA256:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES128-SHA256:DHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES256-SHA256:DHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-CHACHA20-POLY1305:TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256" | ||
|
|
||
| # default ciphers accepted with LDAPAuthenticator in Python >= 3.10 |
There was a problem hiding this comment.
This may change again in a future Python release. Is there an external reference we can link to instead? If there's one for pre_python310_ciphers that'd be ideal too, otherwise we'll need a list of ciphers every time it changes.
Alternatively we could just link to the relevant GitHub issue #293 here?
There was a problem hiding this comment.
Is there an external reference we can link to instead?
I wrote this above, I didn't find something better to link to detailing exactly what ciphers are used without running this code snippet:
The default list of ciphers stem from ssl.create_default_context().get_ciphers().
If there's one for
pre_python310_ciphersthat'd be ideal too, otherwise we'll need a list of ciphers every time it changes.
Yes! There is a variable just above in the example named this :)
There was a problem hiding this comment.
I have btw checked that the ciphers has been the same before and after python 3.10 respectively for versions python 3.7 - 3.13.
There was a problem hiding this comment.
Sorry, I was unclear 😄. I know you've got both variables, I was more thinking of how to avoid the README getting infinitely long with information that isn't generally useful and in the worst case misleads people into thinking it's fine to just copy the configuration without understanding why (the ciphers will have been removed from Python for a good reason, and we don't want a lengthy explanation in the README of all the caveats).
How about merge and deal with it later?
There was a problem hiding this comment.
Ah okay yeah - its tricky since the readme is the sole place of documentation currently. I emphasized a lot with the challenge of debugging #293 and the challenge in finding a resolution, so it felt important to have something quite concrete.
In the end the cipher choice is determined by the intersection of accepted ciphers among LDAPAuthenticator and the LDAP server.
I'll do a final look to see if I can manage to make things more robust long term while being very concretely helpful for users with issues and then go for a merge.
There was a problem hiding this comment.
I made 56ceff6, I think this shouldn't require maintenance burden long term in any way.
consideRatio
force-pushed
the
pr/tls-handshake-error
branch
from
29a6fbc to
56ceff6
Compare
|
Thank you @manics for reviewing and helping this move forward!! ❤️ 🎉 |
