Mailing List Archive

SA 3.3.1 and NetAddr::IP 4.034
Hi,

this morning Gentoo people liked to upgrade NetAddr::IP from 4.033 to 4.034.

People with "stable" systems (a Gentoo feature) actually runs SpamAssassin
3.3.1.

Soon after upgrading NetAddr::IP, a lint run reported these:

warn: netset: cannot include 0:0:0:0:0:0:0:1/128 as it has already been
included
warn: netset: cannot include 0:0:0:0:0:0:0:1/128 as it has already been
included
warn: netset: cannot include 0:0:0:0:0:0:0:1/128 as it has already been
included
warn: netset: cannot include 192.168.0.0/16 as it has already been included
warn: netset: cannot include 10.x.0.0/16 as it has already been included
warn: netset: cannot include x.x.x.x/32 as it has already been included
warn: netset: cannot include y.y.y.y/32 as it has already been included
warn: netset: cannot include z.z.z.z/32 as it has already been included
warn: netset: cannot include k.k.k.k/32 as it has already been included


Worse, every and each mail message I used to test SA, triggered the
ALL_TRUSTED rule!!!

I know NetAddr::IP recently got a re-design and probably SA have to cope
with this.

Do I have to place a bug report, or this is a known problem? I didn't find
any bug related to this, but maybe it's me...

Thank you,

Giampaolo
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
On Thursday 28 October 2010 14:29:41 Giampaolo Tomassoni wrote:
> this morning Gentoo people liked to upgrade NetAddr::IP from 4.033 to
> 4.034.
>
> People with "stable" systems (a Gentoo feature) actually runs SpamAssassin
> 3.3.1.
>
> Soon after upgrading NetAddr::IP, a lint run reported these:
>
> warn: netset: cannot include 0:0:0:0:0:0:0:1/128 as it has already been
> included
> [...]
> Worse, every and each mail message I used to test SA, triggered the
> ALL_TRUSTED rule!!!
>
> I know NetAddr::IP recently got a re-design and probably SA have to cope
> with this.
>
> Do I have to place a bug report, or this is a known problem? I didn't find
> any bug related to this, but maybe it's me...

Looking into it...

Mark
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
> Looking into it...

> > I know NetAddr::IP recently got a re-design and probably SA have to cope
> > with this.

Looks like a but in NetAddr::IP 4.034, it forgets to adjust the CIDR mask
when converting an IPv4 address to an IPv6 notation:

correct (NetAddr-IP-4.033):
$ perl -le 'use NetAddr::IP; print NetAddr::IP->new6("127.0.0.0/8")'
0:0:0:0:0:0:7F00:0/104

wrong (NetAddr-IP-4.034):
$ perl -le 'use NetAddr::IP; print NetAddr::IP->new6("127.0.0.0/8")'
0:0:0:0:0:0:7F00:0/8


Mark
RE: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
> > Looking into it...
>
> > > I know NetAddr::IP recently got a re-design and probably SA have to
> cope
> > > with this.
>
> Looks like a but in NetAddr::IP 4.034, it forgets to adjust the CIDR
> mask
> when converting an IPv4 address to an IPv6 notation:
>
> correct (NetAddr-IP-4.033):
> $ perl -le 'use NetAddr::IP; print NetAddr::IP->new6("127.0.0.0/8")'
> 0:0:0:0:0:0:7F00:0/104
>
> wrong (NetAddr-IP-4.034):
> $ perl -le 'use NetAddr::IP; print NetAddr::IP->new6("127.0.0.0/8")'
> 0:0:0:0:0:0:7F00:0/8

Mmmh.

In fact it seems they bobbed few lines from 4.033...

You sure new6 may be used with IPv4 address, huh?

If you confirm this, I can take care of reporting the bug to upstream.

Giampaolo


> Mark
RE: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
> Mmmh.
>
> In fact it seems they bobbed few lines from 4.033...
>
> You sure new6 may be used with IPv4 address, huh?
>
> If you confirm this, I can take care of reporting the bug to upstream.
>
> Giampaolo

Aha, you're too fast! ;)

Giampaolo


>
>
> > Mark
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
> > Looks like a but in NetAddr::IP 4.034, it forgets to adjust the CIDR
> > mask when converting an IPv4 address to an IPv6 notation:

s/but/BUG/ :)

