Skip to content
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: ListenPacket can't be used on multicast address #34728

Open
quentinmit opened this issue Oct 7, 2019 · 9 comments
Open

net: ListenPacket can't be used on multicast address #34728

quentinmit opened this issue Oct 7, 2019 · 9 comments

Comments

@quentinmit
Copy link
Contributor

@quentinmit quentinmit commented Oct 7, 2019

What version of Go are you using (go version)?

go version go1.12.9

Does this issue reproduce with the latest release?

Yes, the code is unchanged on master.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/quentin/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/quentin/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/kn/0gfzt13x37j63313z_5szy3h0000gn/T/go-build593496325=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

net.ListenPacket("udp4", "224.0.0.128:5076")

What did you expect to see?

A socket bound to 224.0.0.128:5076 (but not joined to a multicast group)

What did you see instead?

A socket bound to 0.0.0.0:5076 (and not joined to any multicast group)

net.ListenPacket's documentation says:

For UDP and IP networks, if the host in the address parameter is empty or a literal unspecified IP address, ListenPacket listens on all available IP addresses of the local system except multicast IP addresses.

But in fact it also applies this logic if the address is a multicast address. I suspect this is because net.ListenMulticastUDP is documented as having this behavior:

ListenMulticastUDP listens on all available IP addresses of the local system including the group, multicast IP address.

and it shares an implementation function with ListenPacket:

https://github.com/golang/go/blob/master/src/net/sock_posix.go#L210

And no, golang.org/x/net/ipv4 does not provide a workaround here, because all of its APIs require that you start with net.Listen* to get a net.PacketConn. Likewise, the syscall package doesn't provide an escape hatch because net.FilePacketConn is not implemented on Windows (though I will likely end up doing that anyway, with my resulting code only supporting POSIX environments).

quentinmit added a commit to quentinmit/go-pvaccess that referenced this issue Oct 7, 2019
@katiehockman
Copy link
Member

@katiehockman katiehockman commented Oct 7, 2019

/cc @bradfitz

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Oct 7, 2019

/cc @mikioh

@networkimprov
Copy link

@networkimprov networkimprov commented Oct 7, 2019

Last issue-tracker comments by mikioh were last spring, despite several CCs since. Maybe his github notifications are off?

https://github.com/golang/go/issues?utf8=%E2%9C%93&q=is%3Aissue+commenter%3Amikioh

@l0k18
Copy link

@l0k18 l0k18 commented Nov 26, 2019

I have hit up against this problem myself. Actually there is two bugs in my particular case, it's not nearly as clean as I would like it but I found the fix.

First issue is that net.DialUDP by default binds to 0.0.0.0: when I use 224.0.0.1 multicast address on it. This means you can't listen on that same port on any interface once it binds, which would preclude making a p2p multicast app (at least, without my work-around)

As the bug report here says, listening to that multicast address also binds to 0.0.0.0 and if it had no direct impact on functionality I'd say it's not that important. But because listening to the multicast address binds to all interfaces on the same port as the multicast listener, I can't also send to the address from the same machine (which led me to investigating how to tell it what listen port to use for sending).

Second issue is when I have a tun0 interface (VPN) running, it doesn't multicast out the physical network interface, so I filter out interfaces without hardware addresses that don't have the mulitcast flag set. These both relate to the dialer.

Note, it works fine with no tunnel interface, presumably set to higher priority, and the network stack dutifully sends it only out of the higher prioritized virtual interface and the rest of the lan hears nothing. But it is then impossible to listen on the same port on the same machine that is connecting.

To solve it:

  1. Filter the interfaces of anything without multicast flag set, no hardware address, and no network address assigned (in my case, unplugged ethernet as my machine is connected over wifi, so I have to manually set the local address to :0) - that specifically required the filtering of addresses for the local address for DialUDP and creating a net.UDPAddr with that and a :0 to avoid a conflict with the binding of the listener.

  2. Use the discovered interface with net.ListenPacket to the multicast address, convert to ipv4.PacketConn using ipv4.NewPacketConn, join the multicast group.

  3. Using the interface from the discovered connected, multicast enabled physical interface, I get its IP address by splitting the CIDR suffix (strings.Split with "/") and then use that as the local address with net.DialUDP.

