Consider dns as public address (#913)

libp2p/protocols/connectivity/autonat/service.nim

@@ -171,8 +171,7 @@ proc addressMapper(
   for listenAddr in listenAddrs:
     var processedMA = listenAddr
     try:
-      let hostIP = initTAddress(listenAddr).get()
+      if not listenAddr.isPublicMA() and self.networkReachability == NetworkReachability.Reachable:
-      if not hostIP.isGlobal() and self.networkReachability == NetworkReachability.Reachable:
         processedMA = peerStore.guessDialableAddr(listenAddr) # handle manual port forwarding
     except CatchableError as exc:
       debug "Error while handling address mapper", msg = exc.msg

libp2p/protocols/connectivity/dcutr/client.nim

@@ -81,8 +81,8 @@ proc startSync*(self: DcutrClient, switch: Switch, remotePeerId: PeerId, addrs:
     debug "Dcutr initiator could not connect to the remote peer, all connect attempts timed out", peerDialableAddrs, msg = err.msg
     raise newException(DcutrError, "Dcutr initiator could not connect to the remote peer, all connect attempts timed out", err)
   except CatchableError as err:
-    debug "Unexpected error when trying direct conn", err = err.msg
+    debug "Unexpected error when Dcutr initiator tried to connect to the remote peer", err = err.msg
-    raise newException(DcutrError, "Unexpected error when trying a direct conn", err)
+    raise newException(DcutrError, "Unexpected error when Dcutr initiator tried to connect to the remote peer", err)
   finally:
     if stream != nil:
       await stream.close()

libp2p/protocols/connectivity/dcutr/server.nim

@@ -72,8 +72,8 @@ proc new*(T: typedesc[Dcutr], switch: Switch, connectTimeout = 15.seconds, maxDi
       debug "Dcutr receiver could not connect to the remote peer, all connect attempts timed out", peerDialableAddrs, msg = err.msg
       raise newException(DcutrError, "Dcutr receiver could not connect to the remote peer, all connect attempts timed out", err)
     except CatchableError as err:
-      warn "Unexpected error in dcutr handler", msg = err.msg
+      warn "Unexpected error when Dcutr receiver tried to connect to the remote peer", msg = err.msg
-      raise newException(DcutrError, "Unexpected error in dcutr handler", err)
+      raise newException(DcutrError, "Unexpected error when Dcutr receiver tried to connect to the remote peer", err)
 
   let self = T()
   self.handler = handleStream

libp2p/services/hpservice.nim

@@ -30,13 +30,9 @@ type
     onNewStatusHandler: StatusAndConfidenceHandler
     autoRelayService: AutoRelayService
     autonatService: AutonatService
-    isPublicIPAddrProc: IsPublicIPAddrProc
 
-  IsPublicIPAddrProc* = proc(ta: TransportAddress): bool {.gcsafe, raises: [].}
+proc new*(T: typedesc[HPService], autonatService: AutonatService, autoRelayService: AutoRelayService): T =
-
+  return T(autonatService: autonatService, autoRelayService: autoRelayService)
-proc new*(T: typedesc[HPService], autonatService: AutonatService, autoRelayService: AutoRelayService,
-          isPublicIPAddrProc: IsPublicIPAddrProc = isGlobal): T =
-  return T(autonatService: autonatService, autoRelayService: autoRelayService, isPublicIPAddrProc: isPublicIPAddrProc)
 
 proc tryStartingDirectConn(self: HPService, switch: Switch, peerId: PeerId): Future[bool] {.async.} =
   proc tryConnect(address: MultiAddress): Future[bool] {.async.} =
@@ -51,12 +47,8 @@ proc tryStartingDirectConn(self: HPService, switch: Switch, peerId: PeerId): Fut
       let isRelayed = address.contains(multiCodec("p2p-circuit"))
       if isRelayed.isErr() or isRelayed.get():
         continue
-      if DNS.matchPartial(address):
+      if address.isPublicMA():
         return await tryConnect(address)
-      else:
-        let ta = initTAddress(address)
-        if ta.isOk() and self.isPublicIPAddrProc(ta.get()):
-          return await tryConnect(address)
     except CatchableError as err:
       debug "Failed to create direct connection.", err = err.msg
       continue

libp2p/wire.nim

@@ -20,14 +20,14 @@ else:
 
 const
   RTRANSPMA* = mapOr(
-    TCP_IP,
+    TCP,
-    WebSockets_IP,
+    WebSockets,
     UNIX
   )
 
   TRANSPMA* = mapOr(
     RTRANSPMA,
-    UDP_IP
+    UDP,
   )
 
 
@@ -37,7 +37,7 @@ proc initTAddress*(ma: MultiAddress): MaResult[TransportAddress] =
   ## MultiAddress must be wire address, e.g. ``{IP4, IP6, UNIX}/{TCP, UDP}``.
   ##
 
-  if TRANSPMA.match(ma):
+  if mapOr(TCP_IP, WebSockets_IP, UNIX, UDP_IP).match(ma):
     var pbuf: array[2, byte]
     let code = (?(?ma[0]).protoCode())
     if code == multiCodec("unix"):
@@ -213,3 +213,10 @@ proc getLocalAddress*(sock: AsyncFD): TransportAddress =
   if getsockname(SocketHandle(sock), cast[ptr SockAddr](addr saddr),
                  addr slen) == 0:
     fromSAddr(addr saddr, slen, result)
+
+proc isPublicMA*(ma: MultiAddress): bool =
+  if DNS.matchPartial(ma):
+    return true
+
+  let hostIP = initTAddress(ma).valueOr: return false
+  return hostIP.isGlobal()

tests/helpers.nim

@@ -10,10 +10,11 @@ import ../libp2p/stream/lpstream
 import ../libp2p/stream/chronosstream
 import ../libp2p/muxers/mplex/lpchannel
 import ../libp2p/protocols/secure/secure
+import ../libp2p/switch
+import ../libp2p/nameresolving/[nameresolver, mockresolver]
 
 import ./asyncunit
-export asyncunit
+export asyncunit, mockresolver
-
 
 const
   StreamTransportTrackerName = "stream.transport"
@@ -138,3 +139,15 @@ proc unorderedCompare*[T](a, b: seq[T]): bool =
     return true
 
   return false
+
+proc default*(T: typedesc[MockResolver]): T =
+  let resolver = MockResolver.new()
+  resolver.ipResponses[("localhost", false)] = @["127.0.0.1"]
+  resolver.ipResponses[("localhost", true)] = @["::1"]
+  resolver
+
+proc setDNSAddr*(switch: Switch) {.gcsafe, async.} =
+  proc addressMapper(listenAddrs: seq[MultiAddress]): Future[seq[MultiAddress]] {.gcsafe, async.} =
+      return @[MultiAddress.init("/dns4/localhost/").tryGet() & listenAddrs[0][1].tryGet()]
+  switch.peerInfo.addressMappers.add(addressMapper)
+  await switch.peerInfo.update()

tests/stubs/switchstub.nim

@@ -17,7 +17,14 @@ import ../../libp2p/[peerid, multiaddress, switch]
 type
   SwitchStub* = ref object of Switch
     switch*: Switch
-    connectStub*: proc(): Future[void] {.async.}
+    connectStub*: connectStubType
+
+  connectStubType* = proc (self: SwitchStub,
+                           peerId: PeerId,
+                           addrs: seq[MultiAddress],
+                           forceDial = false,
+                           reuseConnection = true,
+                           upgradeDir = Direction.Out): Future[void]  {.gcsafe, async.}
 
 method connect*(
  self: SwitchStub,
@@ -27,11 +34,11 @@ method connect*(
  reuseConnection = true,
  upgradeDir = Direction.Out) {.async.} =
   if (self.connectStub != nil):
-    await self.connectStub()
+    await self.connectStub(self, peerId, addrs, forceDial, reuseConnection, upgradeDir)
   else:
     await self.switch.connect(peerId, addrs, forceDial, reuseConnection, upgradeDir)
 
-proc new*(T: typedesc[SwitchStub], switch: Switch, connectStub: proc (): Future[void] {.async.} = nil): T =
+proc new*(T: typedesc[SwitchStub], switch: Switch, connectStub: connectStubType = nil): T =
   return SwitchStub(
     switch: switch,
     peerInfo: switch.peerInfo,

tests/testautonat.nim

@@ -107,13 +107,9 @@ suite "Autonat":
     await allFutures(doesNothingListener.stop(), src.stop(), dst.stop())
 
   asyncTest "dialMe dials dns and returns public address":
-    let resolver = MockResolver.new()
-    resolver.ipResponses[("localhost", false)] = @["127.0.0.1"]
-    resolver.ipResponses[("localhost", true)] = @["::1"]
-
     let
       src = newStandardSwitch()
-      dst = createAutonatSwitch(nameResolver = resolver)
+      dst = createAutonatSwitch(nameResolver = MockResolver.default())
 
     await src.start()
     await dst.start()

tests/testautonatservice.nim

@@ -16,10 +16,11 @@ import ../libp2p/[builders,
                   switch,
                   protocols/connectivity/autonat/client,
                   protocols/connectivity/autonat/service]
+import ../libp2p/nameresolving/[nameresolver, mockresolver]
 import ./helpers
 import stubs/autonatclientstub
 
-proc createSwitch(autonatSvc: Service = nil, withAutonat = true, maxConnsPerPeer = 1, maxConns = 100): Switch =
+proc createSwitch(autonatSvc: Service = nil, withAutonat = true, maxConnsPerPeer = 1, maxConns = 100, nameResolver: NameResolver = nil): Switch =
   var builder = SwitchBuilder.new()
     .withRng(newRng())
     .withAddresses(@[ MultiAddress.init("/ip4/0.0.0.0/tcp/0").tryGet() ])
@@ -35,6 +36,9 @@ proc createSwitch(autonatSvc: Service = nil, withAutonat = true, maxConnsPerPeer
   if autonatSvc != nil:
     builder = builder.withServices(@[autonatSvc])
 
+  if nameResolver != nil:
+    builder = builder.withNameResolver(nameResolver)
+
   return builder.build()
 
 suite "Autonat Service":
@@ -257,7 +261,9 @@ suite "Autonat Service":
     let autonatService = AutonatService.new(AutonatClient.new(), newRng(), some(1.seconds), maxQueueSize = 1)
 
     let switch1 = createSwitch(autonatService, maxConnsPerPeer = 0)
-    let switch2 = createSwitch(maxConnsPerPeer = 0)
+    await switch1.setDNSAddr()
+
+    let switch2 = createSwitch(maxConnsPerPeer = 0, nameResolver = MockResolver.default())
 
     let awaiter = newFuture[void]()
 

tests/testdcutr.nim

@@ -90,7 +90,12 @@ suite "Dcutr":
 
   asyncTest "Client connect timeout":
 
-     proc connectTimeoutProc(): Future[void] {.async.} =
+     proc connectTimeoutProc(self: SwitchStub,
+                             peerId: PeerId,
+                             addrs: seq[MultiAddress],
+                             forceDial = false,
+                             reuseConnection = true,
+                             upgradeDir = Direction.Out): Future[void] {.async.} =
        await sleepAsync(100.millis)
 
      let behindNATSwitch = SwitchStub.new(newStandardSwitch(), connectTimeoutProc)
@@ -104,7 +109,12 @@ suite "Dcutr":
 
   asyncTest "All client connect attempts fail":
 
-    proc connectErrorProc(): Future[void] {.async.} =
+    proc connectErrorProc(self: SwitchStub,
+                          peerId: PeerId,
+                          addrs: seq[MultiAddress],
+                          forceDial = false,
+                          reuseConnection = true,
+                          upgradeDir = Direction.Out): Future[void] {.async.} =
       raise newException(CatchableError, "error")
 
     let behindNATSwitch = SwitchStub.new(newStandardSwitch(), connectErrorProc)
@@ -116,7 +126,7 @@ suite "Dcutr":
       except DcutrError as err:
         check err.parent of AllFuturesFailedError
 
-  proc ductrServerTest(connectStub: proc (): Future[void] {.async.}) {.async.} =
+  proc ductrServerTest(connectStub: connectStubType) {.async.} =
     let behindNATSwitch = newStandardSwitch()
     let publicSwitch = SwitchStub.new(newStandardSwitch())
 
@@ -144,14 +154,24 @@ suite "Dcutr":
 
   asyncTest "DCUtR server timeout when establishing a new connection":
 
-    proc connectProc(): Future[void] {.async.} =
+    proc connectProc(self: SwitchStub,
+                     peerId: PeerId,
+                     addrs: seq[MultiAddress],
+                     forceDial = false,
+                     reuseConnection = true,
+                     upgradeDir = Direction.Out): Future[void] {.async.} =
         await sleepAsync(100.millis)
 
     await ductrServerTest(connectProc)
 
   asyncTest "DCUtR server error when establishing a new connection":
 
-    proc connectProc(): Future[void] {.async.} =
+    proc connectProc(self: SwitchStub,
+                     peerId: PeerId,
+                     addrs: seq[MultiAddress],
+                     forceDial = false,
+                     reuseConnection = true,
+                     upgradeDir = Direction.Out): Future[void] {.async.} =
         raise newException(CatchableError, "error")
 
     await ductrServerTest(connectProc)

tests/testhpservice.nim

@@ -18,18 +18,14 @@ import ./helpers
 import ./stubs/switchstub
 import ../libp2p/[builders,
                   switch,
+                  wire,
                   services/hpservice,
                   services/autorelayservice]
 import ../libp2p/protocols/connectivity/relay/[relay, client]
 import ../libp2p/protocols/connectivity/autonat/[service]
-import ../libp2p/nameresolving/nameresolver
+import ../libp2p/nameresolving/[nameresolver, mockresolver]
-import ../libp2p/nameresolving/mockresolver
-
 import stubs/autonatclientstub
 
-proc isPublicAddrIPAddrMock(ta: TransportAddress): bool =
-  return true
-
 proc createSwitch(r: Relay = nil, hpService: Service = nil, nameResolver: NameResolver = nil): Switch {.raises: [LPError].} =
   var builder = SwitchBuilder.new()
     .withRng(newRng())
@@ -68,19 +64,25 @@ suite "Hole Punching":
     let privatePeerRelayAddr = newFuture[seq[MultiAddress]]()
 
     let publicPeerSwitch = createSwitch(RelayClient.new())
+
+    proc addressMapper(listenAddrs: seq[MultiAddress]): Future[seq[MultiAddress]] {.gcsafe, async.} =
+        return @[MultiAddress.init("/dns4/localhost/").tryGet() & listenAddrs[0][1].tryGet()]
+    publicPeerSwitch.peerInfo.addressMappers.add(addressMapper)
+    await publicPeerSwitch.peerInfo.update()
+
     proc checkMA(address: seq[MultiAddress]) =
       if not privatePeerRelayAddr.completed():
         privatePeerRelayAddr.complete(address)
 
     let autoRelayService = AutoRelayService.new(1, relayClient, checkMA, newRng())
 
-    let hpservice = HPService.new(autonatService, autoRelayService, isPublicAddrIPAddrMock)
+    let hpservice = HPService.new(autonatService, autoRelayService)
 
-    let privatePeerSwitch = createSwitch(relayClient, hpservice)
+    let privatePeerSwitch = createSwitch(relayClient, hpservice, nameresolver = MockResolver.default())
     let peerSwitch = createSwitch()
     let switchRelay = createSwitch(Relay.new())
 
-    await allFutures(switchRelay.start(), privatePeerSwitch.start(), publicPeerSwitch.start(), peerSwitch.start())
+    await allFuturesThrowing(switchRelay.start(), privatePeerSwitch.start(), publicPeerSwitch.start(), peerSwitch.start())
 
     await privatePeerSwitch.connect(switchRelay.peerInfo.peerId, switchRelay.peerInfo.addrs)
     await privatePeerSwitch.connect(peerSwitch.peerInfo.peerId, peerSwitch.peerInfo.addrs) # for autonat
@@ -103,16 +105,8 @@ suite "Hole Punching":
     let relayClient = RelayClient.new()
     let privatePeerRelayAddr = newFuture[seq[MultiAddress]]()
 
-    let resolver = MockResolver.new()
+    let publicPeerSwitch = createSwitch(RelayClient.new())
-    resolver.ipResponses[("localhost", false)] = @["127.0.0.1"]
+    await publicPeerSwitch.setDNSAddr()
-    resolver.ipResponses[("localhost", true)] = @["::1"]
-
-    let publicPeerSwitch = createSwitch(RelayClient.new(), nameResolver = resolver)
-
-    proc addressMapper(listenAddrs: seq[MultiAddress]): Future[seq[MultiAddress]] {.gcsafe, async.} =
-        return @[MultiAddress.init("/dns4/localhost/").tryGet() & listenAddrs[0][1].tryGet()]
-    publicPeerSwitch.peerInfo.addressMappers.add(addressMapper)
-    await publicPeerSwitch.peerInfo.update()
 
     proc checkMA(address: seq[MultiAddress]) =
       if not privatePeerRelayAddr.completed():
@@ -120,13 +114,13 @@ suite "Hole Punching":
 
     let autoRelayService = AutoRelayService.new(1, relayClient, checkMA, newRng())
 
-    let hpservice = HPService.new(autonatService, autoRelayService, isPublicAddrIPAddrMock)
+    let hpservice = HPService.new(autonatService, autoRelayService)
 
-    let privatePeerSwitch = createSwitch(relayClient, hpservice, nameResolver = resolver)
+    let privatePeerSwitch = createSwitch(relayClient, hpservice, nameResolver = MockResolver.default())
     let peerSwitch = createSwitch()
     let switchRelay = createSwitch(Relay.new())
 
-    await allFutures(switchRelay.start(), privatePeerSwitch.start(), publicPeerSwitch.start(), peerSwitch.start())
+    await allFuturesThrowing(switchRelay.start(), privatePeerSwitch.start(), publicPeerSwitch.start(), peerSwitch.start())
 
     await privatePeerSwitch.connect(switchRelay.peerInfo.peerId, switchRelay.peerInfo.addrs)
     await privatePeerSwitch.connect(peerSwitch.peerInfo.peerId, peerSwitch.peerInfo.addrs) # for autonat
@@ -140,9 +134,7 @@ suite "Hole Punching":
     await allFuturesThrowing(
       privatePeerSwitch.stop(), publicPeerSwitch.stop(), switchRelay.stop(), peerSwitch.stop())
 
-  proc holePunchingTest(connectStub: proc (): Future[void] {.async.},
+  proc holePunchingTest(initiatorConnectStub: connectStubType, rcvConnectStub: connectStubType, answer: Answer) {.async.} =
-                        isPublicIPAddrProc: IsPublicIPAddrProc,
-                        answer: Answer) {.async.} =
     # There's no check in this test cause it can't test hole punching locally. It exists just to be sure the rest of
     # the code works properly.
 
@@ -165,11 +157,12 @@ suite "Hole Punching":
     let autoRelayService1 = AutoRelayService.new(1, relayClient1, checkMA, newRng())
     let autoRelayService2 = AutoRelayService.new(1, relayClient2, nil, newRng())
 
-    let hpservice1 = HPService.new(autonatService1, autoRelayService1, isPublicIPAddrProc)
+    let hpservice1 = HPService.new(autonatService1, autoRelayService1)
     let hpservice2 = HPService.new(autonatService2, autoRelayService2)
 
-    let privatePeerSwitch1 = SwitchStub.new(createSwitch(relayClient1, hpservice1))
+    let privatePeerSwitch1 = SwitchStub.new(createSwitch(relayClient1, hpservice1, nameresolver = MockResolver.default()))
-    let privatePeerSwitch2 = createSwitch(relayClient2, hpservice2)
+    let privatePeerSwitch2 = SwitchStub.new(createSwitch(relayClient2, hpservice2))
+    await privatePeerSwitch2.setDNSAddr()
     let switchRelay = createSwitch(Relay.new())
     let switchAux = createSwitch()
     let switchAux2 = createSwitch()
@@ -178,7 +171,7 @@ suite "Hole Punching":
 
     var awaiter = newFuture[void]()
 
-    await allFutures(
+    await allFuturesThrowing(
       switchRelay.start(), privatePeerSwitch1.start(), privatePeerSwitch2.start(),
       switchAux.start(), switchAux2.start(), switchAux3.start(), switchAux4.start()
     )
@@ -196,21 +189,43 @@ suite "Hole Punching":
     await privatePeerSwitch2.connect(switchAux3.peerInfo.peerId, switchAux3.peerInfo.addrs)
     await privatePeerSwitch2.connect(switchAux4.peerInfo.peerId, switchAux4.peerInfo.addrs)
 
-    privatePeerSwitch1.connectStub = connectStub
+    privatePeerSwitch1.connectStub = initiatorConnectStub
     await privatePeerSwitch2.connect(privatePeerSwitch1.peerInfo.peerId, (await privatePeerRelayAddr1))
+    privatePeerSwitch2.connectStub = rcvConnectStub
 
-    await sleepAsync(200.millis)
+    checkExpiring:
+      # we can't hole punch when both peers are in the same machine. This means that the simultaneous dialings will result
+      # in two connections attemps, instead of one. The server dial is going to fail because it is acting as the
+      # tcp simultaneous incoming upgrader in the dialer which works only in the simultaneous open case, but the client
+      # dial will succeed.
+      privatePeerSwitch1.connManager.connCount(privatePeerSwitch2.peerInfo.peerId) == 1 and
+        not isRelayed(privatePeerSwitch1.connManager.selectMuxer(privatePeerSwitch2.peerInfo.peerId).connection)
 
     await allFuturesThrowing(
       privatePeerSwitch1.stop(), privatePeerSwitch2.stop(), switchRelay.stop(),
       switchAux.stop(), switchAux2.stop(), switchAux3.stop(), switchAux4.stop())
 
   asyncTest "Hole punching when peers addresses are private":
-    await holePunchingTest(nil, isGlobal, NotReachable)
+    proc connectStub(self: SwitchStub,
+                    peerId: PeerId,
+                    addrs: seq[MultiAddress],
+                    forceDial = false,
+                    reuseConnection = true,
+                    upgradeDir = Direction.Out): Future[void] {.async.} =
+      self.connectStub = nil # this stub should be called only once
+      await sleepAsync(100.millis) # avoid simultaneous dialing that causes address in use error
+      await self.switch.connect(peerId, addrs, forceDial, reuseConnection, upgradeDir)
+    await holePunchingTest(nil, connectStub, NotReachable)
 
   asyncTest "Hole punching when there is an error during unilateral direct connection":
 
-    proc connectStub(): Future[void] {.async.} =
+    proc connectStub(self: SwitchStub,
+                    peerId: PeerId,
+                    addrs: seq[MultiAddress],
+                    forceDial = false,
+                    reuseConnection = true,
+                    upgradeDir = Direction.Out): Future[void] {.async.} =
+      self.connectStub = nil # this stub should be called only once
       raise newException(CatchableError, "error")
 
-    await holePunchingTest(connectStub, isPublicAddrIPAddrMock, Reachable)
+    await holePunchingTest(connectStub, nil, Reachable)