Remove dns_adblameinfo from dns_adb

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.
This commit is contained in:
Ondřej Surý 2023-09-21 15:20:58 +02:00
parent 7fb4e354a9
commit 2b20db05e3
No known key found for this signature in database
GPG key ID: 2820F37E873DEA41
4 changed files with 8 additions and 243 deletions

View 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) {

View file

@ -43,11 +43,6 @@
* 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
*

View 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");

View 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) {