Skip to content

Commit

Permalink
lib/beacon, lib/discover: Send IPv4 limited broadcast when address li…
Browse files Browse the repository at this point in the history
…sting fails (fixes syncthing#1628) (syncthing#9087)
  • Loading branch information
bt90 authored Sep 12, 2023
1 parent 415f320 commit ed66fba
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 21 deletions.
10 changes: 6 additions & 4 deletions lib/beacon/broadcast.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
19 changes: 7 additions & 12 deletions lib/discover/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand All @@ -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)
}
}
Expand Down
12 changes: 7 additions & 5 deletions lib/discover/local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")
}
}

0 comments on commit ed66fba

Please sign in to comment.