You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Chris Darroch <ch...@pearsoncmg.com> on 2008/11/03 20:12:34 UTC

Re: AuthzMergeRules blocks everything in default configuration

Dan Poirier wrote:

> I'd find it much easier to understand if we had fewer directives, and 
> just built up the more complicated ideas by writing boolean expressions, 
> which most of us already know how to cope with.

   Perhaps, and the underlying code should support a range of alternative
configuration schemes; if someone wants to add an expression parser,
that should be feasible.


   My own perspective was that I wanted to satisfy number of goals,
in descending order of priority:

   First, and most important, I wanted to ensure default 2.2-style
authz.  Imagine administrating a large mass virtual hosting service
whose customers have uploaded thousands of .htaccess files, and trying
to upgrade to 2.4.  Clearly, those .htaccess files need to work exactly
as before.  Even if we supplied a batch conversion script that could
find and auto-upgrade them, customers would later upload their own private
copies of their old files, thus inadvertently breaking their sites.

   So, that meant an OR-like context for Require directives, and
no merging of authz configurations by default.  This whole thread
started because I was trying mod_authz_dbd and noticed it didn't
work as expected because AuthzMergeRules was On (i.e., "OR") by default.
(In my previous message I described switching to an AND-like default
context for Require directives, but realized before committing that
that would break with this prime directive of backwards-compatibility.)

   So, if people could please test with 2.2-style authz configurations
and make sure everything works as expected, that would be superb.


   Second, I wanted to simplify things a little.  The revised
mod_auth.h and mod_authz_core.c have shrunk a little, and I felt I
could remove the default authn/z modules.  (Having both core and
default modules for authn and authz, any of which could be compiled
out, seemed a likely source of trouble.)


   Third, while looking into how mod_authz_core worked, I found
some ways to configure it that would cause unexpected results,
and while trying to fix those came to the conclusion I needed to
start over with a tree-based implementation.

   Doing that and working through the implications of a non-Boolean
tri-state logic (where negating a false value results in a neutral,
not true, value) I found that adding negated authz container directives
was something that just fell out naturally.


   Finally there was a certain amount of bike-shed re-painting in
the form of renaming configuration directives.  I settled on
Match, <MatchAll>, etc. based on Eric Covener's comments.

   If people dislike those names, please jump in and change them.
Or if most folks think we'd be better off without authz containers
entirely, please vote for the following patch, which simply turns all
that stuff off, leaving (I hope) a fairly clean core authz implementation
that supports default 2.2-style behaviour and is extensible down the road,
should that be desired.

Chris.


Index: mod_authz_core.c
===================================================================
--- mod_authz_core.c    (revision 710120)
+++ mod_authz_core.c    (working copy)
@@ -405,6 +405,7 @@
     return NULL;
 }
 
+#ifdef AUTHZ_CORE_CONTAINERS
 static const char *add_authz_section(cmd_parms *cmd, void *mconfig,
                                      const char *args)
 {
@@ -534,6 +535,7 @@
 
     return NULL;
 }
+#endif /* AUTHZ_CORE_CONTAINERS */
 
 static int authz_core_check_section(apr_pool_t *p, server_rec *s,
                                     authz_section_conf *section, int is_conf)
@@ -634,6 +636,7 @@
                      "specifies legacy authorization directives "
                      "of which one must pass "
                      "for a request to suceeed"),
+#ifdef AUTHZ_CORE_CONTAINERS
     AP_INIT_RAW_ARGS("Match", add_authz_provider, NULL, OR_AUTHCFG,
                      "specifies authorization directives that must pass "
                      "(or not) for a request to suceeed"),
@@ -656,6 +659,7 @@
                   "controls how a <Directory>, <Location>, or similar "
                   "directive's authorization directives are combined with "
                   "those of its predecessor"),
+#endif /* AUTHZ_CORE_CONTAINERS */
     {NULL}
 };
 

Re: AuthzMergeRules blocks everything in default configuration

Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
 

