lib/upnp: Each service is it's own NAT device

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4625

4 files changed, 59 insertions, 166 deletions

diff --git a/lib/nat/service.go b/lib/nat/service.go
index c03528cf1d..8a97a6c0e5 100644
--- a/lib/nat/service.go
+++ b/lib/nat/service.go
@@ -58,7 +58,7 @@ func (s *Service) Serve() {
 					if found == 1 {
 						suffix = ""
 					}
-					l.Infoln("Detected", found, "NAT device"+suffix)
+					l.Infoln("Detected", found, "NAT service"+suffix)
 				})
 			}
 		case <-s.stop:
diff --git a/lib/upnp/igd.go b/lib/upnp/igd.go
deleted file mode 100644
index 26e126f860..0000000000
--- a/lib/upnp/igd.go
+++ /dev/null
@@ -1,117 +0,0 @@
-// Copyright (C) 2016 The Syncthing Authors.
-//
-// Adapted from https://github.com/jackpal/Taipei-Torrent/blob/dd88a8bfac6431c01d959ce3c745e74b8a911793/IGD.go
-// Copyright (c) 2010 Jack Palevich (https://github.com/jackpal/Taipei-Torrent/blob/dd88a8bfac6431c01d959ce3c745e74b8a911793/LICENSE)
-//
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are
-// met:
-//
-//    * Redistributions of source code must retain the above copyright
-// notice, this list of conditions and the following disclaimer.
-//    * Redistributions in binary form must reproduce the above
-// copyright notice, this list of conditions and the following disclaimer
-// in the documentation and/or other materials provided with the
-// distribution.
-//    * Neither the name of Google Inc. nor the names of its
-// contributors may be used to endorse or promote products derived from
-// this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-//
-
-package upnp
-
-import (
-	"net"
-	"net/url"
-	"strings"
-	"time"
-
-	"github.com/syncthing/syncthing/lib/nat"
-)
-
-// An IGD is a UPnP InternetGatewayDevice.
-type IGD struct {
-	uuid           string
-	friendlyName   string
-	services       []IGDService
-	url            *url.URL
-	localIPAddress net.IP
-}
-
-func (n *IGD) ID() string {
-	return n.uuid
-}
-
-func (n *IGD) FriendlyName() string {
-	return n.friendlyName
-}
-
-// FriendlyIdentifier returns a friendly identifier (friendly name + IP
-// address) for the IGD.
-func (n *IGD) FriendlyIdentifier() string {
-	return "'" + n.FriendlyName() + "' (" + strings.Split(n.URL().Host, ":")[0] + ")"
-}
-
-func (n *IGD) URL() *url.URL {
-	return n.url
-}
-
-// AddPortMapping adds a port mapping to all relevant services on the
-// specified InternetGatewayDevice. Port mapping will fail and return an error
-// if action is fails for _any_ of the relevant services. For this reason, it
-// is generally better to configure port mapping for each individual service
-// instead.
-func (n *IGD) AddPortMapping(protocol nat.Protocol, internalPort, externalPort int, description string, duration time.Duration) (int, error) {
-	for _, service := range n.services {
-		err := service.AddPortMapping(n.localIPAddress, protocol, internalPort, externalPort, description, duration)
-		if err != nil {
-			return externalPort, err
-		}
-	}
-	return externalPort, nil
-}
-
-// DeletePortMapping deletes a port mapping from all relevant services on the
-// specified InternetGatewayDevice. Port mapping will fail and return an error
-// if action is fails for _any_ of the relevant services. For this reason, it
-// is generally better to configure port mapping for each individual service
-// instead.
-func (n *IGD) DeletePortMapping(protocol nat.Protocol, externalPort int) error {
-	for _, service := range n.services {
-		err := service.DeletePortMapping(protocol, externalPort)
-		if err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
-// GetExternalIPAddress returns the external IP address of the IGD, or an error
-// if no service providing this feature exists.
-func (n *IGD) GetExternalIPAddress() (ip net.IP, err error) {
-	for _, service := range n.services {
-		ip, err = service.GetExternalIPAddress()
-		if err == nil {
-			break
-		}
-	}
-	return
-}
-
-// GetLocalIPAddress returns the IP address of the local network interface
-// which is facing the IGD.
-func (n *IGD) GetLocalIPAddress() net.IP {
-	return n.localIPAddress
-}
diff --git a/lib/upnp/igd_service.go b/lib/upnp/igd_service.go
index faa31f8d25..e7d74636c4 100644
--- a/lib/upnp/igd_service.go
+++ b/lib/upnp/igd_service.go
@@ -43,13 +43,16 @@ import (
 
 // An IGDService is a specific service provided by an IGD.
 type IGDService struct {
-	ID  string
-	URL string
-	URN string
+	UUID      string
+	Device    upnpDevice
+	ServiceID string
+	URL       string
+	URN       string
+	LocalIP   net.IP
 }
 
 // AddPortMapping adds a port mapping to the specified IGD service.
-func (s *IGDService) AddPortMapping(localIPAddress net.IP, protocol nat.Protocol, internalPort, externalPort int, description string, duration time.Duration) error {
+func (s *IGDService) AddPortMapping(protocol nat.Protocol, internalPort, externalPort int, description string, duration time.Duration) (int, error) {
 	tpl := `<u:AddPortMapping xmlns:u="%s">
 	<NewRemoteHost></NewRemoteHost>
 	<NewExternalPort>%d</NewExternalPort>
@@ -60,21 +63,21 @@ func (s *IGDService) AddPortMapping(localIPAddress net.IP, protocol nat.Protocol
 	<NewPortMappingDescription>%s</NewPortMappingDescription>
 	<NewLeaseDuration>%d</NewLeaseDuration>
 	</u:AddPortMapping>`
-	body := fmt.Sprintf(tpl, s.URN, externalPort, protocol, internalPort, localIPAddress, description, duration/time.Second)
+	body := fmt.Sprintf(tpl, s.URN, externalPort, protocol, internalPort, s.LocalIP, description, duration/time.Second)
 
 	response, err := soapRequest(s.URL, s.URN, "AddPortMapping", body)
 	if err != nil && duration > 0 {
 		// Try to repair error code 725 - OnlyPermanentLeasesSupported
 		envelope := &soapErrorResponse{}
 		if unmarshalErr := xml.Unmarshal(response, envelope); unmarshalErr != nil {
-			return unmarshalErr
+			return externalPort, unmarshalErr
 		}
 		if envelope.ErrorCode == 725 {
-			return s.AddPortMapping(localIPAddress, protocol, internalPort, externalPort, description, 0)
+			return s.AddPortMapping(protocol, internalPort, externalPort, description, 0)
 		}
 	}
 
-	return err
+	return externalPort, err
 }
 
 // DeletePortMapping deletes a port mapping from the specified IGD service.
@@ -114,3 +117,13 @@ func (s *IGDService) GetExternalIPAddress() (net.IP, error) {
 
 	return result, nil
 }
+
+// GetLocalIPAddress returns local IP address used to contact this service
+func (s *IGDService) GetLocalIPAddress() net.IP {
+	return s.LocalIP
+}
+
+// ID returns a unique ID for the servic
+func (s *IGDService) ID() string {
+	return s.UUID + "/" + s.Device.FriendlyName + "/" + s.ServiceID + "/" + s.URN + "/" + s.URL
+}
diff --git a/lib/upnp/upnp.go b/lib/upnp/upnp.go
index af722652de..1440617121 100644
--- a/lib/upnp/upnp.go
+++ b/lib/upnp/upnp.go
@@ -83,7 +83,7 @@ func Discover(renewal, timeout time.Duration) []nat.Device {
 		return results
 	}
 
-	resultChan := make(chan IGD)
+	resultChan := make(chan nat.Device)
 
 	wg := &sync.WaitGroup{}
 
@@ -111,21 +111,14 @@ func Discover(renewal, timeout time.Duration) []nat.Device {
 nextResult:
 	for result := range resultChan {
 		if seenResults[result.ID()] {
-			l.Debugf("Skipping duplicate result %s with services:", result.uuid)
-			for _, service := range result.services {
-				l.Debugf("* [%s] %s", service.ID, service.URL)
-			}
+			l.Debugf("Skipping duplicate result %s", result.ID())
 			continue nextResult
 		}
 
-		result := result // Reallocate as we need to keep a pointer
-		results = append(results, &result)
+		results = append(results, result)
 		seenResults[result.ID()] = true
 
-		l.Debugf("UPnP discovery result %s with services:", result.uuid)
-		for _, service := range result.services {
-			l.Debugf("* [%s] %s", service.ID, service.URL)
-		}
+		l.Debugf("UPnP discovery result %s", result.ID())
 	}
 
 	return results
@@ -133,7 +126,7 @@ nextResult:
 
 // Search for UPnP InternetGatewayDevices for <timeout> seconds, ignoring responses from any devices listed in knownDevices.
 // The order in which the devices appear in the result list is not deterministic
-func discover(intf *net.Interface, deviceType string, timeout time.Duration, results chan<- IGD) {
+func discover(intf *net.Interface, deviceType string, timeout time.Duration, results chan<- nat.Device) {
 	ssdp := &net.UDPAddr{IP: []byte{239, 255, 255, 250}, Port: 1900}
 
 	tpl := `M-SEARCH * HTTP/1.1
@@ -185,34 +178,37 @@ USER-AGENT: syncthing/1.0
 			}
 			break
 		}
-		igd, err := parseResponse(deviceType, resp[:n])
+		igds, err := parseResponse(deviceType, resp[:n])
 		if err != nil {
 			l.Infoln("UPnP parse:", err)
 			continue
 		}
-		results <- igd
+		for _, igd := range igds {
+			igd := igd // Copy before sending pointer to the channel.
+			results <- &igd
+		}
 	}
 	l.Debugln("Discovery for device type", deviceType, "on", intf.Name, "finished.")
 }
 
-func parseResponse(deviceType string, resp []byte) (IGD, error) {
+func parseResponse(deviceType string, resp []byte) ([]IGDService, error) {
 	l.Debugln("Handling UPnP response:\n\n" + string(resp))
 
 	reader := bufio.NewReader(bytes.NewBuffer(resp))
 	request := &http.Request{}
 	response, err := http.ReadResponse(reader, request)
 	if err != nil {
-		return IGD{}, err
+		return nil, err
 	}
 
 	respondingDeviceType := response.Header.Get("St")
 	if respondingDeviceType != deviceType {
-		return IGD{}, errors.New("unrecognized UPnP device of type " + respondingDeviceType)
+		return nil, errors.New("unrecognized UPnP device of type " + respondingDeviceType)
 	}
 
 	deviceDescriptionLocation := response.Header.Get("Location")
 	if deviceDescriptionLocation == "" {
-		return IGD{}, errors.New("invalid IGD response: no location specified")
+		return nil, errors.New("invalid IGD response: no location specified")
 	}
 
 	deviceDescriptionURL, err := url.Parse(deviceDescriptionLocation)
@@ -223,29 +219,24 @@ func parseResponse(deviceType string, resp []byte) (IGD, error) {
 
 	deviceUSN := response.Header.Get("USN")
 	if deviceUSN == "" {
-		return IGD{}, errors.New("invalid IGD response: USN not specified")
+		return nil, errors.New("invalid IGD response: USN not specified")
 	}
 
 	deviceUUID := strings.TrimPrefix(strings.Split(deviceUSN, "::")[0], "uuid:")
 	response, err = http.Get(deviceDescriptionLocation)
 	if err != nil {
-		return IGD{}, err
+		return nil, err
 	}
 	defer response.Body.Close()
 
 	if response.StatusCode >= 400 {
-		return IGD{}, errors.New("bad status code:" + response.Status)
+		return nil, errors.New("bad status code:" + response.Status)
 	}
 
 	var upnpRoot upnpRoot
 	err = xml.NewDecoder(response.Body).Decode(&upnpRoot)
 	if err != nil {
-		return IGD{}, err
-	}
-
-	services, err := getServiceDescriptions(deviceDescriptionLocation, upnpRoot.Device)
-	if err != nil {
-		return IGD{}, err
+		return nil, err
 	}
 
 	// Figure out our IP number, on the network used to reach the IGD.
@@ -254,16 +245,15 @@ func parseResponse(deviceType string, resp []byte) (IGD, error) {
 	// suggestions on a better way to do this...
 	localIPAddress, err := localIP(deviceDescriptionURL)
 	if err != nil {
-		return IGD{}, err
+		return nil, err
+	}
+
+	services, err := getServiceDescriptions(deviceUUID, localIPAddress, deviceDescriptionLocation, upnpRoot.Device)
+	if err != nil {
+		return nil, err
 	}
 
-	return IGD{
-		uuid:           deviceUUID,
-		friendlyName:   upnpRoot.Device.FriendlyName,
-		url:            deviceDescriptionURL,
-		services:       services,
-		localIPAddress: localIPAddress,
-	}, nil
+	return services, nil
 }
 
 func localIP(url *url.URL) (net.IP, error) {
@@ -301,18 +291,18 @@ func getChildServices(d upnpDevice, serviceType string) []upnpService {
 	return result
 }
 
-func getServiceDescriptions(rootURL string, device upnpDevice) ([]IGDService, error) {
+func getServiceDescriptions(deviceUUID string, localIPAddress net.IP, rootURL string, device upnpDevice) ([]IGDService, error) {
 	var result []IGDService
 
 	if device.DeviceType == "urn:schemas-upnp-org:device:InternetGatewayDevice:1" {
-		descriptions := getIGDServices(rootURL, device,
+		descriptions := getIGDServices(deviceUUID, localIPAddress, rootURL, device,
 			"urn:schemas-upnp-org:device:WANDevice:1",
 			"urn:schemas-upnp-org:device:WANConnectionDevice:1",
 			[]string{"urn:schemas-upnp-org:service:WANIPConnection:1", "urn:schemas-upnp-org:service:WANPPPConnection:1"})
 
 		result = append(result, descriptions...)
 	} else if device.DeviceType == "urn:schemas-upnp-org:device:InternetGatewayDevice:2" {
-		descriptions := getIGDServices(rootURL, device,
+		descriptions := getIGDServices(deviceUUID, localIPAddress, rootURL, device,
 			"urn:schemas-upnp-org:device:WANDevice:2",
 			"urn:schemas-upnp-org:device:WANConnectionDevice:2",
 			[]string{"urn:schemas-upnp-org:service:WANIPConnection:2", "urn:schemas-upnp-org:service:WANPPPConnection:2"})
@@ -328,7 +318,7 @@ func getServiceDescriptions(rootURL string, device upnpDevice) ([]IGDService, er
 	return result, nil
 }
 
-func getIGDServices(rootURL string, device upnpDevice, wanDeviceURN string, wanConnectionURN string, URNs []string) []IGDService {
+func getIGDServices(deviceUUID string, localIPAddress net.IP, rootURL string, device upnpDevice, wanDeviceURN string, wanConnectionURN string, URNs []string) []IGDService {
 	var result []IGDService
 
 	devices := getChildDevices(device, wanDeviceURN)
@@ -360,7 +350,14 @@ func getIGDServices(rootURL string, device upnpDevice, wanDeviceURN string, wanC
 
 						l.Debugln(rootURL, "- found", service.Type, "with URL", u)
 
-						service := IGDService{ID: service.ID, URL: u.String(), URN: service.Type}
+						service := IGDService{
+							UUID:      deviceUUID,
+							Device:    device,
+							ServiceID: service.ID,
+							URL:       u.String(),
+							URN:       service.Type,
+							LocalIP:   localIPAddress,
+						}
 
 						result = append(result, service)
 					}