search

captncraig / caddy-realip

Real-IP middleware for caddy
MIT License
18 stars 9 forks source link

Preserve original header value in a new header #12

Open sporkmonger opened 7 years ago

sporkmonger commented 7 years ago

Since the XFF header potentially has security implications, it seems worth preserving the original value, particularly in cases where the transparent directive in the proxy module is likely to be interacting w/ this. I'm not sure if this should be 'always-on' or optional though. Happy to drop it behind a flag if preferred.

captncraig commented 7 years ago

I'm confused why this is needed here. This plugin does not change XFF at all. Just reads it.

sporkmonger commented 7 years ago

That's true, but proxy transparent does change it. And since there's poten

sporkmonger commented 7 years ago

Bumped the close button.

It's true that realip doesn't change the header but proxy transparent does and because realip changes the remote IP, in conjunction, they're really hard to debug. In fairness, the more I think about it, the more I think this should be behind a preserve or preserve_header etc flag, but yeah, having the original header value makes debugging XFF issues in longer load balancer/proxy scenarios massively easier.

I could see an argument that proxy should be responsible for preserving the header, but in that case realip should probably be responsible for providing an option to expose the original remote IP value.

sporkmonger commented 7 years ago

Refactored, adds a preserve flag, documents it, and preserves the RemoteAddr value (the specific information that's destroyed by the realip plugin rather than something that's modified only in conjunction w/ another plugin). Adds tests for the above.