You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by co...@apache.org on 2013/03/04 22:54:25 UTC
svn commit: r1452551 - in /httpd/httpd/trunk: include/ap_mmn.h
include/util_ldap.h modules/ldap/util_ldap.c
Author: covener
Date: Mon Mar 4 21:54:24 2013
New Revision: 1452551
URL: http://svn.apache.org/r1452551
Log:
PR54587: LDAP connections used for authn were not respecting
LDAPConnectionPoolTimeout due to confusion over what "bound" means.
Added some LDAP trace at TRACE5 to track how LDAP connections are
reused and rebound.
Modified:
httpd/httpd/trunk/include/ap_mmn.h
httpd/httpd/trunk/include/util_ldap.h
httpd/httpd/trunk/modules/ldap/util_ldap.c
Modified: httpd/httpd/trunk/include/ap_mmn.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?rev=1452551&r1=1452550&r2=1452551&view=diff
==============================================================================
--- httpd/httpd/trunk/include/ap_mmn.h (original)
+++ httpd/httpd/trunk/include/ap_mmn.h Mon Mar 4 21:54:24 2013
@@ -415,6 +415,7 @@
* 20121222.2 (2.5.0-dev) Add ap_password_validate()
* 20121222.3 (2.5.0-dev) Add ppinherit to proxy_server_conf
* 20121222.4 (2.5.0-dev) Add uds_path to proxy_conn_rec
+ * 20121222.5 (2.5.0-dev) Add "r" and "must_rebind" to util_ldap_connection_t
*/
#define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
@@ -422,7 +423,7 @@
#ifndef MODULE_MAGIC_NUMBER_MAJOR
#define MODULE_MAGIC_NUMBER_MAJOR 20121222
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 3 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 5 /* 0...n */
/**
* Determine if the server's current MODULE_MAGIC_NUMBER is at least a
Modified: httpd/httpd/trunk/include/util_ldap.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/util_ldap.h?rev=1452551&r1=1452550&r2=1452551&view=diff
==============================================================================
--- httpd/httpd/trunk/include/util_ldap.h (original)
+++ httpd/httpd/trunk/include/util_ldap.h Mon Mar 4 21:54:24 2013
@@ -129,6 +129,8 @@ typedef struct util_ldap_connection_t {
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 */
+ int must_rebind; /* The connection was last bound with other then binddn/bindpw */
+ request_rec *r; /* request_rec used to find this util_ldap_connection_t */
} util_ldap_connection_t;
typedef struct util_ldap_config_t {
Modified: httpd/httpd/trunk/modules/ldap/util_ldap.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap.c?rev=1452551&r1=1452550&r2=1452551&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ldap/util_ldap.c (original)
+++ httpd/httpd/trunk/modules/ldap/util_ldap.c Mon Mar 4 21:54:24 2013
@@ -156,10 +156,12 @@ static void uldap_connection_close(util_
*/
if (!ldc->keep) {
uldap_connection_unbind(ldc);
+ ldc->r = NULL;
}
else {
/* mark our connection as available for reuse */
ldc->freed = apr_time_now();
+ ldc->r = NULL;
#if APR_HAS_THREADS
apr_thread_mutex_unlock(ldc->lock);
#endif
@@ -178,6 +180,9 @@ static apr_status_t uldap_connection_unb
if (ldc) {
if (ldc->ldap) {
+ if (ldc->r) {
+ ap_log_rerror(APLOG_MARK, APLOG_TRACE5, 0, ldc->r, "LDC %pp unbind", ldc);
+ }
ldap_unbind_s(ldc->ldap);
ldc->ldap = NULL;
}
@@ -318,6 +323,8 @@ static int uldap_connection_init(request
return(result->rc);
}
+ ap_log_rerror(APLOG_MARK, APLOG_TRACE5, 0, r, "LDC %pp init", ldc);
+
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->rebind_pool, ldc->ldap, ldc->binddn, ldc->bindpw);
@@ -513,6 +520,9 @@ static int uldap_simple_bind(util_ldap_c
ldc->reason = "LDAP: ldap_simple_bind() parse result failed";
return uldap_ld_errno(ldc);
}
+ else {
+ ap_log_rerror(APLOG_MARK, APLOG_TRACE5, 0, ldc->r, "LDC %pp bind", ldc);
+ }
return rc;
}
@@ -537,7 +547,7 @@ static int uldap_connection_open(request
/* If the connection is already bound, return
*/
- if (ldc->bound)
+ if (ldc->bound && !ldc->must_rebind)
{
ldc->reason = "LDAP: connection open successful (already bound)";
return LDAP_SUCCESS;
@@ -618,6 +628,7 @@ static int uldap_connection_open(request
}
else {
ldc->bound = 1;
+ ldc->must_rebind = 0;
ldc->reason = "LDAP: connection open successful";
}
@@ -719,9 +730,13 @@ static util_ldap_connection_t *
ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r,
"Removing LDAP connection last used %" APR_TIME_T_FMT " seconds ago",
(now - l->freed) / APR_USEC_PER_SEC);
+ l->r = r;
uldap_connection_unbind(l);
/* Go ahead (by falling through) and use it, so we don't create more just to unbind some other old ones */
}
+ ap_log_rerror(APLOG_MARK, APLOG_TRACE5, 0, r,
+ "Reuse %s LDC %pp",
+ l->bound ? "bound" : "unbound", l);
}
break;
}
@@ -748,12 +763,25 @@ static util_ldap_connection_t *
(l->deref == deref) && (l->secure == secureflag) &&
!compare_client_certs(dc->client_certs, l->client_certs))
{
- /* the bind credentials have changed */
- /* no check for connection_pool_ttl, since we are unbinding any way */
- uldap_connection_unbind(l);
+ if (st->connection_pool_ttl > 0) {
+ if (l->bound && (now - l->freed) > st->connection_pool_ttl) {
+ ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r,
+ "Removing LDAP connection last used %" APR_TIME_T_FMT " seconds ago",
+ (now - l->freed) / APR_USEC_PER_SEC);
+ l->r = r;
+ uldap_connection_unbind(l);
+ /* Go ahead (by falling through) and use it, so we don't create more just to unbind some other old ones */
+ }
+ ap_log_rerror(APLOG_MARK, APLOG_TRACE5, 0, r,
+ "Reuse %s LDC %pp (will rebind)",
+ l->bound ? "bound" : "unbound", l);
+ }
+ /* the bind credentials have changed */
+ l->must_rebind = 1;
util_ldap_strdup((char**)&(l->binddn), binddn);
util_ldap_strdup((char**)&(l->bindpw), bindpw);
+
break;
}
#if APR_HAS_THREADS
@@ -843,6 +871,7 @@ static util_ldap_connection_t *
#if APR_HAS_THREADS
apr_thread_mutex_unlock(st->mutex);
#endif
+ l->r = r;
return l;
}
@@ -1768,10 +1797,10 @@ start_over:
/*
* We have just bound the connection to a different user and password
* combination, which might be reused unintentionally next time this
- * connection is used from the connection pool. To ensure no confusion,
- * we mark the connection as unbound.
+ * connection is used from the connection pool.
*/
- ldc->bound = 0;
+ ldc->must_rebind = 0;
+ ap_log_rerror(APLOG_MARK, APLOG_TRACE5, 0, r, "LDC %pp used for authn, must be rebound", ldc);
}
/*