httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yavor Trapkov <trap...@netscape.net>
Subject [patch] mod_auth_ldap doesn't effectively use the cache with "require user User1 User2 .." directives
Date Sun, 09 Mar 2003 11:11:02 GMT
mod_auth_ldap doesn't effectively use the cache with "require user User1 
User2
.." directives

What the module does:

 - firstly, it checks if the whole string "User1 User2 .." matches the 
CN of the
   authenticated user and as this is a very rear situation it almost always
   fails so each time we request a page, the WEB server sends a LDAP 
query as this
   is never cached as a negative result

 - secondly, there is a loop that checks if every single entry in the list
   matches the CN of the authenticated user
     = it checks if this is a cached positive result
     = and if not it sends a LDAP query
     = this happens until it finds a match or the list finishes

if the authenticated user is the 100th one into the "require user User1 
User2
.." list we are going to have 99 LDAP requests sent and then the cache will
return the positive answer.

In practise we don't really use the cache here, and such kind of 
"require user
User1 User2 .. UserN" directives can send hundreds of LDAP queries
each time we load a web page if the list is long enough! This seems to 
be able
to disable some LDAP servers (at least I've experienced this with NW 
one). The
real trouble is when we use products like DreamWeaver with WebDAV aceess to
the web site - when opens the site, it makes an inventory sending 
PROPFIND to
every object on the site, in such a cases even two UserNames into the 
list are
enough to flood the LDAP server.

What I propose to be changed:

  - firstly, to check all words into the list only against the cache and 
not send
    LDAP queries
  - secondly, if a match is not found then check all words into the list
    sending LDAP queries to the server
  - at last, to check for the whole string "user1 user2 .." as this is very
    rear case and in almost all cases gives a negative result

    (even more sophisticated idea could be to assemble a single "OR" 
query and send
     to the server)

this requires spitting the util_ldap_cache_compare:

util_ldap_cache_only_compare - checks for a cache match
util_ldap_server_only_compare - sends a request to the LDAP server

A proposed patch follows, I haven't compiled it with httpd2.0, but I
successfully applied a similar one with the auth_ldap module for apache 1.3.
Best Regards
Yavor Trapkov

--- mod_auth_ldap.c.org 2003-03-09 08:15:12.000000000 +0100
+++ mod_auth_ldap.c     2003-03-09 10:34:57.000000000 +0100
@@ -548,30 +548,35 @@
                 return sec->auth_authoritative? HTTP_UNAUTHORIZED : 
DECLINED;
             }
             /*
-             * First do a whole-line compare, in case it's something like
-             *   require user Babs Jensen
+             * Now break apart the line and compare each word on it 
against the cache
              */
