Detect recursion loops during query processing

Interrupt query processing when query_recurse() attempts to ask the same
name servers for the same QNAME/QTYPE tuple for two times in a row as
this indicates that query processing may be stuck for an indeterminate
period of time, e.g. due to interactions between features able to
restart query_lookup().

diff --git a/lib/ns/include/ns/query.h b/lib/ns/include/ns/query.h
index 365cb8c7549a7a5e11761e20970e471a25a3e2e7..c9d0cfb43c4b11f67648accc9521b76b3837b898 100644 (file)
--- a/lib/ns/include/ns/query.h
+++ b/lib/ns/include/ns/query.h
@@ -34,6 +34,18 @@ typedef struct ns_dbversion {
        ISC_LINK(struct ns_dbversion)   link;
 } ns_dbversion_t;
 
+/*%
+ * nameserver recursion parameters, to uniquely identify a recursion
+ * query; this is used to detect a recursion loop
+ */
+typedef struct ns_query_recparam {
+       dns_rdatatype_t                 qtype;
+       dns_name_t *                    qname;
+       dns_fixedname_t                 fqname;
+       dns_name_t *                    qdomain;
+       dns_fixedname_t                 fqdomain;
+} ns_query_recparam_t;
+
 /*% nameserver query structure */
 struct ns_query {
        unsigned int                    attributes;
@@ -62,6 +74,7 @@ struct ns_query {
        unsigned int                    dns64_aaaaoklen;
        unsigned int                    dns64_options;
        unsigned int                    dns64_ttl;
+
        struct {
                dns_db_t *              db;
                dns_zone_t *            zone;
@@ -76,6 +89,8 @@ struct ns_query {
                isc_boolean_t           is_zone;
        } redirect;
 
+       ns_query_recparam_t             recparam;
+
        dns_keytag_t root_key_sentinel_keyid;
        isc_boolean_t root_key_sentinel_is_ta;
        isc_boolean_t root_key_sentinel_not_ta;

diff --git a/lib/ns/query.c b/lib/ns/query.c
index d6e9a4dffdc6827804dc3248c539e726430ca219..ec54bfbcc1f462a06e2d7a3fb48e303b43cc257e 100644 (file)
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -347,6 +347,10 @@ query_lookup(query_ctx_t *qctx);
 static void
 fetch_callback(isc_task_t *task, isc_event_t *event);
 
+static void
+recparam_update(ns_query_recparam_t *param, dns_rdatatype_t qtype,
+               const dns_name_t *qname, const dns_name_t *qdomain);
+
 static isc_result_t
 query_recurse(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qname,
              dns_name_t *qdomain, dns_rdataset_t *nameservers,
@@ -701,6 +705,7 @@ query_reset(ns_client_t *client, isc_boolean_t everything) {
        client->query.isreferral = ISC_FALSE;
        client->query.dns64_options = 0;
        client->query.dns64_ttl = ISC_UINT32_MAX;
+       recparam_update(&client->query.recparam, 0, NULL, NULL);
        client->query.root_key_sentinel_keyid = 0;
        client->query.root_key_sentinel_is_ta = ISC_FALSE;
        client->query.root_key_sentinel_not_ta = ISC_FALSE;
@@ -5593,6 +5598,54 @@ fetch_callback(isc_task_t *task, isc_event_t *event) {
        dns_resolver_destroyfetch(&fetch);
 }
 
+/*%
+ * Check whether the recursion parameters in 'param' match the current query's
+ * recursion parameters provided in 'qtype', 'qname', and 'qdomain'.
+ */
+static isc_boolean_t
+recparam_match(const ns_query_recparam_t *param, dns_rdatatype_t qtype,
+              const dns_name_t *qname, const dns_name_t *qdomain)
+{
+       REQUIRE(param != NULL);
+
+       return (ISC_TF(param->qtype == qtype &&
+                      param->qname != NULL && qname != NULL &&
+                      param->qdomain != NULL && qdomain != NULL &&
+                      dns_name_equal(param->qname, qname) &&
+                      dns_name_equal(param->qdomain, qdomain)));
+}
+
+/*%
+ * Update 'param' with current query's recursion parameters provided in
+ * 'qtype', 'qname', and 'qdomain'.
+ */
+static void
+recparam_update(ns_query_recparam_t *param, dns_rdatatype_t qtype,
+               const dns_name_t *qname, const dns_name_t *qdomain)
+{
+       isc_result_t result;
+
+       REQUIRE(param != NULL);
+
+       param->qtype = qtype;
+
+       if (qname == NULL) {
+               param->qname = NULL;
+       } else {
+               param->qname = dns_fixedname_initname(&param->fqname);
+               result = dns_name_copy(qname, param->qname, NULL);
+               RUNTIME_CHECK(result == ISC_R_SUCCESS);
+       }
+
+       if (qdomain == NULL) {
+               param->qdomain = NULL;
+       } else {
+               param->qdomain = dns_fixedname_initname(&param->fqdomain);
+               result = dns_name_copy(qdomain, param->qdomain, NULL);
+               RUNTIME_CHECK(result == ISC_R_SUCCESS);
+       }
+}
+
 /*%
  * Prepare client for recursion, then create a resolver fetch, with
  * the event callback set to fetch_callback(). Afterward we terminate
@@ -5610,6 +5663,19 @@ query_recurse(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qname,
 
        CTRACE(ISC_LOG_DEBUG(3), "query_recurse");
 
+       /*
+        * Check recursion parameters from the previous query to see if they
+        * match.  If not, update recursion parameters and proceed.
+        */
+       if (recparam_match(&client->query.recparam, qtype, qname, qdomain)) {
+               ns_client_log(client, NS_LOGCATEGORY_CLIENT,
+                             NS_LOGMODULE_QUERY, ISC_LOG_INFO,
+                             "recursion loop detected");
+               return (ISC_R_FAILURE);
+       }
+
+       recparam_update(&client->query.recparam, qtype, qname, qdomain);
+
        if (!resuming)
                inc_stats(client, ns_statscounter_recursion);