You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by bu...@bugzilla.spamassassin.org on 2011/05/25 09:22:22 UTC

[Bug 6599] New: Add Geo::IP support

https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

             Bug #: 6599
           Summary: Add Geo::IP support
           Product: Spamassassin
           Version: SVN Trunk (Latest Devel Version)
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: Plugins
        AssignedTo: dev@spamassassin.apache.org
        ReportedBy: hege@hege.li
    Classification: Unclassified


I would say IP::Country::Fast is somewhat obsolete. It hasn't been updated in
nearly in two years and it doesn't have any easy way of updating it's database.

Any objections on using Geo::IP as default for 3.4? Probably should have used
it since few years already. We can leave IP::Country::Fast as backup.

Are there any licensing issues? From http://www.maxmind.com/app/geoip_country:

"Under the license agreement, all advertising materials and documentation
mentioning features or use of this database must display the following
acknowledgment: "This product includes GeoLite data created by MaxMind,
available from http://www.maxmind.com/."

Only other alternative I could find right now is http://www.wipmania.com/, but
it doesn't seem to have a "standard" Perl module or efficient binary database
to download for that matter. Using network lookups would be serious overkill
imho.

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

--- Comment #8 from Henrik Krohns <he...@hege.li> 2011-05-26 07:50:30 UTC ---
(In reply to comment #7)
> Aside - Comment #3: "... have you seen a Perl module or such that updates
> itself
> without intervention?"
> 
> Actually, yes.  The CPAN module itself can do that, if called from cron with:
> 
>   "perl -MCPAN -e update"
> 
> If it finds an update for itself, it will install it.  It can autoconfigure
> itself too (and it did when I upgraded to perl 5.14.0 which came out in the
> past week).

Well, let me clarify myself, if someone else is confused also..

Of course you can cron script xxx to fetch xxx or update xxx. That still
requires user to actually "activate something".

IP::Country::Fast has no updates, other than if the maintainer decides to
update the module every few years and expect people to actually upgrade the
module also. Many people will not have the resources needed to run the database
generation script manually.

Geoip has been "de facto" for years and is in many distributions with automatic
package updates. It's also very simple to cron wget the database. Geoip also
supports IPv6 when we need it.

If anyone has some legimate concerns, especially on if and where to put the
licensing clauses etc, I'm open to hearing that. Other than that, I'll be
probably posting proposed patchset when I have time.

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

--- Comment #12 from Mark Martinec <Ma...@ijs.si> 2011-11-28 15:03:28 UTC ---
> The clause can go into the plugin's POD/manpage, I hope this suffices.

(referring to the MaxMind license clause)

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

--- Comment #6 from AXB <ax...@gmail.com> 2011-05-26 05:54:58 UTC ---
(In reply to comment #5)
> (In reply to comment #4)
> > And it seems debian actually packages the databases as packages, so it's
> > updated automatically.
> 
> Um, Geoip that is..

FTR: Centos provides
GeoIP
GeoIP-data 
in it's "extras" YUM repo

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

--- Comment #13 from Matt Dainty <ma...@bodgit-n-scarper.com> 2011-11-28 15:33:35 UTC ---
I completely agree on having two plugins, I just wrote a new one to keep it
separate as personally I'll never run it on a host that doesn't have Geo::IP
available. I also didn't want to mess with the SA package $OS provided.

Personally, IP::Country is no good to me going forward as it simply doesn't
have IPv6 data so I'd rather just move to Geo::IP entirely, but I wouldn't mind
if IP::Country was the fallback option. The data files then also get shared by
other things using the GeoIP C library.

If it's useful and could potentially be accepted, I'm happy to whip up a patch
to the current plugin that supports both backends, assuming any licensing issue
is resolved.

Oh, I forgot to mention, I think you'll need at least Geo::IP 1.39 to get the
IPv6 API methods, although I've been using 1.40, and that needs something like
1.4.7 of the C library.

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

--- Comment #4 from Henrik Krohns <he...@hege.li> 2011-05-26 05:50:18 UTC ---
And it seems debian actually packages the databases as packages, so it's
updated automatically.

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

Matt Dainty <ma...@bodgit-n-scarper.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |matt@bodgit-n-scarper.com

--- Comment #9 from Matt Dainty <ma...@bodgit-n-scarper.com> 2011-11-27 22:41:22 UTC ---
I think I just scratched this itch as I noticed the RelayCountry plugin wasn't
doing anything for IPv6 addresses.

I've put the plugin here:
https://github.com/bodgit/spamassassin-relaycountry-geoip

I've named it RelayCountry2 so it can live alongside the current plugin but
will update the same metadata so it's a case of enable one or the other.

I've replicated the behaviour of IP::Country in marking private addresses with
"**" and I've added similar rules for IPv6 based on the equivalent RFC.

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] [review] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

--- Comment #16 from Henrik Krohns <he...@hege.li> 2012-04-01 14:24:16 UTC ---
PS. From what I understand, since we aren't actually distributing anything
belonging to GeoIP, there is no need for any license clauses. Feel free to
correct.

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

Henrik Krohns <he...@hege.li> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hege@hege.li

--- Comment #3 from Henrik Krohns <he...@hege.li> 2011-05-26 05:43:17 UTC ---
(In reply to comment #2)
> Maxmind still requires manual updating.  The procedure for updating
> IP::Country::Fast is fairly straightforward.  I don't see any real reason to
> change.

Yes Maxmind requires manual updates, it's no surprise. 99% of things like this
needs manual updates, have you seen a Perl module or such that updates itself
without intervention? But Maxmind provides more accurate and easily available
files. It's much more likely that people have already installed geoip libraries
than IP::Country::Fast, it's not even packaged for debian.

What part of IP::Country::Fast do you consider fairly straightforward?

- The build process that takes 1-2GB of memory and 15+ minutes on 3GHz AMD
- Downloading hundreds of MBs of ripe data for it
- Assuming that your newly created database isn't overwritten by something
(it's in the Perl library path)

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

--- Comment #11 from Mark Martinec <Ma...@ijs.si> 2011-11-28 14:53:20 UTC ---
> Any objections on using Geo::IP as default for 3.4?
> Probably should have used it since few years already.

No objections from me.
The clause can go into the plugin's POD/manpage, I hope this suffices.


> We can leave IP::Country::Fast as backup.

Matt Dainty wrote:
> I think I just scratched this itch as I noticed the RelayCountry plugin
> wasn't doing anything for IPv6 addresses.
> I've put the plugin here:
>   https://github.com/bodgit/spamassassin-relaycountry-geoip
> I've named it RelayCountry2 so it can live alongside the current plugin but
> will update the same metadata so it's a case of enable one or the other.
> I've replicated the behaviour of IP::Country in marking private addresses
> with "**" and I've added similar rules for IPv6 based on the equivalent RFC.

I wouldn't like to see two variants of functionally equivalent plugins
and letting people scratch their heads, deciding which of them to use.
Either switch entirely to Geo::IP and ditch IP::Country::Fast, or
merge both into a single plugin and let it fallback to IP::Country::Fast
if Geo::IP is not installed.

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] [review] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

Henrik Krohns <he...@hege.li> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Add Geo::IP support         |[review] Add Geo::IP
                   |                            |support

--- Comment #15 from Henrik Krohns <he...@hege.li> 2012-04-01 10:06:04 UTC ---
Please review.

IPv6 is supported if Geo::IP 1.39+ is installed (which itself forces API 1.3.7,
unless someone skipped cpan make test..).

Sending        INSTALL
Sending        META.yml
Sending        Makefile.PL
Sending        lib/Mail/SpamAssassin/Plugin/RelayCountry.pm
Sending        lib/Mail/SpamAssassin/Util/DependencyInfo.pm
Sending        rules/init.pre
Transmitting file data ......
Committed revision 1308059.

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

--- Comment #14 from D. Stussy <so...@kd6lvw.ampr.org> 2011-11-29 02:18:27 UTC ---
Note:  Some of the IP->ASN databases return more than the two fields that the
ASN module uses -- some return up to 5 fields, and a country field is often one
of those.  Granted, we only check that for one of the servers in the path (last
external or first untrusted), but maybe we should add code to the ASN module to
identify the appropriate hop's country when this module fails to produce an
answer....

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] [review] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

Kevin A. McGrail <km...@pccc.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kmcgrail@pccc.com

--- Comment #17 from Kevin A. McGrail <km...@pccc.com> 2012-04-02 22:45:09 UTC ---
(In reply to comment #16)
> PS. From what I understand, since we aren't actually distributing anything
> belonging to GeoIP, there is no need for any license clauses. Feel free to
> correct.

I am +1 for using Geo::IP for 3.4.  The licensing looks fair with monthly
updates free with more fine tuned updates available free.  From my perspective,
it seems to be a fair free for some model that the project can support.

And agreed that our code should be fine license-wise.

Regards,
KAM

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

--- Comment #1 from AXB <ax...@gmail.com> 2011-05-25 08:03:32 UTC ---
(In reply to comment #0)
> I would say IP::Country::Fast is somewhat obsolete. It hasn't been updated in
> nearly in two years and it doesn't have any easy way of updating it's database.
> 
> Any objections on using Geo::IP as default for 3.4? Probably should have used
> it since few years already. We can leave IP::Country::Fast as backup.
> 
> Are there any licensing issues? From http://www.maxmind.com/app/geoip_country:
> 
> "Under the license agreement, all advertising materials and documentation
> mentioning features or use of this database must display the following
> acknowledgment: "This product includes GeoLite data created by MaxMind,
> available from http://www.maxmind.com/."
> 
> Only other alternative I could find right now is http://www.wipmania.com/, but
> it doesn't seem to have a "standard" Perl module or efficient binary database
> to download for that matter. Using network lookups would be serious overkill
> imho.

if license permits, I'm +1 for the MaxMind path.

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

--- Comment #5 from Henrik Krohns <he...@hege.li> 2011-05-26 05:50:41 UTC ---
(In reply to comment #4)
> And it seems debian actually packages the databases as packages, so it's
> updated automatically.

Um, Geoip that is..

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

Daniel J McDonald <da...@austinenergy.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dan.mcdonald@austinenergy.c
                   |                            |om

--- Comment #2 from Daniel J McDonald <da...@austinenergy.com> 2011-05-25 22:47:15 UTC ---
Maxmind still requires manual updating.  The procedure for updating
IP::Country::Fast is fairly straightforward.  I don't see any real reason to
change.

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

Benny Pedersen <me...@junc.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |me@junc.org

--- Comment #10 from Benny Pedersen <me...@junc.org> 2011-11-28 02:05:11 UTC ---
success in gentoo/funtoo now, it just need dev-perl/Geo-IP unstable to work

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] [review] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

Mark Martinec <Ma...@ijs.si> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|Undefined                   |3.4.0

--- Comment #18 from Mark Martinec <Ma...@ijs.si> 2012-04-24 18:21:19 UTC ---
Henrik Krohns 2012-04-01 10:06:04 UTC 
> Please review. Committed revision 1308059.

+1

This is already folded-in, works fine.
Closing.

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6599] Add Geo::IP support

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6599

D. Stussy <so...@kd6lvw.ampr.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |software+spamassassin@kd6lv
                   |                            |w.ampr.org

--- Comment #7 from D. Stussy <so...@kd6lvw.ampr.org> 2011-05-26 06:31:21 UTC ---
Aside - Comment #3: "... have you seen a Perl module or such that updates
itself
without intervention?"

Actually, yes.  The CPAN module itself can do that, if called from cron with:

  "perl -MCPAN -e update"

If it finds an update for itself, it will install it.  It can autoconfigure
itself too (and it did when I upgraded to perl 5.14.0 which came out in the
past week).

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.