Packing overflow / compression bug

[armon]

In some cases, I'm using the client to act like a recursive DNS server. However, with some queries, I get the following:

dns: overflow packing uint32

It seems to have to do with something in the response body. I have a minimal test case here: https://gist.github.com/armon/8248769

Once you run that server, use the following dig command:

time dig @127.0.0.1 -p 8600 +qr apple.com ANY

Inside the test case, there is a line with a comment to toggle the value. The compression flag on the response seems to change if this works. With compression enabled, the response goes through fine. Without compression, the packing error happens.

[andrewtj]

Might be barking up the wrong tree here but I think the root cause of this might be the packed length of RRs being miscalculated. In this case the length of the NAPTR record is off by a few bytes as it doesn't include the length bytes for Replacements labels. In types.go if you change:

func (rr *NAPTR) len() int {
        return rr.Hdr.len() + 4 + len(rr.Flags) + len(rr.Service) +
                len(rr.Regexp) + len(rr.Replacement) + 1
}

To:

func (rr *NAPTR) len() int {
        return rr.Hdr.len() + 4 + len(rr.Flags) + len(rr.Service) +
                len(rr.Regexp) + len(rr.Replacement) + CountLabel(rr.Replacement)
}

It should pack okay without compression. I'll take another pass at this tomorrow if @miekg doesn't beat me to it.

[andrewtj]

I was barking up the wrong tree! The length bytes would have gone in place of periods in Replacement.

I think I know why swapping in CountLabel() paved over the problem though. The wire format for Flags, Service and Regexp is character-string so there should be an extra byte for each.

[miekg]

The packLen() function returns the wrong length, which in turns means that some (NAPTR seems an obvious choice) RR returns the wrong length. packLen() is kind of a premature optimization in that tries to get the package length without actually packing it. Doing something ridiculous in msg.go L1347:

msg = make([]byte, dns.packLen()+20) 

Fixes the problem.

Actually tracking down which RR is doing this is quite annoying, but I'll check,

[miekg]

@andrewtj you're comment was almost spot on, the string need a lenght byte and the NAPTR's len function didn't take this into account. Fixed this and added a test.

The test program now works OK even with compress=false