From 11617ed7ee0ef7886d1df380ec86f340439a6fcd Mon Sep 17 00:00:00 2001 From: Jim Jagielski Date: Fri, 17 Aug 2012 13:48:26 +0000 Subject: [PATCH] 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 --- CHANGES | 4 ++ STATUS | 7 --- modules/aaa/mod_authnz_ldap.c | 111 +++++++++++++++++----------------- 3 files changed, 59 insertions(+), 63 deletions(-) diff --git a/CHANGES b/CHANGES index 56438e63e0..242ad2ec65 100644 --- 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 ] + *) 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 6246cfbf13..015861d9e2 100644 --- 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 689d5b76a9..d55b57f5c8 100644 --- 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)); } } -- 2.40.0