> -----Ursprüngliche Nachricht-----
> Von: Chris Darroch 
> Gesendet: Dienstag, 4. November 2008 02:14
> An: dev@httpd.apache.org
> Betreff: Re: AuthzMergeRules blocks everything in default 
> configuration
> 
> Ruediger Pluem wrote:
> 
> > I was hoping that your patches would fix this, but sadly 
> they did not.
> 
>    Ironically, the problem appears to have little to do with authz,
> but rather authn.  The test httpd logs show it's failing to find
> an htpasswd-type file in which to check the user's login and password.
> That's because there's no AuthBasicProvider in the test config, and
> the code -- since way back, I think -- defaults to the "file" authn
> provider.
> 
>    Looking back in SVN it seems like that should have been the
> behaviour for quite a number of years, but I confess I didn't dig
> too deeply.  Nor did I try out the test suite with 2.2.x to see
> if it succeeds as-is, and if so, why.  If it does, my hunch would
> be that it succeeds because the suite doesn't actually use the normal
> authn/z providers, but rather supplies its own module called 
> mod_authany.c.
> 
>    That module contains two different alternative set of functions
> based on an #if AP_MODULE_MAGIC_AT_LEAST(20060110, 0) which 
> corresponds
> to, I think, an attempt to just get it to compile after the authn/z
> refactoring in trunk.  The log comment for r375595 is:
> 
>     - attempt to adapt for trunk aaa changes; this doesn't work
>     but it does at least compile - not sure whether the problem
>     is in this code or the aaa code.
> 
>    At any rate, my guess would be that it works (if it does) with
> 2.2.x because the module supplies its own raw check_user_id 
> (i.e., authn)
> and auth_checker (i.e., authz) hook functions which run as 
> APR_HOOK_FIRST
> and probably bypass all the stuff in the usual APR_HOOK_MIDDLE hook
> functions of mod_auth_basic and mod_authz_file.  So the fact
> that it doesn't specify any AuthBasicProvider never comes up because
> it bypasses mod_auth_basic entirely.  Just a guess.
> 
>    At any rate, the patch below makes it run with trunk, and
> properly too, in the sense that it uses mod_authn_core and
> mod_authz_core to do the heavy lifting and just supplies a tiny
> pair of authn/z providers.  But, this isn't really a perfect
> solution because it's not really correct to put the authn provider
> into the AP_MODULE_MAGIC_AT_LEAST(20060110, 0) block, since it
> doesn't (I think) have anything in particular to do with that
> change.  Sorry not to spend more time on this.

I committed a slightly modified version of your patch as r711245 and
now the test passes for 2.0.x / 2.2.x / trunk.
Thanks for working it out.

Regards

Rüdiger


Re: AuthzMergeRules blocks everything in default configuration

Posted by Chris Darroch <ch...@pearsoncmg.com>.
Ruediger Pluem wrote:

> I was hoping that your patches would fix this, but sadly they did not.

   Ironically, the problem appears to have little to do with authz,
but rather authn.  The test httpd logs show it's failing to find
an htpasswd-type file in which to check the user's login and password.
That's because there's no AuthBasicProvider in the test config, and
the code -- since way back, I think -- defaults to the "file" authn
provider.

   Looking back in SVN it seems like that should have been the
behaviour for quite a number of years, but I confess I didn't dig
too deeply.  Nor did I try out the test suite with 2.2.x to see
if it succeeds as-is, and if so, why.  If it does, my hunch would
be that it succeeds because the suite doesn't actually use the normal
authn/z providers, but rather supplies its own module called mod_authany.c.

   That module contains two different alternative set of functions
based on an #if AP_MODULE_MAGIC_AT_LEAST(20060110, 0) which corresponds
to, I think, an attempt to just get it to compile after the authn/z
refactoring in trunk.  The log comment for r375595 is:

    - attempt to adapt for trunk aaa changes; this doesn't work
    but it does at least compile - not sure whether the problem
    is in this code or the aaa code.

   At any rate, my guess would be that it works (if it does) with
2.2.x because the module supplies its own raw check_user_id (i.e., authn)
and auth_checker (i.e., authz) hook functions which run as APR_HOOK_FIRST
and probably bypass all the stuff in the usual APR_HOOK_MIDDLE hook
functions of mod_auth_basic and mod_authz_file.  So the fact
that it doesn't specify any AuthBasicProvider never comes up because
it bypasses mod_auth_basic entirely.  Just a guess.

   At any rate, the patch below makes it run with trunk, and
properly too, in the sense that it uses mod_authn_core and
mod_authz_core to do the heavy lifting and just supplies a tiny
pair of authn/z providers.  But, this isn't really a perfect
solution because it's not really correct to put the authn provider
into the AP_MODULE_MAGIC_AT_LEAST(20060110, 0) block, since it
doesn't (I think) have anything in particular to do with that
change.  Sorry not to spend more time on this.

