Repository: syncthing/syncthing (language: go)

Issue:
TCP port 0 is announced in the LAN beacon
### Does your log mention database corruption?

No

### Include required information

There is code that end up making connections to tcp port 0 announced in the LAN beacon:

https://github.com/syncthing/syncthing/blob/main/lib/connections/tcp_listen.go#L171-L177

That makes clients try to connect to TCP port 0, and firewalls log those attempts - this is the main annoyance of these undesired connections.

I think that port-0 has a special meaning changing to a proper port if using a Discovery Server, but I have Global Discovery disabled in my setup, only LAN discovery. In any case, I think that  port 0 should not be announced in the LAN beacon.

LAN Beacon announcement from tcpdump:
```
17:39:04.027909 IP6 fe80::xxxxxxxxxxxx.48859 > ff12::8384.21027: UDP, length 108 
         ....
        0x0060:  8b50 6136 1213 7463 703a 2f2f 302e 302e  .Pa6..tcp://0.0.
        0x0070:  302e 303a 3232 3030 3012 0f74 6370 3a2f  0.0:22000..tcp:/
        0x0080:  2f30 2e30 2e30 2e30 3a30 1214 7175 6963  /0.0.0.0:0..quic
        0x0090:  3a2f 2f30 2e30 2e30 2e30 3a32 3230 3030  ://0.0.0.0:22000
        0x00a0:  1880 d3c8 f4c2 bdf4 fb39                 .........9
```

Typical firewall logs:
```
[104118.933309] refused connection: IN=eth0 OUT= MAC=XXXXX SRC=fe80:0000:0000:0000:6ac4:4dff:fe04:xxxx DST=fe80:0000:0000:0000:329c:23ff:fe01:xxxx LEN=80 TC=0 HOPLIMIT=64 FLOWLBL=0 PROTO=TCP SPT=40630 DPT=0 WINDOW=65535 RES=0x00 SYN URGP=0 
```

(I reported this in #4278 but asked to open a new bug report)


Discussion / hints:
Thanks for reporting!

I just noticed this same thing in my firewall logs, and it was nice to find out the reason of those packages.

Btw. the link to the code lines doesn't point to correct line numbers anymore, because the file has changed in the `main` branch since then. I think you meant these lines originally:
https://github.com/syncthing/syncthing/blob/b13b15758ddf708a19110d4a0b5924dd4b109e3a/lib/connections/tcp_listen.go#L171-L177

In more recent tag, v1.19.2, the lines have moved 12 lines forward:
https://github.com/syncthing/syncthing/blob/v1.19.2/lib/connections/tcp_listen.go#L183-L189

In the firewall log the lines about the blocked packets looked like this:
```
[UFW BLOCK] IN=bond0 OUT= MAC=xx:xx:xx:xx:xx:xx:xx:xx:xx:xx:xx:xx:xx:xx SRC=fe80:0000:0000:0000:xxxx:xxxx:xxxx:xxxx DST=fe80:0000:0000:0000:xxxx:xxxx:xxxx:xxxx LEN=80 TC=0 HOPLIMIT=64 FLOWLBL=959141 PROTO=TCP SPT=37698 DPT=0 WINDOW=64660 RES=0x00 SYN URGP=0 
[UFW BLOCK] IN=bond0 OUT= MAC=xx:xx:xx:xx:xx:xx:xx:xx:xx:xx:xx:xx:xx:xx SRC=10.xxx.xxx.xxx DST=10.xxx.xxx.xxx LEN=60 TOS=0x00 PREC=0x00 TTL=64 ID=37427 DF PROTO=TCP SPT=55274 DPT=0 WINDOW=64240 RES=0x00 SYN URGP=0 
```

First I was a bit scared that it's some malware, because I found some other info which suggested that the usage of TCP port 0 might be some kind of OS probing packet and thought that it's a malware doing network scans. Luckily the presence of those firewall log entries in middle of Syncthing related log messages directed me here.

I hope this helps people to find this ticket with search engines if they wonder about similar firewall log entries.

Fix patch:
diff --git a/lib/discover/local.go b/lib/discover/local.go
index a86d45fb409..01e2feb1ccf 100644
--- a/lib/discover/local.go
+++ b/lib/discover/local.go
@@ -109,6 +109,13 @@ func (c *localClient) Error() error {
 // send.
 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)
+
 	if len(addrs) == 0 {
 		// Nothing to announce
 		return msg, false
@@ -281,3 +288,30 @@ 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 {
+	filtered := addrs[:0]
+	for _, addr := range addrs {
+		u, err := url.Parse(addr)
+		if err != nil {
+			continue
+		}
+
+		tcpAddr, err := net.ResolveTCPAddr("tcp", u.Host)
+		if err != nil {
+			continue
+		}
+
+		switch {
+		case len(tcpAddr.IP) == 0:
+		case tcpAddr.Port == 0:
+		case tcpAddr.IP.IsUnspecified():
+		case !tcpAddr.IP.IsGlobalUnicast() && !tcpAddr.IP.IsLinkLocalUnicast():
+		default:
+			filtered = append(filtered, addr)
+		}
+	}
+	return filtered
+}


Test patch:
diff --git a/lib/discover/local_test.go b/lib/discover/local_test.go
index ce5872c2c04..6f78e095778 100644
--- a/lib/discover/local_test.go
+++ b/lib/discover/local_test.go
@@ -9,6 +9,7 @@ package discover
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"net"
 	"testing"
 
@@ -89,3 +90,33 @@ func TestLocalInstanceIDShouldTriggerNew(t *testing.T) {
 		t.Fatal("new instance ID should be new")
 	}
 }
+
+func TestFilterUnspecified(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
+		"tcp://[2001:db8::1]",                    // remove, no port
+		"tcp://192.0.2.42",                       // remove, no port
+		"tcp://foo:bar",                          // remove, host/port does not resolve
+		"tcp://127.0.0.1:22000",                  // remove, not usable from outside
+		"tcp://[::1]:22000",                      // remove, not usable from outside
+		"tcp://224.1.2.3:22000",                  // remove, not usable from outside (multicast)
+		"tcp://[fe80::9ef:dff1:b332:5e56]:55681", // OK
+		"pure garbage",                           // remove, garbage
+		"",                                       // remove, garbage
+	}
+	exp := []string{
+		"quic://[2001:db8::1]:22000",
+		"tcp://192.0.2.42:22000",
+		"tcp://[fe80::9ef:dff1:b332:5e56]:55681",
+	}
+	res := filterUnspecifiedLocal(addrs)
+	if fmt.Sprint(res) != fmt.Sprint(exp) {
+		t.Log(res)
+		t.Error("filterUnspecified returned invalid addresses")
+	}
+}