As I see it there is two clear bugs involved.

The function net.DialUDP

  1. binds local address to a virtual network interface if one exists, (i assume, because of routing priority)
  2. with the same port as the multicast address,

In fact, what the OP says about it being wrong that the multicast listener binds to 0.0.0.0:portnumber is not really a problem.

The problem is in the dialler, which doesn't enable SO_REUSEPORT and monopolises the port number thereby for all interfaces.

Second, it doesn't discriminate which local IP to bind to, and lets the operating system network stack priority list to choose the first one it sees, which is the wrong one. Dialler should not pick a virtual interface for the LAN multicast addresses by default, as this makes LAN multicasting impossible, and it should not do that when multicast address especially that is for LAN multicasting is being sent to.

Third is the dialler uses the same port on 0.0.0.0 (with the wrong virtual interface IP) as you are trying to broadcast to, which causes a port conflict, but I think it is trying this without REUSEPORT or even REUSEADDR enabled so you can't send and receive at the same time.

It's not that difficult to fix these things but it seems like a pretty bad omission that it in two cases automatically chooses things that cause a binding conflict when there is enough information just from the IP to indicate that it's a LAN address and thus should not use a virtual interface by default.

Here is the code I wrote that resolves the issue:

func NewConnection(send, listen, preSharedKey string, maxDatagramSize int, ctx context.Context, multicast bool) (c *Connection, err error) {
	var sendAddr *net.UDPAddr
	sendConn := []*net.UDPConn{}
	var sC *net.UDPConn
	var listenAddr *net.UDPAddr
	var listenConn net.PacketConn
	var mcInterface net.Interface
	var ifi []net.Interface
	ifi, err = net.Interfaces()
	if err != nil {
		log.ERROR(err)
	}
	for i := range ifi {
		ad, _ := ifi[i].Addrs()
		if ifi[i].Flags&net.FlagMulticast != 0 &&
			ifi[i].HardwareAddr != nil &&
			ad != nil {
			mcInterface = ifi[i]
			break
		}
	}
	if listen != "" {
		if multicast {
			var conn net.PacketConn
			conn, err = net.ListenPacket("udp", listen)
			if err != nil {
				log.ERROR(err)
			}
			pc := ipv4.NewPacketConn(conn)
			err = pc.JoinGroup(&mcInterface, &net.UDPAddr{IP: net.IPv4(
				224, 0, 0, 1)})
			if err != nil {
				log.ERROR(err)
				err = conn.Close()
				if err != nil {
					log.ERROR(err)
					return
				}
			}
			listenConn = conn
		} else {
			listenAddr = GetUDPAddr(listen)
			listenConn, err = net.ListenUDP("udp", listenAddr)
			if err != nil {
				log.ERROR(err)
				return
			}
			log.SPEW(listenConn)
		}
	}
	if send != "" {
		mI, _ := mcInterface.Addrs()
		log.SPEW(mI)
		var listenWithoutEveryInterface string
		for i := range mI {
			_ = mI[i]
			log.DEBUG("ADDRESSS", mI[i])
			a := strings.Split(mI[i].String(), "/")[0]
			if strings.Count(a, ":") == 0 {
				listenWithoutEveryInterface = net.JoinHostPort(a, "0")
			}
		}
		log.DEBUG(listenWithoutEveryInterface)
		var laddr *net.UDPAddr
		laddr, err = net.ResolveUDPAddr("udp", listenWithoutEveryInterface)
		if err != nil {
			log.ERROR(err)
			return
		}
		sendAddr = GetUDPAddr(send)
		sC, err = net.DialUDP("udp", laddr, sendAddr)
		if err != nil {
			log.ERROR(err)
			return
		}
		sendConn = append(sendConn, sC)
	}
	ciph := gcm.GetCipher(preSharedKey)
	return &Connection{
		maxDatagramSize: maxDatagramSize,
		buffers:         make(map[string]*MsgBuffer),
		sendAddress:     sendAddr,
		SendConn:        sendConn,
		listenAddress:   listenAddr,
		listenConn:      listenConn,
		ciph:            ciph, // gcm.GetCipher(*cx.Config.MinerPass),
		ctx:             ctx,
		mx:              &sync.Mutex{},
	}, err
}

