Commit 5c84492b authored by Eric Covener's avatar Eric Covener
Browse files

Lay some groundwork for improvements to the connection pool. 

  remove unnecessary uldap_connection_cleanup (nothing needed between unbind 
  and remove)

  properly remove rebind callback info when credentials change

  maintain a separate pool for the rebind callback storage so it can be cleared
  when the connection is unbound.

(major bump for util_ldap function removal)



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1081005 13f79535-47bb-0310-9956-ffa450edef68
parent c3b7e47a

include/ap_mmn.h

+6 −2
Changes for include/ap_mmn.h: 6 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -302,14 +302,18 @@ 
 * 20110203.1 (2.3.11-dev) Add ap_state_query()

 * 20110203.2 (2.3.11-dev) Add ap_run_pre_read_request() hook and

 *                         ap_parse_form_data() util

 * 20110312.0 (2.3.12-dev) remove uldap_connection_cleanup and add 

                           util_ldap_state_t.connectionPoolTTL,

                           util_ldap_connection_t.freed, and

                           util_ldap_connection_t.rebind_pool. 

 */



#define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */



#ifndef MODULE_MAGIC_NUMBER_MAJOR

#define MODULE_MAGIC_NUMBER_MAJOR 20110203

#define MODULE_MAGIC_NUMBER_MAJOR 20110312

#endif

#define MODULE_MAGIC_NUMBER_MINOR 2                     /* 0...n */

#define MODULE_MAGIC_NUMBER_MINOR 0                     /* 0...n */



/**

 * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

include/util_ldap.h

+3 −10
Changes for include/util_ldap.h: 3 added lines, 10 removed lines.
Original line number Diff line number Diff line
@@ -121,6 +121,8 @@ typedef struct util_ldap_connection_t { 


    int ChaseReferrals;                 /* [on|off] (default = AP_LDAP_CHASEREFERRALS_ON)*/

    int ReferralHopLimit;               /* # of referral hops to follow (default = AP_LDAP_DEFAULT_HOPLIMIT) */

    apr_time_t freed;                   /* the time this conn was placed back in the pool */

    apr_pool_t *rebind_pool;            /* frequently cleared pool for rebind data */

} util_ldap_connection_t;



