[rbldnsd] Re: rbldnsd 0.99 TTL issue

Michael Tokarev rbldnsd@corpit.ru
Tue, 07 Oct 2003 01:13:34 +0400


This is a multi-part message in MIME format.
--------------090603080901020908000208
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Victor.Duchovni@morganstanley.com wrote:
[TTL of multiple RRs of the same type]

> Thanks that is a reasonable work-around for now. I guess what I am asking
> for is something more automatic, that reflects the minimum volatility of
> datasets rsynced from third party sources.

Ok.  It seems the attached patch will do the work "automagically".
It keeps all RRs of the same type with the same TTL - the smallest
one, but it does nothing if we're have the same data with different
TTLs - first TTL "wins".

Demonstration:

file a1:

   $TTL 11
   4.3.2.1 :1:test

file a2:

   $TTL 12
   4.3.2.1 :2:test

rbldnsd -b 127.0.0.2/2000 -n x:ip4set:a1 x:ip4set:a2
dig @127.0.0.1 -p 2000 -t any 1.2.3.4.x

1.2.3.4.x.              11      IN      A       127.0.0.1
1.2.3.4.x.              11      IN      TXT     "test"
1.2.3.4.x.              11      IN      A       127.0.0.2


rbldnsd -b 127.0.0.2/2000 -n x:ip4set:a2 x:ip4set:a1
dig @127.0.0.1 -p 2000 -t any 1.2.3.4.x

1.2.3.4.x.              11      IN      A       127.0.0.2
1.2.3.4.x.              12      IN      TXT     "test"
1.2.3.4.x.              11      IN      A       127.0.0.1


So, in both cases, A RRs has smallest TTL of the two.
TXT RR's TTLs are inconsistent, but this is not a real
problem, because we have two *identical* TXT RRs anyway.

>>Note that TTL is a propery of a dataset, and *last* value will be
>>used.  (For $SOA, *first* value will be used, and for $NS, *all*
>>values will be used, so here's quite some inconsistency).  Again,
>>throughts/suggestions for this problem are welcome... ;)
> 
> This makes sense, I don't recall running into this in the docs (which I
> have read only in part thus far), but it is the only sensible default

Heh... Documentation is silent at this point... ;)

> behaviour. It might be nice to be able to suppress metadata explicitly
> from a dataset, so it supplies data records only.

I just don't really know how to do this in a nice way.  As i already
described, $SOA and $TTL handling is inconsistent in this respect:
reall TTL will be of last $TTL statement, but real SOA will be first
encountered.  But the problem is that it is not entirely true.  In
combined dataset for example, one may have different TTLs for different
"components" (subdatasets).  There's default TTL for the whole combined
dataset, but it may be overwritten inside individual subdataset.  Also,
generic dataset and metadata has per-record TTL (unlike other datasets),
and the TTL of an individual record defaults to the TTL of the given
scope.  That all to say: I can't forbid redifinition of a TTL like I do
for $SOA, because this way I'm restricting too much.  The whole thing
is already too complicated to describe, and adding more options (or
"super-metadata") makes the whole thing completely non-understandable.

DJB goes the simplest way here: he chooses arbitrary hardcoded TTL
value which can't be changed at all.  This is very simple and easy,
but is impractical.

There's another very similar issue with A values.  Suppose you want
to combine data from diffferent sources, giving each "source" it's
own A value, so it will be possible to do different things based on
results of one query to this combined zone.  How to force A values
different from the original?  Changing source file - one byte of
several MBs - seems to be impractical too (you have to keep original
file with original timestamp for rsync/whatever).  A natural way
may be to change A values using a trick similar to the one with TTL
above - as suggested earlier, by adding some local files.  But how
to specify all the necessary logic?  And what to do if original
data ALREADY contains several "types" of listings, with different
A records?  Ugh.

There's no clear answer to all of this.  And i think I will leave
all this as it is already, just fixing this issue you pointed out.

Thanks for the bugreport.

