From 784d92f0742b9d960f702dc02a3a4b4b636b32da Mon Sep 17 00:00:00 2001
From: Eric Covener <covener@apache.org>
Date: Sun, 6 Jul 2014 14:06:50 +0000
Subject: [PATCH] Consolidate common code that got duplicated by 2.3.x authz
 refactoring.

Arrange for backend LDAP connections to be returned
to the pool by a fixup hook rather than staying locked
until the end of (a potentially slow) request.

Add a little more trace4 to the authnz_ldap side of LDAP connection obtain/release.




git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1608202 13f79535-47bb-0310-9956-ffa450edef68
---
 CHANGES                       |   3 +
 modules/aaa/mod_authnz_ldap.c | 367 ++++++++++++++--------------------
 2 files changed, 152 insertions(+), 218 deletions(-)

diff --git a/CHANGES b/CHANGES
index a0ed7ca99e..e60027e672 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) mod_authnz_ldap: Return LDAP connections to the pool before the handler
+     is run, instead of waiting until the end of the request. [Eric Covener]
+
   *) mod_ldap: Be more conservative with the last-used time for
      LDAPConnectionPoolTTL. PR54587 [Eric Covener]
 
diff --git a/modules/aaa/mod_authnz_ldap.c b/modules/aaa/mod_authnz_ldap.c
index 11bc73539e..17607d02be 100644
--- a/modules/aaa/mod_authnz_ldap.c
+++ b/modules/aaa/mod_authnz_ldap.c
@@ -88,6 +88,7 @@ typedef struct {
     char *user;                     /* The username provided by the client */
     const char **vals;              /* The additional values pulled during the DN search*/
     char *password;                 /* if this module successfully authenticates, the basic auth password, else null */
+    apr_pool_t *ldc_pool;           /* a short-lived pool to trigger cleanups on any acquired LDCs */
 } authn_ldap_request_t;
 
 enum auth_ldap_phase {
@@ -115,7 +116,6 @@ static APR_OPTIONAL_FN_TYPE(uldap_ssl_supported) *util_ldap_ssl_supported;
 static apr_hash_t *charset_conversions = NULL;
 static char *to_charset = NULL;           /* UTF-8 identifier derived from the charset.conv file */
 
-
 /* Derive a code page ID give a language name or ID */
 static char* derive_codepage_from_lang (apr_pool_t *p, char *language)
 {
@@ -361,6 +361,7 @@ static void *create_authnz_ldap_dir_config(apr_pool_t *p, char *d)
 static apr_status_t authnz_ldap_cleanup_connection_close(void *param)
 {
     util_ldap_connection_t *ldc = param;
+    ap_log_rerror(APLOG_MARK, APLOG_TRACE4, 0, ldc->r, "Release ldc %pp", ldc);
     util_ldap_connection_close(ldc);
     return APR_SUCCESS;
 }
@@ -423,20 +424,28 @@ static const char *ldap_determine_binddn(request_rec *r, const char *user) {
 }
 
 
-/* Some LDAP servers restrict who can search or compare, and the hard-coded ID
- * might be good for the DN lookup but not for later operations.
+/** 
+ * Some LDAP servers restrict who can search or compare, and the hard-coded ID
+ * might be good for the DN lookup but not for later operations. 
+ * Requires the per-request config be set to ensure the connection is cleaned up
  */
 static util_ldap_connection_t *get_connection_for_authz(request_rec *r, enum auth_ldap_optype type) {
     authn_ldap_request_t *req =
         (authn_ldap_request_t *)ap_get_module_config(r->request_config, &authnz_ldap_module);
     authn_ldap_config_t *sec =
         (authn_ldap_config_t *)ap_get_module_config(r->per_dir_config, &authnz_ldap_module);
+    util_ldap_connection_t *ldc = NULL;
 
     char *binddn = sec->binddn;
     char *bindpw = sec->bindpw;
 
-    /* If the per-request config isn't set, we didn't authenticate this user, and leave the default credentials */
-    if (req && req->password &&
+    if (!req) { 
+        ap_log_rerror(APLOG_MARK, APLOG_CRIT, 0, r, "module error: get_connection_for_authz without per-request config");
+        return NULL;
+    }
+
+    /* If the password isn't set in the per-request config , we didn't authenticate this user, and leave the default credentials */
+    if (req->password &&
          ((type == LDAP_SEARCH && sec->search_as_user)    ||
           (type == LDAP_COMPARE && sec->compare_as_user)  ||
           (type == LDAP_COMPARE_AND_SEARCH && sec->compare_as_user && sec->search_as_user))){
@@ -444,10 +453,39 @@ static util_ldap_connection_t *get_connection_for_authz(request_rec *r, enum aut
             bindpw = req->password;
     }
 
-    return util_ldap_connection_find(r, sec->host, sec->port,
+    ldc = util_ldap_connection_find(r, sec->host, sec->port,
                                      binddn, bindpw,
                                      sec->deref, sec->secure);
+
+    ap_log_rerror(APLOG_MARK, APLOG_TRACE4, 0, r, "Obtain ldc %pp for authz", ldc);
+    apr_pool_cleanup_register(req->ldc_pool, ldc,
+                              authnz_ldap_cleanup_connection_close,
+                              apr_pool_cleanup_null);
+    return ldc;
+}
+
+
+static authn_ldap_request_t* build_request_config(request_rec *r) { 
+    authn_ldap_request_t *req =
+        (authn_ldap_request_t *)apr_pcalloc(r->pool, sizeof(authn_ldap_request_t));
+    ap_set_module_config(r->request_config, &authnz_ldap_module, req);
+    apr_pool_create(&(req->ldc_pool), r->pool);
+    ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01740)
+                 "ldap authorize: Creating LDAP req structure");
+    return req;
 }
+
+static void release_ldc(request_rec *r, util_ldap_connection_t *ldc)
+{
+    authn_ldap_request_t *req =
+        (authn_ldap_request_t *)ap_get_module_config(r->request_config, &authnz_ldap_module);
+
+    if (req) { 
+        apr_pool_cleanup_kill(req->ldc_pool, ldc, authnz_ldap_cleanup_connection_close);
+    }
+    authnz_ldap_cleanup_connection_close(ldc);
+}
+
 /*
  * Authentication Phase
  * --------------------
@@ -466,14 +504,12 @@ static authn_status authn_ldap_check_password(request_rec *r, const char *user,
         (authn_ldap_config_t *)ap_get_module_config(r->per_dir_config, &authnz_ldap_module);
 
     util_ldap_connection_t *ldc = NULL;
+    authn_ldap_request_t *req = NULL;
     int result = 0;
     int remote_user_attribute_set = 0;
     const char *dn = NULL;
     const char *utfpassword;
 
-    authn_ldap_request_t *req =
-        (authn_ldap_request_t *)apr_pcalloc(r->pool, sizeof(authn_ldap_request_t));
-    ap_set_module_config(r->request_config, &authnz_ldap_module, req);
 
 /*
     if (!sec->enabled) {
@@ -500,6 +536,7 @@ static authn_status authn_ldap_check_password(request_rec *r, const char *user,
             binddn = ldap_determine_binddn(r, user);
         }
 
+        req = build_request_config(r);
         ldc = util_ldap_connection_find(r, sec->host, sec->port,
                                        binddn, bindpw,
                                        sec->deref, sec->secure);
@@ -517,14 +554,14 @@ static authn_status authn_ldap_check_password(request_rec *r, const char *user,
     if (password == NULL) {
         ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01692)
                       "auth_ldap authenticate: no password specified");
-        util_ldap_connection_close(ldc);
+        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");
-        util_ldap_connection_close(ldc);
+        release_ldc(r, ldc);
         return AUTH_GENERAL_ERROR;
     }
 
@@ -533,7 +570,7 @@ static authn_status authn_ldap_check_password(request_rec *r, const char *user,
         ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02622)
                       "auth_ldap authenticate: ldap filter too long (>%d): %s",
                       FILTER_LENGTH, filtbuf);
-        util_ldap_connection_close(ldc);
+        release_ldc(r, ldc);
         return AUTH_GENERAL_ERROR;
     }
 
@@ -547,7 +584,7 @@ static authn_status authn_ldap_check_password(request_rec *r, const char *user,
     result = util_ldap_cache_checkuserid(r, ldc, sec->url, sec->basedn, sec->scope,
                                          sec->attributes, filtbuf, utfpassword,
                                          &dn, &(req->vals));
-    util_ldap_connection_close(ldc);
+    release_ldc(r, ldc);
 
     /* handle bind failure */
     if (result != LDAP_SUCCESS) {
@@ -619,6 +656,40 @@ static authn_status authn_ldap_check_password(request_rec *r, const char *user,
     return AUTH_GRANTED;
 }
 
+static authz_status get_dn_for_nonldap_authn(request_rec *r, util_ldap_connection_t *ldc)
+{
+    apr_status_t result = APR_SUCCESS;
+    char filtbuf[FILTER_LENGTH];
+    authn_ldap_request_t *req =
+        (authn_ldap_request_t *)ap_get_module_config(r->request_config, &authnz_ldap_module);
+    authn_ldap_config_t *sec =
+        (authn_ldap_config_t *)ap_get_module_config(r->per_dir_config, &authnz_ldap_module);
+    const char *dn = NULL;
+
+    /* Build the username filter */
+    if (APR_SUCCESS != authn_ldap_build_filter(filtbuf, r, r->user, NULL, sec)) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02623)
+                "auth_ldap authorize: ldap filter too long (>%d): %s",
+                FILTER_LENGTH, filtbuf);
+        return AUTHZ_DENIED;
+    }
+
+    /* Search for the user DN */
+    result = util_ldap_cache_getuserdn(r, ldc, sec->url, sec->basedn,
+            sec->scope, sec->attributes, filtbuf, &dn, &(req->vals));
+
+    /* Search failed, log error and return failure */
+    if (result != LDAP_SUCCESS) {
+        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01701)
+                "auth_ldap authorise: User DN not found, %s", ldc->reason);
+        return AUTHZ_DENIED;
+    }
+
+    req->dn = apr_pstrdup(r->pool, dn);
+    req->user = r->user;
+    return AUTHZ_GRANTED;
+}
+
 static authz_status ldapuser_check_authorization(request_rec *r,
                                                  const char *require_args,
                                                  const void *parsed_require_args)
