bind9.16/bind-9.16-CVE-2024-1975.patch

From 34e92fc88943beeba76aa4e408951cb46d8cdb53 Mon Sep 17 00:00:00 2001
From: Petr Mensik <pemensik@redhat.com>
Date: Tue, 16 Jul 2024 19:49:09 +0200
Subject: [PATCH] Resolve CVE-2024-1975

6404.	[security]	Remove SIG(0) support from named as a countermeasure
			for CVE-2024-1975. [GL #4480]

Resolves: CVE-2024-1975
---
 bin/tests/system/tsiggss/authsock.pl |  5 ++
 bin/tests/system/tsiggss/tests.sh    | 12 ++--
 bin/tests/system/upforwd/tests.sh    | 21 +++---
 doc/arm/general.rst                  |  6 +-
 doc/arm/reference.rst                |  4 +-
 doc/arm/security.rst                 |  4 +-
 lib/dns/message.c                    | 97 ++--------------------------
 lib/ns/client.c                      |  7 ++
 8 files changed, 43 insertions(+), 113 deletions(-)

diff --git a/bin/tests/system/tsiggss/authsock.pl b/bin/tests/system/tsiggss/authsock.pl
index ab3833d..0b231ee 100644
--- a/bin/tests/system/tsiggss/authsock.pl
+++ b/bin/tests/system/tsiggss/authsock.pl
@@ -31,6 +31,10 @@ if (!defined($path)) {
 	exit(1);
 }
 
+# Enable output autoflush so that it's not lost when the parent sends TERM.
+select STDOUT;
+$| = 1;
+
 unlink($path);
 my $server = IO::Socket::UNIX->new(Local => $path, Type => SOCK_STREAM, Listen => 8) or
     die "unable to create socket $path";
@@ -53,6 +57,7 @@ if ($timeout != 0) {
 }
 
 while (my $client = $server->accept()) {
+	printf("accept()\n");
 	$client->recv(my $buf, 8, 0);
 	my ($version, $req_len) = unpack('N N', $buf);
 
diff --git a/bin/tests/system/tsiggss/tests.sh b/bin/tests/system/tsiggss/tests.sh
index 632bb87..7977e49 100644
--- a/bin/tests/system/tsiggss/tests.sh
+++ b/bin/tests/system/tsiggss/tests.sh
@@ -116,7 +116,7 @@ status=$((status+ret))
 
 echo_i "testing external update policy (CNAME) with auth sock ($n)"
 ret=0
-$PERL ./authsock.pl --type=CNAME --path=ns1/auth.sock --pidfile=authsock.pid --timeout=120 > /dev/null 2>&1 &
+$PERL ./authsock.pl --type=CNAME --path=ns1/auth.sock --pidfile=authsock.pid --timeout=120 >authsock.log 2>&1 &
 sleep 1
 test_update $n testcname.example.nil. CNAME "86400 CNAME testdenied.example.nil" "testdenied" || ret=1
 n=$((n+1))
@@ -130,17 +130,19 @@ n=$((n+1))
 if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
 status=$((status+ret))
 
-echo_i "testing external policy with SIG(0) key ($n)"
+echo_i "testing external policy with unsupported SIG(0) key ($n)"
 ret=0
-$NSUPDATE -k ns1/Kkey.example.nil.*.private <<END > /dev/null 2>&1 || ret=1
+$NSUPDATE -d -k ns1/Kkey.example.nil.*.private <<END >nsupdate.out${n} 2>&1 || true
+debug
 server 10.53.0.1 ${PORT}
 zone example.nil
 update add fred.example.nil 120 cname foo.bar.
 send
 END
 output=`$DIG $DIGOPTS +short cname fred.example.nil.`
-[ -n "$output" ] || ret=1
-[ $ret -eq 0 ] || echo_i "failed"
+# update must have failed - SIG(0) signer is not supported
+[ -n "$output" ] && ret=1
+grep -F "signer=key.example.nil" authsock.log >/dev/null && ret=1
 n=$((n+1))
 if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
 status=$((status+ret))
diff --git a/bin/tests/system/upforwd/tests.sh b/bin/tests/system/upforwd/tests.sh
index 20fc46f..c8fd54b 100644
--- a/bin/tests/system/upforwd/tests.sh
+++ b/bin/tests/system/upforwd/tests.sh
@@ -224,19 +224,22 @@ fi
 
 if test -f keyname
 then
-	echo_i "checking update forwarding to with sig0 ($n)"
+	echo_i "checking update forwarding to with sig0 (expected to fail) ($n)"
 	ret=0
 	keyname=`cat keyname`
-	$NSUPDATE -k $keyname.private -- - <<EOF
-	local 10.53.0.1
-	server 10.53.0.3 ${PORT}
-	zone example2
-	update add unsigned.example2. 600 A 10.10.10.1
-	update add unsigned.example2. 600 TXT Foo
-	send
+	# SIG(0) is removed, update is expected to fail.
+	{
+	  $NSUPDATE -k $keyname.private -- - <<EOF
+	  local 10.53.0.1
+	  server 10.53.0.3 ${PORT}
+	  zone example2
+	  update add unsigned.example2. 600 A 10.10.10.1
+	  update add unsigned.example2. 600 TXT Foo
+	  send
 EOF
+	} >nsupdate.out.$n 2>&1 && ret=1
 	$DIG -p ${PORT} unsigned.example2 A @10.53.0.1 > dig.out.ns1.test$n
-	grep "status: NOERROR" dig.out.ns1.test$n > /dev/null || ret=1
+	grep "status: NOERROR" dig.out.ns1.test$n >/dev/null && ret=1
 	if [ $ret != 0 ] ; then echo_i "failed"; fi
 	status=`expr $status + $ret`
 	n=`expr $n + 1`
diff --git a/doc/arm/general.rst b/doc/arm/general.rst
index 225576b..0766dfe 100644
--- a/doc/arm/general.rst
+++ b/doc/arm/general.rst
@@ -534,10 +534,8 @@ than a non-authoritative response. This is considered a feature.
 [2] CLASS ANY queries are not supported. This is considered a
 feature.
 
-[3] When receiving a query signed with a SIG(0), the server is
-only able to verify the signature if it has the key in its local
-authoritative data; it cannot do recursion or validation to
-retrieve unknown keys.
+[3] Support for SIG(0) message verification was removed
+as part of the mitigation of CVE-2024-1975.
 
 [4] Compliance is with loading and serving of A6 records only. A6 records were moved
 to the experimental category by :rfc:`3363`.
diff --git a/doc/arm/reference.rst b/doc/arm/reference.rst
index d4ee9d2..ad7ff27 100644
--- a/doc/arm/reference.rst
+++ b/doc/arm/reference.rst
@@ -5789,7 +5789,7 @@ The ``update-policy`` clause allows more fine-grained control over which
 updates are allowed. It specifies a set of rules, in which each rule
 either grants or denies permission for one or more names in the zone to
 be updated by one or more identities. Identity is determined by the key
-that signed the update request, using either TSIG or SIG(0). In most
+that signed the update request, using either TSIG. In most
 cases, ``update-policy`` rules only apply to key-based identities. There
 is no way to specify update permissions based on the client source address.
 
@@ -5846,7 +5846,7 @@ field), and the type of the record to be updated matches the ``types``
 field. Details for each rule type are described below.
 
 The ``identity`` field must be set to a fully qualified domain name. In
-most cases, this represents the name of the TSIG or SIG(0) key that
+most cases, this represents the name of the TSIG key that
 must be used to sign the update request. If the specified name is a
 wildcard, it is subject to DNS wildcard expansion, and the rule may
 apply to multiple identities. When a TKEY exchange has been used to
diff --git a/doc/arm/security.rst b/doc/arm/security.rst
index f7c8bd3..e3abfd1 100644
--- a/doc/arm/security.rst
+++ b/doc/arm/security.rst
@@ -32,7 +32,7 @@ Limiting access to the server by outside parties can help prevent
 spoofing and denial of service (DoS) attacks against the server.
 
 ACLs match clients on the basis of up to three characteristics: 1) The
