You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spamassassin.apache.org by mm...@apache.org on 2013/02/14 18:28:05 UTC

svn commit: r1446278 - in /spamassassin/trunk/lib/Mail/SpamAssassin: AsyncLoop.pm Conf.pm DnsResolver.pm

Author: mmartinec
Date: Thu Feb 14 17:28:05 2013
New Revision: 1446278

URL: http://svn.apache.org/r1446278
Log:
- change description of a dns_options EDNS option, add alias EDNS0
- ignore trailing dot in a domain name which sometimes still creep in
- change word 'response' -> 'reply' in DNS debugging messages (RFC 1035)
- test for truncated DNS reply, issue an info-level warning
- debugging: trying to track down a bug in Net::DNS which can
  return a DNS packet with an empty question section

Submitted by: Mark

Modified:
    spamassassin/trunk/lib/Mail/SpamAssassin/AsyncLoop.pm
    spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm
    spamassassin/trunk/lib/Mail/SpamAssassin/DnsResolver.pm

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/AsyncLoop.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/AsyncLoop.pm?rev=1446278&r1=1446277&r2=1446278&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/AsyncLoop.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/AsyncLoop.pm Thu Feb 14 17:28:05 2013
@@ -281,6 +281,7 @@ filled-in with a query ID.
 sub bgsend_and_start_lookup {
   my($self, $domain, $type, $class, $ent, $cb, %options) = @_;
   $ent = {}  if !$ent;
+  $domain =~ s/\.+\z//s;  # strip trailing dots, these sometimes still sneak in
   $ent->{id} = undef;
   $ent->{query_type} = $type;
   $ent->{query_domain} = $domain;

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm?rev=1446278&r1=1446277&r2=1446278&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm Thu Feb 14 17:28:05 2013
@@ -1716,25 +1716,31 @@ of seconds will tell SpamAssassin how of
 
 =item dns_options opts   (default: empty)
 
-Provides a (whitespace or comma -separated) list of options applying
-to DNS resolving. Available options are 'rotate', 'dns0x20' and 'edns'
-(without quotes). Option name may be negated by prepending a 'no' (e.g.
-'norotate', 'noEDNS') to counteract previously enabled option. Option
-names are not case-sensitive.
+Provides a (whitespace or comma -separated) list of options applying to
+DNS resolving. Available options are (without quotes): 'rotate', 'dns0x20'
+and 'edns' (or 'edns0'). Option name may be negated by prepending a 'no'
+(e.g. 'norotate', 'noEDNS') to counteract previously enabled option.
+Option names are not case-sensitive.
 
 The last setting in configuration files prevails. By default options
 'rotate', 'dns0x20' and 'edns' are disabled.
 
-Option 'edns' may take a value which specifies a requestor's UDP payload
-size according to EDNS0 specifications (RFC 2671bis draft), e.g. edns=4096.
-When the option is enabled but a value is not provided, a conservative default
-of 1240 bytes is implied. It is recommended to enable 'edns' when using a
-local recursive DNS server which supports EDNS0 (like most modern DNS servers
-do). This may avoid a need for a DNS query to fail-over to a TCP query when
-an answer DNS UDP packet would exceed 512 bytes. The option should remain
-disabled when a recursive DNS resolver is only reachable through some
-old-fashioned firewall which cannot cope with DNS UDP packets longer than
-512 bytes or which discards IP fragments.
+Option 'edns' (or 'edsn0') may take a value which specifies a requestor's
+acceptable UDP payload size according to EDNS0 specifications (RFC 2671bis
+draft), e.g. edns=4096. In absence of an 'edns' option a traditional implied
+UDP payload size is 512 bytes. When the option is specified but a value
+is not provided, a conservative default of 1240 bytes is implied. It is
+recommended to enable 'edns' when using a local recursive DNS server which
+supports EDNS0 (like most modern DNS servers do), a suitable setting in
+this case is edns=4096. Allowing packets larger than 512 bytes can avoid
+truncation of answer resource records in large DNS responses (like in TXT
+records of some SPF and DKIM responses, or when an unreasonable number of
+A records is published by some domain). The option should remain disabled
+when a recursive DNS server is only reachable through some old-fashioned
+firewall which bans DNS UDP packets larger than 512 bytes. A suitable value
+when a non-local recursive DNS server is used and a firewall allows EDNS0
+but blocks fragmented IP packets is perhaps 1240 bytes, allowing a DNS UDP
+packet to fit within a single IP packet in most cases.
 
 Option 'rotate' causes SpamAssassin to choose a DNS server at random
 from all servers listed in C</etc/resolv.conf> every 'dns_test_interval'
@@ -1761,16 +1767,18 @@ do not work for no apparent reason.
       my ($self, $key, $value, $line) = @_;
       foreach my $option (split (/[\s,]+/, lc $value)) {
         local($1,$2);
-        if ($option =~ /^no(rotate|dns0x20|edns)\z/) {
+        if ($option =~ /^no(rotate|dns0x20)\z/) {
+          $self->{dns_options}->{$1} = 0;
+        } elsif ($option =~ /^no(edns)0?\z/) {
           $self->{dns_options}->{$1} = 0;
         } elsif ($option =~ /^(rotate|dns0x20)\z/) {
           $self->{dns_options}->{$1} = 1;
-        } elsif ($option =~ /^(edns) (?: = (\d+) )? \z/x) {
+        } elsif ($option =~ /^(edns)0? (?: = (\d+) )? \z/x) {
           # RFC 2671 bis - EDNS0, value is a requestor's UDP payload size
           # defaults to some UDP packet size likely to fit into a single packet
           # which is more likely to pass firewalls which choke on IP fragments.
           # RFC 2460 min MTU is 1280 for IPv6, minus 40 bytes for basic header
-          $self->{dns_options}->{$1} = $2 ? $2 : 1240;
+          $self->{dns_options}->{$1} = $2 || 1240;
         } else {
           return $INVALID_VALUE;
         }

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/DnsResolver.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/DnsResolver.pm?rev=1446278&r1=1446277&r2=1446278&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/DnsResolver.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/DnsResolver.pm Thu Feb 14 17:28:05 2013
@@ -242,7 +242,7 @@ sub available_nameservers {
       } elsif ($addr =~ /:.*:/) {
         push(@filtered_addr_port, $_)  unless $self->{force_ipv4};
       } else {
-        warn "Unrecognized DNS specification: $_";
+        warn "Unrecognized DNS server specification: $_";
       }
     }
     if (@filtered_addr_port < @{$self->{available_dns_servers}}) {
@@ -355,7 +355,7 @@ sub connect_sock {
 
   if ($self->{sock}) {
     $self->{sock}->close()
-      or warn "connect_sock: error closing socket $self->{sock}: $!";
+      or info("connect_sock: error closing socket %s: %s", $self->{sock}, $!);
     $self->{sock} = undef;
   }
   my $sock;
@@ -641,7 +641,7 @@ sub _packet_id {
 
 =item $id = $res->bgsend($domain, $type, $class, $cb)
 
-Quite similar to C<Net::DNS::Resolver::bgsend>, except that when a response
+Quite similar to C<Net::DNS::Resolver::bgsend>, except that when a reply
 packet eventually arrives, and C<poll_responses> is called, the callback
 sub reference C<$cb> will be called.
 
@@ -709,7 +709,7 @@ sub bgsend {
 
 =item $nfound = $res->poll_responses()
 
-See if there are any C<bgsend> response packets ready, and return
+See if there are any C<bgsend> reply packets ready, and return
 the number of such packets delivered to their callbacks.
 
 =cut
@@ -748,27 +748,32 @@ sub poll_responses {
     my $packet = $self->{res}->bgread($self->{sock});
 
     if (!$packet) {
-      dbg("dns: no packet! err: %s", $self->{res}->errorstring);
+      info("dns: bad dns reply: %s", $self->{res}->errorstring);
     } else {
       my $header = $packet->header;
       if (!$header) {
-        info("dns: dns response is missing a header section");
+        info("dns: dns reply is missing a header section");
       } else {
         my $rcode = $header->rcode;
         my $packet_id = $header->id;
+        my $id = $self->_packet_id($packet);
+
         if ($rcode eq 'NOERROR') {  # success
           # NOERROR, may or may not have answer records
-          dbg("dns: dns response %s is OK, %d answer records",
+          dbg("dns: dns reply %s is OK, %d answer records",
               $packet_id, $header->ancount);
+          if ($header->tc) {  # truncation flag turned on
+            my $edns = $self->{conf}->{dns_options}->{edns} || 512;
+            info("dns: reply to %s truncated (%s), %d answer records", $id,
+                 $edns == 512 ? "EDNS off" : "EDNS $edns bytes",
+                 $header->ancount);
+          }
         } else {
           # some failure, e.g. NXDOMAIN, SERVFAIL, FORMERR, REFUSED, ...
           # btw, one reason for SERVFAIL is an RR signature failure in DNSSEC
           # NOTE: qname is encoded in RFC 1035 zone format, decode it
-          dbg("dns: dns response %s, %s, %s", $packet_id, $rcode,
-              join(', ', map(join('/', fmt_dns_question_entry($_)),
-                             $packet->question)));
+          dbg("dns: dns reply to %s: %s", $id, $rcode);
         }
-        my $id = $self->_packet_id($packet);
 
         # A hash lookup: the id must match exactly (case-sensitively).
         # The domain name part of the id was lowercased if dns0x20 is off,