You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by sf...@apache.org on 2011/01/08 15:29:12 UTC
svn commit: r1056713 - in /httpd/httpd/trunk: CHANGES
modules/aaa/mod_authz_core.c
Author: sf
Date: Sat Jan 8 14:29:12 2011
New Revision: 1056713
URL: http://svn.apache.org/viewvc?rev=1056713&view=rev
Log:
Fix a bug in authz logic merging which caused
section->op == AUTHZ_LOGIC_AND
auth_result == AUTHZ_DENIED_NO_USER
child_result == AUTHZ_GRANTED
to return AUTHZ_GRANTED instead of AUTHZ_DENIED_NO_USER.
While there, refactor the if blocks to make them a bit more readable.
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/aaa/mod_authz_core.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1056713&r1=1056712&r2=1056713&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Sat Jan 8 14:29:12 2011
@@ -2,6 +2,9 @@
Changes with Apache 2.3.11
+ *) mod_authz_core: Fix bug in merging logic if user-based and non-user-based
+ authorization directives were mixed. [Stefan Fritsch]
+
*) mod_authn_socache: change directive name from AuthnCacheProvider
to AuthnCacheProvideFor. The term "provider" is overloaded in
this module, and we should avoid confusion between the provider
Modified: httpd/httpd/trunk/modules/aaa/mod_authz_core.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_authz_core.c?rev=1056713&r1=1056712&r2=1056713&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/aaa/mod_authz_core.c (original)
+++ httpd/httpd/trunk/modules/aaa/mod_authz_core.c Sat Jan 8 14:29:12 2011
@@ -730,21 +730,28 @@ static authz_status apply_authz_sections
* AUTHZ_DENIED_NO_USER if providing a user may change the
* result, AUTHZ_DENIED otherwise.
*/
- if (!(section->op == AUTHZ_LOGIC_AND
- && auth_result == AUTHZ_DENIED
- && child_result == AUTHZ_DENIED_NO_USER)
- && !(section->op == AUTHZ_LOGIC_OR
- && auth_result == AUTHZ_DENIED_NO_USER
- && child_result == AUTHZ_DENIED) )
- {
- auth_result = child_result;
+ if (section->op == AUTHZ_LOGIC_AND) {
+ if (child_result == AUTHZ_DENIED) {
+ auth_result = child_result;
+ break;
+ }
+ if ((child_result == AUTHZ_DENIED_NO_USER
+ && auth_result != AUTHZ_DENIED)
+ || (auth_result == AUTHZ_NEUTRAL)) {
+ auth_result = child_result;
+ }
}
-
- if ((section->op == AUTHZ_LOGIC_AND
- && child_result == AUTHZ_DENIED)
- || (section->op == AUTHZ_LOGIC_OR
- && child_result == AUTHZ_GRANTED)) {
- break;
+ else {
+ /* AUTHZ_LOGIC_OR */
+ if (child_result == AUTHZ_GRANTED) {
+ auth_result = child_result;
+ break;
+ }
+ if ((child_result == AUTHZ_DENIED_NO_USER
+ && auth_result == AUTHZ_DENIED)
+ || (auth_result == AUTHZ_NEUTRAL)) {
+ auth_result = child_result;
+ }
}
}
Re: handling of security issues in alphas?
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Sunday 09 January 2011, William A. Rowe Jr. wrote:
> On 1/8/2011 8:50 AM, Stefan Fritsch wrote:
> > On Saturday 08 January 2011, sf@apache.org wrote:
> >> Author: sf
> >> Date: Sat Jan 8 14:29:12 2011
> >> New Revision: 1056713
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1056713&view=rev
> >> Log:
> >> Fix a bug in authz logic merging which caused
> >>
> >> section->op == AUTHZ_LOGIC_AND
> >> auth_result == AUTHZ_DENIED_NO_USER
> >> child_result == AUTHZ_GRANTED
> >>
> >> to return AUTHZ_GRANTED instead of AUTHZ_DENIED_NO_USER.
> >>
> >> While there, refactor the if blocks to make them a bit more
> >> readable.
> >>
> >> Modified:
> >> httpd/httpd/trunk/CHANGES
> >> httpd/httpd/trunk/modules/aaa/mod_authz_core.c
> >
> > This was broken since r964156 / 2.3.8.
> >
> > Is there some agreed upon policy how to handle security issues
> > that only affect alphas and/or betas? Do we need a CVE id?
> >
> > IMO: No for alphas, but maybe yes for betas?
>
> Well, if it was a concern, it wouldn't be committed until we were
> prepared to release the replacement, which is why such patches are
> first discussed at the security@httpd.a.o by exclusively project
> members who are willing to review security-related changes.
>
> Since the ASF and others run alphas in production, these certainly
> require appropriate handling, alpha beta or GA.
OK.
> The only question is whether this represents an undisclosed and
> unanticipated security flaw, or is a blatent and obvious bug in
> behavior. If it's the former, it gets a CVE. If it's the later,
> it just gets fixed. Because the code is in the auth section does
> not automatically elevate it to a 'security vulnerability'. What
> would elevate it is unexpected or inconsistent results that would
> not be plainly observed by every administrator setting up this
> configuration.
I agree that this bug is not quite a security issue. Even though it
causes somewhat unexpected behaviour, because it depends on the order
of the require statements in the configuration file.
Re: handling of security issues in alphas?
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 1/8/2011 8:50 AM, Stefan Fritsch wrote:
> On Saturday 08 January 2011, sf@apache.org wrote:
>> Author: sf
>> Date: Sat Jan 8 14:29:12 2011
>> New Revision: 1056713
>>
>> URL: http://svn.apache.org/viewvc?rev=1056713&view=rev
>> Log:
>> Fix a bug in authz logic merging which caused
>> section->op == AUTHZ_LOGIC_AND
>> auth_result == AUTHZ_DENIED_NO_USER
>> child_result == AUTHZ_GRANTED
>> to return AUTHZ_GRANTED instead of AUTHZ_DENIED_NO_USER.
>>
>> While there, refactor the if blocks to make them a bit more
>> readable.
>>
>> Modified:
>> httpd/httpd/trunk/CHANGES
>> httpd/httpd/trunk/modules/aaa/mod_authz_core.c
>
> This was broken since r964156 / 2.3.8.
>
> Is there some agreed upon policy how to handle security issues that
> only affect alphas and/or betas? Do we need a CVE id?
>
> IMO: No for alphas, but maybe yes for betas?
Well, if it was a concern, it wouldn't be committed until we were
prepared to release the replacement, which is why such patches are
first discussed at the security@httpd.a.o by exclusively project
members who are willing to review security-related changes.
Since the ASF and others run alphas in production, these certainly
require appropriate handling, alpha beta or GA.
The only question is whether this represents an undisclosed and
unanticipated security flaw, or is a blatent and obvious bug in
behavior. If it's the former, it gets a CVE. If it's the later,
it just gets fixed. Because the code is in the auth section does
not automatically elevate it to a 'security vulnerability'. What
would elevate it is unexpected or inconsistent results that would
not be plainly observed by every administrator setting up this
configuration.
handling of security issues in alphas?
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Saturday 08 January 2011, sf@apache.org wrote:
> Author: sf
> Date: Sat Jan 8 14:29:12 2011
> New Revision: 1056713
>
> URL: http://svn.apache.org/viewvc?rev=1056713&view=rev
> Log:
> Fix a bug in authz logic merging which caused
> section->op == AUTHZ_LOGIC_AND
> auth_result == AUTHZ_DENIED_NO_USER
> child_result == AUTHZ_GRANTED
> to return AUTHZ_GRANTED instead of AUTHZ_DENIED_NO_USER.
>
> While there, refactor the if blocks to make them a bit more
> readable.
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/aaa/mod_authz_core.c
This was broken since r964156 / 2.3.8.
Is there some agreed upon policy how to handle security issues that
only affect alphas and/or betas? Do we need a CVE id?
IMO: No for alphas, but maybe yes for betas?