@l0k18
Copy link

@l0k18 l0k18 commented Feb 22, 2020

I see there has been no progress on this issue. I have also discovered that the multicast group join function in dgramopt sets the wrong options for darwin and freebsd platforms. I suspect there needs to be a different implementation between linux/android/windows and *bsd/darwin. I wrote the code above primarily for the broadcast purpose anyway, I can't figure out why but this code works for the same exact purpose:

package main

import (
	"fmt"
	"net"
	"time"

	"github.com/p9c/pod/pkg/log"
)

const UDP4MulticastAddress = "224.0.0.1:11049"

func main() {
	ready := make(chan struct{})
	go func() {
		log.INFO("listening")
		ready <- struct{}{}
		Listen(UDP4MulticastAddress, func(addr *net.UDPAddr, count int, data []byte) {
			log.INFO(addr, count, string(data[:count]))
		})
	}()
	bc, err := NewBroadcaster(UDP4MulticastAddress)
	if err != nil {
		log.ERROR(err)
	}
	<-ready
	for i := 0; i < 10; i++ {
		log.INFO("sending", i)
		// var n int
		_, err = bc.Write([]byte(fmt.Sprintf("this is a test %d", i)))
		// log.INFO(n, err)
		if err != nil {
			log.ERROR(err)
		}
	}
	time.Sleep(time.Second * 1)
}

const (
	maxDatagramSize = 8192
)

// NewBroadcaster creates a new UDP multicast connection on which to broadcast
func NewBroadcaster(address string) (*net.UDPConn, error) {
	addr, err := net.ResolveUDPAddr("udp", address)
	if err != nil {
		return nil, err
	}

	conn, err := net.DialUDP("udp", nil, addr)
	if err != nil {
		return nil, err
	}

	return conn, nil
}

// Listen binds to the UDP address and port given and writes packets received
// from that address to a buffer which is passed to a hander
func Listen(address string, handler func(*net.UDPAddr, int, []byte)) {
	// Parse the string address
	addr, err := net.ResolveUDPAddr("udp", address)
	if err != nil {
		log.ERROR(err)
	}

	// Open up a connection
	conn, err := net.ListenMulticastUDP("udp", nil, addr)
	if err != nil {
		log.ERROR(err)
	}

	conn.SetReadBuffer(maxDatagramSize)

	// Loop forever reading from the socket
	for {
		buffer := make([]byte, maxDatagramSize)
		numBytes, src, err := conn.ReadFromUDP(buffer)
		if err != nil {
			log.ERROR("ReadFromUDP failed:", err)
		}

		handler(src, numBytes, buffer)
	}
}
@networkimprov
Copy link

@networkimprov networkimprov commented Feb 22, 2020

@l0k18
Copy link

@l0k18 l0k18 commented Feb 22, 2020

EDIT2: This code does work on freebsd: https://github.com/p9c/pod/blob/master/pkg/transport/cmd/example/listenservemulticast.go

I must be doing something wrong.

EDIT: I may need to not set the options for sends to non-multicast addresses here?

I got it working on both darwin and linux now, here is the altered code related to the NewConnection function presented above.

The trick seems to be that the listener has to have the SO_REUSEADDR toggled, and it seems to need a net.PacketConn (that might not be essential but yes, this works). I'm not sure if this is actually relevant or helpful relating to the bug in this topic exactly, but as you can see it uses ListenPacket and if you feed NewConnection a multicast address it works (and doesn't hear its own packets also, importantly).

