diff --git a/storage/nat.nim b/storage/nat.nim index 2f87693f..b45cba0d 100644 --- a/storage/nat.nim +++ b/storage/nat.nim @@ -62,6 +62,31 @@ proc resetMappings(m: NatPortMapper) = m.activeTcpPort = none(Port) m.activeUdpPort = none(Port) +# libplum seams, extracted as methods so tests can override them without I/O. + +method initPlum*(m: NatPortMapper): Result[void, string] {.base, gcsafe.} = + let plumLogLevel = + if getEnv("DEBUG") == "1": PlumLogLevel.Verbose else: PlumLogLevel.None + init( + logLevel = plumLogLevel, + discoverTimeout = m.discoverTimeout.int32, + mappingTimeout = m.mappingTimeout.int32, + recheckPeriod = m.recheckPeriod.int32, + ) + +method createMappingFor*( + m: NatPortMapper, protocol: PlumProtocol, port: uint16 +): Future[Result[MappingResult, string]] {. + base, async: (raises: [CancelledError]), gcsafe +.} = + await createMapping(protocol, port, port) + +method destroyMappingFor*(m: NatPortMapper, id: cint) {.base, gcsafe.} = + destroyMapping(id) + +method hasLiveMapping*(m: NatPortMapper, id: cint): bool {.base, gcsafe.} = + hasMapping(id) + method mapNatPorts*( m: NatPortMapper ): Future[Option[(Port, Port, MappingProtocol)]] {. @@ -71,20 +96,12 @@ method mapNatPorts*( return none((Port, Port, MappingProtocol)) # If both mappings are still active, return the stored ports without recreating. - if m.tcpMappingId.isSome and hasMapping(m.tcpMappingId.get) and m.udpMappingId.isSome and - hasMapping(m.udpMappingId.get): + if m.tcpMappingId.isSome and m.hasLiveMapping(m.tcpMappingId.get) and + m.udpMappingId.isSome and m.hasLiveMapping(m.udpMappingId.get): return some((m.activeTcpPort.get, m.activeUdpPort.get, m.activeMappingProtocol.get)) if not m.plumInitialized: - # 5s matches the old NatPortMappingTimeout used with miniupnpc/libnatpmp. - let plumLogLevel = - if getEnv("DEBUG") == "1": PlumLogLevel.Verbose else: PlumLogLevel.None - let res = init( - logLevel = plumLogLevel, - discoverTimeout = m.discoverTimeout.int32, - mappingTimeout = m.mappingTimeout.int32, - recheckPeriod = m.recheckPeriod.int32, - ) + let res = m.initPlum() if res.isErr: warn "Failed to initialize plum", msg = res.error return none((Port, Port, MappingProtocol)) @@ -94,15 +111,15 @@ method mapNatPorts*( # so we delete the mappings to recreate them. m.resetMappings() - let tcpRes = await createMapping(TCP, m.tcpPort.uint16, m.tcpPort.uint16) + let tcpRes = await m.createMappingFor(TCP, m.tcpPort.uint16) if tcpRes.isErr: warn "TCP port mapping failed", msg = tcpRes.error return none((Port, Port, MappingProtocol)) - let udpRes = await createMapping(UDP, m.discoveryPort.uint16, m.discoveryPort.uint16) + let udpRes = await m.createMappingFor(UDP, m.discoveryPort.uint16) if udpRes.isErr: warn "UDP port mapping failed", msg = udpRes.error - destroyMapping(tcpRes.value.id) + m.destroyMappingFor(tcpRes.value.id) return none((Port, Port, MappingProtocol)) m.tcpMappingId = some(tcpRes.value.id) diff --git a/tests/storage/testnatreaction.nim b/tests/storage/testnatreaction.nim index fe265285..728bbcbb 100644 --- a/tests/storage/testnatreaction.nim +++ b/tests/storage/testnatreaction.nim @@ -1,4 +1,4 @@ -import std/[net] +import std/[importutils, net] import pkg/chronos import pkg/libp2p/[multiaddress, multihash, multicodec] import pkg/libp2p/protocols/connectivity/autonat/types @@ -29,6 +29,36 @@ method mapNatPorts*( method hasMappingIds*(m: MockNatPortMapper): bool = m.activeMapping +type MockMapNatPortMapper = ref object of NatPortMapper + tcpResult: Result[MappingResult, string] + udpResult: Result[MappingResult, string] + live: bool + created: seq[PlumProtocol] + destroyed: seq[cint] + +method initPlum(m: MockMapNatPortMapper): Result[void, string] {.gcsafe.} = + ok() + +method hasLiveMapping(m: MockMapNatPortMapper, id: cint): bool {.gcsafe.} = + m.live + +method createMappingFor( + m: MockMapNatPortMapper, protocol: PlumProtocol, port: uint16 +): Future[Result[MappingResult, string]] {.async: (raises: [CancelledError]), gcsafe.} = + m.created.add(protocol) + if protocol == TCP: m.tcpResult else: m.udpResult + +method destroyMappingFor(m: MockMapNatPortMapper, id: cint) {.gcsafe.} = + m.destroyed.add(id) + +proc mappingOk(id: cint, port: uint16): Result[MappingResult, string] = + Result[MappingResult, string].ok( + MappingResult( + id: id, + mapping: PlumMapping(mappingProtocol: MappingProtocol.UPnP, externalPort: port), + ) + ) + asyncchecksuite "NAT reaction - port mapping": var sw: Switch var key: PrivateKey @@ -230,3 +260,80 @@ asyncchecksuite "NAT reaction - address announcing": # Ensure that nothing is injected because there is no active mapping check sw.peerInfo.addrs == sw.peerInfo.listenAddrs + +proc mapperWith(protocol: MappingProtocol): Option[NatPortMapper] = + some(NatPortMapper(activeMappingProtocol: some(protocol))) + +asyncchecksuite "NAT - portMappingStr": + test "no mapper is none": + check portMappingStr(none(NatPortMapper)) == "none" + + test "mapper without an active protocol is none": + check portMappingStr(some(NatPortMapper())) == "none" + + test "UPnP maps to upnp": + check portMappingStr(mapperWith(MappingProtocol.UPnP)) == "upnp" + + test "NAT-PMP maps to pmp": + check portMappingStr(mapperWith(MappingProtocol.NatPmp)) == "pmp" + + test "PCP maps to pcp": + check portMappingStr(mapperWith(MappingProtocol.PCP)) == "pcp" + + test "Direct maps to direct": + check portMappingStr(mapperWith(MappingProtocol.Direct)) == "direct" + + test "Unknown maps to none": + check portMappingStr(mapperWith(MappingProtocol.Unknown)) == "none" + +asyncchecksuite "NAT - mapNatPorts": + test "returns the mapped ports when both mappings succeed": + let mapper = MockMapNatPortMapper( + tcpResult: mappingOk(cint(1), 9000), udpResult: mappingOk(cint(2), 9001) + ) + + check (await mapper.mapNatPorts()) == + some((Port(9000), Port(9001), MappingProtocol.UPnP)) + check mapper.destroyed.len == 0 + + test "destroys the TCP mapping when the UDP mapping fails": + let mapper = MockMapNatPortMapper( + tcpResult: mappingOk(cint(42), 9000), + udpResult: Result[MappingResult, string].err("udp mapping failed"), + ) + + check (await mapper.mapNatPorts()).isNone + check mapper.destroyed == @[cint(42)] + + test "gives up without touching UDP when the TCP mapping fails": + let mapper = MockMapNatPortMapper( + tcpResult: Result[MappingResult, string].err("tcp mapping failed"), + udpResult: mappingOk(cint(2), 9001), + ) + + check (await mapper.mapNatPorts()).isNone + check mapper.created == @[PlumProtocol.TCP] # UDP never attempted + check mapper.destroyed.len == 0 # nothing to clean up + + test "does not map when configured with an external IP": + let mapper = MockMapNatPortMapper( + natConfig: nat.NatConfig(hasExtIp: true, extIp: parseIpAddress("1.2.3.4")) + ) + + check (await mapper.mapNatPorts()).isNone + check mapper.created.len == 0 # short-circuits before any mapping + + test "reuses the existing mapping when both are still live": + privateAccess(NatPortMapper) + let mapper = MockMapNatPortMapper( + live: true, + activeTcpPort: some(Port(9000)), + activeUdpPort: some(Port(9001)), + activeMappingProtocol: some(MappingProtocol.UPnP), + ) + mapper.tcpMappingId = some(cint(1)) # private field, set via privateAccess + mapper.udpMappingId = some(cint(2)) + + check (await mapper.mapNatPorts()) == + some((Port(9000), Port(9001), MappingProtocol.UPnP)) + check mapper.created.len == 0 # reuse path, nothing recreated