@@ -638,8 +709,6 @@ static authz_status ldapuser_check_authorization(request_rec *r,
     const char *t;
     char *w;
 
-    char filtbuf[FILTER_LENGTH];
-    const char *dn = NULL;
 
     if (!r->user) {
         return AUTHZ_DENIED_NO_USER;
@@ -649,18 +718,21 @@ static authz_status ldapuser_check_authorization(request_rec *r,
         return AUTHZ_DENIED;
     }
 
-    if (sec->host) {
-        ldc = get_connection_for_authz(r, LDAP_COMPARE);
-        apr_pool_cleanup_register(r->pool, ldc,
-                                  authnz_ldap_cleanup_connection_close,
-                                  apr_pool_cleanup_null);
-    }
-    else {
+    if (!sec->host) {
         ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01698)
                       "auth_ldap authorize: no sec->host - weird...?");
         return AUTHZ_DENIED;
     }
 
+    if (!req) {
+        authz_status rv = AUTHZ_DENIED;
+        req = build_request_config(r);
+        if (AUTHZ_GRANTED != (rv = get_dn_for_nonldap_authn(r, ldc))) { 
+            return rv;
+        }
+    }
+    ldc = get_connection_for_authz(r, LDAP_COMPARE);
+
     /*
      * If we have been authenticated by some other module than mod_authnz_ldap,
      * the req structure needed for authorization needs to be created
@@ -674,38 +746,6 @@ static authz_status ldapuser_check_authorization(request_rec *r,
             r->ap_auth_type);
     }
 
-    if(!req) {
-        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01700)
-            "ldap authorize: Creating LDAP req structure");
-
-        req = (authn_ldap_request_t *)apr_pcalloc(r->pool,
-            sizeof(authn_ldap_request_t));
-
-        /* Build the username filter */
-        if (APR_SUCCESS != authn_ldap_build_filter(filtbuf, r, r->user, NULL, sec)) {
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02623)
-                          "auth_ldap authorize: ldap filter too long (>%d): %s",
-                          FILTER_LENGTH, filtbuf);
-            return AUTHZ_DENIED;
-        }
-
-        /* Search for the user DN */
-        result = util_ldap_cache_getuserdn(r, ldc, sec->url, sec->basedn,
-             sec->scope, sec->attributes, filtbuf, &dn, &(req->vals));
-
-        /* Search failed, log error and return failure */
-        if(result != LDAP_SUCCESS) {
-            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01701)
-                "auth_ldap authorise: User DN not found, %s", ldc->reason);
-            return AUTHZ_DENIED;
-        }
-
-        ap_set_module_config(r->request_config, &authnz_ldap_module, req);
-        req->dn = apr_pstrdup(r->pool, dn);
-        req->user = r->user;
-
-    }
-
     if (req->dn == NULL || strlen(req->dn) == 0) {
         ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01702)
                       "auth_ldap authorize: require user: user's DN has not "
