From 1349e1e5bcfa5fb3db8aa2f02825b7e70bf47cdb Mon Sep 17 00:00:00 2001 From: Philip Hazel Date: Tue, 7 Feb 2006 16:36:25 +0000 Subject: [PATCH] Always do explicit A/AAAA lookups; no longer make use of the additional RR section from MX/SRV lookups. --- doc/doc-txt/ChangeLog | 12 ++- src/src/host.c | 214 +++++++----------------------------------- test/stderr/0419 | 4 + test/stderr/0499 | 5 + test/stderr/1006 | 5 + test/stdout/0430 | 4 +- 6 files changed, 61 insertions(+), 183 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index e22757f38..d914efb43 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -1,4 +1,4 @@ -$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.285 2006/02/07 14:20:58 ph10 Exp $ +$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.286 2006/02/07 16:36:25 ph10 Exp $ Change log file for Exim from version 4.21 ------------------------------------------- @@ -89,6 +89,16 @@ PH/14 When a uid/gid is specified for the queryprogram router, it cannot be PH/15 Minor change to Makefile for building test_host (undocumented testing feature). +PH/16 As discussed on the list in Nov/Dec: Exim no longer looks at the + additional section of a DNS packet that returns MX or SRV records. + Instead, it always explicitly searches for A/AAAA records. This avoids + major problems that occur when a DNS server includes only records of one + type (A or AAAA) in an MX/SRV packet. A byproduct of this change has + fixed another bug: if SRV records were looked up and the corresponding + address records were *not* found in the additional section, the port + values from the SRV records were lost. + + Exim version 4.60 ----------------- diff --git a/src/src/host.c b/src/src/host.c index 317d257bb..87cd7c494 100644 --- a/src/src/host.c +++ b/src/src/host.c @@ -1,4 +1,4 @@ -/* $Cambridge: exim/src/src/host.c,v 1.20 2006/02/07 11:19:00 ph10 Exp $ */ +/* $Cambridge: exim/src/src/host.c,v 1.21 2006/02/07 16:36:25 ph10 Exp $ */ /************************************************* * Exim - an Internet mail transport agent * @@ -2192,10 +2192,10 @@ return yield; * Fill in a host address from the DNS * *************************************************/ -/* Given a host item, with its name and mx fields set, and its address field -set to NULL, fill in its IP address from the DNS. If it is multi-homed, create -additional host items for the additional addresses, copying all the other -fields, and randomizing the order. +/* Given a host item, with its name, port and mx fields set, and its address +field set to NULL, fill in its IP address from the DNS. If it is multi-homed, +create additional host items for the additional addresses, copying all the +other fields, and randomizing the order. On IPv6 systems, A6 records are sought first (but only if support for A6 is configured - they may never become mainstream), then AAAA records are sought, @@ -2250,7 +2250,6 @@ if (allow_ip && string_is_ip_address(host->name, NULL) != 0) #endif host->address = host->name; - host->port = PORT_NONE; return HOST_FOUND; } @@ -2360,7 +2359,6 @@ for (; i >= 0; i--) if (strcmpic(host->name, rr->name) != 0) host->name = string_copy_dnsdomain(rr->name); host->address = da->address; - host->port = PORT_NONE; host->sort_key = host->mx * 1000 + random_number(500) + randoffset; host->status = hstatus_unknown; host->why = hwhy_unknown; @@ -2395,10 +2393,9 @@ for (; i >= 0; i--) if (new_sort_key < host->sort_key) { - *next = *host; + *next = *host; /* Copies port */ host->next = next; host->address = da->address; - host->port = PORT_NONE; host->sort_key = new_sort_key; if (thishostlast == host) thishostlast = next; /* Local last */ if (*lastptr == host) *lastptr = next; /* Global last */ @@ -2415,10 +2412,9 @@ for (; i >= 0; i--) if (new_sort_key < h->next->sort_key) break; h = h->next; } - *next = *h; + *next = *h; /* Copies port */ h->next = next; next->address = da->address; - next->port = PORT_NONE; next->sort_key = new_sort_key; if (h == thishostlast) thishostlast = next; /* Local last */ if (h == *lastptr) *lastptr = next; /* Global last */ @@ -2439,15 +2435,15 @@ return (host->address == NULL)? HOST_IGNORED : HOST_FOUND; /************************************************* -* Find IP addresses and names for host via DNS * +* Find IP addresses and host names via DNS * *************************************************/ -/* The input is a host_item structure with the name filled in and the address -field set to NULL. This may be in a chain of other host items. The lookup may -result in more than one IP address, in which case we must created new host -blocks for the additional addresses, and insert them into the chain. The -original name may not be fully qualified. Use the fully_qualified_name argument -to return the official name, as returned by the resolver. +/* The input is a host_item structure with the name field filled in and the +address field set to NULL. This may be in a chain of other host items. The +lookup may result in more than one IP address, in which case we must created +new host blocks for the additional addresses, and insert them into the chain. +The original name may not be fully qualified. Use the fully_qualified_name +argument to return the official name, as returned by the resolver. Arguments: host point to initial host item @@ -2638,7 +2634,7 @@ for (rr = dns_next_rr(&dnsa, &dnss, RESET_ANSWERS); { int precedence; int weight = 0; /* For SRV records */ - int port = PORT_NONE; /* For SRV records */ + int port = PORT_NONE; uschar *s; /* MUST be unsigned for GETSHORT */ uschar data[256]; @@ -2879,175 +2875,33 @@ if (ind_type == T_SRV) } /* Move on to the next host */ } -/* Now we have to ensure addresses exist for all the hosts. We have ensured -above that the names in the host items are all unique. The addresses may have -been returned in the additional data section of the DNS query. Because it is -more expensive to scan the returned DNS records (because you have to expand the -names) we do a single scan over them, and multiple scans of the chain of host -items (which is typically only 3 or 4 long anyway.) Add extra host items for -multi-homed hosts. */ - -for (rr = dns_next_rr(&dnsa, &dnss, RESET_ADDITIONAL); - rr != NULL; - rr = dns_next_rr(&dnsa, &dnss, RESET_NEXT)) - { - dns_address *da; - int status = hstatus_unknown; - int why = hwhy_unknown; - int randoffset; - - if (rr->type != T_A - #if HAVE_IPV6 - && ( disable_ipv6 || - ( - rr->type != T_AAAA - #ifdef SUPPORT_A6 - && rr->type != T_A6 - #endif - ) - ) - #endif - ) continue; - - /* Find the first host that matches this record's name. If there isn't - one, move on to the next RR. */ - - for (h = host; h != last->next; h = h->next) - { if (strcmpic(h->name, rr->name) == 0) break; } - if (h == last->next) continue; - - /* For IPv4 addresses, add 500 to the random part of the sort key, to ensure - they sort after IPv6 addresses. */ - - randoffset = (rr->type == T_A)? 500 : 0; - - /* Get the list of textual addresses for this RR. There may be more than one - if it is an A6 RR. Then loop to handle multiple addresses from an A6 record. - If there are none, nothing will get done - the record is ignored. */ - - for (da = dns_address_from_rr(&dnsa, rr); da != NULL; da = da->next) - { - /* Set status for an ignorable host. */ - - #ifndef STAND_ALONE - if (ignore_target_hosts != NULL && - verify_check_this_host(&ignore_target_hosts, NULL, h->name, - da->address, NULL) == OK) - { - DEBUG(D_host_lookup) - debug_printf("ignored host %s [%s]\n", h->name, da->address); - status = hstatus_unusable; - why = hwhy_ignored; - } - #endif - - /* If the address is already set for this host, it may be that - we just have a duplicate DNS record. Alternatively, this may be - a multi-homed host. Search all items with the same host name - (they will all be together) and if this address is found, skip - to the next RR. */ - - if (h->address != NULL) - { - int new_sort_key; - host_item *thishostlast; - host_item *hh = h; - - do - { - if (hh->address != NULL && Ustrcmp(CS da->address, hh->address) == 0) - goto DNS_NEXT_RR; /* Need goto to escape from inner loop */ - thishostlast = hh; - hh = hh->next; - } - while (hh != last->next && strcmpic(hh->name, rr->name) == 0); - - /* We have a multi-homed host, since we have a new address for - an existing name. Create a copy of the current item, and give it - the new address. RRs can be in arbitrary order, but one is supposed - to randomize the addresses of multi-homed hosts, so compute a new - sorting key and do that. [Latest SMTP RFC says not to randomize multi- - homed hosts, but to rely on the resolver. I'm not happy about that - - caching in the resolver will not rotate as often as the name server - does.] */ - - new_sort_key = h->mx * 1000 + random_number(500) + randoffset; - hh = store_get(sizeof(host_item)); - - /* New address goes first: insert the new block after the first one - (so as not to disturb the original pointer) but put the new address - in the original block. */ - - if (new_sort_key < h->sort_key) - { - *hh = *h; /* Note: copies the port */ - h->next = hh; - h->address = da->address; - h->sort_key = new_sort_key; - h->status = status; - h->why = why; - } - - /* Otherwise scan down the addresses for this host to find the - one to insert after. */ - - else - { - while (h != thishostlast) - { - if (new_sort_key < h->next->sort_key) break; - h = h->next; - } - *hh = *h; /* Note: copies the port */ - h->next = hh; - hh->address = da->address; - hh->sort_key = new_sort_key; - hh->status = status; - hh->why = why; - } - - if (h == last) last = hh; /* Inserted after last */ - } - - /* The existing item doesn't have its address set yet, so just set it. - Ensure that an IPv4 address gets its sort key incremented in case an IPv6 - address is found later. */ - - else - { - h->address = da->address; /* Port should be set already */ - h->status = status; - h->why = why; - h->sort_key += randoffset; - } - } /* Loop for addresses extracted from one RR */ - - /* Carry on to the next RR. It would be nice to be able to be able to stop - when every host on the list has an address, but we can't be sure there won't - be an additional address for a multi-homed host further down the list, so - we have to continue to the end. */ - - DNS_NEXT_RR: continue; - } - -/* Set the default yield to failure */ - -yield = HOST_FIND_FAILED; - -/* If we haven't found all the addresses in the additional section, we -need to search for A or AAAA records explicitly. The names shouldn't point to -CNAMES, but we use the general lookup function that handles them, just -in case. If any lookup gives a soft error, change the default yield. +/* Now we have to find IP addresses for all the hosts. We have ensured above +that the names in all the host items are unique. Before release 4.61 we used to +process records from the additional section in the DNS packet that returned the +MX or SRV records. However, a DNS name server is free to drop any resource +records from the additional section. In theory, this has always been a +potential problem, but it is exacerbated by the advent of IPv6. If a host had +several IPv4 addresses and some were not in the additional section, at least +Exim would try the others. However, if a host had both IPv4 and IPv6 addresses +and all the IPv4 (say) addresses were absent, Exim would try only for a IPv6 +connection, and never try an IPv4 address. When there was only IPv4 +connectivity, this was a disaster that did in practice occur. + +So, from release 4.61 onwards, we always search for A and AAAA records +explicitly. The names shouldn't point to CNAMES, but we use the general lookup +function that handles them, just in case. If any lookup gives a soft error, +change the default yield. For these DNS lookups, we must disable qualify_single and search_parents; otherwise invalid host names obtained from MX or SRV records can cause trouble if they happen to match something local. */ -dns_init(FALSE, FALSE); +yield = HOST_FIND_FAILED; /* Default yield */ +dns_init(FALSE, FALSE); /* Disable qualify_single and search_parents */ for (h = host; h != last->next; h = h->next) { - if (h->address != NULL || h->status == hstatus_unusable) continue; + if (h->address != NULL) continue; /* Inserted by a multihomed host */ rc = set_address_from_dns(h, &last, ignore_target_hosts, allow_mx_to_ip, NULL); if (rc != HOST_FOUND) { diff --git a/test/stderr/0419 b/test/stderr/0419 index d70278174..e828361e6 100644 --- a/test/stderr/0419 +++ b/test/stderr/0419 @@ -24,6 +24,10 @@ dnslookup router called for k@mxt13.test.ex domain = mxt13.test.ex DNS lookup of mxt13.test.ex (MX) using fakens DNS lookup of mxt13.test.ex (MX) succeeded +DNS lookup of other1.test.ex (A) using fakens +DNS lookup of other1.test.ex (A) succeeded +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") other1.test.ex in hosts_treat_as_local? no (end of list) other2.test.ex in "!mxt13.test.ex : !other1.test.ex : *.test.ex"? yes (matched "*.test.ex") diff --git a/test/stderr/0499 b/test/stderr/0499 index 0fd4df4fe..47a8229c7 100644 --- a/test/stderr/0499 +++ b/test/stderr/0499 @@ -17,6 +17,8 @@ local_part=ph domain=mxt1.test.ex checking domains DNS lookup of mxt1.test.ex (MX) using fakens 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: eximtesthost.test.ex ip4.ip4.ip4.ip4 MX=5 @@ -25,6 +27,9 @@ mxt1.test.ex in "+anymx"? yes (matched "+anymx") checking "condition" DNS lookup of mxt1.test.ex (MX) using fakens DNS lookup of mxt1.test.ex (MX) succeeded +DNS lookup of eximtesthost.test.ex-AAAA: using cached value DNS_NODATA +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: eximtesthost.test.ex ip4.ip4.ip4.ip4 MX=5 diff --git a/test/stderr/1006 b/test/stderr/1006 index bce98fb26..09421f9a8 100644 --- a/test/stderr/1006 +++ b/test/stderr/1006 @@ -6,6 +6,9 @@ DNS lookup of mx46.test.ex (MX) succeeded DNS lookup of 46.test.ex (AAAA) succeeded DNS lookup of 46.test.ex (A) using fakens DNS lookup of 46.test.ex (A) succeeded +DNS lookup of 46.test.ex (AAAA) succeeded +DNS lookup of 46.test.ex (A) using fakens +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 @@ -21,6 +24,8 @@ DNS lookup of mx46.test.ex (MX) using fakens DNS lookup of mx46.test.ex (MX) succeeded DNS lookup of 46.test.ex (A) using fakens DNS lookup of 46.test.ex (A) succeeded +DNS lookup of 46.test.ex (A) using fakens +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 diff --git a/test/stdout/0430 b/test/stdout/0430 index 19713dbd6..d41c39573 100644 --- a/test/stdout/0430 +++ b/test/stdout/0430 @@ -31,10 +31,10 @@ x@random.manual.route host ten-1.test.ex [V4NET.0.0.1] y@random.manual.route router = r2, transport = t1 - host ten-6.test.ex [V4NET.0.0.6] - host ten-1.test.ex [V4NET.0.0.1] host ten-2.test.ex [V4NET.0.0.2] MX=5 host ten-3.test.ex [V4NET.0.0.3] MX=6 + host ten-6.test.ex [V4NET.0.0.6] + host ten-1.test.ex [V4NET.0.0.1] x@random.manual.route router = r2, transport = t1 host ten-6.test.ex [V4NET.0.0.6] -- 2.25.1