func reusePort(network, address string, conn syscall.RawConn) error {
	return conn.Control(func(descriptor uintptr) {
		syscall.SetsockoptInt(int(descriptor), syscall.SOL_SOCKET, syscall.SO_REUSEADDR, 1)
	})
}

// NewConnection creates a new connection with a defined default send
// connection and listener and pre shared key password for encryption on the
// local network
func NewConnection(send, listen, preSharedKey string, maxDatagramSize int,
	ctx context.Context, multicast bool) (c *Connection, err error) {
	sendAddr := &net.UDPAddr{}
	sendConn := &net.UDPConn{}
	listenAddr := &net.UDPAddr{}
	var listenConn net.PacketConn
	if listen != "" {
		config := &net.ListenConfig{Control: reusePort}
		listenConn, err = config.ListenPacket(context.Background(), "udp4", listen)
		if err != nil {
			log.ERROR(err)
		}
	}
	if send != "" {
		sendAddr, err = net.ResolveUDPAddr("udp4", send)
		if err != nil {
			log.ERROR(err)
		}
		sendConn, err = net.DialUDP("udp4", nil, sendAddr)
		if err != nil {
			log.ERROR(err, sendAddr)
		}
		// log.SPEW(sendConn)
	}
	ciph := gcm.GetCipher(preSharedKey)
	return &Connection{
		maxDatagramSize: maxDatagramSize,
		buffers:         make(map[string]*MsgBuffer),
		sendAddress:     sendAddr,
		SendConn:        sendConn,
		listenAddress:   listenAddr,
		listenConn:      &listenConn,
		ciph:            ciph, // gcm.GetCipher(*cx.Config.MinerPass),
		ctx:             ctx,
		mx:              &sync.Mutex{},
	}, err
}

I want to also report something that might be helpful in the future: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=149086

The code above works fine on Linux and MacOS for sure, but the sender doesn't work on FreeBSD. I was relieved when I found this issue on the FreeBSD Bugzilla because now I know where it is probably from. So you might be able to add this one to the caveats about known bugs on the root of the net library documentation.

But there is definitely an issue with the UDP multicast parts of the net library. Implicitly any sender or listener using a multicast address needs to have the SO_REUSEADDR or SO_REUSEPORT (I lean towards the former as being more consistently supported across windows, linux, darwin, and *bsds. Just my opinion.

@mchughj
Copy link

@mchughj mchughj commented Aug 25, 2020

Can we focus on @quentinmit 's original issue report and talk about the setting of the addr to IPv4zero/IPv6unspecified? (I see that the socket and port reuse issue that the subsequent comments focused on has been resolved and really that was an independent problem anyway.)

The current implementation behavior is incorrect and it means that if I were to listen for multicast traffic on 239.2.3.101 by calling net.ListenPacket("udp4", "239.2.3.101:10000") and then joining the group then I would get all multicast packets sent to this machine and port 10000. You can even have a completely separate application on the same box which has subscribed to a different multicast address - for example 239.2.1.94 - and if anyone sends to 239.2.1.94 port 10000 then the golang PacketConn would get those as well even though it doesn't care about 239.2.1.94 explicitly.

This is broken behavior for any system that is subscribing to multicast traffic from many different multicast addresses where the ports for the packets happen to overlap.

The documentation for https://godoc.org/golang.org/x/net/ipv4 posits that you can look at the control messages and determine if you actually care about the packet of data but this is hugely inefficient and simply won't scale for some workloads.

The 'fix' is to remove
https://github.com/golang/go/blob/master/src/net/sock_posix.go#L222-L227

If someone wants the behavior described in the comment then they can specify the IP address of IPv4zero/IPv6unspecified -- if that is what they want.

Does this sound acceptable?

@networkimprov
Copy link

@networkimprov networkimprov commented Aug 25, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.