From 7630a64141a997b5247d9ad4a7dfff6ac6d9a485 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 29 Jul 2020 23:36:03 +1000 Subject: [PATCH 1/4] Update-policy 'subdomain' was incorrectly treated as 'zonesub' resulting in names outside the specified subdomain having the wrong restrictions for the given key. --- bin/named/zoneconf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c index 111d4d4b3b..1a4777f09b 100644 --- a/bin/named/zoneconf.c +++ b/bin/named/zoneconf.c @@ -252,7 +252,8 @@ configure_zone_ssutable(const cfg_obj_t *zconfig, dns_zone_t *zone, str = cfg_obj_asstring(matchtype); CHECK(dns_ssu_mtypefromstring(str, &mtype)); - if (mtype == dns_ssumatchtype_subdomain) { + if (mtype == dns_ssumatchtype_subdomain && + strcasecmp(str, "zonesub") == 0) { usezone = true; } From 5bf457e89a3fdc355aad74140f5e010b42d1df82 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 29 Jul 2020 23:36:03 +1000 Subject: [PATCH 2/4] Add a test for update-policy 'subdomain' The new test checks that 'update-policy subdomain' is properly enforced. --- bin/tests/system/nsupdate/ns1/named.conf.in | 6 +++++ bin/tests/system/nsupdate/tests.sh | 25 +++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/bin/tests/system/nsupdate/ns1/named.conf.in b/bin/tests/system/nsupdate/ns1/named.conf.in index e7b6adb39e..53dbe56907 100644 --- a/bin/tests/system/nsupdate/ns1/named.conf.in +++ b/bin/tests/system/nsupdate/ns1/named.conf.in @@ -37,6 +37,11 @@ key altkey { secret "1234abcd8765"; }; +key restricted.example.nil { + algorithm hmac-md5; + secret "1234abcd8765"; +}; + include "ddns.key"; zone "example.nil" { @@ -46,6 +51,7 @@ zone "example.nil" { check-mx ignore; update-policy { grant ddns-key.example.nil subdomain example.nil ANY; + grant restricted.example.nil subdomain restricted.example.nil ANY; }; allow-transfer { any; }; }; diff --git a/bin/tests/system/nsupdate/tests.sh b/bin/tests/system/nsupdate/tests.sh index 018d5dd54f..89603e5bc2 100755 --- a/bin/tests/system/nsupdate/tests.sh +++ b/bin/tests/system/nsupdate/tests.sh @@ -640,6 +640,31 @@ then echo_i "failed"; status=1 fi +n=`expr $n + 1` +ret=0 +echo_i "check that 'update-policy subdomain' is properly enforced ($n)" +# "restricted.example.nil" matches "grant ... subdomain restricted.example.nil" +# and thus this UPDATE should succeed. +$NSUPDATE -d < nsupdate.out1-$n 2>&1 || ret=1 +server 10.53.0.1 ${PORT} +key restricted.example.nil 1234abcd8765 +update add restricted.example.nil 0 IN TXT everywhere. +send +END +$DIG $DIGOPTS +tcp @10.53.0.1 restricted.example.nil TXT > dig.out.1.test$n || ret=1 +grep "TXT.*everywhere" dig.out.1.test$n > /dev/null || ret=1 +# "example.nil" does not match "grant ... subdomain restricted.example.nil" and +# thus this UPDATE should fail. +$NSUPDATE -d < nsupdate.out2-$n 2>&1 && ret=1 +server 10.53.0.1 ${PORT} +key restricted.example.nil 1234abcd8765 +update add example.nil 0 IN TXT everywhere. +send +END +$DIG $DIGOPTS +tcp @10.53.0.1 example.nil TXT > dig.out.2.test$n || ret=1 +grep "TXT.*everywhere" dig.out.2.test$n > /dev/null && ret=1 +[ $ret = 0 ] || { echo_i "failed"; status=1; } + n=`expr $n + 1` ret=0 echo_i "check that changes to the DNSKEY RRset TTL do not have side effects ($n)" From 14aa0c5df65d28cf6aaf437151c6a008afb66fb1 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 4 Aug 2020 11:41:33 +1000 Subject: [PATCH 3/4] Add a test for update-policy 'zonesub' The new test checks that 'update-policy zonesub' is properly enforced. --- bin/tests/system/nsupdate/ns1/named.conf.in | 6 ++++ bin/tests/system/nsupdate/tests.sh | 35 ++++++++++++++++++--- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/bin/tests/system/nsupdate/ns1/named.conf.in b/bin/tests/system/nsupdate/ns1/named.conf.in index 53dbe56907..346b6472fa 100644 --- a/bin/tests/system/nsupdate/ns1/named.conf.in +++ b/bin/tests/system/nsupdate/ns1/named.conf.in @@ -42,6 +42,11 @@ key restricted.example.nil { secret "1234abcd8765"; }; +key zonesub-key.example.nil { + algorithm hmac-md5; + secret "1234subk8765"; +}; + include "ddns.key"; zone "example.nil" { @@ -50,6 +55,7 @@ zone "example.nil" { check-integrity no; check-mx ignore; update-policy { + grant zonesub-key.example.nil zonesub TXT; grant ddns-key.example.nil subdomain example.nil ANY; grant restricted.example.nil subdomain restricted.example.nil ANY; }; diff --git a/bin/tests/system/nsupdate/tests.sh b/bin/tests/system/nsupdate/tests.sh index 89603e5bc2..36ade49c76 100755 --- a/bin/tests/system/nsupdate/tests.sh +++ b/bin/tests/system/nsupdate/tests.sh @@ -430,7 +430,7 @@ EOF # this also proves that the server is still running. $DIG $DIGOPTS +tcp +noadd +nosea +nostat +noquest +nocmd +norec example.\ @10.53.0.3 nsec3param > dig.out.ns3.$n || ret=1 -grep "ANSWER: 0" dig.out.ns3.$n > /dev/null || ret=1 +grep "ANSWER: 0," dig.out.ns3.$n > /dev/null || ret=1 grep "flags:[^;]* aa[ ;]" dig.out.ns3.$n > /dev/null || ret=1 [ $ret = 0 ] || { echo_i "failed"; status=1; } @@ -445,7 +445,7 @@ EOF $DIG $DIGOPTS +tcp +noadd +nosea +nostat +noquest +nocmd +norec nsec3param.test.\ @10.53.0.3 nsec3param > dig.out.ns3.$n || ret=1 -grep "ANSWER: 1" dig.out.ns3.$n > /dev/null || ret=1 +grep "ANSWER: 1," dig.out.ns3.$n > /dev/null || ret=1 grep "3600.*NSEC3PARAM" dig.out.ns3.$n > /dev/null || ret=1 grep "flags:[^;]* aa[ ;]" dig.out.ns3.$n > /dev/null || ret=1 [ $ret = 0 ] || { echo_i "failed"; status=1; } @@ -462,7 +462,7 @@ EOF _ret=1 for i in 0 1 2 3 4 5 6 7 8 9; do $DIG $DIGOPTS +tcp +norec +time=1 +tries=1 @10.53.0.3 nsec3param.test. NSEC3PARAM > dig.out.ns3.$n || _ret=1 - if grep "ANSWER: 2" dig.out.ns3.$n > /dev/null; then + if grep "ANSWER: 2," dig.out.ns3.$n > /dev/null; then _ret=0 break fi @@ -487,7 +487,7 @@ EOF _ret=1 for i in 0 1 2 3 4 5 6 7 8 9; do $DIG $DIGOPTS +tcp +norec +time=1 +tries=1 @10.53.0.3 nsec3param.test. NSEC3PARAM > dig.out.ns3.$n || _ret=1 - if grep "ANSWER: 1" dig.out.ns3.$n > /dev/null; then + if grep "ANSWER: 1," dig.out.ns3.$n > /dev/null; then _ret=0 break fi @@ -665,6 +665,33 @@ $DIG $DIGOPTS +tcp @10.53.0.1 example.nil TXT > dig.out.2.test$n || ret=1 grep "TXT.*everywhere" dig.out.2.test$n > /dev/null && ret=1 [ $ret = 0 ] || { echo_i "failed"; status=1; } +n=`expr $n + 1` +ret=0 +echo_i "check that 'update-policy zonesub' is properly enforced ($n)" +# grant zonesub-key.example.nil zonesub TXT; +# the A record update should be rejected as it is not in the type list +$NSUPDATE -d < nsupdate.out1-$n 2>&1 && ret=1 +server 10.53.0.1 ${PORT} +key zonesub-key.example.nil 1234subk8765 +update add zonesub.example.nil 0 IN A 1.2.3.4 +send +END +$DIG $DIGOPTS +tcp @10.53.0.1 zonesub.example.nil A > dig.out.1.test$n || ret=1 +grep "status: REFUSED" nsupdate.out1-$n > /dev/null || ret=1 +grep "ANSWER: 0," dig.out.1.test$n > /dev/null || ret=1 +# the TXT record update should be accepted as it is in the type list +$NSUPDATE -d < nsupdate.out2-$n 2>&1 || ret=1 +server 10.53.0.1 ${PORT} +key zonesub-key.example.nil 1234subk8765 +update add zonesub.example.nil 0 IN TXT everywhere. +send +END +$DIG $DIGOPTS +tcp @10.53.0.1 zonesub.example.nil TXT > dig.out.2.test$n || ret=1 +grep "status: REFUSED" nsupdate.out2-$n > /dev/null && ret=1 +grep "ANSWER: 1," dig.out.2.test$n > /dev/null || ret=1 +grep "TXT.*everywhere" dig.out.2.test$n > /dev/null || ret=1 +[ $ret = 0 ] || { echo_i "failed"; status=1; } + n=`expr $n + 1` ret=0 echo_i "check that changes to the DNSKEY RRset TTL do not have side effects ($n)" From b3b46c58425aac102c8b83273c93d0ab58cb2646 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 29 Jul 2020 23:36:03 +1000 Subject: [PATCH 4/4] Add CHANGES and release note for GL #2055 --- CHANGES | 7 +++++++ doc/notes/notes-current.rst | 9 +++++++++ 2 files changed, 16 insertions(+) diff --git a/CHANGES b/CHANGES index 3bc5cf42d3..6e182ee0ca 100644 --- a/CHANGES +++ b/CHANGES @@ -10,6 +10,13 @@ system, but the Duplicate Address Detection (DAD) mechanism had not yet finished. [GL #2038] +5481. [security] "update-policy" rules of type "subdomain" were + incorrectly treated as "zonesub" rules, which allowed + keys used in "subdomain" rules to update names outside + of the specified subdomains. The problem was fixed by + making sure "subdomain" rules are again processed as + described in the ARM. (CVE-2020-8624) [GL #2055] + 5480. [security] When BIND 9 was compiled with native PKCS#11 support, it was possible to trigger an assertion failure in code determining the number of bits in the PKCS#11 RSA public diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index f93d125abf..96791435a4 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -44,6 +44,15 @@ Security Fixes ISC would like to thank Lyu Chiy for bringing this vulnerability to our attention. [GL #2037] +- ``update-policy`` rules of type ``subdomain`` were incorrectly treated + as ``zonesub`` rules, which allowed keys used in ``subdomain`` rules + to update names outside of the specified subdomains. The problem was + fixed by making sure ``subdomain`` rules are again processed as + described in the ARM. This was disclosed in CVE-2020-8624. + + ISC would like to thank Joop Boonen of credativ GmbH for bringing this + vulnerability to our attention. [GL #2055] + Known Issues ~~~~~~~~~~~~