From 7d8d08c484958a90f5d5744894b9bc2f723bee4e Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sun, 1 Sep 2019 19:44:31 +0100 Subject: Support TTL from SOA for NXDOMAIN & NODATA cache entries. Bug 1395 --- src/src/dns.c | 173 ++++++++++++++++++++++++++++++++++++++++++++---------- src/src/os.c | 2 +- src/src/search.c | 6 +- src/src/structs.h | 6 +- 4 files changed, 152 insertions(+), 35 deletions(-) (limited to 'src') diff --git a/src/src/dns.c b/src/src/dns.c index e3845978c..b309207cf 100644 --- a/src/src/dns.c +++ b/src/src/dns.c @@ -435,6 +435,7 @@ dnss->aptr += dnss->srr.size; /* Advance to next RR */ /* Return a pointer to the dns_record structure within the dns_answer. This is for convenience so that the scans can use nice-looking for loops. */ +TRACE debug_printf("%s: return %s\n", __FUNCTION__, dns_text_type(dnss->srr.type)); return &dnss->srr; null_return: @@ -593,6 +594,10 @@ static void dns_fail_tag(uschar * buf, const uschar * name, int dns_type) { res_state resp = os_get_dns_resolver_res(); + +/*XX buf needs to be 255 +1 + (max(typetext) == 5) +1 + max(chars_for_long-max) +1 +We truncate the name here for safety... could use a dynamic string. */ + sprintf(CS buf, "%.255s-%s-%lx", name, dns_text_type(dns_type), (unsigned long) resp->options); } @@ -606,21 +611,140 @@ caching. Arguments: name the domain name type the lookup type + expiry time TTL expires, or zero for unlimited rc the return code Returns: the return code */ +/*XXX the derivation of this value needs explaining */ +#define DNS_FAILTAG_MAX 290 + +static int +dns_fail_return(const uschar * name, int type, time_t expiry, int rc) +{ +uschar node_name[DNS_FAILTAG_MAX]; +tree_node * previous, * new; +expiring_data * e; + +dns_fail_tag(node_name, name, type); +if ((previous = tree_search(tree_dns_fails, node_name))) + e = previous->data.ptr; +else + { + new = store_get_perm( + sizeof(tree_node) + DNS_FAILTAG_MAX + sizeof(expiring_data), is_tainted(name)); + + dns_fail_tag(new->name, name, type); + e = (expiring_data *)((char *)new + sizeof(tree_node) + DNS_FAILTAG_MAX); + new->data.ptr = e; + (void)tree_insertnode(&tree_dns_fails, new); + } + +DEBUG(D_dns) debug_printf(" %s neg-cache entry for %s, ttl %d\n", + previous ? "update" : "writing", + node_name, expiry ? (int)(expiry - time(NULL)) : -1); +e->expiry = expiry; +e->data.val = rc; +return rc; +} + + +/* Return the cached result of a known-bad lookup, or -1. +*/ static int -dns_return(const uschar * name, int type, int rc) +dns_fail_cache_hit(const uschar * name, int type) { -tree_node *node = store_get_perm(sizeof(tree_node) + 290, TRUE); -dns_fail_tag(node->name, name, type); -node->data.val = rc; -(void)tree_insertnode(&tree_dns_fails, node); +uschar node_name[DNS_FAILTAG_MAX]; +tree_node * previous; +expiring_data * e; +int val, rc; + +dns_fail_tag(node_name, name, type); +if (!(previous = tree_search(tree_dns_fails, node_name))) + return -1; + +e = previous->data.ptr; +val = e->data.val; +rc = e->expiry && e->expiry <= time(NULL) ? -1 : val; + +DEBUG(D_dns) debug_printf("DNS lookup of %.255s-%s: %scached value %s%s\n", + name, dns_text_type(type), + rc == -1 ? "" : "using ", + val == DNS_NOMATCH ? "DNS_NOMATCH" : + val == DNS_NODATA ? "DNS_NODATA" : + val == DNS_AGAIN ? "DNS_AGAIN" : + val == DNS_FAIL ? "DNS_FAIL" : "??", + rc == -1 ? " past valid time" : ""); + return rc; } + + +/* Return the TTL suitable for an NXDOMAIN result, which is given +in the SOA. We hope that one was returned in the lookup, and do not +bother doing a separate lookup; if not found return a forever TTL. +*/ + +static time_t +dns_expire_from_soa(dns_answer * dnsa) +{ +const HEADER * h = (const HEADER *)dnsa->answer; +dns_scan dnss; + +/* This is really gross. The successful return value from res_search() is +the packet length, which is stored in dnsa->answerlen. If we get a +negative DNS reply then res_search() returns -1, which causes the bounds +checks for name decompression to fail when it is treated as a packet +length, which in turn causes the authority search to fail. The correct +packet length has been lost inside libresolv, so we have to guess a +replacement value. (The only way to fix this properly would be to +re-implement res_search() and res_query() so that they don't muddle their +success and packet length return values.) For added safety we only reset +the packet length if the packet header looks plausible. */ + +if ( h->qr == 1 && h->opcode == QUERY && h->tc == 0 + && (h->rcode == NOERROR || h->rcode == NXDOMAIN) + && (ntohs(h->qdcount) == 1 || f.running_in_test_harness) + && ntohs(h->ancount) == 0 + && ntohs(h->nscount) >= 1) + dnsa->answerlen = sizeof(dnsa->answer); + +for (dns_record * rr = dns_next_rr(dnsa, &dnss, RESET_AUTHORITY); + rr; rr = dns_next_rr(dnsa, &dnss, RESET_NEXT) + ) if (rr->type == T_SOA) + { + const uschar * p = rr->data; + uschar discard_buf[256]; + int len; + unsigned long ttl; + + /* Skip the mname & rname strings */ + + if ((len = dn_expand(dnsa->answer, dnsa->answer + dnsa->answerlen, + p, (DN_EXPAND_ARG4_TYPE)discard_buf, 256)) < 0) + break; + p += len; + if ((len = dn_expand(dnsa->answer, dnsa->answer + dnsa->answerlen, + p, (DN_EXPAND_ARG4_TYPE)discard_buf, 256)) < 0) + break; + p += len; + + /* Skip the SOA serial, refresh, retry & expire. Grab the TTL */ + + if (p > dnsa->answer + dnsa->answerlen - 5 * NS_INT32SZ) + break; + p += 4 * NS_INT32SZ; + GETLONG(ttl, p); + + return time(NULL) + ttl; + } +DEBUG(D_dns) debug_printf("DNS: no SOA record found for neg-TTL\n"); +return 0; +} + + /************************************************* * Do basic DNS lookup * *************************************************/ @@ -651,32 +775,21 @@ Returns: DNS_SUCCEED successful lookup */ int -dns_basic_lookup(dns_answer *dnsa, const uschar *name, int type) +dns_basic_lookup(dns_answer * dnsa, const uschar * name, int type) { +int rc; #ifndef STAND_ALONE -int rc = -1; -const uschar *save_domain; +const uschar * save_domain; #endif -tree_node *previous; -uschar node_name[290]; - /* DNS lookup failures of any kind are cached in a tree. This is mainly so that a timeout on one domain doesn't happen time and time again for messages that have many addresses in the same domain. We rely on the resolver and name server -caching for successful lookups. */ +caching for successful lookups. +*/ -dns_fail_tag(node_name, name, type); -if ((previous = tree_search(tree_dns_fails, node_name))) - { - DEBUG(D_dns) debug_printf("DNS lookup of %.255s-%s: using cached value %s\n", - name, dns_text_type(type), - previous->data.val == DNS_NOMATCH ? "DNS_NOMATCH" : - previous->data.val == DNS_NODATA ? "DNS_NODATA" : - previous->data.val == DNS_AGAIN ? "DNS_AGAIN" : - previous->data.val == DNS_FAIL ? "DNS_FAIL" : "??"); - return previous->data.val; - } +if ((rc = dns_fail_cache_hit(name, type)) > 0) + return rc; #ifdef SUPPORT_I18N /* Convert all names to a-label form before doing lookup */ @@ -779,7 +892,7 @@ if (dnsa->answerlen < 0) switch (h_errno) case HOST_NOT_FOUND: DEBUG(D_dns) debug_printf("DNS lookup of %s (%s) gave HOST_NOT_FOUND\n" "returning DNS_NOMATCH\n", name, dns_text_type(type)); - return dns_return(name, type, DNS_NOMATCH); + return dns_fail_return(name, type, dns_expire_from_soa(dnsa), DNS_NOMATCH); case TRY_AGAIN: DEBUG(D_dns) debug_printf("DNS lookup of %s (%s) gave TRY_AGAIN\n", @@ -795,30 +908,30 @@ if (dnsa->answerlen < 0) switch (h_errno) if (rc != OK) { DEBUG(D_dns) debug_printf("returning DNS_AGAIN\n"); - return dns_return(name, type, DNS_AGAIN); + return dns_fail_return(name, type, 0, DNS_AGAIN); } DEBUG(D_dns) debug_printf("%s is in dns_again_means_nonexist: returning " "DNS_NOMATCH\n", name); - return dns_return(name, type, DNS_NOMATCH); + return dns_fail_return(name, type, dns_expire_from_soa(dnsa), DNS_NOMATCH); #else /* For stand-alone tests */ - return dns_return(name, type, DNS_AGAIN); + return dns_fail_return(name, type, 0, DNS_AGAIN); #endif case NO_RECOVERY: DEBUG(D_dns) debug_printf("DNS lookup of %s (%s) gave NO_RECOVERY\n" "returning DNS_FAIL\n", name, dns_text_type(type)); - return dns_return(name, type, DNS_FAIL); + return dns_fail_return(name, type, 0, DNS_FAIL); case NO_DATA: DEBUG(D_dns) debug_printf("DNS lookup of %s (%s) gave NO_DATA\n" "returning DNS_NODATA\n", name, dns_text_type(type)); - return dns_return(name, type, DNS_NODATA); + return dns_fail_return(name, type, dns_expire_from_soa(dnsa), DNS_NODATA); default: DEBUG(D_dns) debug_printf("DNS lookup of %s (%s) gave unknown DNS error %d\n" "returning DNS_FAIL\n", name, dns_text_type(type), h_errno); - return dns_return(name, type, DNS_FAIL); + return dns_fail_return(name, type, 0, DNS_FAIL); } DEBUG(D_dns) debug_printf("DNS lookup of %s (%s) succeeded\n", diff --git a/src/src/os.c b/src/src/os.c index c44bd3e99..ae9c6043c 100644 --- a/src/src/os.c +++ b/src/src/os.c @@ -831,7 +831,7 @@ return type. res_state os_get_dns_resolver_res(void) { - return &_res; +return &_res; } #endif /* OS_GET_DNS_RESOLVER_RES */ diff --git a/src/src/search.c b/src/src/search.c index df10f773a..9d1a10a5a 100644 --- a/src/src/search.c +++ b/src/src/search.c @@ -492,7 +492,7 @@ if ( (t = tree_search(c->item_cache, keystring)) && (!(e = t->data.ptr)->expiry || e->expiry > time(NULL)) ) { /* Data was in the cache already; set the pointer from the tree node */ - data = e->ptr; + data = e->data.ptr; DEBUG(D_lookup) debug_printf_indent("cached data used for lookup of %s%s%s\n", keystring, filename ? US"\n in " : US"", filename ? filename : US""); @@ -532,13 +532,13 @@ else if (t) /* Previous, out-of-date cache entry. Update with the */ { /* new result and forget the old one */ e->expiry = do_cache == UINT_MAX ? 0 : time(NULL)+do_cache; - e->ptr = data; + e->data.ptr = data; } else { e = store_get(sizeof(expiring_data) + sizeof(tree_node) + len, is_tainted(keystring)); e->expiry = do_cache == UINT_MAX ? 0 : time(NULL)+do_cache; - e->ptr = data; + e->data.ptr = data; t = (tree_node *)(e+1); memcpy(t->name, keystring, len); t->data.ptr = e; diff --git a/src/src/structs.h b/src/src/structs.h index bc6edc586..338dccbf1 100644 --- a/src/src/structs.h +++ b/src/src/structs.h @@ -728,7 +728,11 @@ for the lookups cache */ typedef struct expiring_data { time_t expiry; /* if nonzero, data invalid after this time */ - void *ptr; /* pointer to data */ + union + { + void *ptr; /* pointer to data */ + int val; /* or integer data */ + } data; } expiring_data; /* Structure for holding the handle and the cached last lookup for searches. -- cgit v1.2.3