Re: DNSBL and answer id check missing
On Sat, 27 Sep 2008 13:56:58 +0200, Diego d'Ambra wrote: > To me it seems that plugin DNSBL is using Net::DNS bgsend/bgread, but > is not checking the id of the reply received. > > If true this means that an attacker can white- or blacklist any email > by sending fake dns replies (only randomisation is source port). > Furthermore any other application on same machine also doing dns > lookup may end up using same source port and have it's replies being > mixed with those plugin DNSBL is waiting for. > > Spamassassin is also using Net::DNS bgsend/bgread, but does verify if > the dns answer id matched the request. > > Maybe Net::DNS requires the caller to do the validation, or did I > miss something? > > I'm working on a way to test this, but would love to hear others > opinion, before doing to much work for maybe nothing :-) I'm pretty sure you're probably right. The async version uses ParaDNS which does do the id checking. (we should probably do 0x20 checking too, but I think that's beyond my skill levels :-/ )
Re: DNSBL and answer id check missing
Hi Diego, On Sat, 27 Sep 2008 10:11:15 -0400 Matt Sergeant <[EMAIL PROTECTED]> wrote: > On Sat, 27 Sep 2008 13:56:58 +0200, Diego d'Ambra wrote: > > To me it seems that plugin DNSBL is using Net::DNS bgsend/bgread, but > > is not checking the id of the reply received. [...] > > I'm working on a way to test this, but would love to hear others > > opinion, before doing to much work for maybe nothing :-) > > I'm pretty sure you're probably right. The async version uses ParaDNS > which does do the id checking. What about a generic resolver module for qpsmtpd? We're doing quite a lot of lookups in the different plugins. $ grep -lr bgsend plugins/ plugins/dnsbl plugins/rhsbl plugins/dns_whitelist_soft plugins/uribl Plus the non async lookups in require_resolvable_fromhost and the core. Hanno
Re: DNSBL and answer id check missing
Hanno Hecker wrote: Hi Diego, On Sat, 27 Sep 2008 10:11:15 -0400 Matt Sergeant <[EMAIL PROTECTED]> wrote: On Sat, 27 Sep 2008 13:56:58 +0200, Diego d'Ambra wrote: To me it seems that plugin DNSBL is using Net::DNS bgsend/bgread, but is not checking the id of the reply received. [...] I'm working on a way to test this, but would love to hear others opinion, before doing to much work for maybe nothing :-) I'm pretty sure you're probably right. The async version uses ParaDNS which does do the id checking. What about a generic resolver module for qpsmtpd? We're doing quite a lot of lookups in the different plugins. $ grep -lr bgsend plugins/ plugins/dnsbl plugins/rhsbl plugins/dns_whitelist_soft plugins/uribl Plus the non async lookups in require_resolvable_fromhost and the core. Excellent idea and especially if the "other places" also need to be fixed. I'll gladly assist, but maybe somebody with greater knowledge in qpsmtpd's inner working should draft some minimum requirements? Best regards, Diego d'Ambra smime.p7s Description: S/MIME Cryptographic Signature
Re: DNSBL and answer id check missing
On Sat, 27 Sep 2008 13:56:58 +0200, Diego d'Ambra wrote: > To me it seems that plugin DNSBL is using Net::DNS bgsend/bgread, but > is not checking the id of the reply received. > > If true this means that an attacker can white- or blacklist any email Thinking more about this - since we don't do any "dnswl" type stuff, it doesn't seem that relevant. All the attacker can do is blacklist more emails, which given the timings surely he can only blacklist his own emails? Just a thought - wondering if this really needs to be fixed. Matt.
Re: DNSBL and answer id check missing
Matt Sergeant wrote: > On Sat, 27 Sep 2008 13:56:58 +0200, Diego d'Ambra wrote: >> To me it seems that plugin DNSBL is using Net::DNS bgsend/bgread, but >> is not checking the id of the reply received. >> >> If true this means that an attacker can white- or blacklist any email > > Thinking more about this - since we don't do any "dnswl" type stuff, it > doesn't seem that relevant. > > All the attacker can do is blacklist more emails, which given the > timings surely he can only blacklist his own emails? > > Just a thought - wondering if this really needs to be fixed. I've extended the async dnsbl plugin to do scoring. It occured to me a few days ago that DNSBLs with negative scores (DNSWLs) should be treated as a hit if they get a timeout or other failure. This has prompted me to comment about checking ids too. The stock one doesn't do scoring, and hence can't do DNSWL. You want my code? You might not like my logging conventions however ;-)
Re: DNSBL and answer id check missing
On Sat, 27 Sep 2008 20:09:37 -0400, Chris Lewis wrote: > I've extended the async dnsbl plugin to do scoring. It occured to me a > few days ago that DNSBLs with negative scores (DNSWLs) should be treated > as a hit if they get a timeout or other failure. This has prompted me > to comment about checking ids too. > > The stock one doesn't do scoring, and hence can't do DNSWL. You want my > code? You might not like my logging conventions however ;-) Like I said, the async version doesn't suffer from this bug, because it uses ParaDNS which checks the id fields (but doesn't do 0x20 checking - I wish I knew enough to be able to extend it to do so).
Re: DNSBL and answer id check missing
Matt Sergeant wrote: On Sat, 27 Sep 2008 13:56:58 +0200, Diego d'Ambra wrote: To me it seems that plugin DNSBL is using Net::DNS bgsend/bgread, but is not checking the id of the reply received. If true this means that an attacker can white- or blacklist any email Thinking more about this - since we don't do any "dnswl" type stuff, it doesn't seem that relevant. All the attacker can do is blacklist more emails, which given the timings surely he can only blacklist his own emails? Just a thought - wondering if this really needs to be fixed. I made a little change to DNSBL to ensure it randomize scr port and I see a major difference. ---snip--- my $res = new Net::DNS::Resolver; $res->tcp_timeout(30); $res->udp_timeout(30); $res->srcport(1024+int(rand(64511))); ---snip--- Anyway the worst is maybe that with current solution, results are very wrong - e.g. blacklisting a sender that shouldn't be, but is because no checking of received dns replies. The more busy your server is, the more it happens - on a server handling around 2 millions SMTP sessions per day, I see blacklisting of "valid" sender about every 30 seconds. I vote for a fix :-) Best regards, Diego d'Ambra smime.p7s Description: S/MIME Cryptographic Signature
Re: DNSBL and answer id check missing
Diego d'Ambra wrote: [...] I made a little change to DNSBL to ensure it randomize scr port and I see a major difference. Maybe latest version of Net::DNS does this (after Dan's widely report exploit). My box is an older Sarge solution, so others may not see same issue. But even with random src port it will happen - on my system probably around 100 times per day. Best regards, Diego d'Ambra smime.p7s Description: S/MIME Cryptographic Signature
Re: DNSBL and answer id check missing
On Sep 28, 2008, at 12:01 AM, Diego d'Ambra wrote: my $res = new Net::DNS::Resolver; $res->tcp_timeout(30); $res->udp_timeout(30); $res->srcport(1024+int(rand(64511))); Shouldn't this fix be in Net::DNS::Resolver? - ask -- http://develooper.com/ - http://askask.com/
Re: DNSBL and answer id check missing
On Sun, 28 Sep 2008, Ask Bj?rn Hansen wrote: On Sep 28, 2008, at 12:01 AM, Diego d'Ambra wrote: my $res = new Net::DNS::Resolver; $res->tcp_timeout(30); $res->udp_timeout(30); $res->srcport(1024+int(rand(64511))); Shouldn't this fix be in Net::DNS::Resolver? http://search.cpan.org/src/OLAF/Net-DNS-0.63/Changes Apparently some hjp caused that to be so in June 2007.
Re: DNSBL and answer id check missing
On Mon, 29 Sep 2008, Charlie Brady wrote: On Sun, 28 Sep 2008, Ask Bj?rn Hansen wrote: On Sep 28, 2008, at 12:01 AM, Diego d'Ambra wrote: > my $res = new Net::DNS::Resolver; > $res->tcp_timeout(30); > $res->udp_timeout(30); > $res->srcport(1024+int(rand(64511))); Shouldn't this fix be in Net::DNS::Resolver? http://search.cpan.org/src/OLAF/Net-DNS-0.63/Changes Apparently some hjp caused that to be so in June 2007. And from http://rt.cpan.org/Public/Bug/Display.html?id=23961: Patch made it into release 0.60
Re: DNSBL and answer id check missing
Charlie Brady wrote: > > On Sun, 28 Sep 2008, Ask Bj�rn Hansen wrote: > >> On Sep 28, 2008, at 12:01 AM, Diego d'Ambra wrote: >> >>> my $res = new Net::DNS::Resolver; >>> $res->tcp_timeout(30); >>> $res->udp_timeout(30); >>> $res->srcport(1024+int(rand(64511))); >> >> Shouldn't this fix be in Net::DNS::Resolver? > > http://search.cpan.org/src/OLAF/Net-DNS-0.63/Changes > > Apparently some hjp caused that to be so in June 2007. In 0.60 actually: Fix rt 23961 Randomized the ID on the queries. Thanks to "hjp" for reporting and suggesting a fix. The randomization of the src port is supposed to be handled by the setting the source port to "0" (default). Overriding the default or using persistent sockets may be problematic. Also see: http://www.potaroo.net/ietf/idref/draft-hubert-dns-anti-spoofing/
Re: DNSBL and answer id check missing
How about making qpsmtpd require Net::DNS 0.60 then? - ask
Re: DNSBL and answer id check missing
On 2008-09-30 01:30:44 -0700, Ask Bjørn Hansen wrote: > How about making qpsmtpd require Net::DNS 0.60 then? I've built in a work-around for the problem in older releases of Net::DNS. But I'm not sure if that still works - from a casual glance at the source I'd say no, and I'm not sure how it could ever have worked. Maybe the dnsbl plugin didn't use async at the time? hp -- _ | Peter J. Holzer| Openmoko has already embedded |_|_) | Sysadmin WSR | voting system. | | | [EMAIL PROTECTED] | Named "If you want it -- write it" __/ | http://www.hjp.at/ | -- Ilja O. on [EMAIL PROTECTED] signature.asc Description: Digital signature
PATCH: Re: DNSBL and answer id check missing
Diego d'Ambra wrote:
To me it seems that plugin DNSBL is using Net::DNS bgsend/bgread, but is
not checking the id of the reply received.
[...]
Attached suggested patch. Large part of it inspired by how Spamassassin
does it.
Changelog:
* Added source port randomisation to DNS queries
* Added id validation of DNS replies
I did notice a small (but important) change from the version I tested
and latest DNSBL taken from svn - $dom - it actually saves the request
and later matches the reply against it. So as I say in OP, maybe a storm
for nothing.
Anyway, I do think id checking is the best solution (if port and id are
random).
The 0x20 stuff is (also) beyond my skills ;-)
Best regards,
Diego d'Ambra
Index: plugins/dnsbl
===
--- plugins/dnsbl (revision 947)
+++ plugins/dnsbl (working copy)
@@ -54,24 +54,31 @@
my $res = new Net::DNS::Resolver;
$res->tcp_timeout(30);
$res->udp_timeout(30);
+ $res->srcport(1024+int(rand(64511)));
my $sel = IO::Select->new();
my $dom;
+ my %packet_ids;
for my $dnsbl (keys %dnsbl_zones) {
# fix to find A records, if the dnsbl_zones line has a second field
20/1/04 ++msp
$dom->{"$reversed_ip.$dnsbl"} = 1;
if (defined($dnsbl_zones{$dnsbl})) {
$self->log(LOGDEBUG, "Checking $reversed_ip.$dnsbl for A record in the
background");
- $sel->add($res->bgsend("$reversed_ip.$dnsbl"));
+ $packet = _dns_packet("$reversed_ip.$dnsbl");
+ $packet_ids{ _packet_id($packet) } = 1;
+ $sel->add( $res->bgsend($packet) );
} else {
$self->log(LOGDEBUG, "Checking $reversed_ip.$dnsbl for TXT record in the
background");
- $sel->add($res->bgsend("$reversed_ip.$dnsbl", "TXT"));
+ $packet = _dns_packet( "$reversed_ip.$dnsbl", "TXT" );
+ $packet_ids{ _packet_id($packet) } = 1;
+ $sel->add( $res->bgsend($packet) );
}
}
$self->qp->connection->notes('dnsbl_sockets', $sel);
$self->qp->connection->notes('dnsbl_domains', $dom);
+ $self->qp->connection->notes('dnsbl_ids', \%packet_ids);
return DECLINED;
}
@@ -92,6 +99,7 @@
my $sel = $conn->notes('dnsbl_sockets') or return "";
my $dom = $conn->notes('dnsbl_domains');
+ my $packet_ids = $conn->notes('dnsbl_ids');
my $remote_ip = $self->qp->connection->remote_ip;
my $result;
@@ -113,6 +121,11 @@
if ($query) {
my $a_record = 0;
+
+ #check reply is a valid
+ my $id = _packet_id($query);
+ next unless ( exists( $packet_ids->{$id} ) );
+
foreach my $rr ($query->answer) {
my $name = $rr->name;
$self->log(LOGDEBUG, "name $name");
@@ -206,6 +219,51 @@
return DECLINED;
}
+#_dns_packet: private function to create new Net::DNS::Packet object
+#arg0: str with host
+#arg1: str with type
+#ret0: ref to dns packet object
+sub _dns_packet
+{
+my $host = shift;#arg0
+my $type = shift;#arg1
+
+#set default
+$type = 'A' if ( !defined($type) );
+
+#create new dns packet
+my $packet = Net::DNS::Packet->new( $host, $type );
+
+#return packet object
+return ($packet);
+}
+
+#_packet_id: private function that calculate id to match dns replies.
+#arg0: ref to dns packet object
+#ret0: str with id
+sub _packet_id
+{
+my $packet = shift;#arg0
+
+#extract data from packet
+my $header= $packet->header;
+my $id= $header->id;
+my @questions = $packet->question;
+my $ques = $questions[0];
+
+#calculate id
+if ( defined $ques )
+{
+return join '/', $id, $ques->qname, $ques->qtype, $ques->qclass;
+}
+else
+{
+
+#just return (safe) id to support broken DNS servers that replies with
on question section
+return $id . "NO_QUESTION_IN_PACKET";
+}
+}
+
1;
=head1 Usage
smime.p7s
Description: S/MIME Cryptographic Signature
