AuthN only once per request instead once every subrequest

[PlagiaTUM <pl...@fs.tum.de>]

Dear List!

We are trying to use mod_autoindex on an access-restricted web server
with lots of directories. Our AuthN is costly (we are using
mod_auth_external). With ~ 700 directories, the generation of the index
takes > 1 minute.

With a little profiling we found out that authentication is done for
every subrequest, of which mod_autoindex uses plenty.

There is some logic in ap_process_request_internal() that should
optimize these out; however, it does not work for us. The comparison
(r->main->per_dir_config == r->per_dir_config) never succeeds as
ap_merge_per_dir_configs() always returns a new configuration vector.

Cf. request.c lines 145ff (inside of ap_process_request_internal()) and
466ff (ap_directory_walk()) and config.c lines 230ff
(ap_merge_per_dir_configs()).

Why is it necessary to re-authenticate within a subrequest?
When the per_dir configuration has changed, AuthZ has of course to be
rechecked. AuthN, on the other hand, could be taken from the parent
request, where it already has been verified. What are we missing?

With the attached patch (against 2.2.17), the page generation is down to
one AuthN process (ca. 2 seconds overall) on our machine.
The patch is meant for communication purposes only, to illustrate what 
we are talking about. It is not intended to be a ready made solution to 
the problem we found. To make our changes stand out, we also forwent 
correct indentation.

We would have liked to include a check like ``if the user is the same as
in the parent request and it already has been AuthN'd'', but we don't
know the best way to do that.

A quick test with a .htaccess file showed that Access Restrictions are
still honored within a subrequest.

Thank you for your consideration and time!

PlagiaTUM

Re: AuthN only once per request instead once every subrequest

[PlagiaTUM <pl...@fs.tum.de>]

On 05/24/2011 09:29 PM, Stefan Fritsch wrote:
>> The mod_auth_socache way should indeed work, but is a work-around,
>> not a fix. We would like to fix the situation for the benefit of
>> all httpd users.
> There may already be something for you in 2.3:
>
> http://svn.apache.org/viewvc?view=revision&revision=644525
>
> Can you try if 2.3.12 or the version from svn trunk fixes your
> problem?

We had some trouble getting 2.3.12 and 2.3.13 to build on our old and
interestingly configured OpenSolaris machine, but 2.3.14 built OK. We
cannot test this still, because mod-auth-external [1] has not yet been
converted to the 2.4 AAA API. We've written its maintainer to see
whether there is work underway for doing that, maybe we even can help out.

In the meantime, thanks a lot for your helpful response!

If we progress further at this pace, we surely will have a solution
until 2.4 is out.

[1] http://code.google.com/p/mod-auth-external/

-- 
PlagiaTUM - Not responding for months, we're cool like that

Re: AuthN only once per request instead once every subrequest

[Stefan Fritsch <sf...@sfritsch.de>]

Hi,

On Tuesday 24 May 2011, PlagiaTUM wrote:
> Sorry for not writing so long,  and please excuse our being such a
> pest about this.
> 
> You probably have overseen the remaining questions in our last
> email.
> 
> The mod_auth_socache way should indeed work, but is a work-around,
> not a fix. We would like to fix the situation for the benefit of
> all httpd users.

There may already be something for you in 2.3:

http://svn.apache.org/viewvc?view=revision&revision=644525

Can you try if 2.3.12 or the version from svn trunk fixes your 
problem?

I fear these changes will never be backported to 2.2.x, because they 
are way too intrusive. But 2.4.0 will hopefully come real soon now 
(tm). We would be really interested to hear about your experience with 
2.3.x.

> Using show_forbidden does not work, because the subrequest is fired
> anyway, including re-authorization. From what we (us three) can see
> in the code, the subrequests are pretty much needed, for example
> for find_title(), find_icon() and find_alt().
> 
> The most promising and hopefully also un-intrusive way in our
> opinion is to fix the per_dir_config comparison in
> ap_process_request_internal() that should optimize out the
> unnecessary re-auths.
> 
> We wrote (Fri, 18 Mar 2011 03:23:01 -0700):
>  > The comparison
>  > (r->main->per_dir_config == r->per_dir_config) never succeeds
>  > [...]
> 
> Nick Kew wrote (Fri, 18 Mar 2011 03:46:05 -0700):
>  > It does if the configurations to r and r->main are not the same.
> 
>  From looking into the debugger, the _content_ of the
> per_dir_configs is, in our case, the same. The memory location is
> not. So the == operator returns false.
> 
> In server.c we find this:
> > /* Skip authn/authz if the parent or prior request passed the
> > authn/authz, * and that configuration didn't change (this
> > requires optimized _walk() * functions in map_to_storage that
> > use the same merge results given * identical input.) If the
> > config changes, we must re-auth. */
> 
> Might this be relevant for us? Do we have to modify a _walk() or
> _merge() function? Can you give us a hint where to look?

The above commit contains some modifications of the _merge() 
functions. I have not looked at them more closely, though.
The commit message contains a link to some mailing list discussion, 
maybe that helps.

Cheers,
Stefan

Re: AuthN only once per request instead once every subrequest

[PlagiaTUM <pl...@fs.tum.de>]

In fact, a memcmp() should very well solve our problem,	but we don't 
know the size of the ap_conf_vector_t structure. Or do we?

On 24.05.2011 04:01, PlagiaTUM wrote:
> From looking into the debugger, the _content_ of the per_dir_configs
> is, in our case, the same. The memory location is not. So the ==
> operator returns false.

