From 2546388c27720eaaada4bb63574ba1f32e6ddf4e Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sat, 11 Feb 2017 18:20:41 +0000 Subject: [PATCH] DNS: return explicit error code to caller on dnssec failure, for better logging --- src/src/host.c | 101 ++++++++++++++---------- src/src/macros.h | 4 +- src/src/routers/dnslookup.c | 5 ++ src/src/routers/rf_lookup_hostlist.c | 6 ++ src/src/transports/smtp.c | 10 ++- src/src/verify.c | 2 +- test/scripts/4800-dnssec-dnslookup/4801 | 2 +- test/stderr/0368 | 4 +- test/stderr/0419 | 2 +- test/stderr/0499 | 4 +- test/stderr/4801 | 6 -- test/stdout/4801 | 6 +- 12 files changed, 90 insertions(+), 62 deletions(-) delete mode 100644 test/stderr/4801 diff --git a/src/src/host.c b/src/src/host.c index b3b8b1882..b5af8f92b 100644 --- a/src/src/host.c +++ b/src/src/host.c @@ -1831,7 +1831,7 @@ the names, and accepts only those that have the correct IP address. */ save_hostname = sender_host_name; /* Save for error messages */ aliases = sender_host_aliases; -for (hname = sender_host_name; hname != NULL; hname = *aliases++) +for (hname = sender_host_name; hname; hname = *aliases++) { int rc; BOOL ok = FALSE; @@ -1859,7 +1859,7 @@ for (hname = sender_host_name; hname != NULL; hname = *aliases++) h.dnssec == DS_YES ? "DNSSEC verified (AD)" : "unverified"); if (h.dnssec != DS_YES) sender_host_dnssec = FALSE; - for (hh = &h; hh != NULL; hh = hh->next) + for (hh = &h; hh; hh = hh->next) if (host_is_in_net(hh->address, sender_host_address, 0)) { HDEBUG(D_host_lookup) debug_printf(" %s OK\n", hh->address); @@ -2273,6 +2273,7 @@ Arguments: Returns: HOST_FIND_FAILED couldn't find A record HOST_FIND_AGAIN try again later + HOST_FIND_SECURITY dnssec required but not acheived HOST_FOUND found AAAA and/or A record(s) HOST_IGNORED found, but all IPs ignored */ @@ -2286,6 +2287,7 @@ set_address_from_dns(host_item *host, host_item **lastptr, dns_record *rr; host_item *thishostlast = NULL; /* Indicates not yet filled in anything */ BOOL v6_find_again = FALSE; +BOOL dnssec_fail = FALSE; int i; /* If allow_ip is set, a name which is an IP address returns that value @@ -2381,8 +2383,8 @@ for (; i >= 0; i--) { if (dnssec_require) { - log_write(L_host_lookup_failed, LOG_MAIN, - "dnssec fail on %s for %.256s", + dnssec_fail = TRUE; + DEBUG(D_host_lookup) debug_printf("dnssec fail on %s for %.256s", i>0 ? "AAAA" : "A", host->name); continue; } @@ -2504,10 +2506,14 @@ for (; i >= 0; i--) } } -/* Control gets here only if the econdookup (the A record) succeeded. +/* Control gets here only if the second lookup (the A record) succeeded. However, the address may not be filled in if it was ignored. */ -return host->address ? HOST_FOUND : HOST_IGNORED; +return host->address + ? HOST_FOUND + : dnssec_fail + ? HOST_FIND_SECURITY + : HOST_IGNORED; } @@ -2546,6 +2552,7 @@ Returns: HOST_FIND_FAILED Failed to find the host or domain; if there was a syntax error, host_find_failed_syntax is set. HOST_FIND_AGAIN Could not resolve at this time + HOST_FIND_SECURITY dnsssec required but not acheived HOST_FOUND Host found HOST_FOUND_LOCAL The lowest MX record points to this machine, if MX records were found, or @@ -2652,7 +2659,7 @@ same domain. The result will be DNS_NODATA if the domain exists but has no MX records. On DNS failures, we give the "try again" error unless the domain is listed as one for which we continue. */ -if (rc != DNS_SUCCEED && (whichrrs & HOST_FIND_BY_MX) != 0) +if (rc != DNS_SUCCEED && whichrrs & HOST_FIND_BY_MX) { ind_type = T_MX; dnssec = DS_UNK; @@ -2660,13 +2667,12 @@ if (rc != DNS_SUCCEED && (whichrrs & HOST_FIND_BY_MX) != 0) rc = dns_lookup_timerwrap(&dnsa, host->name, ind_type, fully_qualified_name); DEBUG(D_dns) - if ((dnssec_request || dnssec_require) - & !dns_is_secure(&dnsa) - & dns_is_aa(&dnsa)) + if ( (dnssec_request || dnssec_require) + && !dns_is_secure(&dnsa) + && dns_is_aa(&dnsa)) debug_printf("DNS lookup of %.256s (MX) requested AD, but got AA\n", host->name); if (dnssec_request) - { if (dns_is_secure(&dnsa)) { DEBUG(D_host_lookup) debug_printf("%s MX DNSSEC\n", host->name); @@ -2676,7 +2682,6 @@ if (rc != DNS_SUCCEED && (whichrrs & HOST_FIND_BY_MX) != 0) { dnssec = DS_NO; lookup_dnssec_authenticated = US"no"; } - } switch (rc) { @@ -2686,17 +2691,22 @@ if (rc != DNS_SUCCEED && (whichrrs & HOST_FIND_BY_MX) != 0) case DNS_SUCCEED: if (!dnssec_require || dns_is_secure(&dnsa)) break; - log_write(L_host_lookup_failed, LOG_MAIN, - "dnssec fail on MX for %.256s", host->name); + DEBUG(D_host_lookup) + debug_printf("dnssec fail on MX for %.256s", host->name); +#ifndef STAND_ALONE + if (match_isinlist(host->name, CUSS &mx_fail_domains, 0, NULL, NULL, + MCL_DOMAIN, TRUE, NULL) != OK) + { yield = HOST_FIND_SECURITY; goto out; } +#endif rc = DNS_FAIL; /*FALLTHROUGH*/ case DNS_FAIL: case DNS_AGAIN: - #ifndef STAND_ALONE +#ifndef STAND_ALONE if (match_isinlist(host->name, CUSS &mx_fail_domains, 0, NULL, NULL, MCL_DOMAIN, TRUE, NULL) != OK) - #endif +#endif { yield = HOST_FIND_AGAIN; goto out; } DEBUG(D_host_lookup) debug_printf("DNS_%s treated as DNS_NODATA " "(domain in mx_fail_domains)\n", (rc == DNS_FAIL)? "FAIL":"AGAIN"); @@ -3062,13 +3072,17 @@ for (h = host; h != last->next; h = h->next) if (rc != HOST_FOUND) { h->status = hstatus_unusable; - if (rc == HOST_FIND_AGAIN) + switch (rc) { - yield = rc; - h->why = hwhy_deferred; + case HOST_FIND_AGAIN: + yield = rc; h->why = hwhy_deferred; break; + case HOST_FIND_SECURITY: + yield = rc; h->why = hwhy_insecure; break; + case HOST_IGNORED: + h->why = hwhy_ignored; break; + default: + h->why = hwhy_failed; break; } - else - h->why = rc == HOST_IGNORED ? hwhy_ignored : hwhy_failed; } } @@ -3077,7 +3091,7 @@ been explicitly ignored, and remove them from the list, as if they did not exist. If we end up with just a single, ignored host, flatten its fields as if nothing was found. */ -if (ignore_target_hosts != NULL) +if (ignore_target_hosts) { host_item *prev = NULL; for (h = host; h != last->next; h = h->next) @@ -3113,24 +3127,22 @@ single MX preference value, IPv6 addresses come first. This can separate the addresses of a multihomed host, but that should not matter. */ #if HAVE_IPV6 -if (h != last && !disable_ipv6) +if (h != last && !disable_ipv6) for (h = host; h != last; h = h->next) { - for (h = host; h != last; h = h->next) - { - host_item temp; - host_item *next = h->next; - if (h->mx != next->mx || /* If next is different MX */ - h->address == NULL || /* OR this one is unset */ - Ustrchr(h->address, ':') != NULL || /* OR this one is IPv6 */ - (next->address != NULL && - Ustrchr(next->address, ':') == NULL)) /* OR next is IPv4 */ - continue; /* move on to next */ - temp = *h; /* otherwise, swap */ - temp.next = next->next; - *h = *next; - h->next = next; - *next = temp; - } + host_item temp; + host_item *next = h->next; + + if (h->mx != next->mx || /* If next is different MX */ + h->address == NULL || /* OR this one is unset */ + Ustrchr(h->address, ':') != NULL || /* OR this one is IPv6 */ + (next->address != NULL && + Ustrchr(next->address, ':') == NULL)) /* OR next is IPv4 */ + continue; /* move on to next */ + temp = *h; /* otherwise, swap */ + temp.next = next->next; + *h = *next; + h->next = next; + *next = temp; } #endif @@ -3154,6 +3166,7 @@ DEBUG(D_host_lookup) debug_printf("host_find_bydns yield = %s (%d); returned hosts:\n", (yield == HOST_FOUND)? "HOST_FOUND" : (yield == HOST_FOUND_LOCAL)? "HOST_FOUND_LOCAL" : + (yield == HOST_FIND_SECURITY)? "HOST_FIND_SECURITY" : (yield == HOST_FIND_AGAIN)? "HOST_FIND_AGAIN" : (yield == HOST_FIND_FAILED)? "HOST_FIND_FAILED" : "?", yield); @@ -3285,9 +3298,13 @@ while (Ufgets(buffer, 256, stdin) != NULL) : host_find_bydns(&h, NULL, flags, US"smtp", NULL, NULL, &d, &fully_qualified_name, NULL); - if (rc == HOST_FIND_FAILED) printf("Failed\n"); - else if (rc == HOST_FIND_AGAIN) printf("Again\n"); - else if (rc == HOST_FOUND_LOCAL) printf("Local\n"); + switch (rc) + { + case HOST_FIND_FAILED: printf("Failed\n"); break; + case HOST_FIND_AGAIN: printf("Again\n"); break; + case HOST_FIND_SECURITY: printf("Security\n"); break; + case HOST_FOUND_LOCAL: printf("Local\n"); break; + } } printf("\n> "); diff --git a/src/src/macros.h b/src/src/macros.h index 2692714f7..e4dcf3ccd 100644 --- a/src/src/macros.h +++ b/src/src/macros.h @@ -744,7 +744,8 @@ enum { hstatus_unknown, hstatus_usable, hstatus_unusable, /* Reasons why a host is unusable (for clearer log messages) */ -enum { hwhy_unknown, hwhy_retry, hwhy_failed, hwhy_deferred, hwhy_ignored }; +enum { hwhy_unknown, hwhy_retry, hwhy_insecure, hwhy_failed, hwhy_deferred, + hwhy_ignored }; /* Domain lookup types for routers */ @@ -809,6 +810,7 @@ enum { SCH_NONE, SCH_AUTH, SCH_DATA, SCH_BDAT, enum { HOST_FIND_FAILED, /* failed to find the host */ HOST_FIND_AGAIN, /* could not resolve at this time */ + HOST_FIND_SECURITY, /* dnssec required but not acheived */ HOST_FOUND, /* found host */ HOST_FOUND_LOCAL, /* found, but MX points to local host */ HOST_IGNORED /* found but ignored - used internally only */ diff --git a/src/src/routers/dnslookup.c b/src/src/routers/dnslookup.c index d2be40e0f..e4f7a2539 100644 --- a/src/src/routers/dnslookup.c +++ b/src/src/routers/dnslookup.c @@ -291,6 +291,11 @@ for (;;) /* Deferral returns forthwith, and anything other than failure breaks the loop. */ + if (rc == HOST_FIND_SECURITY) + { + addr->message = US"host lookup done insecurely"; + return DEFER; + } if (rc == HOST_FIND_AGAIN) { if (rblock->pass_on_timeout) diff --git a/src/src/routers/rf_lookup_hostlist.c b/src/src/routers/rf_lookup_hostlist.c index 78eda22fb..c826857a7 100644 --- a/src/src/routers/rf_lookup_hostlist.c +++ b/src/src/routers/rf_lookup_hostlist.c @@ -146,6 +146,12 @@ for (prev = NULL, h = addr->host_list; h; h = next_h) /* Temporary failure defers, unless pass_on_timeout is set */ + if (rc == HOST_FIND_SECURITY) + { + addr->message = string_sprintf("host lookup for %s done insecurely" , h->name); + addr->basic_errno = ERRNO_DNSDEFER; + return DEFER; + } if (rc == HOST_FIND_AGAIN) { if (rblock->pass_on_timeout) diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c index cbd09c00c..02d1e671e 100644 --- a/src/src/transports/smtp.c +++ b/src/src/transports/smtp.c @@ -3718,7 +3718,7 @@ for (cutoff_retry = 0; commonly points to a configuration error, but the best action is still to carry on for the next host. */ - if (rc == HOST_FIND_AGAIN || rc == HOST_FIND_FAILED) + if (rc == HOST_FIND_AGAIN || rc == HOST_FIND_SECURITY || rc == HOST_FIND_FAILED) { retry_add_item(addrlist, string_sprintf("R:%s", host->name), 0); expired = FALSE; @@ -3731,8 +3731,11 @@ for (cutoff_retry = 0; { if (addr->transport_return != DEFER) continue; addr->basic_errno = ERRNO_UNKNOWNHOST; - addr->message = - string_sprintf("failed to lookup IP address for %s", host->name); + addr->message = string_sprintf( + rc == HOST_FIND_SECURITY + ? "lookup of IP address for %s was insecure" + : "failed to lookup IP address for %s", + host->name); } continue; } @@ -3871,6 +3874,7 @@ for (cutoff_retry = 0; { case hwhy_retry: hosts_retry++; break; case hwhy_failed: hosts_fail++; break; + case hwhy_insecure: case hwhy_deferred: hosts_defer++; break; } diff --git a/src/src/verify.c b/src/src/verify.c index a152a8f24..9ff1807d4 100644 --- a/src/src/verify.c +++ b/src/src/verify.c @@ -1803,7 +1803,7 @@ while (addr_new) dnssec_domains = &ob->dnssec; } - (void)host_find_bydns(host, NULL, flags, NULL, NULL, NULL, + (void) host_find_bydns(host, NULL, flags, NULL, NULL, NULL, dnssec_domains, NULL, NULL); } } diff --git a/test/scripts/4800-dnssec-dnslookup/4801 b/test/scripts/4800-dnssec-dnslookup/4801 index 30ba44ed0..491080e0d 100644 --- a/test/scripts/4800-dnssec-dnslookup/4801 +++ b/test/scripts/4800-dnssec-dnslookup/4801 @@ -5,7 +5,7 @@ exim -bt user@mx-unsec-a-unsec.test.ex 1 exim -bt user@mx-unsec-a-sec.test.ex **** -2 +1 exim -bt user@mx-sec-a-unsec.test.ex **** exim -bt user@mx-sec-a-sec.test.ex diff --git a/test/stderr/0368 b/test/stderr/0368 index a02088556..cdeaf2cb7 100644 --- a/test/stderr/0368 +++ b/test/stderr/0368 @@ -3,14 +3,14 @@ configuration file is TESTSUITE/test-config admin user discarded duplicate host ten-1.test.ex (MX=8) fully qualified name = mxt9.test.ex -host_find_bydns yield = HOST_FOUND (2); returned hosts: +host_find_bydns yield = HOST_FOUND (3); returned hosts: ten-1.test.ex V4NET.0.0.1 MX=5 ten-2.test.ex V4NET.0.0.2 MX=6 ten-3.test.ex V4NET.0.0.3 MX=7 duplicate IP address V4NET.0.0.5 (MX=5) removed duplicate IP address V4NET.0.0.6 (MX=6) removed fully qualified name = mxt14.test.ex -host_find_bydns yield = HOST_FOUND (2); returned hosts: +host_find_bydns yield = HOST_FOUND (3); returned hosts: ten-5-6.test.ex V4NET.0.0.5 MX=4 ten-5-6.test.ex V4NET.0.0.6 MX=4 finding IP address for ten-1.test.ex diff --git a/test/stderr/0419 b/test/stderr/0419 index 5fc95009c..37bab88f2 100644 --- a/test/stderr/0419 +++ b/test/stderr/0419 @@ -37,7 +37,7 @@ local host in host list - removed hosts: other2.test.ex V4NET.12.3.2 5 other2.test.ex V4NET.12.3.1 5 fully qualified name = mxt13.test.ex -host_find_bydns yield = HOST_FOUND (2); returned hosts: +host_find_bydns yield = HOST_FOUND (3); returned hosts: other1.test.ex V4NET.12.4.5 MX=4 set transport smtp queued for smtp transport: local_part = k diff --git a/test/stderr/0499 b/test/stderr/0499 index 2ecd699aa..6435e8fc1 100644 --- a/test/stderr/0499 +++ b/test/stderr/0499 @@ -21,7 +21,7 @@ DNS lookup of mxt1.test.ex (MX) succeeded DNS lookup of eximtesthost.test.ex (A) using fakens DNS lookup of eximtesthost.test.ex (A) succeeded local host has lowest MX -host_find_bydns yield = HOST_FOUND_LOCAL (3); returned hosts: +host_find_bydns yield = HOST_FOUND_LOCAL (4); returned hosts: eximtesthost.test.ex ip4.ip4.ip4.ip4 MX=5 mxt1.test.ex in "@mx_any"? yes (matched "@mx_any") mxt1.test.ex in "+anymx"? yes (matched "+anymx") @@ -31,7 +31,7 @@ DNS lookup of mxt1.test.ex (MX) succeeded DNS lookup of eximtesthost.test.ex (A) using fakens DNS lookup of eximtesthost.test.ex (A) succeeded local host has lowest MX -host_find_bydns yield = HOST_FOUND_LOCAL (3); returned hosts: +host_find_bydns yield = HOST_FOUND_LOCAL (4); returned hosts: eximtesthost.test.ex ip4.ip4.ip4.ip4 MX=5 mxt1.test.ex in "@mx_any"? yes (matched "@mx_any") mxt1.test.ex in "+anymx"? yes (matched "+anymx") diff --git a/test/stderr/4801 b/test/stderr/4801 deleted file mode 100644 index 4a9d58975..000000000 --- a/test/stderr/4801 +++ /dev/null @@ -1,6 +0,0 @@ -LOG: host_lookup_failed MAIN - dnssec fail on MX for mx-unsec-a-unsec.test.ex -LOG: host_lookup_failed MAIN - dnssec fail on MX for mx-unsec-a-sec.test.ex -LOG: host_lookup_failed MAIN - dnssec fail on A for a-unsec.test.ex diff --git a/test/stdout/4801 b/test/stdout/4801 index fe6d359a2..25d83a182 100644 --- a/test/stdout/4801 +++ b/test/stdout/4801 @@ -1,6 +1,6 @@ -user@mx-unsec-a-unsec.test.ex cannot be resolved at this time: host lookup did not complete -user@mx-unsec-a-sec.test.ex cannot be resolved at this time: host lookup did not complete -user@mx-sec-a-unsec.test.ex is undeliverable: all relevant MX records point to non-existent hosts +user@mx-unsec-a-unsec.test.ex cannot be resolved at this time: host lookup done insecurely +user@mx-unsec-a-sec.test.ex cannot be resolved at this time: host lookup done insecurely +user@mx-sec-a-unsec.test.ex cannot be resolved at this time: host lookup done insecurely user@mx-sec-a-sec.test.ex router = dnslookup, transport = smtp host a-sec.test.ex [V4NET.0.0.100] MX=5 AD -- 2.25.1