Introduce new config option LDAPTimeout to set the timeout for operations

like bind and search.

For operations other than search, the support of the timeout depends on the
LDAP SDK supporting LDAP_OPT_TIMEOUT (e.g. OpenLDAP >= 2.4.4). An exception is
the ldap_simple_bind() in uldap_connection_open(): Here we put some extra
effort into detecting a timeout in a portable way, to catch an idle connection
having been dropped by a firewall.

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

diff --git a/CHANGES b/CHANGES
index 5a71ddcac88e778bca1c7b533d1d79585f7ce001..0ef44b57e4134a7807bc2ff6d143aec5e7ed97a7 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,9 @@
 
 Changes with Apache 2.3.5
 
+  *) mod_ldap: Introduce new config option LDAPTimeout to set the timeout
+     for LDAP operations like bind and search. [Stefan Fritsch]
+
   *) mod_proxy, mod_proxy_ftp: Move ProxyFtpDirCharset from mod_proxy to
      mod_proxy_ftp. [Takashi Sato]
 

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index b6813f0ee47dbe9a1e1115ea5e00779c12b1a427..ceb8f0b98c551b4085ae6216c8e589bae2fad75c 100644 (file)
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -210,6 +210,7 @@
  *                         to mod_proxy_connect
  * 20091230.0 (2.3.5-dev)  Move ftp_directory_charset from proxy_dir_conf
  *                         to proxy_ftp_dir_conf(mod_proxy_ftp)
+ * 20091230.1 (2.3.5-dev)  add util_ldap_state_t.opTimeout
  *
  */
 
@@ -218,7 +219,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20091230
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 0                     /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 1                     /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

diff --git a/include/util_ldap.h b/include/util_ldap.h
index a1ebb33a9dfaf471d8de502dee4e307e7e8d5750..b5c54c91da62b8219fd272953ab443f2b0e60082 100644 (file)
--- a/include/util_ldap.h
+++ b/include/util_ldap.h
@@ -159,6 +159,7 @@ typedef struct util_ldap_state_t {
     long  connectionTimeout;
     int   verify_svr_cert;
     int debug_level;                    /* SDK debug level */
+    struct timeval *opTimeout;
 
 } util_ldap_state_t;
 

