cmd/bootnode, p2p: use an alternate mapped port

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"
@@ -108,20 +109,18 @@ func main() {
 		utils.Fatalf("-ListenUDP: %v", err)
 	}
 
-	realaddr := conn.LocalAddr().(*net.UDPAddr)
-	if natm != nil {
-		if !realaddr.IP.IsLoopback() {
-			go nat.Map(natm, nil, "udp", realaddr.Port, realaddr.Port, "ethereum discovery")
-		}
-		if ext, err := natm.ExternalIP(); err == nil {
-			realaddr = &net.UDPAddr{IP: ext, Port: realaddr.Port}
+	db, _ := enode.OpenDB("")
+	ln := enode.NewLocalNode(db, nodeKey)
+
+	listenerAddr := conn.LocalAddr().(*net.UDPAddr)
+	if natm != nil && !listenerAddr.IP.IsLoopback() {
+		natAddr := doPortMapping(natm, ln, listenerAddr)
+		if natAddr != nil {
+			listenerAddr = natAddr
 		}
 	}
 
-	printNotice(&nodeKey.PublicKey, *realaddr)
-
-	db, _ := enode.OpenDB("")
-	ln := enode.NewLocalNode(db, nodeKey)
+	printNotice(&nodeKey.PublicKey, *listenerAddr)
 	cfg := discover.Config{
 		PrivateKey:  nodeKey,
 		NetRestrict: restrictList,
@@ -148,3 +147,60 @@ func printNotice(nodeKey *ecdsa.PublicKey, addr net.UDPAddr) {
 	fmt.Println("Note: you're using cmd/bootnode, a developer tool.")
 	fmt.Println("We recommend using a regular node as bootstrap node for production deployments.")
 }
+
+func doPortMapping(natm nat.Interface, ln *enode.LocalNode, addr *net.UDPAddr) *net.UDPAddr {
+	const (
+		protocol = "udp"
+		name     = "ethereum discovery"
+	)
+	newLogger := func(external int, internal int) log.Logger {
+		return log.New("proto", protocol, "extport", external, "intport", internal, "interface", natm)
+	}
+
+	var (
+		intport    = addr.Port
+		extaddr    = &net.UDPAddr{IP: addr.IP, Port: addr.Port}
+		mapTimeout = nat.DefaultMapTimeout
+		log        = newLogger(addr.Port, intport)
+	)
+	addMapping := func() {
+		// Get the external address.
+		var err error
+		extaddr.IP, err = natm.ExternalIP()
+		if err != nil {
+			log.Debug("Couldn't get external IP", "err", err)
+			return
+		}
+		// Create the mapping.
+		p, err := natm.AddMapping(protocol, extaddr.Port, intport, name, mapTimeout)
+		if err != nil {
+			log.Debug("Couldn't add port mapping", "err", err)
+			return
+		}
+		if p != uint16(extaddr.Port) {
+			extaddr.Port = int(p)
+			log = newLogger(extaddr.Port, intport)
+			log.Info("NAT mapped alternative port")
+		} else {
+			log.Info("NAT mapped port")
+		}
+		// Update IP/port information of the local node.
+		ln.SetStaticIP(extaddr.IP)
+		ln.SetFallbackUDP(extaddr.Port)
+	}
+
+	// Perform mapping once, synchronously.
+	log.Info("Attempting port mapping")
+	addMapping()
+
+	// Refresh the mapping periodically.
+	go func() {
+		refresh := time.NewTimer(mapTimeout)
+		for range refresh.C {
+			addMapping()
+			refresh.Reset(mapTimeout)
+		}
+	}()
+
+	return extaddr
+}

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,23 @@ 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.
+//
+// Note that Map does not handle the situation where the NAT interface assigns a different
+// external port than the requested one.
 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 +120,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 +138,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 +196,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,21 @@ 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.
+	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"
@@ -40,6 +42,7 @@ type upnp struct {
 	client      upnpClient
 	mu          sync.Mutex
 	lastReqTime time.Time
+	rand        *rand.Rand
 }
 
 type upnpClient interface {
@@ -76,18 +79,50 @@ 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 = n.randomPort()
+	err := n.client.AddPortMapping("", uint16(extport), protocol, uint16(intport), ip.String(), true, desc, lifetimeS)
+	if err != nil {
+		return 0, err
+	}
+	return uint16(extport), nil
+}
+
+func (n *upnp) randomPort() int {
+	if n.rand == nil {
+		n.rand = rand.New(rand.NewSource(time.Now().UnixNano()))
+	}
+	return n.rand.Intn(math.MaxUint16-10000) + 10000
 }
 
 func (n *upnp) internalAddress() (net.IP, error) {

p2p/server.go

@@ -195,6 +195,9 @@ type Server struct {
 	discmix   *enode.FairMix
 	dialsched *dialScheduler
 
+	// This is read by the NAT port mapping loop.
+	portMappingRegister chan *portMapping
+
 	// Channels into the run loop.
 	quit                    chan struct{}
 	addtrusted              chan *enode.Node
@@ -483,6 +486,8 @@ func (srv *Server) Start() (err error) {
 	if err := srv.setupLocalNode(); err != nil {
 		return err
 	}
+	srv.setupPortMapping()
+
 	if srv.ListenAddr != "" {
 		if err := srv.setupListening(); err != nil {
 			return err
@@ -521,24 +526,6 @@ func (srv *Server) setupLocalNode() error {
 			srv.localnode.Set(e)
 		}
 	}
-	switch srv.NAT.(type) {
-	case nil:
-		// No NAT interface, do nothing.
-	case nat.ExtIP:
-		// ExtIP doesn't block, set the IP right away.
-		ip, _ := srv.NAT.ExternalIP()
-		srv.localnode.SetStaticIP(ip)
-	default:
-		// Ask the router about the IP. This takes a while and blocks startup,
-		// do it in the background.
-		srv.loopWG.Add(1)
-		go func() {
-			defer srv.loopWG.Done()
-			if ip, err := srv.NAT.ExternalIP(); err == nil {
-				srv.localnode.SetStaticIP(ip)
-			}
-		}()
-	}
 	return nil
 }
 
@@ -656,14 +643,15 @@ func (srv *Server) setupListening() error {
 	srv.ListenAddr = listener.Addr().String()
 
 	// Update the local node record and map the TCP listening port if NAT is configured.
-	if tcp, ok := listener.Addr().(*net.TCPAddr); ok {
+	tcp, isTCP := listener.Addr().(*net.TCPAddr)
+	if isTCP {
 		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.loopWG.Done()
-			}()
+		if !tcp.IP.IsLoopback() && !tcp.IP.IsPrivate() {
+			srv.portMappingRegister <- &portMapping{
+				protocol: "TCP",
+				name:     "ethereum p2p",
+				port:     tcp.Port,
+			}
 		}
 	}
 
@@ -688,18 +676,17 @@ func (srv *Server) setupUDPListening() (*net.UDPConn, error) {
 	if err != nil {
 		return nil, err
 	}
-	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.loopWG.Done()
-			}()
+	laddr := conn.LocalAddr().(*net.UDPAddr)
+	srv.localnode.SetFallbackUDP(laddr.Port)
+	srv.log.Debug("UDP listener up", "addr", laddr)
+	if !laddr.IP.IsLoopback() && !laddr.IP.IsPrivate() {
+		srv.portMappingRegister <- &portMapping{
+			protocol: "UDP",
+			name:     "ethereum peer discovery",
+			port:     laddr.Port,
 		}
 	}
-	srv.localnode.SetFallbackUDP(realaddr.Port)
+
 	return conn, nil
 }