EDNS decoder issue?

[tarko]

hey,

EDNS decoder seems to be bit optimistic about the OPT RRs, go-dns-collector produces a lot of log like this:

Mar 27 13:44:07 asd.estpak.ee go-dnscollector[119352]: ERROR: 2022/03/27 13:44:07.290620 procesor dnstap parser - malformed edns options in DNS packet: edns, packet contained too many OPT RRs - {{INET UDP - - 45.57.8.1 53 - -} {REPLY [123 151 132 0 0 1 0 2 0 2 0 5 15 111 99 99 45 48 45 49 53 48 48 45 49 53 48 49 1 49 6 110 102 108 120 115 111 3 110 101 116 0 0 1 0 1 192 12 0 1 0 1 0 0 0 30 0 4 45 57 36 151 192 12 0 1 0 1 0 0 0 30 0 4 45 57 36 148 192 30 0 2 0 1 0 1 81 128 0 7 1 101 2 110 115 192 30 192 30 0 2 0 1 0 1 81 128 0 4 1 102 192 92 0 0 41 4 176 0 0 0 0 0 0 192 90 0 1 0 1 0 1 81 128 0 4 45 57 8 1 192 90 0 28 0 1 0 1 81 128 0 16 42 0 134 192 32 8 0 0 0 0 0 0 0 0 0 1 192 109 0 1 0 1 0 1 81 128 0 4 45 57 9 1 192 109 0 28 0 1 0 1 81 128 0 16 42 0 134 192 32 9 0 0 0 0 0 0 0 0 0 1] 212 31639 0 NOERROR occ-0-1500-1501.1.nflxso.net net nflxso.net A {true false true false false} {[{occ-0-1500-1501.1.nflxso.net A 1 30 45.57.36.151} {occ-0-1500-1501.1.nflxso.net A 1 30 45.57.36.148}] [{nflxso.net NS 1 86400 e.ns.nflxso.net} {nflxso.net NS 1 86400 f.ns.nflxso.net}] [{e.ns.nflxso.net A 1 86400 45.57.8.1} {e.ns.nflxso.net AAAA 1 86400 2a00:86c0:2008::1} {f.ns.nflxso.net A 1 86400 45.57.9.1} {f.ns.nflxso.net AAAA 1 86400 2a00:86c0:2009::1}]} 1} {0 0 0 0 0 []} {RESOLVER_RESPONSE mus-dns4.estpak.ee 2022-03-27T13:44:06.780074Z 1.648388646780074e+09 1648388646 780074000 0 -} {- - -}}

Mostly originated by certain auth NSes:

root@asd:/tmp# journalctl -u dnscollector.service -n 10000 | grep "packet contained too many OPT RRs" | grep --color -P "\S+ NS \d+ \d+ \S+" --only-matching  | sort | uniq -c | sort -nr | head -10
   3261 {dradis.netflix.com NS 1 86400 f.ns.nflxso.net}]
   3261 [{dradis.netflix.com NS 1 86400 e.ns.nflxso.net}
   1786 {nflxso.net NS 1 86400 f.ns.nflxso.net}]
   1786 [{nflxso.net NS 1 86400 e.ns.nflxso.net}
    200 {ftl.netflix.com NS 1 86400 f.ns.nflxso.net}]
    200 [{ftl.netflix.com NS 1 86400 e.ns.nflxso.net}
    111 {c10r.instagram.com NS 1 60 d.ns.c10r.instagram.com}]
    111 {c10r.instagram.com NS 1 60 c.ns.c10r.instagram.com}
    111 [{c10r.instagram.com NS 1 60 b.ns.c10r.instagram.com}
    111 {c10r.instagram.com NS 1 60 a.ns.c10r.instagram.com}

It would probably also make sense to handle this as malformed packet. Currently, with log-malformed and verbose set to false, those errors are still logged.

[dmachard]

thanks to report that, the error is generated because of the following check:

// RFC 6891 says "When an OPT RR is included within any DNS message, it MUST be the
// only OPT RR in that message."
if ednsFound {
return edns, offset, ErrDecodeEdnsTooManyOpts
}

I will check why the log-malformed is not properly take in account in this case.

[dmachard]

issue fixed in the new beta release v0.18.0-b3 available

[dmachard]

New release v0.18.0 available :)

