Merge r1231255, r1231257 from trunk:

  *) mod_authnz_ldap: Don't try a potentially expensive nested groups
     search before exhausting all AuthLDAPGroupAttribute checks on the
     current group. PR52464

whitespace only: shift a block refactored in r1231255 over 8 spaces.

Submitted by: covener
Reviewed/backported by: jim

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1374256 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 56438e63e032b76ed43085500f0aeef8e7cb3df9..242ad2ec654e5368e4d4e9817ecfb00e970c96a6 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -7,6 +7,10 @@ Changes with Apache 2.4.3
      possible XSS for a site where untrusted users can upload files to
      a location with MultiViews enabled. [Niels Heinen <heinenn google.com>]
 
+  *) mod_authnz_ldap: Don't try a potentially expensive nested groups
+     search before exhausting all AuthLDAPGroupAttribute checks on the
+     current group. PR52464 [Eric Covener]
+
   *) mod_lua: Add new directive LuaAuthzProvider to allow implementing an
      authorization provider in lua. [Stefan Fritsch]
 

diff --git a/STATUS b/STATUS
index 6246cfbf13ec4e491e53140760a1b3f2818f91f3..015861d9e290aaab340bcb3cbcdf1ed1d660491a 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -88,13 +88,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-   * mod_authnz_ldap: Don't try a potentially expensive nested groups
-     search before exhausting all AuthLDAPGroupAttribute checks on the
-     current group. PR52464
-     trunk patch: http://svn.apache.org/viewvc?rev=1231255&view=rev
-                  + whitespace change http://svn.apache.org/viewvc?rev=1231257&view=rev
-     2.4.x patch: trunk patch works
-     +1: covener, rjung, jim
 
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:

diff --git a/modules/aaa/mod_authnz_ldap.c b/modules/aaa/mod_authnz_ldap.c
index 689d5b76a94d6d7ffe6da907064755d7297088c0..d55b57f5c8d66b68573d9a0b3f4f27f05cb92bfd 100644 (file)
--- a/modules/aaa/mod_authnz_ldap.c
+++ b/modules/aaa/mod_authnz_ldap.c
@@ -870,6 +870,7 @@ static authz_status ldapgroup_check_authorization(request_rec *r,
                   "membership in \"%s\"",
                   t);
 
+    /* PR52464 exhaust attrs in base group before checking subgroups */
     for (i = 0; i < sec->groupattr->nelts; i++) {
         ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01714)
                       "auth_ldap authorize: require group: testing for %s: "
