Merge branch '2762-check-opcode' into 'main'

check returned opcodes

Closes #2762

See merge request isc-projects/bind9!5286

CHANGES

@@ -1,3 +1,6 @@
+5685.	[bug]		Check the opcodes of messages returned by
+			dns_request_getresponse.  [GL #2762]
+
 5684.	[func]		Changes to the DNS-over-HTTP (DoH) configuration
 			syntax:
 

bin/nsupdate/nsupdate.c

@@ -2445,6 +2445,10 @@ update_completed(isc_task_t *task, isc_event_t *event) {
 		check_result(result, "dns_request_getresponse");
 	}
 
+	if (answer->opcode != dns_opcode_update) {
+		fatal("invalid OPCODE in response to UPDATE request");
+	}
+
 	if (answer->rcode != dns_rcode_noerror) {
 		seenerror = true;
 		if (!debugging) {
@@ -2651,6 +2655,10 @@ recvsoa(isc_task_t *task, isc_event_t *event) {
 		show_message(stderr, rcvmsg, "Reply from SOA query:");
 	}
 
+	if (rcvmsg->opcode != dns_opcode_query) {
+		fatal("invalid OPCODE in response to SOA query");
+	}
+
 	if (rcvmsg->rcode != dns_rcode_noerror &&
 	    rcvmsg->rcode != dns_rcode_nxdomain) {
 		fatal("response to SOA query was unsuccessful");
@@ -3124,6 +3132,10 @@ recvgss(isc_task_t *task, isc_event_t *event) {
 			     "recvmsg reply from GSS-TSIG query");
 	}
 
+	if (rcvmsg->opcode != dns_opcode_query) {
+		fatal("invalid OPCODE in response to GSS-TSIG query");
+	}
+
 	if (rcvmsg->rcode == dns_rcode_formerr && !tried_other_gsstsig) {
 		ddebug("recvgss trying %s GSS-TSIG",
 		       use_win2k_gsstsig ? "Standard" : "Win2k");

bin/tests/system/digdelv/clean.sh

@@ -28,3 +28,4 @@ rm -f ./ns2/dsset-example.
 rm -f ./ns2/example.db ./ns2/K* ./ns2/keyid ./ns2/keydata
 rm -f ./nslookup.out.test*
 rm -f ./yamlget.out.*
+rm -f ./nsupdate.out.test*

bin/tests/system/digdelv/tests.sh

@@ -101,6 +101,24 @@ if [ -x "$HOST" -a $checkupdate -eq 1 ] ; then
 
 fi
 
+if [ -x "$NSUPDATE" -a $checkupdate -eq 1 ] ; then
+
+  n=$((n+1))
+  echo_i "check nsupdate handles UPDATE response to QUERY ($n)"
+  ret=0
+  res=0
+  $NSUPDATE << EOF > nsupdate.out.test$n 2>&1 || res=$?
+server 10.53.0.7 ${PORT}
+add x.example.com 300 in a 1.2.3.4
+send
+EOF
+  test $res -eq 1 || ret=1
+  grep "invalid OPCODE in response to SOA query" nsupdate.out.test$n > /dev/null || ret=1
+  if [ $ret -ne 0 ]; then echo_i "failed"; fi
+  status=$((status+ret))
+
+fi
+
 if [ -x "$DIG" ] ; then
 
   if [ $checkupdate -eq 1 ] ; then

doc/notes/notes-current.rst

@@ -20,6 +20,12 @@ Security Fixes
   ISC would like to thank Ville Heikkila of Synopsys Cybersecurity Research
   Center for responsibly disclosing the vulnerability to us. :gl:`#2787`
 
+- Named failed to check the opcode of responses when performing refresh,
+  stub updates, and UPDATE forwarding.  This could lead to an assertion
+  failure under particular conditions.  This has been addressed by checking
+  the opcode of those responses and rejecting the messages if they don't
+  match the expected value. :gl:`#2762`
+
 Known Issues
 ~~~~~~~~~~~~
 

lib/dns/include/dns/rdata.h

@@ -516,7 +516,7 @@ dns_rdata_tostruct(const dns_rdata_t *rdata, void *target, isc_mem_t *mctx);
  *
  * Requires:
  *
- *\li	'rdata' is a valid, non-empty rdata.
+ *\li	'rdata' is a valid, non-empty, non-pseudo rdata.
  *
  *\li	'target' to point to a valid pointer for the type and class.
  *

lib/dns/rdata.c

@@ -1222,6 +1222,7 @@ dns_rdata_tostruct(const dns_rdata_t *rdata, void *target, isc_mem_t *mctx) {
 
 	REQUIRE(rdata != NULL);
 	REQUIRE(DNS_RDATA_VALIDFLAGS(rdata));
+	REQUIRE((rdata->flags & DNS_RDATA_UPDATE) == 0);
 
 	TOSTRUCTSWITCH
 

lib/dns/zone.c

@@ -58,6 +58,7 @@
 #include <dns/name.h>
 #include <dns/nsec.h>
 #include <dns/nsec3.h>
+#include <dns/opcode.h>
 #include <dns/peer.h>
 #include <dns/private.h>
 #include <dns/rcode.h>
@@ -13181,6 +13182,23 @@ stub_glue_response_cb(isc_task_t *task, isc_event_t *event) {
 		goto cleanup;
 	}
 
+	/*
+	 * Unexpected opcode.
+	 */
+	if (msg->opcode != dns_opcode_query) {
+		char opcode[128];
+		isc_buffer_t rb;
+
+		isc_buffer_init(&rb, opcode, sizeof(opcode));
+		(void)dns_opcode_totext(msg->rcode, &rb);
+
+		dns_zone_log(zone, ISC_LOG_INFO,
+			     "refreshing stub: "
+			     "unexpected opcode (%.*s) from %s (source %s)",
+			     (int)rb.used, opcode, master, source);
+		goto cleanup;
+	}
+
 	/*
 	 * Unexpected rcode.
 	 */
@@ -13593,6 +13611,23 @@ stub_callback(isc_task_t *task, isc_event_t *event) {
 		goto next_master;
 	}
 
+	/*
+	 * Unexpected opcode.
+	 */
+	if (msg->opcode != dns_opcode_query) {
+		char opcode[128];
+		isc_buffer_t rb;
+
+		isc_buffer_init(&rb, opcode, sizeof(opcode));
+		(void)dns_opcode_totext(msg->rcode, &rb);
+
+		dns_zone_log(zone, ISC_LOG_INFO,
+			     "refreshing stub: "
+			     "unexpected opcode (%.*s) from %s (source %s)",
+			     (int)rb.used, opcode, master, source);
+		goto next_master;
+	}
+
 	/*
 	 * Unexpected rcode.
 	 */
@@ -13982,6 +14017,23 @@ refresh_callback(isc_task_t *task, isc_event_t *event) {
 		goto next_master;
 	}
 
+	/*
+	 * Unexpected opcode.
+	 */
+	if (msg->opcode != dns_opcode_query) {
+		char opcode[128];
+		isc_buffer_t rb;
+
+		isc_buffer_init(&rb, opcode, sizeof(opcode));
+		(void)dns_opcode_totext(msg->rcode, &rb);
+
+		dns_zone_log(zone, ISC_LOG_INFO,
+			     "refresh: "
+			     "unexpected opcode (%.*s) from %s (source %s)",
+			     (int)rb.used, opcode, master, source);
+		goto next_master;
+	}
+
 	/*
 	 * Unexpected rcode.
 	 */
@@ -18221,6 +18273,23 @@ forward_callback(isc_task_t *task, isc_event_t *event) {
 		goto next_master;
 	}
 
+	/*
+	 * Unexpected opcode.
+	 */
+	if (msg->opcode != dns_opcode_update) {
+		char opcode[128];
+		isc_buffer_t rb;
+
+		isc_buffer_init(&rb, opcode, sizeof(opcode));
+		(void)dns_opcode_totext(msg->rcode, &rb);
+
+		dns_zone_log(zone, ISC_LOG_INFO,
+			     "forwarding dynamic update: "
+			     "unexpected opcode (%.*s) from %s",
+			     (int)rb.used, opcode, master);
+		goto next_master;
+	}
+
 	switch (msg->rcode) {
 	/*
 	 * Pass these rcodes back to client.