From 03ecba2cdc8d9a6cb6bdf863ffa1e230cb4ff223 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Mon, 4 Jun 2018 15:57:58 -0700 Subject: [PATCH 1/2] allow-recursion could incorrectly inherit from the default allow-query --- CHANGES | 7 ++++++- bin/named/server.c | 43 ++++++++++++++++++++++++++++++++----------- doc/arm/notes.xml | 8 ++++++-- 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/CHANGES b/CHANGES index c09900074d..a6c65eca9f 100644 --- a/CHANGES +++ b/CHANGES @@ -29,7 +29,12 @@ 4961. [protocol] Remove support for ECC-GOST (GOST R 34.11-94). [GL #295] -4960. [placeholder] +4960. [security] When recursion is enabled, but the "allow-recursion" + and "allow-query-cache" ACLs are not specified, + they should be limited to local networks, + but were inadvertently set to match the default + "allow-query", thus allowing remote queries. + (CVE-2018-5738) [GL #309] 4959. [func] NSID logging (enabled by the "request-nsid" option) now has its own "nsid" category, instead of using the diff --git a/bin/named/server.c b/bin/named/server.c index c8ff38e4c9..8644581df9 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -3726,10 +3726,6 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, CHECKM(named_config_getport(config, &port), "port"); dns_view_setdstport(view, port); - CHECK(configure_view_acl(vconfig, config, named_g_config, - "allow-query", NULL, actx, - named_g_mctx, &view->queryacl)); - /* * Make the list of response policy zone names for a view that * is used for real lookups and so cares about hints. @@ -4714,21 +4710,35 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, "allow-query-cache-on", NULL, actx, named_g_mctx, &view->cacheonacl)); /* - * Set "allow-query-cache", "allow-recursion", and - * "allow-recursion-on" acls if configured in named.conf. - * (Ignore the global defaults for now, because these ACLs - * can inherit from each other when only some of them set at - * the options/view level.) + * Set the "allow-query", "allow-query-cache", "allow-recursion", + * and "allow-recursion-on" ACLs if configured in named.conf, but + * NOT from the global defaults. This is done by leaving the third + * argument to configure_view_acl() NULL. + * + * We ignore the global defaults here because these ACLs + * can inherit from each other. If any are still unset after + * applying the inheritance rules, we'll look up the defaults at + * that time. */ - CHECK(configure_view_acl(vconfig, config, NULL, "allow-query-cache", - NULL, actx, named_g_mctx, &view->cacheacl)); + + /* named.conf only */ + CHECK(configure_view_acl(vconfig, config, NULL, + "allow-query", NULL, actx, + named_g_mctx, &view->queryacl)); + + /* named.conf only */ + CHECK(configure_view_acl(vconfig, config, NULL, + "allow-query-cache", NULL, actx, + named_g_mctx, &view->cacheacl)); if (strcmp(view->name, "_bind") != 0 && view->rdclass != dns_rdataclass_chaos) { + /* named.conf only */ CHECK(configure_view_acl(vconfig, config, NULL, "allow-recursion", NULL, actx, named_g_mctx, &view->recursionacl)); + /* named.conf only */ CHECK(configure_view_acl(vconfig, config, NULL, "allow-recursion-on", NULL, actx, named_g_mctx, &view->recursiononacl)); @@ -4766,18 +4776,21 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, * the global config. */ if (view->recursionacl == NULL) { + /* global default only */ CHECK(configure_view_acl(NULL, NULL, named_g_config, "allow-recursion", NULL, actx, named_g_mctx, &view->recursionacl)); } if (view->recursiononacl == NULL) { + /* global default only */ CHECK(configure_view_acl(NULL, NULL, named_g_config, "allow-recursion-on", NULL, actx, named_g_mctx, &view->recursiononacl)); } if (view->cacheacl == NULL) { + /* global default only */ CHECK(configure_view_acl(NULL, NULL, named_g_config, "allow-query-cache", NULL, actx, named_g_mctx, @@ -4791,6 +4804,14 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, CHECK(dns_acl_none(mctx, &view->cacheacl)); } + if (view->queryacl == NULL) { + /* global default only */ + CHECK(configure_view_acl(NULL, NULL, named_g_config, + "allow-query", NULL, + actx, named_g_mctx, + &view->queryacl)); + } + /* * Ignore case when compressing responses to the specified * clients. This causes case not always to be preserved, diff --git a/doc/arm/notes.xml b/doc/arm/notes.xml index abc681dd28..627a3f9f50 100644 --- a/doc/arm/notes.xml +++ b/doc/arm/notes.xml @@ -65,7 +65,11 @@ - None. + When recursion is enabled but the allow-recursion + and allow-query-cache ACLs are not specified, they + should be limited to local networks, but they were inadvertently set + to match the default allow-query, thus allowing + remote queries. This flaw is disclosed in CVE-2018-5738. [GL #309] @@ -89,7 +93,7 @@ information about root key rollover status can be gathered. To disable this feature, add root-key-sentinel no; to - named.conf. + named.conf. [GL #37] From 05f25f8970856323dd7f4f685e1426f007509f5f Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 6 Jun 2018 15:54:49 -0700 Subject: [PATCH 2/2] add a regression test for default allow-recursion settings --- bin/tests/system/allow-query/clean.sh | 3 ++- .../system/allow-query/ns1/named.conf.in | 23 ++++++++++++++++++ bin/tests/system/allow-query/ns1/root.db | 16 +++++++++++++ bin/tests/system/allow-query/ns3/named.args | 2 ++ .../system/allow-query/ns3/named.conf.in | 24 +++++++++++++++++++ bin/tests/system/allow-query/setup.sh | 2 ++ bin/tests/system/allow-query/tests.sh | 11 +++++++++ util/copyrights | 4 ++++ 8 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 bin/tests/system/allow-query/ns1/named.conf.in create mode 100644 bin/tests/system/allow-query/ns1/root.db create mode 100644 bin/tests/system/allow-query/ns3/named.args create mode 100644 bin/tests/system/allow-query/ns3/named.conf.in diff --git a/bin/tests/system/allow-query/clean.sh b/bin/tests/system/allow-query/clean.sh index 17ec1a815f..ae7d59b27a 100644 --- a/bin/tests/system/allow-query/clean.sh +++ b/bin/tests/system/allow-query/clean.sh @@ -14,7 +14,8 @@ # rm -f dig.out.* -rm -f ns2/named.conf ns2/controls.conf +rm -f ns*/named.conf +rm -f ns2/controls.conf rm -f */named.memstats rm -f ns*/named.lock rm -f ns*/named.run ns*/named.run.prev diff --git a/bin/tests/system/allow-query/ns1/named.conf.in b/bin/tests/system/allow-query/ns1/named.conf.in new file mode 100644 index 0000000000..607925b26e --- /dev/null +++ b/bin/tests/system/allow-query/ns1/named.conf.in @@ -0,0 +1,23 @@ +/* + * 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. + */ + +options { + port @PORT@; + pid-file "named.pid"; + listen-on { 10.53.0.1; }; + listen-on-v6 { none; }; + recursion no; +}; + +zone "." { + type master; + file "root.db"; +}; diff --git a/bin/tests/system/allow-query/ns1/root.db b/bin/tests/system/allow-query/ns1/root.db new file mode 100644 index 0000000000..0040271648 --- /dev/null +++ b/bin/tests/system/allow-query/ns1/root.db @@ -0,0 +1,16 @@ +; 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. + +$TTL 300 +@ SOA ns1. hostmaster.localhost. 1 3600 1200 604800 3600 + NS ns1. +ns1. A 10.53.0.1 + +normal.example. NS ns2.normal.example. +ns2.normal.example. A 10.53.0.2 diff --git a/bin/tests/system/allow-query/ns3/named.args b/bin/tests/system/allow-query/ns3/named.args new file mode 100644 index 0000000000..a8f679b8a2 --- /dev/null +++ b/bin/tests/system/allow-query/ns3/named.args @@ -0,0 +1,2 @@ +# this server only has 127.0.0.1 in its localhost/localnets ACLs +-m record,size,mctx -c named.conf -d 99 -X named.lock -g -T clienttest -T fixedlocal diff --git a/bin/tests/system/allow-query/ns3/named.conf.in b/bin/tests/system/allow-query/ns3/named.conf.in new file mode 100644 index 0000000000..6eace597bb --- /dev/null +++ b/bin/tests/system/allow-query/ns3/named.conf.in @@ -0,0 +1,24 @@ +/* + * 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. + */ + +options { + port @PORT@; + pid-file "named.pid"; + listen-on { 10.53.0.3; }; + listen-on-v6 { none; }; + recursion yes; + dnssec-validation no; +}; + +zone "." { + type hint; + file "../../common/root.hint"; +}; diff --git a/bin/tests/system/allow-query/setup.sh b/bin/tests/system/allow-query/setup.sh index 3d2c2c82c4..10b3c934a5 100644 --- a/bin/tests/system/allow-query/setup.sh +++ b/bin/tests/system/allow-query/setup.sh @@ -13,4 +13,6 @@ SYSTEMTESTTOP=.. . $SYSTEMTESTTOP/conf.sh copy_setports ../common/controls.conf.in ns2/controls.conf +copy_setports ns1/named.conf.in ns1/named.conf copy_setports ns2/named01.conf.in ns2/named.conf +copy_setports ns3/named.conf.in ns3/named.conf diff --git a/bin/tests/system/allow-query/tests.sh b/bin/tests/system/allow-query/tests.sh index f36e5db611..fb6059d5b8 100644 --- a/bin/tests/system/allow-query/tests.sh +++ b/bin/tests/system/allow-query/tests.sh @@ -602,5 +602,16 @@ grep '^a.aclnotallow.example' dig.out.ns2.2.$n > /dev/null && ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=`expr $status + $ret` +# Test 58 - allow-recursion inheritance +n=`expr $n + 1` +echo_i "test $n: default recursion configuration" +ret=0 +$DIG -p ${PORT} @10.53.0.3 -b 127.0.0.1 a.normal.example a > dig.out.ns3.1.$n +grep 'status: NOERROR' dig.out.ns3.1.$n > /dev/null || ret=1 +$DIG -p ${PORT} @10.53.0.3 -b 10.53.0.1 a.normal.example a > dig.out.ns3.2.$n +grep 'status: REFUSED' dig.out.ns3.2.$n > /dev/null || ret=1 +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/util/copyrights b/util/copyrights index 3759639638..3d2aa50290 100644 --- a/util/copyrights +++ b/util/copyrights @@ -461,6 +461,8 @@ ./bin/tests/system/addzone/tests.sh SH 2010,2011,2012,2013,2014,2015,2016,2017,2018 ./bin/tests/system/allow-query/.gitignore X 2018 ./bin/tests/system/allow-query/clean.sh SH 2010,2012,2014,2015,2016,2018 +./bin/tests/system/allow-query/ns1/named.conf.in CONF-C 2018 +./bin/tests/system/allow-query/ns1/root.db ZONE 2018 ./bin/tests/system/allow-query/ns2/generic.db ZONE 2018 ./bin/tests/system/allow-query/ns2/named01.conf.in CONF-C 2010,2016,2018 ./bin/tests/system/allow-query/ns2/named02.conf.in CONF-C 2010,2016,2018 @@ -494,6 +496,8 @@ ./bin/tests/system/allow-query/ns2/named55.conf.in CONF-C 2010,2016,2018 ./bin/tests/system/allow-query/ns2/named56.conf.in CONF-C 2010,2016,2018 ./bin/tests/system/allow-query/ns2/named57.conf.in CONF-C 2013,2016,2018 +./bin/tests/system/allow-query/ns3/named.args X 2018 +./bin/tests/system/allow-query/ns3/named.conf.in CONF-C 2018 ./bin/tests/system/allow-query/setup.sh SH 2010,2012,2016,2018 ./bin/tests/system/allow-query/tests.sh SH 2010,2012,2013,2016,2018 ./bin/tests/system/ans.pl PERL 2011,2012,2014,2016,2017,2018