zeroconf icon indicating copy to clipboard operation
zeroconf copied to clipboard

Wrong IP returned when querying from multiple interfaces

Open leelynne opened this issue 6 years ago • 4 comments

When multiple network interfaces are present (like with docker) zeroconf can return the wrong result.

I registered my server on the wlp2s0 interface only. However this is the result of a query:

avahi-browse -rv _test._tcp

= docker0 IPv4 myserver                                    _test._tcp     local
hostname = [comp.local]
address = [172.17.0.1]
port = [2808]
txt = ["txtv=0"]
= wlp2s0 IPv6 myserver                                    _test._tcp     local
hostname = [comp.local]
address = [192.168.193.138]
port = [2808]
txt = ["txtv=0"]
= wlp2s0 IPv4 myserver                                    _test._tcp     local
hostname = [comp.local]
address = [192.168.193.138]
port = [2808]
txt = ["txtv=0"]

The address returned by the docker0 interface is the IP of the docker0 interface which my service is not registered with.

The issue looks to be when a query originates from 172.17.0.1 the appendAddr function - https://github.com/grandcat/zeroconf/blob/master/server.go#L600 is passed the interface index of the caller as ifIndex.

For some reason the code gets the IP of this interface to add to the answer section:

	iface, _ := net.InterfaceByIndex(ifIndex)
	if iface != nil {
		v4, v6 = addrsForInterface(iface)
	} else {
		v4 = s.service.AddrIPv4
		v6 = s.service.AddrIPv6
	}

It seems like the code should only ever be using the else condition there:

		v4 = s.service.AddrIPv4
		v6 = s.service.AddrIPv6

I can create a pull request for this if this is the right fix?

leelynne avatar Sep 22 '18 04:09 leelynne

Long time ago, but why this thing exists is that mdns is usually used in dynamic environments. So, it is likely that interface IP address can change from time to time.

Probably, the current solution is also not very corrext. However, just assuming static addresses for the lifetime of a running process also might not be right.

Probably, it would need some more sophisticated to register in changes of the interface.

I think docker is a case I did not plan for.

grandcat avatar Sep 22 '18 06:09 grandcat

The idea of appendAddr is that a separate response is sent back over each network interface, containing only the IP addresses for that interface.

In my fork I actually removed service.AddrIPv[46] completely, and only respond with the IPs from each interface:

-		v4 = s.service.AddrIPv4
-		v6 = s.service.AddrIPv6
+		for _, iface := range s.ifaces {
+			i4, i6 := addrsForInterface(&iface)
+			v4 = append(v4, i4...)
+			v6 = append(v6, i6...)
+		}

tmm1 avatar Oct 09 '18 01:10 tmm1

Had the same problem where my WSL IP was being sent to WiFi. A solution that has worked for me is to retrieve the interfaces myself and register on each interface separately.

Edit: For whatever reason, the server still always broadcasts all IP addresses at start up. Then it switches back to just the correct IP for that interface


// Single call to start broadcast
func startMDNSBroadcast(port int, timeoutAfter uint32) error {
	ifaces, _ := net.Interfaces()
	var wg sync.WaitGroup
	for _, iface := range ifaces {
		wg.Add(1)
                
		go startBroadcastOnInterface(&wg, port, iface, timeoutAfter)


	}
	wg.Wait()
       // All interfaces have shutdown
	return nil
}

// Start broadcast on a single interface
func startBroadcastOnInterface(wg *sync.WaitGroup, port int, iface net.Interface, timeoutAfter uint32) error {
	defer wg.Done()
	server, err := zeroconf.Register("GoZeroconf", "_workstation._tcp", "local.", port, []string{"txtv=0", "lo=1", "la=2"}, []net.Interface{iface})
	if err != nil {
		fmt.Println(err)
		return  err
	}
	defer server.Shutdown()

	sig := make(chan os.Signal, 1)

	signal.Notify(sig, os.Interrupt, syscall.SIGTERM)
	select {
	case <-sig:
              // Client exit
	case <-time.After(time.Second * time.Duration(timeoutAfter)):
              // Timed out
	}
	return nil
}

NduatiK avatar Apr 26 '22 07:04 NduatiK

The idea of appendAddr is that a separate response is sent back over each network interface, containing only the IP addresses for that interface.

In my fork I actually removed service.AddrIPv[46] completely, and only respond with the IPs from each interface:

-		v4 = s.service.AddrIPv4
-		v6 = s.service.AddrIPv6
+		for _, iface := range s.ifaces {
+			i4, i6 := addrsForInterface(&iface)
+			v4 = append(v4, i4...)
+			v6 = append(v6, i6...)
+		}
iSrc, err := net.InterfaceByIndex(ifIndex)
if err == nil {
	// fmt.Println(i1.Name)
	for _, i := range s.ifaces {
		if i.Name == iSrc.Name {
			a4, a6 := addrsForInterface(iSrc)
			v4 = append(v4, a4...)
			v6 = append(v6, a6...)
		}
	}
}

q974259836 avatar Aug 12 '22 09:08 q974259836