Bug 1220698 - geoiplookup should support IPv6 transparently
Summary: geoiplookup should support IPv6 transparently
Keywords:
Status: NEW
Alias: None
Product: Fedora
Classification: Fedora
Component: GeoIP
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Paul Howarth
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: dualstack 1220694
TreeView+ depends on / blocked
 
Reported: 2015-05-12 08:48 UTC by Nikos Mavrogiannopoulos
Modified: 2018-04-23 23:10 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Enhancement
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-05-12 13:41:21 UTC
Type: Bug


Attachments (Terms of Use)

Description Nikos Mavrogiannopoulos 2015-05-12 08:48:13 UTC
Description of problem:

The current system of using different software (geoiplookup vs geoiplookup6) for different versions of the IP stack is broken.  As the IPv6 transition moves ahead its unreasonable to expect the user to do the IP address parsing and decide to use the appropriate tool for the address. 

geoiplookup should work like traceroute, ssh or all other applications supporting both protocol versions by default.

How reproducible:
Always

Steps to Reproduce:
1. geoiplookup 2a03:2880:20:4f06:face:b00c:0:1

 
Actual results:
GeoIP Country Edition: can't resolve hostname ( 2a03:2880:20:4f06:face:b00c:0:1 )

Expected results:
GeoIP Country V6 Edition: IE, Ireland
GeoIP ASNum V6 Edition: AS32934 Facebook, Inc.

Comment 1 Paul Howarth 2015-05-12 10:59:36 UTC
Raised upstream:

https://github.com/maxmind/geoip-api-c/issues/61

Comment 2 Nikos Mavrogiannopoulos 2015-05-12 13:18:18 UTC
thank you

Comment 3 Paul Howarth 2015-05-12 13:41:21 UTC
So, upstream's view is that GeoIP is a legacy library for accessing a legacy database and is unlikely to be updated. The "modern" approach is to use the GeoIP2 databases with the new libmaxminddb library and the accompanying mmdblookup tool, which is already protocol-transparent.

So I'm going to say this one won't be fixed, and someone needs to submit the GeoIP2 packages for review instead.

Comment 4 Nikos Mavrogiannopoulos 2015-05-12 14:22:51 UTC
That doesn't resolve the bug. Let's close it when the new geoip2 packages are available in fedora.

Comment 5 Paul Howarth 2015-05-12 14:48:18 UTC
It should probably be a bug on the "distribution" component then, rather than GeoIP.

Comment 6 Nikos Mavrogiannopoulos 2015-05-12 14:57:16 UTC
Well, if geoip isn't a deprecated component in Fedora the bug is still valid. If that component was already orphan or planned to be dropped from fedora it would make sense to close the bug. Otherwise it looks like we are going to live with it, and if we do, the bugs will not disappear.

Comment 7 Paul Howarth 2015-05-12 15:13:27 UTC
I would personally be happy to drop it but there are still plenty of things depending on it in Fedora and I'm not aware of anyone working towards migrating things to the new library (and introducing that library in Fedora).

So as neither upstream nor downstream seems to be doing anything to change things, I suspect this issue will linger on for quite some time.

Comment 8 Philip Prindeville 2015-05-12 15:29:29 UTC
(In reply to Paul Howarth from comment #7)
> I would personally be happy to drop it but there are still plenty of things
> depending on it in Fedora and I'm not aware of anyone working towards
> migrating things to the new library (and introducing that library in Fedora).

Not just in Fedora, but upstream in mod_geoip (Apache), mod_geoip (ProFTPd), URILocalBL (spamassassin), Wireshark, etc.

Comment 9 Nikos Mavrogiannopoulos 2015-05-12 15:36:24 UTC
I understand but from the looks of it libmaxminddb is far from a drop in replacement of libgeoip. From a quick glimpse the latter for example allows for a default database for applications to use while the former requires each application to specify their own database. Even the tool mmdblookup requires the user to specify the geolocation database files. 

So if my understanding is correct we are going to live with geoip for quite some time.

Comment 10 Nikos Mavrogiannopoulos 2015-05-19 09:04:18 UTC
What we could do is to only keep the libraries in this package and remove the command line applications. The ipcalc in f23 will provide a part of their functionality. Are there packages depending on them?

Comment 11 Paul Howarth 2015-05-19 09:08:31 UTC
(In reply to Nikos Mavrogiannopoulos from comment #10)
> What we could do is to only keep the libraries in this package and remove
> the command line applications. The ipcalc in f23 will provide a part of
> their functionality. Are there packages depending on them?

Not that I know of. Just the libraries in the vast majority of cases anyway.

Comment 12 Greg Oschwald 2015-05-19 14:02:27 UTC
As the upstream contact on this, I don't think we are opposed to accepting a reasonable patch to do this, but it is something that we are unlikely to do ourselves. The code is in maintenance mode and our development efforts are focused on the new format.

Comment 13 Nikos Mavrogiannopoulos 2015-05-19 14:19:15 UTC
(In reply to Greg Oschwald from comment #12)
> As the upstream contact on this, I don't think we are opposed to accepting a
> reasonable patch to do this, but it is something that we are unlikely to do
> ourselves. The code is in maintenance mode and our development efforts are
> focused on the new format.

Two questions arise with the new format.
1. Why can't the current API handle the new format? I mean a library API is independent of the database storage (or should be), otherwise in the next revision of the format we'll again have the same issues.
2. As I see in the new library, there is nothing equivalent to _GeoIP_setup_dbfilename() meaning each and every program using it has to figure out where the database is stored, i.e., around 500 lines of code as seen in wireshark. For a distribution like Fedora where things are pre-packaged and paths are standardized, this is a no-go. We cannot have tools that require the user to provide a full database path by default every time. It would be reasonable to have default initialization which uses the shipped or downloaded database, by expecting it in a preconfigured path.

Comment 14 Greg Oschwald 2015-05-19 14:29:22 UTC
1. The new format is completely different and the old API is filled with cruft and poor design decisions that accumulated over time.

2. We preferred to keep the API simple and not build in a lot of decisions such as where the files are stored as this varies between platforms and use cases. Also, the new API is database agnostic. It is not specific to GeoIP Country or City and does not give them special treatment as the old API did. We briefly considered a GeoIP2 C API in addition to the format API, but there hasn't been much demand.

Comment 15 Jan Safranek 2015-06-30 14:37:18 UTC
If I understand it correctly, while 'legacy' GeoIP library is not actively developed, it is still maintained (bugs are being fixed) and appropriate databases are regularly released.

If it is so, we do not need to port anything to GeoIPv2 APIs and databases, v1 works well and hopefully will keep working.

Regarding geoiplookup, can't we rename it to /bin/geoiplookup4 and create simple wrapper script /bin/geoiplookup which calls v4/v6 tool?

Comment 16 Greg Oschwald 2015-06-30 14:58:32 UTC
Your understanding is correct. We would also consider accepting a reasonable patch to support using geoiplookup for IPv6 as long as it wasn't too invasive. I don't think it would be terribly hard to implement.


Note You need to log in before you can comment on or make changes to this bug.