turbobytes / geoipdb

GeoIP related related helper functions for TurboBytes stack
MIT License
0 stars 2 forks source link

Detect if input string is RFC1918 #8

Closed sajal closed 7 years ago

sajal commented 7 years ago

If the input string is private network, then return some sort of answer(TBD) indicating so.

https://en.wikipedia.org/wiki/Private_network Include private and link-local ips for both v4 and v6

Perhaps export and reuse islocalip from https://github.com/turbobytes/pulse/blob/e60f6c0a7bcab4575000e39050bea20ecae8064f/utils/misc.go#L15

sajal commented 7 years ago

I am thinking... we could move islocalip into geoipdb package, and have minion access it from here, but that would introduce cgo dependency for minion making cross-compilation complicated.

sajal commented 7 years ago

What do we use for asn and description if the source ip is private?

Initial suggestion ASN: RFC1918 or AS0 ... AS0 is a good choice, but it has special meaning in other TurboBytes components, unrelated to Pulse. ASN description: Private Network

sajal commented 7 years ago

An example of the issue can be seen at https://pulse.turbobytes.com/results/584916d3ecbe407b4f000005/

See result for 104-HEDII. It incorrectly identifies 80.10.246.2 and 80.10.246.129 as Local Network .

Another example: https://pulse.turbobytes.com/results/58494e98ecbe407b4f0001a5/ It tries to lookup asn for ns[1-2].google.com.

I propose following "pre-flight" checks :-

  1. Detect if the input is a valid IP or not, by using net.ParseIP. If not then return appropriate error.
  2. Check the IP against islocalip(), if so then return an error type, which caller can use to determine local network.
  3. Check if the IP is v4 or v6. if v6 then return some sort of "not implemented" error until #9 is implemented.

Ideally, I would like the logic of islocalip() to live in one place. Note: github.com/turbobytes/utils cannot have geoipdb as dependency because of cgo requirement.

coolparadox commented 7 years ago

We have been refining the options of good approaches for exporting islocalip() functionality. Sajal suggested the following additional approaches, both would serve:

  1. Export ⁠⁠⁠⁠IslocalIp()⁠⁠⁠⁠ from pulse repository. Geoipdb would import and use it.
  2. Move IslocalIp() to a sub package within geoipdb. This would allow minions to import it separately, avoiding cgo dependency.

In absence of stronger arguments, I'll go with the latter approach at it looks cleaner in the sense of dependency tracking.

coolparadox commented 7 years ago

@sajal is the possibility of tuning / disabling islocalip() detection on the fly an important requirement? I see this is exercised in test code only, but haven't found any "real world" usage of it.

sajal commented 7 years ago

The reason I wanted a way to tune it is for testing purpose. In the future I intend to have a mock server running locally which would have a local IP. And I want a way to bypass this test.

You could simply continue using islocalip() in pulse code which calls iputils.IsLocalIp() . For testing I could mock/overwrite the local islocalip() function.

coolparadox commented 7 years ago

+1

coolparadox commented 7 years ago

Branch detect_private_network contains subpackage iputils that exports IsLocalIP() for telling if an IP address is local to a network. This is detected by the same original logic as islocalip() from pulse repository, with extended CIDR comparison tables.

Moreover, LookupAsn() provides new error codes that meet above requirements.