Bug 7223

Summary: Net::DNS 1.01 breaks DnsResolver
Product: Spamassassin Reporter: Adam Sampson <ats-spamassassin>
Component: spamassassinAssignee: SpamAssassin Developer Mailing List <dev>
Status: RESOLVED FIXED    
Severity: blocker CC: kmcgrail, puppe, Ralf.Friedl
Priority: P1    
Version: 3.4.1   
Target Milestone: 3.4.2   
Hardware: PC   
OS: Linux   
Whiteboard:
Attachments: Patch to set RD to 1

Description Adam Sampson 2015-07-15 20:12:00 UTC
Created attachment 5314 [details]
Patch to set RD to 1

Net::DNS 1.01 has an undocumented, non-backwards compatible behaviour
change from 0.83: Net::DNS::Packet->new doesn't set the RD (recursion
desired) bit in new packets. As a result, SpamAssassin winds up sending
RBL queries without the RD bit set, and my network's caching namespace
rejects them, which results in lots of errors like this in
SpamAssassin's log:

info: dns: a likely matching query: 19367/IN/TXT/9.202.137.198.bl.spamcop.net

I've reported this as a bug to the Net::DNS maintainers and they say
it's deliberate; see my report and Dick Franks' response below. I've
attached a patch to make SpamAssassin explicitly set RD, which should
also work with older Net::DNS versions.

Thanks,
Adam

-----

Subject: [rt.cpan.org #105860] Net::DNS 1.01 creates new packets with RD=0
From: "Dick Franks via RT" <bug-Net-DNS@rt.cpan.org>
Date: Wed, 15 Jul 2015 15:35:36 -0400

<URL: https://rt.cpan.org/Ticket/Display.html?id=105860 >

On Tue Jul 14 11:57:57 2015, ats@offog.org wrote:
> Hi Net::DNS maintainers,
> 
> I've just upgraded Net::DNS from 0.83 to 1.01, and found suddenly
> SpamAssassin can't do outgoing RBL queries any more.
> 
> After a bit of debugging, this appears to be because
> Net::DNS::Packet::new sets the "recursion desired" bit by default in
> 0.83 ("$self->header->rd(1);"), but it doesn't do so in 1.01.
> 
> With 0.83:
> 
> $ perl -MNet::DNS -e '$p = new Net::DNS::Packet("example.org", "IN",
> "A"); print $p->string'
> ;; HEADER SECTION
> ;;      id = 35567
> ;;      qr = 0  aa = 0  tc = 0  rd = 1  opcode = QUERY
> ;;      ra = 0  z  = 0  ad = 0  cd = 0  rcode  = NOERROR
> ;;      qdcount = 1     ancount = 0     nscount = 0     arcount == 0
> ;;      do = 0
> 
> With 1.01:
> 
> $ perl -MNet::DNS -e '$p = new Net::DNS::Packet("example.org", "IN",
> "A"); print $p->string'
> ;; HEADER SECTION
> ;;      id = 64343
> ;;      qr = 0  aa = 0  tc = 0  rd = 0  opcode = QUERY
> ;;      ra = 0  z  = 0  ad = 0  cd = 0  rcode  = NOERROR
> ;;      qdcount = 1     ancount = 0     nscount = 0     arcount == 0
> ;;      do = 0
> 
> Since SpamAssassin doesn't explicitly set (or clear) rd, my recursive
> nameserver then rejects all its queries, resulting in lots of errors
> like this in SpamAssassin's log:
> info: dns: a likely matching query:
> 19367/IN/TXT/9.202.137.198.bl.spamcop.net
> ... and also resulting in lots of spam in my inbox!
>

Control of the RD flag is an essential requirement of the DNS protocol.
SpamAssassin has elected to do its own thing for making DNS queries.
Having chosen to do so, observing the protocol rules is very firmly
SpamAssassin's responsibility.


> This change isn't mentioned in CHANGES, so I assume it's accidental.
> I'd guess other software will probably break for the same reason, so
> it would probably be best to revert this change to maintain
> compatibility with older versions of Net::DNS?
>

Nowhere does the API specify the initial state of the RD or any other
flags.  This is internal behaviour and entirely within Net::DNS design
space.

The old behaviour gets in the way of both dynamic update and AXFR, and
for query packets (the only case where it might have been useful) was
ignored altogether.  Removing this entirely unnecessary complication was
a positive design move and not accidental.

resolver->send() sets RD in outbound query packets according to the
state of the resolver recurse flag which is 1 by default.  This renders
the state of RD in new query packets entirely irrelevant. The code line
that does this has not been changed since 0.75. Something similar is
present as far back as 0.47.

Query packets delivered to the network have RD set by default, as
demonstrated by the following code:

  use Net::DNS;

  my $resolver = new Net::DNS::Resolver();

  my $query = new Net::DNS::Packet( qw(example.org IN A) );

  my $reply = $resolver->send( $query );

  $query->print;

  $reply->print;


which produces:

  ;; HEADER SECTION
  ;;      id = 3038
  ;;      qr = 0  aa = 0  tc = 0  rd = 1  opcode = QUERY
  ;;      ra = 0  z  = 0  ad = 0  cd = 0  rcode  = NOERROR
  ;;      qdcount = 1     ancount = 0     nscount = 0     arcount == 0
  ;;      do = 0

  ;; QUESTION SECTION (1 record)
  ;; example.org. IN      A

  ;; ANSWER SECTION (0 records)

  ;; AUTHORITY SECTION (0 records)

  ;; ADDITIONAL SECTION (0 records)

  ;; Answer received from ::1 (45 bytes)
  ;; HEADER SECTION
  ;;      id = 3038
  ;;      qr = 1  aa = 0  tc = 0  rd = 1  opcode = QUERY
  ;;      ra = 1  z  = 0  ad = 0  cd = 0  rcode  = NOERROR
  ;;      qdcount = 1     ancount = 1     nscount = 0     arcount == 0
  ;;      do = 0

  ;; QUESTION SECTION (1 record)
  ;; example.org. IN      A

  ;; ANSWER SECTION (1 record)
  example.org.     5      IN      A       93.184.216.34

  ;; AUTHORITY SECTION (0 records)

  ;; ADDITIONAL SECTION (0 records)
Comment 1 Mark Martinec 2015-07-16 08:45:46 UTC
Sigh...

Thanks for attempting to bring it upstream!

To be applied (can't do it right now).
Comment 2 Kevin A. McGrail 2015-07-16 09:15:48 UTC
(In reply to Mark Martinec from comment #1)
> Sigh...
> 
> Thanks for attempting to bring it upstream!
> 
> To be applied (can't do it right now).

+1
Comment 3 Mark Martinec 2015-07-20 18:25:20 UTC
trunk:
  Sending lib/Mail/SpamAssassin/DnsResolver.pm
Committed revision 1691991.

spamassassin-3.4:
  Sending lib/Mail/SpamAssassin/DnsResolver.pm
Committed revision 1691992.
Comment 4 Mark Martinec 2016-06-23 14:25:11 UTC
> trunk:
>   Committed revision 1691991. 
> spamassassin-3.4:
>   Committed revision 1691992.

Fixed for 3.4.2 and later, closing.
Comment 5 Bill Cole 2017-10-11 01:56:06 UTC
*** Bug 7476 has been marked as a duplicate of this bug. ***