-- 
PlagiaTUM is Heike Muni, Thomas Kittel and Florian Sesser.
Messing with others' code when we should be studying.

Re: AuthN only once per request instead once every subrequest

[PlagiaTUM <pl...@fs.tum.de>]

Hello again!

Sorry for not writing so long,  and please excuse our being such a pest 
about this.

You probably have overseen the remaining questions in our last email.

The mod_auth_socache way should indeed work, but is a work-around, not a 
fix. We would like to fix the situation for the benefit of all httpd users.

Using show_forbidden does not work, because the subrequest is fired 
anyway, including re-authorization. From what we (us three) can see in 
the code, the subrequests are pretty much needed, for example for 
find_title(), find_icon() and find_alt().

The most promising and hopefully also un-intrusive way in our opinion is 
to fix the per_dir_config comparison in ap_process_request_internal() 
that should optimize out the unnecessary re-auths.

We wrote (Fri, 18 Mar 2011 03:23:01 -0700):
 > The comparison
 > (r->main->per_dir_config == r->per_dir_config) never succeeds
 > [...]

Nick Kew wrote (Fri, 18 Mar 2011 03:46:05 -0700):
 > It does if the configurations to r and r->main are not the same.

 From looking into the debugger, the _content_ of the per_dir_configs 
is, in our case, the same. The memory location is not. So the == 
operator returns false.

In server.c we find this:
> /* Skip authn/authz if the parent or prior request passed the authn/authz,
> * and that configuration didn't change (this requires optimized _walk()
> * functions in map_to_storage that use the same merge results given
> * identical input.) If the config changes, we must re-auth.
> */

Might this be relevant for us? Do we have to modify a _walk() or 
_merge() function? Can you give us a hint where to look?

Thanks again!

Re: AuthN only once per request instead once every subrequest

[PlagiaTUM <pl...@fs.tum.de>]

Thank you for your prompt reply! And your two fine suggestions.

On 18/03/11 11:45, Nick Kew wrote:
> On 18 Mar 2011, at 10:22, PlagiaTUM wrote:
>> With a little profiling we found out that authentication is done
>> for every subrequest, of which mod_autoindex uses plenty.
> 
> You have two good solutions to that.  Either use the ShowForbidden 
> option to mod_autoindex, or use mod_authn_socache.

We did not know about mod_authn_socache; It is not in 2.2. Sorry for not
looking more thoroughly. Sounds like a sensible addition from our
perspective!

For our case, we cannot confirm that ShowForbidden helps at all.

The mod_autoindex code (looking at 2.2 trunk) looks like the subrequest
is done anyways; with ShowForbidden it is just ignoring its return value.

We can create a patch for mod_autoindex if you'd like us to.
We were hoping to fix the issue a level below that, possibly catching
more than just the mod_autoindex module in the process.

>> There is some logic in ap_process_request_internal() that should 
>> optimize these out; however, it does not work for us. The
>> comparison (r->main->per_dir_config == r->per_dir_config) never
>> succeeds as ap_merge_per_dir_configs() always returns a new
>> configuration vector.
> 
> It does if the configurations to r and r->main are not the same.

Yes, but can the pointer ever be the same?

request.c (2.2 trunk), lines 169ff:
> /* Reset to the server default config prior to running
> map_to_storage */ r->per_dir_config = r->server->lookup_defaults;

Just a page down of that file, the comparison is done:

same file, line 204
> if (r->prev && (r->prev->per_dir_config == r->per_dir_config)) {

Also, the comments in the source code above indicate that the pointer
equality is reached by having ``optimized'' _walk() functions:

> /* Skip authn/authz if the parent or prior request passed the
> authn/authz, * and that configuration didn't change (this requires
> optimized _walk() * functions in map_to_storage that use the same
> merge results given * identical input.)

That sounded like a fruitful way to go, but we didn't understand what to
do. Are these _walk()s ``optimized'' already?

Please excuse our being so stubborn about this.


Greetings from Munich!


-- 
PlagiaTUM is Heike Muni, Thomas Kittel and Florian Sesser.
Messing with others' code when we should be studying.

Re: AuthN only once per request instead once every subrequest

[Nick Kew <ni...@webthing.com>]

On 18 Mar 2011, at 10:22, PlagiaTUM wrote:

> Dear List!
> 
> We are trying to use mod_autoindex on an access-restricted web server
> with lots of directories. Our AuthN is costly (we are using
> mod_auth_external). With ~ 700 directories, the generation of the index
> takes > 1 minute.
> 
> With a little profiling we found out that authentication is done for
> every subrequest, of which mod_autoindex uses plenty.

You have two good solutions to that.  Either use the ShowForbidden
option to mod_autoindex, or use mod_authn_socache.

> There is some logic in ap_process_request_internal() that should
> optimize these out; however, it does not work for us. The comparison
> (r->main->per_dir_config == r->per_dir_config) never succeeds as
> ap_merge_per_dir_configs() always returns a new configuration vector.

It does if the configurations to r and r->main are not the same.

> Cf. request.c lines 145ff (inside of ap_process_request_internal()) and
> 466ff (ap_directory_walk()) and config.c lines 230ff
> (ap_merge_per_dir_configs()).
> 
> Why is it necessary to re-authenticate within a subrequest?
> When the per_dir configuration has changed, AuthZ has of course to be
> rechecked. AuthN, on the other hand, could be taken from the parent
> request, where it already has been verified. What are we missing?

There's nothing to stop the subrequest running off an entirely different
AuthUserFile/equiv to the main request.

-- 
Nick Kew

Available for work, contract or permanent
http://www.webthing.com/~nick/cv.html