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

Updates to support TLS in ldap_user_search plugin #643

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

payerle
Copy link
Contributor

@payerle payerle commented Dec 10, 2024

This PR should address concerns of #631

Basically:

  1. Adds new config parameter LDAP_USER_SEARCH_CERT_VALIDATE_MODE which is passed to the ldap3.Tls constructor as validate. Accepts as values:
    'required' : Certs are required and must validate
    'optional' : Certs are optional, but must validate if provided
    'none' (or None): Certs are ignored.
    The default is None

  2. The LDAP_USER_SEARCH_CERT_VALIDATE_MODE is passed as the validate field to the ldap3.Tls constructor

  3. If LDAP_USE_TLS is set, we pass the connection parameter 'auto_bind' as ldap3.AUTO_BIND_TLS_BEFORE_BIND instead of simply True Inspection of ldap3 code shows that when this parameter is set to True (a value which is no longer listed in docs as valid) it is treated as AUTO_BIND_NO_TLS, so the previous before of leaving this as True was not doing TLS despite claiming to do TLS. This fix should change that.

This PR should address concerns of ubccr#631

Basically:
1) Adds new config parameter LDAP_USER_SEARCH_CERT_VALIDATE_MODE
which is passed to the ldap3.Tls constructor as validate.
Accepts as values:
	'required' : Certs are required and must validate
	'optional' : Certs are optional, but must validate if provided
	'none' (or None): Certs are ignored.
The default is None

2) The LDAP_USER_SEARCH_CERT_VALIDATE_MODE is passed as the validate
field to the ldap3.Tls constructor

3) If LDAP_USE_TLS is set, we pass the connection parameter 'auto_bind'
as ldap3.AUTO_BIND_TLS_BEFORE_BIND instead of simply True
Inspection of ldap3 code shows that when this parameter is set
to True (a value which is no longer listed in docs as valid) it
is treated as AUTO_BIND_NO_TLS, so the previous before of leaving
this as True was not doing TLS despite claiming to do TLS.
This fix should change that.
@payerle
Copy link
Contributor Author

payerle commented Dec 10, 2024

The above PR should address this issue. Running under coldfront runserver with same config file, the tcpdump shows the LDAP query as encrypted. When repeated with bind_dn and passwd, the encryption is starting before the bind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant