You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ji...@apache.org on 2009/09/20 19:50:19 UTC
svn commit: r817064 - in /httpd/httpd/branches/2.2.x: STATUS
modules/aaa/mod_authnz_ldap.c
Author: jim
Date: Sun Sep 20 17:50:19 2009
New Revision: 817064
URL: http://svn.apache.org/viewvc?rev=817064&view=rev
Log:
* mod_ldap: Pre-scan the requirements array before doing any LDAP lookups,
for cases where an LDAP URL is configured but non-LDAP authn/authz is in
effect. This stops us from trying to resolve file-based userids to a DN
when the AuthLDAPURL has been defined at a very high level.
PR 45946
Trunk patch: n/a due to authz refactoring (no provider called without require-ments)
2.2.x version of patch: http://people.apache.org/~covener/httpd-2.2.x-authnz_ldap-skipdnloookup-3.diff
+1: covener, minfrin, jim
Modified:
httpd/httpd/branches/2.2.x/STATUS
httpd/httpd/branches/2.2.x/modules/aaa/mod_authnz_ldap.c
Modified: httpd/httpd/branches/2.2.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?rev=817064&r1=817063&r2=817064&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/STATUS (original)
+++ httpd/httpd/branches/2.2.x/STATUS Sun Sep 20 17:50:19 2009
@@ -89,15 +89,6 @@
PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
[ start all new proposals below, under PATCHES PROPOSED. ]
- * mod_ldap: Pre-scan the requirements array before doing any LDAP lookups,
- for cases where an LDAP URL is configured but non-LDAP authn/authz is in
- effect. This stops us from trying to resolve file-based userids to a DN
- when the AuthLDAPURL has been defined at a very high level.
- PR 45946
- Trunk patch: n/a due to authz refactoring (no provider called without require-ments)
- 2.2.x version of patch: http://people.apache.org/~covener/httpd-2.2.x-authnz_ldap-skipdnloookup-3.diff
- +1: covener, minfrin, jim
-
PATCHES PROPOSED TO BACKPORT FROM TRUNK:
[ New proposals should be added at the end of the list ]
Modified: httpd/httpd/branches/2.2.x/modules/aaa/mod_authnz_ldap.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/aaa/mod_authnz_ldap.c?rev=817064&r1=817063&r2=817064&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/aaa/mod_authnz_ldap.c (original)
+++ httpd/httpd/branches/2.2.x/modules/aaa/mod_authnz_ldap.c Sun Sep 20 17:50:19 2009
@@ -527,6 +527,29 @@
return DECLINED;
}
+ /* pre-scan for ldap-* requirements so we can get out of the way early */
+ for(x=0; x < reqs_arr->nelts; x++) {
+ if (! (reqs[x].method_mask & (AP_METHOD_BIT << m))) {
+ continue;
+ }
+
+ t = reqs[x].requirement;
+ w = ap_getword_white(r->pool, &t);
+
+ if (strncmp(w, "ldap-",5) == 0) {
+ required_ldap = 1;
+ break;
+ }
+ }
+
+ if (!required_ldap) {
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+ "[%" APR_PID_T_FMT "] auth_ldap authorise: declining to authorise (no ldap requirements)", getpid());
+ return DECLINED;
+ }
+
+
+
if (sec->host) {
ldc = util_ldap_connection_find(r, sec->host, sec->port,
sec->binddn, sec->bindpw, sec->deref,
@@ -559,12 +582,6 @@
#endif
}
- if (!reqs_arr) {
- ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
- "[%" APR_PID_T_FMT "] auth_ldap authorise: no requirements array", getpid());
- return sec->auth_authoritative? HTTP_UNAUTHORIZED : DECLINED;
- }
-
/*
* If we have been authenticated by some other module than mod_auth_ldap,
* the req structure needed for authorization needs to be created
@@ -615,7 +632,6 @@
w = ap_getword_white(r->pool, &t);
if (strcmp(w, "ldap-user") == 0) {
- required_ldap = 1;
if (req->dn == NULL || strlen(req->dn) == 0) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
"[%" APR_PID_T_FMT "] auth_ldap authorise: "
@@ -665,7 +681,6 @@
}
}
else if (strcmp(w, "ldap-dn") == 0) {
- required_ldap = 1;
if (req->dn == NULL || strlen(req->dn) == 0) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
"[%" APR_PID_T_FMT "] auth_ldap authorise: "
@@ -693,7 +708,6 @@
else if (strcmp(w, "ldap-group") == 0) {
struct mod_auth_ldap_groupattr_entry_t *ent = (struct mod_auth_ldap_groupattr_entry_t *) sec->groupattr->elts;
int i;
- required_ldap = 1;
if (sec->group_attrib_is_dn) {
if (req->dn == NULL || strlen(req->dn) == 0) {
@@ -743,7 +757,6 @@
}
}
else if (strcmp(w, "ldap-attribute") == 0) {
- required_ldap = 1;
if (req->dn == NULL || strlen(req->dn) == 0) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
"[%" APR_PID_T_FMT "] auth_ldap authorise: "
@@ -779,7 +792,6 @@
}
}
else if (strcmp(w, "ldap-filter") == 0) {
- required_ldap = 1;
if (req->dn == NULL || strlen(req->dn) == 0) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
"[%" APR_PID_T_FMT "] auth_ldap authorise: "
@@ -843,9 +855,9 @@
return OK;
}
- if (!required_ldap || !sec->auth_authoritative) {
+ if (!sec->auth_authoritative) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
- "[%" APR_PID_T_FMT "] auth_ldap authorise: declining to authorise", getpid());
+ "[%" APR_PID_T_FMT "] auth_ldap authorise: declining to authorise (not authoritative)", getpid());
return DECLINED;
}
RE: svn commit: r817064 - in /httpd/httpd/branches/2.2.x: STATUS modules/aaa/mod_authnz_ldap.c
Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
Yes, this answers my question. Thanks for the pointer.
Regards
Rüdiger
________________________________
From: Jeff Trawick [mailto:trawick@gmail.com]
Sent: Montag, 21. September 2009 13:06
To: dev@httpd.apache.org
Subject: Re: svn commit: r817064 - in /httpd/httpd/branches/2.2.x: STATUS modules/aaa/mod_authnz_ldap.c
On Mon, Sep 21, 2009 at 6:54 AM, Graham Leggett <mi...@sharp.fm> wrote:
Ruediger Pluem wrote:
>> + /* pre-scan for ldap-* requirements so we can get out of the way early */
>> + for(x=0; x < reqs_arr->nelts; x++) {
>
> Why do we know that reqs_arr != NULL always?
Wasn't that the FIXME comment that was included in the previous version
of this patch?
I hope this is the answer ;)
http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?r1=815281&r2=815280&pathrev=815281
Re: svn commit: r817064 - in /httpd/httpd/branches/2.2.x: STATUS
modules/aaa/mod_authnz_ldap.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Mon, Sep 21, 2009 at 6:54 AM, Graham Leggett <mi...@sharp.fm> wrote:
> Ruediger Pluem wrote:
>
> >> + /* pre-scan for ldap-* requirements so we can get out of the way
> early */
> >> + for(x=0; x < reqs_arr->nelts; x++) {
> >
> > Why do we know that reqs_arr != NULL always?
>
> Wasn't that the FIXME comment that was included in the previous version
> of this patch?
>
I hope this is the answer ;)
http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?r1=815281&r2=815280&pathrev=815281
RE: svn commit: r817064 - in /httpd/httpd/branches/2.2.x: STATUS modules/aaa/mod_authnz_ldap.c
Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
> -----Original Message-----
> From: Graham Leggett
> Sent: Montag, 21. September 2009 12:54
> To: dev@httpd.apache.org
> Subject: Re: svn commit: r817064 - in
> /httpd/httpd/branches/2.2.x: STATUS modules/aaa/mod_authnz_ldap.c
>
> Ruediger Pluem wrote:
>
> >> @@ -559,12 +582,6 @@
> >> #endif
> >> }
> >>
> >> - if (!reqs_arr) {
> >> - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
> >> - "[%" APR_PID_T_FMT "] auth_ldap
> authorise: no requirements array", getpid());
> >> - return sec->auth_authoritative? HTTP_UNAUTHORIZED
> : DECLINED;
> >> - }
> >> -
> >
> > Why is this not needed any longer?
>
> I read it that this:
>
> >> - if (!reqs_arr) {
> >> - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
> >> - "[%" APR_PID_T_FMT "] auth_ldap
> authorise: no
> requirements array", getpid());
> >> - return sec->auth_authoritative? HTTP_UNAUTHORIZED
> : DECLINED;
> >> - }
> >> -
>
> was replaced by this:
>
> >> + if (!required_ldap) {
> >> + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
> >> + "[%" APR_PID_T_FMT "] auth_ldap authorise:
> declining to authorise (no ldap requirements)", getpid());
> >> + return DECLINED;
> >> + }
Yes, but the new code always returns DECLINED whereas the old one does
return DECLINED or HTTP_UNAUTHORIZED depending on sec->auth_authoritative.
But maybe it makes sense to return always DECLINED if there is no ldap-
require.
Regards
Rüdiger
Re: svn commit: r817064 - in /httpd/httpd/branches/2.2.x: STATUS
modules/aaa/mod_authnz_ldap.c
Posted by Graham Leggett <mi...@sharp.fm>.
Ruediger Pluem wrote:
>> + /* pre-scan for ldap-* requirements so we can get out of the way early */
>> + for(x=0; x < reqs_arr->nelts; x++) {
>
> Why do we know that reqs_arr != NULL always?
Wasn't that the FIXME comment that was included in the previous version
of this patch?
>> + if (! (reqs[x].method_mask & (AP_METHOD_BIT << m))) {
>> + continue;
>> + }
>> +
>> + t = reqs[x].requirement;
>> + w = ap_getword_white(r->pool, &t);
>> +
>> + if (strncmp(w, "ldap-",5) == 0) {
>> + required_ldap = 1;
>> + break;
>> + }
>> + }
>> +
>> + if (!required_ldap) {
>> + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
>> + "[%" APR_PID_T_FMT "] auth_ldap authorise: declining to authorise (no ldap requirements)", getpid());
>> + return DECLINED;
>> + }
>> +
>> +
>> +
>> if (sec->host) {
>> ldc = util_ldap_connection_find(r, sec->host, sec->port,
>> sec->binddn, sec->bindpw, sec->deref,
>> @@ -559,12 +582,6 @@
>> #endif
>> }
>>
>> - if (!reqs_arr) {
>> - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
>> - "[%" APR_PID_T_FMT "] auth_ldap authorise: no requirements array", getpid());
>> - return sec->auth_authoritative? HTTP_UNAUTHORIZED : DECLINED;
>> - }
>> -
>
> Why is this not needed any longer?
I read it that this:
>> - if (!reqs_arr) {
>> - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
>> - "[%" APR_PID_T_FMT "] auth_ldap authorise: no
requirements array", getpid());
>> - return sec->auth_authoritative? HTTP_UNAUTHORIZED : DECLINED;
>> - }
>> -
was replaced by this:
>> + if (!required_ldap) {
>> + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
>> + "[%" APR_PID_T_FMT "] auth_ldap authorise:
declining to authorise (no ldap requirements)", getpid());
>> + return DECLINED;
>> + }
Regards,
Graham
--
Re: svn commit: r817064 - in /httpd/httpd/branches/2.2.x: STATUS
modules/aaa/mod_authnz_ldap.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 09/20/2009 07:50 PM, jim@apache.org wrote:
> Author: jim
> Date: Sun Sep 20 17:50:19 2009
> New Revision: 817064
>
> URL: http://svn.apache.org/viewvc?rev=817064&view=rev
> Log:
> * mod_ldap: Pre-scan the requirements array before doing any LDAP lookups,
> for cases where an LDAP URL is configured but non-LDAP authn/authz is in
> effect. This stops us from trying to resolve file-based userids to a DN
> when the AuthLDAPURL has been defined at a very high level.
> PR 45946
> Trunk patch: n/a due to authz refactoring (no provider called without require-ments)
> 2.2.x version of patch: http://people.apache.org/~covener/httpd-2.2.x-authnz_ldap-skipdnloookup-3.diff
> +1: covener, minfrin, jim
>
>
>
> Modified:
> httpd/httpd/branches/2.2.x/STATUS
> httpd/httpd/branches/2.2.x/modules/aaa/mod_authnz_ldap.c
>
> Modified: httpd/httpd/branches/2.2.x/modules/aaa/mod_authnz_ldap.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/aaa/mod_authnz_ldap.c?rev=817064&r1=817063&r2=817064&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.2.x/modules/aaa/mod_authnz_ldap.c (original)
> +++ httpd/httpd/branches/2.2.x/modules/aaa/mod_authnz_ldap.c Sun Sep 20 17:50:19 2009
> @@ -527,6 +527,29 @@
> return DECLINED;
> }
>
> + /* pre-scan for ldap-* requirements so we can get out of the way early */
> + for(x=0; x < reqs_arr->nelts; x++) {
Why do we know that reqs_arr != NULL always?
> + if (! (reqs[x].method_mask & (AP_METHOD_BIT << m))) {
> + continue;
> + }
> +
> + t = reqs[x].requirement;
> + w = ap_getword_white(r->pool, &t);
> +
> + if (strncmp(w, "ldap-",5) == 0) {
> + required_ldap = 1;
> + break;
> + }
> + }
> +
> + if (!required_ldap) {
> + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
> + "[%" APR_PID_T_FMT "] auth_ldap authorise: declining to authorise (no ldap requirements)", getpid());
> + return DECLINED;
> + }
> +
> +
> +
> if (sec->host) {
> ldc = util_ldap_connection_find(r, sec->host, sec->port,
> sec->binddn, sec->bindpw, sec->deref,
> @@ -559,12 +582,6 @@
> #endif
> }
>
> - if (!reqs_arr) {
> - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
> - "[%" APR_PID_T_FMT "] auth_ldap authorise: no requirements array", getpid());
> - return sec->auth_authoritative? HTTP_UNAUTHORIZED : DECLINED;
> - }
> -
Why is this not needed any longer?
Regards
Rüdiger