diff --git a/modules/ldap/util_ldap.c b/modules/ldap/util_ldap.c
index ad16ed46dbd86848d0424fecdf4b156aedc86b89..9a137a326331b292ad2727b9f60250f209811ee7 100644 (file)
--- a/modules/ldap/util_ldap.c
+++ b/modules/ldap/util_ldap.c
@@ -284,7 +284,7 @@ static int uldap_connection_init(request_rec *r,
     int version  = LDAP_VERSION3;
     apr_ldap_err_t *result = NULL;
 #ifdef LDAP_OPT_NETWORK_TIMEOUT
-    struct timeval timeOut = {10,0};    /* 10 second connection timeout */
+    struct timeval connectionTimeout = {10,0};    /* 10 second connection timeout */
 #endif
     util_ldap_state_t *st =
         (util_ldap_state_t *)ap_get_module_config(r->server->module_config,
@@ -435,12 +435,12 @@ static int uldap_connection_init(request_rec *r,
 
 #ifdef LDAP_OPT_NETWORK_TIMEOUT
     if (st->connectionTimeout > 0) {
-        timeOut.tv_sec = st->connectionTimeout;
+        connectionTimeout.tv_sec = st->connectionTimeout;
     }
 
     if (st->connectionTimeout >= 0) {
         rc = apr_ldap_set_option(r->pool, ldc->ldap, LDAP_OPT_NETWORK_TIMEOUT,
-                                 (void *)&timeOut, &(result));
+                                 (void *)&connectionTimeout, &(result));
         if (APR_SUCCESS != rc) {
             ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
                              "LDAP: Could not set the connection timeout");
@@ -448,6 +448,25 @@ static int uldap_connection_init(request_rec *r,
     }
 #endif
 
+#ifdef LDAP_OPT_TIMEOUT
+    /*
+     * LDAP_OPT_TIMEOUT is not portable, but it influences all synchronous ldap
+     * function calls and not just ldap_search_ext_s(), which accepts a timeout
+     * parameter.
+     * XXX: It would be possible to simulate LDAP_OPT_TIMEOUT by replacing all
+     * XXX: synchronous ldap function calls with asynchronous calls and using
+     * XXX: ldap_result() with a timeout.
+     */
+    if (st->opTimeout) {
+        rc = apr_ldap_set_option(r->pool, ldc->ldap, LDAP_OPT_TIMEOUT,
+                                 st->opTimeout, &(result));
+        if (APR_SUCCESS != rc) {
+            ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
+                             "LDAP: Could not set LDAP_OPT_TIMEOUT");
+        }
+    }
+#endif
+
     return(rc);
 }
 
@@ -462,6 +481,8 @@ static int uldap_connection_open(request_rec *r,
 {
     int rc = 0;
     int failures = 0;
+    int new_connection = 0;
+    util_ldap_state_t *st;
 
     /* sanity check for NULL */
     if (!ldc) {
@@ -480,6 +501,7 @@ static int uldap_connection_open(request_rec *r,
     */
     if (NULL == ldc->ldap)
     {
+       new_connection = 1;
        rc = uldap_connection_init( r, ldc );
        if (LDAP_SUCCESS != rc)
        {
@@ -488,22 +510,54 @@ static int uldap_connection_open(request_rec *r,
     }
 
 
+    st = (util_ldap_state_t *)ap_get_module_config(r->server->module_config,
+                                                   &ldap_module);
+
     /* loop trying to bind up to 10 times if LDAP_SERVER_DOWN error is
-     * returned.  Break out of the loop on Success or any other error.
+     * returned. If LDAP_TIMEOUT is returned on the first try, maybe the
+     * connection was idle for a long time and has been dropped by a firewall.
+     * In this case close the connection immediately and try again. To set
+     * a timeout in a portable way, we have to use ldap_simple_bind() and 
+     * ldap_result() instead of ldap_simple_bind_s().
+     *
+     * On Success or any other error, break out of the loop.
      *
      * NOTE: Looping is probably not a great idea. If the server isn't
      * responding the chances it will respond after a few tries are poor.
      * However, the original code looped and it only happens on
      * the error condition.
-      */
+     */
     for (failures=0; failures<10; failures++)
     {
-        rc = ldap_simple_bind_s(ldc->ldap,
-                                (char *)ldc->binddn,
-                                (char *)ldc->bindpw);
-        if (!AP_LDAP_IS_SERVER_DOWN(rc)) {
+        LDAPMessage *result;
+        int msgid = ldap_simple_bind(ldc->ldap,
+                                     (char *)ldc->binddn,
+                                     (char *)ldc->bindpw);
+        if (msgid == -1) {
+            ldc->reason = "LDAP: ldap_simple_bind() failed";
+            break;
+        }
+        rc = ldap_result(ldc->ldap, msgid, 0, st->opTimeout, &result);
+        if (rc == -1) {
+            ldc->reason = "LDAP: ldap_simple_bind() result retrieval failed";
             break;
-        } else if (failures == 5) {
+        }
+        else if (rc == 0) {
+            ldc->reason = "LDAP: ldap_simple_bind() timed out";
+            rc = LDAP_TIMEOUT;
+        } else if (ldap_parse_result(ldc->ldap, result, &rc, NULL,
+                                     NULL, NULL, NULL, 1 ) == -1) {
+            ldc->reason = "LDAP: ldap_simple_bind() parse result failed";
+            break;
+        }
+        if ((AP_LDAP_IS_SERVER_DOWN(rc) && failures == 5) ||
+            (rc == LDAP_TIMEOUT && failures == 0))
+        {
+           if (rc == LDAP_TIMEOUT && !new_connection) {
+               ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+                             "ldap_simple_bind() timed out on reused "
+                             "connection, dropped by firewall?");
+           }
            /* attempt to init the connection once again */
            uldap_connection_unbind( ldc );
            rc = uldap_connection_init( r, ldc );
@@ -511,7 +565,10 @@ static int uldap_connection_open(request_rec *r,
            {
                break;
            }
-       }
+        }
+        else if (!AP_LDAP_IS_SERVER_DOWN(rc)) {
+            break;
+        }
     }
 
     /* free the handle if there was an error
@@ -519,7 +576,7 @@ static int uldap_connection_open(request_rec *r,
     if (LDAP_SUCCESS != rc)
     {
        uldap_connection_unbind(ldc);
-        ldc->reason = "LDAP: ldap_simple_bind_s() failed";
+        ldc->reason = "LDAP: ldap_simple_bind() failed";
     }
     else {
         ldc->bound = 1;
@@ -809,7 +866,7 @@ start_over:
     /* search for reqdn */
     result = ldap_search_ext_s(ldc->ldap, (char *)reqdn, LDAP_SCOPE_BASE,
                                "(objectclass=*)", NULL, 1,
-                               NULL, NULL, NULL, APR_LDAP_SIZELIMIT, &res);
+                               NULL, NULL, st->opTimeout, APR_LDAP_SIZELIMIT, &res);
     if (AP_LDAP_IS_SERVER_DOWN(result))
     {
         ldc->reason = "DN Comparison ldap_search_ext_s() "
@@ -817,6 +874,16 @@ start_over:
         uldap_connection_unbind(ldc);
         goto start_over;
     }
+    if (result == LDAP_TIMEOUT && failures == 0) {
+        /*
+         * we are reusing a connection that doesn't seem to be active anymore
+         * (firewall state drop?), let's try a new connection.
+         */
+        ldc->reason = "DN Comparison ldap_search_ext_s() "
+                      "failed with timeout";
+        uldap_connection_unbind(ldc);
+        goto start_over;
+    }
     if (result != LDAP_SUCCESS) {
         /* search for reqdn failed - no match */
         ldc->reason = "DN Comparison ldap_search_ext_s() failed";
@@ -957,6 +1024,15 @@ start_over:
         uldap_connection_unbind(ldc);
         goto start_over;
     }
+    if (result == LDAP_TIMEOUT && failures == 0) {
+        /*
+         * we are reusing a connection that doesn't seem to be active anymore
+         * (firewall state drop?), let's try a new connection.
+         */
+        ldc->reason = "ldap_compare_s() failed with timeout";
+        uldap_connection_unbind(ldc);
+        goto start_over;
+    }
 
     ldc->reason = "Comparison complete";
     if ((LDAP_COMPARE_TRUE == result) ||
@@ -1057,6 +1133,15 @@ start_over:
         uldap_connection_unbind(ldc);
         goto start_over;
     }
+    if (result == LDAP_TIMEOUT && failures == 0) {
+        /*
+         * we are reusing a connection that doesn't seem to be active anymore
+         * (firewall state drop?), let's try a new connection.
+         */
+        ldc->reason = "ldap_search_ext_s() for subgroups failed with timeout";
+        uldap_connection_unbind(ldc);
+        goto start_over;
+    }
 
     /* if there is an error (including LDAP_NO_SUCH_OBJECT) return now */
     if (result != LDAP_SUCCESS) {
@@ -1517,7 +1602,7 @@ start_over:
     result = ldap_search_ext_s(ldc->ldap,
                                (char *)basedn, scope,
                                (char *)filter, attrs, 0,
-                               NULL, NULL, NULL, APR_LDAP_SIZELIMIT, &res);
+                               NULL, NULL, st->opTimeout, APR_LDAP_SIZELIMIT, &res);
     if (AP_LDAP_IS_SERVER_DOWN(result))
     {
         ldc->reason = "ldap_search_ext_s() for user failed with server down";
@@ -1764,7 +1849,7 @@ start_over:
     result = ldap_search_ext_s(ldc->ldap,
                                (char *)basedn, scope,
                                (char *)filter, attrs, 0,
-                               NULL, NULL, NULL, APR_LDAP_SIZELIMIT, &res);
+                               NULL, NULL, st->opTimeout, APR_LDAP_SIZELIMIT, &res);
     if (AP_LDAP_IS_SERVER_DOWN(result))
     {
         ldc->reason = "ldap_search_ext_s() for user failed with server down";
@@ -2421,6 +2506,55 @@ static void *util_ldap_create_dir_config(apr_pool_t *p, char *d) {
    return dc;
 }
 
+static const char *util_ldap_set_op_timeout(cmd_parms *cmd,
+                                            void *dummy,
+                                            const char *val)
+{
+    long timeout;
+    char *endptr;
+    util_ldap_state_t *st =
+        (util_ldap_state_t *)ap_get_module_config(cmd->server->module_config,
+                                                  &ldap_module);
+    const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+
+    if (err != NULL) {
+        return err;
+    }
+
+    timeout = strtol(val, &endptr, 10);
+    if ((val == endptr) || (*endptr != '\0')) {
+        return "Timeout not numerical";
+    }
+    if (timeout < 0) {
+        return "Timeout must be non-negative";
+    }
+
+    if (timeout) {
+        if (!st->opTimeout) {
+            st->opTimeout = apr_pcalloc(cmd->pool, sizeof(struct timeval));
+        }
+        st->opTimeout->tv_sec = timeout;
+    }
+    else {
+        st->opTimeout = NULL;
+    }
+
+    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, cmd->server,
+                 "[%" APR_PID_T_FMT "] ldap connection: Setting op timeout "
+                 "to %d seconds.", getpid(), timeout);
+
+#ifndef LDAP_OPT_TIMEOUT
+
+    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, cmd->server,
+                 "LDAP: LDAP_OPT_TIMEOUT option not supported by the "
+                 "LDAP library in use. Using LDAPTimeout value as search "
+                 "timeout only." );
+#endif
+
+    return NULL;
+}
+
+
 
 static void *util_ldap_create_config(apr_pool_t *p, server_rec *s)
 {
@@ -2448,6 +2582,8 @@ static void *util_ldap_create_config(apr_pool_t *p, server_rec *s)
     st->secure = APR_LDAP_NONE;
     st->secure_set = 0;
     st->connectionTimeout = 10;
+    st->opTimeout = apr_pcalloc(p, sizeof(struct timeval));
+    st->opTimeout->tv_sec = 60;
     st->verify_svr_cert = 1;
 
     return st;
@@ -2495,6 +2631,7 @@ static void *util_ldap_merge_config(apr_pool_t *p, void *basev,
         trying to make special expections for one LDAP SDK, GLOBAL_ONLY 
         is being enforced on this setting as well. */
     st->connectionTimeout = base->connectionTimeout;
+    st->opTimeout = base->opTimeout;
     st->verify_svr_cert = base->verify_svr_cert;
     st->debug_level = base->debug_level;
 
@@ -2773,6 +2910,11 @@ static const command_rec util_ldap_cmds[] = {
                   NULL, RSRC_CONF,
                   "Enable debugging in LDAP SDK (Default: off, values: SDK specific"),
 
+    AP_INIT_TAKE1("LDAPTimeout", util_ldap_set_op_timeout,
+                  NULL, RSRC_CONF,
+                  "Specify the LDAP bind/search timeout in seconds "
+                  "(0 = no limit). Default: 60"),
+
     {NULL}
 };