Date: Thu, 19 Jan 2017 20:02:10 -0500 From: Rich Felker <dalias@...c.org> To: musl@...ts.openwall.com Subject: Re: [PATCH 1/2] Fix name_from_dns_search to handle errors properly. On Fri, Jan 20, 2017 at 12:13:36AM +0000, KARL BIELEFELDT wrote: > On 01/19/2017 05:11 PM, Rich Felker wrote: > > On Thu, Jan 19, 2017 at 09:49:01PM +0000, KARL BIELEFELDT wrote: > >> This function previously exited after the first search failure > >> due to an inverted test condition, and incorrect testing of > >> return codes in name_from_dns. This commit corrects those > >> self-cancelling errors that were only evident when another type > >> of error such as a network timeout occurred. > >> --- > >> src/network/lookup_name.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/src/network/lookup_name.c b/src/network/lookup_name.c > >> index fb7303a3..bfa76d38 100644 > >> --- a/src/network/lookup_name.c > >> +++ b/src/network/lookup_name.c > >> @@ -164,8 +164,8 @@ static int name_from_dns(struct address buf[static MAXADDRS], char canon[static > >> > >> if (ctx.cnt) return ctx.cnt; > >> if (alens < 4 || (abuf & 15) == 2) return EAI_AGAIN; > >> - if ((abuf & 15) == 0) return EAI_NONAME; > >> - if ((abuf & 15) == 3) return 0; > >> + if ((abuf & 15) == 3) return EAI_NONAME; > >> + if ((abuf & 15) == 0) return 0; > >> return EAI_FAIL; > >> } > >> > >> @@ -201,7 +201,7 @@ static int name_from_dns_search(struct address buf[static MAXADDRS], char canon[ > >> memcpy(canon+l+1, p, z-p); > >> canon[z-p+1+l] = 0; > >> int cnt = name_from_dns(buf, canon, canon, family, &conf); > >> - if (cnt) return cnt; > >> + if (!cnt) return cnt; > >> } > >> } > > I think you're mistaken here. The intent throughout __lookup_name is > > that, when one backend returns 0 (no results), the lookup process > > continues to the next backend. Conversely, if a backend returns a > > positive number of results or an error, lookup must stop and report > > what was found. If this principle is not followed then transient > > errors are not reported to the caller but instead _silently_ change > > the result of the lookup. > > > > In the case of dns lookups, a response code of 0 from the nameserver > > with 0 records is a result of "this dns label exists, but doesn't have > > a record of the requested type". Usually that happens when only A was > > available but you requested AAAA, or vice versa. In that case we must > > return EAI_NONAME as the result rather than continuing the search. > > Otherwise, requests for AF_UNSPEC will return results from different > > sources than what you would get from at least one of AF_INET or > > AF_INET6. > > > > On the other hand, a result of NxDomain is not an error; it simply > > means the requested dns label does not exist at all. In that case, > > it's valid to continue the search. > > > OK, I follow that. The problem we're seeing though is when one of the > entries > on the "search" line of /etc/resolv.conf causes a network timeout error, > it never > gets down to the last line of the function, which as far as I can tell > tests the absolute > name without any search list elements appended, and would have returned > a successful > result. This is the only way to get consistent results; otherwise, which result you get changes depending on whether you experience a transient error on the search element. Note that this generally only happens with ndots>1. With the default, ndots=1, only queries with no dots in them (which are generally not valid as standalone dns lookups, although sometimes now they are with the ridiculous new tlds) are subject to search. Use of ndots>1 has lots of other issues like making dns lookups twice as slow. Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.