]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Remove dns_adblameinfo from dns_adb
authorOndřej Surý <ondrej@isc.org>
Thu, 21 Sep 2023 13:20:58 +0000 (15:20 +0200)
committerOndřej Surý <ondrej@isc.org>
Thu, 12 Oct 2023 10:35:00 +0000 (12:35 +0200)
Keeping the information about lame server in the ADB was done in !322 to
fix following security issue:

    [CVE-2021-25219] Disable "lame-ttl" cache

The handling of the lame servers needs to be redesigned and it is not
going to be enabled any time soon, and the current code is just dead
code that takes up space, code and stands in the way of making ADB work
faster.

Remove all the internals needed for handling the lame servers in the ADB
for now.  It might get reintroduced later if and when we redesign ADB.

lib/dns/adb.c
lib/dns/include/dns/adb.h
lib/dns/resolver.c
lib/dns/zone.c

index c47b2219aae417b2c42e3cee362ab8380de1dd57..226b10e3ead1c53fb66679495db92b6d98263f6b 100644 (file)
@@ -50,8 +50,6 @@
 #define DNS_ADBNAME_VALID(x)    ISC_MAGIC_VALID(x, DNS_ADBNAME_MAGIC)
 #define DNS_ADBNAMEHOOK_MAGIC   ISC_MAGIC('a', 'd', 'N', 'H')
 #define DNS_ADBNAMEHOOK_VALID(x) ISC_MAGIC_VALID(x, DNS_ADBNAMEHOOK_MAGIC)
-#define DNS_ADBLAMEINFO_MAGIC   ISC_MAGIC('a', 'd', 'b', 'Z')
-#define DNS_ADBLAMEINFO_VALID(x) ISC_MAGIC_VALID(x, DNS_ADBLAMEINFO_MAGIC)
 #define DNS_ADBENTRY_MAGIC      ISC_MAGIC('a', 'd', 'b', 'E')
 #define DNS_ADBENTRY_VALID(x)   ISC_MAGIC_VALID(x, DNS_ADBENTRY_MAGIC)
 #define DNS_ADBFETCH_MAGIC      ISC_MAGIC('a', 'd', 'F', '4')
@@ -87,7 +85,6 @@
 typedef ISC_LIST(dns_adbname_t) dns_adbnamelist_t;
 typedef struct dns_adbnamehook dns_adbnamehook_t;
 typedef ISC_LIST(dns_adbnamehook_t) dns_adbnamehooklist_t;
-typedef struct dns_adblameinfo dns_adblameinfo_t;
 typedef ISC_LIST(dns_adbentry_t) dns_adbentrylist_t;
 typedef struct dns_adbfetch dns_adbfetch_t;
 typedef struct dns_adbfetch6 dns_adbfetch6_t;
@@ -201,23 +198,6 @@ struct dns_adbnamehook {
        ISC_LINK(dns_adbnamehook_t) entry_link;
 };
 
-/*%
- * dns_adblameinfo structure:
- *
- * This is a small widget that holds qname-specific information about an
- * address.  Currently limited to lameness, but could just as easily be
- * extended to other types of information about zones.
- */
-struct dns_adblameinfo {
-       unsigned int magic;
-
-       dns_name_t qname;
-       dns_rdatatype_t qtype;
-       isc_stdtime_t lame_timer;
-
-       ISC_LINK(dns_adblameinfo_t) plink;
-};
-
 /*%
  * dns_adbentry structure:
  *
@@ -269,9 +249,6 @@ struct dns_adbentry {
         * entry.
         */
 
-       /* FIXME */
-       ISC_LIST(dns_adblameinfo_t) lameinfo;
-
        ISC_LINK(dns_adbentry_t) link;
 };
 
@@ -304,10 +281,6 @@ static dns_adbnamehook_t *
 new_adbnamehook(dns_adb_t *adb);
 static void
 free_adbnamehook(dns_adb_t *adb, dns_adbnamehook_t **namehookp);
-static dns_adblameinfo_t *
-new_adblameinfo(dns_adb_t *, const dns_name_t *, dns_rdatatype_t);
-static void
-free_adblameinfo(dns_adb_t *, dns_adblameinfo_t **);
 static dns_adbentry_t *
 new_adbentry(dns_adb_t *adb, const isc_sockaddr_t *addr);
 static void