@@ -791,8 +831,6 @@ static authz_status ldapgroup_check_authorization(request_rec *r,
 
     const char *t;
 
-    char filtbuf[FILTER_LENGTH];
-    const char *dn = NULL;
     struct mod_auth_ldap_groupattr_entry_t *ent;
     int i;
 
@@ -804,18 +842,21 @@ static authz_status ldapgroup_check_authorization(request_rec *r,
         return AUTHZ_DENIED;
     }
 
-    if (sec->host) {
-        ldc = get_connection_for_authz(r, LDAP_COMPARE); /* for the top-level group only */
-        apr_pool_cleanup_register(r->pool, ldc,
-                                  authnz_ldap_cleanup_connection_close,
-                                  apr_pool_cleanup_null);
-    }
-    else {
+    if (!sec->host) {
         ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01708)
                       "auth_ldap authorize: no sec->host - weird...?");
         return AUTHZ_DENIED;
     }
 
+    if (!req) {
+        authz_status rv = AUTHZ_DENIED;
+        req = build_request_config(r);
+        if (AUTHZ_GRANTED != (rv = get_dn_for_nonldap_authn(r, ldc))) {
+            return rv;
+        }
+    }
+    ldc = get_connection_for_authz(r, LDAP_COMPARE);
+
     /*
      * If there are no elements in the group attribute array, the default should be
      * member and uniquemember; populate the array now.
@@ -864,36 +905,7 @@ static authz_status ldapgroup_check_authorization(request_rec *r,
             r->ap_auth_type);
     }
 
-    if(!req) {
-        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01710)
-            "ldap authorize: Creating LDAP req structure");
-
-        req = (authn_ldap_request_t *)apr_pcalloc(r->pool,
-            sizeof(authn_ldap_request_t));
-        /* Build the username filter */
-        if (APR_SUCCESS != authn_ldap_build_filter(filtbuf, r, r->user, NULL, sec)) {
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02624)
-                          "auth_ldap authorize: ldap filter too long (>%d): %s",
-                          FILTER_LENGTH, filtbuf);
-            return AUTHZ_DENIED;
-        }
-
-        /* Search for the user DN */
-        result = util_ldap_cache_getuserdn(r, ldc, sec->url, sec->basedn,
-             sec->scope, sec->attributes, filtbuf, &dn, &(req->vals));
-
-        /* Search failed, log error and return failure */
-        if(result != LDAP_SUCCESS) {
-            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01711)
-                "auth_ldap authorise: User DN not found, %s", ldc->reason);
-            return AUTHZ_DENIED;
-        }
-
-        ap_set_module_config(r->request_config, &authnz_ldap_module, req);
-        req->dn = apr_pstrdup(r->pool, dn);
-        req->user = r->user;
-    }
-
+   
     ent = (struct mod_auth_ldap_groupattr_entry_t *) sec->groupattr->elts;
 
     if (sec->group_attrib_is_dn) {
@@ -957,21 +969,18 @@ static authz_status ldapgroup_check_authorization(request_rec *r,
         }
     }
     