[tarko]

Thanks, works fine. I'm still puzzled by the multiple OPT RR issue as if you dump the logged DNS packet then there is just single OPT RR present there (as expected).

[jtt]

There is indeed a bug in EDNS parsing which causes EDNS parsing to return invalid error about multiple OPT RR's being present.

I'll add PR fixing this shortly.

[tarko]

Nice, looking at the patch is seems fairly obvious. I was actually testing this with the problematic payload from the log but was unable to trigger this, something like this:

func TestDecodeEdns_MultipleOpts2(t *testing.T) {
        payload := []byte{
                0x7B, 0x97, 0x84, 0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x02, 0x00, 0x05, 0x0F, 0x6F, 0x63, 0x63, 0x2D, 0x30, 0x2D, 0x31, 0x35, 0x30, 0x30, 0x2D, 0x31, 0x35, 0x30, 0x31, 0x01, 0x31, 0x06, 0x6E, 0x66, 0x6C, 0x78, 0x73, 0x6F, 0x03, 0x6E, 0x65, 0x74, 0x00, 0x00, 0x01, 0x00, 0x01, 0xC0, 0x0C, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x1E, 0x00, 0x04, 0x2D, 0x39, 0x24, 0x97, 0xC0, 0x0C, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x1E, 0x00, 0x04, 0x2D, 0x39, 0x24, 0x94, 0xC0, 0x1E, 0x00, 0x02, 0x00, 0x01, 0x00, 0x01, 0x51, 0x80, 0x00, 0x07, 0x01, 0x65, 0x02, 0x6E, 0x73, 0xC0, 0x1E, 0xC0, 0x1E, 0x00, 0x02, 0x00, 0x01, 0x00, 0x01, 0x51, 0x80, 0x00, 0x04, 0x01, 0x66, 0xC0, 0x5C, 0x00, 0x00, 0x29, 0x04, 0xB0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xC0, 0x5A, 0x00, 0x01, 0x00, 0x01, 0x00, 0x01, 0x51, 0x80, 0x00, 0x04, 0x2D, 0x39, 0x08, 0x01, 0xC0, 0x5A, 0x00, 0x1C, 0x00, 0x01, 0x00, 0x01, 0x51, 0x80, 0x00, 0x10, 0x2A, 0x00, 0x86, 0xC0, 0x20, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0xC0, 0x6D, 0x00, 0x01, 0x00, 0x01, 0x00, 0x01, 0x51, 0x80, 0x00, 0x04, 0x2D, 0x39, 0x09, 0x01, 0xC0, 0x6D, 0x00, 0x1C, 0x00, 0x01, 0x00, 0x01, 0x51, 0x80, 0x00, 0x10, 0x2A, 0x00, 0x86, 0xC0, 0x20, 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
        }

        _, _, offsset_rr, err := DecodeQuestion(1, payload)
        if err != nil {
                t.Errorf("Unable to decode question: %v", err)
        }
        _, _, erra := DecodeAnswer(1, offsset_rr, payload)
        if erra != nil {
                t.Errorf("Unable to decode question: %v", erra)
        }
        _, _, erre := DecodeEDNS(2, offsset_rr, payload)
        if erre != nil {
                t.Errorf("bad error received: %v", erre)
        }
}

[jtt]

The test data you were using contained 2 Answers, 2 Authority Records and 5 Additional RR's (one of which was the EDNS OPT RR), thus the test code did not give DecodeEDNS() the right offset_rr as parameter (and invalid record count) and due to this failed the trigger the bug.

To ensure right amount of records are parsed, you could do

dm := dnsutils.DnsMessage{}
dm.DNS.Payload = payload
dm.DNS.Length = len(payload)
header, err := dnsutils.DecodeDns(payload)
if err != nil {
    t.Errorf("error when deocoding header: %v", err)
}

if err := decodePayload(&dm, &header, dnsutils.GetFakeConfig()); err != nil {
    t.Errorf("unexpected error while decoding payload: %v", err)
}

this would mimic the actual parsing of the data and hit the bug.

[tarko]

Yeah, as said, now that the fix is there it seems pretty obvious :)

[dmachard]

Binaries v0.19.0 released!