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

Wrong IP returned when querying from multiple interfaces #43

Open
leelynne opened this issue Sep 22, 2018 · 4 comments
Open

Wrong IP returned when querying from multiple interfaces #43

leelynne opened this issue Sep 22, 2018 · 4 comments

Comments

@leelynne
Copy link

leelynne commented Sep 22, 2018

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?

@grandcat
Copy link
Owner

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.

@tmm1
Copy link
Contributor

tmm1 commented Oct 9, 2018

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...)
+		}

@NduatiK
Copy link

NduatiK commented Apr 26, 2022

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
}

@q974259836
Copy link

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...)
		}
	}
}

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

No branches or pull requests

5 participants