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

Filter internal IP addresses from X-Forwarded-For header #24

Open
floatdrop opened this issue Mar 15, 2017 · 14 comments
Open

Filter internal IP addresses from X-Forwarded-For header #24

floatdrop opened this issue Mar 15, 2017 · 14 comments

Comments

@floatdrop
Copy link

Internal ip's (like 192.168.0.1) is not useful in any way for server, so we often filter internal networks from headers.

@pbojinov
Copy link
Owner

What if we exposed a flag that you can set, lets say ignoreInteral or ignoreLocal and if its true, then we return null when it's a internal ip such as 192.168.0.1.

Also thoughts on getting a cohesive list of internal ips?

@floatdrop
Copy link
Author

floatdrop commented Mar 15, 2017

@pbojinov yeah, sounds nice. We used netmask for a while, until IPv6 came into play. I think ip is good candidate for this task.

@pbojinov
Copy link
Owner

@floatdrop in this case, will using ip.isPrivate('127.0.0.1') // true from ip work out ok?

Do you have any example headers we can use for the tests? Want to make sure we're using real live data to cover these test cases.

@pbojinov
Copy link
Owner

pbojinov commented Mar 15, 2017

What about thoughts on naming for the flag: ignorePrivate, ignoreLocal, ignoreInternal?

@floatdrop
Copy link
Author

@pbojinov we have only tests for filtering ipv4 adresses (all of them from private networks), but no actual headers.

What about thoughts on naming for the flag: ignorePrivate, ignoreLocal, ignoreInternal?

I think ignorePrivate is good choice, because of ip.isPrivate method.

@pbojinov
Copy link
Owner

Sounds good, I'll add this in. Thanks!

@fluxsauce
Copy link
Contributor

There's an existing dependency on is.js, maybe add it upstream?

@pbojinov
Copy link
Owner

@fluxsauce the dependency is in the package.json. Am I missing something else?

  "dependencies": {
    "is_js": "^0.9.0"
  }

It should install from npm. I'm seeing this when I run a fresh install:

> npm install request-ip
[email protected] /Users/petar/test
└─┬ [email protected]
  └── [email protected]

@fluxsauce
Copy link
Contributor

@pbojinov I meant add is.ipPrivate to is.js first, as it could be useful in some other places.

@Redmega
Copy link

Redmega commented Apr 22, 2017

Any update on this?

@pbojinov
Copy link
Owner

pbojinov commented Apr 25, 2017

@Redmega yes it's in progress. I'll have something out soon.

@Redmega
Copy link

Redmega commented Apr 26, 2017

Awesome to hear. I'm doing a geoip check on an aws box and its grabbing the IP of the box itself. It's hard to tell without intense console logging if its due to incorrectly set headers or the request-ip package itself. It isn't a high priority issue for us so I've left it alone for now, looking forward to the release 👍

@maximium
Copy link

then we return null when it's a internal ip such as 192.168.0.1

Probably a better way is to return the first not private ip from x-forwarded-for header instead of null
eg. 195.189.143.147 for X-Forwarded-For: 127.0.0.1, 192.168.0.100, 195.189.143.147, 130.236.236.80

@evdama
Copy link

evdama commented Oct 3, 2019

what's the status on this?

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

6 participants