httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Eric Prud'hommeaux" <e...@w3.org>
Subject Re: file-level access control: ap_sub_req_lookup_file bypasses auth check
Date Fri, 19 Mar 1999 16:29:38 GMT
On Thu, Mar 18, 1999 at 05:07:38PM -0800, Dean Gaudet wrote:
> On Thu, 18 Mar 1999, Eric Prud'hommeaux wrote:
> 
> > On Wed, Mar 17, 1999 at 05:25:28PM -0800, Dean Gaudet wrote:
> > > Uh, it also does a file_walk. 
> > 
> > Yes, and when it's done, it checks to see if it arrived at the same
> > dir_config that was stored in the parent request (eg /1999/abc).
> > If it's the same dir_config, it returns immediately, bypassing the
> > end of ap_sub_req_lookup_file.
> 
> Right, none of those checks should be any different if the dir_config is
> the same.  So they're redundant... or at least that's how things should
> be.  I'm asking you to lead me down the path and show me how they aren't
> redundant.  (I *used* to understand this code well, but I haven't
> "swapped" that knowledge back into my head yet, I'm trying not to swap it
> in unless you can demonstrate the bug :)

Fair enough. The problem may arrise from my trying to implement
dynamic ACLs (as you observe later) without doing my homework. I have
no mechanism to create a different dir_config when examining /1999/abc
and /1999/abc.html even though there is only an ACL entry for the latter.

> > To be more clear - here is the diff that removes the shortcut code:
> 
> I understand what you want to remove.  I just question why it's necessary
> to remove this for correctness.

I didn't reallize before that all conventional mechanisms for
file-level ACLs would create alternate dir_configs. It may not be
correct, but may be an extension.

> > I have a new theory for why noone else complained about file-level
> > ACLs before; file-level ACLs were always implemented by adding
> > directives to the directory config or the .htaccess files so the
> > dir_config for /1999/blah actually had a different dir_config than
> > /1999/blah.html so (rnew->per_dir_config == r->per_dir_config) never
> > tested true.
> 
> Even if a file-level acl is global, the merge which adds it to the
> configuration will cause rnew->per_dir_config to change. 
> 
> > > Can you give a minimal example where it doesn't work?  I don't understand
> > > what you're saying above... show a config please.
> > 
> > <Directory ~ /home/httpd/html/199(8|9)/>
> >   Options +Indexes
> >   Order allow,deny
> >   Allow from all
> >   AuthName W3CACL
> >   AuthType Basic
> >   <Limit GET>
> >     Require certACL
> >   </Limit>
> > </Directory>
> > 
> > The certACL module has the ACLs for /1999/blah.html in a database.
> > I don't have an entry for /1999/blah because it doesn't exist. I
> > assumed certACL would again be called when mod_negotiation does
> > a fixup to /1999/blah.html.
> 
> Ah!  OK you're doing something that we've never designed this to do. 
> Apache only knows about objects which you specify <Location>, <Directory>,
> <Files>, or .htaccess for -- it *assumes* that if two objects match the
> same set of those containers then their access rights are identical.

comprendo.

> I personally think this is important... and actually I'd love to not have
> regex's either, because there are some totally rad optimizations you can
> perform.  We haven't performed (m)any of those optimizations yet though... 
> so it's not obvious why we have this restriction. 
> 
> One consequence of removing the code you've asked to be removed is that it
> could cause other auth modules, which maybe have a non-trivial lookup
> cost, to be run more frequently.  In particular, when you do a directory
> index every entry goes through this shortcut currently... (an example of
> one of the places where this optimization helps). 

The added cost of dropping this shortcut is somewhat mitigated by the
fact that any place where it can be used, ie. situations where the
complete ACL for the request in question is in dir_config, will result
in a memory-only auth query. Any place the auth system has to do an
expensive file or database query would mean the info was not in the
dir_config anyways so the shortcut was inappropriate.

This is not so much an argument, as much as a "it's not as bad as it
could be" point.

> It would seem that you're trying to implement dynamic ACLs.  What could
> solve the problem for you is an extensible .htaccess mechanism -- a method
> for modules to add to the foo_walk() mechanisms.  Needs some thought... 
> you need to generate per_dir_configs on the fly -- which has some
> performance consequences, especially in the multiprocess model (you might
> pay N times the memory, where N is the number of children, because the
> children won't have inherited it c.o.w. from the parent...) 

Is the product of a dir_merge ephemoral (existant only for the
duration of the request)? I beleieve this is the case from this code
in file_walk:
                per_dir_defaults = ap_merge_per_dir_configs(r->pool,
                                                         per_dir_defaults,
                                                         this_conf);
If so, and the DB query were moved into the foo_walk call, there would
be no re-use of dir_configs for frequently accessed resources, but it
wouldn't cause a memory burden. There would be only one static
dir_config for the entire dynamic ACL area.

If I have mis-read or mis-remembered the code, and there is a cachig
mechanism, then it would indeed be expensive to maintain sepparate
dir_configs for each resource. Perhaps the the foo_walk call could
maintain a hash set of them so it could find one that already matched
the ACLs for this resource.

Note (to self?): foo_walk should mimick file_walk and _not_ call the
dir_merge if the ACL's are the same as for the previous resource.

Perhaps a HasDynamicACLs directive to bypass the shortcut would be
simpler. What else depends on an accurate ACL in the dir_config? Do we
want to see such a precedent set?
-- 
-eric

(eric@w3.org)

Mime
View raw message