From 7983015dfa58be21526a01300b9e13a84278266a Mon Sep 17 00:00:00 2001

From: Evan Hunt <each@isc.org>

Date: Mon, 4 Jun 2018 21:55:41 -0700

Subject: [PATCH] allow-recursion could incorrectly inherit from the default

 allow-query

---

 bin/named/server.c | 50 ++++++++++++++++++++++++++++++++++++--------------

 doc/arm/notes.xml  | 41 +++++------------------------------------

 2 files changed, 41 insertions(+), 50 deletions(-)

diff --git a/bin/named/server.c b/bin/named/server.c

index 64a5180..41a1826 100644

--- a/bin/named/server.c

+++ b/bin/named/server.c

@@ -3376,10 +3376,6 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist,

 		dns_acache_setcachesize(view->acache, max_acache_size);

 	}

-	CHECK(configure_view_acl(vconfig, config, ns_g_config,

-				 "allow-query", NULL, actx,

-				 ns_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.

@@ -4258,9 +4254,6 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist,

 	INSIST(result == ISC_R_SUCCESS);

 	view->trust_anchor_telemetry = cfg_obj_asboolean(obj);

-	CHECK(configure_view_acl(vconfig, config, ns_g_config,

-				 "allow-query-cache-on", NULL, actx,

-				 ns_g_mctx, &view->cacheonacl));

 	/*

 	 * Set sources where additional data and CNAME/DNAME

 	 * targets for authoritative answers may be found.

@@ -4287,22 +4280,40 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist,

 		view->additionalfromcache = ISC_TRUE;

 	}

+	CHECK(configure_view_acl(vconfig, config, ns_g_config,

+				 "allow-query-cache-on", NULL, actx,

+				 ns_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, ns_g_mctx, &view->cacheacl));

+

+	/* named.conf only */

+	CHECK(configure_view_acl(vconfig, config, NULL,

+				 "allow-query", NULL, actx,

+				 ns_g_mctx, &view->queryacl));

+

+	/* named.conf only */

+	CHECK(configure_view_acl(vconfig, config, NULL,

+				 "allow-query-cache", NULL, actx,

+				 ns_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,

 					 ns_g_mctx, &view->recursionacl));

+		/* named.conf only */

 		CHECK(configure_view_acl(vconfig, config, NULL,

 					 "allow-recursion-on", NULL, actx,

 					 ns_g_mctx, &view->recursiononacl));

@@ -4340,18 +4351,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, ns_g_config,

 						 "allow-recursion", NULL,

 						 actx, ns_g_mctx,

 						 &view->recursionacl));

 		}

 		if (view->recursiononacl == NULL) {

+			/* global default only */

 			CHECK(configure_view_acl(NULL, NULL, ns_g_config,

 						 "allow-recursion-on", NULL,

 						 actx, ns_g_mctx,

 						 &view->recursiononacl));

 		}

 		if (view->cacheacl == NULL) {

+			/* global default only */

 			CHECK(configure_view_acl(NULL, NULL, ns_g_config,

 						 "allow-query-cache", NULL,

 						 actx, ns_g_mctx,

@@ -4365,6 +4379,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, ns_g_config,

+					 "allow-query", NULL,

+					 actx, ns_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 ea9b61f..9a1832d 100644

--- a/doc/arm/notes.xml

+++ b/doc/arm/notes.xml

@@ -103,42 +103,11 @@

     <itemizedlist>

       <listitem>

 	<para>

-	  An error in TSIG handling could permit unauthorized zone

-	  transfers or zone updates. These flaws are disclosed in

-	  CVE-2017-3142 and CVE-2017-3143. [RT #45383]

-	</para>

-      </listitem>

-      <listitem>

-	<para>

-	  The BIND installer on Windows used an unquoted service path,

-	  which can enable privilege escalation. This flaw is disclosed

-	  in CVE-2017-3141. [RT #45229]

-	</para>

-      </listitem>

-      <listitem>

-	<para>

-	  With certain RPZ configurations, a response with TTL 0

-	  could cause <command>named</command> to go into an infinite

-	  query loop. This flaw is disclosed in CVE-2017-3140.

-	  [RT #45181]

-	</para>

-      </listitem>

-      <listitem>

-	<para>

-	  Addresses could be referenced after being freed during resolver

-	  processing, causing an assertion failure. The chances of this

-	  happening were remote, but the introduction of a delay in

-	  resolution increased them. This bug is disclosed in

-	  CVE-2017-3145. [RT #46839]

-	</para>

-      </listitem>

-      <listitem>

-	<para>

-	  update-policy rules that otherwise ignore the name field now

-	  require that it be set to "." to ensure that any type list

-	  present is properly interpreted.  If the name field was omitted

-	  from the rule declaration and a type list was present it wouldn't

-	  be interpreted as expected.

+	  When recursion is enabled but the <command>allow-recursion</command>

+	  and <command>allow-query-cache</command> ACLs are not specified, they

+	  should be limited to local networks, but they were inadvertently set

+	  to match the default <command>allow-query</command>, thus allowing

+	  remote queries. This flaw is disclosed in CVE-2018-5738. [GL #309]

 	</para>

       </listitem>

     </itemizedlist>

-- 

2.14.4