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!?
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.
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
Created attachment 205993 [details] Add libmaxminddb (GeoIP2) support to mitler-greylist and switch to autoreconf
find -x . -name Makefile -exec grep -H MAXMINDDB_DESC {} \; reveals that ./dns/gdnsd3 and ./dns/gdnsd2 use this most likely incorrect description. CCing zi@
This patch is much better, I will have a closer look at it. Thanks!
Seems fine, but still need to run QA. By the way did you have a chance to do some run-time testing?
(In reply to m.tsatsenko from comment #6) Yes, I'm running it in production since I posted the diff without attracting attention, utilizing geoipdb "/usr/local/share/GeoIP/GeoLite2-Country.mmdb". It's the only change I made in the config. Not very high traffic on the system: About 200 greylisted not coming back each day, about 100 skipped due to valid SPF or whitelisted, and only 1 or 2 per day really delayed. But in summary, this GeoIP2 version was invoked about 8000 times during the last 4 weeks.
Created attachment 206837 [details] patch Please see the patch attached and confirm it works for you.
No submitter feedback. Good news we do not really need it.
Created attachment 208287 [details] the patch - Bring geoip support back - Do not pass SENDMAIL_FLAGS to the build. It was an old workaround apparently not needed anymore - Pet portlint - Bump PORTREVISION QA poudriere 113amd64 - OK portlint - OK
@Maintainer 1) Might the OPTION name better be GEOIP as 1) that what it was pre-removal 2) Describes the feature, not the implementation. I believe this is the best UX for users and for any future changes. 2) Can/should the UPDATING entry now be removed? I imagine it should be.
The UPDATING entry applied to a couple dozen ports.
(In reply to Kubilay Kocak from comment #11) Excellent. My thanks to all for fixing this breakage of production stuff. :)
Preserving original OPTION name makes perfect sense for me. I will update the patch tomorrow
(In reply to m.tsatsenko from comment #14) ping? :)
Created attachment 212437 [details] the patch - Preserve original GEOIP OPTION name. No functional changes 121amd64 still builds OK
Comment on attachment 212437 [details] the patch This patch seems to be travelling back in time (PORTREVISION going from 5 to 4!) Please check and provide an updated patch.
The GeoIP2 patch has been merged upstream > http://ftp.espci.fr/pub/milter-greylist/milter-greylist-4.6.3.tgz > > ChangeLog since 4.6.2 > Add support for GeoIP2 (Markus Wennrich) > Build fixes for conflicting ns_type in SPF and NSupdate code > Quiet build warnings > Missing bits to make rawfrom usable (Marcus Schopen) > Fix crash when GeoIP for IPv6 is not configured (Paul Howarth) > Report queueId for maxpeek overflow warnings (Attila Bruncsak) > Sendmail access.db usage documentation (Johann Klasek)
That means extra patches are not needed anymore. I was trying to make them obsolete, but wasnt able to without uploading a new one. I will have a look at the latest release this weekend.
Created attachment 216919 [details] the patch - Update to 4.6.3 and bring back geoip support - Add SmapAssassin and DRAC support - Do not pass sendmail flags from make.conf. This is a part of very old workaround which is not needed anymore. Also it is know to cause build issues in some cases. QA: portlint - No errors poudrierre 121amd64 - OK
A commit references this bug: Author: brnrd Date: Thu Jan 7 10:04:50 UTC 2021 New revision: 560697 URL: https://svnweb.freebsd.org/changeset/ports/560697 Log: mail/milter-greylist: Update to 4.6.3 PR: 239334 Submitted by: <m tsatsenko gmail com> (maintainer) Changes: head/mail/milter-greylist/Makefile head/mail/milter-greylist/distinfo