Michael Haardt's patch to do LDAP network timeouts better for OpenLDAP.

3 files changed, 67 insertions, 34 deletions

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 59f1019c6..fc77a704a 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -1,4 +1,4 @@
-$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.19 2004/11/10 10:29:56 ph10 Exp $
+$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.20 2004/11/10 14:15:20 ph10 Exp $
 
 Change log file for Exim from version 4.21
 -------------------------------------------
@@ -70,6 +70,10 @@ Exim version 4.44
 
 19. Added -dd to debug only the daemon process.
 
+20. Incorporated Michael Haardt's patch to ldap.c for improving the way it
+    handles timeouts, both on the server side and network timeouts. Renamed the
+    CONNECT parameter as NETTIMEOUT (but kept the old name for compatibility).
+
 
 Exim version 4.43
 -----------------
diff --git a/src/ACKNOWLEDGMENTS b/src/ACKNOWLEDGMENTS
index b20e5e245..91a0cf066 100644
--- a/src/ACKNOWLEDGMENTS
+++ b/src/ACKNOWLEDGMENTS
@@ -1,4 +1,4 @@
-$Cambridge: exim/src/ACKNOWLEDGMENTS,v 1.3 2004/11/10 10:29:56 ph10 Exp $
+$Cambridge: exim/src/ACKNOWLEDGMENTS,v 1.4 2004/11/10 14:15:20 ph10 Exp $
 
 EXIM ACKNOWLEDGEMENTS
 
@@ -134,6 +134,7 @@ Michael Haardt            Tidies to make the code stricter
                           Module to support Sieve (RFC 3028) filters and
                             continued maintenance of same
                           Patch for faster sort algorithm in queue.c
+                          Patch for LDAP timeout handling 
 Thomas Hager              Patch for saslauthd crash bug
 Richard Hall              Fix for file descriptor leak in redirection
 Steve Haslam              Lots of stuff, including
diff --git a/src/src/lookups/ldap.c b/src/src/lookups/ldap.c
index aa2e7bfd8..c4777bc87 100644
--- a/src/src/lookups/ldap.c
+++ b/src/src/lookups/ldap.c
@@ -1,4 +1,4 @@
-/* $Cambridge: exim/src/src/lookups/ldap.c,v 1.1 2004/10/07 13:10:01 ph10 Exp $ */
+/* $Cambridge: exim/src/src/lookups/ldap.c,v 1.2 2004/11/10 14:15:20 ph10 Exp $ */
 
 /*************************************************
 *     Exim - an Internet mail transport agent    *
@@ -72,13 +72,6 @@ LDAP_LIB_OPENLDAP1
 #endif
 
 
-/* For libraries without TCP connect timeouts */
-
-#ifndef LDAP_X_IO_TIMEOUT_NO_TIMEOUT
-#define LDAP_X_IO_TIMEOUT_NO_TIMEOUT (-1)
-#endif
-
-
 /* Four types of LDAP search are implemented */
 
 #define SEARCH_LDAP_MULTIPLE 0       /* Get attributes from multiple entries */
@@ -136,7 +129,7 @@ Arguments:
   password      password for authentication, or NULL
   sizelimit     max number of entries returned, or 0 for no limit
   timelimit     max time to wait, or 0 for no limit
-  tcplimit      max time to connect, or NULL for OS default
+  tcplimit      max time to connect, or 0 for OS default
   deference     the dereference option, which is one of
                   LDAP_DEREF_{NEVER,SEARCHING,FINDING,ALWAYS}
 
@@ -376,9 +369,20 @@ if (lcp == NULL)
   in Netscape SDK v4.1; I don't know about other libraries. */
 
   #ifdef LDAP_X_OPT_CONNECT_TIMEOUT
-  ldap_set_option(ld, LDAP_X_OPT_CONNECT_TIMEOUT, (void *)&tcplimit);
+  if (tcplimit > 0)
+    {
+    unsigned int timeout1000 = tcplimit*1000;
+    ldap_set_option(ld, LDAP_X_OPT_CONNECT_TIMEOUT, (void *)&timeout1000);
+    }
   #endif
 
+  /* Set the TCP connect timeout. This works with OpenLDAP 2.2.14. */
+
+  #ifdef LDAP_OPT_NETWORK_TIMEOUT
+  if (tcplimit > 0)
+    ldap_set_option(ld, LDAP_OPT_NETWORK_TIMEOUT, (void *)timeoutptr);
+  #endif 
+
   /* I could not get TLS to work until I set the version to 3. That version
   seems to be the default nowadays. The RFC is dated 1997, so I would hope
   that all the LDAP libraries support it. Therefore, if eldap_version hasn't
@@ -443,6 +447,15 @@ else
       host, porttext);
   }
 
+/* Whatever follows, obey this timeout in any requests. */
+
+if (tcplimit > 0)
+  {
+  timeout.tv_sec = tcplimit;
+  timeout.tv_usec = 0;
+  timeoutptr = &timeout;
+  }
+
 /* Bind with the user/password supplied, or an anonymous bind if these values
 are NULL, unless a cached connection is already bound with the same values. */
 