typedef struct util_ldap_config_t {
@@ -163,7 +165,7 @@ typedef struct util_ldap_state_t { 
    struct timeval *opTimeout;



    int debug_level;                    /* SDK debug level */



    int connectionPoolTTL;

} util_ldap_state_t;



/* Used to store arrays of attribute labels/values. */
@@ -208,15 +210,6 @@ APR_DECLARE_OPTIONAL_FN(void,uldap_connection_close,(util_ldap_connection_t *ldc 
 */

APR_DECLARE_OPTIONAL_FN(apr_status_t,uldap_connection_unbind,(void *param));



/**

 * Cleanup a connection to an LDAP server

 * @param ldc A structure containing the expanded details of the server

 *            that was connected.

 * @tip This functions unbinds and closes the connection to the LDAP server

 * @fn apr_status_t util_ldap_connection_cleanup(util_ldap_connection_t *ldc)

 */

APR_DECLARE_OPTIONAL_FN(apr_status_t,uldap_connection_cleanup,(void *param));



/**

 * Find a connection in a list of connections

 * @param r The request record

modules/ldap/util_ldap.c

+27 −56
Changes for modules/ldap/util_ldap.c: 27 added lines, 56 removed lines.
Original line number Diff line number Diff line
@@ -65,6 +65,7 @@ 


module AP_MODULE_DECLARE_DATA ldap_module;

static const char *ldap_cache_mutex_type = "ldap-cache";

static apr_status_t uldap_connection_unbind(void *param);



#define LDAP_CACHE_LOCK() do {                                  \

    if (st->util_ldap_cache_lock)                               \
@@ -76,8 +77,6 @@ static const char *ldap_cache_mutex_type = "ldap-cache"; 
        apr_global_mutex_unlock(st->util_ldap_cache_lock);      \

} while (0)



static apr_status_t util_ldap_connection_remove (void *param);



static void util_ldap_strdup (char **str, const char *newstr)

{

    if (*str) {
@@ -144,19 +143,12 @@ static int util_ldap_handler(request_rec *r) 
static void uldap_connection_close(util_ldap_connection_t *ldc)

{



    /*

     * QUESTION:

     *

     * Is it safe leaving bound connections floating around between the

     * different modules? Keeping the user bound is a performance boost,

     * but it is also a potential security problem - maybe.

     *

     * For now we unbind the user when we finish with a connection, but

     * we don't have to...

     /* We leave bound LDAP connections floating around in our pool,

      * but always check/fix the binddn/bindpw when we take them out

      * of the pool

      */



     if (!ldc->keep) { 

         util_ldap_connection_remove(ldc);

         uldap_connection_unbind(ldc);

     }

     else { 

         /* mark our connection as available for reuse */
@@ -182,52 +174,18 @@ static apr_status_t uldap_connection_unbind(void *param) 
            ldc->ldap = NULL;

        }

        ldc->bound = 0;

    }



    return APR_SUCCESS;

}





/*

 * Clean up an LDAP connection by unbinding and unlocking the connection.

 * This cleanup does not remove the util_ldap_connection_t from the 

 * per-virtualhost list of connections, does not remove the storage

 * for the util_ldap_connection_t or its data, and is NOT run automatically.

 */

static apr_status_t uldap_connection_cleanup(void *param)

{

    util_ldap_connection_t *ldc = param;



    if (ldc) {

        /* Release the rebind info for this connection. No more referral rebinds required. */

        /* forget the rebind info for this conn */

        if (ldc->ChaseReferrals == AP_LDAP_CHASEREFERRALS_ON) {

            apr_ldap_rebind_remove(ldc->ldap);

            apr_pool_clear(ldc->rebind_pool);

        }



        /* unbind and disconnect from the LDAP server */

        uldap_connection_unbind(ldc);



        /* free the username and password */

        if (ldc->bindpw) {

            free((void*)ldc->bindpw);

            ldc->bindpw = NULL;

        }

        if (ldc->binddn) {

            free((void*)ldc->binddn);

            ldc->binddn = NULL;

        }

        /* ldc->reason is allocated from r->pool */

        if (ldc->reason) {

            ldc->reason = NULL;

        }

        /* unlock this entry */

        uldap_connection_close(ldc);



    }



    return APR_SUCCESS;

}





/*

 * util_ldap_connection_remove frees all storage associated with the LDAP

 * connection and removes it completely from the per-virtualhost list of
@@ -263,9 +221,6 @@ static apr_status_t util_ldap_connection_remove (void *param) { 
        prev = l;

    }



    /* Some unfortunate duplication between this method

     * and uldap_connection_cleanup()

    */

    if (ldc->bindpw) {

        free((void*)ldc->bindpw);

    }
@@ -339,7 +294,7 @@ static int uldap_connection_init(request_rec *r, 


    if (ldc->ChaseReferrals == AP_LDAP_CHASEREFERRALS_ON) {

        /* Now that we have an ldap struct, add it to the referral list for rebinds. */

        rc = apr_ldap_rebind_add(ldc->pool, ldc->ldap, ldc->binddn, ldc->bindpw);

        rc = apr_ldap_rebind_add(ldc->rebind_pool, ldc->ldap, ldc->binddn, ldc->bindpw);

        if (rc != APR_SUCCESS) {

            ap_log_error(APLOG_MARK, APLOG_ERR, rc, r->server,

                    "LDAP: Unable to add rebind cross reference entry. Out of memory?");
@@ -732,7 +687,13 @@ static util_ldap_connection_t * 
                !compare_client_certs(dc->client_certs, l->client_certs))

            {

                /* the bind credentials have changed */

                l->bound = 0;

                uldap_connection_unbind(l);

                        

                /* forget the rebind info for this conn */

                if (l->ChaseReferrals == AP_LDAP_CHASEREFERRALS_ON) {

                    apr_ldap_rebind_remove(l->ldap);

                }



                util_ldap_strdup((char**)&(l->binddn), binddn);

                util_ldap_strdup((char**)&(l->bindpw), bindpw);

                break;
@@ -801,6 +762,17 @@ static util_ldap_connection_t * 


        l->keep = 1;



        if (l->ChaseReferrals == AP_LDAP_CHASEREFERRALS_ON) { 

            if (apr_pool_create(&(l->rebind_pool), l->pool) != APR_SUCCESS) {

                ap_log_rerror(APLOG_MARK, APLOG_CRIT, 0, r,

                              "util_ldap: Failed to create memory pool");

#if APR_HAS_THREADS

                apr_thread_mutex_unlock(st->mutex);

#endif

                return NULL;

            }

        }



        if (p) {

            p->next = l;

        }
@@ -2949,7 +2921,6 @@ static void util_ldap_register_hooks(apr_pool_t *p) 
    APR_REGISTER_OPTIONAL_FN(uldap_connection_open);

    APR_REGISTER_OPTIONAL_FN(uldap_connection_close);

    APR_REGISTER_OPTIONAL_FN(uldap_connection_unbind);

    APR_REGISTER_OPTIONAL_FN(uldap_connection_cleanup);

    APR_REGISTER_OPTIONAL_FN(uldap_connection_find);

    APR_REGISTER_OPTIONAL_FN(uldap_cache_comparedn);

    APR_REGISTER_OPTIONAL_FN(uldap_cache_compare);