httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Max Bowsher <ma...@ukf.net>
Subject Re: [PATCH] Re: mod_authz_core:check_provider_list bug?
Date Thu, 09 Mar 2006 17:15:33 GMT
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

>> Joe Orton wrote:
>>> Found by the Coverity report, this one looks like a real bug:
>>>
>>> check_provider_list has a if() branch to handle the passed-in 
>>> current_provider being NULL, but never sets it to anything else; 
>>> current_provider is later dereferenced unconditionally.

> Max Bowsher wrote:
>> It looks like the authn-provider implementation was cloned to 
>> produce starting point of the authz-provider implementation, and
>> this code wasn't removed when it became redundant.
>> 
>> All callers of check_provider_list() ensure that they pass a 
>> non-NULL current_provider. The AUTHZ_DEFAULT_PROVIDER that is being
>> looked up in this code is never registered. The
>> no-providers-configured case is dealt with in authorize_user(),
>> before check_provider_list() is ever called. I suggest the
>> following patch:
>>
>> [[[
>> * modules/aaa/mod_authz_core.c (check_provider_list):
>>   Remove redundant code.
>> * modules/aaa/mod_auth.h (AUTHZ_DEFAULT_PROVIDER):
>>   Remove redundant definition.
>> ]]]
>>
...

Brad Nicholes wrote:
> +1  During the develop, mod_authz_default flip-flopped between being a
> provider vs. hook.  It turned out that ultimately authz_default need to
> remain a hook so that it could be guaranteed to run last.  So the
> provider 'default' doesn't actually exist.  Also given that with the
> checks that are in place before check_provider_list() is called prevent
> current_provider from being a NULL value, removing the code that
> attempts to load a default authz provider seems reasonable.  Also the
> concept of a default provider should happen fairly automatically anyway

(((
> since the access control directives 'Allow/Deny' have been folded in as
> providers.
)))
^^^ This bit isn't true.

I agree with everything else, though.

> Setting 'Require All Granted' or 'Require All Denied' for a
> directory carries through to sub directories as a default provider. 
> Given that our standard httpd.conf specifies 'Require all denied' for
> root, the 'all denied' becomes the default provider. 



Max.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (Cygwin)

iD8DBQFEEGK1fFNSmcDyxYARAok5AJ9W2/x2dya0O52dyQBzNT0pO6fjJACfQEeN
KZLg36IBuSkJp06Nb41nvjw=
=5Tw3
-----END PGP SIGNATURE-----

Mime
View raw message