New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: Listen is unfriendly to multiple address families, endpoints and subflows #9334

Open
pmarks-net opened this Issue Dec 16, 2014 · 11 comments

Comments

Projects
None yet
8 participants
@pmarks-net
Contributor

pmarks-net commented Dec 16, 2014

The following example program:

package main

import (
        "net"
)

func main() {
        net.Listen("tcp", "localhost:8080")
        select{}
}

Currently yields this result:

$ netstat -nl | grep 8080
tcp        0      0 127.0.0.1:8080          0.0.0.0:*               LISTEN

While the following result would be optimal:

$ netstat -nl | grep 8080
tcp6       0      0 127.0.0.1:8080          :::*                    LISTEN
tcp6       0      0 ::1:8080                :::*                    LISTEN

(Note that the first socket is actually dualstack, and bound to ::ffff:127.0.0.1, but that's less critical than adding the second socket bound to ::1.)

More generally, when you call net.Listen() on a hostname which resolves to multiple IPv4/IPv6 addresses, only the first IPv4 address is selected. An analogous problem occurs if you Listen("tcp", ":8080") on an operating system that doesn't support dualstack sockets: instead of returning a pair of sockets bound to [::]:8080 and 0.0.0.0:80, you only get IPv4.

The fundamental flaw is that Listen() assumes a single socket, which is a leaky abstraction that's inappropriate for high-level things like example servers, e.g.:
http://golang.org/pkg/net/#pkg-overview

Go should either adapt the Listen() API to support multiple sockets, or if that's not feasible, a new multi-socket API should be introduced, which deprecates Listen() for all cases except simple non-wildcard addresses.

@bradfitz

This comment has been minimized.

Member

bradfitz commented Dec 16, 2014

/cc @mikioh

@minux

This comment has been minimized.

Member

minux commented Dec 16, 2014

The correct way to listen for both tcp4 and tcp6 is to leave out the
hostname part.
net.Listen("tcp", ":8080")

Will listen on tcp6 if it's dual-stack system.

I don't think net.Listen should automatically create multiple listener for
each IP
that the hostname resolve to, that is a higher-level thing that the client
should
take care of.

For example, what if the machine added another network interface and another
ip, should the net package track that and automatically listen on the newly
added
IP? This is a decision that the client program should make, not net
package's.

@pmarks-net

This comment has been minimized.

Contributor

pmarks-net commented Dec 16, 2014

net.Listen("tcp", ":8080") ... Will listen on tcp6 if it's dual-stack system.

That's not entirely accurate. There do exist dual-stack systems which don't support dual-stack sockets, so AF_INET and AF_INET6 must be kept separate. In that case, Listen falls back to IPv4, which is suboptimal.

for each IP that the hostname resolve to, that is a higher-level thing

If Listen didn't accept hostnames then I'd agree with you, but hostnames are accepted, and with great power comes great responsibility. Arbitrarily picking one IP address just isn't right. If the "prefer IPv4" bug were fixed, then localhost listeners would flip to ::1 only, which would surprise a lot of people.

@minux

This comment has been minimized.

Member

minux commented Dec 16, 2014

What should happen if more IPs are added to a hostname? Should
the net package automatically listen on those newly added addrs?

No matter what we do, I think the behavior will surprise some users.

Also, what do you think should the Listener's Addr() method return
for a listener that listens on multiple addresses? Return the hostname
is not entirely correct. Because when the Addr() is called, hostname
might resolve to a different set of addresses. Also note that Addr
represents a network end point address, i don't think a hostname
fits here.

IMHO, the existing documentation actually precludes listening on
multiples addresses by a single listener.

I always think supporting hostname in Listen is just a convenience,
and if you want absolute control, you should use explicit address.

@pmarks-net

This comment has been minimized.

Contributor

pmarks-net commented Dec 16, 2014

what do you think should the Listener's Addr() method return for a listener that listens on multiple addresses?

I don't have a good answer, and that's the fundamental flaw I was referring to. Since there's nothing sane for Addr() to return, it may be necessary to create a new MultiListener, and migrate the prominent documentation and sample code.

What should happen if more IPs are added to a hostname?

Perhaps have a type that stores a resolved AddressSet, and another type that holds the listening sockets, where you can Read the AddressSet (to see which sockets exist) or Write it (to open new sockets and close obsolete ones). Then sufficiently-crazy users could resolve multiple hostnames, merge the results together, and dynamically update the pool of sockets without interrupting existing listeners.

But I don't care strongly about the dynamic stuff; I just think listening on localhost or ::+0.0.0.0 should be easy.

The problem is that net.Listen() is easy, popular, and wrong. The alternative (keeping track of multiple listening sockets, with dual-stack behavior that varies by OS) is so much more involved that people avoid doing the right thing, and IPv6 compatibility suffers as a result.

@minux

This comment has been minimized.

Member

minux commented Dec 16, 2014

I wouldn't call the current net.Listen wrong. It just can't handle every
possible
cases. And I don't think a dual-stack system without IPv4-mapped IPv6
support
is the common case. (Even if we add such support, how could we test it on
the builders?)

See also #8124, which has more in-depth discussion of the prefer-IPv4 issue.
Supporting all possible configurations is just an impossible task.

@pmarks-net

This comment has been minimized.

Contributor

pmarks-net commented Dec 16, 2014

Even if we add such support, how could we test it on the builders?

I encountered the same question when working on a C networking library, and the solution was to funnel the IPV6_V6ONLY calls through a function with a test-only flag that emulates an OS without dualstack sockets:

/* Returns 1 on success (dual-stack), 0 on failure (single-stack). */
static int set_socket_dualstack(int fd) {
  if (!forbid_dualstack_sockets_for_testing) {
    const int off = 0;
    return 0 == setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &off, sizeof(off));
  } else {
    /* Force an IPv6-only socket, for testing purposes. */
    const int on = 1;
    setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &on, sizeof(on));
    return 0;
  }
}

