bad referrals

[Don Lewis]

On Dec 8, 11:51pm, Mark.Andrews at iengines.com wrote:
} Subject: Re: bad referrals

} 	The first hunk is definitly wrong even with forwarders.
}
} 	At this point in the code we have identified a referral
} 	answer, so even if we get one from a forwarder it is lame
} 	as far as we are concerned.

I'm not convinced of that.  Lameness is only a property of authoritative
servers.  If a forwarder answers from its cache, the AA bit will
not be set.  If the answer is a cached NODATA and the NS RRset
expired from the forwarder's cache before it answered the query
(which shouldn't happen given sane TTLs), the response will look a
lot like a lame delegation.  We shouldn't care, though, since if we
are using a forwarder we really only care about the answer section.

}       The identifing the referral
} 	should be made stronger as the code assumes wrongly that
} 	the SOA record, if present, will be first in the section.

We should also probably fix the forwarder to refetch the SOA if it
has expired before the cached NODATA or NXDOMAIN before it responds
to its client so that the client knows the proper TTL to use when
it caches the answer.

} 	The second and third hucks cover too much in the if.

After looking at the code again, I agree.

} 	Testing for "fwd != NULL" in the "else if (...)" would be
} 	a more reasonable patch.

Blindly looking at fwd is no longer correct now that BIND does
selective forwarding.  Responses to recursive and non-recursive
responses need to be treated differently in a number of cases.
Probably the best thing to do is to only set fwd if the RD and RA
bits are set in the response.  That would fix the ambiguity of when
to update RTT.  It would slightly speed up the handling of responses
to non-recursive queries.

Here's a revised patch:

--- ns_resp.c.orig	Thu Nov  4 20:40:57 1999
+++ ns_resp.c	Wed Dec  8 14:54:59 1999
@@ -303,7 +303,7 @@
 	time_t rtrip;
 	struct hashbuf *htp;
 	struct namebuf *np;
-	struct fwdinfo *fwd;
+	struct fwdinfo *fwd = NULL;
 	struct databuf *dp;
 	int forcecmsg = 0;
 	char *tname = NULL;
@@ -462,16 +462,11 @@
 	 * might have forwarded the query will be dropped.
 	 * XXX - should put this in STATS somewhere.
 	 */
-	for (fwd = NS_ZFWDTAB(qp->q_fzone); fwd; fwd = fwd->next)
-		if (ina_equal(fwd->fwdaddr.sin_addr, from.sin_addr))
-			break;
-	/*
-	 * XXX:	note bad ambiguity here.  if one of our forwarders is also
-	 *	a delegated server for some domain, then we will not update
-	 *	the RTT information on any replies we get from those servers.
-	 *	Workaround: disable recursion on authoritative servers so that
-	 *	the ambiguity does not arise.
-	 */
+	if (hp->rd && hp->ra) {
+		for (fwd = NS_ZFWDTAB(qp->q_fzone); fwd; fwd = fwd->next)
+			if (ina_equal(fwd->fwdaddr.sin_addr, from.sin_addr))
+				break;
+	}
 	/*
 	 * If we weren't using a forwarder, find the qinfo pointer and update
 	 * the rtt and fact that we have called on this server before.
@@ -751,7 +746,8 @@
 		 * classes tend to not have good strong delegation graphs).
 		 */
 
-		if (type == T_NS && ns_samedomain(qp->q_domain, name)) {
+		if (type == T_NS && fwd == NULL &&
+		    ns_samedomain(qp->q_domain, name)) {
 			nameserIncr(from.sin_addr, nssRcvdLDel);
 			mark_lame(qp, from);
 			mark_bad(qp, from);
@@ -984,8 +980,9 @@
 							name[0] ? name : ".");
 						db_freedata(dp);
 						continue;
-					} else if (!ns_samedomain(name,
-							       qp->q_domain)) {
+					} else if (fwd == NULL &&
+						   !ns_samedomain(name,
+							        qp->q_domain)) {
 						if (!externalcname)
 						    ns_info(ns_log_resp_checks,
 						    "bad referral (%s !< %s)",