-    for (i = 0; i < sec->groupattr->nelts; i++) {
-        /* nested groups need searches and compares, so grab a new handle */
-        authnz_ldap_cleanup_connection_close(ldc);
-        apr_pool_cleanup_kill(r->pool, ldc,authnz_ldap_cleanup_connection_close);
-
+    /* nested groups need searches and compares, so grab a new handle */
+    if (sec->groupattr->nelts > 0) { 
+        release_ldc(r, ldc);
         ldc = get_connection_for_authz(r, LDAP_COMPARE_AND_SEARCH);
-        apr_pool_cleanup_register(r->pool, ldc,
-                                  authnz_ldap_cleanup_connection_close,
-                                  apr_pool_cleanup_null);
-
         ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01716)
-                       "auth_ldap authorise: require group \"%s\": "
-                       "failed [%s][%d - %s], checking sub-groups",
-                       t, ldc->reason, result, ldap_err2string(result));
+                "auth_ldap authorise: require group \"%s\": "
+                "failed [%s][%d - %s], checking sub-groups",
+                t, ldc->reason, result, ldap_err2string(result));
+    }
+
 
+    for (i = 0; i < sec->groupattr->nelts; i++) {
         result = util_ldap_cache_check_subgroups(r, ldc, sec->url, t, ent[i].name,
                                                  sec->group_attrib_is_dn ? req->dn : req->user,
                                                  sec->sgAttributes[0] ? sec->sgAttributes : default_attributes,
@@ -1023,9 +1032,6 @@ static authz_status ldapdn_check_authorization(request_rec *r,
 
     const char *t;
 
-    char filtbuf[FILTER_LENGTH];
-    const char *dn = NULL;
-
     if (!r->user) {
         return AUTHZ_DENIED_NO_USER;
     }
@@ -1034,13 +1040,7 @@ static authz_status ldapdn_check_authorization(request_rec *r,
         return AUTHZ_DENIED;
     }
 
-    if (sec->host) {
-        ldc = get_connection_for_authz(r, LDAP_SEARCH); /* _comparedn is a searche */
-        apr_pool_cleanup_register(r->pool, ldc,
-                                  authnz_ldap_cleanup_connection_close,
-                                  apr_pool_cleanup_null);
-    }
-    else {
+    if (!sec->host) {
         ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01721)
                       "auth_ldap authorize: no sec->host - weird...?");
         return AUTHZ_DENIED;
@@ -1058,35 +1058,14 @@ static authz_status ldapdn_check_authorization(request_rec *r,
             r->ap_auth_type);
     }
 
-    if(!req) {
-        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01723)
-            "ldap authorize: Creating LDAP req structure");
-
-        req = (authn_ldap_request_t *)apr_pcalloc(r->pool,
-            sizeof(authn_ldap_request_t));
-        /* Build the username filter */
-        if (APR_SUCCESS != authn_ldap_build_filter(filtbuf, r, r->user, NULL, sec)) {
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02625)
-                          "auth_ldap authorize: ldap filter too long (>%d): %s",
-                          FILTER_LENGTH, filtbuf);
-            return AUTHZ_DENIED;
-        }
-
-        /* Search for the user DN */
-        result = util_ldap_cache_getuserdn(r, ldc, sec->url, sec->basedn,
-             sec->scope, sec->attributes, filtbuf, &dn, &(req->vals));
-
-        /* Search failed, log error and return failure */
-        if(result != LDAP_SUCCESS) {
-            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01724)
-                "auth_ldap authorise: User DN not found with filter %s: %s", filtbuf, ldc->reason);
-            return AUTHZ_DENIED;
+    if (!req) {
+        authz_status rv = AUTHZ_DENIED;
+        req = build_request_config(r);
+        if (AUTHZ_GRANTED != (rv = get_dn_for_nonldap_authn(r, ldc))) {
+            return rv;
         }
-
-        ap_set_module_config(r->request_config, &authnz_ldap_module, req);
-        req->dn = apr_pstrdup(r->pool, dn);
-        req->user = r->user;
     }
