Merge pull request #2269 from calmh/externaladdr

Add external address tracker object

cmd/stfinddevice/main.go

@@ -36,7 +36,7 @@ func main() {
 	}
 
 	discoverer := discover.NewDiscoverer(protocol.LocalDeviceID, nil, nil)
-	discoverer.StartGlobal([]string{server}, 1)
+	discoverer.StartGlobal([]string{server}, nil)
 	addresses, relays := discoverer.Lookup(id)
 	for _, addr := range addresses {
 		log.Println("address:", addr)

cmd/syncthing/connections.go

@@ -18,6 +18,7 @@ import (
 	"github.com/syncthing/protocol"
 	"github.com/syncthing/relaysrv/client"
 	"github.com/syncthing/syncthing/lib/config"
+	"github.com/syncthing/syncthing/lib/discover"
 	"github.com/syncthing/syncthing/lib/events"
 	"github.com/syncthing/syncthing/lib/model"
 	"github.com/syncthing/syncthing/lib/osutil"
@@ -41,6 +42,7 @@ type connectionSvc struct {
 	myID       protocol.DeviceID
 	model      *model.Model
 	tlsCfg     *tls.Config
+	discoverer *discover.Discoverer
 	conns      chan model.IntermediateConnection
 
 	lastRelayCheck map[protocol.DeviceID]time.Time
@@ -50,13 +52,14 @@ type connectionSvc struct {
 	relaysEnabled bool
 }
 
-func newConnectionSvc(cfg *config.Wrapper, myID protocol.DeviceID, mdl *model.Model, tlsCfg *tls.Config) *connectionSvc {
+func newConnectionSvc(cfg *config.Wrapper, myID protocol.DeviceID, mdl *model.Model, tlsCfg *tls.Config, discoverer *discover.Discoverer) *connectionSvc {
 	svc := &connectionSvc{
 		Supervisor: suture.NewSimple("connectionSvc"),
 		cfg:        cfg,
 		myID:       myID,
 		model:      mdl,
 		tlsCfg:     tlsCfg,
+		discoverer: discoverer,
 		conns:      make(chan model.IntermediateConnection),
 
 		connType:       make(map[protocol.DeviceID]model.ConnectionType),
@@ -257,8 +260,8 @@ func (s *connectionSvc) connect() {
 			var relays []string
 			for _, addr := range deviceCfg.Addresses {
 				if addr == "dynamic" {
-					if discoverer != nil {
+					if s.discoverer != nil {
-						t, r := discoverer.Lookup(deviceID)
+						t, r := s.discoverer.Lookup(deviceID)
 						addrs = append(addrs, t...)
 						relays = append(relays, r...)
 					}

cmd/syncthing/externaladdr.go

@@ -0,0 +1,107 @@
+// Copyright (C) 2015 The Syncthing Authors.
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at http://mozilla.org/MPL/2.0/.
+
+package main
+
+import (
+	"fmt"
+	"net"
+	"net/url"
+
+	"github.com/syncthing/syncthing/lib/config"
+)
+
+type externalAddr struct {
+	upnpSvc *upnpSvc
+	cfg     *config.Wrapper
+}
+
+func newExternalAddr(upnpSvc *upnpSvc, cfg *config.Wrapper) *externalAddr {
+	return &externalAddr{
+		upnpSvc: upnpSvc,
+		cfg:     cfg,
+	}
+}
+
+// ExternalAddresses returns a list of addresses that are our best guess for
+// where we are reachable from the outside. As a special case, we may return
+// one or more addresses with an empty IP address (0.0.0.0 or ::) and just
+// port number - this means that the outside address of a NAT gateway should
+// be substituted.
+func (e *externalAddr) ExternalAddresses() []string {
+	var addrs []string
+
+	// Grab our listen addresses from the config. Unspecified ones are passed
+	// on verbatim (to be interpreted by a global discovery server or local
+	// discovery peer). Public addresses are passed on verbatim. Private
+	// addresses are filtered.
+	for _, addrStr := range e.cfg.Options().ListenAddress {
+		addrURL, err := url.Parse(addrStr)
+		if err != nil {
+			l.Infoln("Listen address", addrStr, "is invalid:", err)
+			continue
+		}
+		addr, err := net.ResolveTCPAddr("tcp", addrURL.Host)
+		if err != nil {
+			l.Infoln("Listen address", addrStr, "is invalid:", err)
+			continue
+		}
+
+		if addr.IP == nil || addr.IP.IsUnspecified() {
+			// Address like 0.0.0.0:22000 or [::]:22000 or :22000; include as is.
+			addrs = append(addrs, "tcp://"+addr.String())
+		} else if isPublicIPv4(addr.IP) || isPublicIPv6(addr.IP) {
+			// A public address; include as is.
+			addrs = append(addrs, "tcp://"+addr.String())
+		}
+	}
+
+	// Get an external port mapping from the upnpSvc, if it has one. If so,
+	// add it as another unspecified address.
+	if e.upnpSvc != nil {
+		if port := e.upnpSvc.ExternalPort(); port != 0 {
+			addrs = append(addrs, fmt.Sprintf("tcp://:%d", port))
+		}
+	}
+
+	l.Infoln("External addresses:", addrs)
+
+	return addrs
+}
+
+func isPublicIPv4(ip net.IP) bool {
+	ip = ip.To4()
+	if ip == nil {
+		// Not an IPv4 address (IPv6)
+		return false
+	}
+
+	// IsGlobalUnicast below only checks that it's not link local or
+	// multicast, and we want to exclude private (NAT:ed) addresses as well.
+	rfc1918 := []net.IPNet{
+		{IP: net.IP{10, 0, 0, 0}, Mask: net.IPMask{255, 0, 0, 0}},
+		{IP: net.IP{172, 16, 0, 0}, Mask: net.IPMask{255, 240, 0, 0}},
+		{IP: net.IP{192, 168, 0, 0}, Mask: net.IPMask{255, 255, 0, 0}},
+	}
+	for _, n := range rfc1918 {
+		if n.Contains(ip) {
+			return false
+		}
+	}
+
+	return ip.IsGlobalUnicast()
+}
+
+func isPublicIPv6(ip net.IP) bool {
+	if ip.To4() != nil {
+		// Not an IPv6 address (IPv4)
+		// (To16() returns a v6 mapped v4 address so can't be used to check
+		// that it's an actual v6 address)
+		return false
+	}
+
+	return ip.IsGlobalUnicast()
+}

cmd/syncthing/gui.go

@@ -57,21 +57,23 @@ type apiSvc struct {
 	cfg             config.GUIConfiguration
 	assetDir        string
 	model           *model.Model
+	eventSub        *events.BufferedSubscription
+	discoverer      *discover.Discoverer
 	listener        net.Listener
 	fss             *folderSummarySvc
 	stop            chan struct{}
 	systemConfigMut sync.Mutex
-	eventSub        *events.BufferedSubscription
 }
 
-func newAPISvc(id protocol.DeviceID, cfg config.GUIConfiguration, assetDir string, m *model.Model, eventSub *events.BufferedSubscription) (*apiSvc, error) {
+func newAPISvc(id protocol.DeviceID, cfg config.GUIConfiguration, assetDir string, m *model.Model, eventSub *events.BufferedSubscription, discoverer *discover.Discoverer) (*apiSvc, error) {
 	svc := &apiSvc{
 		id:              id,
 		cfg:             cfg,
 		assetDir:        assetDir,
 		model:           m,
-		systemConfigMut: sync.NewMutex(),
 		eventSub:        eventSub,
+		discoverer:      discoverer,
+		systemConfigMut: sync.NewMutex(),
 	}
 
 	var err error
@@ -628,8 +630,8 @@ func (s *apiSvc) getSystemStatus(w http.ResponseWriter, r *http.Request) {
 	res["alloc"] = m.Alloc
 	res["sys"] = m.Sys - m.HeapReleased
 	res["tilde"] = tilde
-	if cfg.Options().GlobalAnnEnabled && discoverer != nil {
+	if cfg.Options().GlobalAnnEnabled && s.discoverer != nil {
-		res["extAnnounceOK"] = discoverer.ExtAnnounceOK()
+		res["extAnnounceOK"] = s.discoverer.ExtAnnounceOK()
 	}
 	if relaySvc != nil {
 		res["relayClientStatus"] = relaySvc.ClientStatus()
@@ -681,8 +683,8 @@ func (s *apiSvc) postSystemDiscovery(w http.ResponseWriter, r *http.Request) {
 	var qs = r.URL.Query()
 	var device = qs.Get("device")
 	var addr = qs.Get("addr")
-	if len(device) != 0 && len(addr) != 0 && discoverer != nil {
+	if len(device) != 0 && len(addr) != 0 && s.discoverer != nil {
-		discoverer.Hint(device, []string{addr})
+		s.discoverer.Hint(device, []string{addr})
 	}
 }
 
@@ -690,11 +692,11 @@ func (s *apiSvc) getSystemDiscovery(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "application/json; charset=utf-8")
 	devices := map[string][]discover.CacheEntry{}
 
-	if discoverer != nil {
+	if s.discoverer != nil {
 		// Device ids can't be marshalled as keys so we need to manually
 		// rebuild this map using strings. Discoverer may be nil if discovery
 		// has not started yet.
-		for device, entries := range discoverer.All() {
+		for device, entries := range s.discoverer.All() {
 			devices[device.String()] = entries
 		}
 	}

cmd/syncthing/main.go

@@ -114,7 +114,6 @@ var (
 	writeRateLimit *ratelimit.Bucket
 	readRateLimit  *ratelimit.Bucket
 	stop           = make(chan int)
-	discoverer     *discover.Discoverer
 	relaySvc       *relay.Svc
 	cert           tls.Certificate
 	lans           []*net.IPNet
@@ -674,10 +673,6 @@ func syncthingMain() {
 
 	mainSvc.Add(m)
 
-	// GUI
-
-	setupGUI(mainSvc, cfg, m, apiSub)
-
 	// The default port we announce, possibly modified by setupUPnP next.
 
 	uri, err := url.Parse(opts.ListenAddress[0])
@@ -690,27 +685,40 @@ func syncthingMain() {
 		l.Fatalln("Bad listen address:", err)
 	}
 
-	// Start the relevant services
+	// The externalAddr tracks our external addresses for discovery purposes.
 
-	connectionSvc := newConnectionSvc(cfg, myID, m, tlsCfg)
+	var extAddr *externalAddr
-	mainSvc.Add(connectionSvc)
-
-	if opts.RelaysEnabled && (opts.GlobalAnnEnabled || opts.RelayWithoutGlobalAnn) {
-		relaySvc = relay.NewSvc(cfg, tlsCfg, connectionSvc.conns)
-		connectionSvc.Add(relaySvc)
-	}
-
-	// Start discovery
-
-	localPort := addr.Port
-	discoverer = discovery(localPort, relaySvc)
 
 	// Start UPnP. The UPnP service will restart global discovery if the
 	// external port changes.
 
 	if opts.UPnPEnabled {
-		upnpSvc := newUPnPSvc(cfg, localPort)
+		upnpSvc := newUPnPSvc(cfg, addr.Port)
 		mainSvc.Add(upnpSvc)
+
+		// The external address tracker needs to know about the UPnP service
+		// so it can check for an external mapped port.
+		extAddr = newExternalAddr(upnpSvc, cfg)
+	} else {
+		extAddr = newExternalAddr(nil, cfg)
+	}
+
+	// Start discovery
+
+	discoverer := discovery(extAddr, relaySvc)
+
+	// GUI
+
+	setupGUI(mainSvc, cfg, m, apiSub, discoverer)
+
+	// Start connection management
+
+	connectionSvc := newConnectionSvc(cfg, myID, m, tlsCfg, discoverer)
+	mainSvc.Add(connectionSvc)
+
+	if opts.RelaysEnabled && (opts.GlobalAnnEnabled || opts.RelayWithoutGlobalAnn) {
+		relaySvc = relay.NewSvc(cfg, tlsCfg, connectionSvc.conns)
+		connectionSvc.Add(relaySvc)
 	}
 
 	if cpuProfile {
@@ -833,7 +841,7 @@ func startAuditing(mainSvc *suture.Supervisor) {
 	l.Infoln("Audit log in", auditFile)
 }
 
-func setupGUI(mainSvc *suture.Supervisor, cfg *config.Wrapper, m *model.Model, apiSub *events.BufferedSubscription) {
+func setupGUI(mainSvc *suture.Supervisor, cfg *config.Wrapper, m *model.Model, apiSub *events.BufferedSubscription, discoverer *discover.Discoverer) {
 	opts := cfg.Options()
 	guiCfg := overrideGUIConfig(cfg.GUI(), guiAddress, guiAuthentication, guiAPIKey)
 
@@ -862,7 +870,7 @@ func setupGUI(mainSvc *suture.Supervisor, cfg *config.Wrapper, m *model.Model, a
 
 			urlShow := fmt.Sprintf("%s://%s/", proto, net.JoinHostPort(hostShow, strconv.Itoa(addr.Port)))
 			l.Infoln("Starting web GUI on", urlShow)
-			api, err := newAPISvc(myID, guiCfg, guiAssets, m, apiSub)
+			api, err := newAPISvc(myID, guiCfg, guiAssets, m, apiSub, discoverer)
 			if err != nil {
 				l.Fatalln("Cannot start GUI:", err)
 			}
@@ -933,7 +941,7 @@ func shutdown() {
 	stop <- exitSuccess
 }
 
-func discovery(extPort int, relaySvc *relay.Svc) *discover.Discoverer {
+func discovery(extAddr *externalAddr, relaySvc *relay.Svc) *discover.Discoverer {
 	opts := cfg.Options()
 	disc := discover.NewDiscoverer(myID, opts.ListenAddress, relaySvc)
 	if opts.LocalAnnEnabled {
@@ -947,7 +955,7 @@ func discovery(extPort int, relaySvc *relay.Svc) *discover.Discoverer {
 			// to relay servers.
 			time.Sleep(5 * time.Second)
 			l.Infoln("Starting global discovery announcements")
-			disc.StartGlobal(opts.GlobalAnnServers, uint16(extPort))
+			disc.StartGlobal(opts.GlobalAnnServers, extAddr)
 		}()
 
 	}

cmd/syncthing/upnpsvc.go

@@ -11,6 +11,8 @@ import (
 	"time"
 
 	"github.com/syncthing/syncthing/lib/config"
+	"github.com/syncthing/syncthing/lib/events"
+	"github.com/syncthing/syncthing/lib/sync"
 	"github.com/syncthing/syncthing/lib/upnp"
 )
 
@@ -19,6 +21,8 @@ import (
 type upnpSvc struct {
 	cfg        *config.Wrapper
 	localPort  int
+	extPort    int
+	extPortMut sync.Mutex
 	stop       chan struct{}
 }
 
@@ -26,11 +30,11 @@ func newUPnPSvc(cfg *config.Wrapper, localPort int) *upnpSvc {
 	return &upnpSvc{
 		cfg:        cfg,
 		localPort:  localPort,
+		extPortMut: sync.NewMutex(),
 	}
 }
 
 func (s *upnpSvc) Serve() {
-	extPort := 0
 	foundIGD := true
 	s.stop = make(chan struct{})
 
@@ -38,7 +42,15 @@ func (s *upnpSvc) Serve() {
 		igds := upnp.Discover(time.Duration(s.cfg.Options().UPnPTimeoutS) * time.Second)
 		if len(igds) > 0 {
 			foundIGD = true
-			extPort = s.tryIGDs(igds, extPort)
+			s.extPortMut.Lock()
+			oldExtPort := s.extPort
+			s.extPortMut.Unlock()
+
+			newExtPort := s.tryIGDs(igds, oldExtPort)
+
+			s.extPortMut.Lock()
+			s.extPort = newExtPort
+			s.extPortMut.Unlock()
 		} else if foundIGD {
 			// Only print a notice if we've previously found an IGD or this is
 			// the first time around.
@@ -64,6 +76,13 @@ func (s *upnpSvc) Stop() {
 	close(s.stop)
 }
 
+func (s *upnpSvc) ExternalPort() int {
+	s.extPortMut.Lock()
+	port := s.extPort
+	s.extPortMut.Unlock()
+	return port
+}
+
 func (s *upnpSvc) tryIGDs(igds []upnp.IGD, prevExtPort int) int {
 	// Lets try all the IGDs we found and use the first one that works.
 	// TODO: Use all of them, and sort out the resulting mess to the
@@ -76,13 +95,8 @@ func (s *upnpSvc) tryIGDs(igds []upnp.IGD, prevExtPort int) int {
 		}
 
 		if extPort != prevExtPort {
-			// External port changed; refresh the discovery announcement.
-			// TODO: Don't reach out to some magic global here?
 			l.Infof("New UPnP port mapping: external port %d to local port %d.", extPort, s.localPort)
-			if s.cfg.Options().GlobalAnnEnabled {
+			events.Default.Log(events.ExternalPortMappingChanged, map[string]int{"port": extPort})
-				discoverer.StopGlobal()
-				discoverer.StartGlobal(s.cfg.Options().GlobalAnnServers, uint16(extPort))
-			}
 		}
 		if debugNet {
 			l.Debugf("Created/updated UPnP port mapping for external port %d on device %s.", extPort, igd.FriendlyIdentifier())

lib/discover/client_udp.go

@@ -15,6 +15,7 @@ import (
 	"time"
 
 	"github.com/syncthing/protocol"
+	"github.com/syncthing/syncthing/lib/events"
 	"github.com/syncthing/syncthing/lib/sync"
 )
 
@@ -113,44 +114,24 @@ func (d *UDPClient) broadcast() {
 	}
 
 	timer := time.NewTimer(0)
+
+	eventSub := events.Default.Subscribe(events.ExternalPortMappingChanged)
+	defer events.Default.Unsubscribe(eventSub)
+
 	for {
 		select {
 		case <-d.stop:
 			return
 
+		case <-eventSub.C():
+			ok := d.sendAnnouncement(remote, conn)
+
+			d.mut.Lock()
+			d.status = ok
+			d.mut.Unlock()
+
 		case <-timer.C:
-			var ok bool
+			ok := d.sendAnnouncement(remote, conn)
-
-			if debug {
-				l.Debugf("discover %s: broadcast: Sending self announcement to %v", d.url, remote)
-			}
-
-			ann := d.announcer.Announcement()
-			pkt, err := ann.MarshalXDR()
-			if err != nil {
-				timer.Reset(d.errorRetryInterval)
-				continue
-			}
-
-			_, err = conn.WriteTo(pkt, remote)
-			if err != nil {
-				if debug {
-					l.Debugf("discover %s: broadcast: Failed to send self announcement: %s", d.url, err)
-				}
-				ok = false
-			} else {
-				// Verify that the announce server responds positively for our device ID
-
-				time.Sleep(1 * time.Second)
-
-				pkt, err := d.Lookup(protocol.DeviceIDFromBytes(ann.This.ID))
-				if err != nil && debug {
-					l.Debugf("discover %s: broadcast: Self-lookup failed: %v", d.url, err)
-				} else if debug {
-					l.Debugf("discover %s: broadcast: Self-lookup returned: %v", d.url, pkt.This.Addresses)
-				}
-				ok = len(pkt.This.Addresses) > 0
-			}
 
 			d.mut.Lock()
 			d.status = ok
@@ -165,6 +146,40 @@ func (d *UDPClient) broadcast() {
 	}
 }
 
+func (d *UDPClient) sendAnnouncement(remote net.Addr, conn *net.UDPConn) bool {
+	if debug {
+		l.Debugf("discover %s: broadcast: Sending self announcement to %v", d.url, remote)
+	}
+
+	ann := d.announcer.Announcement()
+	pkt, err := ann.MarshalXDR()
+	if err != nil {
+		return false
+	}
+
+	myID := protocol.DeviceIDFromBytes(ann.This.ID)
+
+	_, err = conn.WriteTo(pkt, remote)
+	if err != nil {
+		if debug {
+			l.Debugf("discover %s: broadcast: Failed to send self announcement: %s", d.url, err)
+		}
+		return false
+	}
+
+	// Verify that the announce server responds positively for our device ID
+
+	time.Sleep(1 * time.Second)
+
+	ann, err = d.Lookup(myID)
+	if err != nil && debug {
+		l.Debugf("discover %s: broadcast: Self-lookup failed: %v", d.url, err)
+	} else if debug {
+		l.Debugf("discover %s: broadcast: Self-lookup returned: %v", d.url, ann.This.Addresses)
+	}
+	return len(ann.This.Addresses) > 0
+}
+
 func (d *UDPClient) Lookup(device protocol.DeviceID) (Announce, error) {
 	extIP, err := net.ResolveUDPAddr(d.url.Scheme, d.url.Host)
 	if err != nil {

lib/discover/discover.go

@@ -33,7 +33,7 @@ type Discoverer struct {
 	cacheLifetime       time.Duration
 	negCacheCutoff      time.Duration
 	beacons             []beacon.Interface
-	extPort             uint16
+	extAddr             externalAddr
 	localBcastTick      <-chan time.Time
 	forcedBcastTick     chan time.Time
 
@@ -50,6 +50,10 @@ type relayStatusProvider interface {
 	ClientStatus() map[string]bool
 }
 
+type externalAddr interface {
+	ExternalAddresses() []string
+}
+
 type CacheEntry struct {
 	Address string
 	Seen    time.Time
@@ -115,7 +119,7 @@ func (d *Discoverer) startLocalIPv6Multicasts(localMCAddr string) {
 	go d.recvAnnouncements(mb)
 }
 
-func (d *Discoverer) StartGlobal(servers []string, extPort uint16) {
+func (d *Discoverer) StartGlobal(servers []string, extAddr externalAddr) {
 	d.mut.Lock()
 	defer d.mut.Unlock()
 
@@ -123,7 +127,7 @@ func (d *Discoverer) StartGlobal(servers []string, extPort uint16) {
 		d.stopGlobal()
 	}
 
-	d.extPort = extPort
+	d.extAddr = extAddr
 	wg := sync.NewWaitGroup()
 	clients := make(chan Client, len(servers))
 	for _, address := range servers {
@@ -303,8 +307,8 @@ func (d *Discoverer) Announcement() Announce {
 
 func (d *Discoverer) announcementPkt(allowExternal bool) Announce {
 	var addrs []string
-	if d.extPort != 0 && allowExternal {
+	if allowExternal && d.extAddr != nil {
-		addrs = []string{fmt.Sprintf("tcp://:%d", d.extPort)}
+		addrs = d.extAddr.ExternalAddresses()
 	} else {
 		addrs = resolveAddrs(d.listenAddrs)
 	}

lib/discover/discover_test.go

@@ -103,7 +103,7 @@ func TestGlobalDiscovery(t *testing.T) {
 		"test1://23.23.23.23:234",
 		"test2://234.234.234.234.2345",
 	}
-	d.StartGlobal(servers, 1234)
+	d.StartGlobal(servers, nil)
 
 	if len(d.clients) != 3 {
 		t.Fatal("Wrong number of clients")

lib/events/events.go

@@ -39,6 +39,7 @@ const (
 	FolderCompletion
 	FolderErrors
 	FolderScanProgress
+	ExternalPortMappingChanged
 
 	AllEvents = (1 << iota) - 1
 )
@@ -87,6 +88,8 @@ func (t EventType) String() string {
 		return "DeviceResumed"
 	case FolderScanProgress:
 		return "FolderScanProgress"
+	case ExternalPortMappingChanged:
+		return "ExternalPortMappingChanged"
 	default:
 		return "Unknown"
 	}