From c705a3eac69286b47a70b851aa5dd9119d04512f Mon Sep 17 00:00:00 2001
From: Petr Mensik <pemensik@redhat.com>
Date: Tue, 23 Jul 2019 16:43:55 +0200
Subject: [PATCH] Fix CVE-2018-5745
Squashed commit of the following:
commit c38e1dd10567e246bb802d889c3b2d2d286c7616
Author: Evan Hunt <each@isc.org>
Date: Fri Dec 21 17:24:47 2018 -0800
use algorithm 255 for both unsupported keys
(cherry picked from commit de8b2d4a6a97bb2ddf19024918581e70512ebc41)
commit caf8a62270c850fbc59cfa6bb9dcedb2ef7228c2
Author: Matthijs Mekking <matthijs@isc.org>
Date: Wed Dec 19 18:45:43 2018 +0100
Add tests for mkeys with unsupported algorithm
These tests check if a key with an unsupported algorithm in
managed-keys is ignored and when seeing an algorithm rollover to
an unsupported algorithm, the new key will be ignored too.
(cherry picked from commit 144cb53d0ae3aa5e6e3123720b603f9ab2bd1fa9)
(cherry picked from commit 8c2a8ca50946449bf26a7e0843cc5e54e36071ae)
commit 634655f38385595fb9a35e93ec3a72ed4c48bda6
Author: Matthijs Mekking <matthijs@isc.org>
Date: Wed Dec 19 18:47:43 2018 +0100
Update keyfetch_done compute_tag check
If in keyfetch_done the compute_tag fails (because for example the
algorithm is not supported), don't crash, but instead ignore the
key.
(cherry picked from commit b1d5411569ae10830b63f07560091193646cc739)
(cherry picked from commit 8f64928e2eb9395d8cdcd62183a1eaec3b1c5256)
commit e5cb28c3f3df4c37d528665e67fb460cc1662259
Author: Matthijs Mekking <github@pletterpet.nl>
Date: Wed Dec 12 14:06:10 2018 +0100
Don't free key in compute_tag in case of failure
If `dns_dnssec_keyfromrdata` failed we don't need to call
`dst_key_free` because no `dstkey` was created. Doing so
nevertheless will result in an assertion failure.
This can happen if the key uses an unsupported algorithm.
(cherry picked from commit 7a1ca39b950b7d5230b605ac60f15a1cb94e3d69)
(cherry picked from commit acae423ef4274c5535da324da78ce1441628d5f6)
---
bin/tests/system/mkeys/README | 3 +
bin/tests/system/mkeys/clean.sh | 2 +
bin/tests/system/mkeys/ns1/root.db | 20 +++----
bin/tests/system/mkeys/ns1/sign.sh | 7 ++-
bin/tests/system/mkeys/ns1/unsupported.key | 1 +
bin/tests/system/mkeys/ns6/named.args | 1 +
bin/tests/system/mkeys/ns6/named.conf.in | 43 +++++++++++++++
bin/tests/system/mkeys/ns6/setup.sh | 30 ++++++++++
.../system/mkeys/ns6/unsupported-managed.key | 1 +
bin/tests/system/mkeys/ns7/named.conf.in | 50 +++++++++++++++++
bin/tests/system/mkeys/setup.sh | 1 +
bin/tests/system/mkeys/tests.sh | 55 +++++++++++++++++++
lib/dns/include/dst/dst.h | 3 +-
lib/dns/zone.c | 27 ++++++++-
14 files changed, 229 insertions(+), 15 deletions(-)
create mode 100644 bin/tests/system/mkeys/ns1/unsupported.key
create mode 100644 bin/tests/system/mkeys/ns6/named.args
create mode 100644 bin/tests/system/mkeys/ns6/named.conf.in
create mode 100644 bin/tests/system/mkeys/ns6/setup.sh
create mode 100644 bin/tests/system/mkeys/ns6/unsupported-managed.key
create mode 100644 bin/tests/system/mkeys/ns7/named.conf.in
diff --git a/bin/tests/system/mkeys/README b/bin/tests/system/mkeys/README
index 700e6c21ca..257ef5406f 100644
--- a/bin/tests/system/mkeys/README
+++ b/bin/tests/system/mkeys/README
@@ -16,3 +16,6 @@ ns3 is a validator with a broken key in managed-keys.
ns5 is a validator which is prevented from getting a response from the
root server, causing key refresh queries to fail.
+
+ns6 is a validator which has unsupported algorithms, one at start up,
+one because of an algorithm rollover.
diff --git a/bin/tests/system/mkeys/clean.sh b/bin/tests/system/mkeys/clean.sh
index 17bd50f273..844d813eb4 100644
--- a/bin/tests/system/mkeys/clean.sh
+++ b/bin/tests/system/mkeys/clean.sh
@@ -11,6 +11,7 @@
rm -f */K* */*.signed */trusted.conf */*.jnl */*.bk
rm -f dsset-. ns1/dsset-.
+rm -f ns1/zone.key
rm -f ns*/named.lock
rm -f */managed-keys.bind* */named.secroots
rm -f */managed.conf ns1/managed.key ns1/managed.key.id
@@ -19,3 +20,4 @@ rm -f dig.out* delv.out* rndc.out* signer.out*
rm -f ns1/named.secroots ns1/root.db.signed* ns1/root.db.tmp
rm -f */named.conf
rm -f ns5/named.args
+rm -f ns7/view1.mkeys ns7/view2.mkeys
diff --git a/bin/tests/system/mkeys/ns1/root.db b/bin/tests/system/mkeys/ns1/root.db
index 6ba922af09..0070f13942 100644
--- a/bin/tests/system/mkeys/ns1/root.db
+++ b/bin/tests/system/mkeys/ns1/root.db
@@ -8,16 +8,16 @@
; information regarding copyright ownership.
$TTL 20
-. IN SOA gson.nominum.com. a.root.servers.nil. (
- 2000042100 ; serial
- 600 ; refresh
- 600 ; retry
- 1200 ; expire
- 2 ; minimum
- )
-. NS a.root-servers.nil.
-a.root-servers.nil. A 10.53.0.1
+. IN SOA gson.nominum.com. a.root.servers.nil. (
+ 2000042100 ; serial
+ 600 ; refresh
+ 600 ; retry
+ 1200 ; expire
+ 2 ; minimum
+ )
+. NS a.root-servers.nil.
+a.root-servers.nil. A 10.53.0.1
; no delegation
-example. TXT "This is a test."
+example. TXT "This is a test."
diff --git a/bin/tests/system/mkeys/ns1/sign.sh b/bin/tests/system/mkeys/ns1/sign.sh
index ccc7889ad9..e5e7ec05d6 100644
--- a/bin/tests/system/mkeys/ns1/sign.sh
+++ b/bin/tests/system/mkeys/ns1/sign.sh
@@ -25,13 +25,18 @@ keyfile_to_managed_keys $keyname > managed.conf
cp managed.conf ../ns2/managed.conf
cp managed.conf ../ns5/managed.conf
-# Configure a trusted key statement (used by delv)
+# Configure a trusted key statement (used by delv).
keyfile_to_trusted_keys $keyname > trusted.conf
+# Prepare an unsupported algorithm key.
+unsupportedkey=Kunknown.+255+00000
+cp unsupported.key "${unsupportedkey}.key"
+
#
# Save keyname and keyid for managed key id test.
#
echo "$keyname" > managed.key
+echo "$zskkeyname" > zone.key
keyid=`expr $keyname : 'K\.+00.+\([0-9]*\)'`
keyid=`expr $keyid + 0`
echo "$keyid" > managed.key.id
diff --git a/bin/tests/system/mkeys/ns1/unsupported.key b/bin/tests/system/mkeys/ns1/unsupported.key
new file mode 100644
index 0000000000..7435d03b63
--- /dev/null
+++ b/bin/tests/system/mkeys/ns1/unsupported.key
@@ -0,0 +1 @@
+. IN DNSKEY 257 3 255 BJiXuidPHuGIne8GlCBLG+Oq/FZruQd2s3uBo+SxY16NUP/Vwl8MctMK62KsblDU1gIJAdEMVep2tsOkuSm0bIbJ8NBex+N9rSvzH2YJlDCT9QnNfv4q5RRTcVA3lk9nkmWHo6zcAT33yuS+THOCSznOMCJRq8JGZ6xqMJLv9FucuK6CCe6QBAZ5e98dpyGTWQLu7AERKKFqda9YCk3KQfdzx/HZ4SpQpRLncIXvGm1PIMT8Ar95NB/BsFJGwr5ZTaQtRYOXf2DD7wD3pfMsTJCdZyC0J0EtGBG109I+Oou1cswUfqZLXip/aV3eaBAUqLcZpg8P8vAbrvEq4uMS4OMZeXL6nu0irrdS1Pqmax8RsC+x3fg9EBH3QmHroJZtiU5h+0x4qApp7HE4Z5zFRuxIp9iB
diff --git a/bin/tests/system/mkeys/ns6/named.args b/bin/tests/system/mkeys/ns6/named.args
new file mode 100644
index 0000000000..02f8f670f6
--- /dev/null
+++ b/bin/tests/system/mkeys/ns6/named.args
@@ -0,0 +1 @@
+-m record,size,mctx -T clienttest -c named.conf -d 99 -X named.lock -g -T mkeytimers=5/10/20
diff --git a/bin/tests/system/mkeys/ns6/named.conf.in b/bin/tests/system/mkeys/ns6/named.conf.in
new file mode 100644
index 0000000000..8d76f7f2e7
--- /dev/null
+++ b/bin/tests/system/mkeys/ns6/named.conf.in
@@ -0,0 +1,43 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+// NS6
+
+options {
+ query-source address 10.53.0.6;
+ notify-source 10.53.0.6;
+ transfer-source 10.53.0.6;
+ port @PORT@;
+ pid-file "named.pid";
+ listen-on { 10.53.0.6; };
+ listen-on-v6 { none; };
+ recursion yes;
+ notify no;
+ dnssec-enable yes;
+ dnssec-validation yes;
+ trust-anchor-telemetry no;
+};
+
+key rndc_key {
+ secret "1234abcd8765";
+ algorithm hmac-sha256;
+};
+
+controls {
+ inet 10.53.0.6 port @CONTROLPORT@ allow { any; } keys { rndc_key; };
+};
+
+zone "." {
+ type hint;
+ file "../../common/root.hint";
+};
+
+include "managed.conf";
diff --git a/bin/tests/system/mkeys/ns6/setup.sh b/bin/tests/system/mkeys/ns6/setup.sh
new file mode 100644
index 0000000000..5ba1647da5
--- /dev/null
+++ b/bin/tests/system/mkeys/ns6/setup.sh
@@ -0,0 +1,30 @@
+#!/bin/sh -e
+#
+# Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# See the COPYRIGHT file distributed with this work for additional
+# information regarding copyright ownership.
+
+SYSTEMTESTTOP=../..
+. $SYSTEMTESTTOP/conf.sh
+
+zone=.
+zonefile=root.db
+
+# an RSA key
+rsakey=`$KEYGEN -a rsasha256 -qfk rsasha256.`
+
+# a key with unsupported algorithm
+unsupportedkey=Kunknown.+255+00000
+cp unsupported-managed.key "${unsupportedkey}.key"
+
+# root key
+rootkey=`cat ../ns1/managed.key`
+cp "../ns1/${rootkey}.key" .
+
+# Configure the resolving server with a managed trusted key.
+keyfile_to_managed_keys $unsupportedkey $rsakey $rootkey > managed.conf
diff --git a/bin/tests/system/mkeys/ns6/unsupported-managed.key b/bin/tests/system/mkeys/ns6/unsupported-managed.key
new file mode 100644
index 0000000000..be872a00f0
--- /dev/null
+++ b/bin/tests/system/mkeys/ns6/unsupported-managed.key
@@ -0,0 +1 @@
+unsupported. IN DNSKEY 257 3 255 BOOVAhiJDPqhfU7+yGXjhetrtC/rtjmwO1yo52BUHUd8R4hQ/ZPdYCVvQlvNkRxDblPkFM5YRXkesS30pJSoNYrg+djbMNumJrLG+lbhFIc/ahTjlYOxb1zm2z00ubHju/1uGBifiRvKWSK0Vr0u6NtS4PKZfsnXt+piSHiRAHSfkjGHwqPYYKh9EUW12kJmIzlMaM6WYl+gJOvL+f8VqNLtvsMPT6OPK/3h/Dnfnxyeudp/jzAnNDDiTgX2XfzIXB4UwxtzIOGaHLnprpNf3zoBm0kyaEdSQQ/qKkpCOqjBasYEHRjVz3RncPUkdLr7PQuPBfFDr3SUMMJqufJrO4IJjtD4cCBT7K1i39Jg471nEzU1vkPzxF+Rw1QHT4nZaXbltf3BEZGS4Knoe9XPwi5KjGW6
diff --git a/bin/tests/system/mkeys/ns7/named.conf.in b/bin/tests/system/mkeys/ns7/named.conf.in
new file mode 100644
index 0000000000..a9aba00733
--- /dev/null
+++ b/bin/tests/system/mkeys/ns7/named.conf.in
@@ -0,0 +1,50 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+// NS7
+
+options {
+ query-source address 10.53.0.7;
+ notify-source 10.53.0.7;
+ transfer-source 10.53.0.7;
+ port @PORT@;
+ pid-file "named.pid";
+ listen-on { 10.53.0.7; };
+ listen-on-v6 { none; };
+ recursion yes;
+ notify no;
+ dnssec-enable yes;
+ dnssec-validation auto;
+ bindkeys-file "managed.conf";
+};
+
+key rndc_key {
+ secret "1234abcd8765";
+ algorithm hmac-sha256;
+};
+
+controls {
+ inet 10.53.0.7 port @CONTROLPORT@ allow { any; } keys { rndc_key; };
+};
+
+view view1 {
+ zone "." {
+ type hint;
+ file "../../common/root.hint";
+ };
+};
+
+view view2 {
+ zone "." {
+ type hint;
+ file "../../common/root.hint";
+ };
+};
diff --git a/bin/tests/system/mkeys/setup.sh b/bin/tests/system/mkeys/setup.sh
index bd3169f9b6..100a86959b 100644
--- a/bin/tests/system/mkeys/setup.sh
+++ b/bin/tests/system/mkeys/setup.sh
@@ -25,3 +25,4 @@ copy_setports ns5/named.conf.in ns5/named.conf
cp ns5/named1.args ns5/named.args
( cd ns1 && $SHELL sign.sh )
+( cd ns6 && $SHELL setup.sh )
diff --git a/bin/tests/system/mkeys/tests.sh b/bin/tests/system/mkeys/tests.sh
index f65f49e98d..b8410902d7 100644
--- a/bin/tests/system/mkeys/tests.sh
+++ b/bin/tests/system/mkeys/tests.sh
@@ -701,6 +701,8 @@ rm -f ns1/root.db.signed.jnl
nextpart ns5/named.run > /dev/null
mkeys_reconfig_on 1
wait_for_log "Returned from key fetch in keyfetch_done() for '.': success" ns5/named.run
+#mkeys_secroots_on 5
+#grep '; managed' ns5/named.secroots > /dev/null || ret=1
# ns1 should not longer REFUSE queries from ns5, so managed keys should be
# correctly refreshed and resolving should succeed
$DIG $DIGOPTS +noauth example. @10.53.0.5 txt > dig.out.ns5.b.test$n || ret=1
@@ -710,5 +712,58 @@ grep "status: NOERROR" dig.out.ns5.b.test$n > /dev/null || ret=1
if [ $ret != 0 ]; then echo_i "failed"; fi
status=`expr $status + $ret`
+echo_i "reinitialize trust anchors, add unsupported algorithm ($n)"
+ret=0
+$PERL $SYSTEMTESTTOP/stop.pl --use-rndc --port ${CONTROLPORT} mkeys ns6
+rm -f ns6/managed-keys.bind*
+nextpart ns6/named.run > /dev/null
+$PERL $SYSTEMTESTTOP/start.pl --noclean --restart --port ${PORT} mkeys ns6
+# log when an unsupported algorithm is encountered during startup
+wait_for_log "skipping managed key for 'unsupported\.': algorithm is unsupported" ns6/named.run
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
+n=`expr $n + 1`
+echo_i "skipping unsupported algorithm in managed-keys ($n)"
+ret=0
+mkeys_status_on 6 > rndc.out.$n 2>&1
+# there should still be only two keys listed (for . and rsasha256.)
+count=`grep -c "keyid: " rndc.out.$n`
+[ "$count" -eq 2 ] || ret=1
+# two lines indicating trust status
+count=`grep -c "trust" rndc.out.$n`
+[ "$count" -eq 2 ] || ret=1
+
+n=`expr $n + 1`
+echo_i "introduce unsupported algorithm rollover in authoritative zone ($n)"
+ret=0
+cp ns1/root.db ns1/root.db.orig
+ksk=`cat ns1/managed.key`
+zsk=`cat ns1/zone.key`
+cat "ns1/${ksk}.key" "ns1/${zsk}.key" ns1/unsupported.key >> ns1/root.db
+grep "\..*IN.*DNSKEY.*257 3 255" ns1/root.db > /dev/null || ret=1
+$SIGNER -K ns1 -N unixtime -o . ns1/root.db $ksk $zsk > /dev/null 2>/dev/null || ret=1
+grep "DNSKEY.*257 3 255" ns1/root.db.signed > /dev/null || ret=1
+cp ns1/root.db.orig ns1/root.db
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
+n=`expr $n + 1`
+echo_i "skipping unsupported algorithm in rollover ($n)"
+ret=0
+mkeys_reload_on 1
+mkeys_refresh_on 6
+mkeys_status_on 6 > rndc.out.$n 2>&1
+# there should still be only two keys listed (for . and rsasha256.)
+count=`grep -c "keyid: " rndc.out.$n`
+[ "$count" -eq 2 ] || ret=1
+# two lines indicating trust status
+count=`grep -c "trust" rndc.out.$n`
+[ "$count" -eq 2 ] || ret=1
+# log when an unsupported algorithm is encountered during rollover
+wait_for_log "Cannot compute tag for key in zone \.: algorithm is unsupported" ns6/named.run
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
echo_i "exit status: $status"
[ $status -eq 0 ] || exit 1
diff --git a/lib/dns/include/dst/dst.h b/lib/dns/include/dst/dst.h
index e8c1a3c287..91f4a6e300 100644
--- a/lib/dns/include/dst/dst.h
+++ b/lib/dns/include/dst/dst.h
@@ -67,8 +67,7 @@ typedef struct dst_context dst_context_t;
#define DST_ALG_HMACSHA512 165 /* XXXMPA */
#define DST_ALG_INDIRECT 252
#define DST_ALG_PRIVATE 254
-#define DST_ALG_EXPAND 255
-#define DST_MAX_ALGS 255
+#define DST_MAX_ALGS 256
/*% A buffer of this size is large enough to hold any key */
#define DST_KEY_MAXSIZE 1280
diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index 055b2417eb..96c98d585c 100644
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -3903,9 +3903,10 @@ compute_tag(dns_name_t *name, dns_rdata_dnskey_t *dnskey, isc_mem_t *mctx,
dns_rdatatype_dnskey, dnskey, &buffer);
result = dns_dnssec_keyfromrdata(name, &rdata, mctx, &dstkey);
- if (result == ISC_R_SUCCESS)
+ if (result == ISC_R_SUCCESS) {
*tag = dst_key_id(dstkey);
- dst_key_free(&dstkey);
+ dst_key_free(&dstkey);
+ }
return (result);
}
@@ -9364,6 +9365,17 @@ keyfetch_done(isc_task_t *task, isc_event_t *event) {
dns_keydata_todnskey(&keydata, &dnskey, NULL);
result = compute_tag(keyname, &dnskey, mctx, &keytag);
+ if (result != ISC_R_SUCCESS) {
+ /*
+ * Skip if we cannot compute the key tag.
+ * This may happen if the algorithm is unsupported
+ */
+ dns_zone_log(zone, ISC_LOG_ERROR,
+ "Cannot compute tag for key in zone %s: %s "
+ "(skipping)",
+ namebuf, dns_result_totext(result));
+ continue;
+ }
RUNTIME_CHECK(result == ISC_R_SUCCESS);
/*
@@ -9475,6 +9487,17 @@ keyfetch_done(isc_task_t *task, isc_event_t *event) {
continue;
result = compute_tag(keyname, &dnskey, mctx, &keytag);
+ if (result != ISC_R_SUCCESS) {
+ /*
+ * Skip if we cannot compute the key tag.
+ * This may happen if the algorithm is unsupported
+ */
+ dns_zone_log(zone, ISC_LOG_ERROR,
+ "Cannot compute tag for key in zone %s: %s "
+ "(skipping)",
+ namebuf, dns_result_totext(result));
+ continue;
+ }
RUNTIME_CHECK(result == ISC_R_SUCCESS);
revoked = ISC_TF(dnskey.flags & DNS_KEYFLAG_REVOKE);
--
2.20.1