Discussion:
[PATCH] Default Listen with IPv6 enabled incorrect
(too old to reply)
Colm MacCarthaigh,,,
2003-08-13 00:59:34 UTC
Permalink
--On Tuesday, August 12, 2003 5:58 PM +0100 "Colm MacCarthaigh,,,"
Even lo0 ? That's hard ;)
% ifconfig -a6
%
As an asside I maintain that this is a broken system configuration, it's
just as if you had IPv4 built in, but chose not to configure any IPv4
addresses. What would have Apache do in such a situation ? Surely exiting
is appropriate ...

But since you're less wrong than I am on this one, using AI_ADDRCONFIG
is best. This will weed out the problem.

But just to make clear I think "Listen 80" *really* needs to mean
"Listen on port 80", not "Listen on port 80 in IPv4 only" :)
if getaddrinfo for PF_UNSPEC returns '::' in this situation
then it's really a lib(c|socket|nsl) bug, it does this on
Linux aswell, but the KAME implementation (and I believe windows)
have it fixed.
No, the OS is not returning '::'. We're not using PF_UNSPEC, but
hard-coding PF_INET6. (AF_INET6, AP_INET6, whatever the #define is.)
This is the real bug, Apache shouldnt be second-guessing getaddrinfo,
APR's call_resolver is an excellent example of how to do it properly,
AI_ADDRCONFIG is the key. Trouble is apr_sockaddr_info_get isnt
calling it for us. This is an APR bug, see the attached patch.
In alloc_listener, we're hardcoding '::' and PF_INET6 unconditionally if
the OS supports IPv6 (as determined by find_default_family). (See the
!addr branch.)
It's not as broken as it seems, it does:

254 apr_sockaddr_info_get(&sa, NULL, APR_INET6, 0, 0, p) == APR_SUCCESS

So it *should* be valid. Problem is it *always* returns APR_SUCCESS
for a NULL hostname :( Line 583 of apr/network_io/unix/sockaddr.c
is the real problem.
I have a hunch that we may be able to create ephemeral ports (NULL address
passed in) without a configured IPv6 interface on Solaris. Which strikes
me as not being totally wrong on Solaris's part.
I still maintain that my patch should be applied. Let PF_UNSPEC figure it
out.
But your patch didnt do this, it just blindly returned 0.0.0.0. This
would break literally hundreds of peoples configs!

APR patch attached, it should fix your problem. I don't
have any systems I can configure easily into the state you
describe, so I can't test it properly just yet.

What should happen:
apr_sockaddr_info will call find_addresses, which will call
call_resolver which will have AI_ADDRCONFIG dutifuly set. So
if apply the patch , the problem should go away.

This all leaves listen.c bigger than it needs to be. So there's
changes in the patch for that too. The original find_default_family
didnt even compile for me anyway ;) The changes to listen.c create
one very slight logic problem in that:

"Listen 80
Listen 0.0.0.0 80"

will now result in a failure of apache to start because binding
to the socket twice will fail, wheras previously the dupe would
have been cought. I don't think this is a major problem.

Now, the extra cookie that we all deserve. This all shows up
a massive problem with apr_socket_create, it only creates
one socket. But apr_sockaddr_info_get returns a linked
list to many. It's allowable for AI_PASSIVE to return both
:: and 0.0.0.0, for example, or for round-robin DNS records
to return multiple addresses and this isnt handled properly.
My head hurts already.

There really needs to be an APR call that means: "walk this
linked list and creete/bind sockets for each node."
--
Colm MacCárthaigh Public Key: colm+***@stdlib.net
***@stdlib.net http://www.stdlib.net/
Wan-Teh Chang
2003-08-13 01:17:12 UTC
Permalink
Regarding your patch: I think it is fine to
assume that AI_PASSIVE is defined if
getaddrinfo() exists. AI_PASSIVE is in RFC
2133, the first "Basic Socket Interface Extensions
for IPv6" RFC.

Regarding a previous patch to use AI_ADDRCONFIG:
all the getaddrinfo() implementations I know of
conform to either RFC 2133 or RFC 2553 and do
not take the AI_ADDRCONFIG flag. (RFC 2553 has
AI_ADDRCONFIG but it is for getipnodebyname().)
It is risky to assume that when AI_ADDRCONFIG is
passed to these getaddrinfo() implementations,
getaddrinfo() will either silently ignore it or
fail with the EAI_BADFLAGS error. It is better
to confirm that getaddrinfo() conforms to RFC
3493 before passing AI_ADDRCONFIG to it.

Wan-Teh
Colm MacCarthaigh,,,
2003-08-13 01:58:51 UTC
Permalink
Post by Wan-Teh Chang
Regarding your patch: I think it is fine to
assume that AI_PASSIVE is defined if
getaddrinfo() exists.
Same here, but it seemed more consistent with the existing
code to check anyway.
Post by Wan-Teh Chang
all the getaddrinfo() implementations I know of
conform to either RFC 2133 or RFC 2553 and do
not take the AI_ADDRCONFIG flag. (RFC 2553 has
AI_ADDRCONFIG but it is for getipnodebyname().)
Solaris getaddrinfo takes it, at least in the
version I'm using. Though now that I've tested it,
it doesnt seem to honour it :(

Solaris is where Justin is seeing the problem.
Gah, back to sqaure one. (well, square 1.1).
--
Colm MacCárthaigh Public Key: colm+***@stdlib.net
***@stdlib.net http://www.stdlib.net/
Justin Erenkrantz
2003-08-13 19:29:20 UTC
Permalink
--On Wednesday, August 13, 2003 2:58 AM +0100 "Colm MacCarthaigh,,,"
Post by Colm MacCarthaigh,,,
Solaris getaddrinfo takes it, at least in the
version I'm using. Though now that I've tested it,
it doesn't seem to honour it :(
Solaris is where Justin is seeing the problem.
Gah, back to sqaure one. (well, square 1.1).
Since Joe Orton was the one who added that AI_ADDRCONFIG flag, he should be
the one who speaks up on this.

I now have a server that runs, which is more than I could say yesterday. --
justin
Joe Orton
2003-08-19 10:55:12 UTC
Permalink
Post by Wan-Teh Chang
all the getaddrinfo() implementations I know of
conform to either RFC 2133 or RFC 2553 and do
not take the AI_ADDRCONFIG flag.
glibc is getting support for this.
Post by Wan-Teh Chang
(RFC 2553 has
AI_ADDRCONFIG but it is for getipnodebyname().)
It is risky to assume that when AI_ADDRCONFIG is
passed to these getaddrinfo() implementations,
getaddrinfo() will either silently ignore it or
fail with the EAI_BADFLAGS error. It is better
to confirm that getaddrinfo() conforms to RFC
3493 before passing AI_ADDRCONFIG to it.
I think it's a pretty small risk, but you're probably
right. I tested all the getaddrinfo() implementations I
could find and they did the right thing; it's not like
this is going to be a risk for anyone porting APR to old
Unixes.

Regards,

joe

Justin Erenkrantz
2003-08-13 19:23:24 UTC
Permalink
--On Wednesday, August 13, 2003 1:59 AM +0100 "Colm MacCarthaigh,,,"
This is the real bug, Apache shouldn't be second-guessing getaddrinfo,
APR's call_resolver is an excellent example of how to do it properly,
AI_ADDRCONFIG is the key. Trouble is apr_sockaddr_info_get isnt
calling it for us. This is an APR bug, see the attached patch.
Okay, I just committed variants of your patch to CVS that work for me on
Solaris. The biggest gotcha was setting servname to be non-NULL when hostname
was NULL.

Thanks! -- justin
Loading...