> > correct (NetAddr-IP-4.033):
> > $ perl -le 'use NetAddr::IP; print NetAddr::IP->new6("127.0.0.0/8")'
> > 0:0:0:0:0:0:7F00:0/104
> >
> > wrong (NetAddr-IP-4.034):
> > $ perl -le 'use NetAddr::IP; print NetAddr::IP->new6("127.0.0.0/8")'
> > 0:0:0:0:0:0:7F00:0/8


> Mmmh.
> In fact it seems they bobbed few lines from 4.033...

The offending change is:

--- NetAddr-IP-4.033/Lite/Lite.pm 2010-09-29 19:35:16.000000000 +0200
+++ NetAddr-IP-4.034/Lite/Lite.pm 2010-10-26 01:20:12.000000000 +0200
@@ -696,3 +696,3 @@
if ($mask =~ /^(\d+)$/) {
- if (index($ip,':') < 0) { # is ipV4
+ if (! $isV6 && index($ip,':') < 0) { # is ipV4
if ($1 == 32) { # cidr 32

(which is in fact the only functional change in 4.034)


> You sure new6 may be used with IPv4 address, huh?

Yes. According to docs:

Methods
"->new([$addr, [ $mask|IPv6 ]])"
"->new6([$addr, [ $mask]])"
"->new_no([$addr, [ $mask]])"
"->new_from_aton($netaddr)"
The first two methods create a new address with the supplied
address in $addr and an optional netmask $mask
[...]
"->new6" marks the address as being in ipV6 address space even if
the format would suggest otherwise.

i.e. ->new6('1.2.3.4') will result in ::102:304

addresses submitted to ->new in ipV6 notation will
remain in that notation permanently. i.e.
->new('::1.2.3.4') will result in ::102:304
whereas new('1.2.3.4') would print out as 1.2.3.4

$addr can be almost anything that can be resolved to an IP address
in all the notations I have seen over time. It can optionally
contain the mask in CIDR notation.


> If you confirm this, I can take care of reporting the bug to upstream.

Please do so, thanks!

Mark
RE: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
> > If you confirm this, I can take care of reporting the bug to
> upstream.
>
> Please do so, thanks!

I'm too late: Steve Huff already did it...

See: https://rt.cpan.org/Public/Bug/Display.html?id=62521 .

Giampaolo

> Mark
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
On Thursday 28 October 2010 17:34:28 Giampaolo Tomassoni wrote:
> I'm too late: Steve Huff already did it...
> See: https://rt.cpan.org/Public/Bug/Display.html?id=62521 .

Perfect. Thank you guys.

| Thu Oct 28 19:41:16 2010 michael [...] bizsystems.com
fixed in release 4.035


Mark
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
> | Thu Oct 28 19:41:16 2010 michael [...] bizsystems.com
> fixed in release 4.035

Actually ... maybe not fixed ... investigating

Mark
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
On Friday 29 October 2010 16:35:31 Mark Martinec wrote:
> > | Thu Oct 28 19:41:16 2010 michael [...] bizsystems.com
> >
> > fixed in release 4.035
>
> Actually ... maybe not fixed ... investigating


NetAddr::IP 4.035:

correct, this case is now fixed:
$ perl -le 'use NetAddr::IP; print NetAddr::IP->new6("127.0.0.0/8")'
0:0:0:0:0:0:7F00:0/104

still incorrect:
$ perl -le 'use NetAddr::IP; print NetAddr::IP->new6("127/8")'
0:0:0:0:0:0:7F00:0/8



A workaround for SpamAssassin is to avoid shorthand IPv4 network
specifications, both in a config file (trusted/internal networks,
if any), as well as the hardwired one:

--- Mail/SpamAssassin/Conf.pm~
+++ Mail/SpamAssassin/Conf.pm
@@ -3972,7 +3972,7 @@
sub new_netset {
my ($self) = @_;
my $set = Mail::SpamAssassin::NetSet->new();
- $set->add_cidr ('127/8');
+ $set->add_cidr ('127.0.0.0/8');
$set->add_cidr ('::1');
return $set;
}


Mark
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
Patch for SpamAssassin Ports, rpm's and yum? or would it hurt to do this
to 3.3.2 before it gets released?


On 10/29/10 11:01 AM, Mark Martinec wrote:
> A workaround for SpamAssassin is to avoid shorthand IPv4 network
> specifications, both in a config file (trusted/internal networks,
> if any), as well as the hardwired one:
>
> --- Mail/SpamAssassin/Conf.pm~
> +++ Mail/SpamAssassin/Conf.pm
> @@ -3972,7 +3972,7 @@
> sub new_netset {
> my ($self) = @_;
> my $set = Mail::SpamAssassin::NetSet->new();
> - $set->add_cidr ('127/8');
> + $set->add_cidr ('127.0.0.0/8');
> $set->add_cidr ('::1');
> return $set;
> }

--
Michael Scheidell, CTO
o: 561-999-5000
d: 561-948-2259
ISN: 1259*1300
>*| *SECNAP Network Security Corporation

* Certified SNORT Integrator
* 2008-9 Hot Company Award Winner, World Executive Alliance
* Five-Star Partner Program 2009, VARBusiness
* Best in Email Security,2010: Network Products Guide
* King of Spam Filters, SC Magazine 2008

______________________________________________________________________
This email has been scanned and certified safe by SpammerTrap(r).
For Information please see http://www.secnap.com/products/spammertrap/
______________________________________________________________________
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
Giampaolo,

> > still incorrect:
> > $ perl -le 'use NetAddr::IP; print NetAddr::IP->new6("127/8")'
> > 0:0:0:0:0:0:7F00:0/8
>
> This seems way too ambiguos to me, isn't?

No, it isn't ambiguous, it is a perfectly valid syntax for an IPv4
network, although nowadays somewhat deprecated in favour for the
more obvious full quad notation.

> How could the NetAddr::IP module get you're instantiating an IPv6 net
> address, after all. That seems to me a valid IPv6 syntax, too...

No, it isn't a valid IPv6 address. The IPv6 addressing syntax
does not allow shortening in the same way as was allowed for IPv4.
An IPv6 address must specify full 128 bits, but allows for
compaction using a :: notation, e.g. 2001:db8::/32

> > A workaround for SpamAssassin is to avoid shorthand IPv4 network
> > specifications, both in a config file (trusted/internal networks,
> > if any), as well as the hardwired one:
> >
> > - $set->add_cidr ('127/8');
> > + $set->add_cidr ('127.0.0.0/8');
>
> Wow, such a beast was in Conf.pm, uhu?

Well, yes. Not nice, but not wrong either.


> So, SA 3.3.2 will be NetAddr::IP 4.035 -compatible, right?

There would still be an issue if a site config file uses a
shorthand IPv4 network syntax, so the NetAddr::IP would better
be fixed anyway.


Michael,

> > A workaround for SpamAssassin is to avoid shorthand IPv4 network
> > specifications, both in a config file (trusted/internal networks,
> > if any), as well as the hardwired one:

> Patch for SpamAssassin Ports, rpm's and yum? or would it hurt to do this
> to 3.3.2 before it gets released?

Sure, go ahead, can't hurt. The patch is now in the SA trunk.
Is it worth opening a ticket and putting it into the 3.3 branch too?

Mark
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
On 10/29/10 12:11 PM, Mark Martinec wrote:
> Sure, go ahead, can't hurt. The patch is now in the SA trunk.
> Is it worth opening a ticket and putting it into the 3.3 branch too?
>
> Mark
looks like Freebsd ports has an older version, so it should be ok.

pkg_info | grep NetAddr
p5-NetAddr-IP-4.02.8 Perl module for working with IP addresses and
blocks thereo


--
Michael Scheidell, CTO
o: 561-999-5000
d: 561-948-2259
ISN: 1259*1300
>*| *SECNAP Network Security Corporation

* Certified SNORT Integrator
* 2008-9 Hot Company Award Winner, World Executive Alliance
* Five-Star Partner Program 2009, VARBusiness
* Best in Email Security,2010: Network Products Guide
* King of Spam Filters, SC Magazine 2008

______________________________________________________________________
This email has been scanned and certified safe by SpammerTrap(r).
For Information please see http://www.secnap.com/products/spammertrap/
______________________________________________________________________
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
On 10/29/10 9:18 AM, Michael Scheidell wrote:
> On 10/29/10 12:11 PM, Mark Martinec wrote:
>> Sure, go ahead, can't hurt. The patch is now in the SA trunk.
>> Is it worth opening a ticket and putting it into the 3.3 branch too?
>>
>> Mark
> looks like Freebsd ports has an older version, so it should be ok.
>
> pkg_info | grep NetAddr
> p5-NetAddr-IP-4.02.8 Perl module for working with IP addresses and blocks thereo
>
>

You might be able to get better results with: Net-Patricia-1.18 which I released earlier this week.
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
On Tuesday November 2 2010 16:40:49 Rob McMahon wrote:
> The fix to NetAddr::IP seems to be as simple as
>--- NetAddr/IP/Lite.pm.bak Fri Oct 29 00:33:06 2010
> +++ NetAddr/IP/Lite.pm Tue Nov 2 15:18:05 2010
> @@ -740,7 +740,7 @@
> if ($mval == 128) { # cidr 128
> $mask = Ones;
> }
> - elsif ($ip =~ /^\d+\.\d+\.\d+\.\d+$/) { # corner case of ipV4
> with new6
> + elsif ($ip =~ /^\d+(\.\d+(\.\d+(\.\d+)?)?)?$/) { # corner
> case of ipV4 with new6
> $mask = shiftleft(Ones,32 -$mval);
> }
> elsif ($mask < 128) { # small cidr
>Having made that change SpamAssassin runs `make test' cleanly. Before
> it failed on four tests in cidrs.t, including
> ok !tryone "128.0.0.1", "127/8";
> ok !tryone "128.0.0.1", "127.0/16";
> ok !tryone "128.0.0.1", "127.0.0/24";
> and one in recreate.t (complaining about the "netset: cannot include"
> warnings).

Thanks. The bug has been reopened now:
https://rt.cpan.org/Public/Bug/Display.html?id=62521
you may want to attach your solution there.




Philip Prindeville wrote:

> You might be able to get better results with: Net-Patricia-1.18
> which I released earlier this week.

Thank you for bringing it to our attention. Now that you have added
the AF_INET6 support is became useful. I toyed with it for a while,
looks good and fast (and a bit memory fat compared to NetAddr::IP).
Lookups are more than 16 times faster for a set of 30 networks (our
real setup) using 750.000 real SMTP client's addresses collected from
our mailer log (using AF_INET6 database variant to store both inet
and inet6 addresses) - compared to the current SpamAssassin solution.

It does allow storing data with each network, which we need to
implement includes/excludes. One thing that we need to figure out
is how to test if one network (not a single IP address) is in
the trie - this is needed to check validity of config files,
but is not used later for mail checking, so it need not be
very fast.

One suggestion: currently it is not possible to store 0 and 1
as a data item associated with each net, because a 0 is treated
the same as undef and replaced by the key.

And the AF_NET6 argument to new() needs to be documented in a POD.

Thanks for your efforts!

Mark
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
> > You might be able to get better results with: Net-Patricia-1.18
> > which I released earlier this week.
>
> Thank you for bringing it to our attention. Now that you have added
> the AF_INET6 support is became useful. I toyed with it for a while,
> looks good and fast
> (and a bit memory fat compared to NetAddr::IP).

To be fair, more than half of the size is due to inclusion of a
Socket6 module, which may already be loaded for other reasons.


Btw, this could be more gracefully handled:

$ perl -e 'use Socket6; use Net::Patricia'
Prototype mismatch: sub main::AF_INET6: none vs ()
at /usr/local/lib/perl5/5.12.2/Exporter.pm line 64.

Mark
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
On 11/2/10 7:35 PM, Mark Martinec wrote:
>
> One suggestion: currently it is not possible to store 0 and 1
> as a data item associated with each net, because a 0 is treated
> the same as undef and replaced by the key.
>
> And the AF_NET6 argument to new() needs to be documented in a POD.
>
> Thanks for your efforts!
>
> Mark

Try the following patch. If it works for you, I'll rerelease as 1.19:

--- Patricia.pm.orig 2010-10-23 17:26:03.000000000 -0600
+++ Patricia.pm 2010-11-07 21:36:30.000000000 -0700
@@ -216,7 +216,7 @@

sub add {
my ($self, $ip, $bits, $data) = @_;
- $data ||= $bits ? "$ip/$bits" : $ip;
+ $data ||= defined $bits ? "$ip/$bits" : $ip;
my $packed = inet_pton(AF_INET6, $ip) || croak("invalid key");
$self->SUPER::_add(AF_INET6,$packed,(defined $bits ? $bits : 128), $data);
}
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
On 11/7/10 9:19 PM, Philip Prindeville wrote:
>
> Try the following patch. If it works for you, I'll rerelease as 1.19:

Actually, I released it as Net-Patricia-1.18_01
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
On 11/2/10 8:14 PM, Mark Martinec wrote:
> Btw, this could be more gracefully handled:
>
> $ perl -e 'use Socket6; use Net::Patricia'
> Prototype mismatch: sub main::AF_INET6: none vs ()
> at /usr/local/lib/perl5/5.12.2/Exporter.pm line 64.
>
> Mark

That's someone else's bug:

https://rt.cpan.org/Public/Bug/Display.html?id=32362

and represents a defect in Socket6. The work-around is to include Socket before Socket6.

-Philip
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
Philip,

> Try the following patch. If it works for you, I'll rerelease as 1.19:
> my ($self, $ip, $bits, $data) = @_;
> - $data ||= $bits ? "$ip/$bits" : $ip;
> + $data ||= defined $bits ? "$ip/$bits" : $ip;
> my $packed = inet_pton(AF_INET6, $ip) || croak("invalid key");

Hmm. What I had in mind was a:

$data = $bits ? "$ip/$bits" : $ip if !defined $data;

> > $ perl -e 'use Socket6; use Net::Patricia'
> > Prototype mismatch: sub main::AF_INET6: none vs ()
> > at /usr/local/lib/perl5/5.12.2/Exporter.pm line 64.
>
> That's someone else's bug:
> https://rt.cpan.org/Public/Bug/Display.html?id=32362
>
> and represents a defect in Socket6. The work-around is to include
> Socket before Socket6.

Thanks for the reference. I can live with that.


The Net::Patricia use (optional) is now in the SpamAssassin
svn trunk (on its way to become 3.4), see:
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6508

Thanks for pointing out its existence and for writing/maintaining it.


Btw, one more suggestion: it would be nice to be able to AVOID
loading the Net::CIDR::Lite module into memory (even when it is
available) if one does not need add_cidr() and remove_cidr()
routines. For services running in many instances (like child
processes in spamd) saving any memory is beneficial.


Mark
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
Philip,

Thanks for your off-list reply. Unfortunately I cannot
reply, as your mailer is refusing connections:

$ host -t mx redfish-solutions.com
redfish-solutions.com mail is handled by 10 mail.redfish-solutions.com.
$ telnet -s mail4.ijs.si mail.redfish-solutions.com 25
Trying 66.232.79.143...
Connected to mail.redfish-solutions.com.
554 mail.redfish-solutions.com ESMTP not accepting messages

(the message is now sitting in our queue, retrying periodically)

Mark
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
On 11/8/10 5:58 PM, Mark Martinec wrote:
> Philip,
>
> Thanks for your off-list reply. Unfortunately I cannot
> reply, as your mailer is refusing connections:
>
> $ host -t mx redfish-solutions.com
> redfish-solutions.com mail is handled by 10 mail.redfish-solutions.com.
> $ telnet -s mail4.ijs.si mail.redfish-solutions.com 25
> Trying 66.232.79.143...
> Connected to mail.redfish-solutions.com.
> 554 mail.redfish-solutions.com ESMTP not accepting messages
>
> (the message is now sitting in our queue, retrying periodically)
>
> Mark

Oh, sorry. Fixed.
Re: SA 3.3.1 and NetAddr::IP 4.034 [ In reply to ]
On Tuesday November 9 2010 09:29:57 Marcin Mirosław wrote:
>
> > Trying 66.232.79.143...
> > Connected to mail.redfish-solutions.com.
> > 554 mail.redfish-solutions.com ESMTP not accepting messages
> > (the message is now sitting in our queue, retrying periodically)
>
> Just from curiosity, You mail server treats 5xx as a temporary error?

http://tech.groups.yahoo.com/group/postfix-users/message/264310

| > I don't understand why Postfix does not bounce the message?! It's a
| > fatal error!
|
| This behavior is configurable, and the default is "safe".
|
| Wietse
|
| smtp_skip_5xx_greeting (default: yes)
| Skip SMTP servers that greet with a 5XX status code (go away, do not
| try again later).
|
| By default, the Postfix SMTP client moves on the next mail exchanger.
| Specify "smtp_skip_5xx_greeting = no" if Postfix should bounce the mail
| immediately. The default setting is incorrect, but it is what a lot of
| people expect to happen.


Mark