-client's IP address; 2) the TSIG or SIG(0) key that was used to sign the
+client's IP address; 2) the TSIG key that was used to sign the
 request, if any; and 3) an address prefix encoded in an EDNS
 Client-Subnet option, if any.
 
@@ -73,7 +73,7 @@ and no queries at all from the networks specified in ``bogusnets``.
 
 In addition to network addresses and prefixes, which are matched against
 the source address of the DNS request, ACLs may include ``key``
-elements, which specify the name of a TSIG or SIG(0) key.
+elements, which specify the name of a TSIG key.
 
 When BIND 9 is built with GeoIP support, ACLs can also be used for
 geographic access restrictions. This is done by specifying an ACL
diff --git a/lib/dns/message.c b/lib/dns/message.c
index 1993b2e..04315bc 100644
--- a/lib/dns/message.c
+++ b/lib/dns/message.c
@@ -3287,109 +3287,24 @@ dns_message_dumpsig(dns_message_t *msg, char *txt1) {
 
 isc_result_t
 dns_message_checksig(dns_message_t *msg, dns_view_t *view) {
-	isc_buffer_t b, msgb;
+	isc_buffer_t msgb;
 
 	REQUIRE(DNS_MESSAGE_VALID(msg));
 
-	if (msg->tsigkey == NULL && msg->tsig == NULL && msg->sig0 == NULL) {
+	if (msg->tsigkey == NULL && msg->tsig == NULL) {
 		return (ISC_R_SUCCESS);
 	}
 
 	INSIST(msg->saved.base != NULL);
 	isc_buffer_init(&msgb, msg->saved.base, msg->saved.length);
 	isc_buffer_add(&msgb, msg->saved.length);
-	if (msg->tsigkey != NULL || msg->tsig != NULL) {
 #ifdef SKAN_MSG_DEBUG
-		dns_message_dumpsig(msg, "dns_message_checksig#1");
+	dns_message_dumpsig(msg, "dns_message_checksig#1");
 #endif /* ifdef SKAN_MSG_DEBUG */
-		if (view != NULL) {
-			return (dns_view_checksig(view, &msgb, msg));
-		} else {
-			return (dns_tsig_verify(&msgb, msg, NULL, NULL));
-		}
+	if (view != NULL) {
+		return (dns_view_checksig(view, &msgb, msg));
 	} else {
-		dns_rdata_t rdata = DNS_RDATA_INIT;
-		dns_rdata_sig_t sig;
-		dns_rdataset_t keyset;
-		isc_result_t result;
-
-		result = dns_rdataset_first(msg->sig0);
-		INSIST(result == ISC_R_SUCCESS);
-		dns_rdataset_current(msg->sig0, &rdata);
-
-		/*
-		 * This can occur when the message is a dynamic update, since
-		 * the rdata length checking is relaxed.  This should not
-		 * happen in a well-formed message, since the SIG(0) is only
-		 * looked for in the additional section, and the dynamic update
-		 * meta-records are in the prerequisite and update sections.
-		 */
-		if (rdata.length == 0) {
-			return (ISC_R_UNEXPECTEDEND);
-		}
-
-		result = dns_rdata_tostruct(&rdata, &sig, msg->mctx);
-		if (result != ISC_R_SUCCESS) {
-			return (result);
-		}
-
-		dns_rdataset_init(&keyset);
-		if (view == NULL) {
-			return (DNS_R_KEYUNAUTHORIZED);
-		}
-		result = dns_view_simplefind(view, &sig.signer,
-					     dns_rdatatype_key /* SIG(0) */, 0,
-					     0, false, &keyset, NULL);
-
-		if (result != ISC_R_SUCCESS) {
-			/* XXXBEW Should possibly create a fetch here */
-			result = DNS_R_KEYUNAUTHORIZED;
-			goto freesig;
-		} else if (keyset.trust < dns_trust_secure) {
-			/* XXXBEW Should call a validator here */
-			result = DNS_R_KEYUNAUTHORIZED;
-			goto freesig;
-		}
-		result = dns_rdataset_first(&keyset);
-		INSIST(result == ISC_R_SUCCESS);
-		for (; result == ISC_R_SUCCESS;
-		     result = dns_rdataset_next(&keyset)) {
-			dst_key_t *key = NULL;
-
-			dns_rdata_reset(&rdata);
-			dns_rdataset_current(&keyset, &rdata);
-			isc_buffer_init(&b, rdata.data, rdata.length);
-			isc_buffer_add(&b, rdata.length);
-
-			result = dst_key_fromdns(&sig.signer, rdata.rdclass, &b,
-						 view->mctx, &key);
-			if (result != ISC_R_SUCCESS) {
-				continue;
-			}
-			if (dst_key_alg(key) != sig.algorithm ||
-			    dst_key_id(key) != sig.keyid ||
-			    !(dst_key_proto(key) == DNS_KEYPROTO_DNSSEC ||
-			      dst_key_proto(key) == DNS_KEYPROTO_ANY))
-			{
-				dst_key_free(&key);
-				continue;
-			}
-			result = dns_dnssec_verifymessage(&msgb, msg, key);
-			dst_key_free(&key);
-			if (result == ISC_R_SUCCESS) {
-				break;
-			}
-		}
-		if (result == ISC_R_NOMORE) {
-			result = DNS_R_KEYUNAUTHORIZED;
-		}
-
-	freesig:
-		if (dns_rdataset_isassociated(&keyset)) {
-			dns_rdataset_disassociate(&keyset);
-		}
-		dns_rdata_freestruct(&sig);
-		return (result);
+		return (dns_tsig_verify(&msgb, msg, NULL, NULL));
 	}
 }
 
diff --git a/lib/ns/client.c b/lib/ns/client.c
index 967e21b..87b8a18 100644
--- a/lib/ns/client.c
+++ b/lib/ns/client.c
@@ -2060,6 +2060,13 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult,
 		ns_client_log(client, DNS_LOGCATEGORY_SECURITY,
 			      NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(3),
 			      "request is signed by a nonauthoritative key");
+	} else if (result == DNS_R_NOTVERIFIEDYET &&
+		   client->message->sig0 != NULL)
+	{
+		ns_client_log(client, DNS_LOGCATEGORY_SECURITY,
+			      NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(3),
+			      "request has a SIG(0) signature but its support "
+			      "was removed (CVE-2024-1975)");
 	} else {
 		char tsigrcode[64];
 		isc_buffer_t b;
-- 
2.45.2