Bug 14967 - getaddrinfo(NULL) with AI_PASSIVE returns 0.0.0.0 and :: (in this order)
Summary: getaddrinfo(NULL) with AI_PASSIVE returns 0.0.0.0 and :: (in this order)
Status: RESOLVED DUPLICATE of bug 9981
Alias: None
Product: glibc
Classification: Unclassified
Component: network (show other bugs)
Version: 2.17
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-16 16:08 UTC by Pavel Šimerda
Modified: 2014-06-14 05:33 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments
Proposed patch (329 bytes, patch)
2013-02-23 19:11 UTC, Ruslan N. Marchenko
Details | Diff
Proposed patch (v2) (1.07 KB, patch)
2013-02-26 20:24 UTC, Ruslan N. Marchenko
Details | Diff
testing code (1.05 KB, text/x-csrc)
2013-02-26 20:47 UTC, Ruslan N. Marchenko
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Šimerda 2012-12-16 16:08:35 UTC
On Linux, with net.ipv6.bindv6only = 0 (the default), getaddrinfo(NULL) with AI_PASSIVE returns 0.0.0.0 and :: (in this order). AI_PASSIVE was meant to return a list of addresses to listen on, but it is impossible to listen on 0.0.0.0 and :: at the same time, if :: implies dualstack mode.

Even the order is wrong, as listening on :: would work for both protocols, while listening on 0.0.0.0 does not. If the order is fixed, applications could just spit out a warning but still work correctly.

The only proper fix I can think of, is test for net.ipv6.bindv6only and return only :: if this is 0, otherwise return both addresses (preferably IPv6 first).

Upstream bug report:

https://bugs.launchpad.net/ubuntu/+source/eglibc/+bug/673708

More information:

https://fedoraproject.org/wiki/Networking/NameResolution#Binding_to_the_INADDR_ANY_and.2For_in6addr_any_addresses

Cheers,

Pavel
Comment 1 Ruslan N. Marchenko 2013-02-23 18:52:55 UTC
Here's how rfc3484_sort arguments look like

a1 = {dest_addr = 0x603010, 
	source_addr = {
		sin6_family = 10, sin6_port = 1153, sin6_flowinfo = 0, 
		sin6_addr = {
			__in6_u = {
        			__u6_addr8 = '\000' <repeats 15 times>, "\001", 
				__u6_addr16 = {0, 0, 0, 0, 0, 0, 0, 256}, 
				__u6_addr32 = {0, 0, 0, 16777216}
			}
		}, 
		sin6_scope_id = 0
	}, 
  	source_addr_len = 28 '\034', 
	got_source_addr = true, 
	source_addr_flags = 0 '\000', 
	prefixlen = 128 '\200', 
	index = 1, native = -1
}
a2 = {dest_addr = 0x603070, 
	source_addr = {
		sin6_family = 2, sin6_port = 44220, sin6_flowinfo = 16777343, 
		sin6_addr = {
			__in6_u = {
        			__u6_addr8 = "\000\000\000\000\000\000\000\000\000\000\377\377\177\000\000\001", 
				__u6_addr16 = {0, 0, 0, 0, 0, 65535, 127, 256}, 
				__u6_addr32 = {0, 0, 4294901760, 16777343}
			}
		}, 
		sin6_scope_id = 0
	}, 
	source_addr_len = 16 '\020', 
	got_source_addr = true, 
	source_addr_flags = 0 '\000', 
	prefixlen = 8 '\b', 
	index = 1, native = -1
}

Definitely "1660   /* Rule 5: Prefer matching label.  */" prefers a2 - it has same lablel for SRC & DST, while a1 has 0 (lo) for SRC and 3 (any) for DST.
In other words here is a collision between implementation, default behaviour of connect() and RFC recommended default labels definition. Either new label should be attached to IPv4 loopback
label ::ffff:0f00:1/128 8 (gai.conf)
or getaddrinfo should reset loopback to any in SRC before sorting.
Comment 2 Ruslan N. Marchenko 2013-02-23 19:11:26 UTC
Created attachment 6890 [details]
Proposed patch

Just a draft, unfortunately I don't have capabilities to rebuild GLIBC right now but it should reflect the idea.
Comment 3 Pavel Šimerda 2013-02-24 22:15:14 UTC
(In reply to comment #2)
> Created attachment 6890 [details]
> Proposed patch
> 
> Just a draft, unfortunately I don't have capabilities to rebuild GLIBC right
> now but it should reflect the idea.

What exactly is the patch supposed to do?
Comment 4 Ruslan N. Marchenko 2013-02-24 23:16:14 UTC
Reset source address to in6addr_any if it is loopback address returned by connect to in6addr_any. To make the sorting fair.
Comment 5 Ruslan N. Marchenko 2013-02-24 23:21:00 UTC
In proposed draft there is a typo
			&& IN6_IS_ADDR_LOOPBACK(&results[i].source_addr))
should read as
			&& IN6_IS_ADDR_LOOPBACK(&results[i].source_addr.sin6_addr))

