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