From 67711478ce7c8bf9af804b6e17b6b2782b966f29 Mon Sep 17 00:00:00 2001 From: diegomrsantos Date: Tue, 13 Jun 2023 17:58:41 +0200 Subject: [PATCH] Consider dns as public address (#913) --- .../connectivity/autonat/service.nim | 3 +- .../protocols/connectivity/dcutr/client.nim | 4 +- .../protocols/connectivity/dcutr/server.nim | 4 +- libp2p/services/hpservice.nim | 14 +--- libp2p/wire.nim | 15 +++- tests/helpers.nim | 17 +++- tests/stubs/switchstub.nim | 13 ++- tests/testautonat.nim | 6 +- tests/testautonatservice.nim | 10 ++- tests/testdcutr.nim | 30 +++++-- tests/testhpservice.nim | 83 +++++++++++-------- 11 files changed, 127 insertions(+), 72 deletions(-) diff --git a/libp2p/protocols/connectivity/autonat/service.nim b/libp2p/protocols/connectivity/autonat/service.nim index a06429c7d..33a7b20e9 100644 --- a/libp2p/protocols/connectivity/autonat/service.nim +++ b/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 hostIP.isGlobal() and self.networkReachability == NetworkReachability.Reachable: + if not listenAddr.isPublicMA() 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 diff --git a/libp2p/protocols/connectivity/dcutr/client.nim b/libp2p/protocols/connectivity/dcutr/client.nim index a604a4829..cda04c72a 100644 --- a/libp2p/protocols/connectivity/dcutr/client.nim +++ b/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 - raise newException(DcutrError, "Unexpected error when trying a direct conn", err) + debug "Unexpected error when Dcutr initiator tried to connect to the remote peer", err = err.msg + raise newException(DcutrError, "Unexpected error when Dcutr initiator tried to connect to the remote peer", err) finally: if stream != nil: await stream.close() diff --git a/libp2p/protocols/connectivity/dcutr/server.nim b/libp2p/protocols/connectivity/dcutr/server.nim index 1a9bd92ae..4b5a032a4 100644 --- a/libp2p/protocols/connectivity/dcutr/server.nim +++ b/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 - raise newException(DcutrError, "Unexpected error in dcutr handler", err) + warn "Unexpected error when Dcutr receiver tried to connect to the remote peer", msg = err.msg + raise newException(DcutrError, "Unexpected error when Dcutr receiver tried to connect to the remote peer", err) let self = T() self.handler = handleStream diff --git a/libp2p/services/hpservice.nim b/libp2p/services/hpservice.nim index 60c943474..ce1e4522e 100644 --- a/libp2p/services/hpservice.nim +++ b/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, - isPublicIPAddrProc: IsPublicIPAddrProc = isGlobal): T = - return T(autonatService: autonatService, autoRelayService: autoRelayService, isPublicIPAddrProc: isPublicIPAddrProc) +proc new*(T: typedesc[HPService], autonatService: AutonatService, autoRelayService: AutoRelayService): T = + return T(autonatService: autonatService, autoRelayService: autoRelayService) 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 diff --git a/libp2p/wire.nim b/libp2p/wire.nim index 86f5f74f5..030fa310e 100644 --- a/libp2p/wire.nim +++ b/libp2p/wire.nim @@ -20,14 +20,14 @@ else: const RTRANSPMA* = mapOr( - TCP_IP, - WebSockets_IP, + TCP, + 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() diff --git a/tests/helpers.nim b/tests/helpers.nim index b4c27de45..84b2d2583 100644 --- a/tests/helpers.nim +++ b/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() diff --git a/tests/stubs/switchstub.nim b/tests/stubs/switchstub.nim index 2accd2cd8..e93065b01 100644 --- a/tests/stubs/switchstub.nim +++ b/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, diff --git a/tests/testautonat.nim b/tests/testautonat.nim index 738f15921..6217d5e0e 100644 --- a/tests/testautonat.nim +++ b/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() diff --git a/tests/testautonatservice.nim b/tests/testautonatservice.nim index 276777179..969d24f83 100644 --- a/tests/testautonatservice.nim +++ b/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]() diff --git a/tests/testdcutr.nim b/tests/testdcutr.nim index 8cb7e946c..83822e872 100644 --- a/tests/testdcutr.nim +++ b/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) diff --git a/tests/testhpservice.nim b/tests/testhpservice.nim index 2afe73994..4f6c7fb5c 100644 --- a/tests/testhpservice.nim +++ b/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/mockresolver - +import ../libp2p/nameresolving/[nameresolver, 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() - resolver.ipResponses[("localhost", false)] = @["127.0.0.1"] - 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() + let publicPeerSwitch = createSwitch(RelayClient.new()) + await publicPeerSwitch.setDNSAddr() 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.}, - isPublicIPAddrProc: IsPublicIPAddrProc, - answer: Answer) {.async.} = + proc holePunchingTest(initiatorConnectStub: connectStubType, rcvConnectStub: connectStubType, 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 privatePeerSwitch2 = createSwitch(relayClient2, hpservice2) + let privatePeerSwitch1 = SwitchStub.new(createSwitch(relayClient1, hpservice1, nameresolver = MockResolver.default())) + 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)