From b574c5baeb94e3a176c63982f507a5b48bf3917b Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sun, 27 Oct 2019 18:00:05 +0000 Subject: [PATCH 1/1] Refactor the res_search() fail hack --- src/src/dns.c | 65 ++++++++++++++++++++++++------------------------ test/stderr/0002 | 1 + test/stderr/0044 | 2 ++ test/stderr/0094 | 2 ++ test/stderr/0183 | 7 ++++++ test/stderr/0361 | 3 +++ test/stderr/0381 | 2 ++ test/stderr/0419 | 2 ++ test/stderr/0545 | 2 ++ test/stderr/1006 | 4 +++ test/stderr/2201 | 1 + test/stderr/4802 | 2 ++ test/stderr/4803 | 2 ++ 13 files changed, 62 insertions(+), 33 deletions(-) diff --git a/src/src/dns.c b/src/src/dns.c index 8d9ec4708..28bc5958d 100644 --- a/src/src/dns.c +++ b/src/src/dns.c @@ -681,17 +681,6 @@ 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. -*/ - -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 @@ -703,12 +692,38 @@ 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 +static void +fake_dnsa_len_for_fail(dns_answer * dnsa) +{ +const HEADER * h = (const HEADER *)dnsa->answer; + +if ( h->qr == 1 /* a response */ + && h->opcode == QUERY + && h->tc == 0 /* nmessage not truncated */ && (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); + && ( ntohs(h->qdcount) == 1 /* one question record */ + || f.running_in_test_harness) + && ntohs(h->ancount) == 0 /* no answer records */ + && ntohs(h->nscount) >= 1) /* authority records */ + { + DEBUG(D_dns) debug_printf("faking res_search() response length as %d\n", + (int)sizeof(dnsa->answer)); + dnsa->answerlen = sizeof(dnsa->answer); + } +} + + +/* 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. +*/ + +time_t +dns_expire_from_soa(dns_answer * dnsa) +{ +dns_scan dnss; + +fake_dnsa_len_for_fail(dnsa); for (dns_record * rr = dns_next_rr(dnsa, &dnss, RESET_AUTHORITY); rr; rr = dns_next_rr(dnsa, &dnss, RESET_NEXT) @@ -1185,23 +1200,7 @@ switch (type) if (rc == DNS_NOMATCH) { - /* 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. */ - - const HEADER * h = (const HEADER *)dnsa->answer; - if (h->qr == 1 && h->opcode == QUERY && h->tc == 0 - && (h->rcode == NOERROR || h->rcode == NXDOMAIN) - && ntohs(h->qdcount) == 1 && ntohs(h->ancount) == 0 - && ntohs(h->nscount) >= 1) - dnsa->answerlen = sizeof(dnsa->answer); + fake_dnsa_len_for_fail(dnsa); for (rr = dns_next_rr(dnsa, &dnss, RESET_AUTHORITY); rr; rr = dns_next_rr(dnsa, &dnss, RESET_NEXT) diff --git a/test/stderr/0002 b/test/stderr/0002 index 8f85e7146..3fbb40d2b 100644 --- a/test/stderr/0002 +++ b/test/stderr/0002 @@ -298,6 +298,7 @@ looking up host name for V4NET.0.0.1 DNS lookup of 1.0.0.V4NET.in-addr.arpa (PTR) using fakens DNS lookup of 1.0.0.V4NET.in-addr.arpa (PTR) succeeded IP address lookup yielded "ten-1.test.ex" +faking res_search() response length as 65535 DNS lookup of ten-1.test.ex (A) using fakens DNS lookup of ten-1.test.ex (A) succeeded ten-1.test.ex V4NET.0.0.1 mx=-1 sort=xx diff --git a/test/stderr/0044 b/test/stderr/0044 index d2891b87a..1dcde9f8e 100644 --- a/test/stderr/0044 +++ b/test/stderr/0044 @@ -240,7 +240,9 @@ new DNS lookup for 99.99.99.V4NET.rbl.test.ex DNS lookup of 99.99.99.V4NET.rbl.test.ex (A) using fakens DNS lookup of 99.99.99.V4NET.rbl.test.ex (A) gave HOST_NOT_FOUND returning DNS_NOMATCH +faking res_search() response length as 65535 writing neg-cache entry for 99.99.99.V4NET.rbl.test.ex-A-xxxx, ttl 3000 +faking res_search() response length as 65535 dnslists: wrote cache entry, ttl=3000 DNS lookup for 99.99.99.V4NET.rbl.test.ex failed => that means V4NET.99.99.99 is not listed at rbl.test.ex diff --git a/test/stderr/0094 b/test/stderr/0094 index 26cf91605..d464d0a50 100644 --- a/test/stderr/0094 +++ b/test/stderr/0094 @@ -83,12 +83,14 @@ DNS lookup of 90.99.99.V4NET.in-addr.arpa (PTR) using fakens DNS lookup of 90.99.99.V4NET.in-addr.arpa (PTR) succeeded IP address lookup yielded "oneback.test.ex" alias "host1.masq.test.ex" +faking res_search() response length as 65535 DNS lookup of oneback.test.ex (A) using fakens DNS lookup of oneback.test.ex (A) succeeded oneback.test.ex V4NET.99.99.90 mx=-1 sort=xx checking addresses for oneback.test.ex Forward DNS security status: unverified V4NET.99.99.90 OK +faking res_search() response length as 65535 DNS lookup of host1.masq.test.ex (A) using fakens DNS lookup of host1.masq.test.ex (A) succeeded host1.masq.test.ex V4NET.90.90.90 mx=-1 sort=xx diff --git a/test/stderr/0183 b/test/stderr/0183 index 8b3450b78..46d536a11 100644 --- a/test/stderr/0183 +++ b/test/stderr/0183 @@ -92,6 +92,7 @@ ten-1.test.ex in "*"? yes (matched "*") DNS lookup of ten-1.test.ex (MX) using fakens DNS lookup of ten-1.test.ex (MX) gave NO_DATA returning DNS_NODATA +faking res_search() response length as 65535 writing neg-cache entry for ten-1.test.ex-MX-xxxx, ttl 3000 ten-1.test.ex (MX resp) DNSSEC DNS lookup of ten-1.test.ex (A) using fakens @@ -300,6 +301,7 @@ ten-1.test.ex in "*"? yes (matched "*") DNS lookup of ten-1.test.ex (MX) using fakens DNS lookup of ten-1.test.ex (MX) gave NO_DATA returning DNS_NODATA +faking res_search() response length as 65535 writing neg-cache entry for ten-1.test.ex-MX-xxxx, ttl 3000 ten-1.test.ex (MX resp) DNSSEC DNS lookup of ten-1.test.ex (A) using fakens @@ -450,6 +452,7 @@ nonexist.test.ex in "*"? yes (matched "*") DNS lookup of nonexist.test.ex (MX) using fakens DNS lookup of nonexist.test.ex (MX) gave HOST_NOT_FOUND returning DNS_NOMATCH +faking res_search() response length as 65535 writing neg-cache entry for nonexist.test.ex-MX-xxxx, ttl 3000 lookuphost router declined for userx@nonexist.test.ex "more" is false: skipping remaining routers @@ -509,6 +512,7 @@ ten-1.test.ex in "*"? yes (matched "*") DNS lookup of ten-1.test.ex (MX) using fakens DNS lookup of ten-1.test.ex (MX) gave NO_DATA returning DNS_NODATA +faking res_search() response length as 65535 writing neg-cache entry for ten-1.test.ex-MX-xxxx, ttl 3000 ten-1.test.ex (MX resp) DNSSEC DNS lookup of ten-1.test.ex (A) using fakens @@ -553,6 +557,7 @@ nonexist.test.ex in "*"? yes (matched "*") DNS lookup of nonexist.test.ex (A) using fakens DNS lookup of nonexist.test.ex (A) gave HOST_NOT_FOUND returning DNS_NOMATCH +faking res_search() response length as 65535 writing neg-cache entry for nonexist.test.ex-A-xxxx, ttl 3000 useryz router: defer for usery@nonexist.test.ex message: lookup of host "nonexist.test.ex" failed in useryz router @@ -724,6 +729,7 @@ nonexist.example.com in "*"? yes (matched "*") DNS lookup of nonexist.example.com (MX) using fakens DNS lookup of nonexist.example.com (MX) gave HOST_NOT_FOUND returning DNS_NOMATCH +faking res_search() response length as 65535 writing neg-cache entry for nonexist.example.com-MX-xxxx, ttl 2 lookuphost router declined for userx@nonexist.example.com "more" is false: skipping remaining routers @@ -767,6 +773,7 @@ DNS lookup of nonexist.example.com-MX: cached value DNS_NOMATCH past valid time DNS lookup of nonexist.example.com (MX) using fakens DNS lookup of nonexist.example.com (MX) gave HOST_NOT_FOUND returning DNS_NOMATCH +faking res_search() response length as 65535 update neg-cache entry for nonexist.example.com-MX-xxxx, ttl 2 delay router declined for userd@nonexist.example.com "more" is false: skipping remaining routers diff --git a/test/stderr/0361 b/test/stderr/0361 index 0e5ef679a..a8634a3f4 100644 --- a/test/stderr/0361 +++ b/test/stderr/0361 @@ -100,14 +100,17 @@ recurse.test.ex in "*"? yes (matched "*") DNS lookup of recurse.test.ex (MX) using fakens DNS lookup of recurse.test.ex (MX) gave HOST_NOT_FOUND returning DNS_NOMATCH +faking res_search() response length as 65535 writing neg-cache entry for recurse.test.ex-MX-xxxx, ttl 3000 r1 router widened recurse.test.ex to recurse.test.ex.test.ex recurse.test.ex.test.ex in "*"? yes (matched "*") DNS lookup of recurse.test.ex.test.ex (MX) using fakens DNS lookup of recurse.test.ex.test.ex (MX) gave NO_DATA returning DNS_NODATA +faking res_search() response length as 65535 writing neg-cache entry for recurse.test.ex.test.ex-MX-xxxx, ttl 3000 recurse.test.ex.test.ex (MX resp) DNSSEC +faking res_search() response length as 65535 DNS lookup of recurse.test.ex.test.ex (A) using fakens DNS lookup of recurse.test.ex.test.ex (A) succeeded fully qualified name = recurse.test.ex.test.ex diff --git a/test/stderr/0381 b/test/stderr/0381 index 42b52d031..9cab0970b 100644 --- a/test/stderr/0381 +++ b/test/stderr/0381 @@ -41,12 +41,14 @@ DNS lookup of 97.99.99.V4NET.in-addr.arpa (PTR) using fakens DNS lookup of 97.99.99.V4NET.in-addr.arpa (PTR) succeeded IP address lookup yielded "x.gov.uk.test.ex" alias "x.co.uk.test.ex" +faking res_search() response length as 65535 DNS lookup of x.gov.uk.test.ex (A) using fakens DNS lookup of x.gov.uk.test.ex (A) succeeded x.gov.uk.test.ex V4NET.99.99.97 mx=-1 sort=xx checking addresses for x.gov.uk.test.ex Forward DNS security status: unverified V4NET.99.99.97 OK +faking res_search() response length as 65535 DNS lookup of x.co.uk.test.ex (A) using fakens DNS lookup of x.co.uk.test.ex (A) succeeded x.co.uk.test.ex V4NET.99.99.97 mx=-1 sort=xx diff --git a/test/stderr/0419 b/test/stderr/0419 index 39c797009..0671390b9 100644 --- a/test/stderr/0419 +++ b/test/stderr/0419 @@ -25,8 +25,10 @@ dnslookup router called for k@mxt13.test.ex mxt13.test.ex in "*"? yes (matched "*") DNS lookup of mxt13.test.ex (MX) using fakens DNS lookup of mxt13.test.ex (MX) succeeded +faking res_search() response length as 65535 DNS lookup of other1.test.ex (A) using fakens DNS lookup of other1.test.ex (A) succeeded +faking res_search() response length as 65535 DNS lookup of other2.test.ex (A) using fakens DNS lookup of other2.test.ex (A) succeeded other1.test.ex in "!mxt13.test.ex : !other1.test.ex : *.test.ex"? no (matched "!other1.test.ex") diff --git a/test/stderr/0545 b/test/stderr/0545 index 93cdfc62d..a17553ace 100644 --- a/test/stderr/0545 +++ b/test/stderr/0545 @@ -25,6 +25,7 @@ CNAME found: change to eximtesthost.test.ex DNS lookup of eximtesthost.test.ex (MX) using fakens DNS lookup of eximtesthost.test.ex (MX) gave NO_DATA returning DNS_NODATA +faking res_search() response length as 65535 writing neg-cache entry for eximtesthost.test.ex-MX-xxxx, ttl 3000 alias-eximtesthost (MX resp) DNSSEC DNS lookup of alias-eximtesthost (A) using fakens @@ -99,6 +100,7 @@ CNAME found: change to eximtesthost.test.ex DNS lookup of eximtesthost.test.ex (MX) using fakens DNS lookup of eximtesthost.test.ex (MX) gave NO_DATA returning DNS_NODATA +faking res_search() response length as 65535 writing neg-cache entry for eximtesthost.test.ex-MX-xxxx, ttl 3000 alias-eximtesthost.test.ex (MX resp) DNSSEC DNS lookup of alias-eximtesthost.test.ex (A) using fakens diff --git a/test/stderr/1006 b/test/stderr/1006 index d569343d0..c5dec1804 100644 --- a/test/stderr/1006 +++ b/test/stderr/1006 @@ -13,11 +13,13 @@ DNS lookup of 46.test.ex (A) succeeded DNS lookup of v6.test.ex (MX) using fakens DNS lookup of v6.test.ex (MX) gave NO_DATA returning DNS_NODATA +faking res_search() response length as 65535 writing neg-cache entry for v6.test.ex-MX-xxxx, ttl 3000 DNS lookup of v6.test.ex (AAAA) succeeded DNS lookup of v6.test.ex (A) using fakens DNS lookup of v6.test.ex (A) gave NO_DATA returning DNS_NODATA +faking res_search() response length as 65535 writing neg-cache entry for v6.test.ex-A-xxxx, ttl 3000 >>>>>>>>>>>>>>>> Exim pid=pppp (main) terminating with rc=0 >>>>>>>>>>>>>>>> Exim version x.yz .... @@ -33,10 +35,12 @@ DNS lookup of 46.test.ex (A) succeeded DNS lookup of v6.test.ex (MX) using fakens DNS lookup of v6.test.ex (MX) gave NO_DATA returning DNS_NODATA +faking res_search() response length as 65535 writing neg-cache entry for v6.test.ex-MX-xxxx, ttl 3000 DNS lookup of v6.test.ex (A) using fakens DNS lookup of v6.test.ex (A) gave NO_DATA returning DNS_NODATA +faking res_search() response length as 65535 writing neg-cache entry for v6.test.ex-A-xxxx, ttl 3000 >>>>>>>>>>>>>>>> Exim pid=pppp (main) terminating with rc=2 >>>>>>>>>>>>>>>> diff --git a/test/stderr/2201 b/test/stderr/2201 index 23ac93746..051836904 100644 --- a/test/stderr/2201 +++ b/test/stderr/2201 @@ -99,6 +99,7 @@ dnsdb key: unknown DNS lookup of unknown (TXT) using fakens DNS lookup of unknown (TXT) gave HOST_NOT_FOUND returning DNS_NOMATCH +faking res_search() response length as 65535 writing neg-cache entry for unknown-TXT-xxxx, ttl 3000 lookup failed unknown in "dnsdb;unknown"? no (end of list) diff --git a/test/stderr/4802 b/test/stderr/4802 index 9575f881f..a2510cc3a 100644 --- a/test/stderr/4802 +++ b/test/stderr/4802 @@ -4,6 +4,7 @@ admin user dropping to exim gid; retaining priv uid DNS lookup of mx-sec-a-aa.test.ex (MX) using fakens DNS lookup of mx-sec-a-aa.test.ex (MX) succeeded +faking res_search() response length as 65535 DNS lookup of a-aa.test.ex (A) using fakens DNS lookup of a-aa.test.ex (A) succeeded DNS lookup of a-aa.test.ex (A/AAAA) requested AD, but got AA @@ -15,6 +16,7 @@ dropping to exim gid; retaining priv uid DNS lookup of mx-aa-a-sec.test.ex (MX) using fakens DNS lookup of mx-aa-a-sec.test.ex (MX) succeeded DNS lookup of mx-aa-a-sec.test.ex (MX) requested AD, but got AA +faking res_search() response length as 65535 DNS lookup of a-sec.test.ex (A) using fakens DNS lookup of a-sec.test.ex (A) succeeded >>>>>>>>>>>>>>>> Exim pid=pppp (main) terminating with rc=0 >>>>>>>>>>>>>>>> diff --git a/test/stderr/4803 b/test/stderr/4803 index a7aa7bfa8..e86035acd 100644 --- a/test/stderr/4803 +++ b/test/stderr/4803 @@ -4,6 +4,7 @@ admin user dropping to exim gid; retaining priv uid DNS lookup of mx-sec-a-aa.test.ex (MX) using fakens DNS lookup of mx-sec-a-aa.test.ex (MX) succeeded +faking res_search() response length as 65535 DNS lookup of a-aa.test.ex (A) using fakens DNS lookup of a-aa.test.ex (A) succeeded DNS faked the AD bit (got AA and matched with dns_trust_aa (test.ex in *)) @@ -19,6 +20,7 @@ DNS lookup of mx-aa-a-sec.test.ex (MX) succeeded DNS faked the AD bit (got AA and matched with dns_trust_aa (test.ex in *)) DNS faked the AD bit (got AA and matched with dns_trust_aa (test.ex in *)) DNS faked the AD bit (got AA and matched with dns_trust_aa (test.ex in *)) +faking res_search() response length as 65535 DNS lookup of a-sec.test.ex (A) using fakens DNS lookup of a-sec.test.ex (A) succeeded >>>>>>>>>>>>>>>> Exim pid=pppp (main) terminating with rc=0 >>>>>>>>>>>>>>>> -- 2.30.2