public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH][BZ 21295] getaddrinfo: do not overwrite IPv6 IPs with IPv4 when using AF_UNSPEC
@ 2017-03-23 13:21 kmeaw
  2017-04-20 12:41 ` Florian Weimer
  2017-06-24 14:59 ` Florian Weimer
  0 siblings, 2 replies; 10+ messages in thread
From: kmeaw @ 2017-03-23 13:21 UTC (permalink / raw)
  To: libc-alpha

CVE-2016-3706 patch introduces a regression which disrupts connectivity
from IPv6-only to dual-stack hosts. This is caused by
convert_hostent_to_gaih_addrtuple which frees the result opposed to
appending to it (prior to the CVE patch in gaih_inet).

This change replaces free(*result) call with a loop which looks for the
pointer to the end of the linked list (&(*result)->next), so successive
calls append the result to the list instead of overwriting it.

Bugzilla entry #21295 describes a way to reproduce the issue.

---
 ChangeLog                   | 5 +++++
 sysdeps/posix/getaddrinfo.c | 8 ++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 7809c3dc2b..56179d6164 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2017-03-23  Dmitry Bilunov  <kmeaw@kmeaw.com>
+
+       * sysdeps/posix/getaddrinfo.c (onvert_hostent_to_gaih_addrtuple):
+       do not overwrite list of IPv6 addresses with IPv4; merge them instead.
+
 2017-03-22  Zack Weinberg  <zackw@panix.com>

        * stdio-common/bug25.c: Include stdlib.h.
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index eed7264850..cf1d99b2e2 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -190,16 +190,16 @@ gaih_inet_serv (const char *servicename, const struct gaih_typeproto *tp,

 /* Convert struct hostent to a list of struct gaih_addrtuple objects.
    h_name is not copied, and the struct hostent object must not be
-   deallocated prematurely.  *RESULT must be NULL or a pointer to an
-   object allocated using malloc, which is freed.  */
+   deallocated prematurely.  *RESULT must be NULL or a pointer to a
+   linked-list, which is scanned to the end.  */
 static bool
 convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
                                   int family,
                                   struct hostent *h,
                                   struct gaih_addrtuple **result)
 {
-  free (*result);
-  *result = NULL;
+  while (*result)
+    result = &(*result)->next;

   /* Count the number of addresses in h->h_addr_list.  */
   size_t count = 0;
--
2.12.0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH][BZ 21295] getaddrinfo: do not overwrite IPv6 IPs with IPv4 when using AF_UNSPEC
  2017-03-23 13:21 [PATCH][BZ 21295] getaddrinfo: do not overwrite IPv6 IPs with IPv4 when using AF_UNSPEC kmeaw
@ 2017-04-20 12:41 ` Florian Weimer
  2017-04-20 12:58   ` Florian Weimer
  2017-04-20 13:44   ` kmeaw
  2017-06-24 14:59 ` Florian Weimer
  1 sibling, 2 replies; 10+ messages in thread
From: Florian Weimer @ 2017-04-20 12:41 UTC (permalink / raw)
  To: kmeaw; +Cc: libc-alpha

On 03/23/2017 02:21 PM, kmeaw@kmeaw.com wrote:
> CVE-2016-3706 patch introduces a regression which disrupts connectivity
> from IPv6-only to dual-stack hosts. This is caused by
> convert_hostent_to_gaih_addrtuple which frees the result opposed to
> appending to it (prior to the CVE patch in gaih_inet).
> 
> This change replaces free(*result) call with a loop which looks for the
> pointer to the end of the linked list (&(*result)->next), so successive
> calls append the result to the list instead of overwriting it.
> 
> Bugzilla entry #21295 describes a way to reproduce the issue.

I'm trying to write a test for this, but I haven't been successful so 
far.  What's the exact container setup that shows this?  What are its 
network interfaces and sysctl settings for IPv6?

I don't think the bug can happen with nss_dns and the upstream sources. 
We either use AF_UNSPEC and the name4 lookup function, or we have just a 
AF_INET or AF_INET6 lookup, so the current overriding behavior does not 
matter.  This means that in order to reproduce the bug, we'd need a 
custom NSS module which does not implement the name4 lookup function.

It's puzzling that you see a problem on Ubuntu.

Thanks,
Florian

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH][BZ 21295] getaddrinfo: do not overwrite IPv6 IPs with IPv4 when using AF_UNSPEC
  2017-04-20 12:41 ` Florian Weimer
@ 2017-04-20 12:58   ` Florian Weimer
  2017-04-20 13:44   ` kmeaw
  1 sibling, 0 replies; 10+ messages in thread
From: Florian Weimer @ 2017-04-20 12:58 UTC (permalink / raw)
  To: kmeaw; +Cc: libc-alpha

On 04/20/2017 02:41 PM, Florian Weimer wrote:
> On 03/23/2017 02:21 PM, kmeaw@kmeaw.com wrote:
>> CVE-2016-3706 patch introduces a regression which disrupts connectivity
>> from IPv6-only to dual-stack hosts. This is caused by
>> convert_hostent_to_gaih_addrtuple which frees the result opposed to
>> appending to it (prior to the CVE patch in gaih_inet).
>>
>> This change replaces free(*result) call with a loop which looks for the
>> pointer to the end of the linked list (&(*result)->next), so successive
>> calls append the result to the list instead of overwriting it.
>>
>> Bugzilla entry #21295 describes a way to reproduce the issue.
> 
> I'm trying to write a test for this, but I haven't been successful so 
> far.  What's the exact container setup that shows this?  What are its 
> network interfaces and sysctl settings for IPv6?
> 
> I don't think the bug can happen with nss_dns and the upstream sources. 
> We either use AF_UNSPEC and the name4 lookup function, or we have just a 
> AF_INET or AF_INET6 lookup, so the current overriding behavior does not 
> matter.  This means that in order to reproduce the bug, we'd need a 
> custom NSS module which does not implement the name4 lookup function.

I think I found another corner case: AF_INET6 with AI_ALL and 
AI_V4MAPPED as flags.  This is independent of the host IPv4/IPv6 support 
level.

> It's puzzling that you see a problem on Ubuntu.

This still mystifies me.

Thanks,
Florian

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH][BZ 21295] getaddrinfo: do not overwrite IPv6 IPs with IPv4 when using AF_UNSPEC
  2017-04-20 12:41 ` Florian Weimer
  2017-04-20 12:58   ` Florian Weimer
@ 2017-04-20 13:44   ` kmeaw
  2017-04-20 14:05     ` Florian Weimer
  1 sibling, 1 reply; 10+ messages in thread
From: kmeaw @ 2017-04-20 13:44 UTC (permalink / raw)
  To: Florian Weimer; +Cc: libc-alpha

On Thu, Apr 20, 2017 at 02:41:26PM +0200, Florian Weimer wrote:
> On 03/23/2017 02:21 PM, kmeaw@kmeaw.com wrote:
> >CVE-2016-3706 patch introduces a regression which disrupts connectivity
> >from IPv6-only to dual-stack hosts. This is caused by
> >convert_hostent_to_gaih_addrtuple which frees the result opposed to
> >appending to it (prior to the CVE patch in gaih_inet).
> >
> >This change replaces free(*result) call with a loop which looks for the
> >pointer to the end of the linked list (&(*result)->next), so successive
> >calls append the result to the list instead of overwriting it.
> >
> >Bugzilla entry #21295 describes a way to reproduce the issue.
> 
> I'm trying to write a test for this, but I haven't been successful
> so far.  What's the exact container setup that shows this?  What are
> its network interfaces and sysctl settings for IPv6?

I have a basic configuration required for Docker to have IPv6-enabled
containers:

# this is a real(hardware) host:

kmeaw@yaws-5477-05:~$ cat /etc/docker/daemon.json 
{
    "fixed-cidr-v6": "2a02:6b8:b010:702c:3027::/80",
    "ipv6": true
}

kmeaw@yaws-5477-05:~$ docker network inspect bridge
[
    {
        "Name": "bridge",
        "Id": "ec8176b9886736996f1a8f6854df2716d84c7ea2dae6054a7278f182585c65b8",
        "Scope": "local",
        "Driver": "bridge",
        "EnableIPv6": true,
        "IPAM": {
            "Driver": "default",
            "Options": null,
            "Config": [
                {
                    "Subnet": "172.17.0.0/16",
                    "Gateway": "172.17.0.1"
                },
                {
                    "Subnet": "2a02:6b8:b010:702c:3027::/80",
                    "Gateway": "2a02:6b8:b010:702c:3027::1"
                }
            ]
        },
        "Internal": false,
        "Containers": {
            "9a1313723f65962d86131e59f296fd32d878464b55202d6ede1f66ce3ff26d03": {
                "Name": "suspicious_mestorf",
                "EndpointID": "0d5591769edc79b19f7112fa95a9247d44f4de890062823d7ce12ccb83333dcb",
                "MacAddress": "02:42:ac:11:00:03",
                "IPv4Address": "172.17.0.3/16",
                "IPv6Address": "2a02:6b8:b010:702c:3027:242:ac11:3/80"
            }
        },
        "Options": {
            "com.docker.network.bridge.default_bridge": "true",
            "com.docker.network.bridge.enable_icc": "true",
            "com.docker.network.bridge.enable_ip_masquerade": "true",
            "com.docker.network.bridge.host_binding_ipv4": "0.0.0.0",
            "com.docker.network.bridge.name": "docker0",
            "com.docker.network.driver.mtu": "1500"
        },
        "Labels": {}
    }
]

kmeaw@yaws-5477-05:~$ cat /etc/sysctl.conf  | grep -v '^#' | grep ipv6
net.ipv6.conf.eth0.autoconf = 2
net.ipv6.conf.eth0.acecpt_ra = 1

# container:

root@9a1313723f65:~# ip -6 a
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 state UNKNOWN qlen 1
    inet6 ::1/128 scope host
       valid_lft forever preferred_lft forever
154: eth0@if155: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 state UP
    inet6 2a02:6b8:b010:702c:3027:242:ac11:3/80 scope global nodad
       valid_lft forever preferred_lft forever
    inet6 fe80::42:acff:fe11:3/64 scope link
       valid_lft forever preferred_lft forever
root@9a1313723f65:~# ip -6 ro
2a02:6b8:b010:702c:3027::/80 dev eth0  proto kernel  metric 256
fe80::/64 dev eth0  proto kernel  metric 256
default via 2a02:6b8:b010:702c:3027::1 dev eth0  metric 1024
root@9a1313723f65:~# ip -4 a
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN qlen 1
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
154: eth0@if155: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP
    inet 172.17.0.3/16 scope global eth0
       valid_lft forever preferred_lft forever
root@9a1313723f65:~# ip -4 ro
default via 172.17.0.1 dev eth0
172.17.0.0/16 dev eth0  proto kernel  scope link  src 172.17.0.3
root@9a1313723f65:~# sysctl -a 2> /dev/null | grep disable_ipv6   
net.ipv6.conf.all.disable_ipv6 = 0
net.ipv6.conf.default.disable_ipv6 = 0
net.ipv6.conf.eth0.disable_ipv6 = 0
net.ipv6.conf.lo.disable_ipv6 = 0

To reproduce the problem, you need global scope (RFC1918 is fine) for
both IPv4 and IPv6. If you don't want to properly setup IPv6 networking
in docker, you can try something like this:

  kmeaw-pc# docker ps
  CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS              PORTS               NAMES
  6e6d4b17326a        ubuntu:12.04        "bash"              3 minutes ago       Up 3 minutes                            vibrant_rosalind
  kmeaw-pc# docker inspect --format '{{.State.Pid}}' 6e6d4b17326a
  23348
  kmeaw-pc# nsenter --net=/proc/23348/ns/net                     
  kmeaw-pc# echo 0 > /proc/sys/net/ipv6/conf/eth0/disable_ipv6
  kmeaw-pc# ip -6 a add 2001:db8::1/64 dev eth0

> I don't think the bug can happen with nss_dns and the upstream
> sources. We either use AF_UNSPEC and the name4 lookup function, or
> we have just a AF_INET or AF_INET6 lookup, so the current overriding
> behavior does not matter.  This means that in order to reproduce the
> bug, we'd need a custom NSS module which does not implement the
> name4 lookup function.

That's right. Latest upstream libnss_dns has name4:

[kmeaw@fill ~]$ strings /lib/libnss_dns.so|grep name4
_nss_dns_gethostbyname4_r
[kmeaw@fill ~]$ /lib/libc.so.6 | head -n1
GNU C Library (GNU libc) stable release version 2.25, by Roland McGrath et al.

But Ubuntu 12.04 does not:

root@6e6d4b17326a:/# dpkg-query -s libc6 | grep Version:
Version: 2.15-0ubuntu10.17
root@6e6d4b17326a:/# strings /lib/x86_64-linux-gnu/libnss_dns.so.2|grep name4
root@6e6d4b17326a:/# strings /lib/x86_64-linux-gnu/libnss_dns.so.2|grep name 
__ns_name_unpack
__ns_name_ntop
_nss_dns_gethostbyname2_r
_nss_dns_gethostbyname_r
_nss_dns_gethostbyname3_r
__dn_skipname
_nss_dns_getnetbyname_r
_nss_dns_getcanonname_r
root@6e6d4b17326a:/# 

> It's puzzling that you see a problem on Ubuntu.

At some point in time upstream libc have implemented gethostbyname4
interface in libnss_dns, then CVE-2016-3706 was fixed.
That fix introduces a bug (regression) which is actually never seen if
you are using upstream version of libc, because it touches only
gethostbyname[23] code paths.
However, you can hide gethostbyname4 implementation from the dynamic
linker:

[kmeaw@fill tmp]$ ./foo | uniq
2a02:6b8::2:242
87.250.250.242
[kmeaw@fill tmp]$ sed -e 's/gethostbyname4/gethostbyname5/g' -i.bak ./libnss_dns.so
[kmeaw@fill tmp]$ ./foo | uniq
87.250.250.242
[kmeaw@fill tmp]$ 

Ubuntu have backported the CVE-2016-3706 fix but have not backported new
gethostbyname4 interface implementation for libnss_dns. That's why this
issue is observable in Ubuntu 12.04.

-- 
kmeaw

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH][BZ 21295] getaddrinfo: do not overwrite IPv6 IPs with IPv4 when using AF_UNSPEC
  2017-04-20 13:44   ` kmeaw