Chris.


Index: mod_authany.c
===================================================================
--- mod_authany.c       (revision 710145)
+++ mod_authany.c       (working copy)
@@ -5,6 +5,7 @@
    require user any-user
    AuthType Basic
    AuthName authany
+   AuthBasicProvider any
 </Location>
 
 #endif
@@ -19,6 +20,18 @@
 #include "ap_provider.h"
 #include "mod_auth.h"
 
+static authn_status authn_check_password(request_rec *r, const char *user,
+                                         const char *password)
+{
+    return strtrue(r->user) && strcmp(r->user, "guest") == 0
+        ? AUTH_GRANTED : AUTH_DENIED;
+}
+
+static const authn_provider authn_any_provider =
+{
+    &authn_check_password
+};
+
 static authz_status any_check_authorization(request_rec *r,
                                             const char *requirement)
 {
@@ -28,11 +41,13 @@
 
 static const authz_provider authz_any_provider =
 {
-    &any_check_authorization,
+    &any_check_authorization
 };
 
 static void extra_hooks(apr_pool_t *p)
 {
+    ap_register_provider(p, AUTHN_PROVIDER_GROUP,
+                         "any", "0", &authn_any_provider);
     ap_register_provider(p, AUTHZ_PROVIDER_GROUP,
                          "user", "0", &authz_any_provider);
 }

Re: AuthzMergeRules blocks everything in default configuration

Posted by Ruediger Pluem <rp...@apache.org>.

On 11/03/2008 08:12 PM, Chris Darroch wrote:
> Dan Poirier wrote:
> 
>> I'd find it much easier to understand if we had fewer directives, and
>> just built up the more complicated ideas by writing boolean
>> expressions, which most of us already know how to cope with.
> 
>   Perhaps, and the underlying code should support a range of alternative
> configuration schemes; if someone wants to add an expression parser,
> that should be feasible.
> 
> 
>   My own perspective was that I wanted to satisfy number of goals,
> in descending order of priority:
> 
>   First, and most important, I wanted to ensure default 2.2-style
> authz.  Imagine administrating a large mass virtual hosting service
> whose customers have uploaded thousands of .htaccess files, and trying
> to upgrade to 2.4.  Clearly, those .htaccess files need to work exactly
> as before.  Even if we supplied a batch conversion script that could
> find and auto-upgrade them, customers would later upload their own private
> copies of their old files, thus inadvertently breaking their sites.
> 
>   So, that meant an OR-like context for Require directives, and
> no merging of authz configurations by default.  This whole thread
> started because I was trying mod_authz_dbd and noticed it didn't
> work as expected because AuthzMergeRules was On (i.e., "OR") by default.
> (In my previous message I described switching to an AND-like default
> context for Require directives, but realized before committing that
> that would break with this prime directive of backwards-compatibility.)
> 
>   So, if people could please test with 2.2-style authz configurations
> and make sure everything works as expected, that would be superb.

One of the authz tests breaks on trunk since a long time (I think it
started to break after Brad refactored the code):

t/http11/basicauth....1..3
# Running under perl version 5.008008 for linux
# Current time local: Mon Nov  3 20:46:36 2008
# Current time GMT:   Mon Nov  3 19:46:36 2008
# Using Test.pm version 1.25
# Using Apache/Test.pm version 1.31
ok 1
not ok 2
# Failed test 2 in t/http11/basicauth.t at line 24
ok 3
FAILED test 2
        Failed 1/3 tests, 66.67% okay
Failed Test          Stat Wstat Total Fail  Failed  List of Failed
-------------------------------------------------------------------------------
t/http11/basicauth.t                3    1  33.33%  2
Failed 1/1 test scripts, 0.00% okay. 1/3 subtests failed, 66.67% okay.


I was hoping that your patches would fix this, but sadly they did not.
>From what I reviewed, the authz code should now react similar to the
2.2.x authz code, but apprently it does not.
As you have crawled that deeply in the authz code you seem to be
the natural person to have a look at this failing test :-).
I think this would be greatly appreciated.
Otherwise I think it is cool work that can be used for very
complex configuration needs.

Regards

Rüdiger