From d95e25a94c7b0f7331b952bc144c65dcd80ffed5 Mon Sep 17 00:00:00 2001
From: Denis Machard <5562930+dmachard@users.noreply.github.com>
Date: Sat, 30 Nov 2024 22:01:49 +0100
Subject: [PATCH] dns parser: set RCODE only in DNS responses (#885)

* set RCODE only in DNS responses
* prometheus logger: ignore default rcode in counter
---
 README.md                           |  2 +-
 dnsutils/dns_parser.go              |  5 +++-
 dnsutils/dns_parser_payload_test.go | 37 ++++++++++++++++++++++++++---
 workers/prometheus.go               | 10 ++++----
 4 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/README.md b/README.md
index 48168222..3f15bb7c 100644
--- a/README.md
+++ b/README.md
@@ -1,7 +1,7 @@
 <p align="center">
   <img src="https://goreportcard.com/badge/github.com/dmachard/go-dns-collector" alt="Go Report"/>
   <img src="https://img.shields.io/badge/go%20version-min%201.21-green" alt="Go version"/>
-  <img src="https://img.shields.io/badge/go%20tests-508-green" alt="Go tests"/>
+  <img src="https://img.shields.io/badge/go%20tests-509-green" alt="Go tests"/>
   <img src="https://img.shields.io/badge/go%20bench-21-green" alt="Go bench"/>
   <img src="https://img.shields.io/badge/go%20lines-31977-green" alt="Go lines"/>
 </p>
diff --git a/dnsutils/dns_parser.go b/dnsutils/dns_parser.go
index 3c3644ba..1dbc5b1c 100644
--- a/dnsutils/dns_parser.go
+++ b/dnsutils/dns_parser.go
@@ -163,7 +163,10 @@ func DecodePayload(dm *DNSMessage, header *DNSHeader, config *pkgconfig.Config)
 	}
 
 	dm.DNS.ID = header.ID
-	dm.DNS.Rcode = RcodeToString(header.Rcode)
+	// set rcode only with response
+	if header.Qr == 1 {
+		dm.DNS.Rcode = RcodeToString(header.Rcode)
+	}
 	dm.DNS.Opcode = header.Opcode
 
 	// update dnstap operation if the opcode is equal to 5 (dns update)
diff --git a/dnsutils/dns_parser_payload_test.go b/dnsutils/dns_parser_payload_test.go
index e72f64e1..54f427cd 100644
--- a/dnsutils/dns_parser_payload_test.go
+++ b/dnsutils/dns_parser_payload_test.go
@@ -43,7 +43,6 @@ func TestDecodePayload_QueryHappy(t *testing.T) {
 
 	if dm.DNS.ID != 0x9e84 ||
 		dm.DNS.Opcode != 0 ||
-		dm.DNS.Rcode != RcodeToString(0) ||
 		dm.DNS.Flags.QR ||
 		dm.DNS.Flags.TC ||
 		dm.DNS.Flags.AA ||
@@ -72,7 +71,6 @@ func TestDecodePayload_QueryHappy(t *testing.T) {
 		len(dm.DNS.DNSRRs.Records) != 0 {
 		t.Errorf("Unexpected sections parsed")
 	}
-
 }
 func TestDecodePayload_QueryInvalid(t *testing.T) {
 	payload := []byte{
@@ -996,7 +994,6 @@ func TestDecodePayload_Truncated(t *testing.T) {
 	if dm.DNS.MalformedPacket != true {
 		t.Errorf("expected packet to be malformed")
 	}
-
 }
 
 // Dynamic query (UPDATE)
@@ -1086,3 +1083,37 @@ func TestDecodePayload_MDNSResponseWithNoQuestion(t *testing.T) {
 		t.Error("expected no error on decode", err)
 	}
 }
+
+// Rcode should not be set on query
+func TestDecodePayload_Query_NoRcode(t *testing.T) {
+	payload := []byte{
+		// transaction ID
+		0xb1, 0x17,
+		// flags
+		0x01, 0x00,
+		// questions
+		0x00, 0x01,
+		// answer
+		0x00, 0x00,
+		// authority
+		0x00, 0x00,
+		// additional
+		0x00, 0x00,
+		// queries
+		0x00, 0x00, 0x06, 0x00, 0x01,
+	}
+
+	dm := DNSMessage{}
+	dm.Init()
+	dm.DNS.Payload = payload
+	dm.DNS.Length = len(payload)
+
+	// decode header and paylo
+	header, _ := DecodeDNS(payload)
+	DecodePayload(&dm, &header, pkgconfig.GetDefaultConfig())
+
+	// check the rcode
+	if dm.DNS.Rcode != "-" {
+		t.Errorf("invalid rcode: %s", dm.DNS.Rcode)
+	}
+}
diff --git a/workers/prometheus.go b/workers/prometheus.go
index 0f1f2087..45d515c1 100644
--- a/workers/prometheus.go
+++ b/workers/prometheus.go
@@ -372,10 +372,12 @@ func (w *PrometheusCountersSet) Record(dm dnsutils.DNSMessage) {
 		w.epsCounters.TotalQtypes[dm.DNS.Qtype]++
 	}
 
-	if _, exists := w.epsCounters.TotalRcodes[dm.DNS.Rcode]; !exists {
-		w.epsCounters.TotalRcodes[dm.DNS.Rcode] = 1
-	} else {
-		w.epsCounters.TotalRcodes[dm.DNS.Rcode]++
+	if dm.DNS.Rcode != "-" {
+		if _, exists := w.epsCounters.TotalRcodes[dm.DNS.Rcode]; !exists {
+			w.epsCounters.TotalRcodes[dm.DNS.Rcode] = 1
+		} else {
+			w.epsCounters.TotalRcodes[dm.DNS.Rcode]++
+		}
 	}
 
 	if _, exists := w.epsCounters.TotalOperations[dm.DNSTap.Operation]; !exists {