Merge pull request #72 from jtt/edns-parse

Fix a bug in EDNS parsing causing parsing to fail if there are multiple additional Resource Records
dmachard · Apr 25, 2022 · f827b2a · f827b2a
2 parents 6610cc1 + 78e9374
commit f827b2a
Show file tree

Hide file tree

Showing 3 changed files with 171 additions and 0 deletions.
diff --git 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

diff --git 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
@@ -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)
+	}
+
+}