lib/beacon, lib/discover: Send IPv4 limited broadcast when address listing fails (fixes #1628) (#9087)

3 files changed, 20 insertions, 21 deletions

diff --git a/lib/beacon/broadcast.go b/lib/beacon/broadcast.go
index e580bb7f1..6a4a05e16 100644
--- a/lib/beacon/broadcast.go
+++ b/lib/beacon/broadcast.go
@@ -46,8 +46,9 @@ func writeBroadcasts(ctx context.Context, inbox <-chan []byte, port int) error {
 
 		intfs, err := net.Interfaces()
 		if err != nil {
-			l.Debugln(err)
-			return err
+			l.Debugln("Failed to list interfaces:", err)
+			// net.Interfaces() is broken on Android. see https://github.com/golang/go/issues/40569
+			// Use the general broadcast address 255.255.255.255 instead.
 		}
 
 		var dsts []net.IP
@@ -58,8 +59,9 @@ func writeBroadcasts(ctx context.Context, inbox <-chan []byte, port int) error {
 
 			addrs, err := intf.Addrs()
 			if err != nil {
-				l.Debugln(err)
-				return err
+				l.Debugln("Failed to list interface addresses:", err)
+				// Interface discovery might work while retrieving the addresses doesn't. So log the error and carry on.
+				continue
 			}
 
 			for _, addr := range addrs {
diff --git a/lib/discover/local.go b/lib/discover/local.go
index ba950f441..a64c218a1 100644
--- a/lib/discover/local.go
+++ b/lib/discover/local.go
@@ -110,11 +110,8 @@ func (c *localClient) Error() error {
 func (c *localClient) announcementPkt(instanceID int64, msg []byte) ([]byte, bool) {
 	addrs := c.addrList.AllAddresses()
 
-	// The list of all addresses can include unspecified addresses intended
-	// for a discovery server to complete, based on the packet source. We
-	// don't do that for local discovery, so filter out addresses that are
-	// usable as-is.
-	addrs = filterUnspecifiedLocal(addrs)
+	// remove all addresses which are not dialable
+	addrs = filterUndialableLocal(addrs)
 
 	// do not leak relay tokens to discovery
 	addrs = sanitizeRelayAddresses(addrs)
@@ -266,7 +263,7 @@ func (c *localClient) registerDevice(src net.Addr, device Announce) bool {
 				continue
 			}
 			u.Host = net.JoinHostPort(host, strconv.Itoa(tcpAddr.Port))
-			l.Debugf("discover: Reconstructed URL is %#v", u)
+			l.Debugf("discover: Reconstructed URL is %v", u)
 			validAddresses = append(validAddresses, u.String())
 			l.Debugf("discover: Replaced address %v in %s to get %s", tcpAddr.IP, addr, u.String())
 		} else {
@@ -292,9 +289,9 @@ func (c *localClient) registerDevice(src net.Addr, device Announce) bool {
 	return isNewDevice
 }
 
-// filterUnspecifiedLocal returns the list of addresses after removing any
-// unspecified, localhost, multicast, broadcast or port-zero addresses.
-func filterUnspecifiedLocal(addrs []string) []string {
+// filterUndialableLocal returns the list of addresses after removing any
+// localhost, multicast, broadcast or port-zero addresses.
+func filterUndialableLocal(addrs []string) []string {
 	filtered := addrs[:0]
 	for _, addr := range addrs {
 		u, err := url.Parse(addr)
@@ -310,9 +307,7 @@ func filterUnspecifiedLocal(addrs []string) []string {
 		switch {
 		case len(tcpAddr.IP) == 0:
 		case tcpAddr.Port == 0:
-		case tcpAddr.IP.IsUnspecified():
-		case !tcpAddr.IP.IsGlobalUnicast() && !tcpAddr.IP.IsLinkLocalUnicast():
-		default:
+		case tcpAddr.IP.IsGlobalUnicast(), tcpAddr.IP.IsLinkLocalUnicast(), tcpAddr.IP.IsUnspecified():
 			filtered = append(filtered, addr)
 		}
 	}
diff --git a/lib/discover/local_test.go b/lib/discover/local_test.go
index 6f78e0957..7fe0649dc 100644
--- a/lib/discover/local_test.go
+++ b/lib/discover/local_test.go
@@ -91,14 +91,14 @@ func TestLocalInstanceIDShouldTriggerNew(t *testing.T) {
 	}
 }
 
-func TestFilterUnspecified(t *testing.T) {
+func TestFilterUndialable(t *testing.T) {
 	addrs := []string{
 		"quic://[2001:db8::1]:22000",             // OK
 		"tcp://192.0.2.42:22000",                 // OK
 		"quic://[2001:db8::1]:0",                 // remove, port zero
 		"tcp://192.0.2.42:0",                     // remove, port zero
-		"quic://[::]:22000",                      // remove, unspecified
-		"tcp://0.0.0.0:22000",                    // remove, unspecified
+		"quic://[::]:22000",                      // OK
+		"tcp://0.0.0.0:22000",                    // OK
 		"tcp://[2001:db8::1]",                    // remove, no port
 		"tcp://192.0.2.42",                       // remove, no port
 		"tcp://foo:bar",                          // remove, host/port does not resolve
@@ -112,11 +112,13 @@ func TestFilterUnspecified(t *testing.T) {
 	exp := []string{
 		"quic://[2001:db8::1]:22000",
 		"tcp://192.0.2.42:22000",
+		"quic://[::]:22000",
+		"tcp://0.0.0.0:22000",
 		"tcp://[fe80::9ef:dff1:b332:5e56]:55681",
 	}
-	res := filterUnspecifiedLocal(addrs)
+	res := filterUndialableLocal(addrs)
 	if fmt.Sprint(res) != fmt.Sprint(exp) {
 		t.Log(res)
-		t.Error("filterUnspecified returned invalid addresses")
+		t.Error("filterUndialableLocal returned invalid addresses")
 	}
 }