folkslinux
/
CBL-Mariner
mirror of https://github.com/microsoft/CBL-Mariner.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity
CBL-Mariner/SPECS/bind/CVE-2024-1975.patch

433 lines
16 KiB
Diff
Raw Blame History

From 6a4565566db942eb660e421786f73bc5baffc7b8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20=C5=A0pa=C4=8Dek?= <pspacek@isc.org>
Date: Thu, 16 May 2024 12:10:41 +0200
Subject: [PATCH 1/5] Remove support for SIG(0) message verification
(cherry picked from commit 857fd5c346e3309ee8e280c29174b46579af5a13)
Signed-off-by: Muhammad Falak R Wani <falakreyaz@gmail.com>
---
lib/dns/message.c | 99 +++--------------------------------------------
lib/ns/client.c | 7 ++++
2 files changed, 13 insertions(+), 93 deletions(-)
diff --git a/lib/dns/message.c b/lib/dns/message.c
index 22aa552..12331ab 100644
--- a/lib/dns/message.c
+++ b/lib/dns/message.c
@@ -3301,111 +3301,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, NULL);
- if (result != ISC_R_SUCCESS) {
- return (result);
- }
-
- dns_rdataset_init(&keyset);
- if (view == NULL) {
- result = DNS_R_KEYUNAUTHORIZED;
- goto freesig;
- }
- 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 d4ce000..2679a5e 100644
--- a/lib/ns/client.c
+++ b/lib/ns/client.c
@@ -2041,6 +2041,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.40.1
From afd9c8976d78a5145a92ff0cccc2954083042555 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20=C5=A0pa=C4=8Dek?= <pspacek@isc.org>
Date: Thu, 16 May 2024 12:15:23 +0200
Subject: [PATCH 2/5] Document SIG(0) verification removal
(cherry picked from commit 654ba34d80b8b6ed805461d7ada2466f8c19a6f1)
Signed-off-by: Muhammad Falak R Wani <falakreyaz@gmail.com>
---
doc/arm/advanced.rst | 18 +++---------------
doc/arm/general.rst | 6 ++----
doc/arm/reference.rst | 4 ++--
doc/arm/security.rst | 4 ++--
4 files changed, 9 insertions(+), 23 deletions(-)
diff --git a/doc/arm/advanced.rst b/doc/arm/advanced.rst
index 4405b5c..f3325d9 100644
--- a/doc/arm/advanced.rst
+++ b/doc/arm/advanced.rst
@@ -537,7 +537,7 @@ zone).
The TKEY process is initiated by a client or server by sending a query
of type TKEY to a TKEY-aware server. The query must include an
appropriate KEY record in the additional section, and must be signed
-using either TSIG or SIG(0) with a previously established key. The
+using either TSIG with a previously established key. The
server's response, if successful, contains a TKEY record in its
answer section. After this transaction, both participants have
enough information to calculate a shared secret using Diffie-Hellman key
@@ -555,20 +555,8 @@ deletion" mode.
SIG(0)
------
-BIND partially supports DNSSEC SIG(0) transaction signatures as
-specified in :rfc:`2535` and :rfc:`2931`. SIG(0) uses public/private keys to
-authenticate messages. Access control is performed in the same manner as with
-TSIG keys; privileges can be granted or denied in ACL directives based
-on the key name.
-
-When a SIG(0) signed message is received, it is only verified if
-the key is known and trusted by the server. The server does not attempt
-to recursively fetch or validate the key.
-
-SIG(0) signing of multiple-message TCP streams is not supported.
-
-The only tool shipped with BIND 9 that generates SIG(0) signed messages
-is ``nsupdate``.
+Support for DNSSEC SIG(0) transaction signatures was removed.
+This is a countermeasure for CVE-2024-1975.
.. include:: managed-keys.rst
.. include:: pkcs11.rst
diff --git a/doc/arm/general.rst b/doc/arm/general.rst
index d7b7c20..136e806 100644
--- a/doc/arm/general.rst
+++ b/doc/arm/general.rst
@@ -367,10 +367,8 @@ Notes
.. [#rfc1035_2] CLASS ANY queries are not supported. This is considered a
feature.
-.. [#rfc2931] 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.
+.. [#rfc2931] Support for SIG(0) message verification was removed
+ as a countermeasure for CVE-2024-1975.
.. [#rfc2874] 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 ecc84d4..f982e0a 100644
--- a/doc/arm/reference.rst
+++ b/doc/arm/reference.rst
@@ -5900,7 +5900,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.
@@ -5957,7 +5957,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 817ebd0..92b1668 100644
--- a/doc/arm/security.rst
+++ b/doc/arm/security.rst
@@ -83,7 +83,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.
@@ -124,7 +124,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
--
2.40.1
From d58461e425e61c1740ff4e914c7d41513c972850 Mon Sep 17 00:00:00 2001
From: Aram Sargsyan <aram@isc.org>
Date: Tue, 21 May 2024 08:45:48 +0000
Subject: [PATCH 3/5] Enable stdout autoflush in authsock.pl
With enabled buffering the output gets lost when the process
receives a TERM signal. Disable the buffering.
(cherry picked from commit a0311dfb6e2a51f89dfa8b200b96a0f4675fb654)
Signed-off-by: Muhammad Falak R Wani <falakreyaz@gmail.com>
---
bin/tests/system/tsiggss/authsock.pl | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/bin/tests/system/tsiggss/authsock.pl b/bin/tests/system/tsiggss/authsock.pl
index d629c65..d181b1a 100644
--- a/bin/tests/system/tsiggss/authsock.pl
+++ b/bin/tests/system/tsiggss/authsock.pl
@@ -33,6 +33,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";
--
2.40.1
From d8431d0c68df185077cf656edf46a985f3291a30 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20=C5=A0pa=C4=8Dek?= <pspacek@isc.org>
Date: Fri, 17 May 2024 12:23:05 +0200
Subject: [PATCH 4/5] Adapt the tsiggss test to the SIG(0) removal
Test that SIG(0) signer is NOT sent to the external socket for
authorization. It MUST NOT be considered a valid signature by
any chance.
Also check that the signer's name does not appear in authsock.pl
output.
(cherry picked from commit cf8838085905171fbc00747eb210e8b8284ca0e1)
Signed-off-by: Muhammad Falak R Wani <falakreyaz@gmail.com>
---
bin/tests/system/tsiggss/authsock.pl | 1 +
bin/tests/system/tsiggss/clean.sh | 2 +-
bin/tests/system/tsiggss/tests.sh | 12 +++++++-----
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/bin/tests/system/tsiggss/authsock.pl b/bin/tests/system/tsiggss/authsock.pl
index d181b1a..b3888fb 100644
--- a/bin/tests/system/tsiggss/authsock.pl
+++ b/bin/tests/system/tsiggss/authsock.pl
@@ -59,6 +59,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/clean.sh b/bin/tests/system/tsiggss/clean.sh
index 0ace209..ce885d5 100644
--- a/bin/tests/system/tsiggss/clean.sh
+++ b/bin/tests/system/tsiggss/clean.sh
@@ -21,7 +21,7 @@ rm -f ns1/_default.tsigkeys
rm -f */named.memstats
rm -f */named.conf
rm -f */named.run
-rm -f authsock.pid
+rm -f authsock.log authsock.pid
rm -f ns1/core
rm -f nsupdate.out*
rm -f ns*/named.lock
diff --git a/bin/tests/system/tsiggss/tests.sh b/bin/tests/system/tsiggss/tests.sh
index a665703..34b8c89 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))
--
2.40.1
From db69c8bb093a19eafb016b14aff45b69803f0065 Mon Sep 17 00:00:00 2001
From: Aram Sargsyan <aram@isc.org>
Date: Tue, 21 May 2024 09:29:35 +0000
Subject: [PATCH 5/5] Adapt the upforwd test to the SIG(0) removal
Change the check so that update with SIG(0) is expected to fail.
(cherry picked from commit 5f7558f6dbb0527c08caf281299245ab8de268cd)
Signed-off-by: Muhammad Falak R Wani <falakreyaz@gmail.com>
---
bin/tests/system/upforwd/tests.sh | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/bin/tests/system/upforwd/tests.sh b/bin/tests/system/upforwd/tests.sh
index 9165ba9..89e2241 100644
--- a/bin/tests/system/upforwd/tests.sh
+++ b/bin/tests/system/upforwd/tests.sh
@@ -262,10 +262,12 @@ if $FEATURETEST --enable-dnstap; then
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
+ # 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
@@ -273,8 +275,9 @@ if test -f keyname; then
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 || ret=1
- 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)
--
2.40.1
Reference in New Issue View Git Blame Copy Permalink