@@ -457,23 +470,41 @@ if (!lcp->bound ||
   {
   DEBUG(D_lookup) debug_printf("%sbinding with user=%s password=%s\n",
     (lcp->bound)? "re-" : "", user, password);
-  if ((rc = ldap_bind_s(lcp->ld, CS user, CS password, LDAP_AUTH_SIMPLE))
-       != LDAP_SUCCESS)
+  if ((msgid = ldap_bind(lcp->ld, CS user, CS password, LDAP_AUTH_SIMPLE))
+       == -1)
     {
-    /* Invalid credentials when just checking credentials returns FAIL. This
-    stops any further servers being tried. */
+    *errmsg = string_sprintf("failed to bind the LDAP connection to server "
+      "%s%s - LDAP error", host, porttext);
+    goto RETURN_ERROR;
+    }
 
-    if (search_type == SEARCH_LDAP_AUTH && rc == LDAP_INVALID_CREDENTIALS)
-      {
-      DEBUG(D_lookup)
-        debug_printf("Invalid credentials: ldapauth returns FAIL\n");
-      error_yield = FAIL;
-      goto RETURN_ERROR_NOMSG;
-      }
+  if ((rc = ldap_result( lcp->ld, msgid, 1, timeoutptr, &result )) <= 0)
+    {
+    *errmsg = string_sprintf("failed to bind the LDAP connection to server "
+      "%s%s - LDAP error: %s", host, porttext, 
+      rc == -1 ? "result retrieval failed" : "timeout" );
+    result = NULL;
+    goto RETURN_ERROR;
+    }
+
+  rc = ldap_result2error( lcp->ld, result, 0 );
+
+  /* Invalid credentials when just checking credentials returns FAIL. This
+  stops any further servers being tried. */
 
-    /* Otherwise we have a problem that doesn't stop further servers from being
-    tried. */
+  if (search_type == SEARCH_LDAP_AUTH && rc == LDAP_INVALID_CREDENTIALS)
+    {
+    DEBUG(D_lookup)
+      debug_printf("Invalid credentials: ldapauth returns FAIL\n");
+    error_yield = FAIL;
+    goto RETURN_ERROR_NOMSG;
+    }
 
+  /* Otherwise we have a problem that doesn't stop further servers from being
+  tried. */
+
+  if (rc != LDAP_SUCCESS)
+    {
     *errmsg = string_sprintf("failed to bind the LDAP connection to server "
       "%s%s - LDAP error %d: %s", host, porttext, rc, ldap_err2string(rc));
     goto RETURN_ERROR;
@@ -484,6 +515,9 @@ if (!lcp->bound ||
   lcp->bound = TRUE;
   lcp->user = (user == NULL)? NULL : string_copy(user);
   lcp->password = (password == NULL)? NULL : string_copy(password);
+
+  ldap_msgfree(result);
+  result = NULL;
   }
 
 /* If we are just checking credentials, return OK. */
@@ -528,13 +562,6 @@ if (msgid == -1)
 /* Loop to pick up results as they come in, setting a timeout if one was
 given. */
 
-if (timelimit > 0)
-  {
-  timeout.tv_sec = timelimit;
-  timeout.tv_usec = 0;
-  timeoutptr = &timeout;
-  }
-
 while ((rc = ldap_result(lcp->ld, msgid, 0, timeoutptr, &result)) ==
         LDAP_RES_SEARCH_ENTRY)
   {
@@ -916,7 +943,7 @@ control_ldap_search(uschar *ldap_url, int search_type, uschar **res,
 BOOL defer_break = FALSE;
 int timelimit = LDAP_NO_LIMIT;
 int sizelimit = LDAP_NO_LIMIT;
-int tcplimit = LDAP_X_IO_TIMEOUT_NO_TIMEOUT;
+int tcplimit = 0;
 int dereference = LDAP_DEREF_NEVER;
 int sep = 0;
 uschar *url = ldap_url;
@@ -949,7 +976,8 @@ while (strncmpic(url, US"ldap", 4) != 0)
       else if (strncmpic(name, US"PASS=", namelen) == 0) password = value;
       else if (strncmpic(name, US"SIZE=", namelen) == 0) sizelimit = Uatoi(value);
       else if (strncmpic(name, US"TIME=", namelen) == 0) timelimit = Uatoi(value);
-      else if (strncmpic(name, US"CONNECT=", namelen) == 0) tcplimit = Uatoi(value) * 1000;
+      else if (strncmpic(name, US"CONNECT=", namelen) == 0) tcplimit = Uatoi(value);
+      else if (strncmpic(name, US"NETTIME=", namelen) == 0) tcplimit = Uatoi(value);
 
       /* Don't know if all LDAP libraries have LDAP_OPT_DEREF */