Bug: LDAP user plugin not supporting TLS??? #631
Labels
bug
Something isn't working
Comments
payerle
added a commit
to payerle/coldfront
that referenced
this issue
Dec 9, 2024
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
added a commit
to payerle/coldfront
that referenced
this issue
Dec 10, 2024
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.
|
I have verified that the issue is present in ColdFront 1.1.6 (commit 0b96bd3) as well. I have cloned the latest ColdFront from git, added it to a virtual environment, set .env in site-packages to started a local webserver with coldfront runserver |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What happened?
I am running a somewhat old version of ColdFront v1.1.0 (I hope to start working on upgrading in the spring, and will be sending a lot of pull requests then) and I need to get TLS working with this version of ColdFront. I saw that the documentation for the current version mentions a config variable LDAP_USER_SEARCH_USE_TLS seems to do what I want, so I looked at LDAPUserSearch plugin from the github master in order to backport it to my current version. However, it looks to me like this is not working, both based on my reading of the code and on some (admittedly hacked) attempts to use the code.
Doing a grep of the latest version of ColdFront, the string LDAP_USER_SEARCH_USE_TLS is only used in
coldfront/plugins/ldap_user_search/utils.py to set the attribute LDAP_USE_TLS, and that attribute is only used in the utils.py to set the ldap3 Tls instance tls to specify client private key and/or certs to check against when creating the ldap3 Server instance.
In particular, the auto_bind parameter passed to the ldap3 Connection constructor is set to True. The use of boolean values for the auto_bind parameter of the ldap3 Connection class seems to be somewhat deprecated (at least no longer mentioned in the docs), but looking into the code it is still accepted for backwards compatibility, with the value True being converted to AUTO_BIND_NO_TLS.
I have also copied the LDAPUserPlugin class (from github master) into a script, with some hacks (e.g. replacing the import_from_settings with hard coded values), and running a test case of a lookup, and capturing traffic to the LDAP server, and TLS is not being used when the LDAP_USER_TLS parameter is set to True, but is used when the auto_bind parameter is set to AUTO_BIND_TLS_BEFORE _BIND.
I apologize that I do not currently have the resources to perform a less hacked test, but unless I am missing something big this does appear to be a mismatch between what the documentation is claiming and what the ColdFront plugin is doing.
If you confirm the bug, I can provide a pull request which should fix it (as I am developing a fix for my version anyway)
Version
1.1.1
Component
Other
What browsers are you seeing the problem on?
None
Relevant log output
No response
Tasks/ user tests when bug is fixed
The text was updated successfully, but these errors were encountered: