| Message ID | 20170323132130.GA25347@kmeaw.com |
|---|---|
| State | New, archived |
| Headers |
Received: (qmail 89312 invoked by alias); 23 Mar 2017 13:21:33 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 89295 invoked by uid 89); 23 Mar 2017 13:21:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy=H*MI:20170323132130, scanned, family X-HELO: kmeaw.com Date: Thu, 23 Mar 2017 16:21:30 +0300 From: kmeaw@kmeaw.com To: libc-alpha@sourceware.org Subject: [PATCH][BZ 21295] getaddrinfo: do not overwrite IPv6 IPs with IPv4 when using AF_UNSPEC Message-ID: <20170323132130.GA25347@kmeaw.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) |
Commit Message
kmeaw@kmeaw.com
March 23, 2017, 1:21 p.m. UTC
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(-) -- 2.12.0
Comments
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
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
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.
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
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;