From d8d9f9301c9a31c826635bbdd334bb4be99ea05a Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Thu, 7 Jun 2018 18:08:22 +0100 Subject: [PATCH] Follow CNAME chains only one step. Bug 2264 --- doc/doc-docbook/spec.xfpt | 14 ++++++++++++++ doc/doc-txt/ChangeLog | 7 +++++++ doc/doc-txt/OptionLists.txt | 1 + src/README.UPDATING | 8 ++++++++ src/src/dns.c | 19 +++++++++++++------ src/src/globals.c | 1 + src/src/globals.h | 1 + src/src/readconf.c | 1 + src/src/verify.c | 5 ++--- test/aux-var-src/tls_conf_prefix | 1 + test/stdout/0572 | 2 ++ test/stdout/0577 | 1 + 12 files changed, 52 insertions(+), 9 deletions(-) diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt index c4b3837da..431d4560c 100644 --- a/doc/doc-docbook/spec.xfpt +++ b/doc/doc-docbook/spec.xfpt @@ -13873,6 +13873,7 @@ listed in more than one group. .row &%av_scanner%& "specify virus scanner" .row &%check_rfc2047_length%& "check length of RFC 2047 &""encoded &&& words""&" +.row &%dns_cname_loops%& "follow CNAMEs returned by resolver" .row &%dns_csa_search_limit%& "control CSA parent search depth" .row &%dns_csa_use_reverse%& "en/disable CSA IP reverse search" .row &%header_maxsize%& "total size of message header" @@ -14775,6 +14776,19 @@ This option controls whether or not an IP address, given as a CSA domain, is reversed and looked up in the reverse DNS, as described in more detail in section &<>&. +.new +.option dns_cname_loops main integer 1 +.cindex DNS "CNAME following" +This option controls the following of CNAME chains, needed if the resolver does +not do it internally. +As of 2018 most should, and the default can be left. +If you have an ancient one, a value of 10 is likely needed. + +The default value of one CNAME-follow is needed +thanks to the observed return for an MX request, +given no MX presence but a CNAME to an A, of the CNAME. +.wen + .option dns_dnssec_ok main integer -1 .cindex "DNS" "resolver options" diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 36f2d7048..6b36763fe 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -52,6 +52,13 @@ JH/09 Bug 2274: Fix logging of cmdline args when starting in an unlinked cwd. JH/10 Fix ARC signing for case when DKIM signing failed. Previously this would segfault. +JH/11 Bug 2264: Exim now only follows CNAME chains one step by default. We'd + like zero, since the resolver should be doing this for us, But we need one + as a CNAME but no MX presence gets the CNAME returned; we need to check + that doesn't point to an MX to declare it "no MX returned" rather than + "error, loop". A new main option is added so the older capability of + following some limited number of chain links is maintained. + Exim version 4.91 ----------------- diff --git a/doc/doc-txt/OptionLists.txt b/doc/doc-txt/OptionLists.txt index dfb0219cb..91ce182fd 100644 --- a/doc/doc-txt/OptionLists.txt +++ b/doc/doc-txt/OptionLists.txt @@ -182,6 +182,7 @@ dmarc_history_file string unset main dmarc_tld_file string unset main 4.82 if experimental_dmarc dns_again_means_nonexist domain list unset main 1.89 dns_check_names_pattern string + main 2.11 +dns_cname_loops integer 0 main 4.92 Set to 9 for older behaviour dns_csa_search_limit integer 5 main 4.60 dns_csa_use_reverse boolean true main 4.60 dns_dnssec_ok integer -1 main 4.82 diff --git a/src/README.UPDATING b/src/README.UPDATING index 73b52e4a0..2438cc953 100644 --- a/src/README.UPDATING +++ b/src/README.UPDATING @@ -26,6 +26,14 @@ The rest of this document contains information about changes in 4.xx releases that might affect a running system. +Exim version 4.92 +----------------- + + * Exim used to manually follow CNAME chains, to a limited depth. In this + day-and-age we expect the resolver to be doing this for us, so the loop + is limited to one retry unless the (new) config option dns_cname_loops + is changed. + Exim version 4.91 ----------------- diff --git a/src/src/dns.c b/src/src/dns.c index 83de7c266..cb1766618 100644 --- a/src/src/dns.c +++ b/src/src/dns.c @@ -674,10 +674,10 @@ 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" : "??"); + 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; } @@ -836,6 +836,8 @@ return DNS_SUCCEED; /* Look up the given domain name, using the given type. Follow CNAMEs if necessary, but only so many times. There aren't supposed to be CNAME chains in the DNS, but you are supposed to cope with them if you find them. +By default, follow one CNAME since a resolver has been seen, faced with +an MX request and a CNAME (to an A) but no MX present, returning the CNAME. The assumption is made that if the resolver gives back records of the requested type *and* a CNAME, we don't need to make another call to look up @@ -871,9 +873,14 @@ int i; const uschar *orig_name = name; BOOL secure_so_far = TRUE; -/* Loop to follow CNAME chains so far, but no further... */ +/* By default, assume the resolver follows CNAME chains (and returns NODATA for +an unterminated one). If it also does that for a CNAME loop, fine; if it returns +a CNAME (maybe the last?) whine about it. However, retain the coding for dumb +resolvers hiding behind a config variable. Loop to follow CNAME chains so far, +but no further... The testsuite tests the latter case, mostly assuming that the +former will work. */ -for (i = 0; i < 10; i++) +for (i = 0; i <= dns_cname_loops; i++) { uschar * data; dns_record *rr, cname_rr, type_rr; diff --git a/src/src/globals.c b/src/src/globals.c index ec948151f..138a29e8a 100644 --- a/src/src/globals.c +++ b/src/src/globals.c @@ -698,6 +698,7 @@ BOOL dmarc_enable_forensic = FALSE; uschar *dns_again_means_nonexist = NULL; int dns_csa_search_limit = 5; BOOL dns_csa_use_reverse = TRUE; +int dns_cname_loops = 1; #ifdef SUPPORT_DANE int dns_dane_ok = -1; #endif diff --git a/src/src/globals.h b/src/src/globals.h index 9c7d8ccd9..f9be8b832 100644 --- a/src/src/globals.h +++ b/src/src/globals.h @@ -421,6 +421,7 @@ extern BOOL dmarc_enable_forensic; /* Set via ACL control statement. When se extern uschar *dns_again_means_nonexist; /* Domains that are badly set up */ extern int dns_csa_search_limit; /* How deep to search for CSA SRV records */ extern BOOL dns_csa_use_reverse; /* Check CSA in reverse DNS? (non-standard) */ +extern int dns_cname_loops; /* Follow CNAMEs returned by resolver to this depth */ extern uschar *dns_ipv4_lookup; /* For these domains, don't look for AAAA (or A6) */ #ifdef SUPPORT_DANE extern int dns_dane_ok; /* Ok to use DANE when checking TLS authenticity */ diff --git a/src/src/readconf.c b/src/src/readconf.c index 919fbc215..3f307fd5c 100644 --- a/src/src/readconf.c +++ b/src/src/readconf.c @@ -123,6 +123,7 @@ static optionlist optionlist_config[] = { #endif { "dns_again_means_nonexist", opt_stringptr, &dns_again_means_nonexist }, { "dns_check_names_pattern", opt_stringptr, &check_dns_names_pattern }, + { "dns_cname_loops", opt_int, &dns_cname_loops }, { "dns_csa_search_limit", opt_int, &dns_csa_search_limit }, { "dns_csa_use_reverse", opt_bool, &dns_csa_use_reverse }, { "dns_dnssec_ok", opt_int, &dns_dnssec_ok }, diff --git a/src/src/verify.c b/src/src/verify.c index 95876d1cd..5d0551e89 100644 --- a/src/src/verify.c +++ b/src/src/verify.c @@ -3411,9 +3411,8 @@ else /* If the lookup succeeded, cache the RHS address. The code allows for more than one address - this was for complete generality and the possible - use of A6 records. However, A6 records have been reduced to experimental - status (August 2001) and may die out. So they may never get used at all, - let alone in dnsbl records. However, leave the code here, just in case. + use of A6 records. However, A6 records are no longer supported. Leave the code + here, just in case. Quite apart from one A6 RR generating multiple addresses, there are DNS lists that return more than one A record, so we must handle multiple diff --git a/test/aux-var-src/tls_conf_prefix b/test/aux-var-src/tls_conf_prefix index 20b6fe85f..e357b996d 100644 --- a/test/aux-var-src/tls_conf_prefix +++ b/test/aux-var-src/tls_conf_prefix @@ -11,4 +11,5 @@ log_file_path = DIR/spool/log/%slog gecos_pattern = "" gecos_name = CALLER_NAME +dns_cname_loops = 9 chunking_advertise_hosts = diff --git a/test/stdout/0572 b/test/stdout/0572 index 96ab5611b..272aa06a1 100644 --- a/test/stdout/0572 +++ b/test/stdout/0572 @@ -85,6 +85,7 @@ spool_directory = TESTSUITE/spool log_file_path = TESTSUITE/spool/log/%slog gecos_pattern = "" gecos_name = CALLER_NAME +dns_cname_loops = 9 chunking_advertise_hosts = # 1 "TESTSUITE/aux-var/std_conf_prefix" # 5 "TESTSUITE/test-config" @@ -124,6 +125,7 @@ spool_directory = TESTSUITE/spool log_file_path = TESTSUITE/spool/log/%slog gecos_pattern = "" gecos_name = CALLER_NAME +dns_cname_loops = 9 chunking_advertise_hosts = # 1 "TESTSUITE/aux-var/std_conf_prefix" # 5 "TESTSUITE/test-config" diff --git a/test/stdout/0577 b/test/stdout/0577 index 893f92243..80cf3da88 100644 --- a/test/stdout/0577 +++ b/test/stdout/0577 @@ -11,6 +11,7 @@ spool_directory = TESTSUITE/spool log_file_path = TESTSUITE/spool/log/%slog gecos_pattern = "" gecos_name = CALLER_NAME +dns_cname_loops = 9 chunking_advertise_hosts = # 1 "TESTSUITE/aux-var/std_conf_prefix" tls_advertise_hosts = -- 2.30.2