@@ -444,7 +417,6 @@ enum {
 #define FIND_AVOIDFETCHES(fn)  (((fn)->options & DNS_ADBFIND_AVOIDFETCHES) != 0)
 #define FIND_STARTATZONE(fn)   (((fn)->options & DNS_ADBFIND_STARTATZONE) != 0)
 #define FIND_HAS_ADDRS(fn)     (!ISC_LIST_EMPTY((fn)->list))
-#define FIND_RETURNLAME(fn)    (((fn)->options & DNS_ADBFIND_RETURNLAME) != 0)
 #define FIND_NOFETCH(fn)       (((fn)->options & DNS_ADBFIND_NOFETCH) != 0)
 
 /*
@@ -1087,39 +1059,6 @@ free_adbnamehook(dns_adb_t *adb, dns_adbnamehook_t **namehook) {
        isc_mem_put(adb->mctx, nh, sizeof(*nh));
 }
 
-static dns_adblameinfo_t *
-new_adblameinfo(dns_adb_t *adb, const dns_name_t *qname,
-               dns_rdatatype_t qtype) {
-       dns_adblameinfo_t *li = isc_mem_get(adb->mctx, sizeof(*li));
-
-       dns_name_init(&li->qname, NULL);
-       dns_name_dup(qname, adb->mctx, &li->qname);
-       li->magic = DNS_ADBLAMEINFO_MAGIC;
-       li->lame_timer = 0;
-       li->qtype = qtype;
-       ISC_LINK_INIT(li, plink);
-
-       return (li);
-}
-
-static void
-free_adblameinfo(dns_adb_t *adb, dns_adblameinfo_t **lameinfo) {
-       dns_adblameinfo_t *li = NULL;
-
-       REQUIRE(lameinfo != NULL && DNS_ADBLAMEINFO_VALID(*lameinfo));
-
-       li = *lameinfo;
-       *lameinfo = NULL;
-
-       REQUIRE(!ISC_LINK_LINKED(li, plink));
-
-       dns_name_free(&li->qname, adb->mctx);
-
-       li->magic = 0;
-
-       isc_mem_put(adb->mctx, li, sizeof(*li));
-}
-
 static dns_adbentry_t *
 new_adbentry(dns_adb_t *adb, const isc_sockaddr_t *addr) {
        dns_adbentry_t *entry = NULL;
@@ -1128,7 +1067,6 @@ new_adbentry(dns_adb_t *adb, const isc_sockaddr_t *addr) {
        *entry = (dns_adbentry_t){
                .srtt = isc_random_uniform(0x1f) + 1,
                .sockaddr = *addr,
-               .lameinfo = ISC_LIST_INITIALIZER,
                .link = ISC_LINK_INITIALIZER,
                .magic = DNS_ADBENTRY_MAGIC,
        };
@@ -1154,7 +1092,6 @@ static void
 destroy_adbentry(dns_adbentry_t *entry) {
        REQUIRE(DNS_ADBENTRY_VALID(entry));
 
-       dns_adblameinfo_t *li = NULL;
        dns_adb_t *adb = entry->adb;
        uint_fast32_t active;
 
@@ -1171,13 +1108,6 @@ destroy_adbentry(dns_adbentry_t *entry) {
                isc_mem_put(adb->mctx, entry->cookie, entry->cookielen);
        }
 
-       li = ISC_LIST_HEAD(entry->lameinfo);
-       while (li != NULL) {
-               ISC_LIST_UNLINK(entry->lameinfo, li, plink);
-               free_adblameinfo(adb, &li);
-               li = ISC_LIST_HEAD(entry->lameinfo);
-       }
-
        isc_mutex_destroy(&entry->lock);
        isc_mem_put(adb->mctx, entry, sizeof(*entry));
 
@@ -1531,48 +1461,6 @@ get_attached_and_locked_entry(dns_adb_t *adb, isc_stdtime_t now,
        return (adbentry);
 }
 
-/*
- * The entry must be locked.
- */
-static bool
-entry_is_lame(dns_adb_t *adb, dns_adbentry_t *entry, const dns_name_t *qname,
-             dns_rdatatype_t qtype, isc_stdtime_t now) {
-       dns_adblameinfo_t *li = NULL, *next_li = NULL;
-       bool is_bad = false;
-
-       li = ISC_LIST_HEAD(entry->lameinfo);
-       if (li == NULL) {
-               return (false);
-       }
-       while (li != NULL) {
-               next_li = ISC_LIST_NEXT(li, plink);
-
-               /*
-                * Has the entry expired?
-                */
-               if (li->lame_timer < now) {
-                       ISC_LIST_UNLINK(entry->lameinfo, li, plink);
-                       free_adblameinfo(adb, &li);
-               }
-
-               /*
-                * Order tests from least to most expensive.
-                *
-                * We do not break out of the main loop here as
-                * we use the loop for house keeping.
-                */
-               if (li != NULL && !is_bad && li->qtype == qtype &&
-                   dns_name_equal(qname, &li->qname))
-               {
-                       is_bad = true;
-               }
-
-               li = next_li;
-       }
-
-       return (is_bad);
-}
-
 static void
 log_quota(dns_adbentry_t *entry, const char *fmt, ...) {
        va_list ap;
@@ -1595,9 +1483,7 @@ log_quota(dns_adbentry_t *entry, const char *fmt, ...) {
 }
 
 static void
-copy_namehook_lists(dns_adb_t *adb, dns_adbfind_t *find,
-                   const dns_name_t *qname, dns_rdatatype_t qtype,
-                   dns_adbname_t *name, isc_stdtime_t now) {
+copy_namehook_lists(dns_adb_t *adb, dns_adbfind_t *find, dns_adbname_t *name) {
        dns_adbnamehook_t *namehook = NULL;
        dns_adbentry_t *entry = NULL;
 
@@ -1609,15 +1495,7 @@ copy_namehook_lists(dns_adb_t *adb, dns_adbfind_t *find,
                        LOCK(&entry->lock);
 
                        if (adbentry_overquota(entry)) {
-                               find->options |= (DNS_ADBFIND_LAMEPRUNED |
-                                                 DNS_ADBFIND_OVERQUOTA);
-                               goto nextv4;
-                       }
-
-                       if (!FIND_RETURNLAME(find) &&
-                           entry_is_lame(adb, entry, qname, qtype, now))
-                       {
-                               find->options |= DNS_ADBFIND_LAMEPRUNED;
+                               find->options |= DNS_ADBFIND_OVERQUOTA;
                                goto nextv4;
                        }
 
@@ -1641,17 +1519,10 @@ copy_namehook_lists(dns_adb_t *adb, dns_adbfind_t *find,
                        LOCK(&entry->lock);
 
                        if (adbentry_overquota(entry)) {
-                               find->options |= (DNS_ADBFIND_LAMEPRUNED |
-                                                 DNS_ADBFIND_OVERQUOTA);
+                               find->options |= DNS_ADBFIND_OVERQUOTA;
                                goto nextv6;
                        }
 
-                       if (!FIND_RETURNLAME(find) &&
-                           entry_is_lame(adb, entry, qname, qtype, now))
-                       {
-                               find->options |= DNS_ADBFIND_LAMEPRUNED;
-                               goto nextv6;
-                       }
                        addrinfo = new_adbaddrinfo(adb, entry, find->port);
 
                        /*
@@ -2067,7 +1938,7 @@ dns_adb_shutdown(dns_adb_t *adb) {
 isc_result_t
 dns_adb_createfind(dns_adb_t *adb, isc_loop_t *loop, isc_job_cb cb, void *cbarg,
                   const dns_name_t *name, const dns_name_t *qname,
-                  dns_rdatatype_t qtype, unsigned int options,
+                  dns_rdatatype_t qtype ISC_ATTR_UNUSED, unsigned int options,
                   isc_stdtime_t now, dns_name_t *target, in_port_t port,
                   unsigned int depth, isc_counter_t *qc,
                   dns_adbfind_t **findp) {
@@ -2299,7 +2170,7 @@ fetch:
         * Run through the name and copy out the bits we are
         * interested in.
         */
-       copy_namehook_lists(adb, find, qname, qtype, adbname, now);
+       copy_namehook_lists(adb, find, adbname);
 
 post_copy:
        if (NAME_FETCH_A(adbname)) {
@@ -2568,9 +2439,7 @@ static void
 dump_entry(FILE *f, dns_adb_t *adb, dns_adbentry_t *entry, bool debug,
           isc_stdtime_t now) {
        char addrbuf[ISC_NETADDR_FORMATSIZE];
-       char typebuf[DNS_RDATATYPE_FORMATSIZE];
        isc_netaddr_t netaddr;
-       dns_adblameinfo_t *li = NULL;
 
        isc_netaddr_fromsockaddr(&netaddr, &entry->sockaddr);
        isc_netaddr_format(&netaddr, addrbuf, sizeof(addrbuf));
@@ -2607,15 +2476,6 @@ dump_entry(FILE *f, dns_adb_t *adb, dns_adbentry_t *entry, bool debug,
        }
 
        fprintf(f, "\n");
-       for (li = ISC_LIST_HEAD(entry->lameinfo); li != NULL;
-            li = ISC_LIST_NEXT(li, plink))
-       {
-               fprintf(f, ";\t\t");
-               dns_name_print(&li->qname, f);
-               dns_rdatatype_format(li->qtype, typebuf, sizeof(typebuf));
-               fprintf(f, " %s [lame TTL %d]\n", typebuf,
-                       (int)(li->lame_timer - now));
-       }
 }
 
 static void
@@ -3164,41 +3024,6 @@ cleanup:
        return (result);
 }
 
-isc_result_t
-dns_adb_marklame(dns_adb_t *adb, dns_adbaddrinfo_t *addr,
-                const dns_name_t *qname, dns_rdatatype_t qtype,
-                isc_stdtime_t expire_time) {
-       REQUIRE(DNS_ADB_VALID(adb));
-       REQUIRE(DNS_ADBADDRINFO_VALID(addr));
-       REQUIRE(qname != NULL);
-
-       isc_result_t result = ISC_R_SUCCESS;
-       dns_adblameinfo_t *li = NULL;
-       dns_adbentry_t *entry = addr->entry;
-
-       LOCK(&entry->lock);
-       li = ISC_LIST_HEAD(entry->lameinfo);
-       while (li != NULL &&
-              (li->qtype != qtype || !dns_name_equal(qname, &li->qname)))
-       {
-               li = ISC_LIST_NEXT(li, plink);
-       }
-       if (li != NULL) {
-               if (expire_time > li->lame_timer) {
-                       li->lame_timer = expire_time;
-               }
-               goto unlock;
-       }
-       li = new_adblameinfo(adb, qname, qtype);
-       li->lame_timer = expire_time;
-
-       ISC_LIST_PREPEND(addr->entry->lameinfo, li, plink);
-
-unlock:
-       UNLOCK(&entry->lock);
-       return (result);
-}
-
 void
 dns_adb_adjustsrtt(dns_adb_t *adb, dns_adbaddrinfo_t *addr, unsigned int rtt,
                   unsigned int factor) {
index f7dee889e1de42cf09306191fcc9c2032e6a767f..e1b94ed369d0094752bd0fe44a97adea103f5310 100644 (file)
  * Records are stored internally until a timer expires. The timer is the
  * smaller of the TTL or signature validity period.
  *
- * Lameness is stored per <qname,qtype> tuple, and this data hangs off each
- * address field.  When an address is marked lame for a given tuple the address
- * will not be returned to a caller.
- *
- *
  * MP:
  *
  *\li  The ADB takes care of all necessary locking.
@@ -159,13 +154,6 @@ struct dns_adbfind {
  * _STARTATZONE:
  *     Fetches will start using the closest zone data or use the root servers.
  *     This is useful for reestablishing glue that has expired.
- *
- * _RETURNLAME:
- *     Return lame servers in a find, so that all addresses are returned.
- *
- * _LAMEPRUNED:
- *     At least one address was omitted from the list because it was lame.
- *     This bit will NEVER be set if _RETURNLAME is set in the createfind().
  */
 /*% Return addresses of type INET. */
 #define DNS_ADBFIND_INET 0x00000001
@@ -192,15 +180,6 @@ struct dns_adbfind {
  *     This is useful for reestablishing glue that has expired.
  */
 #define DNS_ADBFIND_STARTATZONE 0x00000020
-/*%
- *     Return lame servers in a find, so that all addresses are returned.
- */
-#define DNS_ADBFIND_RETURNLAME 0x00000100
-/*%
- *      Only schedule an event if no addresses are known.
- *      Must set _WANTEVENT for this to be meaningful.
- */
-#define DNS_ADBFIND_LAMEPRUNED 0x00000200
 /*%
  *      The server's fetch quota is exceeded; it will be treated as
  *      lame for this query.
@@ -320,8 +299,7 @@ dns_adb_createfind(dns_adb_t *adb, isc_loop_t *loop, isc_job_cb cb, void *cbarg,
  *
  * The list of addresses returned is unordered.  The caller must impose
  * any ordering required.  The list will not contain "known bad" addresses,
- * however.  For instance, it will not return hosts that are known to be
- * lame for the zone in question.
+ * however.
  *
  * The caller cannot (directly) modify the contents of the address list's
  * fields other than the "link" field.  All values can be read at any
@@ -441,29 +419,6 @@ dns_adb_dump(dns_adb_t *adb, FILE *f);
  *\li  f != NULL, and is a file open for writing.
  */
 
-isc_result_t
-dns_adb_marklame(dns_adb_t *adb, dns_adbaddrinfo_t *addr,
-                const dns_name_t *qname, dns_rdatatype_t type,
-                isc_stdtime_t expire_time);
-/*%<
- * Mark the given address as lame for the <qname,qtype>.  expire_time should
- * be set to the time when the entry should expire.  That is, if it is to
- * expire 10 minutes in the future, it should set it to (now + 10 * 60).
- *
- * Requires:
- *
- *\li  adb be valid.
- *
- *\li  addr be valid.
- *
- *\li  qname be the qname used in the dns_adb_createfind() call.
- *
- * Returns:
- *
- *\li  #ISC_R_SUCCESS          -- all is well.
- *\li  #ISC_R_NOMEMORY         -- could not mark address as lame.
- */
-
 /*
  * Reasonable defaults for RTT adjustments
  *
index bd467d52690a61195c1f8a544f6b37730246a149..2c91606b16ae6d5a0376f1be4c25edd46ede3fd7 100644 (file)
@@ -3393,8 +3393,6 @@ findname(fetchctx_t *fctx, const dns_name_t *name, in_port_t port,
                        *overquota = true;
                }
                fctx->quotacount++; /* quota exceeded */
-       } else if ((find->options & DNS_ADBFIND_LAMEPRUNED) != 0) {
-               fctx->lamecount++; /* cached lame server */
        } else {
                fctx->adberr++; /* unreachable server, etc. */
        }
@@ -9829,17 +9827,6 @@ rctx_lameserver(respctx_t *rctx) {
 
        inc_stats(fctx->res, dns_resstatscounter_lame);
        log_lame(fctx, query->addrinfo);
-       if (fctx->res->lame_ttl != 0) {
-               result = dns_adb_marklame(fctx->adb, query->addrinfo,
-                                         fctx->name, fctx->type,
-                                         rctx->now + fctx->res->lame_ttl);
-               if (result != ISC_R_SUCCESS) {
-                       isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER,
-                                     DNS_LOGMODULE_RESOLVER, ISC_LOG_ERROR,
-                                     "could not mark server as lame: %s",
-                                     isc_result_totext(result));
-               }
-       }
        rctx->broken_server = DNS_R_LAME;
        rctx->next_server = true;
        FCTXTRACE("lame server");
index eb023ec967d18cb664047a2a3c78218129aaf1c3..b9bea598ef095be1dc6c454f04518293e4fdc962 100644 (file)
@@ -12051,8 +12051,7 @@ notify_find_address(dns_notify_t *notify) {
        dns_adb_t *adb = NULL;
 
        REQUIRE(DNS_NOTIFY_VALID(notify));
-       options = DNS_ADBFIND_WANTEVENT | DNS_ADBFIND_INET | DNS_ADBFIND_INET6 |
-                 DNS_ADBFIND_RETURNLAME;
+       options = DNS_ADBFIND_WANTEVENT | DNS_ADBFIND_INET | DNS_ADBFIND_INET6;
 
        dns_view_getadb(notify->zone->view, &adb);
        if (adb == NULL) {
@@ -20491,8 +20490,7 @@ checkds_find_address(dns_checkds_t *checkds) {
        dns_adb_t *adb = NULL;
 
        REQUIRE(DNS_CHECKDS_VALID(checkds));
-       options = DNS_ADBFIND_WANTEVENT | DNS_ADBFIND_INET | DNS_ADBFIND_INET6 |
-                 DNS_ADBFIND_RETURNLAME;
+       options = DNS_ADBFIND_WANTEVENT | DNS_ADBFIND_INET | DNS_ADBFIND_INET6;
 
        dns_view_getadb(checkds->zone->view, &adb);
        if (adb == NULL) {