getaddrinfo usage with wrong ip family

getaddrinfo usage with wrong ip family

[Julien Ramseier]

[-- Attachment #1: Type: text/plain, Size: 835 bytes --]

Hello,

I discovered a potential problem in getaddrinfo/__lookup_name.
When calling getaddrinfo  with an IP string not matching the specified family,
name_from_numeric() in __lookup_name() will not recognize it and the 
external dns resolver will be used.

So the following code:

const struct addrinfo hints = {
    .ai_flags = AI_ADDRCONFIG,
    .ai_family = AF_INET,
    .ai_socktype = SOCK_STREAM,
};

getaddrinfo("::1", NULL, &hints, &result);

will actually succeed instead of returning EAI_NONAME,
and perform a "A ::1" query.
Some misbehaving dns servers will then answer with 0.0.0.1.

I don’t know if this behavior is desirable. If not, I’m still not sure
where this should be fixed. Maybe should we prevent sending
A and AAAA dns queries with IP as hostname in __res_mkquery() ?

—
Julien


[-- Attachment #2: Type: text/html, Size: 2002 bytes --]

Re: getaddrinfo usage with wrong ip family

[Rich Felker]

On Tue, Sep 22, 2015 at 04:40:30PM +0200, Julien Ramseier wrote:
> Hello,
> 
> I discovered a potential problem in getaddrinfo/__lookup_name.
> When calling getaddrinfo  with an IP string not matching the specified family,
> name_from_numeric() in __lookup_name() will not recognize it and the 
> external dns resolver will be used.
> 
> So the following code:
> 
> const struct addrinfo hints = {
>     .ai_flags = AI_ADDRCONFIG,
>     .ai_family = AF_INET,
>     .ai_socktype = SOCK_STREAM,
> };
> 
> getaddrinfo("::1", NULL, &hints, &result);
> 
> will actually succeed instead of returning EAI_NONAME,
> and perform a "A ::1" query.
> Some misbehaving dns servers will then answer with 0.0.0.1.
> 
> I don’t know if this behavior is desirable. If not, I’m still not sure
> where this should be fixed. Maybe should we prevent sending
> A and AAAA dns queries with IP as hostname in __res_mkquery() ?

Sometime (it's been "soon" for a long time) I intend to add IDN
support, so the same place that goes would be the natural place to
pre-validate strings before sending them off in DNS queries. But I'm
not sure what the right filtering would be.

Another approach might be having __lookup_numeric always parse with
AF_UNSPEC, but return error rather than 0 results if the resulting
family does not match the requested family.

Anyone else have opinions on these ideas?

Rich

Re: getaddrinfo usage with wrong ip family

[Julien Ramseier]

[-- Attachment #1: Type: text/plain, Size: 380 bytes --]


> Le 22 sept. 2015 à 17:16, Rich Felker <dalias@libc.org> a écrit :
> 
> Another approach might be having __lookup_numeric always parse with
> AF_UNSPEC, but return error rather than 0 results if the resulting
> family does not match the requested family.

This seems the simplest solution in the meantime.

Here’s the patch I applied to my trunk.



—
Julien

[-- Attachment #2.1: Type: text/html, Size: 2933 bytes --]

[-- Attachment #2.2: lookup_ipliteral-error-on-incorrect-family.patch --]
[-- Type: application/octet-stream, Size: 2053 bytes --]

diff --git a/src/network/lookup_ipliteral.c b/src/network/lookup_ipliteral.c
index 7bcb85f..209dc55 100644
--- a/src/network/lookup_ipliteral.c
+++ b/src/network/lookup_ipliteral.c
@@ -15,38 +15,43 @@ int __lookup_ipliteral(struct address buf[static 1], const char *name, int famil
 {
 	struct in_addr a4;
 	struct in6_addr a6;
-	if (family != AF_INET6 && __inet_aton(name, &a4)>0) {
+	if (__inet_aton(name, &a4) > 0) {
+		if (family == AF_INET6) /* wrong family */
+			return EAI_NONAME;
 		memcpy(&buf[0].addr, &a4, sizeof a4);
 		buf[0].family = AF_INET;
 		buf[0].scopeid = 0;
 		return 1;
 	}
-	if (family != AF_INET) {
-		char tmp[64];
-		char *p = strchr(name, '%'), *z;
-		unsigned long long scopeid;
-		if (p && p-name < 64) {
-			memcpy(tmp, name, p-name);
-			tmp[p-name] = 0;
-			name = tmp;
-		}
-		if (inet_pton(AF_INET6, name, &a6)<=0) return 0;
-		memcpy(&buf[0].addr, &a6, sizeof a6);
-		buf[0].family = AF_INET6;
-		if (p) {
-			if (isdigit(*++p)) scopeid = strtoull(p, &z, 10);
-			else z = p-1;
-			if (*z) {
-				if (!IN6_IS_ADDR_LINKLOCAL(&a6) &&
-				    !IN6_IS_ADDR_MC_LINKLOCAL(&a6))
-					return EAI_NONAME;
-				scopeid = if_nametoindex(p);
-				if (!scopeid) return EAI_NONAME;
-			}
-			if (scopeid > UINT_MAX) return EAI_NONAME;
-			buf[0].scopeid = scopeid;
+
+	char tmp[64];
+	char *p = strchr(name, '%'), *z;
+	unsigned long long scopeid;
+	if (p && p-name < 64) {
+		memcpy(tmp, name, p-name);
+		tmp[p-name] = 0;
+		name = tmp;
+	}
+
+	if (inet_pton(AF_INET6, name, &a6) <= 0)
+		return 0;
+	if (family == AF_INET) /* wrong family */
+		return EAI_NONAME;
+
+	memcpy(&buf[0].addr, &a6, sizeof a6);
+	buf[0].family = AF_INET6;
+	if (p) {
+		if (isdigit(*++p)) scopeid = strtoull(p, &z, 10);
+		else z = p-1;
+		if (*z) {
+			if (!IN6_IS_ADDR_LINKLOCAL(&a6) &&
+			    !IN6_IS_ADDR_MC_LINKLOCAL(&a6))
+				return EAI_NONAME;
+			scopeid = if_nametoindex(p);
+			if (!scopeid) return EAI_NONAME;
 		}
-		return 1;
+		if (scopeid > UINT_MAX) return EAI_NONAME;
+		buf[0].scopeid = scopeid;
 	}
-	return 0;
+	return 1;
 }

[-- Attachment #2.3: Type: text/html, Size: 292 bytes --]

Re: getaddrinfo usage with wrong ip family

[Szabolcs Nagy]

* Julien Ramseier <j.ramseier@gmail.com> [2015-09-24 12:27:22 +0200]:
> > Le 22 sept. 2015 à 17:16, Rich Felker <dalias@libc.org> a écrit :
> > 
> > Another approach might be having __lookup_numeric always parse with
> > AF_UNSPEC, but return error rather than 0 results if the resulting
> > family does not match the requested family.
> 
> This seems the simplest solution in the meantime.
> 
> Here???s the patch I applied to my trunk.
> 

forgot to attach the patch?

Re: getaddrinfo usage with wrong ip family

[Julien Ramseier]

> Le 24 sept. 2015 à 12:59, Szabolcs Nagy <nsz@port70.net> a écrit :
> 
> * Julien Ramseier <j.ramseier@gmail.com> [2015-09-24 12:27:22 +0200]:
>>> Le 22 sept. 2015 à 17:16, Rich Felker <dalias@libc.org> a écrit :
>>> 
>>> Another approach might be having __lookup_numeric always parse with
>>> AF_UNSPEC, but return error rather than 0 results if the resulting
>>> family does not match the requested family.
>> 
>> This seems the simplest solution in the meantime.
>> 
>> Here???s the patch I applied to my trunk.
>> 
> 
> forgot to attach the patch?

No, but maybe my mail client screwed it up.


---
diff --git a/src/network/lookup_ipliteral.c b/src/network/lookup_ipliteral.c
index 7bcb85f..209dc55 100644
--- a/src/network/lookup_ipliteral.c
+++ b/src/network/lookup_ipliteral.c
@@ -15,38 +15,43 @@ int __lookup_ipliteral(struct address buf[static 1], const char *name, int famil
 {
 	struct in_addr a4;
 	struct in6_addr a6;
-	if (family != AF_INET6 && __inet_aton(name, &a4)>0) {
+	if (__inet_aton(name, &a4) > 0) {
+		if (family == AF_INET6) /* wrong family */
+			return EAI_NONAME;
 		memcpy(&buf[0].addr, &a4, sizeof a4);
 		buf[0].family = AF_INET;
 		buf[0].scopeid = 0;
 		return 1;
 	}
-	if (family != AF_INET) {
-		char tmp[64];
-		char *p = strchr(name, '%'), *z;
-		unsigned long long scopeid;
-		if (p && p-name < 64) {
-			memcpy(tmp, name, p-name);
-			tmp[p-name] = 0;
-			name = tmp;
-		}
-		if (inet_pton(AF_INET6, name, &a6)<=0) return 0;
-		memcpy(&buf[0].addr, &a6, sizeof a6);
-		buf[0].family = AF_INET6;
-		if (p) {
-			if (isdigit(*++p)) scopeid = strtoull(p, &z, 10);
-			else z = p-1;
-			if (*z) {
-				if (!IN6_IS_ADDR_LINKLOCAL(&a6) &&
-				    !IN6_IS_ADDR_MC_LINKLOCAL(&a6))
-					return EAI_NONAME;
-				scopeid = if_nametoindex(p);
-				if (!scopeid) return EAI_NONAME;
-			}
-			if (scopeid > UINT_MAX) return EAI_NONAME;
-			buf[0].scopeid = scopeid;
+
+	char tmp[64];
+	char *p = strchr(name, '%'), *z;
+	unsigned long long scopeid;
+	if (p && p-name < 64) {
+		memcpy(tmp, name, p-name);
+		tmp[p-name] = 0;
+		name = tmp;
+	}
+
+	if (inet_pton(AF_INET6, name, &a6) <= 0)
+		return 0;
+	if (family == AF_INET) /* wrong family */
+		return EAI_NONAME;
+
+	memcpy(&buf[0].addr, &a6, sizeof a6);
+	buf[0].family = AF_INET6;
+	if (p) {
+		if (isdigit(*++p)) scopeid = strtoull(p, &z, 10);
+		else z = p-1;
+		if (*z) {
+			if (!IN6_IS_ADDR_LINKLOCAL(&a6) &&
+			    !IN6_IS_ADDR_MC_LINKLOCAL(&a6))
+				return EAI_NONAME;
+			scopeid = if_nametoindex(p);
+			if (!scopeid) return EAI_NONAME;
 		}
-		return 1;
+		if (scopeid > UINT_MAX) return EAI_NONAME;
+		buf[0].scopeid = scopeid;
 	}
-	return 0;
+	return 1;
 }

Re: getaddrinfo usage with wrong ip family

[Rich Felker]

On Thu, Sep 24, 2015 at 05:11:03PM +0200, Julien Ramseier wrote:
> 
> > Le 24 sept. 2015 à 12:59, Szabolcs Nagy <nsz@port70.net> a écrit :
> > 
> > * Julien Ramseier <j.ramseier@gmail.com> [2015-09-24 12:27:22 +0200]:
> >>> Le 22 sept. 2015 à 17:16, Rich Felker <dalias@libc.org> a écrit :
> >>> 
> >>> Another approach might be having __lookup_numeric always parse with
> >>> AF_UNSPEC, but return error rather than 0 results if the resulting
> >>> family does not match the requested family.
> >> 
> >> This seems the simplest solution in the meantime.
> >> 
> >> Here???s the patch I applied to my trunk.
> >> 
> > 
> > forgot to attach the patch?
> 
> No, but maybe my mail client screwed it up.

It was there, just deeply embedded in multiple layers of MIME.

Rich

Re: getaddrinfo usage with wrong ip family

[Rich Felker]

On Thu, Sep 24, 2015 at 11:39:04AM -0400, Rich Felker wrote:
> On Thu, Sep 24, 2015 at 05:11:03PM +0200, Julien Ramseier wrote:
> > 
> > > Le 24 sept. 2015 à 12:59, Szabolcs Nagy <nsz@port70.net> a écrit :
> > > 
> > > * Julien Ramseier <j.ramseier@gmail.com> [2015-09-24 12:27:22 +0200]:
> > >>> Le 22 sept. 2015 à 17:16, Rich Felker <dalias@libc.org> a écrit :
> > >>> 
> > >>> Another approach might be having __lookup_numeric always parse with
> > >>> AF_UNSPEC, but return error rather than 0 results if the resulting
> > >>> family does not match the requested family.
> > >> 
> > >> This seems the simplest solution in the meantime.
> > >> 
> > >> Here???s the patch I applied to my trunk.
> > >> 
> > > 
> > > forgot to attach the patch?
> > 
> > No, but maybe my mail client screwed it up.
> 
> It was there, just deeply embedded in multiple layers of MIME.

The patch conflicted with commit
cb1c88d42b0ee5e950d85e933c6eb6ecb8175e1d, but I've adapted and applied
it. Let me know if you see anything I did wrong.

Rich