echo icon indicating copy to clipboard operation
echo copied to clipboard

Confusing default behaviour of ExtractIPFromRealIPHeader

Open harrier-lcc opened this issue 3 years ago • 1 comments

Issue Description

For ExtractIPFromRealIPHeader, currently, the default behaviour is to only capture IP in x-real-ip only if it is trusted (for the default setting, this is only the address in private net / loopback etc. (when you use it as e.IPExtractor = ExtractIPFromRealIPHeader())

However, this is usually not the usage for ExtractIPFromRealIPHeader, as often the ingress in front will correctly set/resolve the correct client IP address to X-Real-IP, and thus one would want to use ExtractIPFromRealIPHeader to extract address regardless of it is trusted or not.

So now one will use it as follows:

_, ipV4, _ := net.ParseCIDR("0.0.0.0/0")
_, ipV6, _ := net.ParseCIDR("0:0:0:0:0:0:0:0/0")
e.IPExtractor = echo.ExtractIPFromRealIPHeader(echo.TrustIPRange(ipV4), echo.TrustIPRange(ipV6))

Comparing to the case in ExtractIPFromXFFHeader where it extracts the rightmost untrusted IP, it seems weird that ExtractIPFromRealIPHeader only extracts IP that is trusted (and fallback to network address, which will be some ingress address if there is one). In these two cases "trusted" address seems to have different meaning (in ExtractIPFromXFFHeader its the proxy addresses that are trusted, in ExtractIPFromRealIPHeader it is the address that is trusted to be used) at all and it's very confusing.

https://github.com/labstack/echo/blob/ec92fedf21e817d2d52004a4178292404beb9eaa/ip.go#L223-L236

Checklist

  • [x] Dependencies installed
  • [x] No typos
  • [x] Searched existing issues and docs

Expected behaviour

ExtractIPFromRealIPHeader captures value in X-Real-IP by default, without the "trusted" check

Actual behaviour

ExtractIPFromRealIPHeader only captures if X-Real-IP contains "trusted" address

Steps to reproduce

Version/commit

4.7.2

harrier-lcc avatar Jul 20 '22 04:07 harrier-lcc

Yes, I agree with this issue too much, it confused me for a long time and was too counter-intuitive.

hyacinthus avatar Dec 29 '22 10:12 hyacinthus