Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 19 Jan 2017 18:11: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 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.


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.