Lichtso / netLink

Socket and Networking Library using msgpack.org[C++11]
GNU General Public License v3.0
217 stars 48 forks source link

Server sockets default to IPv6 only on windows #17

Closed powertomato closed 8 years ago

powertomato commented 8 years ago

Hi,

I compiled the tcp example, and played around with it. I noticed that I could not connect using IPv4 addresses (it would connect to "localhost", "::1" but not "127.0.0.1") . I did a bit of research and the problem seems to be that on windows, the "IPV6_V6ONLY" socket option defaults to 1 according to http://stackoverflow.com/questions/1618240/how-to-support-both-ipv4-and-ipv6-connections.

I confirmed that by executing the following snippet within "initSocket" in Socket.cpp:

DWORD ipv6only = 0;
if(setsockopt(handle, IPPROTO_IPV6, IPV6_V6ONLY, (char*)&ipv6only, sizeof(ipv6only)) == -1) {
    disconnect();
    throw Exception(Exception::ERROR_SET_SOCK_OPT);
}

Using that fixed the issue and I could connect using both "::1" and "127.0.0.1". However I'm not really sure where the snippet belongs. I put it right after the other "setsockopt"-calls, but that seems wrong, as it would also be set for UDP or IPv4 clients.

Best regards

Lichtso commented 8 years ago

I don't see any problem with it being setup for all kinds of sockets. Have you tried the UDP example on windows too?

powertomato commented 8 years ago

I tried the UDP example but it doesn't work there.

Edit: maybe I should clarify. I changed the example a little bit to send the message directly and not use a multicast group. When binding to "*" only ipv6 messages arrived, when binding to "127.0.0.1" only ipv4 messages arrived also when bound to "127.0.0.1" sending to "localhost" wouldn't work either

Lichtso commented 8 years ago

I could replicate the same strange behavior of UDP sockets. Thanks anyway. New commit includes your patch and unifies the IPV6_V6ONLY behavior across all platforms.

powertomato commented 8 years ago

Cool, thanks! As for the UDP part: I found the error. It's "getaddrinfo" Figure 12.3 on http://www.masterraghu.com/subjects/np/introduction/unix_network_programming_v1.3/ch12lev1sec2.html helped a lot understanding the issue.

If we resolve a hostname with AF_UNSPEC set, we get a linked list of addres descriptors. The first one has the family that the OS prefers. If a numeric address is given ( e.g. "127.0.0.1", "::1" ) it will end up with the family according to it's format (IPv4 for "127.0.0.1", IPv6 for "::1"). If a hostname is given, then multiple descriptors are returned.

So if we bind/listen to an IPv4 address but the system prefers IPv6, then "localhost" will result in the first descriptor with the type "sockaddr_in6" and "sendto" will fail, with 10049 i.e. "WSAEADDR NOTAVAIL", as "sendto" expects a "sockaddr_in" in our context.

Similary if we bind to"" and end up with a IPv6 socket, but provide a numeric IPv4 address we get a "sockaddr_in" but expect ""sockaddr_in6". The solution here is to simply use IPv6 syntax for IPv6 sockets, even with IPv4 hosts. I tested sending to "::FFFF:127.0.0.1" and that worked. Also using a socket initialized in IPv4 mode, sending to "127.0.0.1" also gets transfered to that same peer. So a ""-bound socket truly is dual stack, as expected after IPV6_V6ONLY is set to 0. It's just even IPv6 dualstack sockets don't like IPv4 syntax.

For the first problem we need to iterate trough the address descriptor list and use the one with a matching family.

The changed case for UDP_PEER in the Socket::send(...) method would be:

case UDP_PEER: {
    AddrinfoContainer _info = getSocketInfoFor(hostRemote.c_str(), portRemote, false);
    struct addrinfo *info = _info.get();
    while( info->ai_next != NULL){
        if(ipVersion == ANY){
            throw Exception(Exception::ERROR_SEND);
        }
        if(ipVersion == IPv6 && info->ai_family == AF_INET6)
            break;
        if(ipVersion == IPv4 && info->ai_family == AF_INET)
            break;
        info = info->ai_next;
    }
    if( (ipVersion == IPv6 && info->ai_family != AF_INET6) || (ipVersion == IPv4 && info->ai_family != AF_INET) ){
        throw Exception(Exception::ERROR_SEND);
    }
    size_t sentBytes = 0;
    while(sentBytes < (size_t)size) {
        int result = ::sendto(handle, (const char*)buffer + sentBytes, size - sentBytes, 0, info->ai_addr, info->ai_addrlen);
        if(result <= 0) {
            status = BUSY;
            throw Exception(Exception::ERROR_SEND);
        }
        sentBytes += result;
    }

    return sentBytes;
}

Edit: I found a much simpler and even better solution. Simply use the family according to the socket in the Socket::getSocketInfoFor(...) method, then there is no need for iteration.

switch(ipVersion){
    case IPv4:
        conf.ai_family = AF_INET; break;
    case IPv6:
        conf.ai_family = AF_INET6; break;
    default:
        conf.ai_family = AF_UNSPEC; break;
}
powertomato commented 8 years ago

Here is the code I used for testing: https://gist.github.com/powertomato/2e98494e52d70c24e2fd9d1c3b422730

Lichtso commented 8 years ago

Thanks again for your patch, it is in the current commit.