@ 2017-04-20 14:05     ` Florian Weimer
  0 siblings, 0 replies; 10+ messages in thread
From: Florian Weimer @ 2017-04-20 14:05 UTC (permalink / raw)
  To: kmeaw; +Cc: libc-alpha

On 04/20/2017 03:44 PM, kmeaw@kmeaw.com wrote:

> But Ubuntu 12.04 does not:
> 
> root@6e6d4b17326a:/# dpkg-query -s libc6 | grep Version:
> Version: 2.15-0ubuntu10.17
> root@6e6d4b17326a:/# strings /lib/x86_64-linux-gnu/libnss_dns.so.2|grep name4
> root@6e6d4b17326a:/# strings /lib/x86_64-linux-gnu/libnss_dns.so.2|grep name
> __ns_name_unpack
> __ns_name_ntop
> _nss_dns_gethostbyname2_r
> _nss_dns_gethostbyname_r
> _nss_dns_gethostbyname3_r
> __dn_skipname
> _nss_dns_getnetbyname_r
> _nss_dns_getcanonname_r
> root@6e6d4b17326a:/#

Very odd.  This is due to the patch 
debian/patches/all/fedora-nss_dns-gethostbyname4-disable.diff:

# Description: disable _nss_dns_gethostbyname4_r for the moment, as it
#  causes problems for IPv6 users; patch from Fedora by Jakub Jelinek
# Ubuntu: https://bugs.launchpad.net/ubuntu/+source/glibc/+bug/313218
# Upstream: https://bugzilla.redhat.com/show_bug.cgi?id=459756

So your reproducer is indeed very Ubuntu-specific at this point.  (Red 
Hat Enterprise Linux 6 backported the single-request resolver option 
instead, which can be used to work around issues with the parallel lookup.)

We still need to fix this upstream because it's also a bug with AI_ALL 
and AI_V4MAPPED (see my other message).

Thanks,
Florian

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH][BZ 21295] getaddrinfo: do not overwrite IPv6 IPs with IPv4 when using AF_UNSPEC
  2017-03-23 13:21 [PATCH][BZ 21295] getaddrinfo: do not overwrite IPv6 IPs with IPv4 when using AF_UNSPEC kmeaw
  2017-04-20 12:41 ` Florian Weimer
