[server] Verify TLS certificate in LDAPS connections #3083
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
csordasmarton
suggested changes
Dec 9, 2020
jimis
force-pushed
the
ldaps_fix_cert_verify-gh3082
branch
from
96cb08f to
cc8eb8d
Compare
|
Rebased and pushed update according to my comment above (default behaviour is secure, compatibility is broken). I also fixed an irrelevant typo that kind of proves my point: the LDAP module is not widely used yet, so it's better to break compatibility at this early stage and enforce secure LDAPS connections by default. Please let me know what you think. Do you prefer insecure default behaviour by default? |
If the certificate verification fails while attempting to connect to an
LDAPS server, then the ldap library returns the misleading error type
SERVER_DOWN with unhelpful details:
{'desc': "Can't contact LDAP server", 'info': '(unknown error code)'}
We now print a more relevant message.
jimis
force-pushed
the
ldaps_fix_cert_verify-gh3082
branch
from
cc8eb8d to
a2eb955
Compare
csordasmarton
suggested changes
Jan 8, 2021
If LDAP authentication is used in the server and the LDAP server connection is using LDAPS, then reject the connection if the certificate is not trusted by the system. This can be overriden by setting the tls_require_cert="never" in the configuration. NOTE: The option used before to skip the SSL certificate verification was OPT_X_TLS_ALLOW, which is documented in python-ldap as "used internally by slapd server". Therefore it was replaced by OPT_X_TLS_NEVER.
This option does not look like it ever worked because of this small typo.
jimis
force-pushed
the
ldaps_fix_cert_verify-gh3082
branch
from
a2eb955 to
7f8119e
Compare
|
Updated, thanks for the review. |
csordasmarton
approved these changes
Jan 11, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If LDAP authentication is used in the server and the LDAP server
connection is using LDAPS, then reject the connection if the certificate
is not trusted by the system.
This can be overriden by setting the environment variable
LDAPTLS_REQCERT=never just like in the ldapsearch command.
NOTE: The option used before to skip the SSL certificate verification
was OPT_X_TLS_ALLOW, which is documented in python-ldap as "used
internally by slapd server". Thus the one used now is OPT_X_TLS_NEVER.
Resolves #3082