Bug #81133 for Net-DNS: Net::DNS::Packet hides that packets are corrupt, but still returns them

Tue Nov 13 05:29:23 2012 calle [...] lysator.liu.se - Ticket created

Subject:

Net::DNS::Packet hides that packets are corrupt, but still returns them

A nameserver returns a packet with a wildly incorrect AUTHORITY field in the header (in the tens of thousands). This makes Net::DNS::Packet try to read beyond the end of the packet, which obviously fails. The problem for us is that the eval{} in Net::DNS::Packet::_section eats the error, passing a corrupt packet up to the calling code (that is, one that claims to have, for example, 22875 Authority records while in reality having only one). We would very much like to have the ability to tell Net::DNS to pass errors like this on to us, either by re-throwing the exception or by returning an undef instead of a broken packet object.

Wed Nov 14 04:12:02 2012 rwfranks [...] acm.org - Correspondence added

From:

rwfranks [...] acm.org

On Tue Nov 13 05:29:23 2012, CDYBED wrote: Show quoted text

> A nameserver returns a packet with a wildly incorrect AUTHORITY field

This is a protocol violation at the nameserver. Show quoted text

> We would very much like to have the ability to tell Net::DNS to pass > errors like this on to us, either by re-throwing the exception or by > returning an undef instead of a broken packet object.

I assume you are using Net::DNS 0.68 and the corrupt packet was returned by resolver->send(). send() always returns a packet object if a response was received. This is, and always has been, required behaviour. Raising an exception would be an incompatible change to the published API, and is of little practical value; in-transit corruption should be detected as a checksum failure in the local UDP/IP stack and unlikely to ever be seen by the resolver. A corrupt reply packet will be partially decoded and any remaining content discarded. For that reason it is unsafe to assume that the actual number of RRs in each section matches the length declared in the header. $reply = $resolver->send( $query ); $header = $reply->header; $nscount = $header->nscount; # 22875 @auth = $reply->authority; $nsfound = scalar @auth; # 1

Wed Nov 14 04:12:04 2012 The RT System itself - Status changed from 'new' to 'open'

Wed Nov 14 04:41:03 2012 calle.dybedahl [...] init.se - Correspondence added

CC:	calle [...] init.se
Subject:	Re: [rt.cpan.org #81133] Net::DNS::Packet hides that packets are corrupt, but still returns them
Date:	Wed, 14 Nov 2012 10:40:47 +0100
To:	bug-Net-DNS [...] rt.cpan.org
From:	Calle Dybedahl <calle.dybedahl [...] init.se>

On 14 nov 2012, at 10:12, "Dick Franks via RT" <bug-Net-DNS@rt.cpan.org> wrote: Show quoted text

> On Tue Nov 13 05:29:23 2012, CDYBED wrote:

>> A nameserver returns a packet with a wildly incorrect AUTHORITY field

> > This is a protocol violation at the name server.

Yes, it is. There is no question about that. Show quoted text

> I assume you are using Net::DNS 0.68 and the corrupt packet was returned > by resolver->send().

Yes. Show quoted text

> Raising an exception would be an incompatible change to the published > API, and is of little practical value;

It would be of significant practical value to us, since what we're writing is a diagnostic tool (https://github.com/dotse/dnscheck). And I'm certainly not asking for a change to the default behavior, but the ability to set a flag (or something along those lines) saying that yes, I do indeed want to get any exceptions that may happen. I had a look at the code to see if this would be easy to implement, but from what I saw it'd be quite difficult (or at least a lot of work). Do you think it'd at least be possible to make Net::DNS not print error messages to stderr when this happens? Show quoted text

> A corrupt reply packet will be partially decoded and any remaining > content discarded. For that reason it is unsafe to assume that the > actual number of RRs in each section matches the length declared in the > header.

It would be nice if this was mentioned in the documentation. -- Calle Dybedahl calle@init.se -*- +46 703 - 970 612

Wed Nov 14 09:32:58 2012 rwfranks [...] acm.org - Correspondence added

From:

rwfranks [...] acm.org

On Wed Nov 14 04:41:03 2012, calle.dybedahl@init.se wrote: Show quoted text

> > On 14 nov 2012, at 10:12, "Dick Franks via RT" <bug-Net- > DNS@rt.cpan.org> wrote: >

> > On Tue Nov 13 05:29:23 2012, CDYBED wrote:

Show quoted text

> Do you think it'd at least be possible to make Net::DNS not print > error messages to stderr when this happens?

Error handling follows the de facto standard Unix/Linux model which developers have been using for 40 years. If you do not want to see the stderr, you need to redirect it to /dev/null or somewhere else. Show quoted text

> It would be nice if this was mentioned in the documentation.

Packet decoding has been reworked for 0.69 in response to bug reports and other reasons. The documentation now reads: $packet = new Net::DNS::Packet(\$data); $packet = new Net::DNS::Packet(\$data, 1); # set debugging If passed a reference to a scalar containing DNS packet data, a new packet object is created by decoding the data. The optional second boolean argument is used to enable debugging output. Returns undef if unable to create a packet object. Decoding errors, including data corruption and truncation, are collected in the $@ ($EVAL_ERROR) variable. ($packet, $length) = new Net::DNS::Packet(\$data); If called in array context, returns a packet object and the number of octets successfully decoded. If you wish to have a preview, try version 0.68_05 from CPAN.

Sun Dec 16 16:02:32 2012 NLNETLABS [...] cpan.org - Correspondence added

Hi Calle, I understand your particular case can be resolved by simply comparing nscount with the actual number of records in the authority sections (as illustrated by Dick). As to error reporting, current interface should be an improvement. Have you tried it yet. If you have a different approach in mind, we would like to hear. For now I am closing the ticket assuming your issues are resolved. Best regards, -- Willem

Sun Dec 16 16:02:33 2012 NLNETLABS [...] cpan.org - Status changed from 'open' to 'resolved'