+    ldc = get_connection_for_authz(r, LDAP_SEARCH); /* comparedn is a search */
 
     require = ap_expr_str_exec(r, expr, &err);
     if (err) {
@@ -1150,9 +1129,6 @@ static authz_status ldapattribute_check_authorization(request_rec *r,
     const char *t;
     char *w, *value;
 
-    char filtbuf[FILTER_LENGTH];
-    const char *dn = NULL;
-
     if (!r->user) {
         return AUTHZ_DENIED_NO_USER;
     }
@@ -1161,13 +1137,7 @@ static authz_status ldapattribute_check_authorization(request_rec *r,
         return AUTHZ_DENIED;
     }
 
-    if (sec->host) {
-        ldc = get_connection_for_authz(r, LDAP_COMPARE);
-        apr_pool_cleanup_register(r->pool, ldc,
-                                  authnz_ldap_cleanup_connection_close,
-                                  apr_pool_cleanup_null);
-    }
-    else {
+    if (!sec->host) {
         ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01729)
                       "auth_ldap authorize: no sec->host - weird...?");
         return AUTHZ_DENIED;
@@ -1185,35 +1155,14 @@ static authz_status ldapattribute_check_authorization(request_rec *r,
             r->ap_auth_type);
     }
 
-    if(!req) {
-        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01731)
-            "ldap authorize: Creating LDAP req structure");
-
-        req = (authn_ldap_request_t *)apr_pcalloc(r->pool,
-            sizeof(authn_ldap_request_t));
-        /* Build the username filter */
-        if (APR_SUCCESS != authn_ldap_build_filter(filtbuf, r, r->user, NULL, sec)) {
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02626)
-                          "auth_ldap authorize: ldap filter too long (>%d): %s",
-                          FILTER_LENGTH, filtbuf);
-            return AUTHZ_DENIED;
-        }
-
-        /* Search for the user DN */
-        result = util_ldap_cache_getuserdn(r, ldc, sec->url, sec->basedn,
-             sec->scope, sec->attributes, filtbuf, &dn, &(req->vals));
-
-        /* Search failed, log error and return failure */
-        if(result != LDAP_SUCCESS) {
-            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01732)
-                "auth_ldap authorise: User DN not found with filter %s: %s", filtbuf, ldc->reason);
-            return AUTHZ_DENIED;
+    if (!req) {
+        authz_status rv = AUTHZ_DENIED;
+        req = build_request_config(r);
+        if (AUTHZ_GRANTED != (rv = get_dn_for_nonldap_authn(r, ldc))) {
+            return rv;
         }
-
-        ap_set_module_config(r->request_config, &authnz_ldap_module, req);
-        req->dn = apr_pstrdup(r->pool, dn);
-        req->user = r->user;
     }