Granted, a flag is a bit unsightly, but it's an easy way to ensure coverage of the :: to ::+0.0.0.0 fallback logic.

Supporting all possible configurations is just an impossible task.

I don't agree with that defeatist position; getaddrinfo() with a loop solves the problem for non-pathological cases, and is significantly better than "Pick one IPv4 address."

When a standard library exposes hostname-based socket interfaces, it's the library's responsibility to follow best practices, instead of taking shortcuts. "one hostname ⇒ one socket" is a shortcut that's not obvious to users of the library, but which seems harmful to the networking ecosystem.

Pretending that no code had yet been written, I don't think a Listen(hostname) design that arbitrarily picks a single IP address would stand up to logical scrutiny.

@mikioh mikioh changed the title from net.Listen("tcp", "localhost:8080") is IPv6-unfriendly to net: Listen("tcp", "localhost:8080") is IPv6-unfriendly Dec 20, 2014

@mikioh

This comment has been minimized.

Contributor

mikioh commented Jan 23, 2015

We perhaps need to support this eventually. In that case the new stuff should support not only for dual-stack TCP listeners but for SCTP, MPTCP listeners.

Random thoughts on API:

  • New interface such as MultiListener seems reasonable
  • It would also implement Listener interface
  • Addr method of MultiListener returns a list of addresses, or always returns the first address

Random thoughts on testing:

  • We can make a small, dedicated testbed network by using builders (even though GCE not support IPv6 yet)
  • The testbed would also be useful for testing some layer's keep-alive, recovery scenarios

Random thoughts on roadmap:

  • For now, adding a concrete type into the net package seems wrong
  • We need to make a easy way to inject external types that implement Conn, Addr interface into the net package for keeping the package small
  • Most hard part would be to re-plumb runtime-integrated network poller

References:

PS: Moreover, I'd want to see what happens with the IP Stack Evolution Program: http://www.ietf.org/proceedings/91/slides/slides-91-iab-techplenary-6.pdf

@mikioh mikioh changed the title from net: Listen("tcp", "localhost:8080") is IPv6-unfriendly to net: Listen is unfriendly to multiple address families, endpoints and subflows Jan 23, 2015

@mattn

This comment has been minimized.

Member

mattn commented Jan 23, 2015

Is this related issue? #7598

@mikioh

This comment has been minimized.

Contributor

mikioh commented Jan 23, 2015

No, #7598 is a simple, Windows-specific investigation; how Windows dual IP stacks behave when we use it.

@gopherbot

This comment has been minimized.

gopherbot commented Oct 25, 2016

CL https://golang.org/cl/31931 mentions this issue.

gopherbot pushed a commit that referenced this issue Oct 25, 2016

net: add hostname warnings to all first(isIPv4) functions.
In general, these functions cannot behave correctly when given a
hostname, because a hostname may represent multiple IP addresses, and
first(isIPv4) chooses at most one.

Updates #9334

Change-Id: Icfb629f84af4d976476385a3071270253c0000b1
Reviewed-on: https://go-review.googlesource.com/31931
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

wincent added a commit to wincent/clipper that referenced this issue Nov 29, 2016

Listen on IPv4 and IPv6 by default
As noted [here](golang/go#9334 (comment)), the "correct way" to listen on both IPv4 and IPv6 loopback interfaces is to omit the address part.

Closes: #2

And will also close: wincent/vim-clipper#1

@rhyas rhyas referenced this issue Dec 21, 2016

Merged

F sockaddr 0.7 #2563

0 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment