From: Evan Hunt Date: Tue, 19 May 2015 22:47:42 +0000 (-0700) Subject: [master] address a possible policy update race X-Git-Tag: v9.11.0a1~808 X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=7e6cf6fc6e700061a1cec3bcf67786706d956fc5;p=thirdparty%2Fbind9.git [master] address a possible policy update race 4120. [bug] A bug in RPZ could cause the server to crash if policy zones were updated while recursion was pending for RPZ processing of an active query. [RT #39415] --- diff --git a/CHANGES b/CHANGES index 0066c52ea89..5b2929a7ab5 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +4120. [bug] A bug in RPZ could cause the server to crash if + policy zones were updated while recursion was + pending for RPZ processing of an active query. + [RT #39415] + 4119. [test] Allow dig to set the message opcode. [RT #39550] 4118. [bug] Teach isc-config.sh about irs. [RT #39213] diff --git a/bin/named/query.c b/bin/named/query.c index 0cf832cd43c..506c0b1430c 100644 --- a/bin/named/query.c +++ b/bin/named/query.c @@ -189,6 +189,10 @@ log_queryerror(ns_client_t *client, isc_result_t result, int line, int level); static void rpz_st_clear(ns_client_t *client); +static isc_boolean_t +rpz_ck_dnssec(ns_client_t *client, isc_result_t qresult, + dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset); + /*% * Increment query statistics counters. */ @@ -695,8 +699,7 @@ ns_query_init(ns_client_t *client) { } static inline ns_dbversion_t * -query_findversion(ns_client_t *client, dns_db_t *db) -{ +query_findversion(ns_client_t *client, dns_db_t *db) { ns_dbversion_t *dbversion; /*% @@ -1019,6 +1022,8 @@ rpz_getdb(ns_client_t *client, dns_name_t *p_name, dns_rpz_type_t rpz_type, dns_dbversion_t *rpz_version = NULL; isc_result_t result; + CTRACE(ISC_LOG_DEBUG(3), "rpz_getdb"); + result = query_getzonedb(client, p_name, dns_rdatatype_any, DNS_GETDB_IGNOREACL, zonep, dbp, &rpz_version); if (result == ISC_R_SUCCESS) { @@ -4104,17 +4109,17 @@ rpz_clean(dns_zone_t **zonep, dns_db_t **dbp, dns_dbnode_t **nodep, } static inline void -rpz_match_clear(dns_rpz_st_t *st) -{ +rpz_match_clear(dns_rpz_st_t *st) { rpz_clean(&st->m.zone, &st->m.db, &st->m.node, &st->m.rdataset); st->m.version = NULL; } static inline isc_result_t -rpz_ready(ns_client_t *client, dns_rdataset_t **rdatasetp) -{ +rpz_ready(ns_client_t *client, dns_rdataset_t **rdatasetp) { REQUIRE(rdatasetp != NULL); + CTRACE(ISC_LOG_DEBUG(3), "rpz_ready"); + if (*rdatasetp == NULL) { *rdatasetp = query_newrdataset(client); if (*rdatasetp == NULL) { @@ -4132,6 +4137,8 @@ static void rpz_st_clear(ns_client_t *client) { dns_rpz_st_t *st = client->query.rpz_st; + CTRACE(ISC_LOG_DEBUG(3), "rpz_st_clear"); + if (st->m.rdataset != NULL) query_putrdataset(client, &st->m.rdataset); rpz_match_clear(st); @@ -4156,38 +4163,40 @@ static dns_rpz_zbits_t rpz_get_zbits(ns_client_t *client, dns_rdatatype_t ip_type, dns_rpz_type_t rpz_type) { - dns_rpz_zones_t *rpzs; dns_rpz_st_t *st; dns_rpz_zbits_t zbits; - rpzs = client->view->rpzs; + REQUIRE(client != NULL); + REQUIRE(client->query.rpz_st != NULL); + + st = client->query.rpz_st; switch (rpz_type) { case DNS_RPZ_TYPE_CLIENT_IP: - zbits = rpzs->have.client_ip; + zbits = st->have.client_ip; break; case DNS_RPZ_TYPE_QNAME: - zbits = rpzs->have.qname; + zbits = st->have.qname; break; case DNS_RPZ_TYPE_IP: if (ip_type == dns_rdatatype_a) { - zbits = rpzs->have.ipv4; + zbits = st->have.ipv4; } else if (ip_type == dns_rdatatype_aaaa) { - zbits = rpzs->have.ipv6; + zbits = st->have.ipv6; } else { - zbits = rpzs->have.ip; + zbits = st->have.ip; } break; case DNS_RPZ_TYPE_NSDNAME: - zbits = rpzs->have.nsdname; + zbits = st->have.nsdname; break; case DNS_RPZ_TYPE_NSIP: if (ip_type == dns_rdatatype_a) { - zbits = rpzs->have.nsipv4; + zbits = st->have.nsipv4; } else if (ip_type == dns_rdatatype_aaaa) { - zbits = rpzs->have.nsipv6; + zbits = st->have.nsipv6; } else { - zbits = rpzs->have.nsip; + zbits = st->have.nsip; } break; default: @@ -4195,8 +4204,6 @@ rpz_get_zbits(ns_client_t *client, break; } - st = client->query.rpz_st; - /* * Choose * the earliest configured policy zone (rpz->num) @@ -4217,7 +4224,7 @@ rpz_get_zbits(ns_client_t *client, * If the client wants recursion, allow only compatible policies. */ if (!RECURSIONOK(client)) - zbits &= rpzs->p.no_rd_ok; + zbits &= st->popt.no_rd_ok; return (zbits); } @@ -4241,6 +4248,8 @@ rpz_rrset_find(ns_client_t *client, dns_name_t *name, dns_rdatatype_t type, dns_clientinfomethods_t cm; dns_clientinfo_t ci; + CTRACE(ISC_LOG_DEBUG(3), "rpz_rrset_find"); + dns_clientinfomethods_init(&cm, ns_client_sourceip); dns_clientinfo_init(&ci, client, NULL); @@ -4339,7 +4348,7 @@ rpz_rrset_find(ns_client_t *client, dns_name_t *name, dns_rdatatype_t type, /* * Compute a policy owner name, p_name, in a policy zone given the needed - * policy type and the trigger name. + * policy type and the trigger name. */ static isc_result_t rpz_get_p_name(ns_client_t *client, dns_name_t *p_name, @@ -4351,6 +4360,8 @@ rpz_get_p_name(ns_client_t *client, dns_name_t *p_name, unsigned int first, labels; isc_result_t result; + CTRACE(ISC_LOG_DEBUG(3), "rpz_get_p_name"); + /* * The policy owner name consists of a suffix depending on the type * and policy zone and a prefix that is the longest possible string @@ -4437,6 +4448,8 @@ rpz_find_p(ns_client_t *client, dns_name_t *self_name, dns_rdatatype_t qtype, REQUIRE(nodep != NULL); + CTRACE(ISC_LOG_DEBUG(3), "rpz_find_p"); + dns_clientinfomethods_init(&cm, ns_client_sourceip); dns_clientinfo_init(&ci, client, NULL); @@ -4613,6 +4626,8 @@ rpz_rewrite_ip(ns_client_t *client, const isc_netaddr_t *netaddr, dns_rpz_policy_t policy; isc_result_t result; + CTRACE(ISC_LOG_DEBUG(3), "rpz_rewrite_ip"); + dns_fixedname_init(&ip_namef); ip_name = dns_fixedname_name(&ip_namef); @@ -4629,6 +4644,13 @@ rpz_rewrite_ip(ns_client_t *client, const isc_netaddr_t *netaddr, break; zbits &= (DNS_RPZ_ZMASK(rpz_num) >> 1); + /* + * In case num_zones has changed since zbits was + * originally calculated + */ + if (rpz_num >= rpzs->p.num_zones) + break; + /* * Do not try applying policy zones that cannot replace a * previously found policy zone. @@ -4666,6 +4688,9 @@ rpz_rewrite_ip(ns_client_t *client, const isc_netaddr_t *netaddr, * data can out of date during races with and among * policy zone updates. */ + CTRACE(ISC_LOG_ERROR, + "rpz_rewrite_ip: mismatched summary data; " + "continuing"); continue; case DNS_R_SERVFAIL: rpz_clean(&p_zone, &p_db, &p_node, p_rdatasetp); @@ -4703,6 +4728,8 @@ rpz_rewrite_ip(ns_client_t *client, const isc_netaddr_t *netaddr, * ensures that we found the longest match. */ if (rpz->policy != DNS_RPZ_POLICY_DISABLED) { + CTRACE(ISC_LOG_DEBUG(3), + "rpz_rewrite_ip: rpz_save_p"); rpz_save_p(st, rpz, rpz_type, policy, p_name, prefix, result, &p_zone, &p_db, &p_node, @@ -4741,6 +4768,8 @@ rpz_rewrite_ip_rrset(ns_client_t *client, struct in6_addr in6a; isc_result_t result; + CTRACE(ISC_LOG_DEBUG(3), "rpz_rewrite_ip_rrset"); + zbits = rpz_get_zbits(client, ip_type, rpz_type); if (zbits == 0) return (ISC_R_SUCCESS); @@ -4832,6 +4861,8 @@ rpz_rewrite_ip_rrsets(ns_client_t *client, dns_name_t *name, dns_rdataset_t *p_rdataset; isc_result_t result; + CTRACE(ISC_LOG_DEBUG(3), "rpz_rewrite_ip_rrsets"); + st = client->query.rpz_st; ip_version = NULL; ip_db = NULL; @@ -4884,6 +4915,7 @@ rpz_rewrite_name(ns_client_t *client, dns_name_t *trig_name, dns_rdatatype_t qtype, dns_rpz_type_t rpz_type, dns_rpz_zbits_t allowed_zbits, dns_rdataset_t **rdatasetp) { + dns_rpz_zones_t *rpzs; dns_rpz_zone_t *rpz; dns_rpz_st_t *st; dns_fixedname_t p_namef; @@ -4897,20 +4929,23 @@ rpz_rewrite_name(ns_client_t *client, dns_name_t *trig_name, dns_rpz_policy_t policy; isc_result_t result; + CTRACE(ISC_LOG_DEBUG(3), "rpz_rewrite_name"); + zbits = rpz_get_zbits(client, qtype, rpz_type); zbits &= allowed_zbits; if (zbits == 0) return (ISC_R_SUCCESS); + rpzs = client->view->rpzs; + /* * If there is only one eligible policy zone, just check it. * If more than one, then use the summary database to find * the bit mask of policy zones with policies for this trigger name. - * x&-x is the least significant bit set in x + * x&(~x+1) is the least significant bit set in x */ if (zbits != (zbits & (~zbits + 1))) { - zbits = dns_rpz_find_name(client->view->rpzs, - rpz_type, zbits, trig_name); + zbits = dns_rpz_find_name(rpzs, rpz_type, zbits, trig_name); if (zbits == 0) return (ISC_R_SUCCESS); } @@ -4932,19 +4967,22 @@ rpz_rewrite_name(ns_client_t *client, dns_name_t *trig_name, * We check the most eligible zone first and so usually check only * one policy zone. */ - for (rpz_num = 0; - zbits != 0; - ++rpz_num, zbits >>= 1) { - if ((zbits & 1) == 0) { - INSIST(rpz_num <= client->view->rpzs->p.num_zones); + for (rpz_num = 0; zbits != 0; ++rpz_num, zbits >>= 1) { + if ((zbits & 1) == 0) continue; - } + + /* + * In case num_zones has changed since the 'have' + * originally calculated + */ + if (rpz_num >= rpzs->p.num_zones) + break; /* * Do not check policy zones that cannot replace a previously * found policy. */ - rpz = client->view->rpzs->zones[rpz_num]; + rpz = rpzs->zones[rpz_num]; if (st->m.policy != DNS_RPZ_POLICY_MISS) { if (st->m.rpz->num < rpz->num) break; @@ -4972,6 +5010,9 @@ rpz_rewrite_name(ns_client_t *client, dns_name_t *trig_name, * data can out of date during races with and among * policy zone updates. */ + CTRACE(ISC_LOG_ERROR, + "rpz_rewrite_name: mismatched summary data; " + "continuing"); continue; case DNS_R_SERVFAIL: rpz_clean(&p_zone, &p_db, &p_node, rdatasetp); @@ -5027,6 +5068,8 @@ rpz_rewrite_name(ns_client_t *client, dns_name_t *trig_name, } #endif if (rpz->policy != DNS_RPZ_POLICY_DISABLED) { + CTRACE(ISC_LOG_DEBUG(3), + "rpz_rewrite_name: rpz_save_p"); rpz_save_p(st, rpz, rpz_type, policy, p_name, 0, result, &p_zone, &p_db, &p_node, @@ -5058,6 +5101,8 @@ rpz_rewrite_ns_skip(ns_client_t *client, dns_name_t *nsname, { dns_rpz_st_t *st; + CTRACE(ISC_LOG_DEBUG(3), "rpz_rewrite_ns_skip"); + st = client->query.rpz_st; if (str != NULL) @@ -5074,9 +5119,11 @@ rpz_rewrite_ns_skip(ns_client_t *client, dns_name_t *nsname, * Look for response policy zone QNAME, NSIP, and NSDNAME rewriting. */ static isc_result_t -rpz_rewrite(ns_client_t *client, dns_rdatatype_t qtype, isc_result_t qresult, - isc_boolean_t resuming) +rpz_rewrite(ns_client_t *client, dns_rdatatype_t qtype, + isc_result_t qresult, isc_boolean_t resuming, + dns_rdataset_t *ordataset, dns_rdataset_t *osigset) { + dns_rpz_zones_t *rpzs; dns_rpz_st_t *st; dns_rdataset_t *rdataset; dns_fixedname_t nsnamef; @@ -5084,8 +5131,32 @@ rpz_rewrite(ns_client_t *client, dns_rdatatype_t qtype, isc_result_t qresult, int qresult_type; dns_rpz_zbits_t zbits; isc_result_t result = ISC_R_SUCCESS; + dns_rpz_have_t have; + dns_rpz_popt_t popt; + int rpz_ver; + CTRACE(ISC_LOG_DEBUG(3), "rpz_rewrite"); + + rpzs = client->view->rpzs; st = client->query.rpz_st; + + if (rpzs == NULL || + (st != NULL && (st->state & DNS_RPZ_REWRITTEN) != 0)) + return (DNS_R_DISALLOWED); + + LOCK(&rpzs->maint_lock); + if (rpzs->p.num_zones == 0 || + (!RECURSIONOK(client) && rpzs->p.no_rd_ok == 0) || + !rpz_ck_dnssec(client, result, ordataset, osigset)) + { + UNLOCK(&rpzs->maint_lock); + return (DNS_R_DISALLOWED); + } + have = rpzs->have; + popt = rpzs->p; + rpz_ver = rpzs->rpz_ver; + UNLOCK(&rpzs->maint_lock); + if (st == NULL) { st = isc_mem_get(client->mctx, sizeof(*st)); if (st == NULL) @@ -5103,6 +5174,9 @@ rpz_rewrite(ns_client_t *client, dns_rdatatype_t qtype, isc_result_t qresult, st->p_name = dns_fixedname_name(&st->_p_namef); st->r_name = dns_fixedname_name(&st->_r_namef); st->fname = dns_fixedname_name(&st->_fnamef); + st->have = have; + st->popt = popt; + st->rpz_ver = rpz_ver; client->query.rpz_st = st; } @@ -5165,7 +5239,7 @@ rpz_rewrite(ns_client_t *client, dns_rdatatype_t qtype, isc_result_t qresult, * Get bits for the policy zones that do not need * to wait for the results of recursion. */ - allowed = client->view->rpzs->have.qname_skip_recurse; + allowed = st->have.qname_skip_recurse; if (allowed == 0) return (ISC_R_SUCCESS); } else { @@ -5271,7 +5345,7 @@ rpz_rewrite(ns_client_t *client, dns_rdatatype_t qtype, isc_result_t qresult, dns_fixedname_init(&nsnamef); dns_name_clone(client->query.qname, dns_fixedname_name(&nsnamef)); - while (st->r.label > client->view->rpzs->p.min_ns_labels) { + while (st->r.label > st->popt.min_ns_labels) { /* * Get NS rrset for each domain in the current qname. */ @@ -5283,7 +5357,8 @@ rpz_rewrite(ns_client_t *client, dns_rdatatype_t qtype, isc_result_t qresult, NULL, nsname); } if (st->r.ns_rdataset == NULL || - !dns_rdataset_isassociated(st->r.ns_rdataset)) { + !dns_rdataset_isassociated(st->r.ns_rdataset)) + { dns_db_t *db = NULL; result = rpz_rrset_find(client, nsname, dns_rdatatype_ns, @@ -5443,6 +5518,8 @@ rpz_ck_dnssec(ns_client_t *client, isc_result_t qresult, dns_rdatatype_t type; isc_result_t result; + CTRACE(ISC_LOG_DEBUG(3), "rpz_ck_dnssec"); + if (client->view->rpzs->p.break_dnssec || !WANTDNSSEC(client)) return (ISC_TRUE); @@ -5507,6 +5584,8 @@ rpz_add_cname(ns_client_t *client, dns_rpz_st_t *st, unsigned int labels; isc_result_t result; + CTRACE(ISC_LOG_DEBUG(3), "rpz_add_cname"); + labels = dns_name_countlabels(cname); if (labels > 2 && dns_name_iswildcard(cname)) { dns_fixedname_init(&prefix); @@ -6418,7 +6497,19 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype) rpz_st = client->query.rpz_st; if (rpz_st != NULL && - (rpz_st->state & DNS_RPZ_RECURSING) != 0) { + (rpz_st->state & DNS_RPZ_RECURSING) != 0) + { + CTRACE(ISC_LOG_DEBUG(3), "resume from RPZ recursion"); + + /* + * Has response policy changed out from under us? + */ + if (rpz_st->rpz_ver != client->view->rpzs->rpz_ver) { + CTRACE(ISC_LOG_ERROR, + "query_find: RPZ settings out of date"); + QUERY_ERROR(DNS_R_SERVFAIL); + goto cleanup; + } is_zone = rpz_st->q.is_zone; authoritative = rpz_st->q.authoritative; zone = rpz_st->q.zone; @@ -6440,10 +6531,11 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype) rpz_st->r.r_rdataset = event->rdataset; query_putrdataset(client, &event->sigrdataset); } else if (REDIRECT(client)) { - /* * Restore saved state. */ + CTRACE(ISC_LOG_DEBUG(3), + "resume from redirect recursion"); qtype = client->query.redirect.qtype; rdataset = client->query.redirect.rdataset; client->query.redirect.rdataset = NULL; @@ -6467,6 +6559,8 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype) if (event->db != NULL) dns_db_detach(&event->db); } else { + CTRACE(ISC_LOG_DEBUG(3), + "resume from normal recursion"); authoritative = ISC_FALSE; qtype = event->qtype; @@ -6757,7 +6851,8 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype) !(result == DNS_R_DELEGATION && !is_zone && RECURSIONOK(client)) && (client->query.rpz_st == NULL || (client->query.rpz_st->state & DNS_RPZ_REWRITTEN) == 0)&& - (client->query.attributes & NS_QUERYATTR_RRL_CHECKED) == 0) { + (client->query.attributes & NS_QUERYATTR_RRL_CHECKED) == 0) + { dns_rdataset_t nc_rdataset; isc_boolean_t wouldlog; char log_buf[DNS_RRL_LOG_BUF_LEN]; @@ -6865,22 +6960,19 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype) } } - if (client->view->rpzs != NULL && - client->view->rpzs->p.num_zones != 0 && - (RECURSIONOK(client) || client->view->rpzs->p.no_rd_ok != 0) && - rpz_ck_dnssec(client, result, rdataset, sigrdataset) && - !RECURSING(client) && - (client->query.rpz_st == NULL || - (client->query.rpz_st->state & DNS_RPZ_REWRITTEN) == 0) && + if (!RECURSING(client) && !dns_name_equal(client->query.qname, dns_rootname)) { isc_result_t rresult; - rresult = rpz_rewrite(client, qtype, result, resuming); + rresult = rpz_rewrite(client, qtype, result, resuming, + rdataset, sigrdataset); rpz_st = client->query.rpz_st; switch (rresult) { case ISC_R_SUCCESS: break; + case DNS_R_DISALLOWED: + goto norpz; case DNS_R_DELEGATION: /* * recursing for NS names or addresses, @@ -6907,14 +6999,17 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype) RECURSE_ERROR(rresult); goto cleanup; } + if (rpz_st->m.policy != DNS_RPZ_POLICY_MISS) rpz_st->state |= DNS_RPZ_REWRITTEN; if (rpz_st->m.policy != DNS_RPZ_POLICY_MISS && rpz_st->m.policy != DNS_RPZ_POLICY_PASSTHRU && (rpz_st->m.policy != DNS_RPZ_POLICY_TCP_ONLY || (client->attributes & NS_CLIENTATTR_TCP) == 0) && - rpz_st->m.policy != DNS_RPZ_POLICY_ERROR) { - /* We got a hit and are going to answer with our + rpz_st->m.policy != DNS_RPZ_POLICY_ERROR) + { + /* + * We got a hit and are going to answer with our * fiction. Ensure that we answer with the name * we looked up even if we were stopped short * in recursion or for a deferral. @@ -6983,7 +7078,7 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype) * We will add this rdataset. */ rdataset->ttl = ISC_MIN(rdataset->ttl, - rpz_st->m.ttl); + rpz_st->m.ttl); } break; case DNS_RPZ_POLICY_WILDCNAME: @@ -7035,6 +7130,7 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype) } } + norpz: switch (result) { case ISC_R_SUCCESS: /* diff --git a/bin/named/server.c b/bin/named/server.c index 64817f43d93..d5340133ecf 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -1934,6 +1934,11 @@ configure_rpz(dns_view_t *view, const cfg_obj_t *rpz_obj, if (*old_rpz_okp) { dns_rpz_detach_rpzs(&view->rpzs); dns_rpz_attach_rpzs(pview->rpzs, &view->rpzs); + } else if (old != NULL && pview != NULL) { + view->rpzs->rpz_ver = pview->rpzs->rpz_ver + 1; + cfg_obj_log(rpz_obj, ns_g_lctx, ISC_LOG_DEBUG(1), + "updated RPZ policy: version %d", + view->rpzs->rpz_ver); } if (pview != NULL) dns_view_detach(&pview); diff --git a/bin/tests/system/rpzrecurse/README b/bin/tests/system/rpzrecurse/README index f56d8eb431a..670d22a745c 100644 --- a/bin/tests/system/rpzrecurse/README +++ b/bin/tests/system/rpzrecurse/README @@ -26,7 +26,7 @@ first query of each test is q01.l2.l1.l0). For all the tests the triggers are constructed as follows: client-ip - match 127.0.0.1/32 ip - match 255.255.255.255/32 (does not matter due to SERVFAIL) -nsdname - match does.not.matter (and it doesn't) +nsdname - match ns.example.org (also does not matter) nsip - match 255.255.255.255/32 (also does not matter) qname - match qXX.l2.l1.l0, where XX is the query sequence number that is intended to be matched by this qname rule. diff --git a/bin/tests/system/rpzrecurse/clean.sh b/bin/tests/system/rpzrecurse/clean.sh index e8cf5aa414d..004401b6bb9 100644 --- a/bin/tests/system/rpzrecurse/clean.sh +++ b/bin/tests/system/rpzrecurse/clean.sh @@ -18,7 +18,7 @@ rm -f dig.out.* rm -f ns2/named.conf rm -f ns2/*.local rm -f ns2/*.queries -rm -f ns2/named.*.conf +rm -f ns2/named.[0-9]*.conf rm -f ns*/named.lock rm -f ns*/named.memstats rm -f ns*/named.run diff --git a/bin/tests/system/rpzrecurse/ns1/example.db b/bin/tests/system/rpzrecurse/ns1/example.db new file mode 100644 index 00000000000..d0fbc3739a0 --- /dev/null +++ b/bin/tests/system/rpzrecurse/ns1/example.db @@ -0,0 +1,5 @@ +$TTL 3600 +@ IN SOA root.example. example. 1 3600 3600 3600 3600 +@ IN NS ns.example. +www IN CNAME cname +cname IN A 10.53.0.1 diff --git a/bin/tests/system/rpzrecurse/ns1/named.conf b/bin/tests/system/rpzrecurse/ns1/named.conf index 4de9d81610c..00ca3c133f0 100644 --- a/bin/tests/system/rpzrecurse/ns1/named.conf +++ b/bin/tests/system/rpzrecurse/ns1/named.conf @@ -23,6 +23,7 @@ options { listen-on { 10.53.0.1; }; listen-on-v6 { none; }; recursion no; + querylog yes; }; zone "." { @@ -30,6 +31,11 @@ zone "." { file "root.db"; }; +zone "test.example.org" { + type master; + file "example.db"; +}; + zone "l0" { type master; file "db.l0"; diff --git a/bin/tests/system/rpzrecurse/ns1/root.db b/bin/tests/system/rpzrecurse/ns1/root.db index 79b7f487688..5f62da28ebb 100644 --- a/bin/tests/system/rpzrecurse/ns1/root.db +++ b/bin/tests/system/rpzrecurse/ns1/root.db @@ -13,15 +13,15 @@ ; PERFORMANCE OF THIS SOFTWARE. $TTL 300 -. IN SOA muks.isc.org. a.root.servers.nil. ( - 2010 ; serial +@ IN SOA muks.isc.org. a.root.servers.nil. ( + 2010 ; serial 600 ; refresh 600 ; retry 1200 ; expire 600 ; minimum ) -. NS a.root-servers.nil. -a.root-servers.nil. A 10.53.0.1 +@ NS ns.example. +ns.example. A 10.53.0.1 l0. NS ns.l0. ns.l0. A 10.53.0.1 diff --git a/bin/tests/system/rpzrecurse/ns2/named.conf.header b/bin/tests/system/rpzrecurse/ns2/named.conf.header index 7a78a1a6178..4988b9854af 100644 --- a/bin/tests/system/rpzrecurse/ns2/named.conf.header +++ b/bin/tests/system/rpzrecurse/ns2/named.conf.header @@ -27,3 +27,13 @@ options { recursion yes; querylog yes; }; + +key rndc_key { + secret "1234abcd8765"; + algorithm hmac-sha256; +}; + +controls { + inet 10.53.0.2 port 9953 allow { any; } keys { rndc_key; }; +}; + diff --git a/bin/tests/system/rpzrecurse/ns2/named.conf.default b/bin/tests/system/rpzrecurse/ns2/named.default.conf similarity index 100% rename from bin/tests/system/rpzrecurse/ns2/named.conf.default rename to bin/tests/system/rpzrecurse/ns2/named.default.conf diff --git a/bin/tests/system/rpzrecurse/ns2/root.hint b/bin/tests/system/rpzrecurse/ns2/root.hint index 5f9268ef6fe..105e41297b5 100644 --- a/bin/tests/system/rpzrecurse/ns2/root.hint +++ b/bin/tests/system/rpzrecurse/ns2/root.hint @@ -13,5 +13,5 @@ ; PERFORMANCE OF THIS SOFTWARE. $TTL 999999 -. IN NS a.root-servers.nil. -a.root-servers.nil. IN A 10.53.0.1 +. IN NS ns.example. +ns.example. IN A 10.53.0.1 diff --git a/bin/tests/system/rpzrecurse/setup.sh b/bin/tests/system/rpzrecurse/setup.sh index 100464bf1ef..3a34348e6f7 100644 --- a/bin/tests/system/rpzrecurse/setup.sh +++ b/bin/tests/system/rpzrecurse/setup.sh @@ -18,4 +18,4 @@ SYSTEMTESTTOP=.. . $SYSTEMTESTTOP/conf.sh perl testgen.pl -cp -f ns2/named.conf.default ns2/named.conf +cp -f ns2/named.default.conf ns2/named.conf diff --git a/bin/tests/system/rpzrecurse/testgen.pl b/bin/tests/system/rpzrecurse/testgen.pl index 73b756af1d1..6d774a0ae64 100755 --- a/bin/tests/system/rpzrecurse/testgen.pl +++ b/bin/tests/system/rpzrecurse/testgen.pl @@ -31,7 +31,13 @@ view "recursive" { response-policy { EOB -my $boilerplate_middle = <<'EOB'; +my $no_option = <<'EOB'; + }; + + # policy zones to be tested +EOB + +my $qname_wait_recurse = <<'EOB'; } qname-wait-recurse no; # policy zones to be tested @@ -41,9 +47,12 @@ my $boilerplate_end = <<'EOB'; }; EOB +my $policy_option = $qname_wait_recurse; + +my $serialnum = "1"; my $policy_zone_header = <<'EOH'; $TTL 60 -@ IN SOA root.ns ns 1996072700 3600 1800 86400 60 +@ IN SOA root.ns ns SERIAL 3600 1800 86400 60 NS ns ns A 127.0.0.1 EOH @@ -62,7 +71,7 @@ sub policy_ip { } sub policy_nsdname { - return "does.not.matter.rpz-nsdname CNAME .\n"; + return "ns.example.org.rpz-nsdname CNAME .\n"; } sub policy_nsip { @@ -113,7 +122,7 @@ sub mkconf { print $conf_fh map { qq{ zone "$_->[0]";\n} } @zones; - print $conf_fh $boilerplate_middle; + print $conf_fh $policy_option; print $conf_fh map { qq{ zone "$_->[0]" { type master; file "db.$_->[0]"; };\n} } @zones; @@ -130,7 +139,9 @@ sub mkconf { open $policy_zone_fh, ">$policy_zone_filename" or die; - print $policy_zone_fh $policy_zone_header; + my $header = $policy_zone_header; + $header =~ s/SERIAL/$serialnum/; + print $policy_zone_fh $header; foreach my $trigger( @$policy_zone_contents ) { if( exists $static_triggers{$trigger} ) { @@ -234,6 +245,28 @@ mkconf( [ 6 ], ); +$policy_option = $no_option; + +mkconf( + '6a', + 0, + [ ], +); + +$serialnum = "2"; +mkconf( + '6b', + 0, + [ 'nsdname' ], +); + +$serialnum = "3"; +mkconf( + '6c', + 0, + [ ], +); + __END__ 0x01 - has client-ip @@ -243,7 +276,7 @@ __END__ 0x10 - has ip 32.255.255.255.255.rpz-ip CNAME . 0x20 - has nsdname - does.not.matter.rpz-nsdname CNAME . + ns.example.org.rpz-nsdname CNAME . 0x40 - has nsip 32.255.255.255.255.rpz-nsip CNAME . diff --git a/bin/tests/system/rpzrecurse/tests.sh b/bin/tests/system/rpzrecurse/tests.sh index 0e6cb384ab9..f833cd4bc95 100644 --- a/bin/tests/system/rpzrecurse/tests.sh +++ b/bin/tests/system/rpzrecurse/tests.sh @@ -69,7 +69,7 @@ expect_recurse() { t=`expr $t + 1` echo "I:testing $NAME recurses (${t})" run_query $TESTNAME $LINE && { - echo "I:test $t failed" + echo "I:test ${t} failed" status=1 } } @@ -78,7 +78,7 @@ t=`expr $t + 1` echo "I:testing that l1.l0 exists without RPZ (${t})" $DIG $DIGOPTS l1.l0 ns @10.53.0.2 -p 5300 > dig.out.${t} grep "status: NOERROR" dig.out.${t} > /dev/null 2>&1 || { - echo "I:test $t failed" + echo "I:test ${t} failed" status=1 } @@ -86,7 +86,7 @@ t=`expr $t + 1` echo "I:testing that l2.l1.l0 returns SERVFAIL without RPZ (${t})" $DIG $DIGOPTS l2.l1.l0 ns @10.53.0.2 -p 5300 > dig.out.${t} grep "status: SERVFAIL" dig.out.${t} > /dev/null 2>&1 || { - echo "I:test $t failed" + echo "I:test ${t} failed" status=1 } @@ -164,5 +164,71 @@ expect_recurse 5a 4 expect_recurse 5a 5 expect_recurse 5a 6 -echo "I:exit status: $status" +# Group 6 +echo "I:check recursive behavior consistency during policy update races" +run_server 6a +sleep 1 +t=`expr $t + 1` +echo "I:running dig to cache CNAME record (${t})" +$DIG $DIGOPTS @10.53.0.2 -p 5300 www.test.example.org CNAME > dig.out.${t} +sleep 1 +echo "I:suspending authority server" +kill -TSTP `cat ns1/named.pid` +echo "I:adding an NSDNAME policy" +cp ns2/db.6a.00.policy.local ns2/saved.policy.local +cp ns2/db.6b.00.policy.local ns2/db.6a.00.policy.local +$RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 reload 6a.00.policy.local 2>&1 | sed 's/^/I:ns2 /' +sleep 1 +t=`expr $t + 1` +echo "I:running dig to follow CNAME (blocks, so runs in the background) (${t})" +$DIG $DIGOPTS @10.53.0.2 -p 5300 www.test.example.org A > dig.out.${t} & +sleep 1 +echo "I:removing the NSDNAME policy" +cp ns2/db.6c.00.policy.local ns2/db.6a.00.policy.local +$RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 reload 6a.00.policy.local 2>&1 | sed 's/^/I:ns2 /' +sleep 1 +echo "I:resuming authority server" +kill -CONT `cat ns1/named.pid` +for n in 1 2 3 4 5 6 7 8 9; do + sleep 1 + [ -s dig.out.${t} ] || continue + grep "status: NOERROR" dig.out.${t} > /dev/null 2>&1 || { + echo "I:test ${t} failed" + status=1 + } +done + +echo "I:check recursive behavior consistency during policy removal races" +cp ns2/saved.policy.local ns2/db.6a.00.policy.local +run_server 6a +sleep 1 +t=`expr $t + 1` +echo "I:running dig to cache CNAME record (${t})" +$DIG $DIGOPTS @10.53.0.2 -p 5300 www.test.example.org CNAME > dig.out.${t} +sleep 1 +echo "I:suspending authority server" +kill -TSTP `cat ns1/named.pid` +echo "I:adding an NSDNAME policy" +cp ns2/db.6b.00.policy.local ns2/db.6a.00.policy.local +$RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 reload 6a.00.policy.local 2>&1 | sed 's/^/I:ns2 /' +sleep 1 +t=`expr $t + 1` +echo "I:running dig to follow CNAME (blocks, so runs in the background) (${t})" +$DIG $DIGOPTS @10.53.0.2 -p 5300 www.test.example.org A > dig.out.${t} & +sleep 1 +echo "I:removing the policy zone" +cp ns2/named.default.conf ns2/db.6a.00.policy.local +$RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 reload 6a.00.policy.local 2>&1 | sed 's/^/I:ns2 /' +sleep 1 +echo "I:resuming authority server" +kill -CONT `cat ns1/named.pid` +for n in 1 2 3 4 5 6 7 8 9; do + sleep 1 + [ -s dig.out.${t} ] || continue + grep "status: NOERROR" dig.out.${t} > /dev/null 2>&1 || { + echo "I:test ${t} failed" + status=1 + } +done + exit $status diff --git a/doc/arm/notes.xml b/doc/arm/notes.xml index 57e46ff811c..096e698df9d 100644 --- a/doc/arm/notes.xml +++ b/doc/arm/notes.xml @@ -631,6 +631,14 @@ fix will not be noticeable. [RT #39229] + + + A bug in RPZ could cause the server to crash if policy + zones were updated (e.g. via rndc reload + or an incoming zone transfer) while RPZ processing was still + ongoing for an active query. [RT #39415] + + diff --git a/lib/dns/include/dns/rpz.h b/lib/dns/include/dns/rpz.h index 28bffaa7fa3..f0201eb8ee3 100644 --- a/lib/dns/include/dns/rpz.h +++ b/lib/dns/include/dns/rpz.h @@ -140,21 +140,53 @@ struct dns_rpz_zone { */ typedef struct dns_rpz_cidr_node dns_rpz_cidr_node_t; +/* + * Bitfields indicating which policy zones have policies of + * which type. + */ +typedef struct dns_rpz_have dns_rpz_have_t; +struct dns_rpz_have { + dns_rpz_zbits_t client_ipv4; + dns_rpz_zbits_t client_ipv6; + dns_rpz_zbits_t client_ip; + dns_rpz_zbits_t qname; + dns_rpz_zbits_t ipv4; + dns_rpz_zbits_t ipv6; + dns_rpz_zbits_t ip; + dns_rpz_zbits_t nsdname; + dns_rpz_zbits_t nsipv4; + dns_rpz_zbits_t nsipv6; + dns_rpz_zbits_t nsip; + dns_rpz_zbits_t qname_skip_recurse; +}; + +/* + * Policy options + */ +typedef struct dns_rpz_popt dns_rpz_popt_t; +struct dns_rpz_popt { + dns_rpz_zbits_t no_rd_ok; + isc_boolean_t break_dnssec; + isc_boolean_t qname_wait_recurse; + unsigned int min_ns_labels; + dns_rpz_num_t num_zones; +}; + /* * Response policy zones known to a view. */ typedef struct dns_rpz_zones dns_rpz_zones_t; struct dns_rpz_zones { - struct { - dns_rpz_zbits_t no_rd_ok; - isc_boolean_t break_dnssec; - isc_boolean_t qname_wait_recurse; - unsigned int min_ns_labels; - dns_rpz_num_t num_zones; - } p; + dns_rpz_popt_t p; dns_rpz_zone_t *zones[DNS_RPZ_MAX_ZONES]; dns_rpz_triggers_t triggers[DNS_RPZ_MAX_ZONES]; + /* + * RPZ policy version number (initially 0, increases whenever + * the server is reconfigured with new zones or policy) + */ + int rpz_ver; + dns_rpz_zbits_t defined; /* @@ -171,20 +203,7 @@ struct dns_rpz_zones { * load rpzs load_begun=1 have=0 */ dns_rpz_zbits_t load_begun; - struct { - dns_rpz_zbits_t client_ipv4; - dns_rpz_zbits_t client_ipv6; - dns_rpz_zbits_t client_ip; - dns_rpz_zbits_t qname; - dns_rpz_zbits_t ipv4; - dns_rpz_zbits_t ipv6; - dns_rpz_zbits_t ip; - dns_rpz_zbits_t nsdname; - dns_rpz_zbits_t nsipv4; - dns_rpz_zbits_t nsipv6; - dns_rpz_zbits_t nsip; - dns_rpz_zbits_t qname_skip_recurse; - } have; + dns_rpz_have_t have; dns_rpz_triggers_t total_triggers; isc_mem_t *mctx; @@ -242,6 +261,7 @@ typedef struct { isc_result_t r_result; dns_rdataset_t *r_rdataset; } r; + /* * State of real query while recursing for NSIP or NSDNAME. */ @@ -256,6 +276,17 @@ typedef struct { dns_rdataset_t *sigrdataset; dns_rdatatype_t qtype; } q; + + /* + * A copy of the 'have' and 'p' structures and the RPZ + * policy version as of the beginning of RPZ processing, + * used to avoid problems when policy is updated while + * RPZ recursion is ongoing. + */ + dns_rpz_have_t have; + dns_rpz_popt_t popt; + int rpz_ver; + /* * p_name: current policy owner name * r_name: recursing for this name to possible policy triggers diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index 5daac5b0857..8f3d8335803 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -1873,8 +1873,7 @@ clean_zone_node(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node, } static void -delete_node(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node) -{ +delete_node(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node) { dns_rbtnode_t *nsecnode; dns_fixedname_t fname; dns_name_t *name; diff --git a/lib/dns/rpz.c b/lib/dns/rpz.c index 7f9bdd58877..e5ac99a312d 100644 --- a/lib/dns/rpz.c +++ b/lib/dns/rpz.c @@ -14,9 +14,6 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id$ */ - - /*! \file */ #include @@ -378,6 +375,7 @@ set_sum_pair(dns_rpz_cidr_node_t *cnode) { } while (cnode != NULL); } +/* Caller must hold rpzs->maint_lock */ static void fix_qname_skip_recurse(dns_rpz_zones_t *rpzs) { dns_rpz_zbits_t mask; @@ -2078,8 +2076,13 @@ dns_rpz_find_ip(dns_rpz_zones_t *rpzs, dns_rpz_type_t rpz_type, dns_rpz_cidr_node_t *found; isc_result_t result; dns_rpz_num_t rpz_num; + dns_rpz_have_t have; int i; + LOCK(&rpzs->maint_lock); + have = rpzs->have; + UNLOCK(&rpzs->maint_lock); + /* * Convert IP address to CIDR tree key. */ @@ -2090,13 +2093,13 @@ dns_rpz_find_ip(dns_rpz_zones_t *rpzs, dns_rpz_type_t rpz_type, tgt_ip.w[3] = ntohl(netaddr->type.in.s_addr); switch (rpz_type) { case DNS_RPZ_TYPE_CLIENT_IP: - zbits &= rpzs->have.client_ipv4; + zbits &= have.client_ipv4; break; case DNS_RPZ_TYPE_IP: - zbits &= rpzs->have.ipv4; + zbits &= have.ipv4; break; case DNS_RPZ_TYPE_NSIP: - zbits &= rpzs->have.nsipv4; + zbits &= have.nsipv4; break; default: INSIST(0); @@ -2116,13 +2119,13 @@ dns_rpz_find_ip(dns_rpz_zones_t *rpzs, dns_rpz_type_t rpz_type, } switch (rpz_type) { case DNS_RPZ_TYPE_CLIENT_IP: - zbits &= rpzs->have.client_ipv6; + zbits &= have.client_ipv6; break; case DNS_RPZ_TYPE_IP: - zbits &= rpzs->have.ipv6; + zbits &= have.ipv6; break; case DNS_RPZ_TYPE_NSIP: - zbits &= rpzs->have.nsipv6; + zbits &= have.nsipv6; break; default: INSIST(0);