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.

Sign up for GitHub

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

Document configuring TLS ciphers and log a link to it on raised handshake error #297

Merged
merged 2 commits into from
Nov 6, 2024
Merged

Document configuring TLS ciphers and log a link to it on raised handshake error #297

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0327d41
Document configuring TLS ciphers and log a link to it on raised hands…
consideRatio Nov 5, 2024
56ceff6
Put handshake error docs in a dedicated README section
consideRatio Nov 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,27 @@ c.LDAPAuthenticator.tls_kwargs = {
}
```

If you have received a TLS handshake error, it could be that no cipher accepted
by LDAPAuthenticator is also accepted by the LDAP server. The default ciphers
accepted by LDAPAuthenticator is dependent on the Python version, where
upgrading to Python 3.10 is known to reduce the set of accepted ciphers. The
default list of ciphers stem from
[ssl.create_default_context().get_ciphers()](https://docs.python.org/3/library/ssl.html#ssl.create_default_context).

To configure LDAPAuthenticator's accepted ciphers explicitly, you can do:

```python
# 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
Copy link
Member

@manics manics Nov 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

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_ciphers that'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 :)

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

I made 56ceff6, I think this shouldn't require maintenance burden long term in any way.

post_python310_ciphers = "DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES128-SHA256:DHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES256-SHA256: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"

c.LDAPAuthenticator.tls_kwargs = {
"ciphers": pre_python310_ciphers,
}
```

#### `LDAPAuthenticator.server_port`

Port on which to contact the LDAP server.
Expand Down
13 changes: 12 additions & 1 deletion ldapauthenticator/ldapauthenticator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import ldap3
from jupyterhub.auth import Authenticator
from ldap3.core.exceptions import LDAPBindError
from ldap3.core.exceptions import LDAPBindError, LDAPSocketOpenError
from ldap3.core.tls import Tls
from ldap3.utils.conv import escape_filter_chars
from ldap3.utils.dn import escape_rdn
Expand Down Expand Up @@ -536,6 +536,17 @@ def get_connection(self, userdn, password):
password=password,
auto_bind=auto_bind,
)
except LDAPSocketOpenError as e:
if "handshake" in str(e).lower():
self.log.error(
"A TLS handshake failure has occurred. "
"It could be an indication that no cipher accepted by "
"LDAPAuthenticator was accepted by the LDAP server. For "
"details on how to handle this, refer to documentation of "
"the tls_kwargs config on how to configure ciphers "
"https://github.com/jupyterhub/ldapauthenticator/#ldapauthenticatortls_kwargs."
)
raise
except LDAPBindError as e:
self.log.debug(
"Failed to bind {userdn}\n{e_type}: {e_msg}".format(
Expand Down