-            result = util_ldap_cache_compare(r, ldc, sec->url, req->dn, 
sec->attribute, t);
-            switch(result) {
-                case LDAP_COMPARE_TRUE: {
-                    ap_log_rerror(APLOG_MARK, 
APLOG_DEBUG|APLOG_NOERRNO, 0, r,
-                                  "[%d] auth_ldap authorise: "
-                                  "require user: authorisation 
successful", getpid());
-                    return OK;
-                }
-                default: {
-                    ap_log_rerror(APLOG_MARK, 
APLOG_DEBUG|APLOG_NOERRNO, 0, r,
-                                  "[%d] auth_ldap authorise: require 
user: "
-                                  "authorisation failed [%s][%s]", 
getpid(),
-                                  ldc->reason, ldap_err2string(result));
+            while (t[0]) {
+                w = ap_getword_conf(r->pool, &t);
+                result = util_ldap_cache_only_compare(r, ldc, sec->url, 
req->dn, sec->attribute, w);
+                switch(result) {
+                    case LDAP_COMPARE_TRUE: {
+                        ap_log_rerror(APLOG_MARK, 
APLOG_DEBUG|APLOG_NOERRNO, 0, r,
+                                      "[%d] auth_ldap authorise: "
+                                      "require user: authorisation 
successful", getpid());
+                        return OK;
+                    }
+                    default: {
+                        ap_log_rerror(APLOG_MARK, 
APLOG_DEBUG|APLOG_NOERRNO, 0, r,
+                                      "[%d] auth_ldap authorise: "
+                                      "require user: authorisation 
failed [%s][%s]",
+                                      getpid(), ldc->reason, 
ldap_err2string(result));
+                    }
                 }
             }
             /*
-             * Now break apart the line and compare each word on it
+             * Now break apart the line and compare each word on it 
against the LDAP server
              */
+           t = reqs[x].requirement;
+           w = ap_getword(r->pool, &t, ' ');
+
             while (t[0]) {
                w = ap_getword_conf(r->pool, &t);
-                result = util_ldap_cache_compare(r, ldc, sec->url, 
req->dn, sec->attribute, w);
+                result = util_ldap_server_only_compare(r, ldc, 
sec->url, req->dn, sec->attribute, w);
                 switch(result) {
                     case LDAP_COMPARE_TRUE: {
                         ap_log_rerror(APLOG_MARK, 
APLOG_DEBUG|APLOG_NOERRNO, 0, r,
@@ -587,6 +592,29 @@
                     }
                 }
             }
+            /*
+             * Do a whole-line compare, in case it's something like 
"require user Babs Jensen"
+             * as this is a very rear case, we leave it for the end
+             */
+            t = reqs[x].requirement;
+            w = ap_getword(r->pool, &t, ' ');
+
+            result = util_ldap_cache_compare(r, ldc, sec->url, req->dn, 
sec->attribute, t);
+            switch(result) {
+                case LDAP_COMPARE_TRUE: {
+                    ap_log_rerror(APLOG_MARK, 
APLOG_DEBUG|APLOG_NOERRNO, 0, r,
+                                  "[%d] auth_ldap authorise: "
+                                  "require user: authorisation 
successful", getpid());
+                    return OK;
+                }
+                default: {
+                    ap_log_rerror(APLOG_MARK, 
APLOG_DEBUG|APLOG_NOERRNO, 0, r,
+                                  "[%d] auth_ldap authorise: require 
user: "
+                                  "authorisation failed [%s][%s]", 
getpid(),
+                                  ldc->reason, ldap_err2string(result));
+                }
+            }
+
         }
         else if (strcmp(w, "dn") == 0) {
             if (req->dn == NULL || strlen(req->dn) == 0) {
--- util_ldap.c.org     2003-03-09 08:46:50.000000000 +0100
+++ util_ldap.c 2003-03-09 10:36:52.000000000 +0100
@@ -733,6 +733,145 @@
     return result;
 }
+LDAP_DECLARE(int) util_ldap_cache_only_compare(request_rec *r, 
util_ldap_connection_t *ldc,
+                          const char *url, const char *dn,
+                          const char *attrib, const char *value)
+{
+    int result = 0;
+    util_url_node_t *curl;
+    util_url_node_t curnode;
+    util_compare_node_t *compare_nodep;
+    util_compare_node_t the_compare_node;
+    apr_time_t curtime;
+
+    util_ldap_state_t *st =
+        (util_ldap_state_t *)ap_get_module_config(r->server->module_config,
+        &ldap_module);
+
+    /* read lock this function */
+    LDAP_CACHE_LOCK_CREATE(st->pool);
+
+    /* get cache entry (or create one) */
+    LDAP_CACHE_WRLOCK();
+    curnode.url = url;
+    curl = util_ald_cache_fetch(util_ldap_cache, &curnode);
+    if (curl == NULL) {
+        curl = util_ald_create_caches(st, url);
+    }
+    LDAP_CACHE_UNLOCK();
+
+    if (curl) {
+        /* make a comparison to the cache */
+        LDAP_CACHE_RDLOCK();
+        curtime = apr_time_now();
+
+        the_compare_node.dn = (char *)dn;
+        the_compare_node.attrib = (char *)attrib;
+        the_compare_node.value = (char *)value;
+        the_compare_node.result = 0;
+
+        compare_nodep = util_ald_cache_fetch(curl->compare_cache, 
&the_compare_node);
+
+        if (compare_nodep != NULL) {
+            /* found it... */
+            if (curtime - compare_nodep->lastcompare > 
st->compare_cache_ttl) {
+                /* ...but it is too old */
+                util_ald_cache_remove(curl->compare_cache, compare_nodep);
+            }
+            else {
+                /* ...and it is good */
+                /* unlock this read lock */
+                LDAP_CACHE_UNLOCK();
+                if (LDAP_COMPARE_TRUE == compare_nodep->result) {
+                    ldc->reason = "Comparison true (cached)";
+                    return compare_nodep->result;
+                }
+                else if (LDAP_COMPARE_FALSE == compare_nodep->result) {
+                    ldc->reason = "Comparison false (cached)";
+                    return compare_nodep->result;
+                }
+                else if (LDAP_NO_SUCH_ATTRIBUTE == compare_nodep->result) {
+                    ldc->reason = "Comparison no such attribute (cached)";
+                    return compare_nodep->result;
+                }
+                else {
+                    ldc->reason = "Comparison undefined (cached)";
+                    return compare_nodep->result;
+                }
+            }
+        }
+        /* unlock this read lock */
+        LDAP_CACHE_UNLOCK();
+    }
+    ldc->reason = "Comparison false (cached)";
+    return result;
+}
+
+LDAP_DECLARE(int) util_ldap_server_only_compare(request_rec *r, 
util_ldap_connection_t *ldc,
+                          const char *url, const char *dn,
+                          const char *attrib, const char *value)
+{
+    int result = 0;
+    util_url_node_t *curl;
+    util_url_node_t curnode;
+    util_compare_node_t *compare_nodep;
+    util_compare_node_t the_compare_node;
+    apr_time_t curtime;
+    int failures = 0;
+
+    util_ldap_state_t *st =
+        (util_ldap_state_t *)ap_get_module_config(r->server->module_config,
+        &ldap_module);
+
+        /* read lock this function */
+        LDAP_CACHE_LOCK_CREATE(st->pool);
+
+start_over:
+    if (failures++ > 10) {
+        /* too many failures */
+        return result;
+    }
+    if (LDAP_SUCCESS != (result = util_ldap_connection_open(r, ldc))) {
+        /* connect failed */
+        return result;
+    }
+
+    if ((result = ldap_compare_s(ldc->ldap, const_cast(dn), 
const_cast(attrib), const_cast(value)))
+        == LDAP_SERVER_DOWN) {
+        /* connection failed - try again */
+        util_ldap_connection_close(ldc);
+        ldc->reason = "ldap_compare_s() failed with server down";
+        goto start_over;
+    }
+
+    ldc->reason = "Comparison complete";
+    if ((LDAP_COMPARE_TRUE == result) ||
+        (LDAP_COMPARE_FALSE == result) ||
+        (LDAP_NO_SUCH_ATTRIBUTE == result)) {
+        if (curl) {
+            /* compare completed; caching result */
+            LDAP_CACHE_WRLOCK();
+            the_compare_node.lastcompare = curtime;
+            the_compare_node.result = result;
+            util_ald_cache_insert(curl->compare_cache, &the_compare_node);
+            LDAP_CACHE_UNLOCK();
+        }
+        if (LDAP_COMPARE_TRUE == result) {
+            ldc->reason = "Comparison true (adding to cache)";
+            return LDAP_COMPARE_TRUE;
+        }
+        else if (LDAP_COMPARE_FALSE == result) {
+            ldc->reason = "Comparison false (adding to cache)";
+            return LDAP_COMPARE_FALSE;
+        }
+        else {
+            ldc->reason = "Comparison no such attribute (adding to cache)";
+            return LDAP_NO_SUCH_ATTRIBUTE;
+        }
+    }
+    return result;
+}
+
 LDAP_DECLARE(int) util_ldap_cache_checkuserid(request_rec *r, 
util_ldap_connection_t *ldc,
                               const char *url, const char *basedn, int 
scope, char **attrs,
                               const char *filter, const char *bindpw, 
const char **binddn,







Mime
View raw message