@@ -879,64 +880,62 @@ static authz_status ldapgroup_check_authorization(request_rec *r,
 
         result = util_ldap_cache_compare(r, ldc, sec->url, t, ent[i].name,
                              sec->group_attrib_is_dn ? req->dn : req->user);
-        switch(result) {
-            case LDAP_COMPARE_TRUE: {
-                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01715)
-                              "auth_ldap authorize: require group: "
-                              "authorization successful (attribute %s) "
-                              "[%s][%d - %s]",
-                              ent[i].name, ldc->reason, result,
-                              ldap_err2string(result));
-                set_request_vars(r, LDAP_AUTHZ);
-                return AUTHZ_GRANTED;
-            }
-            case LDAP_NO_SUCH_ATTRIBUTE:
-            case LDAP_COMPARE_FALSE: {
-                /* nested groups need searches and compares, so grab a new handle */
-                authnz_ldap_cleanup_connection_close(ldc);
-                apr_pool_cleanup_kill(r->pool, ldc,authnz_ldap_cleanup_connection_close);
-
-                ldc = get_connection_for_authz(r, LDAP_COMPARE_AND_SEARCH);
-                apr_pool_cleanup_register(r->pool, ldc,
-                                          authnz_ldap_cleanup_connection_close,
-                                          apr_pool_cleanup_null);
-
-                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01716)
-                               "auth_ldap authorise: require group \"%s\": "
-                               "failed [%s][%d - %s], checking sub-groups",
-                               t, ldc->reason, result, ldap_err2string(result));
-
-                result = util_ldap_cache_check_subgroups(r, ldc, sec->url, t, ent[i].name,
-                                                         sec->group_attrib_is_dn ? req->dn : req->user,
-                                                         sec->sgAttributes[0] ? sec->sgAttributes : default_attributes,
-                                                         sec->subgroupclasses,
-                                                         0, sec->maxNestingDepth);
-                if(result == LDAP_COMPARE_TRUE) {
-                    ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01717)
-                                  "auth_ldap authorise: require group "
-                                  "(sub-group): authorisation successful "
-                                  "(attribute %s) [%s][%d - %s]",
-                                  ent[i].name, ldc->reason, result,
-                                  ldap_err2string(result));
-                    set_request_vars(r, LDAP_AUTHZ);
-                    return AUTHZ_GRANTED;
-                }
-                else {
-                    ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01718)
-                                  "auth_ldap authorise: require group "
-                                  "(sub-group) \"%s\": authorisation failed "
-                                  "[%s][%d - %s]",
-                                  t, ldc->reason, result,
-                                  ldap_err2string(result));
-                }
-                break;
-            }
-            default: {
+        if (result == LDAP_COMPARE_TRUE) {
+            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01715)
+                          "auth_ldap authorize: require group: "
+                          "authorization successful (attribute %s) "
+                          "[%s][%d - %s]",
+                          ent[i].name, ldc->reason, result,
+                          ldap_err2string(result));
+            set_request_vars(r, LDAP_AUTHZ);
+            return AUTHZ_GRANTED;
+        }
+        else { 
                 ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01719)
                               "auth_ldap authorize: require group \"%s\": "
-                              "authorization failed [%s][%d - %s]",
-                              t, ldc->reason, result, ldap_err2string(result));
-            }
+                              "didn't match with attr %s [%s][%d - %s]",
+                              t, ldc->reason, ent[i].name, result, 
+                              ldap_err2string(result));
+        }
+    }
+    
+    for (i = 0; i < sec->groupattr->nelts; i++) {
+        /* nested groups need searches and compares, so grab a new handle */
+        authnz_ldap_cleanup_connection_close(ldc);
+        apr_pool_cleanup_kill(r->pool, ldc,authnz_ldap_cleanup_connection_close);
+
+        ldc = get_connection_for_authz(r, LDAP_COMPARE_AND_SEARCH);
+        apr_pool_cleanup_register(r->pool, ldc,
+                                  authnz_ldap_cleanup_connection_close,
+                                  apr_pool_cleanup_null);
+
+        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01716)
+                       "auth_ldap authorise: require group \"%s\": "
+                       "failed [%s][%d - %s], checking sub-groups",
+                       t, ldc->reason, result, ldap_err2string(result));
+
+        result = util_ldap_cache_check_subgroups(r, ldc, sec->url, t, ent[i].name,
+                                                 sec->group_attrib_is_dn ? req->dn : req->user,
+                                                 sec->sgAttributes[0] ? sec->sgAttributes : default_attributes,
+                                                 sec->subgroupclasses,
+                                                 0, sec->maxNestingDepth);
+        if (result == LDAP_COMPARE_TRUE) {
+            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01717)
+                          "auth_ldap authorise: require group "
+                          "(sub-group): authorisation successful "
+                          "(attribute %s) [%s][%d - %s]",
+                          ent[i].name, ldc->reason, result,
+                          ldap_err2string(result));
+            set_request_vars(r, LDAP_AUTHZ);
+            return AUTHZ_GRANTED;
+        }
+        else {
+            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01718)
+                          "auth_ldap authorise: require group "
+                          "(sub-group) \"%s\": didn't match with attr %s "
+                          "[%s][%d - %s]",
+                          t, ldc->reason, ent[i].name, result, 
+                          ldap_err2string(result));
         }
     }