From 9f74385d6e1321f2505db971c124e61897ae5c7b Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Tue, 5 Mar 2019 13:58:46 -0800 Subject: [PATCH] remove all uses of multiaddrs NATs only care about TCP/UDP and ports. Using multiaddrs here made this library really hard to work with correctly. Furthermore, this library doesn't _actually_ support specifying the internal IP address. However, we'd still _act_ like the internal IP address mattered. This caused all sorts of mismatches. --- go.mod | 2 - go.sum | 21 ------ mapping.go | 81 +++++++++-------------- nat.go | 184 ++++++++++++---------------------------------------- notifier.go | 2 +- 5 files changed, 71 insertions(+), 219 deletions(-) diff --git a/go.mod b/go.mod index ed3c6f5..009c0bd 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,5 @@ require ( github.com/ipfs/go-log v0.0.1 github.com/jbenet/go-cienv v0.0.0-20150120210510-1bb1476777ec // indirect github.com/jbenet/goprocess v0.0.0-20160826012719-b497e2f366b8 - github.com/multiformats/go-multiaddr v0.0.1 - github.com/multiformats/go-multiaddr-net v0.0.1 github.com/whyrusleeping/go-notifier v0.0.0-20170827234753-097c5d47330f ) diff --git a/go.sum b/go.sum index d7b5af6..377167e 100644 --- a/go.sum +++ b/go.sum @@ -4,10 +4,6 @@ github.com/fd/go-nat v1.0.0 h1:DPyQ97sxA9ThrWYRPcWUz/z9TnpTIGRYODIQc/dy64M= github.com/fd/go-nat v1.0.0/go.mod h1:BTBu/CKvMmOMUPkKVef1pngt2WFH/lg7E6yQnulfp6E= github.com/gogo/protobuf v1.2.1 h1:/s5zKNz0uPFCZ5hddgPdo2TK2TVrUNMn0OOX8/aZMTE= github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4= -github.com/gxed/hashland/keccakpg v0.0.1 h1:wrk3uMNaMxbXiHibbPO4S0ymqJMm41WiudyFSs7UnsU= -github.com/gxed/hashland/keccakpg v0.0.1/go.mod h1:kRzw3HkwxFU1mpmPP8v1WyQzwdGfmKFJ6tItnhQ67kU= -github.com/gxed/hashland/murmur3 v0.0.1 h1:SheiaIt0sda5K+8FLz952/1iWS9zrnKsEJaOJu4ZbSc= -github.com/gxed/hashland/murmur3 v0.0.1/go.mod h1:KjXop02n4/ckmZSnY2+HKcLud/tcmvhST0bie/0lS48= github.com/huin/goupnp v0.0.0-20180415215157-1395d1447324 h1:PV190X5/DzQ/tbFFG5YpT5mH6q+cHlfgqI5JuRnH9oE= github.com/huin/goupnp v0.0.0-20180415215157-1395d1447324/go.mod h1:MZ2ZmwcBpvOoJ22IJsc7va19ZwoheaBk43rKg12SKag= github.com/ipfs/go-log v0.0.1 h1:9XTUN/rW64BCG1YhPK9Hoy3q8nr4gOmHHBpgFdfw6Lc= @@ -26,20 +22,6 @@ github.com/mattn/go-colorable v0.1.1 h1:G1f5SKeVxmagw/IyvzvtZE4Gybcc4Tr1tf7I8z0X github.com/mattn/go-colorable v0.1.1/go.mod h1:FuOcm+DKB9mbwrcAfNl7/TZVBZ6rcnceauSikq3lYCQ= github.com/mattn/go-isatty v0.0.5 h1:tHXDdz1cpzGaovsTB+TVB8q90WEokoVmfMqoVcrLUgw= github.com/mattn/go-isatty v0.0.5/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= -github.com/minio/blake2b-simd v0.0.0-20160723061019-3f5f724cb5b1 h1:lYpkrQH5ajf0OXOcUbGjvZxxijuBwbbmlSxLiuofa+g= -github.com/minio/blake2b-simd v0.0.0-20160723061019-3f5f724cb5b1/go.mod h1:pD8RvIylQ358TN4wwqatJ8rNavkEINozVn9DtGI3dfQ= -github.com/minio/sha256-simd v0.0.0-20190131020904-2d45a736cd16 h1:5W7KhL8HVF3XCFOweFD3BNESdnO8ewyYTFT2R+/b8FQ= -github.com/minio/sha256-simd v0.0.0-20190131020904-2d45a736cd16/go.mod h1:2FMWW+8GMoPweT6+pI63m9YE3Lmw4J71hV56Chs1E/U= -github.com/mr-tron/base58 v1.1.0 h1:Y51FGVJ91WBqCEabAi5OPUz38eAx8DakuAm5svLcsfQ= -github.com/mr-tron/base58 v1.1.0/go.mod h1:xcD2VGqlgYjBdcBLw+TuYLr8afG+Hj8g2eTVqeSzSU8= -github.com/multiformats/go-multiaddr v0.0.1 h1:/QUV3VBMDI6pi6xfiw7lr6xhDWWvQKn9udPn68kLSdY= -github.com/multiformats/go-multiaddr v0.0.1/go.mod h1:xKVEak1K9cS1VdmPZW3LSIb6lgmoS58qz/pzqmAxV44= -github.com/multiformats/go-multiaddr-dns v0.0.1 h1:jQt9c6tDSdQLIlBo4tXYx7QUHCPjxsB1zXcag/2S7zc= -github.com/multiformats/go-multiaddr-dns v0.0.1/go.mod h1:9kWcqw/Pj6FwxAwW38n/9403szc57zJPs45fmnznu3Q= -github.com/multiformats/go-multiaddr-net v0.0.1 h1:76O59E3FavvHqNg7jvzWzsPSW5JSi/ek0E4eiDVbg9g= -github.com/multiformats/go-multiaddr-net v0.0.1/go.mod h1:nw6HSxNmCIQH27XPGBuX+d1tnvM7ihcFwHMSstNAVUU= -github.com/multiformats/go-multihash v0.0.1 h1:HHwN1K12I+XllBCrqKnhX949Orn4oawPkegHMu2vDqQ= -github.com/multiformats/go-multihash v0.0.1/go.mod h1:w/5tugSrLEbWqlcgJabL3oHFKTwfvkofsjW2Qa1ct4U= github.com/opentracing/opentracing-go v1.0.2 h1:3jA2P6O1F9UOrWVpwrIo17pu01KWvNWg4X946/Y5Zwg= github.com/opentracing/opentracing-go v1.0.2/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -51,12 +33,9 @@ github.com/whyrusleeping/go-logging v0.0.0-20170515211332-0457bb6b88fc h1:9lDbC6 github.com/whyrusleeping/go-logging v0.0.0-20170515211332-0457bb6b88fc/go.mod h1:bopw91TMyo8J3tvftk8xmU2kPmlrt4nScJQZU2hE5EM= github.com/whyrusleeping/go-notifier v0.0.0-20170827234753-097c5d47330f h1:M/lL30eFZTKnomXY6huvM6G0+gVquFNf6mxghaWlFUg= github.com/whyrusleeping/go-notifier v0.0.0-20170827234753-097c5d47330f/go.mod h1:cZNvX9cFybI01GriPRMXDtczuvUhgbcYr9iCGaNlRv8= -golang.org/x/crypto v0.0.0-20190211182817-74369b46fc67 h1:ng3VDlRp5/DHpSWl02R4rM9I+8M2rhmsuLwAMmkLQWE= -golang.org/x/crypto v0.0.0-20190211182817-74369b46fc67/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/net v0.0.0-20180524181706-dfa909b99c79/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190227160552-c95aed5357e7 h1:C2F/nMkR/9sfUTpvR3QrjBuTdvMUC/cFajkphs1YLQo= golang.org/x/net v0.0.0-20190227160552-c95aed5357e7/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= -golang.org/x/sys v0.0.0-20190219092855-153ac476189d/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223 h1:DH4skfRX4EBpamg7iV4ZlCpblAHI6s6TDM39bFZumv8= golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= diff --git a/mapping.go b/mapping.go index b03b002..33f25f6 100644 --- a/mapping.go +++ b/mapping.go @@ -2,12 +2,11 @@ package nat import ( "fmt" + "net" "sync" "time" "github.com/jbenet/goprocess" - ma "github.com/multiformats/go-multiaddr" - manet "github.com/multiformats/go-multiaddr-net" ) // Mapping represents a port mapping in a NAT. @@ -27,12 +26,9 @@ type Mapping interface { // established, port will be 0 ExternalPort() int - // InternalAddr returns the internal address. - InternalAddr() ma.Multiaddr - // ExternalAddr returns the external facing address. If the mapping is not // established, addr will be nil, and and ErrNoMapping will be returned. - ExternalAddr() (addr ma.Multiaddr, err error) + ExternalAddr() (addr net.Addr, err error) // Close closes the port mapping Close() error @@ -47,12 +43,9 @@ type mapping struct { intport int extport int permanent bool - intaddr ma.Multiaddr proc goprocess.Process - comment string - - cached ma.Multiaddr + cached net.IP cacheTime time.Time cacheLk sync.Mutex } @@ -87,55 +80,41 @@ func (m *mapping) setExternalPort(p int) { m.extport = p } -func (m *mapping) InternalAddr() ma.Multiaddr { - m.Lock() - defer m.Unlock() - return m.intaddr -} - -func (m *mapping) ExternalAddr() (ma.Multiaddr, error) { +func (m *mapping) ExternalAddr() (net.Addr, error) { m.cacheLk.Lock() - ctime := m.cacheTime - cval := m.cached - m.cacheLk.Unlock() - if time.Since(ctime) < CacheTime { - return cval, nil - } - - if m.ExternalPort() == 0 { // dont even try right now. + defer m.cacheLk.Unlock() + oport := m.ExternalPort() + if oport == 0 { + // dont even try right now. return nil, ErrNoMapping } - m.nat.natmu.Lock() - ip, err := m.nat.nat.GetExternalAddress() - m.nat.natmu.Unlock() - if err != nil { - return nil, err - } + if time.Since(m.cacheTime) >= CacheTime { + m.nat.natmu.Lock() + cval, err := m.nat.nat.GetExternalAddress() + m.nat.natmu.Unlock() - ipmaddr, err := manet.FromIP(ip) - if err != nil { - return nil, fmt.Errorf("error parsing ip") - } + if err != nil { + return nil, err + } - // call m.ExternalPort again, as mapping may have changed under our feet. (tocttou) - extport := m.ExternalPort() - if extport == 0 { - return nil, ErrNoMapping + m.cached = cval + m.cacheTime = time.Now() } - - tcp, err := ma.NewMultiaddr(fmt.Sprintf("/%s/%d", m.Protocol(), extport)) - if err != nil { - return nil, err + switch m.Protocol() { + case "tcp": + return &net.TCPAddr{ + IP: m.cached, + Port: oport, + }, nil + case "udp": + return &net.UDPAddr{ + IP: m.cached, + Port: oport, + }, nil + default: + panic(fmt.Sprintf("invalid protocol %q", m.Protocol())) } - - maddr2 := ipmaddr.Encapsulate(tcp) - - m.cacheLk.Lock() - m.cached = maddr2 - m.cacheTime = time.Now() - m.cacheLk.Unlock() - return maddr2, nil } func (m *mapping) Close() error { diff --git a/nat.go b/nat.go index 0c47a4c..938d115 100644 --- a/nat.go +++ b/nat.go @@ -1,11 +1,9 @@ package nat import ( + "context" "errors" "fmt" - "net" - "strconv" - "strings" "sync" "time" @@ -13,8 +11,6 @@ import ( logging "github.com/ipfs/go-log" goprocess "github.com/jbenet/goprocess" periodic "github.com/jbenet/goprocess/periodic" - ma "github.com/multiformats/go-multiaddr" - manet "github.com/multiformats/go-multiaddr-net" ) var ( @@ -33,19 +29,38 @@ const CacheTime = time.Second * 15 // DiscoverNAT looks for a NAT device in the network and // returns an object that can manage port mappings. -func DiscoverNAT() *NAT { - nat, err := nat.DiscoverGateway() +func DiscoverNAT(ctx context.Context) (*NAT, error) { + var ( + natInstance nat.NAT + err error + ) + + done := make(chan struct{}) + go func() { + defer close(done) + // This will abort in 10 seconds anyways. + natInstance, err = nat.DiscoverGateway() + }() + + select { + case <-done: + case <-ctx.Done(): + return nil, ctx.Err() + } + if err != nil { - log.Debug("DiscoverGateway error:", err) - return nil + return nil, err } - addr, err := nat.GetDeviceAddress() + + // Log the device addr. + addr, err := natInstance.GetDeviceAddress() if err != nil { log.Debug("DiscoverGateway address error:", err) } else { log.Debug("DiscoverGateway address:", addr) } - return newNAT(nat) + + return newNAT(natInstance), nil } // NAT is an object that manages address port mappings in @@ -55,7 +70,7 @@ func DiscoverNAT() *NAT { type NAT struct { natmu sync.Mutex nat nat.NAT - proc goprocess.Process // manages nat mappings lifecycle + proc goprocess.Process mappingmu sync.RWMutex // guards mappings mappings map[*mapping]struct{} @@ -116,66 +131,31 @@ func (nat *NAT) rmMapping(m *mapping) { // NAT devices may not respect our port requests, and even lie. // Clients should not store the mapped results, but rather always // poll our object for the latest mappings. -func (nat *NAT) NewMapping(maddr ma.Multiaddr) (Mapping, error) { +func (nat *NAT) NewMapping(protocol string, port int) (Mapping, error) { if nat == nil { return nil, fmt.Errorf("no nat available") } - network, addr, err := manet.DialArgs(maddr) - if err != nil { - return nil, fmt.Errorf("DialArgs failed on addr: %s", maddr.String()) - } - - var ip net.IP - switch network { - case "tcp", "tcp4", "tcp6": - addr, err := net.ResolveTCPAddr(network, addr) - if err != nil { - return nil, err - } - ip = addr.IP - network = "tcp" - case "udp", "udp4", "udp6": - addr, err := net.ResolveUDPAddr(network, addr) - if err != nil { - return nil, err - } - ip = addr.IP - network = "udp" + switch protocol { + case "tcp", "udp": default: - return nil, fmt.Errorf("transport not supported by NAT: %s", network) - } - - // XXX: Known limitation: doesn't handle multiple internal addresses. - // If this applies to you, you can figure it out yourself. Ideally, the - // NAT library would allow us to handle this case but the "go way" - // appears to be to just "shrug" at edge-cases. - if !ip.IsUnspecified() { - internalAddr, err := nat.nat.GetInternalAddress() - if err != nil { - return nil, fmt.Errorf("failed to discover address on nat: %s", err) - } - if !ip.Equal(internalAddr) { - return nil, fmt.Errorf("nat address is %s, refusing to map %s", internalAddr, ip) - } - } - - intports := strings.Split(addr, ":")[1] - intport, err := strconv.Atoi(intports) - if err != nil { - return nil, err + return nil, fmt.Errorf("invalid protocol: %s", protocol) } m := &mapping{ + intport: port, nat: nat, - proto: network, - intport: intport, - intaddr: maddr, + proto: protocol, } + m.proc = goprocess.WithTeardown(func() error { nat.rmMapping(m) + nat.natmu.Lock() + defer nat.natmu.Unlock() + nat.nat.DeletePortMapping(m.Protocol(), m.InternalPort()) return nil }) + nat.addMapping(m) m.proc.AddChild(periodic.Every(MappingDuration/3, func(worker goprocess.Process) { @@ -193,9 +173,6 @@ func (nat *NAT) establishMapping(m *mapping) { log.Debugf("Attempting port map: %s/%d", m.Protocol(), m.InternalPort()) comment := "libp2p" - if m.comment != "" { - comment = "libp2p-" + m.comment - } nat.natmu.Lock() newport, err := nat.nat.AddPortMapping(m.Protocol(), m.InternalPort(), comment, MappingDuration) @@ -205,7 +182,7 @@ func (nat *NAT) establishMapping(m *mapping) { } nat.natmu.Unlock() - failure := func() { + if err != nil || newport == 0 { m.setExternalPort(0) // clear mapping // TODO: log.Event log.Warningf("failed to establish port mapping: %s", err) @@ -215,22 +192,11 @@ func (nat *NAT) establishMapping(m *mapping) { // we do not close if the mapping failed, // because it may work again next time. - } - - if err != nil || newport == 0 { - failure() return } m.setExternalPort(newport) - ext, err := m.ExternalAddr() - if err != nil { - log.Debugf("NAT Mapping addr error: %s %s", m.InternalAddr(), err) - failure() - return - } - - log.Debugf("NAT Mapping: %s --> %s", m.InternalAddr(), ext) + log.Debugf("NAT Mapping: %s --> %s (%s)", m.ExternalPort(), m.InternalPort(), m.Protocol()) if oldport != 0 && newport != oldport { log.Debugf("failed to renew same port mapping: ch %d -> %d", oldport, newport) nat.Notifier.notifyAll(func(n Notifiee) { @@ -242,73 +208,3 @@ func (nat *NAT) establishMapping(m *mapping) { n.MappingSuccess(nat, m) }) } - -// PortMapAddrs attempts to open (and continue to keep open) -// port mappings for given addrs. This function blocks until -// all addresses have been tried. This allows clients to -// retrieve results immediately after: -// -// nat.PortMapAddrs(addrs) -// mapped := nat.ExternalAddrs() -// -// Some may not succeed, and mappings may change over time; -// NAT devices may not respect our port requests, and even lie. -// Clients should not store the mapped results, but rather always -// poll our object for the latest mappings. -func (nat *NAT) PortMapAddrs(addrs []ma.Multiaddr) { - // spin off addr mappings independently. - var wg sync.WaitGroup - for _, addr := range addrs { - // do all of them concurrently - wg.Add(1) - go func(addr ma.Multiaddr) { - defer wg.Done() - nat.NewMapping(addr) - }(addr) - } - wg.Wait() -} - -// MappedAddrs returns address mappings NAT believes have been -// successfully established. Unsuccessful mappings are nil. This is: -// -// map[internalAddr]externalAddr -// -// This set of mappings _may not_ be correct, as NAT devices are finicky. -// Consider this with _best effort_ semantics. -func (nat *NAT) MappedAddrs() map[ma.Multiaddr]ma.Multiaddr { - - mappings := nat.Mappings() - addrmap := make(map[ma.Multiaddr]ma.Multiaddr, len(mappings)) - - for _, m := range mappings { - i := m.InternalAddr() - e, err := m.ExternalAddr() - if err != nil { - addrmap[i] = nil - } else { - addrmap[i] = e - } - } - return addrmap -} - -// ExternalAddrs returns a list of addresses that NAT believes have -// been successfully established. Unsuccessful mappings are omitted, -// so nat.ExternalAddrs() may return less addresses than nat.InternalAddrs(). -// To see which addresses are mapped, use nat.MappedAddrs(). -// -// This set of mappings _may not_ be correct, as NAT devices are finicky. -// Consider this with _best effort_ semantics. -func (nat *NAT) ExternalAddrs() []ma.Multiaddr { - mappings := nat.Mappings() - addrs := make([]ma.Multiaddr, 0, len(mappings)) - for _, m := range mappings { - a, err := m.ExternalAddr() - if err != nil { - continue // this mapping not currently successful. - } - addrs = append(addrs, a) - } - return addrs -} diff --git a/notifier.go b/notifier.go index 462a78b..10fb6ac 100644 --- a/notifier.go +++ b/notifier.go @@ -36,7 +36,7 @@ type Notifiee interface { // Called when mapping a port succeeds, but the mapping is // with a different port than an earlier success. - MappingChanged(nat *NAT, m Mapping, oldport int, newport int) + MappingChanged(nat *NAT, m Mapping, oldport, newport int) // Called when a port mapping fails. NAT will continue attempting after // the next period. To stop trying, use: mapping.Close(). After this failure,