+    ldc = get_connection_for_authz(r, LDAP_COMPARE);
 
     if (req->dn == NULL || strlen(req->dn) == 0) {
         ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01733)
@@ -1294,13 +1243,7 @@ static authz_status ldapfilter_check_authorization(request_rec *r,
         return AUTHZ_DENIED;
     }
 
-    if (sec->host) {
-        ldc = get_connection_for_authz(r, LDAP_SEARCH);
-        apr_pool_cleanup_register(r->pool, ldc,
-                                  authnz_ldap_cleanup_connection_close,
-                                  apr_pool_cleanup_null);
-    }
-    else {
+    if (!sec->host) {
         ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01738)
                       "auth_ldap authorize: no sec->host - weird...?");
         return AUTHZ_DENIED;
@@ -1318,35 +1261,14 @@ static authz_status ldapfilter_check_authorization(request_rec *r,
             r->ap_auth_type);
     }
 
-    if(!req) {
-        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01740)
-            "ldap authorize: Creating LDAP req structure");
-
-        req = (authn_ldap_request_t *)apr_pcalloc(r->pool,
-            sizeof(authn_ldap_request_t));
-        /* Build the username filter */
-        if (APR_SUCCESS != authn_ldap_build_filter(filtbuf, r, r->user, NULL, sec)) {
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02627)
-                          "auth_ldap authorize: ldap filter too long (>%d): %s",
-                          FILTER_LENGTH, filtbuf);
-            return AUTHZ_DENIED;
-        }
-
-        /* Search for the user DN */
-        result = util_ldap_cache_getuserdn(r, ldc, sec->url, sec->basedn,
-             sec->scope, sec->attributes, filtbuf, &dn, &(req->vals));
-
-        /* Search failed, log error and return failure */
-        if(result != LDAP_SUCCESS) {
-            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01741)
-                "auth_ldap authorise: User DN not found with filter %s: %s", filtbuf, ldc->reason);
-            return AUTHZ_DENIED;
+    if (!req) {
+        authz_status rv = AUTHZ_DENIED;
+        req = build_request_config(r);
+        if (AUTHZ_GRANTED != (rv = get_dn_for_nonldap_authn(r, ldc))) {
+            return rv;
         }
-
-        ap_set_module_config(r->request_config, &authnz_ldap_module, req);
-        req->dn = apr_pstrdup(r->pool, dn);
-        req->user = r->user;
     }
