merge-similar-outgoing-queries.patch and lame servers
Gerrit Pape <[email protected]>
| Newsgroups | gmane.network.djbdns |
|---|---|
| Message-ID | <20090303140153.22121.qmail@2e5f793c37f63b.315fe32.mid.smarden.org> |
On Mon, Feb 09, 2009 at 01:41:37PM -0600, Kevin Day wrote: > Specifically, two patches were developed by Jeff King ([email protected]) > that close the poisoning attack weaknesses discussed here. The first > prevents duplicate identical queries from being sent at once. Multiple > requests for the same data from one or more clients are merged > together into a single request. Apart from the security implications, > there are several benefits from installing this patch - a small > bandwidth savings, and protection from a lot of users all trying to > request the same data from an overloaded server won't make matters > worse. Hi, it looks like the patch doesn't handle responses from lame servers properly. If such a single request gets a response from a lame server, it can happen that no further requests are sent. To reproduce, look for a domain with a lame server, and do DNSCACHEIP=<ip-of-patched-dnscache> dnsqr soa domain.with.lame.server & dnsqr soa domain.with.lame.server & You can try multiple times. If you're unlucky, all subsequent same client requests will be merged into a single request that no longer exists; you'll only see 'txpb' lines in the logs, no 'tx' lines. No answer is sent back to clients, they run in a timeout. The quick&dirty patch below, against a patched dnscache, works around this. Regards, Gerrit. PS: The 'goto LOWERLEVEL;' case should be checked too.
diff
(text/plain, 2.7 KB)
diff --git a/qmerge.c b/qmerge.c
index 7c92299..f6b8284 100644
--- a/qmerge.c
+++ b/qmerge.c
@@ -49,7 +49,7 @@ void qmerge_free(struct qmerge **x)
int qmerge_start(struct qmerge **qm, const char servers[64], int flagrecursive,
const char *q, const char qtype[2], const char localip[4],
- const char *control)
+ const char *control, unsigned int forcetx)
{
struct qmerge_key k;
int i;
@@ -62,6 +62,7 @@ int qmerge_start(struct qmerge **qm, const char servers[64], int flagrecursive,
for (i = 0; i < QMERGE_MAX; i++) {
if (!inprogress[i].active) continue;
if (!qmerge_key_equal(&k, &inprogress[i].key)) continue;
+ if (forcetx) goto TRANSMIT;
log_tx_piggyback(q, qtype, control);
inprogress[i].active++;
*qm = &inprogress[i];
@@ -74,6 +75,7 @@ int qmerge_start(struct qmerge **qm, const char servers[64], int flagrecursive,
break;
if (i == QMERGE_MAX) return -1;
+TRANSMIT:
log_tx(q, qtype, control, servers, 0);
r = dns_transmit_start(&inprogress[i].dt, servers, flagrecursive, q, qtype, localip);
if (r == -1) { qmerge_key_free(&k); return -1; }
diff --git a/qmerge.h b/qmerge.h
index 9a58157..80ddb29 100644
--- a/qmerge.h
+++ b/qmerge.h
@@ -16,7 +16,7 @@ struct qmerge {
int state; /* -1 = error, 0 = need io, 1 = need get, 2 = got packet */
};
-extern int qmerge_start(struct qmerge **,const char *,int,const char *,const char *,const char *,const char *);
+extern int qmerge_start(struct qmerge **,const char *,int,const char *,const char *,const char *,const char *, unsigned int);
extern void qmerge_io(struct qmerge *,iopause_fd *,struct taia *);
extern int qmerge_get(struct qmerge **,const iopause_fd *,const struct taia *);
extern void qmerge_free(struct qmerge **);
diff --git a/query.c b/query.c
index f091fdd..aaf8d41 100644
--- a/query.c
+++ b/query.c
@@ -193,6 +193,7 @@ static int doit(struct query *z,int state)
int k;
int p;
int q;
+ unsigned int forcetx = 0;
errno = error_io;
if (state == 1) goto HAVEPACKET;
@@ -430,7 +431,7 @@ static int doit(struct query *z,int state)
dns_sortip(z->servers[z->level],64);
dtype = z->level ? DNS_T_A : z->type;
- if (qmerge_start(&z->qm,z->servers[z->level],flagforwardonly,z->name[z->level],dtype,z->localip,z->control[z->level]) == -1) goto DIE;
+ if (qmerge_start(&z->qm,z->servers[z->level],flagforwardonly,z->name[z->level],dtype,z->localip,z->control[z->level], forcetx) == -1) goto DIE;
return 0;
@@ -514,6 +515,7 @@ static int doit(struct query *z,int state)
if (dns_domain_equal(referral,control) || !dns_domain_suffix(referral,control)) {
log_lame(whichserver,control,referral);
byte_zero(whichserver,4);
+ forcetx = 1;
goto HAVENS;
}