From 3d1d63be07bbb47f37139e215763ca5521c3d5ae Mon Sep 17 00:00:00 2001 From: Jukka Taimisto Date: Mon, 25 Apr 2022 00:34:07 +0300 Subject: [PATCH 1/2] Advance offset when parsing EDNS data Fixes a bug where additional Resource Records after EDNS data caused invalid error about multiple EDNS OPT RR's being present. Also fixes parsing EDNS data after any other additional RR's. --- dnsutils/edns.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dnsutils/edns.go b/dnsutils/edns.go index 09f29c4e..ecf8c235 100644 --- a/dnsutils/edns.go +++ b/dnsutils/edns.go @@ -160,7 +160,15 @@ func DecodeEDNS(arcount int, start_offset int, payload []byte) (DnsExtended, int edns.Options = options ednsFound = true + offset = offset_next + } else { + // advance to next RR + rdlength := binary.BigEndian.Uint16(payload[offset_next+8 : offset_next+10]) + if len(payload[offset_next+10:]) < int(rdlength) { + return edns, offset, ErrDecodeEdnsDataTooShort + } + offset = offset_next + 10 + int(rdlength) } } return edns, offset, nil From 78e93745932b6ca2cf78ca47947b453c035247bb Mon Sep 17 00:00:00 2001 From: Jukka Taimisto Date: Mon, 25 Apr 2022 00:35:28 +0300 Subject: [PATCH 2/2] Additional unit tests for EDNS data parsing. Ensure EDNS data is parsed properly even if there are multiple additional Resource Records present. --- dnsutils/edns_test.go | 112 ++++++++++++++++++++++++++++++++++ subprocessors/decoder_test.go | 51 ++++++++++++++++ 2 files changed, 163 insertions(+) diff --git a/dnsutils/edns_test.go b/dnsutils/edns_test.go index fa791398..1f4cf482 100644 --- a/dnsutils/edns_test.go +++ b/dnsutils/edns_test.go @@ -751,3 +751,115 @@ func TestDecodeEdns_MultipleOpts(t *testing.T) { t.Errorf("bad error received: %v", err) } } + +func TestDecodeEdns_NonEDNSFollows(t *testing.T) { + payload := []byte{ + // empty name + 0x00, + // type OPT + 0x00, 0x29, + // class / UDP Payload size + 0x04, 0xd0, + // TTL / EXT-RCODE=0, VERSION=0, DO=1, Z=0 + 0x00, 0x00, 0x80, 0x00, + // RDLENGTH + 0x00, 0x00, + // no RDATA + // next RR, + // Name + 0x03, 0x46, 0x4f, 0x4f, 0x03, 0x4e, 0x45, 0x54, 0x00, + // type A, class IN + 0x00, 0x01, 0x00, 0x01, + // TTL + 0x00, 0x00, 0x0e, 0x10, + // RDLENGTH + 0x00, 0x04, + // RDATA 127.0.0.1 + 0x7f, 0x00, 0x00, 0x01, + } + + edns, offset, err := DecodeEDNS(2, 0x00, payload) + if err != nil { + t.Errorf("Unexpected error %v", err) + } + if offset != len(payload) { + t.Errorf("expected offset %d, got %d", len(payload), offset) + } + if len(edns.Options) != 0 { + t.Errorf("Did not expect any Options on EDNS") + } + if edns.UdpSize != 1232 { + t.Errorf("expected UDP Size of 1232, got %d", edns.UdpSize) + } +} + +func TestDecodeEdns_EDNSFollows(t *testing.T) { + payload := []byte{ + 0x03, 0x46, 0x4f, 0x4f, 0x03, 0x4e, 0x45, 0x54, 0x00, + // type A, class IN + 0x00, 0x01, 0x00, 0x01, + // TTL + 0x00, 0x00, 0x0e, 0x10, + // RDLENGTH + 0x00, 0x04, + // RDATA 127.0.0.1 + 0x7f, 0x00, 0x00, 0x01, + // EDNS DATA + // empty name + 0x00, + // type OPT + 0x00, 0x29, + // class / UDP Payload size + 0x04, 0xd0, + // TTL / EXT-RCODE=0, VERSION=0, DO=1, Z=0 + 0x00, 0x00, 0x80, 0x00, + // RDLENGTH + 0x00, 0x00, + } + + edns, offset, err := DecodeEDNS(2, 0x00, payload) + if err != nil { + t.Errorf("Unexpected error %v", err) + } + if offset != len(payload) { + t.Errorf("expected offset %d, got %d", len(payload), offset) + } + if len(edns.Options) != 0 { + t.Errorf("Did not expect any Options on EDNS") + } + if edns.UdpSize != 1232 { + t.Errorf("expected UDP Size of 1232, got %d", edns.UdpSize) + } +} + +func TestDecodeEdns_invalidRRFollows(t *testing.T) { + payload := []byte{ + // empty name + 0x00, + // type OPT + 0x00, 0x29, + // class / UDP Payload size + 0x04, 0xd0, + // TTL / EXT-RCODE=0, VERSION=0, DO=1, Z=0 + 0x00, 0x00, 0x80, 0x00, + // RDLENGTH + 0x00, 0x00, + // no RDATA + // next RR, + // Name + 0x03, 0x46, 0x4f, 0x4f, 0x03, 0x4e, 0x45, 0x54, 0x00, + // type A, class IN + 0x00, 0x01, 0x00, 0x01, + // TTL + 0x00, 0x00, 0x0e, 0x10, + // RDLENGTH + 0x00, 0x04, + // not enough RDATA + 0x7f, 0x00, + } + + _, _, err := DecodeEDNS(2, 0x00, payload) + if !errors.Is(err, ErrDecodeEdnsDataTooShort) { + t.Errorf("bad error received: %v", err) + } +} diff --git a/subprocessors/decoder_test.go b/subprocessors/decoder_test.go index c11861ea..7198ff76 100644 --- a/subprocessors/decoder_test.go +++ b/subprocessors/decoder_test.go @@ -791,3 +791,54 @@ func TestDecodePayload_AnswerError_Invalid(t *testing.T) { } } + +func TestDecodePayload_AdditionalRRAndEDNS(t *testing.T) { + // payload containing both addition RR and EDNS, ensure we are + // able to parse all of them + payload := []byte{ + 0x7b, 0x97, 0x84, 0x0, 0x0, 0x1, 0x0, 0x2, + 0x0, 0x2, 0x0, 0x5, 0xf, 0x6f, 0x63, 0x63, + 0x2d, 0x30, 0x2d, 0x31, 0x35, 0x30, 0x30, + 0x2d, 0x31, 0x35, 0x30, 0x31, 0x1, 0x31, + 0x6, 0x6e, 0x66, 0x6c, 0x78, 0x73, 0x6f, + 0x3, 0x6e, 0x65, 0x74, 0x0, 0x0, 0x1, 0x0, + 0x1, 0xc0, 0xc, 0x0, 0x1, 0x0, 0x1, 0x0, + 0x0, 0x0, 0x1e, 0x0, 0x4, 0x2d, 0x39, 0x24, + 0x97, 0xc0, 0xc, 0x0, 0x1, 0x0, 0x1, 0x0, + 0x0, 0x0, 0x1e, 0x0, 0x4, 0x2d, 0x39, 0x24, + 0x94, 0xc0, 0x1e, 0x0, 0x2, 0x0, 0x1, 0x0, + 0x1, 0x51, 0x80, 0x0, 0x7, 0x1, 0x65, 0x2, + 0x6e, 0x73, 0xc0, 0x1e, 0xc0, 0x1e, 0x0, + 0x2, 0x0, 0x1, 0x0, 0x1, 0x51, 0x80, 0x0, + 0x4, 0x1, 0x66, 0xc0, 0x5c, 0x0, 0x0, 0x29, + 0x4, 0xb0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xc0, + 0x5a, 0x0, 0x1, 0x0, 0x1, 0x0, 0x1, 0x51, 0x80, + 0x0, 0x4, 0x2d, 0x39, 0x8, 0x1, 0xc0, 0x5a, + 0x0, 0x1c, 0x0, 0x1, 0x0, 0x1, 0x51, 0x80, 0x0, + 0x10, 0x2a, 0x0, 0x86, 0xc0, 0x20, 0x8, 0x0, 0x0, + 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0xc0, 0x6d, + 0x0, 0x1, 0x0, 0x1, 0x0, 0x1, 0x51, 0x80, 0x0, 0x4, + 0x2d, 0x39, 0x9, 0x1, 0xc0, 0x6d, 0x0, 0x1c, 0x0, 0x1, + 0x0, 0x1, 0x51, 0x80, 0x0, 0x10, 0x2a, 0x0, 0x86, 0xc0, + 0x20, 0x9, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1} + + 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) + } + + if len(dm.DNS.DnsRRs.Answers) != 2 || len(dm.DNS.DnsRRs.Nameservers) != 2 || + len(dm.DNS.DnsRRs.Records) != 4 || dm.EDNS.UdpSize != 1200 || + len(dm.EDNS.Options) != 0 { + t.Errorf("unexpected result while parsing payload: %#v", dm.DNS) + } + +}