From ed66fba42b0673487260d4219084df198a505975 Mon Sep 17 00:00:00 2001 From: bt90 Date: Tue, 12 Sep 2023 14:28:17 +0200 Subject: [PATCH] lib/beacon, lib/discover: Send IPv4 limited broadcast when address listing fails (fixes #1628) (#9087) --- lib/beacon/broadcast.go | 10 ++++++---- lib/discover/local.go | 19 +++++++------------ lib/discover/local_test.go | 12 +++++++----- 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") } }