From be02bf65712ee54148496aac3edb3ca7d061327f Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Mon, 4 Jun 2018 21:46:23 -0700 Subject: [PATCH] allow-recursion could incorrectly inherit from the default allow-query --- CHANGES | 7 +++++++ bin/named/server.c | 43 ++++++++++++++++++++++++++++++++----------- doc/arm/notes.xml | 43 +++++++++++++++++++++++++++++-------------- 3 files changed, 68 insertions(+), 25 deletions(-) diff --git a/CHANGES b/CHANGES index 5e4d4eed2c..cc8fc845ed 100644 --- a/CHANGES +++ b/CHANGES @@ -24,6 +24,13 @@ 4962. [cleanup] Move 'named -T' processing to its own function. [GL #316] +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] + 4958. [bug] Remove redundant space from NSEC3 record. [GL #281] 4955. [cleanup] Silence cppcheck warnings in lib/dns/master.c. diff --git a/bin/named/server.c b/bin/named/server.c index 1618130339..a4fb2c25d6 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -3727,10 +3727,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. @@ -4699,21 +4695,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)); @@ -4751,18 +4761,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, @@ -4776,6 +4789,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 c4873008ca..fb6ec90e27 100644 --- a/doc/arm/notes.xml +++ b/doc/arm/notes.xml @@ -40,29 +40,31 @@
Security Fixes + + + 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] + + The serve-stale feature could cause an assertion failure in rbtdb.c even when stale-answer-enable was false. The simultaneous use of stale cache records and NSEC aggressive negative caching could trigger a recursion loop in the - named process. (CVE-2018-5737) [GL #185] + named process. This flaw is disclosed in + CVE-2018-5737. [GL #185] A bug in zone database reference counting could lead to a crash when multiple versions of a slave zone were transferred from a - master in close succession. (CVE-2018-5736) [GL #134] - - - - - 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. Previously, if the - name field was omitted from the rule declaration but a type list - was present, it wouldn't be interpreted as expected. + master in close succession. This flaw is disclosed in + CVE-2018-5736. [GL #134] @@ -72,9 +74,22 @@ - Add root key sentinel support which enables resolvers to test - which trust anchors are configured for the root. To disable, add - 'root-key-sentinel no;' to named.conf. [GL #37] + 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. Previously, if the + name field was omitted from the rule declaration but a type list + was present, it wouldn't be interpreted as expected. + + + + + named now supports the "root key sentinel" + mechanism. This enables validating resolvers to indicate + which trust anchors are configured for the root, so that + information about root key rollover status can be gathered. + To disable this feature, add + root-key-sentinel no; to + named.conf. [GL #37]