so - if it is V6 family, and original address was any, but returned address is loopback - reset it to any. Loopback is just a behavioural return, it's not proper address, a result of connect() to unspecified address.
Comment 6 Pavel Šimerda 2013-02-25 08:12:45 UTC
(In reply to comment #4)
> Reset source address to in6addr_any if it is loopback address returned by
> connect to in6addr_any.

This doesn't fix the problem described in this bug report and it is incorrect. If the family is AF_INET6, the problem doesn't occur at all.

(In reply to comment #5)
> Loopback is just a behavioural return,

getaddrinfo doesn't return loopback addresses in this case (NULL hostname and AI_PASSIVE) at all. So I'm afraid you are trying to fix a nonexistent problem instead of the one described in the bug report.
Comment 7 Ruslan N. Marchenko 2013-02-25 18:57:54 UTC
You're wrong, this is exactly fix for described problem. Just open the code of getaddrinfo implementation and you'll see this.
later today I'll attach normal tested patch as I've cloned trunk git.

getaddrinfo retruns 0.0.0.0 and :: in this order because order is based on comparing :: with ::1 and 0.0.0.0 with 127.0.0.1
Comment 8 Pavel Šimerda 2013-02-25 23:48:06 UTC
(In reply to comment #7)
> You're wrong, this is exactly fix for described problem. Just open the code of
> getaddrinfo implementation and you'll see this.
> later today I'll attach normal tested patch as I've cloned trunk git.
> 
> getaddrinfo retruns 0.0.0.0 and :: in this order because order is based on
> comparing :: with ::1 and 0.0.0.0 with 127.0.0.1

If this is so, then it really looks like an ugly hack (and is not documented as such, at the least). Shouldn't it be better to always have the correct addresses there in the first place? I can go through the code later but actually that should be just a sorting function that should *only* compare addresses that are on the list. You should never need to rewrite them.

Anyway, it is IMO wrong to return both of them even in the opposite order when binding :: binds dualstack and then 0.0.0.0 fails. But nevertheless it would fix at least part of the problem.
Comment 9 Ruslan N. Marchenko 2013-02-26 07:39:50 UTC
Well, I can oppose that validating source address with connect() for addresses
supposed to be used by bind() is an ugly hack :). 
getaddrinfo() should return all available addresses, it's just if AF_INET6 is
available by default it should returned first. getaddrinfo() cannot know
whether you're going to use V6ONLY option or not, and with that option both
returned addresses are valid and bind()-able. 
Sorry, preparing patch takes longer than I expected, will try to finish till
that evening.
Comment 10 Pavel Šimerda 2013-02-26 08:27:48 UTC
(In reply to comment #9)
> Well, I can oppose that validating source address with connect() for addresses
> supposed to be used by bind() is an ugly hack :).

Who are you opposing, though? I don't know of anybody.

> getaddrinfo() cannot know whether you're going to use V6ONLY option or not, and with that option both

You have a point here. But I guess you should use AF_INET6 if you are going to do V6ONLY.

> returned addresses are valid and bind()-able.

From another point of view, we might ask the developers to always use V6ONLY for IPv6. But something should be done to resolve it.

> Sorry, preparing patch takes longer than I expected, will try to finish till
> that evening.

Don't worry, we're not in hurry for this one (at least that's what I think).
Comment 11 Ruslan N. Marchenko 2013-02-26 20:09:23 UTC
Ok, here is the test script:

Native GLIBC - wrong result (0.0.0.0,::)
>> 
glibc-build$ ../b 1234
socket(2, 2, 17)
socket: Success
bind/connect: Success
getsockname: AF=2; LEN=16
 [0.0.0.0:1234] ---> [(null):1234]
Len: 16, 16
socket(10, 2, 17)
socket: Success
bind/connect: Success
getsockname: AF=10; LEN=28
 [:::1234] ---> [(null):1234]
Len: 28, 28
<<<
Patched GLIBC - correct result (::,0.0.0.0)
>>>
glibc-build$ /opt/jack/lib/ld-linux-x86-64.so.2 --library-path /opt/jack/lib ../b 1234
socket(10, 2, 17)
socket: Success
bind/connect: Success
getsockname: AF=10; LEN=28
 [:::1234] ---> [(null):1234]
Len: 28, 28
socket(2, 2, 17)
socket: Success
bind/connect: Success
getsockname: AF=2; LEN=16
 [0.0.0.0:1234] ---> [(null):1234]
Len: 16, 16
<<<
Native GLIBC+GAI V4Mapped lo workaround - correct result (::,0.0.0.0)
>>>
glibc-build$ sudo bash -c "echo 'label ::ffff:0f00:1/128 8' >> /etc/gai.conf"
[sudo] password for ruff: 
glibc-build$ ../b 1234
socket(10, 2, 17)
socket: Success
bind/connect: Success
getsockname: AF=10; LEN=28
 [:::1234] ---> [(null):1234]
Len: 28, 28
socket(2, 2, 17)
socket: Success
bind/connect: Success
getsockname: AF=2; LEN=16
 [0.0.0.0:1234] ---> [(null):1234]
Len: 16, 16
glibc-build$ 
>>>
Comment 12 Ruslan N. Marchenko 2013-02-26 20:24:37 UTC
Created attachment 6901 [details]
Proposed patch (v2)

And here's the patch for above test case
Comment 13 Ruslan N. Marchenko 2013-02-26 20:47:37 UTC
Created attachment 6902 [details]
testing code

simple testing code
compile with 
gcc -g -o b b.c
Comment 14 Pavel Šimerda 2013-02-26 21:35:46 UTC
Ah, it's actually in one of the hackier parts of code. Nice if it works for you. Sorry for not having enough time to test it myself right now.
Comment 15 Jackie Rosen 2014-02-16 17:51:16 UTC Comment hidden (spam)
Comment 16 Craig McQueen 2014-04-10 00:41:34 UTC
Is this a duplicate of bug #9981?
Comment 17 Pavel Šimerda 2014-04-10 06:55:01 UTC
(In reply to Craig McQueen from comment #16)
> Is this a duplicate of bug #9981?

Yes, it's the very same problem, I don't know how I could overlook it.

*** This bug has been marked as a duplicate of bug 9981 ***