You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Dan Poirier <po...@pobox.com> on 2008/10/06 15:34:50 UTC
Re: AuthzMergeRules blocks everything in default configuration
On Tue, 23 Sep 2008 11:05:45 -0700, "Chris Darroch"
<ch...@pearsoncmg.com> said:
> Dan Poirier wrote:
>...
> > I read through some previous discussion of the authz inheritance
> > behavior, but it doesn't seem to have considered the effect of having
> > "Require all denied" at the top level, which is overriding everything
> > else by default even when other sections specify other permissions.
[omitted some history]
> Let's say you've got two authz configs, A and B, and A is considered
> before B (maybe A is in <Directory /> and B is in <Directory /foo> for
> a given request (e.g., for /foo/bar).
...
> Previously [in trunk] the default merge rule was "OR", i.e., A ||
> B. Thus in the case where A allowed access and B didn't, you'd be
> let in despite B's local authz denying you access. That would, I
> thought, have created a lot of unexpected security holes for people
> who upgraded from 2.2, where B's local authz was the only authority.
Agreed.
> Brad Nicholes changed that a few months ago so that the default
> rule should be "AND", i.e., A && B. Thus in the case above, you'd
> be denied access because of B's config.
>
> This closes off the worst security problems, I think. As you note,
> though, it can produce situations where A's config denies you access
> and so B's is just ignored, even if B wants to let you have access.
> You can set AuthzMergeRules Off for B, but also as you note you'd have to
> set it everywhere. And since it's value isn't "inherited", if
> A, B, and C all apply to the request, then turning it Off for B
> means you'll still get B && C applied, not just C.
>
> We can obviously change the example configs so as to make
> the <Directory /> setting have "Require all granted". However, I
> think we'll find that fair numbers of people get themselves into
> this situation -- by naively upgrading using 2.2 configs, assuming
> things work the 2.2 way (B isn't affected by A), assuming AuthzMergeRules
> values are "inherited", etc.
I don't think we really want to change <Directory /> to "Require
all granted", do we? Wouldn't that open up the entire system by
default?
> Hence my feeling that the default should be Off (to be closer
> to 2.2 behaviour), and when explicitly On, the rule that's applied
> should be AND -- as is now true, thanks to Brad!
>
> One further thought of mine from ages ago was that On (which
> currently means AND) could be replaced by two non-default options,
> And and Or (or SastifyAll and SastifyOne, respectively). Then
> the administrator would have full control over the inter-block
> merge rule, similar to the control they have in trunk over the
> intra-block rules via <SastifyAll> and <SastifyOne>.
I like the idea of replacing ON with AND and OR. It would not
only provide more control, but make it explicit what kind of merging
was going to happen.
I have mixed thoughts about changing the default to OFF.
Cons: That would mean every container directive would have to specify
some sort of access control (or at least AuthzMergeRules AND) or it'd
be wide open, right?
Pros: On the other hand, that's the behavior in 2.2, and changing it
would surely cause confusion for people trying to migrate to the new
behavior. As you point out, it's also confusing to put configuration
in B that ought to grant access, and find that access is still denied
due to a setting in A.
One other idea occurs to me. Would it seem more intuitive if a
context that had no authz directives inherited the settings from its
predecessor, but as soon you added authz directives, it behaved as if
you were starting from scratch? (As if you'd included AuthzMergeRules
Off. You could always add AuthzMergeRules AND if you wanted that
behavior.) Or would it just be more confusing? I guess you'd still
have to go through the default configuration files adding "Require
all granted" or "AuthzMergeRules Off" everywhere in order to give
access to anything, since otherwise they'd still inherit from the
top-level's not granting any access.
> Thoughts? I apologize profusely for having so little time to
> devote to actual useful coding at the moment.
I'd like to hear what others think. Can the current behavior be
improved, and if so, how?
Dan
Re: AuthzMergeRules blocks everything in default configuration
Posted by Ruediger Pluem <rp...@apache.org>.
On 10/06/2008 03:59 PM, Eric Covener wrote:
> On Mon, Oct 6, 2008 at 9:34 AM, Dan Poirier <po...@pobox.com> wrote:
>> One other idea occurs to me. Would it seem more intuitive if a
>> context that had no authz directives inherited the settings from its
>> predecessor, but as soon you added authz directives, it behaved as if
>> you were starting from scratch?
>
> FWIW: Isn't that the current non-trunk behavior?
I think so. So this looks like a good idea.
Regards
Rüdiger
Re: AuthzMergeRules blocks everything in default configuration
Posted by Eric Covener <co...@gmail.com>.
On Mon, Oct 6, 2008 at 9:34 AM, Dan Poirier <po...@pobox.com> wrote:
>
> One other idea occurs to me. Would it seem more intuitive if a
> context that had no authz directives inherited the settings from its
> predecessor, but as soon you added authz directives, it behaved as if
> you were starting from scratch?
FWIW: Isn't that the current non-trunk behavior?
--
Eric Covener
covener@gmail.com
Re: AuthzMergeRules blocks everything in default configuration
Posted by Brad Nicholes <bn...@novell.com>.
>>> On 12/4/2008 at 1:30 PM, in message <49...@pearsoncmg.com>, Chris
Darroch <ch...@pearsoncmg.com> wrote:
> Hi --
>
> Eric Covener wrote:
>
>> I had meant iif containers are used, I'd like their name to
>> communicate the "require" or "reject" part while the authz providers
>> would be "match"-like (because the Require on the inside is confusing
>> when surrounted by all the variations)
>
> Yes, I thought that was a good point; my further thought was that
> the container names can't imply require/reject either though, because
> they can be nested and so their meaning can be inverted if they're
> contained in a negated context.
>
>
> Roy T. Fielding wrote:
>
>> But we are already using *Match all over the place to indicate the
>> use of regex matching. :(
>
> These are good points; I hadn't thought of the "overlap" with
> LocationMatch and friends.
>
> A lot of the other obvious access-control-related words and terms
> are also already in use, especially for older authorization directives
> (e.g., Allow, Deny, Order, Limit, Require, Satisfy, etc.) In order
> to avoid confusion, we should probably stay away from all of these too.
>
> Perhaps something like Check or Test would suffice, maybe prefixed
> with Authz? Hopefully someone else has a good idea, or at least
> stronger opinions. :-)
>
I think prefixing it with Authz probably makes more sense.
Brad
Re: AuthzMergeRules blocks everything in default configuration
Posted by Chris Darroch <ch...@pearsoncmg.com>.
Hi --
Eric Covener wrote:
> I had meant iif containers are used, I'd like their name to
> communicate the "require" or "reject" part while the authz providers
> would be "match"-like (because the Require on the inside is confusing
> when surrounted by all the variations)
Yes, I thought that was a good point; my further thought was that
the container names can't imply require/reject either though, because
they can be nested and so their meaning can be inverted if they're
contained in a negated context.
Roy T. Fielding wrote:
> But we are already using *Match all over the place to indicate the
> use of regex matching. :(
These are good points; I hadn't thought of the "overlap" with
LocationMatch and friends.
A lot of the other obvious access-control-related words and terms
are also already in use, especially for older authorization directives
(e.g., Allow, Deny, Order, Limit, Require, Satisfy, etc.) In order
to avoid confusion, we should probably stay away from all of these too.
Perhaps something like Check or Test would suffice, maybe prefixed
with Authz? Hopefully someone else has a good idea, or at least
stronger opinions. :-)
Chris.
--
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
Re: AuthzMergeRules blocks everything in default configuration
Posted by Eric Covener <co...@gmail.com>.
On Mon, Dec 1, 2008 at 6:16 PM, Chris Darroch <ch...@pearsoncmg.com> wrote:
>>> 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.
I had meant iif containers are used, I'd like their name to
communicate the "require" or "reject" part while the authz providers
would be "match"-like (because the Require on the inside is confusing
when surrounted by all the variations)
I do agree that all the way down the authz/aa nature of the
directives/containers needs to remain clear.
I haven't been able to get past the negaqted containers to revisit how
this all works in trunk.
--
Eric Covener
covener@gmail.com
Re: Authz directives
Posted by Chris Darroch <ch...@pearsoncmg.com>.
Roy T. Fielding wrote:
> I don't see a problem with RequireNone inverting the logic, and
> I think it would actually be useful for blocking a set of bad
> clients. Is it difficult to include that without MatchNotAny?
Not at all difficult; trivial, in fact. The only reason I took
it out as well was the name "inversion" issue. I'll put it back
in and commit tomorrow. Might take a few days to deal with the doc
adjustments, but otherwise a simple job. Thanks for the comments!
Chris.
--
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
Re: Authz directives
Posted by "Roy T. Fielding" <fi...@gbiv.com>.
On Dec 9, 2008, at 10:45 AM, Chris Darroch wrote:
>> This is all fairly simple, I think, especially if
>> MatchNotAny/RequireNone is removed as well so that Require retains
>> its
>> apparent meaning everywhere. See if the patch below meets your
>> expectations; if so, I'll commit it and update the docs.
>
> Sorry, here's a slightly updated one without any whitespace issues;
> it should apply cleanly against the latest SVN trunk as of a few
> minutes ago.
I don't see a problem with RequireNone inverting the logic, and
I think it would actually be useful for blocking a set of bad
clients. Is it difficult to include that without MatchNotAny?
In any case, I'd prefer that we clean this up before the next
attempt at an alpha. I wanted to do it this week but have run out
of time (I don't want to commit and fly). Can you commit this or
something like it?
....Roy
Re: Authz directives
Posted by Chris Darroch <ch...@pearsoncmg.com>.
Hi --
> This is all fairly simple, I think, especially if
> MatchNotAny/RequireNone is removed as well so that Require retains its
> apparent meaning everywhere. See if the patch below meets your
> expectations; if so, I'll commit it and update the docs.
Sorry, here's a slightly updated one without any whitespace issues;
it should apply cleanly against the latest SVN trunk as of a few
minutes ago.
Chris.
=========================================================
--- modules/aaa/mod_authz_core.c (revision 724813)
+++ modules/aaa/mod_authz_core.c (working copy)
@@ -52,17 +52,19 @@
#define FORMAT_AUTHZ_COMMAND(p,section) \
((section)->provider \
- ? apr_pstrcat((p), \
+ ? apr_pstrcat((p), "Require ", \
((section)->negate ? \
- "Match not " : "Match "), \
+ "not " : ""), \
(section)->provider_name, " ", \
(section)->provider_args, NULL) \
- : apr_pstrcat((p), "<Match", \
+ : apr_pstrcat((p), "<Require", \
((section)->negate ? "Not" : ""), \
(((section)->op == AUTHZ_LOGIC_AND) \
? "All" : "Any"), \
">", NULL))
+#undef AUTHZ_NEGATIVE_CONFIGS
+
typedef struct provider_alias_rec {
char *provider_name;
char *provider_alias;
@@ -95,7 +97,6 @@
struct authz_core_dir_conf {
authz_section_conf *section;
authz_logic_op op;
- int old_require;
authz_core_dir_conf *next;
};
@@ -314,12 +315,11 @@
return errmsg;
}
-static authz_section_conf* create_default_section(apr_pool_t *p,
- int old_require)
+static authz_section_conf* create_default_section(apr_pool_t *p)
{
authz_section_conf *section = apr_pcalloc(p, sizeof(*section));
- section->op = old_require ? AUTHZ_LOGIC_OR : AUTHZ_LOGIC_AND;
+ section->op = AUTHZ_LOGIC_OR;
return section;
}
@@ -331,21 +331,9 @@
authz_section_conf *section = apr_pcalloc(cmd->pool, sizeof(*section));
authz_section_conf *child;
- if (!strcasecmp(cmd->cmd->name, "Require")) {
- if (conf->section && !conf->old_require) {
- return "Require directive not allowed with "
- "Match and related directives";
- }
-
- conf->old_require = 1;
- }
- else if (conf->old_require) {
- return "Match directive not allowed with Require directives";
- }
-
section->provider_name = ap_getword_conf(cmd->pool, &args);
- if (!conf->old_require && !strcasecmp(section->provider_name, "not")) {
+ if (!strcasecmp(section->provider_name, "not")) {
section->provider_name = ap_getword_conf(cmd->pool, &args);
section->negate = 1;
}
@@ -377,7 +365,7 @@
section->limited = cmd->limited;
if (!conf->section) {
- conf->section = create_default_section(cmd->pool, conf->old_require);
+ conf->section = create_default_section(cmd->pool);
}
if (section->negate && conf->section->op == AUTHZ_LOGIC_OR) {
@@ -416,12 +404,6 @@
apr_int64_t old_limited = cmd->limited;
const char *errmsg;
- if (conf->old_require) {
- return apr_pstrcat(cmd->pool, cmd->cmd->name,
- "> directive not allowed with "
- "Require directives", NULL);
- }
-
if (endp == NULL) {
return apr_pstrcat(cmd->pool, cmd->cmd->name,
"> directive missing closing '>'", NULL);
@@ -437,13 +419,14 @@
section = apr_pcalloc(cmd->pool, sizeof(*section));
- if (!strcasecmp(cmd->cmd->name, "<MatchAll")) {
+ if (!strcasecmp(cmd->cmd->name, "<RequireAll")) {
section->op = AUTHZ_LOGIC_AND;
}
- else if (!strcasecmp(cmd->cmd->name, "<MatchAny")) {
+ else if (!strcasecmp(cmd->cmd->name, "<RequireAny")) {
section->op = AUTHZ_LOGIC_OR;
}
- else if (!strcasecmp(cmd->cmd->name, "<MatchNotAll")) {
+#ifdef AUTHZ_NEGATIVE_CONFIGS
+ else if (!strcasecmp(cmd->cmd->name, "<RequireNotAll")) {
section->op = AUTHZ_LOGIC_AND;
section->negate = 1;
}
@@ -451,6 +434,7 @@
section->op = AUTHZ_LOGIC_OR;
section->negate = 1;
}
+#endif
conf->section = section;
@@ -473,7 +457,7 @@
authz_section_conf *child;
if (!old_section) {
- old_section = conf->section = create_default_section(cmd->pool, 0);
+ old_section = conf->section = create_default_section(cmd->pool);
}
if (section->negate && old_section->op == AUTHZ_LOGIC_OR) {
@@ -521,15 +505,15 @@
if (!strcasecmp(arg, "Off")) {
conf->op = AUTHZ_LOGIC_OFF;
}
- else if (!strcasecmp(arg, "MatchAll")) {
+ else if (!strcasecmp(arg, "And")) {
conf->op = AUTHZ_LOGIC_AND;
}
- else if (!strcasecmp(arg, "MatchAny")) {
+ else if (!strcasecmp(arg, "Or")) {
conf->op = AUTHZ_LOGIC_OR;
}
else {
return apr_pstrcat(cmd->pool, cmd->cmd->name, " must be one of: "
- "Off | MatchAll | MatchAny", NULL);
+ "Off | And | Or", NULL);
}
return NULL;
@@ -612,7 +596,7 @@
authz_core_dir_conf *conf = authz_core_first_dir_conf;
while (conf) {
- if (conf->section && !conf->old_require) {
+ if (conf->section) {
if (authz_core_check_section(p, s, conf->section, 1) != OK) {
return !OK;
}
@@ -631,29 +615,27 @@
"container for grouping an authorization provider's "
"directives under a provider alias"),
AP_INIT_RAW_ARGS("Require", add_authz_provider, NULL, OR_AUTHCFG,
- "specifies legacy authorization directives "
- "of which one must pass "
- "for a request to suceeed"),
- AP_INIT_RAW_ARGS("Match", add_authz_provider, NULL, OR_AUTHCFG,
- "specifies authorization directives that must pass "
- "(or not) for a request to suceeed"),
- AP_INIT_RAW_ARGS("<MatchAll", add_authz_section, NULL, OR_AUTHCFG,
+ "specifies authorization directives "
+ "which one must pass (or not) for a request to suceeed"),
+ AP_INIT_RAW_ARGS("<RequireAll", add_authz_section, NULL, OR_AUTHCFG,
"container for grouping authorization directives "
"of which none must fail and at least one must pass "
"for a request to succeed"),
- AP_INIT_RAW_ARGS("<MatchAny", add_authz_section, NULL, OR_AUTHCFG,
+ AP_INIT_RAW_ARGS("<RequireAny", add_authz_section, NULL, OR_AUTHCFG,
"container for grouping authorization directives "
"of which one must pass "
"for a request to succeed"),
- AP_INIT_RAW_ARGS("<MatchNotAll", add_authz_section, NULL, OR_AUTHCFG,
+#ifdef AUTHZ_NEGATIVE_CONFIGS
+ AP_INIT_RAW_ARGS("<RequireNotAll", add_authz_section, NULL, OR_AUTHCFG,
"container for grouping authorization directives "
"of which some must fail or none must pass "
"for a request to succeed"),
- AP_INIT_RAW_ARGS("<MatchNotAny", add_authz_section, NULL, OR_AUTHCFG,
+ AP_INIT_RAW_ARGS("<RequireNotAny", add_authz_section, NULL, OR_AUTHCFG,
"container for grouping authorization directives "
"of which none must pass "
"for a request to succeed"),
- AP_INIT_TAKE1("MergeAuthz", authz_merge_sections, NULL, OR_AUTHCFG,
+#endif
+ AP_INIT_TAKE1("AuthMerging", authz_merge_sections, NULL, OR_AUTHCFG,
"controls how a <Directory>, <Location>, or similar "
"directive's authorization directives are combined with "
"those of its predecessor"),
Re: Authz directives
Posted by Chris Darroch <ch...@pearsoncmg.com>.
Roy T. Fielding wrote:
> I totally understand the desire to make the implementation more
> modular and to make a more sensible Satisfy logic, but I don't
> understand the need for Match (as opposed to just extending Require)
> and the odd changes in defaults (multiple Require defaults to
> MatchAny semantics, but multiple Match defaults to MatchAll).
Match came up only because negated containers cause the meaning
of a plain old Require to be inverted and to have the opposite meaning
from what it appears. If we drop negated containers that problem
goes away. The negated containers, as I mentioned previously, were
just something which fell out easily from the refactoring; they're
hardly worth keeping if they just cause trouble.
> 1) make the new directives self-documenting
>
> remove MatchNotAll (nobody needs this)
>
> s/MatchAny/RequireAny/ig;
> s/MatchAll/RequireAll/ig;
> s/MatchNotAny/RequireNone/ig;
>
> s/(MergeAuthz|AuthzMerge)/AuthMerging/ig; (off | and | or)
>
> 2) move new Match functionality to Require
>
> 3) default for multiple Require* is RequireAny
> - implies that "Require" and "Require not" are only mixed when
> used within a RequireAll or RequireNone container.
This is all fairly simple, I think, especially if
MatchNotAny/RequireNone is removed as well so that Require retains its
apparent meaning everywhere. See if the patch below meets your
expectations; if so, I'll commit it and update the docs. The next
item on the agenda, then, is probably PR #46214, i.e., getting the
various providers to quiet down their log messages.
Chris.
=========================================================
--- modules/aaa/mod_authz_core.c (revision 724744)
+++ modules/aaa/mod_authz_core.c (working copy)
@@ -52,17 +52,19 @@
#define FORMAT_AUTHZ_COMMAND(p,section) \
((section)->provider \
- ? apr_pstrcat((p), \
+ ? apr_pstrcat((p), "Require ", \
((section)->negate ? \
- "Match not " : "Match "), \
+ "not " : ""), \
(section)->provider_name, " ", \
(section)->provider_args, NULL) \
- : apr_pstrcat((p), "<Match", \
+ : apr_pstrcat((p), "<Require", \
((section)->negate ? "Not" : ""), \
(((section)->op == AUTHZ_LOGIC_AND) \
? "All" : "Any"), \
- ">", NULL)) \
+ ">", NULL))
+#undef AUTHZ_NEGATIVE_CONFIGS
+
typedef struct provider_alias_rec {
char *provider_name;
char *provider_alias;
@@ -95,7 +97,6 @@
struct authz_core_dir_conf {
authz_section_conf *section;
authz_logic_op op;
- int old_require;
authz_core_dir_conf *next;
};
@@ -314,12 +315,11 @@
return errmsg;
}
-static authz_section_conf* create_default_section(apr_pool_t *p,
- int old_require)
+static authz_section_conf* create_default_section(apr_pool_t *p)
{
authz_section_conf *section = apr_pcalloc(p, sizeof(*section));
- section->op = old_require ? AUTHZ_LOGIC_OR : AUTHZ_LOGIC_AND;
+ section->op = AUTHZ_LOGIC_OR;
return section;
}
@@ -331,21 +331,9 @@
authz_section_conf *section = apr_pcalloc(cmd->pool, sizeof(*section));
authz_section_conf *child;
- if (!strcasecmp(cmd->cmd->name, "Require")) {
- if (conf->section && !conf->old_require) {
- return "Require directive not allowed with "
- "Match and related directives";
- }
-
- conf->old_require = 1;
- }
- else if (conf->old_require) {
- return "Match directive not allowed with Require directives";
- }
-
section->provider_name = ap_getword_conf(cmd->pool, &args);
- if (!conf->old_require && !strcasecmp(section->provider_name, "not")) {
+ if (!strcasecmp(section->provider_name, "not")) {
section->provider_name = ap_getword_conf(cmd->pool, &args);
section->negate = 1;
}
@@ -377,7 +365,7 @@
section->limited = cmd->limited;
if (!conf->section) {
- conf->section = create_default_section(cmd->pool, conf->old_require);
+ conf->section = create_default_section(cmd->pool);
}
if (section->negate && conf->section->op == AUTHZ_LOGIC_OR) {
@@ -416,12 +404,6 @@
apr_int64_t old_limited = cmd->limited;
const char *errmsg;
- if (conf->old_require) {
- return apr_pstrcat(cmd->pool, cmd->cmd->name,
- "> directive not allowed with "
- "Require directives", NULL);
- }
-
if (endp == NULL) {
return apr_pstrcat(cmd->pool, cmd->cmd->name,
"> directive missing closing '>'", NULL);
@@ -437,13 +419,14 @@
section = apr_pcalloc(cmd->pool, sizeof(*section));
- if (!strcasecmp(cmd->cmd->name, "<MatchAll")) {
+ if (!strcasecmp(cmd->cmd->name, "<RequireAll")) {
section->op = AUTHZ_LOGIC_AND;
}
- else if (!strcasecmp(cmd->cmd->name, "<MatchAny")) {
+ else if (!strcasecmp(cmd->cmd->name, "<RequireAny")) {
section->op = AUTHZ_LOGIC_OR;
}
- else if (!strcasecmp(cmd->cmd->name, "<MatchNotAll")) {
+#ifdef AUTHZ_NEGATIVE_CONFIGS
+ else if (!strcasecmp(cmd->cmd->name, "<RequireNotAll")) {
section->op = AUTHZ_LOGIC_AND;
section->negate = 1;
}
@@ -451,6 +434,7 @@
section->op = AUTHZ_LOGIC_OR;
section->negate = 1;
}
+#endif
conf->section = section;
@@ -473,7 +457,7 @@
authz_section_conf *child;
if (!old_section) {
- old_section = conf->section = create_default_section(cmd->pool, 0);
+ old_section = conf->section = create_default_section(cmd->pool);
}
if (section->negate && old_section->op == AUTHZ_LOGIC_OR) {
@@ -521,15 +505,15 @@
if (!strcasecmp(arg, "Off")) {
conf->op = AUTHZ_LOGIC_OFF;
}
- else if (!strcasecmp(arg, "MatchAll")) {
+ else if (!strcasecmp(arg, "And")) {
conf->op = AUTHZ_LOGIC_AND;
}
- else if (!strcasecmp(arg, "MatchAny")) {
+ else if (!strcasecmp(arg, "Or")) {
conf->op = AUTHZ_LOGIC_OR;
}
else {
return apr_pstrcat(cmd->pool, cmd->cmd->name, " must be one of: "
- "Off | MatchAll | MatchAny", NULL);
+ "Off | And | Or", NULL);
}
return NULL;
@@ -612,7 +596,7 @@
authz_core_dir_conf *conf = authz_core_first_dir_conf;
while (conf) {
- if (conf->section && !conf->old_require) {
+ if (conf->section) {
if (authz_core_check_section(p, s, conf->section, 1) != OK) {
return !OK;
}
@@ -631,28 +615,26 @@
"container for grouping an authorization provider's "
"directives under a provider alias"),
AP_INIT_RAW_ARGS("Require", add_authz_provider, NULL, OR_AUTHCFG,
- "specifies legacy authorization directives "
- "of which one must pass "
- "for a request to suceeed"),
- AP_INIT_RAW_ARGS("Match", add_authz_provider, NULL, OR_AUTHCFG,
- "specifies authorization directives that must pass "
- "(or not) for a request to suceeed"),
- AP_INIT_RAW_ARGS("<MatchAll", add_authz_section, NULL, OR_AUTHCFG,
+ "specifies authorization directives "
+ "which one must pass (or not) for a request to suceeed"),
+ AP_INIT_RAW_ARGS("<RequireAll", add_authz_section, NULL, OR_AUTHCFG,
"container for grouping authorization directives "
"of which none must fail and at least one must pass " "for a request to succeed"),
- AP_INIT_RAW_ARGS("<MatchAny", add_authz_section, NULL, OR_AUTHCFG,
+ AP_INIT_RAW_ARGS("<RequireAny", add_authz_section, NULL, OR_AUTHCFG,
"container for grouping authorization directives "
"of which one must pass "
"for a request to succeed"),
- AP_INIT_RAW_ARGS("<MatchNotAll", add_authz_section, NULL, OR_AUTHCFG,
+#ifdef AUTHZ_NEGATIVE_CONFIGS
+ AP_INIT_RAW_ARGS("<RequireNotAll", add_authz_section, NULL, OR_AUTHCFG,
"container for grouping authorization directives "
"of which some must fail or none must pass "
"for a request to succeed"),
- AP_INIT_RAW_ARGS("<MatchNotAny", add_authz_section, NULL, OR_AUTHCFG,
+ AP_INIT_RAW_ARGS("<RequireNotAny", add_authz_section, NULL, OR_AUTHCFG,
"container for grouping authorization directives "
"of which none must pass "
"for a request to succeed"),
- AP_INIT_TAKE1("MergeAuthz", authz_merge_sections, NULL, OR_AUTHCFG,
+#endif
+ AP_INIT_TAKE1("AuthMerging", authz_merge_sections, NULL, OR_AUTHCFG,
"controls how a <Directory>, <Location>, or similar "
"directive's authorization directives are combined with "
"those of its predecessor"),
Authz directives
Posted by "Roy T. Fielding" <fi...@gbiv.com>.
I am a little frustrated by the changes to authorization since 2.2.
I don't understand why they were needed in the first place, nor why
we need two different but equally incomprehensible ways to configure
the same things.
I totally understand the desire to make the implementation more
modular and to make a more sensible Satisfy logic, but I don't
understand the need for Match (as opposed to just extending Require)
and the odd changes in defaults (multiple Require defaults to
MatchAny semantics, but multiple Match defaults to MatchAll).
The relevant doc links are, for 2.2:
http://httpd.apache.org/docs/2.2/howto/auth.html
http://httpd.apache.org/docs/2.2/mod/mod_authz_host.html
http://httpd.apache.org/docs/2.2/mod/core.html#require
http://httpd.apache.org/docs/2.2/mod/core.html#satisfy
and for trunk:
http://httpd.apache.org/docs/trunk/howto/auth.html
http://httpd.apache.org/docs/trunk/mod/mod_authz_core.html
http://httpd.apache.org/docs/trunk/mod/mod_access_compat.html
I suggest that we do the following:
1) make the new directives self-documenting
remove MatchNotAll (nobody needs this)
s/MatchAny/RequireAny/ig;
s/MatchAll/RequireAll/ig;
s/MatchNotAny/RequireNone/ig;
s/(MergeAuthz|AuthzMerge)/AuthMerging/ig; (off | and | or)
2) move new Match functionality to Require
3) default for multiple Require* is RequireAny
- implies that "Require" and "Require not" are only mixed when
used within a RequireAll or RequireNone container.
In other words, we have one set of authorization directives with
Require being backward-compatible with old configs, and the authz
docs can tell one story of how to build more and more complex
configurations using the same directives.
Does that make sense to folks? I don't have much experience with
the auth directives (never had much use for them at my sites), so
let me know if I've overlooked something.
....Roy
Re: AuthzMergeRules blocks everything in default configuration
Posted by "Roy T. Fielding" <fi...@gbiv.com>.
On Dec 1, 2008, at 3:16 PM, Chris Darroch wrote:
> Roy T. Fielding wrote:
>
>> IIRC, trunk contains (or contained) a security problem with regard to
>> backward compatibility with 2.x configs. I won't consider it
>> releasable
>> until that has been fixed one way or another, and I can't tell
>> from this
>> mail thread whether the actual fix was committed or not.
>
> This posting might answer some of your questions:
>
> http://marc.info/?l=apache-httpd-dev&m=122573959206980&w=2
>
> Yes, the "fix" was committed, and the current intention is that 2.2
> configurations should be useable as-is with trunk, without changes.
> I don't think I can *promise* that's working as intended, but that's
> the idea; if you encounter bugs, please report them!
>
>
>> AuthzMergeRules Off
>> still appears in docs/conf/httpd.conf:162-167 even though it doesn't
>> appear to be a valid config directive either.
>
> This should be gone in httpd.conf.in; there's no httpd.conf in SVN.
> Is there any chance you need to refresh from SVN trunk?
Urgh, no, just a leftover from an old in-tree test. Never mind.
>> The docs seem to indicate this is now MergeAuthz Off and is off by
>> default? Is that true in the code?
>
> Yes, that's the intention; again, please report bugs or backwards-
> compatibility problems.
>
>
>> (why those
>> directive names are Match* instead of AuthMatch* boggles my mind).
>> What is the conclusion to this thread? Why are all the Authz
>> directives given random names? Am I the only one that finds this
>> feature set impossible to follow?
>
> Please see the final set of comments in the posting I linked to
> above, and the patch in that posting as well. Just to reiterate,
> what I wrote there was:
>
>>> 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.
But we are already using *Match all over the place to indicate the
use of regex matching. :(
I'll think about changing the names. Thanks for committing the
important fix.
....Roy
Re: AuthzMergeRules blocks everything in default configuration
Posted by Chris Darroch <ch...@pearsoncmg.com>.
Roy T. Fielding wrote:
> IIRC, trunk contains (or contained) a security problem with regard to
> backward compatibility with 2.x configs. I won't consider it releasable
> until that has been fixed one way or another, and I can't tell from this
> mail thread whether the actual fix was committed or not.
This posting might answer some of your questions:
http://marc.info/?l=apache-httpd-dev&m=122573959206980&w=2
Yes, the "fix" was committed, and the current intention is that 2.2
configurations should be useable as-is with trunk, without changes.
I don't think I can *promise* that's working as intended, but that's
the idea; if you encounter bugs, please report them!
> AuthzMergeRules Off
>
> still appears in docs/conf/httpd.conf:162-167 even though it doesn't
> appear to be a valid config directive either.
This should be gone in httpd.conf.in; there's no httpd.conf in SVN.
Is there any chance you need to refresh from SVN trunk?
> The docs seem to indicate this is now MergeAuthz Off and is off by
> default? Is that true in the code?
Yes, that's the intention; again, please report bugs or backwards-
compatibility problems.
> (why those
> directive names are Match* instead of AuthMatch* boggles my mind).
>
> What is the conclusion to this thread? Why are all the Authz
> directives given random names? Am I the only one that finds this
> feature set impossible to follow?
Please see the final set of comments in the posting I linked to
above, and the patch in that posting as well. Just to reiterate,
what I wrote there was:
>> 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.
--
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
Re: AuthzMergeRules blocks everything in default configuration
Posted by "Roy T. Fielding" <fi...@gbiv.com>.
IIRC, trunk contains (or contained) a security problem with regard to
backward compatibility with 2.x configs. I won't consider it releasable
until that has been fixed one way or another, and I can't tell from this
mail thread whether the actual fix was committed or not. I thought that
Chris committed the patches indicated below, but there is no
SatisfySections
config directive in trunk and
AuthzMergeRules Off
still appears in docs/conf/httpd.conf:162-167 even though it doesn't
appear to be a valid config directive either. Note that the global
(unconfigured) default must be Off in order to stay in 2.x.
The docs seem to indicate this is now MergeAuthz Off and is off by
default? Is that true in the code? The code seems to confirm that
conf->op = AUTHZ_LOGIC_OFF;
is the default until one of the Match* containers is used (why those
directive names are Match* instead of AuthMatch* boggles my mind).
What is the conclusion to this thread? Why are all the Authz
directives given random names? Am I the only one that finds this
feature set impossible to follow?
....Roy
On Oct 29, 2008, at 3:17 PM, Chris Darroch wrote:
> Dan Poirier wrote:
>
>> I like the idea of replacing ON with AND and OR. It would not
>> only provide more control, but make it explicit what kind of merging
>> was going to happen.
>> I have mixed thoughts about changing the default to OFF. Cons:
>> That would mean every container directive would have to specify
>> some sort of access control (or at least AuthzMergeRules AND) or it'd
>> be wide open, right?
>
> I don't think so; at least, that's not what I was intending.
> Rather, something much like 2.2's behaviour: containers that don't
> specify any authz are simply protected by the "nearest" container
> merged ahead of them that does specify authz.
>
>
> I'm hoping to put this thread to bed shortly with the patches
> available here:
>
> http://people.apache.org/~chrisd/patches/httpd_authnz_configs/
>
> My intent is to finish up the necessary documentation changes
> and get everything committed to trunk in the next few days. (Fingers
> crossed!)
>
> In the meantime, an overview follows. Many, many thanks are
> due to Brad Nicholes, whose massive refactoring of the authn/z
> system makes all of this work possible.
>
>
> 1) <Limit> and <LimitExcept> are made nestable, with an error in
> the case where all methods are configured out. There are also some
> tuneups related to <Limit>/<LimitExcept> being intended to contain
> authz configurations only and to not be functional outside
> <Directory>/
> <Location>/etc.
>
> 2) A setting of "AuthType None" is allowed, which sets ap_auth_type
> () to
> NULL and thus provides a way to turn off authentication for a
> sub-directory. This corresponds to several convenient ways in
> 2.4 to
> turn off authorization, including "Require all granted" (and, at a
> deeper level, the new "SatisfySections Off").
>
> 3) The mod_authz_core.c module is rewritten to attempt to deal with
> the
> issues discussed on this thread and the previous one, as well as
> those described at the end of this email. The authz_provider_list
> two-pronged linked lists are replaced by a tree structure that
> mirrors
> what is configured via <SatisfyAll> and <SatisfyAny>.
>
> A pair of negative authz containers are introduced, <SatisfyNotAll>
> and <SatisfyNotAny>, which negate their operands in the same
> manner as Reject. Thus we have the following table:
>
> Require A
> Reject !A
> <SatisfyAll> (A && B && ...)
> <SatisfyAny> (A || B || ...)
> <SatisfyNotAll> !(A && B && ...)
> <SatisfyNotAny> !(A || B || ...)
>
> The <SatisfyAny> directive is renamed from <SatisfyOne> so as not
> to imply XOR-like functionality (requiring exactly one successful
> operand).
>
> A number of configuration-time checks are implemented to warn
> administrators regarding redundant or non-functional authz
> configurations. In particular, since the negative authz directives
> can not contribute meaningfully to OR-like blocks, as they
> can only supply neutral (AUTHZ_NEUTRAL) or false (AUTHZ_DENIED)
> values, they are simply not allowed in these containers. (The
> code should support them, though, if this check is ever removed.)
> Similarly, AND-like blocks without only negative authz directives
> also produce a configuration-time error.
>
> The MergeAuthzRules directive is renamed SatisfySections and
> take three possible values, Off, All, and And. The default is Off,
> meaning that as directory configuration sections are merged,
> new authz configurations replace previously merged ones. However,
> a directory section may specify "SatisfySections All" to force
> its predecessor's authz to be successful as well as its own.
> The "SatisfySections Any" option permits either the predecessor
> or current section's authz to grant the user access. Note that
> the setting of SatisfySections continues to be local only to
> the directory section it appears in; it is not inherited to
> subsequent sections as they are merged.
>
> The default setting of SatisfySections is Off, corresponding to
> traditional pre-2.4 authz logic. Within a directory section,
> the default logic corresponds to an AND-like block (i.e.,
> <SatisfyAll>),
> which differs from the pre-2.4 logic whereby the first Require
> statement to succeed authorized the request.
>
> Legacy 2.2 configurations should, I hope, work with few or no
> changes as a result of these revisions. Few administrators, I hope,
> have configurations with multiple Require directives in a
> section; e.g.:
>
> <Directory /foo>
> Require group shirt
> Require group shoes
> </Directory>
>
> If they do, these would need to be revised to either place all the
> items in a single Require directive (e.g., Require group shirt
> shoes)
> or to use a <SatisfyAny> section. I feel this makes the overall
> intent of the configuration directives clearer, since it is not
> apparent that the example above grants access to members of either
> group, not just those who are members of both.
>
> It also means that the following 2.4-style configuration makes
> intuitive sense, because the negative Reject directive only has
> meaning in an AND-like context:
>
> <Directory /foo>
> Require group shirt
> Reject user noshoes
> </Directory>
>
> However, if this proves to be a point of considerable difficulty
> for people upgrading to 2.4, it is straightforward to make the
> default logic of a section be OR-like by editing the
> create_default_section() function in mod_authz_core.c.
>
> The legacy Satisfy directive's logic is already largely handled in
> request.c in 2.4, so mod_access_compat.c could be simplified
> slightly.
>
> The <Limit> and <LimitExcept> directives are handled by
> tracking the currently applicable set of methods for all Require/
> Reject
> directives within a section and its sub-sections. When the
> request's method does not apply to an authz section, we can then
> immediately return either AUTHZ_GRANTED or AUTHZ_NEUTRAL, depending
> on whether we are in an AND-like or OR-like context, respectively.
>
> 4) The mod_authn_default.c and mod_authz_default.c modules are
> removed,
> shifting the small amount of remaining functionality they provide
> into
> mod_authn_core.c and mod_authz_core.c. The existence of both "core"
> and "default" authn/z modules appeared likely to be a source of some
> confusion (especially since they can all be made optional at
> runtime).
>
> The mod_authz_default.c module, in particular, was also almost
> entirely obviated by the mod_authz_core.c rewrite, and removing
> it clarified the code paths in many cases where administrators
> likely failed to create a sensible configuration, e.g., by engaging
> authentication with an AuthType directive but configuring not
> authorization to match.
>
>
> That's about it for now; comments welcome. Just for historical
> purposes, some things I ran into with the authz_provider_list
> two-pronged linked lists with prompted the rewrite:
>
> First, there were some configurations which could send the
> check_provider_list() function into an infinite loop. For example,
> with
> the following configuration, if one authenticated as user "foo" (i.e.,
> a user not in any of the Require directives), the function recursed
> until
> it reached the end of the list(s), received AUTHZ_DENIED from the last
> invocation, and then the second-last invocation would loop on the goto
> one_next statement. Since nothing had changed, it just repeated
> the final
> invocation over and over.
>
> AuthzMergeRules Off
> <SatisfyOne>
> Require user who
> <SatisfyOne>
> Require user tar
> </SatisfyOne>
> Require user dis
> </SatisfyOne>
>
>
> Second, the two-pronged authz_provider_list lists seemed to lead to
> incorrect authorization behaviour when directives were moved around in
> the configuration file in innocuous ways. For example, with the
> following
> configuration, logging in as user "foo" would succeed:
>
> Require valid-user
> <SatisfyOne>
> Require user who
> Require user foo
> </SatisfyOne>
>
> Moving the first statement to the end of the configuration caused
> the user to be rejected, though:
>
> <SatisfyOne>
> Require user who
> Require user foo
> </SatisfyOne>
> Require valid-user
>
> The difference was that in the first case, the linked lists looked
> roughly like this:
>
> "Require valid-user"
> req_state = AUTHZ_REQSTATE_ALL
> all_next -> "Require user who"
> req_state = AUTHZ_REQSTATE_ONE
> one_next -> "Require user foo"
> req_state = AUTHZ_REQSTATE_ONE
>
> while in the second case, they looked like this:
>
> "Require user who"
> req_state = AUTHZ_REQSTATE_ONE
> all_next -> "Require valid-user"
> req_state = AUTHZ_REQSTATE_ALL
> all_next -> "Require user foo"
> req_state = AUTHZ_REQSTATE_ONE
>
> When check_provider_list() received AUTHZ_DENIED after testing
> user "foo" against "Require user who", it found a NULL one_next
> pointer and just returned AUTHZ_DENIED, rather than proceeding
> any further.
>
> Chris.
>
> --
> GPG Key ID: 366A375B
> GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A
> 375B
>
>
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
Re: AuthzMergeRules blocks everything in default configuration
Posted by Chris Darroch <ch...@pearsoncmg.com>.
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 Dan Poirier <po...@pobox.com>.
Chris Darroch said the following on 10/29/2008 10:35 PM:
> <Directory /humans_and_friendlies_only>
> Require valid-user
> <SatisfyNotAll>
> Require group alien
> <SatisfyAny>
> Require group hostile
> Require group neutral
> Require group noninterventionist
> </SatisfyAny>
> </SatisfyNotAll>
> </Directory>
I wonder if we have to stick with building up complicated authorization
expressions by nesting many different directives.
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.
I'm picturing something like this:
<Directory /humans_only>
AuthorizeIf valid-user AND member-of-group human
</Directory>
Maybe with some syntactic sugar to help break things down into
understandable chunks:
DefineAuthorization friendly NOT (member-of-group hostile OR
member-of-group neutral OR member-of-group noninterventionist)
DefineAuthorization human NOT (member-of-group alien)
<Directory /humans_and_friendlies_only>
AuthorizeIf valid-user AND (human OR friendly)
</Directory>
We could add a few more pre-defined primitives that could be used in
expressions, such as
# Inherit parent authorization, and add another condition
AuthorizeIf authorized-in-parent AND member-of-group vips
# Grant access to all
AuthorizeIf true
Granted, this is very different from the syntax in 2.2 configurations,
so migration would have to be considered. But it might be worthwhile if
something like this reduced confusion and questions on the lists.
Dan
Re: AuthzMergeRules blocks everything in default configuration
Posted by Chris Darroch <ch...@pearsoncmg.com>.
Eric Covener wrote:
> I tend to prefer something closer to the old name, especially with the
> Satisfy containers being optional. IOTW the "sections" here may not
> be something explicit the user can look back to.
(Maybe IOW, "in other words"? :-)
They should be: since <Directory> and friends all function as
<SatisfyAll>, everything is always a section of some type. I could
live with SatisfyOnMerge or something similar, but I'd like "Satisfy"
in there somewhere to clarify the relationship with both the legacy
Satisfy and the new <Satisfy*> blocks.
>> <Directory /foo>
>> Require group shirt
>> Require group shoes
>> </Directory>
>
> More common migration breakage might be using two different authz
> providers there.
Yes, and in that case, adding a <SatisfyAny> would be needed.
Or, changing the default, which is simple. I'm inclined to commit
as written, and if it looks like this is a sticking point, change it.
That should be trivial, since SatisfySections Any creates exactly
the same thing (during a per-dir merge), so the code paths are
all tested already.
> When do we need the negative satisfy containers? I'm having a hard
> time with this one, especially with how negating Require/Reject are
> interpreted inside -- it almost seems like the containers should be
> dictating the verbs and you should just have some more agnostic
> match-type operator with the authz providers.
I suppose the most likely case might be something like:
<Directory /humans_and_friendlies_only>
Require valid-user
<SatisfyNotAll>
Require group alien
<SatisfyAny>
Require group hostile
Require group neutral
Require group noninterventionist
</SatisfyAny>
</SatisfyNotAll>
</Directory>
so "aliens" are OK, unless they're in one or more of a set of
other groups. Presumably you can't just say "Reject group alien"
at the top level because some aliens are OK, nor can you say
"Reject group hostile" at the top level, because some hostiles
are human. Probably makes the most sense with complex LDAP setups
where people belong to lots of overlapping categories.
I think negated sections are pretty clear and have some utility;
at a minimum, providing the ability to negate a single match
(i.e., Reject) but no equivalent negation operator at a higher level
makes for some odd constructions which can be more clearly expressed
with higher-level negation. It's the equivalent of negated
parenthetical constructions in logical expressions, like !(A && B).
As you note, it's a little counter-intuitive to be using Require/Reject
in these locations -- something like Match and NotMatch might make
everything clearer. I'm happy to consider such a thing; I think I'll
get the commit in first and then we can adjust further.
The main reason to keep Require, of course, is backward compatibility.
But we could set that up to work like Match, say, and throw an error
unless it's used in a "naked" context (outside of any <Satisfy*>).
That would be easy, and then for configurations with <Satisfy*> blocks,
one would use Match/NotMatch inside of them.
Chris.
--
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
Re: AuthzMergeRules blocks everything in default configuration
Posted by Eric Covener <co...@gmail.com>.
On Wed, Oct 29, 2008 at 6:17 PM, Chris Darroch <ch...@pearsoncmg.com> wrote:
> The MergeAuthzRules directive is renamed SatisfySections and
> take three possible values, Off, All, and And. The default is Off,
> meaning that as directory configuration sections are merged,
> new authz configurations replace previously merged ones. However,
> a directory section may specify "SatisfySections All" to force
> its predecessor's authz to be successful as well as its own.
> The "SatisfySections Any" option permits either the predecessor
> or current section's authz to grant the user access. Note that
> the setting of SatisfySections continues to be local only to
> the directory section it appears in; it is not inherited to
> subsequent sections as they are merged.
I tend to prefer something closer to the old name, especially with the
Satisfy containers being optional. IOTW the "sections" here may not
be something explicit the user can look back to.
>
> The default setting of SatisfySections is Off, corresponding to
> traditional pre-2.4 authz logic. Within a directory section,
> the default logic corresponds to an AND-like block (i.e., <SatisfyAll>),
> which differs from the pre-2.4 logic whereby the first Require
> statement to succeed authorized the request.
Good to see the behavior default to being true to the verb.
>
> Legacy 2.2 configurations should, I hope, work with few or no
> changes as a result of these revisions. Few administrators, I hope,
> have configurations with multiple Require directives in a section; e.g.:
>
> <Directory /foo>
> Require group shirt
> Require group shoes
> </Directory>
More common migration breakage might be using two different authz
providers there.
> It also means that the following 2.4-style configuration makes
> intuitive sense, because the negative Reject directive only has
> meaning in an AND-like context:
>
> <Directory /foo>
> Require group shirt
> Reject user noshoes
> </Directory>
When do we need the negative satisfy containers? I'm having a hard
time with this one, especially with how negating Require/Reject are
interpreted inside -- it almost seems like the containers should be
dictating the verbs and you should just have some more agnostic
match-type operator with the authz providers.
--
Eric Covener
covener@gmail.com
Re: AuthzMergeRules blocks everything in default configuration
Posted by Chris Darroch <ch...@pearsoncmg.com>.
Dan Poirier wrote:
> I like the idea of replacing ON with AND and OR. It would not
> only provide more control, but make it explicit what kind of merging
> was going to happen.
>
> I have mixed thoughts about changing the default to OFF.
>
> Cons: That would mean every container directive would have to specify
> some sort of access control (or at least AuthzMergeRules AND) or it'd
> be wide open, right?
I don't think so; at least, that's not what I was intending.
Rather, something much like 2.2's behaviour: containers that don't
specify any authz are simply protected by the "nearest" container
merged ahead of them that does specify authz.
I'm hoping to put this thread to bed shortly with the patches
available here:
http://people.apache.org/~chrisd/patches/httpd_authnz_configs/
My intent is to finish up the necessary documentation changes
and get everything committed to trunk in the next few days. (Fingers
crossed!)
In the meantime, an overview follows. Many, many thanks are
due to Brad Nicholes, whose massive refactoring of the authn/z
system makes all of this work possible.
1) <Limit> and <LimitExcept> are made nestable, with an error in
the case where all methods are configured out. There are also some
tuneups related to <Limit>/<LimitExcept> being intended to contain
authz configurations only and to not be functional outside <Directory>/
<Location>/etc.
2) A setting of "AuthType None" is allowed, which sets ap_auth_type() to
NULL and thus provides a way to turn off authentication for a
sub-directory. This corresponds to several convenient ways in 2.4 to
turn off authorization, including "Require all granted" (and, at a
deeper level, the new "SatisfySections Off").
3) The mod_authz_core.c module is rewritten to attempt to deal with the
issues discussed on this thread and the previous one, as well as
those described at the end of this email. The authz_provider_list
two-pronged linked lists are replaced by a tree structure that mirrors
what is configured via <SatisfyAll> and <SatisfyAny>.
A pair of negative authz containers are introduced, <SatisfyNotAll>
and <SatisfyNotAny>, which negate their operands in the same
manner as Reject. Thus we have the following table:
Require A
Reject !A
<SatisfyAll> (A && B && ...)
<SatisfyAny> (A || B || ...)
<SatisfyNotAll> !(A && B && ...)
<SatisfyNotAny> !(A || B || ...)
The <SatisfyAny> directive is renamed from <SatisfyOne> so as not
to imply XOR-like functionality (requiring exactly one successful
operand).
A number of configuration-time checks are implemented to warn
administrators regarding redundant or non-functional authz
configurations. In particular, since the negative authz directives
can not contribute meaningfully to OR-like blocks, as they
can only supply neutral (AUTHZ_NEUTRAL) or false (AUTHZ_DENIED)
values, they are simply not allowed in these containers. (The
code should support them, though, if this check is ever removed.)
Similarly, AND-like blocks without only negative authz directives
also produce a configuration-time error.
The MergeAuthzRules directive is renamed SatisfySections and
take three possible values, Off, All, and And. The default is Off,
meaning that as directory configuration sections are merged,
new authz configurations replace previously merged ones. However,
a directory section may specify "SatisfySections All" to force
its predecessor's authz to be successful as well as its own.
The "SatisfySections Any" option permits either the predecessor
or current section's authz to grant the user access. Note that
the setting of SatisfySections continues to be local only to
the directory section it appears in; it is not inherited to
subsequent sections as they are merged.
The default setting of SatisfySections is Off, corresponding to
traditional pre-2.4 authz logic. Within a directory section,
the default logic corresponds to an AND-like block (i.e., <SatisfyAll>),
which differs from the pre-2.4 logic whereby the first Require
statement to succeed authorized the request.
Legacy 2.2 configurations should, I hope, work with few or no
changes as a result of these revisions. Few administrators, I hope,
have configurations with multiple Require directives in a section; e.g.:
<Directory /foo>
Require group shirt
Require group shoes
</Directory>
If they do, these would need to be revised to either place all the
items in a single Require directive (e.g., Require group shirt shoes)
or to use a <SatisfyAny> section. I feel this makes the overall
intent of the configuration directives clearer, since it is not
apparent that the example above grants access to members of either
group, not just those who are members of both.
It also means that the following 2.4-style configuration makes
intuitive sense, because the negative Reject directive only has
meaning in an AND-like context:
<Directory /foo>
Require group shirt
Reject user noshoes
</Directory>
However, if this proves to be a point of considerable difficulty
for people upgrading to 2.4, it is straightforward to make the
default logic of a section be OR-like by editing the
create_default_section() function in mod_authz_core.c.
The legacy Satisfy directive's logic is already largely handled in
request.c in 2.4, so mod_access_compat.c could be simplified slightly.
The <Limit> and <LimitExcept> directives are handled by
tracking the currently applicable set of methods for all Require/Reject
directives within a section and its sub-sections. When the
request's method does not apply to an authz section, we can then
immediately return either AUTHZ_GRANTED or AUTHZ_NEUTRAL, depending
on whether we are in an AND-like or OR-like context, respectively.
4) The mod_authn_default.c and mod_authz_default.c modules are removed,
shifting the small amount of remaining functionality they provide into
mod_authn_core.c and mod_authz_core.c. The existence of both "core"
and "default" authn/z modules appeared likely to be a source of some
confusion (especially since they can all be made optional at runtime).
The mod_authz_default.c module, in particular, was also almost
entirely obviated by the mod_authz_core.c rewrite, and removing
it clarified the code paths in many cases where administrators
likely failed to create a sensible configuration, e.g., by engaging
authentication with an AuthType directive but configuring not
authorization to match.
That's about it for now; comments welcome. Just for historical
purposes, some things I ran into with the authz_provider_list
two-pronged linked lists with prompted the rewrite:
First, there were some configurations which could send the
check_provider_list() function into an infinite loop. For example, with
the following configuration, if one authenticated as user "foo" (i.e.,
a user not in any of the Require directives), the function recursed until
it reached the end of the list(s), received AUTHZ_DENIED from the last
invocation, and then the second-last invocation would loop on the goto
one_next statement. Since nothing had changed, it just repeated the final
invocation over and over.
AuthzMergeRules Off
<SatisfyOne>
Require user who
<SatisfyOne>
Require user tar
</SatisfyOne>
Require user dis
</SatisfyOne>
Second, the two-pronged authz_provider_list lists seemed to lead to
incorrect authorization behaviour when directives were moved around in
the configuration file in innocuous ways. For example, with the following
configuration, logging in as user "foo" would succeed:
Require valid-user
<SatisfyOne>
Require user who
Require user foo
</SatisfyOne>
Moving the first statement to the end of the configuration caused
the user to be rejected, though:
<SatisfyOne>
Require user who
Require user foo
</SatisfyOne>
Require valid-user
The difference was that in the first case, the linked lists looked
roughly like this:
"Require valid-user"
req_state = AUTHZ_REQSTATE_ALL
all_next -> "Require user who"
req_state = AUTHZ_REQSTATE_ONE
one_next -> "Require user foo"
req_state = AUTHZ_REQSTATE_ONE
while in the second case, they looked like this:
"Require user who"
req_state = AUTHZ_REQSTATE_ONE
all_next -> "Require valid-user"
req_state = AUTHZ_REQSTATE_ALL
all_next -> "Require user foo"
req_state = AUTHZ_REQSTATE_ONE
When check_provider_list() received AUTHZ_DENIED after testing
user "foo" against "Require user who", it found a NULL one_next
pointer and just returned AUTHZ_DENIED, rather than proceeding
any further.
Chris.
--
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B