Usage of $proxy_add_x_forwarded_for on edge proxies

[nanaya]

On Thu, Jan 14, 2021, at 02:45, Maxim Dounin wrote:
> 
> Another question is how often it is used properly.  Given it 
> requires update of two headers, at least one of them being very 
> rare, I would assume the answer is "almost never".  But again, it 
> has nothing to do with $proxy_add_x_forwarded_for.  Rather, 
> properly using "proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;" 
> will make attacks impossible.
> 
> > > While X-Forwarded-For is often misused by applications and 
> > > incorrect configurations by blindly trusting addresses in it, 
> > > removing the header is going to make destroy the information 
> > > available for well-written applications.  While you it might be a 
> > > good idea to remove the header in your particular use case - if 
> > > you are sure enough your applications doesn't use it - this is 
> > > certainly not how things should be configured by default.
> > > 
> > 
> > Yeah, I'm not going to trust X-Forwarded-For sent by client. 
> > Maybe it's just me. $remote_addr to me is their geolocation. 
> > Anything more "sophisticated" just looked like a potential of 
> > failure.
> 
> Again: there is a difference between trusting X-Forwarded-For sent 
> by client and using it in the cases where trust isn't that 
> important.  Certainly no one should blindly trust X-Forwarded-For.  
> But anyone can use it properly, and stripping it as you suggest is 
> just wrong in most cases.
> 
> > And I don't want to have to worry if my $random_app parses the 
> > X-Forwarded-For sanely. At most I'd just log it at the edge 
> > server.
> 
> That's your choice: instead of fixing vulnerabilities in the apps 
> you prefer to drop headers which might be used to exploit these 
> vulnerabilities, dropping also many legitimate uses of the headers.
> 
> > Look at this wonderful function by wordpress (thankfully they do 
> > aware it's "unsafe"):
> > 
> > https://github.com/WordPress/WordPress/blob/c5d1214607be128c99dd27589a58cc5a1d20d522/wp-admin/includes/class-wp-community-events.php#L262
> 
> That's exactly what I'm talking about: as long as you don't need 
> trusted address, X-Forwarded-For can be used to extract some 
> valuable information.
> 

I'm not sure about this exact example though. It looks like it'll lump everyone with 192.168.0.0/24 (or other private network) in same network "id" which would be rather confusing instead.

> Further, note that even with X-Forwarded-For set to $remote_addr, 
> client still can provide arbitrary address to the code via the 
> Client-IP header.  So, if the code in question is somehow used to 
> extract an addresses where trusted address is actually needed, 
> your using $remote_addr instead of $proxy_add_x_forwarded_for 
> won't save you from being vulnerable.
> 

So I think this boils down to how much one trust the apps running behind the proxy.
I tend to err on safe side and don't see much benefit passing the value from client and it has maintenance overhead.

Not to mention if the app has multiple components (websocket server etc) written in different languages, they probably need to reimplement the logic on all of them (hopefully with same logic and config).

>From what I gather the benefits are:
- potentially able to see "actual" client ip instead of their proxy
- audit log

What else am I missing?
Personally, the list above seems to be better done on the edge proxy itself.

I still stand with the default should be $remote_addr and $proxy_add_x_forwarded_for should only be used with extra care. Anything that handles X-Forwarded-For "properly" would know it and it can be passed the client-given value but everything else should just receive $remote_addr.

(also good point on Client-Ip, I do have it overridden as well. It seems to be even less documented for some reason? Know any documentation on it?)