Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential X-Forwarded-For parsing improvements #1453

Open
ttt733 opened this issue Feb 24, 2020 · 5 comments
Open

Potential X-Forwarded-For parsing improvements #1453

ttt733 opened this issue Feb 24, 2020 · 5 comments

Comments

@ttt733
Copy link

ttt733 commented Feb 24, 2020

I noticed a couple of potential issues which might be hard to debug, for people using RemoteAddr() functions.

  1. It does not seem to me that this comparison in context.go should care about string case. (Some people might expect to be able to configure X-FORWARDED-FOR rather than X-Forwarded-For and expect the same functionality.)
if headerName == xForwardedForHeaderKey {
    ...
}
  1. This is maybe something that could be corrected in server configurations outside of the library, but simply taking the first IP in the X-Forwarded-For list opens up client spoofing. The issue (and a potential solution) is explained in more detail at the bottom of this article: https://husobee.github.io/golang/ip-address/2015/12/17/remote-ip-go.html. (Their solution would also probably require new Configuration options allowing users to specify the private subnets they want excluded, though.)
@kataras
Copy link
Owner

kataras commented Feb 24, 2020

Hello @ttt733,

  1. Headers are normalized from go standard library on ctx.GetHeader so X-FORWARDED-FOR and X-Forwarded-For returns exactly the same value.

  2. Needs investigation but if you have some ideas please open a PR.

@kataras
Copy link
Owner

kataras commented Apr 20, 2020

Hello @ttt733,

It's done on master branch (future v12.2.0 release).

// [...]

app.Run(...,
  iris.WithRemoteAddrPrivateSubnet("192.168.0.0" /*start*/, "192.168.255.255" /* end */),
  iris.WithRemoteAddrPrivateSubnet("10.0.0.0", "10.255.255.255"),
)

Thank you a lot for this issue, it was a real improvement that I've never happen to see anywhere else before.

kataras added a commit that referenced this issue Jul 26, 2020
Former-commit-id: e5fde988eda9bf582b04285a1c77ba123910a699
@AlbinoGeek
Copy link

AlbinoGeek commented Sep 11, 2020

The only thing I could request further on this issue is supporting CloudFlare's Client IP headers by option.

CF-Connecting-IP

Or for Enterprise users:

True-Client-IP

Documentation: https://support.cloudflare.com/hc/en-us/articles/206776727-What-is-True-Client-IP-

@kataras
Copy link
Owner

kataras commented Sep 11, 2020

The default list is empty by-default, however CF-Connecting-IP is there at the godoc, True-Client-IP is not but you can add it:

app.Listen/Run(..., iris.WithRemoteAddrHeader("CF-Connecting-IP", "True-Client-IP"))

iris/configuration.go

Lines 806 to 820 in 0f5ec75

// Defaults to an empty slice but an example usage is:
// RemoteAddrHeaders {
// "X-Real-Ip",
// "X-Forwarded-For",
// "CF-Connecting-IP",
// }
//
// Look `context.RemoteAddr()` for more.
RemoteAddrHeaders []string `json:"remoteAddrHeaders,omitempty" yaml:"RemoteAddrHeaders" toml:"RemoteAddrHeaders"`
// RemoteAddrHeadersForce forces the `Context.RemoteAddr()` method
// to return the first entry of a request header as a fallback,
// even if that IP is a part of the `RemoteAddrPrivateSubnets` list.
// The default behavior, if a remote address is part of the `RemoteAddrPrivateSubnets`,
// is to retrieve the IP from the `Request.RemoteAddr` field instead.
RemoteAddrHeadersForce bool `json:"remoteAddrHeadersForce,omitempty" yaml:"RemoteAddrHeadersForce" toml:"RemoteAddrHeadersForce"`

func WithRemoteAddrHeader(header ...string) Configurator {

@AlbinoGeek
Copy link

Thank you! I would definitely say this is solved then :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants