httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Leggett <>
Subject Re: Effective IP address / real IP address
Date Sat, 19 Nov 2011 14:06:15 GMT
On 19 Nov 2011, at 4:49 AM, William A. Rowe Jr. wrote:

> Nevermind that you failed to be consistent in tag values between
> logging schemas...

Can you confirm in more detail what you're referring to? There is only  
one logging line in the patch, and this was based on existing logging  
lines in the same module. Patches should ideally change just one thing  
at a time, but if there are things to fix we should fix them.

> nothing in this proposal addresses the reason
> that I myself had implemented mod_remoteip, which was authn/authz
> control.  In the limited scenario you have considered, authn is
> pretty much a noop on the physical address (no public client would
> ever be routable to that server anyways) so access control is
> shifted to the consumer of the web resources.
> This patch would have a long way to go before being considered...
> and is certainly not a 2.4.x candidate.

The way I approached this was to hunt through the code for instances  
of c->remote_ip and c->remote_addr and address each part of the code  
they touched. In the aaa code I only found reference to them in  
mod_authz_host.c, there was no reference anywhere else. Can you be  
more specific about the authn functionality you believe was missed?

> The very design of mod_remoteip keeps the precious values that
> you are concerned about losing as request notes suitable for
> logging.  But Stefan also points out some flaws in the current
> approach.

Having built a module like this before, I fully appreciate just how  
hard it is to keep the overridden IP address alive long enough to be  
logged given that logging happens in a cleanup, and the module manages  
to do it, but the way it's done now is definitely wrong. We allow  
dangling pointers allocated from a destroyed r->pool to exist in c- 
 >pool structures in the hope that the next request will clean it up,  
and we hope that nobody tries to read c->remote_ip before we do. We  
also slowly leak from c->pool. I recognise this is done to work around  
restrictions to the existing v2.2 API, but as new code, we should fix  
the API and do this properly - none of this should be released in v2.4  
in it's current form.

> The correction is simple; promote the remote_ip up to the request
> rec and log/use for authentication that r->remote_ip throughout
> httpd.  Introduce a wire client logging tag for c->remote_ip.

This is a lot simpler and cleaner I think, let me come up with an  
alternative patch.


View raw message