</listitem>
<listitem>
<para>
- A bug in the RPZ implementation could cause some policy
- zones that did not specifically require recursion to be
- treated as if they did; consequently, setting
- <command>qname-wait-recurse no;</command> was
- sometimes ineffective. This has been corrected.
- In most configurations, behavioral changes due to this
- fix will not be noticeable. [RT #39229]
- </para>
- </listitem>
- <listitem>
- <para>
- A bug in RPZ could cause the server to crash if policy
- zones were updated (e.g. via <command>rndc reload</command>
- or an incoming zone transfer) while RPZ processing was still
- ongoing for an active query. [RT #39415]
- </para>
- </listitem>
+ Several bugs have been fixed in the RPZ implementation:
+ </para>
+ <itemizedlist>
+ <para>
+ Policy zones that did not specifically require recursion
+ could be treated as if they did; consequently, setting
+ <command>qname-wait-recurse no;</command> was
+ sometimes ineffective. This has been corrected.
+ In most configurations, behavioral changes due to this
+ fix will not be noticeable. [RT #39229]
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ The server could crash if policy zones were updated (e.g.
+ via <command>rndc reload</command> or an incoming zone
+ transfer) while RPZ processing was still ongoing for an
+ active query. [RT #39415]
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ On servers with more than one policy zone configured,
+ if one policy zone was updated via AXFR (especially when
+ the zone was very large) a race condition could allow the
+ RPZ summary data to fall out of sync with the policy zones,
+ potentially leading to an assertion failure in rpz.c.
+ [RT #39567]
+ </para>
+ </listitem>
+ </itemizedlist>
</itemizedlist>
</sect2>
<sect2 id="end_of_life">
dns_fixedname_t fname;
dns_name_t *name;
isc_result_t result = ISC_R_UNEXPECTED;
+ unsigned int node_has_rpz;
INSIST(!ISC_LINK_LINKED(node, deadlink));
switch (node->nsec) {
case DNS_RBT_NSEC_NORMAL:
- if (rbtdb->rpzs != NULL && node->rpz) {
- dns_fixedname_init(&fname);
- name = dns_fixedname_name(&fname);
- dns_rbt_fullnamefromnode(node, name);
- dns_rpz_delete(rbtdb->rpzs, rbtdb->rpz_num, name);
- }
+ /*
+ * Though this may be wasteful, it has to be done before
+ * node is deleted.
+ */
+ dns_fixedname_init(&fname);
+ name = dns_fixedname_name(&fname);
+ dns_rbt_fullnamefromnode(node, name);
+
+ node_has_rpz = node->rpz;
result = dns_rbt_deletenode(rbtdb->tree, node, ISC_FALSE);
+ if (result == ISC_R_SUCCESS &&
+ rbtdb->rpzs != NULL && node_has_rpz)
+ dns_rpz_delete(rbtdb->rpzs, rbtdb->rpz_num, name);
break;
case DNS_RBT_NSEC_HAS_NSEC:
dns_fixedname_init(&fname);
isc_result_totext(result));
}
}
- if (rbtdb->rpzs != NULL && node->rpz)
- dns_rpz_delete(rbtdb->rpzs, rbtdb->rpz_num, name);
+ node_has_rpz = node->rpz;
result = dns_rbt_deletenode(rbtdb->tree, node, ISC_FALSE);
+ if (result == ISC_R_SUCCESS &&
+ rbtdb->rpzs != NULL && node_has_rpz)
+ dns_rpz_delete(rbtdb->rpzs, rbtdb->rpz_num, name);
break;
case DNS_RBT_NSEC_NSEC:
result = dns_rbt_deletenode(rbtdb->nsec, node, ISC_FALSE);
}
if (noderesult == ISC_R_SUCCESS) {
- /*
- * Clean rpz entries added above.
- */
- if (rbtdb->rpzs != NULL && node->rpz)
- dns_rpz_delete(rbtdb->load_rpzs, rbtdb->rpz_num, name);
+ unsigned int node_has_rpz;
+
/*
* Remove the node we just added above.
*/
+ node_has_rpz = node->rpz;
tmpresult = dns_rbt_deletenode(rbtdb->tree, node, ISC_FALSE);
- if (tmpresult != ISC_R_SUCCESS)
+ if (tmpresult == ISC_R_SUCCESS) {
+ /*
+ * Clean rpz entries added above.
+ */
+ if (rbtdb->rpzs != NULL && node_has_rpz)
+ dns_rpz_delete(rbtdb->load_rpzs,
+ rbtdb->rpz_num, name);
+ } else {
isc_log_write(dns_lctx,
DNS_LOGCATEGORY_DATABASE,
DNS_LOGMODULE_CACHE,
"dns_rbt_addnode(NSEC): %s",
isc_result_totext(tmpresult),
isc_result_totext(noderesult));
+ }
}
/*
const dns_rpz_cidr_key_t *tgt_ip, dns_rpz_prefix_t tgt_prefix,
isc_boolean_t inc)
{
- int *cnt;
+ dns_rpz_trigger_counter_t *cnt;
dns_rpz_zbits_t *have;
switch (rpz_type) {
* reload the new zone data into a new blank summary database
* if the reload fails, discard the new summary database
* if the new zone data is acceptable, copy the records for the
- * other zones into the new summary database and replace the
- * old summary database with the new.
+ * other zones into the new summary CIDR and RBT databases
+ * and replace the old summary databases with the new, and
+ * correct the triggers and have values for the updated
+ * zone.
*
* At the first attempt to load a zone, there is no summary data
* for the zone and so no records that need to be deleted.
*/
rpzs->load_begun |= tgt;
dns_rpz_attach_rpzs(rpzs, load_rpzsp);
- UNLOCK(&rpzs->search_lock);
- UNLOCK(&rpzs->maint_lock);
-
} else {
- UNLOCK(&rpzs->search_lock);
- UNLOCK(&rpzs->maint_lock);
-
+ /*
+ * Setup the new RPZ struct with empty summary trees.
+ */
result = dns_rpz_new_zones(load_rpzsp, rpzs->mctx);
if (result != ISC_R_SUCCESS)
return (result);
load_rpzs = *load_rpzsp;
+ /*
+ * Initialize some members so that dns_rpz_add() works.
+ */
load_rpzs->p.num_zones = rpzs->p.num_zones;
- load_rpzs->total_triggers = rpzs->total_triggers;
- memmove(load_rpzs->triggers, rpzs->triggers,
- sizeof(load_rpzs->triggers));
- memset(&load_rpzs->triggers[rpz_num], 0,
- sizeof(load_rpzs->triggers[rpz_num]));
+ memset(&load_rpzs->triggers, 0, sizeof(load_rpzs->triggers));
load_rpzs->zones[rpz_num] = rpz;
isc_refcount_increment(&rpz->refs, NULL);
}
+ UNLOCK(&rpzs->search_lock);
+ UNLOCK(&rpzs->maint_lock);
+
return (ISC_R_SUCCESS);
}
+/*
+ * This function updates "have" bits and also the qname_skip_recurse
+ * mask. It must be called when holding rpzs->search_lock.
+ */
static void
fix_triggers(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num) {
dns_rpz_num_t n;
dns_rpz_zbits_t zbit;
char namebuf[DNS_NAME_FORMATSIZE];
-# define SET_TRIG(n, zbit, type) \
+ /*
+ * rpzs->total_triggers is only used to log a message below.
+ */
+
+ memmove(&old_totals, &rpzs->total_triggers, sizeof(old_totals));
+ memset(&rpzs->total_triggers, 0, sizeof(rpzs->total_triggers));
+
+#define SET_TRIG(n, zbit, type) \
if (rpzs->triggers[n].type == 0) { \
rpzs->have.type &= ~zbit; \
} else { \
rpzs->have.type |= zbit; \
}
- memmove(&old_totals, &rpzs->total_triggers, sizeof(old_totals));
- memset(&rpzs->total_triggers, 0, sizeof(rpzs->total_triggers));
for (n = 0; n < rpzs->p.num_zones; ++n) {
zbit = DNS_RPZ_ZBIT(n);
SET_TRIG(n, zbit, client_ipv4);
SET_TRIG(n, zbit, nsipv6);
}
+#undef SET_TRIG
+
fix_qname_skip_recurse(rpzs);
dns_name_format(&rpzs->zones[rpz_num]->origin,
isc_log_write(dns_lctx, DNS_LOGCATEGORY_RPZ,
DNS_LOGMODULE_RBTDB, DNS_RPZ_INFO_LEVEL,
"(re)loading policy zone '%s' changed from"
- " %d to %d qname, %d to %d nsdname,"
- " %d to %d IP, %d to %d NSIP entries",
+ " %ld to %ld qname, %ld to %ld nsdname,"
+ " %ld to %ld IP, %ld to %ld NSIP entries",
namebuf,
old_totals.qname, rpzs->total_triggers.qname,
old_totals.nsdname, rpzs->total_triggers.nsdname,
rpzs->total_triggers.ipv4 + rpzs->total_triggers.ipv6,
old_totals.nsipv4 + old_totals.nsipv6,
rpzs->total_triggers.nsipv4 + rpzs->total_triggers.nsipv6);
-
-# undef SET_TRIG
}
/*
- * Finish loading one zone.
- * The RBTDB write tree lock must be held.
+ * Finish loading one zone. This function is called during a commit when
+ * a RPZ zone loading is complete. The RBTDB write tree lock must be
+ * held.
+ *
+ * Here, rpzs is a pointer to the view's common rpzs
+ * structure. *load_rpzsp is a rpzs structure that is local to the
+ * RBTDB, which is used during a single zone's load.
+ *
+ * During the zone load, i.e., between dns_rpz_beginload() and
+ * dns_rpz_ready(), only the zone that is being loaded updates
+ * *load_rpzsp. These updates in the summary databases inside load_rpzsp
+ * are made only for the rpz_num (and corresponding bit) of that
+ * zone. Nothing else reads or writes *load_rpzsp. The view's common
+ * rpzs is used during this time for queries.
+ *
+ * When zone loading is complete and we arrive here, the parts of the
+ * summary databases (CIDR and nsdname+qname RBT trees) from the view's
+ * common rpzs struct have to be merged into the summary databases of
+ * *load_rpzsp, as the summary databases of the view's common rpzs
+ * struct may have changed during the time the zone was being loaded.
+ *
+ * The function below carries out the merge. During the merge, it holds
+ * the maint_lock of the view's common rpzs struct so that it is not
+ * updated while the merging is taking place.
+ *
+ * After the merging is carried out, *load_rpzsp contains the most
+ * current state of the rpzs structure, i.e., the summary trees contain
+ * data for the new zone that was just loaded, as well as all other
+ * zones.
+ *
+ * Pointers to the summary databases of *load_rpzsp (CIDR and
+ * nsdname+qname RBT trees) are then swapped into the view's common rpz
+ * struct, so that the query path can continue using it. During the
+ * swap, the search_lock of the view's common rpz struct is acquired so
+ * that queries are paused while this swap occurs.
+ *
+ * The trigger counts for the new zone are also copied into the view's
+ * common rpz struct, and some other summary counts and masks are
+ * updated.
*/
isc_result_t
dns_rpz_ready(dns_rpz_zones_t *rpzs,
if (load_rpzs == rpzs) {
/*
- * This is a successful initial zone loading,
- * perhaps for a new instance of a view.
+ * This is a successful initial zone loading, perhaps
+ * for a new instance of a view.
*/
+ LOCK(&rpzs->search_lock);
fix_triggers(rpzs, rpz_num);
+ UNLOCK(&rpzs->search_lock);
UNLOCK(&rpzs->maint_lock);
dns_rpz_detach_rpzs(load_rpzsp);
return (ISC_R_SUCCESS);
}
}
- fix_triggers(load_rpzs, rpz_num);
-
/*
* Exchange the summary databases.
*/
LOCK(&rpzs->search_lock);
+ rpzs->triggers[rpz_num] = load_rpzs->triggers[rpz_num];
+ fix_triggers(rpzs, rpz_num);
+
found = rpzs->cidr;
rpzs->cidr = load_rpzs->cidr;
load_rpzs->cidr = found;
rpzs->rbt = load_rpzs->rbt;
load_rpzs->rbt = rbt;
- rpzs->total_triggers = load_rpzs->total_triggers;
- rpzs->have = load_rpzs->have;
-
UNLOCK(&rpzs->search_lock);
result = ISC_R_SUCCESS;
projects / thirdparty / bind9.git / commitdiff