+    ldc = get_connection_for_authz(r, LDAP_SEARCH);
 
     if (req->dn == NULL || strlen(req->dn) == 0) {
         ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01742)
@@ -1387,8 +1309,7 @@ static authz_status ldapfilter_check_authorization(request_rec *r,
                           "auth_ldap authorize: checking dn match %s", dn);
             if (sec->compare_as_user) {
                 /* ldap-filter is the only authz that requires a search and a compare */
-                apr_pool_cleanup_kill(r->pool, ldc, authnz_ldap_cleanup_connection_close);
-                authnz_ldap_cleanup_connection_close(ldc);
+                release_ldc(r, ldc);
                 ldc = get_connection_for_authz(r, LDAP_COMPARE);
             }
             result = util_ldap_cache_comparedn(r, ldc, sec->url, req->dn, dn,
@@ -1449,9 +1370,6 @@ static authz_status ldapsearch_check_authorization(request_rec *r,
 
     if (sec->host) {
         ldc = get_connection_for_authz(r, LDAP_SEARCH);
-        apr_pool_cleanup_register(r->pool, ldc,
-                                  authnz_ldap_cleanup_connection_close,
-                                  apr_pool_cleanup_null);
     }
     else {
         ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(02636)
@@ -2022,6 +1940,18 @@ static void ImportULDAPOptFn(void)
     util_ldap_cache_check_subgroups = APR_RETRIEVE_OPTIONAL_FN(uldap_cache_check_subgroups);
 }
 
+
+/** Cleanup LDAP connections before EOR. Note, if the authorization is unsuccessful,
+ *  this will not run, but EOR is unlikely to be delayed as in a successful request.
+ */
+static apr_status_t authnz_ldap_fixups(request_rec *r) { 
+    authn_ldap_request_t *req =
+        (authn_ldap_request_t *)ap_get_module_config(r->request_config, &authnz_ldap_module);
+    if (req && req->ldc_pool) { 
+        apr_pool_destroy(req->ldc_pool);
+    }
+    return OK;
+}
 static void register_hooks(apr_pool_t *p)
 {
     /* Register authn provider */
@@ -2056,6 +1986,7 @@ static void register_hooks(apr_pool_t *p)
                               AP_AUTH_INTERNAL_PER_CONF);
 
     ap_hook_post_config(authnz_ldap_post_config,NULL,NULL,APR_HOOK_MIDDLE);
+    ap_hook_fixups(authnz_ldap_fixups,NULL,NULL,APR_HOOK_MIDDLE);
 
     ap_hook_optional_fn_retrieve(ImportULDAPOptFn,NULL,NULL,APR_HOOK_MIDDLE);
 }
-- 
2.40.0