One of my peers at work noticed that mod_authnz_ldap returns a HTTP 500 code,
when the users sents an empty password.
After further investigation I noticed that this only happens in case the
authentication to the LDAP server happens via
AuthLDAPInitialBindAsUser / AuthLDAPInitialBindPattern and not via
AuthLDAPBindDN / AuthLDAPBindPassword (in this case it
correctly returns a HTTP 401 code).
I also found the following comment in line 1875 of modules/ldap/util_ldap.c:
/*
* A bind to the server with an empty password always succeeds, so
* we check to ensure that the password is not empty. This implies
* that users who actually do have empty passwords will never be
* able to authenticate with this module. I don't see this as a big
* problem.
*/
This causes the initial bind with the user credentials to succeed, but the
following ldap_search_ext_s to fail with
"Operations error".
Hence I would propose the following two patches:
1. Do not allow to set an empty bind password via AuthLDAPBindPassword
(no_empty_bind_password.diff).
2. In authn_ldap_check_password move the checks for NULL user / password up
(IMHO we cannot do anything sensible in case they
are NULL) in addition check if the password is empty and return an
AUTH_DENIED if this is the case. This would be similar to
the behavior in case AuthLDAPBindDN / AuthLDAPBindPassword is used
(no_empty_password_check.diff).
Opinions?
Regards
Rüdiger
Index: modules/aaa/mod_authnz_ldap.c
===================================================================
--- modules/aaa/mod_authnz_ldap.c (revision 1885229)
+++ modules/aaa/mod_authnz_ldap.c (working copy)
@@ -1719,6 +1719,10 @@
sec->bindpw = (char *)arg;
}
+ if (!(*sec->bindpw)) {
+ return "Empty passwords are invalid for AuthLDAPBindPassword";
+ }
+
return NULL;
}
Index: modules/aaa/mod_authnz_ldap.c
===================================================================
--- modules/aaa/mod_authnz_ldap.c (revision 1885229)
+++ modules/aaa/mod_authnz_ldap.c (working copy)
@@ -539,6 +539,32 @@
return AUTH_GENERAL_ERROR;
}
+ /* Get the password that the client sent */
+ if (password == NULL) {
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01692)
+ "auth_ldap authenticate: no password specified");
+ return AUTH_GENERAL_ERROR;
+ }
+
+ if (user == NULL) {
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01693)
+ "auth_ldap authenticate: no user specified");
+ return AUTH_GENERAL_ERROR;
+ }
+
+ /*
+ * A bind to the server with an empty password always succeeds, so
+ * we check to ensure that the password is not empty. This implies
+ * that users who actually do have empty passwords will never be
+ * able to authenticate with this module. I don't see this as a big
+ * problem.
+ */
+ if (!(*password)) {
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO()
+ "auth_ldap authenticate: empty password specified");
+ return AUTH_DENIED;
+ }
+
/* There is a good AuthLDAPURL, right? */
if (sec->host) {
const char *binddn = sec->binddn;
@@ -562,21 +588,6 @@
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01691)
"auth_ldap authenticate: using URL %s", sec->url);
- /* Get the password that the client sent */
- if (password == NULL) {
- ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01692)
- "auth_ldap authenticate: no password specified");
- release_ldc(r, ldc);
- return AUTH_GENERAL_ERROR;
- }
-
- if (user == NULL) {
- ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01693)
- "auth_ldap authenticate: no user specified");
- release_ldc(r, ldc);
- return AUTH_GENERAL_ERROR;
- }
-
/* build the username filter */
if (APR_SUCCESS != authn_ldap_build_filter(filtbuf, r, user, NULL, sec)) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02622)