/mjt

--------------090603080901020908000208
Content-Type: text/plain;
 name="rbldnsd-minttl.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="rbldnsd-minttl.diff"

Index: rbldnsd_packet.c
===================================================================
RCS file: /ws/CVS/rbldnsd/rbldnsd_packet.c,v
retrieving revision 1.75
diff -u -r1.75 rbldnsd_packet.c
--- rbldnsd_packet.c	10 Aug 2003 15:48:47 -0000	1.75
+++ rbldnsd_packet.c	6 Oct 2003 20:52:50 -0000
@@ -613,27 +613,52 @@
   return 1;
 }
 
-/* check whenever a given RR is already in the packet
- * (to suppress duplicate answers)
- * May be unnecessary? */
-static int aexists(const struct dnspacket *pkt, unsigned typ,
-                   const void *val, unsigned vlen) {
-  const unsigned char *c, *e;
-  for(c = pkt->p_sans, e = pkt->p_cur; c < e; c = c + 12 + c[11]) {
-    if (c[2] == (typ>>8) && c[3] == (typ&255) &&
-        c[11] == vlen && memcmp(c + 12, val, vlen) == 0)
-      return 1;
-  }
-  return 0;
-}
-
 /* add a new record into answer, check for dups.
  * We just ignore any data that exceeds packet size */
 void addrr_any(struct dnspacket *pkt, unsigned dtp,
                const void *data, unsigned dsz,
                const unsigned char ttl[4]) {
-  register unsigned char *c = pkt->p_cur;
-  if (aexists(pkt, dtp, data, dsz)) return;
+  register unsigned char *c, *e, *f;
+
+  c = pkt->p_sans; e = pkt->p_cur;
+#define nextRR(c) ((c) + 12 + (c)[11])
+#define hasRR(c,e) ((c) < (e))
+#define sameRRT(c,dtp) ((c)[2] == ((dtp)>>8) && (c)[3] == ((dtp)&255))
+#define sameDATA(c,dsz,data) ((c)[11] == (dsz) && memcmp((c)+12, (data), (dsz)) == 0)
+#define rrTTL(c) ((c)+6)
+
+  /* check whenever we already have this (type of) RR in reply */
+  while(hasRR(c,e)) {
+    if (!sameRRT(c,dtp)) { c = nextRR(c); continue; } /* skip different type */
+    if (sameDATA(c,dsz,data)) return; /* first RR has this data - nothing to do */
+    f = c; /* remember first RR of this type */
+    /* see whenever we have other RRs of the same type and with the same data */
+    for(c = nextRR(c); hasRR(c,e); c = nextRR(c))
+      if (sameRRT(c,dtp) && sameDATA(c,dsz,data))
+        return;
+    /* the same RR was not found, so we're going to insert new RR */
+    /* check whenever we should fix TTL.
+     * If new RR has the same TTL as existing ones, nothing to do.
+     * If new RR has larger TTL, use existing (smaller) TTL instead.
+     * If new TTL is smaller, made all other TTLs this small too.
+     */
+    if (memcmp(rrTTL(f), ttl, 4) == 0) break; /* the same, nothing to do */
+    if (unpack32(ttl) > unpack32(rrTTL(f)))
+      ttl = rrTTL(f); /* use existing, smaller TTL for new RR */
+    else { /* change TTLs of existing RRs to new */
+      memcpy(rrTTL(f), ttl, 4);
+      for(f = nextRR(f); hasRR(f,e); f = nextRR(f))
+        if (sameRRT(f,dtp))
+          memcpy(rrTTL(f), ttl, 4);
+    }
+    break;
+  }
+#undef nextRR
+#undef hasRR
+#undef sameRRT
+#undef sameDATA
+#undef rrTTL
+
   if (!fit(pkt, c, 12 + dsz)) {
     setnonauth(pkt->p_buf); /* non-auth answer as we can't fit the record */
     return;

--------------090603080901020908000208--