cmd/bootnode, p2p: use an alternate port mapped to NAT

cmd/bootnode/main.go

@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"net"
 	"os"
+	"time"
 
 	"github.com/ethereum/go-ethereum/cmd/utils"
 	"github.com/ethereum/go-ethereum/crypto"
@@ -110,11 +111,56 @@ func main() {
 
 	realaddr := conn.LocalAddr().(*net.UDPAddr)
 	if natm != nil {
+		var (
+			protocol   = "udp"
+			name       = "ethereum discovery"
+			intport    = realaddr.Port
+			extport    = realaddr.Port
+			mapTimeout = nat.DefaultMapTimeout
+
+			newLogger = func(p string, e int, i int, n nat.Interface) log.Logger {
+				return log.New("proto", p, "extport", e, "intport", i, "interface", n)
+			}
+		)
+
 		if !realaddr.IP.IsLoopback() {
-			go nat.Map(natm, nil, "udp", realaddr.Port, realaddr.Port, "ethereum discovery")
+			log := newLogger(protocol, extport, intport, natm)
+
+			p, err := natm.AddMapping(protocol, extport, intport, name, mapTimeout)
+			if err != nil {
+				log.Debug("Couldn't add port mapping", "err", err)
+			} else {
+				log.Info("Mapped network port")
+				if p != uint16(extport) {
+					log.Debug("Already mapped port", extport, "use alternative port", p)
+					log = newLogger(protocol, int(p), intport, natm)
+					extport = int(p)
+				}
+			}
+
+			go func() {
+				refresh := time.NewTimer(mapTimeout)
+				for {
+					<-refresh.C
+					log.Trace("Start port mapping")
+					p, err := natm.AddMapping(protocol, extport, intport, name, mapTimeout)
+					if err != nil {
+						log.Debug("Couldn't add port mapping", "err", err)
+					} else {
+						if p != uint16(extport) {
+							// If the port mapping is changed after the boot node is executed and the
+							// URL is shared, there is no point in continuing the node. In this case,
+							// re-execute with an available port and share the URL again.
+							natm.DeleteMapping(protocol, int(p), intport)
+							panic(fmt.Errorf("port %d already mapped to another address (hint: use %d", extport, p))
+						}
+					}
+					refresh.Reset(mapTimeout)
+				}
+			}()
 		}
-		if ext, err := natm.ExternalIP(); err == nil {
-			realaddr = &net.UDPAddr{IP: ext, Port: realaddr.Port}
+		if extip, err := natm.ExternalIP(); err == nil {
+			realaddr = &net.UDPAddr{IP: extip, Port: extport}
 		}
 	}
 

p2p/nat/nat.go

@@ -38,7 +38,7 @@ type Interface interface {
 	// protocol is "UDP" or "TCP". Some implementations allow setting
 	// a display name for the mapping. The mapping may be removed by
 	// the gateway when its lifetime ends.
-	AddMapping(protocol string, extport, intport int, name string, lifetime time.Duration) error
+	AddMapping(protocol string, extport, intport int, name string, lifetime time.Duration) (uint16, error)
 	DeleteMapping(protocol string, extport, intport int) error
 
 	// ExternalIP should return the external (Internet-facing)
@@ -91,20 +91,20 @@ func Parse(spec string) (Interface, error) {
 }
 
 const (
-	mapTimeout = 10 * time.Minute
+	DefaultMapTimeout = 10 * time.Minute
 )
 
 // Map adds a port mapping on m and keeps it alive until c is closed.
 // This function is typically invoked in its own goroutine.
 func Map(m Interface, c <-chan struct{}, protocol string, extport, intport int, name string) {
 	log := log.New("proto", protocol, "extport", extport, "intport", intport, "interface", m)
-	refresh := time.NewTimer(mapTimeout)
+	refresh := time.NewTimer(DefaultMapTimeout)
 	defer func() {
 		refresh.Stop()
 		log.Debug("Deleting port mapping")
 		m.DeleteMapping(protocol, extport, intport)
 	}()
-	if err := m.AddMapping(protocol, extport, intport, name, mapTimeout); err != nil {
+	if _, err := m.AddMapping(protocol, extport, intport, name, DefaultMapTimeout); err != nil {
 		log.Debug("Couldn't add port mapping", "err", err)
 	} else {
 		log.Info("Mapped network port")
@@ -117,10 +117,10 @@ func Map(m Interface, c <-chan struct{}, protocol string, extport, intport int,
 			}
 		case <-refresh.C:
 			log.Trace("Refreshing port mapping")
-			if err := m.AddMapping(protocol, extport, intport, name, mapTimeout); err != nil {
+			if _, err := m.AddMapping(protocol, extport, intport, name, DefaultMapTimeout); err != nil {
 				log.Debug("Couldn't add port mapping", "err", err)
 			}
-			refresh.Reset(mapTimeout)
+			refresh.Reset(DefaultMapTimeout)
 		}
 	}
 }
@@ -135,8 +135,8 @@ func (n ExtIP) String() string              { return fmt.Sprintf("ExtIP(%v)", ne
 
 // These do nothing.
 
-func (ExtIP) AddMapping(string, int, int, string, time.Duration) error { return nil }
-func (ExtIP) DeleteMapping(string, int, int) error                     { return nil }
+func (ExtIP) AddMapping(string, int, int, string, time.Duration) (uint16, error) { return 0, nil }
+func (ExtIP) DeleteMapping(string, int, int) error                               { return nil }
 
 // Any returns a port mapper that tries to discover any supported
 // mechanism on the local network.
@@ -193,9 +193,9 @@ func startautodisc(what string, doit func() Interface) Interface {
 	return &autodisc{what: what, doit: doit}
 }
 
-func (n *autodisc) AddMapping(protocol string, extport, intport int, name string, lifetime time.Duration) error {
+func (n *autodisc) AddMapping(protocol string, extport, intport int, name string, lifetime time.Duration) (uint16, error) {
 	if err := n.wait(); err != nil {
-		return err
+		return 0, err
 	}
 	return n.found.AddMapping(protocol, extport, intport, name, lifetime)
 }

p2p/nat/natpmp.go

@@ -44,28 +44,24 @@ func (n *pmp) ExternalIP() (net.IP, error) {
 	return response.ExternalIPAddress[:], nil
 }
 
-func (n *pmp) AddMapping(protocol string, extport, intport int, name string, lifetime time.Duration) error {
+func (n *pmp) AddMapping(protocol string, extport, intport int, name string, lifetime time.Duration) (uint16, error) {
 	if lifetime <= 0 {
-		return fmt.Errorf("lifetime must not be <= 0")
+		return 0, fmt.Errorf("lifetime must not be <= 0")
 	}
 	// Note order of port arguments is switched between our
 	// AddMapping and the client's AddPortMapping.
 	res, err := n.c.AddPortMapping(strings.ToLower(protocol), intport, extport, int(lifetime/time.Second))
 	if err != nil {
-		return err
+		return 0, err
 	}
 
-	// NAT-PMP maps an alternative available port number if the requested
-	// port is already mapped to another address and returns success. In this
-	// case, we return an error because there is no way to return the new port
-	// to the caller.
-	if uint16(extport) != res.MappedExternalPort {
-		// Destroy the mapping in NAT device.
-		n.c.AddPortMapping(strings.ToLower(protocol), intport, 0, 0)
-		return fmt.Errorf("port %d already mapped to another address (%s)", extport, protocol)
-	}
-
-	return nil
+	// NAT-PMP maps an alternative available port number if the requested port
+	// is already mapped to another address and returns success. Handling of
+	// alternate port numbers is done by the caller.
+	//
+	// note: The result of AddPortMapping has several fields, but returns only
+	// MappedExternalPort(no other fields are used).
+	return res.MappedExternalPort, nil
 }
 
 func (n *pmp) DeleteMapping(protocol string, extport, intport int) (err error) {

p2p/nat/natupnp.go

@@ -19,6 +19,8 @@ package nat
 import (
 	"errors"
 	"fmt"
+	"math"
+	"math/rand"
 	"net"
 	"strings"
 	"sync"
@@ -76,18 +78,39 @@ func (n *upnp) ExternalIP() (addr net.IP, err error) {
 	return ip, nil
 }
 
-func (n *upnp) AddMapping(protocol string, extport, intport int, desc string, lifetime time.Duration) error {
+func (n *upnp) AddMapping(protocol string, extport, intport int, desc string, lifetime time.Duration) (uint16, error) {
 	ip, err := n.internalAddress()
 	if err != nil {
-		return nil // TODO: Shouldn't we return the error?
+		return 0, nil // TODO: Shouldn't we return the error?
 	}
 	protocol = strings.ToUpper(protocol)
 	lifetimeS := uint32(lifetime / time.Second)
 	n.DeleteMapping(protocol, extport, intport)
 
-	return n.withRateLimit(func() error {
+	err = n.withRateLimit(func() error {
 		return n.client.AddPortMapping("", uint16(extport), protocol, uint16(intport), ip.String(), true, desc, lifetimeS)
 	})
+	if err == nil {
+		return uint16(extport), nil
+	}
+
+	return uint16(extport), n.withRateLimit(func() error {
+		p, err := n.addAnyPortMapping(protocol, extport, intport, ip, desc, lifetimeS)
+		if err == nil {
+			extport = int(p)
+		}
+		return err
+	})
+}
+
+func (n *upnp) addAnyPortMapping(protocol string, extport, intport int, ip net.IP, desc string, lifetimeS uint32) (uint16, error) {
+	if client, ok := n.client.(*internetgateway2.WANIPConnection2); ok {
+		return client.AddAnyPortMapping("", uint16(extport), protocol, uint16(intport), ip.String(), true, desc, lifetimeS)
+	}
+	// It will retry with a random port number if the client does
+	// not support AddAnyPortMapping.
+	extport = randomPort()
+	return uint16(extport), n.client.AddPortMapping("", uint16(extport), protocol, uint16(intport), ip.String(), true, desc, lifetimeS)
 }
 
 func (n *upnp) internalAddress() (net.IP, error) {
@@ -213,3 +236,8 @@ func discover(out chan<- *upnp, target string, matcher func(goupnp.ServiceClient
 		out <- nil
 	}
 }
+
+func randomPort() int {
+	rand.Seed(time.Now().UnixNano())
+	return rand.Intn(math.MaxUint16-10000) + 10000
+}

p2p/server.go

@@ -571,16 +571,16 @@ func (srv *Server) setupDiscovery() error {
 	}
 	realaddr := conn.LocalAddr().(*net.UDPAddr)
 	srv.log.Debug("UDP listener up", "addr", realaddr)
+
 	if srv.NAT != nil {
 		if !realaddr.IP.IsLoopback() {
 			srv.loopWG.Add(1)
 			go func() {
-				nat.Map(srv.NAT, srv.quit, "udp", realaddr.Port, realaddr.Port, "ethereum discovery")
+				srv.natMapLoop(srv.NAT, "udp", realaddr.Port, realaddr.Port, "ethereum discovery", nat.DefaultMapTimeout)
 				srv.loopWG.Done()
 			}()
 		}
 	}
-	srv.localnode.SetFallbackUDP(realaddr.Port)
 
 	// Discovery V4
 	var unhandled chan discover.ReadPacket
@@ -678,11 +678,10 @@ func (srv *Server) setupListening() error {
 
 	// Update the local node record and map the TCP listening port if NAT is configured.
 	if tcp, ok := listener.Addr().(*net.TCPAddr); ok {
-		srv.localnode.Set(enr.TCP(tcp.Port))
 		if !tcp.IP.IsLoopback() && srv.NAT != nil {
 			srv.loopWG.Add(1)
 			go func() {
-				nat.Map(srv.NAT, srv.quit, "tcp", tcp.Port, tcp.Port, "ethereum p2p")
+				srv.natMapLoop(srv.NAT, "tcp", tcp.Port, tcp.Port, "ethereum p2p", nat.DefaultMapTimeout)
 				srv.loopWG.Done()
 			}()
 		}
@@ -693,6 +692,59 @@ func (srv *Server) setupListening() error {
 	return nil
 }
 
+// natMapLoop performs initialization mapping for nat and repeats refresh.
+func (srv *Server) natMapLoop(natm nat.Interface, protocol string, intport, extport int, name string, interval time.Duration) {
+	var (
+		internal   = intport
+		external   = extport
+		mapTimeout = interval
+
+		newLogger = func(p string, e int, i int, n nat.Interface) log.Logger {
+			return log.New("proto", p, "extport", e, "intport", i, "interface", n)
+		}
+	)
+
+	log := newLogger(protocol, external, internal, natm)
+
+	// Set to 0 to perform initial port mapping. This will return C
+	// immediately and set it to mapTimeout in the next loop.
+	refresh := time.NewTimer(time.Duration(0))
+	defer func() {
+		refresh.Stop()
+		log.Debug("Deleting port mapping")
+		natm.DeleteMapping(protocol, external, internal)
+	}()
+
+	for {
+		select {
+		case _, ok := <-srv.quit:
+			if !ok {
+				return
+			}
+		case <-refresh.C:
+			log.Trace("Start port mapping")
+			p, err := natm.AddMapping(protocol, external, internal, name, mapTimeout)
+			if err != nil {
+				log.Debug("Couldn't add port mapping", "err", err)
+			} else {
+				log.Info("Mapped network port")
+				if p != uint16(external) {
+					log.Debug("Already mapped port", external, "use alternative port", p)
+					log = newLogger(protocol, int(p), internal, natm)
+					external = int(p)
+				}
+				switch protocol {
+				case "tcp":
+					srv.localnode.Set(enr.TCP(external))
+				case "udp":
+					srv.localnode.SetFallbackUDP(external)
+				}
+			}
+			refresh.Reset(mapTimeout)
+		}
+	}
+}
+
 // doPeerOp runs fn on the main loop.
 func (srv *Server) doPeerOp(fn peerOpFunc) {
 	select {