From: Vladimír Čunát <vladimir.cunat@nic.cz>
Date: Wed, 4 Nov 2020 09:07:40 +0000 (+0100)
Subject: modules: fix issues with dropped answers - resolve()
X-Git-Tag: v5.2.0~1^2~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8418840278de033a668d14cbbbcd4b5fd8b55515;p=thirdparty%2Fknot-resolver.git

modules: fix issues with dropped answers - resolve()

Well... practically it still can't happen that an internal request
gets its answer dropped, but ATM my understanding of the API is that
it is allowed to happen already, and the crashes during tests were
bothering me (simulating drops).

This may become more relevant in future, e.g. if we allow dropping
as a policy action; policy authors may not care about the request being
internal.
---

diff --git a/modules/detect_time_skew/detect_time_skew.lua b/modules/detect_time_skew/detect_time_skew.lua
index 2cc792b0b..0c1079605 100644
--- a/modules/detect_time_skew/detect_time_skew.lua
+++ b/modules/detect_time_skew/detect_time_skew.lua
@@ -9,7 +9,7 @@ local event_id = nil
 -- Check time validity of RRSIGs in priming query
 -- luacheck: no unused args
 local function check_time_callback(pkt, req)
-	if pkt:rcode() ~= kres.rcode.NOERROR then
+	if pkt == nil or pkt:rcode() ~= kres.rcode.NOERROR then
 		warn("[detect_time_skew] cannot resolve '.' NS")
 		return nil
 	end
diff --git a/modules/priming/priming.lua b/modules/priming/priming.lua
index c60f25540..46c2f4cd1 100644
--- a/modules/priming/priming.lua
+++ b/modules/priming/priming.lua
@@ -42,7 +42,8 @@ end
 -- When all response is processed internal.nsset is published in resolver engine
 -- luacheck: no unused args
 local function address_callback(pkt, req)
-	if pkt:rcode() ~= kres.rcode.NOERROR then
+	if pkt == nil or pkt:rcode() ~= kres.rcode.NOERROR then
+		pkt = req.qsource.packet
 		warn("[priming] cannot resolve address '%s', type: %d", kres.dname2str(pkt:qname()), pkt:qtype())
 	else
 		local section = pkt:rrsets(kres.section.ANSWER)
@@ -80,7 +81,7 @@ end
 -- These new queries should be resolved from cache.
 -- luacheck: no unused args
 local function priming_callback(pkt, req)
-	if pkt:rcode() ~= kres.rcode.NOERROR then
+	if pkt == nil or pkt:rcode() ~= kres.rcode.NOERROR then
 		warn("[priming] cannot resolve '.' NS, next priming query in %d seconds", priming.retry_time / sec)
 		internal.event = event.after(priming.retry_time, internal.prime)
 		return nil
diff --git a/modules/ta_update/ta_update.lua b/modules/ta_update/ta_update.lua
index ce62c13ed..3abd2ccad 100644
--- a/modules/ta_update/ta_update.lua
+++ b/modules/ta_update/ta_update.lua
@@ -252,7 +252,7 @@ end
 local function active_refresh(keyset, pkt, req, managed)
 	local retry = true
 
-	if pkt:rcode() == kres.rcode.NOERROR then
+	if pkt ~= nil and pkt:rcode() == kres.rcode.NOERROR then
 		local records = pkt:section(kres.section.ANSWER)
 		local new_keys = {}
 		for _, rr in ipairs(records) do
@@ -271,6 +271,8 @@ local function active_refresh(keyset, pkt, req, managed)
 		local qry = req:initial()
 		if qry.flags.DNSSEC_BOGUS == true then
 			warn('[ta_update] active refresh failed, update your trust anchors in "%s"', keyset.filename)
+		elseif pkt == nil then
+			warn('[ta_update] active refresh failed, answer was dropped')
 		else
 			warn('[ta_update] active refresh failed for ' .. kres.dname2str(keyset.owner)
 				.. ' with rcode: ' .. pkt:rcode())
diff --git a/modules/watchdog/watchdog.lua b/modules/watchdog/watchdog.lua
index 1a8065fdd..b7e162b9d 100644
--- a/modules/watchdog/watchdog.lua
+++ b/modules/watchdog/watchdog.lua
@@ -32,13 +32,18 @@ end
 local function check_answer(logbuf)
 	return function (pkt, req)
 		req.trace_log:free()
-		if pkt:rcode() == kres.rcode.NOERROR or pkt:rcode() == kres.rcode.NXDOMAIN then
+		if pkt ~= nil and (pkt:rcode() == kres.rcode.NOERROR
+							or pkt:rcode() == kres.rcode.NXDOMAIN) then
 			private.ok_callback()
 			return
 		end
 		log('[watchdog] watchdog query returned unexpected answer! query verbose log:')
 		log(table.concat(logbuf, ''))
-		log('[watchdog] problematic answer:\n%s', pkt)
+		if pkt ~= nil then
+			log('[watchdog] problematic answer:\n%s', pkt)
+		else
+			log('[watchdog] answer was dropped')
+		end
 		-- failure! quit immediatelly to allow process supervisor to restart us
 		private.fail_callback()
 	end
diff --git a/tests/config/test_utils.lua b/tests/config/test_utils.lua
index afc713b28..53b6c09c8 100644
--- a/tests/config/test_utils.lua
+++ b/tests/config/test_utils.lua
@@ -63,6 +63,7 @@ function M.check_answer(desc, qname, qtype, expected_rcode, expected_rdata)
 
 	local done = false
 	local callback = function(pkt)
+		ok(pkt, 'answer not dropped')
 		same(pkt:rcode(), wire_rcode,
 		     desc .. ': expecting answer for query ' .. qname .. ' ' .. qtype_str
 		      .. ' with rcode ' .. kres.tostring.rcode[wire_rcode])