Bug 239334 - mail/milter-greylist bring GeoIP support back, utilizing libmaxminddb implemented by Markus Wennrich
Summary: mail/milter-greylist bring GeoIP support back, utilizing libmaxminddb impleme...
Status: New
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-ports-bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-20 12:03 UTC by Harald Schmalzbauer
Modified: 2019-08-17 22:01 UTC (History)
2 users (show)

See Also:
bugzilla: maintainer-feedback? (m.tsatsenko)


Attachments
Add libmaxminddb (GeoIP2) support to mitler-greylist (129.02 KB, patch)
2019-07-20 12:03 UTC, Harald Schmalzbauer
no flags Details | Diff
Add libmaxminddb (GeoIP2) support to mitler-greylist and switch to autoreconf (8.84 KB, patch)
2019-07-22 15:00 UTC, Harald Schmalzbauer
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Harald Schmalzbauer 2019-07-20 12:03:10 UTC
Created attachment 205923 [details]
Add libmaxminddb (GeoIP2) support to mitler-greylist

In r490200, GeoIP support was authoritatively removed.  The reason was that Maxmind no longer provides geolocation data in the legacy format used by net/GeoIP, which milter-greylist uses. (https://svnweb.freebsd.org/ports?view=revision&revision=490211).
While there is a UPDATING record, this action has an unexpected and big impact for users already using GeoIP databases with milter-greylist.  It had to be left to their decision if they want to use outdated data, or abandon GeoIP support at all.  Especially in the milter-greylist case, I can't imagine anybody would consider removing GeoIP support was the correct solution. I guess they would live with outdated data until somebody skilled spends a significant amount of time to implement the successor library net/libmaxminddb, which we already have in the ports tree.

Thankfully, Markus Wennrich did that job.  His decision was to publish a complete fork on github: https://github.com/mwennrich/milter-greylist-geoip2

I simply created a patch from his commit, keeping the existing mail/milter-greylist port using the original sources, and adding support by unconditionally applying the patch enabling support for --with-libmaxminddb.

I'm not sure if it's wise to cary such a big patch containing ./configure modifications, which are done by autoconf.  Maybe time to utilize autoreconf?


Please find attached a workaround for making milter-greylist GeoIP aware again.
I haven't done extensive testing!

In case the maintainer doesn't accept that change, please revert the GeoIP removal from milter-greylist, until anybody else offers a different solution, which doesn't break production setups that badly.

Btw: Any hint appreciated what milter-greylist uses ftp/curl for!?
Comment 1 m.tsatsenko 2019-07-21 22:25:52 UTC
Hello,
Thanks for your efforts, but I do not thing committing such a big sized patch is  a good option. Clearly this should be done using autotools.
Comment 2 Harald Schmalzbauer 2019-07-22 14:59:33 UTC
Hello,

haven't included that change since up to now, there was a configure-patch in place, instead of autoreconf.
I'll add a diff for switching to autoreconf additionally, which reduces files/patch-libmaxminddb-support_from_GH-geoip2-fork significantly and removes files/patch-configure.

I have no idea about yacc and bison, hence no idea about the corresponding hunks in the diff, please check if it's correct what Markus Wennrich provides.

I also don't know if
MAXMINDDB_DESC=         Enable GeoIP1 + GeoIP2 Support
is correct.
I copied from a different port.  I assumed the author is right, but according to https://maxmind.github.io/libmaxminddb/ he isn't, if I interpret the description of "typedef struct MMDB_metadata_s" correctly, where we can read »The binary_format_major_version should always be 2«.  Please correct this before commiting.  I just randomly chose one port listed in https://svnweb.freebsd.org/ports?view=revision&revision=490211, which also needs finxing in case really is incorrect.

Again, if this patch isn't an immediate solution for you, please revert r490200.
The GeoIP option removal badly breaks existing setups, which must not happen for this kind of reason/event/problem (the discontinuance of upstream updates).

Thanks,
-harry
Comment 3 Harald Schmalzbauer 2019-07-22 15:00:34 UTC
Created attachment 205993 [details]
Add libmaxminddb (GeoIP2) support to mitler-greylist and switch to autoreconf
Comment 4 Harald Schmalzbauer 2019-07-22 15:34:49 UTC
find -x . -name Makefile -exec grep -H MAXMINDDB_DESC {} \; reveals that
./dns/gdnsd3 and ./dns/gdnsd2 use this most likely incorrect description.

CCing zi@
Comment 5 m.tsatsenko 2019-07-25 22:43:57 UTC
This patch is much better, I will have a closer look at it. Thanks!
Comment 6 m.tsatsenko 2019-08-17 22:01:07 UTC
Seems fine, but still need to run QA.
By the way did you have a chance to do some run-time testing?