@ 2017-06-24 14:59 ` Florian Weimer
  2017-07-06  9:03   ` Stefan Liebler
  1 sibling, 1 reply; 10+ messages in thread
From: Florian Weimer @ 2017-06-24 14:59 UTC (permalink / raw)
  To: kmeaw, libc-alpha

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

On 03/23/2017 02:21 PM, kmeaw@kmeaw.com wrote:
> CVE-2016-3706 patch introduces a regression which disrupts connectivity
> from IPv6-only to dual-stack hosts. This is caused by
> convert_hostent_to_gaih_addrtuple which frees the result opposed to
> appending to it (prior to the CVE patch in gaih_inet).
> 
> This change replaces free(*result) call with a loop which looks for the
> pointer to the end of the linked list (&(*result)->next), so successive
> calls append the result to the list instead of overwriting it.
> 
> Bugzilla entry #21295 describes a way to reproduce the issue.

Thanks.  I have pushed your patch along with a test case tweak to cover
this area of getaddrinfo.

Thanks,
Florian

[-- Attachment #2: test.patch --]
[-- Type: text/x-patch, Size: 1328 bytes --]

resolv/tst-resolv-basic: Add test cases for bug 21295

2017-06-24  Florian Weimer  <fweimer@redhat.com>

	[BZ #21295]
	* resolv/tst-resolv-basic.c (do_test): Add new test cases.

diff --git a/resolv/tst-resolv-basic.c b/resolv/tst-resolv-basic.c
index 92f912b..3dfa165 100644
--- a/resolv/tst-resolv-basic.c
+++ b/resolv/tst-resolv-basic.c
@@ -398,6 +398,22 @@ do_test (void)
   check_ai ("t.nxdomain.example", "80", AF_INET6,
             "error: Name or service not known\n");
 
+  /* Test for bug 21295.  */
+  check_ai_hints ("www.example", "80",
+                  (struct addrinfo) { .ai_family = AF_INET6,
+                      .ai_socktype = SOCK_STREAM,
+                      .ai_flags = AI_V4MAPPED | AI_ALL, },
+                  "flags: AI_V4MAPPED AI_ALL\n"
+                  "address: STREAM/TCP 2001:db8::1 80\n"
+                  "address: STREAM/TCP ::ffff:192.0.2.17 80\n");
+  check_ai_hints ("t.www.example", "80",
+                  (struct addrinfo) { .ai_family = AF_INET6,
+                      .ai_socktype = SOCK_STREAM,
+                      .ai_flags = AI_V4MAPPED | AI_ALL, },
+                  "flags: AI_V4MAPPED AI_ALL\n"
+                  "address: STREAM/TCP 2001:db8::3 80\n"
+                  "address: STREAM/TCP ::ffff:192.0.2.19 80\n");
+
   resolv_test_end (aux);
 
   return 0;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH][BZ 21295] getaddrinfo: do not overwrite IPv6 IPs with IPv4 when using AF_UNSPEC
  2017-06-24 14:59 ` Florian Weimer
@ 2017-07-06  9:03   ` Stefan Liebler
  2017-07-06 12:19     ` Florian Weimer
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Liebler @ 2017-07-06  9:03 UTC (permalink / raw)
  To: libc-alpha

On 06/24/2017 04:59 PM, Florian Weimer wrote:
> On 03/23/2017 02:21 PM, kmeaw@kmeaw.com wrote:
>> CVE-2016-3706 patch introduces a regression which disrupts connectivity
>> from IPv6-only to dual-stack hosts. This is caused by
>> convert_hostent_to_gaih_addrtuple which frees the result opposed to
>> appending to it (prior to the CVE patch in gaih_inet).
>>
>> This change replaces free(*result) call with a loop which looks for the
>> pointer to the end of the linked list (&(*result)->next), so successive
>> calls append the result to the list instead of overwriting it.
>>
>> Bugzilla entry #21295 describes a way to reproduce the issue.
> 
> Thanks.  I have pushed your patch along with a test case tweak to cover
> this area of getaddrinfo.
> 
> Thanks,
> Florian
> 

Hi Florian,

I've recognized the test-fail resolv/tst-resolv-basic on some but not 
all machines of mine on different architectures s390, intel, power and 
according to 2.26 release page, Joseph also recognized it on his arm, 
mips, power32 machines:
warning: could not become root outside namespace (Operation not permitted)
warning: unshare (CLONE_NEWUTS) failed: Operation not permitted
warning: could not enter network namespace
error: addrinfo comparison failure
query: www.example:80 [10]/0x18
--- expected
+++ actual
@@ -1,3 +1,3 @@
  flags: AI_V4MAPPED AI_ALL
-address: STREAM/TCP 2001:db8::1 80
  address: STREAM/TCP ::ffff:192.0.2.17 80
+address: STREAM/TCP 2001:db8::1 80
error: addrinfo comparison failure
query: t.www.example:80 [10]/0x18
--- expected
+++ actual
@@ -1,3 +1,3 @@
  flags: AI_V4MAPPED AI_ALL
-address: STREAM/TCP 2001:db8::3 80
  address: STREAM/TCP ::ffff:192.0.2.19 80
+address: STREAM/TCP 2001:db8::3 80
error: 2 test failures

Does the ordering of the results matter?

Bye.
Stefan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH][BZ 21295] getaddrinfo: do not overwrite IPv6 IPs with IPv4 when using AF_UNSPEC
  2017-07-06  9:03   ` Stefan Liebler
@ 2017-07-06 12:19     ` Florian Weimer
  2017-07-06 15:18       ` Stefan Liebler
  0 siblings, 1 reply; 10+ messages in thread
From: Florian Weimer @ 2017-07-06 12:19 UTC (permalink / raw)
  To: Stefan Liebler, libc-alpha

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

On 07/06/2017 11:02 AM, Stefan Liebler wrote:
> I've recognized the test-fail resolv/tst-resolv-basic on some but not
> all machines of mine on different architectures s390, intel, power and
> according to 2.26 release page, Joseph also recognized it on his arm,
> mips, power32 machines:
> warning: could not become root outside namespace (Operation not permitted)
> warning: unshare (CLONE_NEWUTS) failed: Operation not permitted
> warning: could not enter network namespace
> error: addrinfo comparison failure
> query: www.example:80 [10]/0x18
> --- expected
> +++ actual
> @@ -1,3 +1,3 @@
>  flags: AI_V4MAPPED AI_ALL
> -address: STREAM/TCP 2001:db8::1 80
>  address: STREAM/TCP ::ffff:192.0.2.17 80
> +address: STREAM/TCP 2001:db8::1 80
> error: addrinfo comparison failure
> query: t.www.example:80 [10]/0x18

I saw this as well, but have not figured out whether it's due to UDP
response ordering or system configuration if (network) namespaces are
not in effect.

Would you please try the attached patch?  Thanks.

Florian

[-- Attachment #2: ipv6-order.patch --]
[-- Type: text/x-patch, Size: 3867 bytes --]

resolv: Deal with non-deterministic address order in tst-resolv-basic

2017-07-06  Florian Weimer  <fw@deneb.enyo.de>

	* resolv/tst-resolv-basic.c (test_bug_21295): New function.
	(do_test): Call it.

diff --git a/resolv/tst-resolv-basic.c b/resolv/tst-resolv-basic.c
index 3dfa165..64eedbb 100644
--- a/resolv/tst-resolv-basic.c
+++ b/resolv/tst-resolv-basic.c
@@ -22,6 +22,7 @@
 #include <string.h>
 #include <support/check.h>
 #include <support/check_nss.h>
+#include <support/format_nss.h>
 #include <support/resolv_test.h>
 #include <support/support.h>
 
@@ -204,6 +205,68 @@ check_ai (const char *name, const char *service,
                          expected);
 }
 
+/* Test for bug 21295: getaddrinfo used to discard address information
+   instead of merging it.  */
+static void
+test_bug_21295 (void)
+{
+  /* The address order is unpredictable.  There are two factors which
+     contribute to that: The stub resolver does not perform proper
+     response matching for A/AAAA queries (an A response could be
+     associated with an AAAA query and vice versa), and without
+     namespaces, system configuration could affect address
+     ordering.  */
+  for (int do_tcp = 0; do_tcp < 2; ++do_tcp)
+    {
+      const struct addrinfo hints =
+        {
+          .ai_family = AF_INET6,
+          .ai_socktype = SOCK_STREAM,
+          .ai_flags = AI_V4MAPPED | AI_ALL,
+        };
+      const char *qname;
+      if (do_tcp)
+        qname = "t.www.example";
+      else
+        qname = "www.example";
+      struct addrinfo *ai = NULL;
+      int ret = getaddrinfo (qname, "80", &hints, &ai);
+      TEST_VERIFY_EXIT (ret == 0);
+
+      const char *expected_a;
+      const char *expected_b;
+      if (do_tcp)
+        {
+          expected_a = "flags: AI_V4MAPPED AI_ALL\n"
+            "address: STREAM/TCP 2001:db8::3 80\n"
+            "address: STREAM/TCP ::ffff:192.0.2.19 80\n";
+          expected_b = "flags: AI_V4MAPPED AI_ALL\n"
+            "address: STREAM/TCP ::ffff:192.0.2.19 80\n"
+            "address: STREAM/TCP 2001:db8::3 80\n";
+        }
+      else
+        {
+          expected_a = "flags: AI_V4MAPPED AI_ALL\n"
+            "address: STREAM/TCP 2001:db8::1 80\n"
+            "address: STREAM/TCP ::ffff:192.0.2.17 80\n";
+          expected_b = "flags: AI_V4MAPPED AI_ALL\n"
+            "address: STREAM/TCP ::ffff:192.0.2.17 80\n"
+            "address: STREAM/TCP 2001:db8::1 80\n";
+        }
+
+      char *actual = support_format_addrinfo (ai, ret);
+      if (!(strcmp (actual, expected_a) == 0
+            || strcmp (actual, expected_b) == 0))
+        {
+          support_record_failure ();
+          printf ("error: %s: unexpected response (TCP: %d):\n%s\n",
+                  __func__, do_tcp, actual);
+        }
+      free (actual);
+      freeaddrinfo (ai);
+    }
+}
+
 static int
 do_test (void)
 {
@@ -398,21 +461,7 @@ do_test (void)
   check_ai ("t.nxdomain.example", "80", AF_INET6,
             "error: Name or service not known\n");
 
-  /* Test for bug 21295.  */
-  check_ai_hints ("www.example", "80",
-                  (struct addrinfo) { .ai_family = AF_INET6,
-                      .ai_socktype = SOCK_STREAM,
-                      .ai_flags = AI_V4MAPPED | AI_ALL, },
-                  "flags: AI_V4MAPPED AI_ALL\n"
-                  "address: STREAM/TCP 2001:db8::1 80\n"
-                  "address: STREAM/TCP ::ffff:192.0.2.17 80\n");
-  check_ai_hints ("t.www.example", "80",
-                  (struct addrinfo) { .ai_family = AF_INET6,
-                      .ai_socktype = SOCK_STREAM,
-                      .ai_flags = AI_V4MAPPED | AI_ALL, },
-                  "flags: AI_V4MAPPED AI_ALL\n"
-                  "address: STREAM/TCP 2001:db8::3 80\n"
-                  "address: STREAM/TCP ::ffff:192.0.2.19 80\n");
+  test_bug_21295 ();
 
   resolv_test_end (aux);
 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH][BZ 21295] getaddrinfo: do not overwrite IPv6 IPs with IPv4 when using AF_UNSPEC
  2017-07-06 12:19     ` Florian Weimer
@ 2017-07-06 15:18       ` Stefan Liebler
  2017-07-06 21:05         ` Florian Weimer
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Liebler @ 2017-07-06 15:18 UTC (permalink / raw)
  To: libc-alpha

On 07/06/2017 02:19 PM, Florian Weimer wrote:
> On 07/06/2017 11:02 AM, Stefan Liebler wrote:
>> I've recognized the test-fail resolv/tst-resolv-basic on some but not
>> all machines of mine on different architectures s390, intel, power and
>> according to 2.26 release page, Joseph also recognized it on his arm,
>> mips, power32 machines:
>> warning: could not become root outside namespace (Operation not permitted)
>> warning: unshare (CLONE_NEWUTS) failed: Operation not permitted
>> warning: could not enter network namespace
>> error: addrinfo comparison failure
>> query: www.example:80 [10]/0x18
>> --- expected
>> +++ actual
>> @@ -1,3 +1,3 @@
>>   flags: AI_V4MAPPED AI_ALL
>> -address: STREAM/TCP 2001:db8::1 80
>>   address: STREAM/TCP ::ffff:192.0.2.17 80
>> +address: STREAM/TCP 2001:db8::1 80
>> error: addrinfo comparison failure
>> query: t.www.example:80 [10]/0x18
> 
> I saw this as well, but have not figured out whether it's due to UDP
> response ordering or system configuration if (network) namespaces are
> not in effect.
> 
> Would you please try the attached patch?  Thanks.
> 
> Florian
> 

Yes sure. The test is now also passing on those machines where it failed
without this patch. The out-files only contain the warnings.

Bye.
Stefan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH][BZ 21295] getaddrinfo: do not overwrite IPv6 IPs with IPv4 when using AF_UNSPEC
  2017-07-06 15:18       ` Stefan Liebler
@ 2017-07-06 21:05         ` Florian Weimer
  0 siblings, 0 replies; 10+ messages in thread
From: Florian Weimer @ 2017-07-06 21:05 UTC (permalink / raw)
  To: Stefan Liebler, libc-alpha

On 07/06/2017 05:18 PM, Stefan Liebler wrote:
> Yes sure. The test is now also passing on those machines where it failed
> without this patch. The out-files only contain the warnings.

Thanks.  I pushed this change.  I suspect it's the UDP packet ordering
issue, by the way, and not a matter of system configuration.

Florian

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-07-06 21:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-23 13:21 [PATCH][BZ 21295] getaddrinfo: do not overwrite IPv6 IPs with IPv4 when using AF_UNSPEC kmeaw
2017-04-20 12:41 ` Florian Weimer
2017-04-20 12:58   ` Florian Weimer
2017-04-20 13:44   ` kmeaw
2017-04-20 14:05     ` Florian Weimer
2017-06-24 14:59 ` Florian Weimer
2017-07-06  9:03   ` Stefan Liebler
2017-07-06 12:19     ` Florian Weimer
2017-07-06 15:18       ` Stefan Liebler
2017-07-06 21:05         ` Florian Weimer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).