Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <962bf128-5b13-eec5-935b-3bf49780639a@adtran.com>
Date: Fri, 20 Jan 2017 16:23:58 +0000
From: KARL BIELEFELDT <KARL.BIELEFELDT@...ran.com>
To: "musl@...ts.openwall.com" <musl@...ts.openwall.com>
CC: OMADA_TEAM <OMADA_TEAM@...ran.com>
Subject: Re: [PATCH 1/2] Fix name_from_dns_search to handle errors
 properly.

On 01/19/2017 07:02 PM, Rich Felker wrote:
> 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[0] < 4 || (abuf[0][3] & 15) == 2) return EAI_AGAIN;
>>>> -	if ((abuf[0][3] & 15) == 0) return EAI_NONAME;
>>>> -	if ((abuf[0][3] & 15) == 3) return 0;
>>>> +	if ((abuf[0][3] & 15) == 3) return EAI_NONAME;
>>>> +	if ((abuf[0][3] & 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
>
That makes sense.  I appreciate you taking the time to explain it.

Karl

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.