diff --git a/.dockerignore b/.dockerignore index 6427da1d..e201c832 100644 --- a/.dockerignore +++ b/.dockerignore @@ -3,4 +3,3 @@ build docs metrics nimcache -tests diff --git a/Makefile b/Makefile index f7945253..2a1a05a3 100644 --- a/Makefile +++ b/Makefile @@ -82,10 +82,12 @@ endif coverage \ deps \ libbacktrace \ + libplum \ test \ testAll \ testIntegration \ testLibstorage \ + testNatIntegration \ update ifeq ($(NIM_PARAMS),) @@ -120,11 +122,14 @@ else NIM_PARAMS := $(NIM_PARAMS) -d:release endif -deps: | deps-common nat-libs +deps: | deps-common libplum ifneq ($(USE_LIBBACKTRACE), 0) deps: | libbacktrace endif +libplum: + + "$(MAKE)" -C vendor/nim-libplum/vendor/libplum libplum.a CC=$(CC) $(HANDLE_OUTPUT) + update: | update-common # detecting the os @@ -147,6 +152,12 @@ testIntegration: | build deps echo -e $(BUILD_MSG) "build/$@" && \ $(ENV_SCRIPT) nim testIntegration $(TEST_PARAMS) $(NIM_PARAMS) build.nims +# Builds and runs the UPnP NAT integration test inside a miniupnpd container +DOCKER := $(or $(shell which podman 2>/dev/null), $(shell which docker 2>/dev/null)) +testNatIntegration: + $(DOCKER) build -t miniupnpd-test -f tests/integration/nat/Dockerfile . + $(DOCKER) run --rm --cap-add NET_ADMIN miniupnpd-test + # Builds a C example that uses the libstorage C library and runs it testLibstorage: | build deps $(MAKE) $(if $(ncpu),-j$(ncpu),) libstorage diff --git a/build.nims b/build.nims index b74a931f..1ecc0319 100644 --- a/build.nims +++ b/build.nims @@ -78,6 +78,21 @@ task testIntegration, "Run integration tests": # test "testIntegration", params = "-d:chronicles_sinks=textlines[notimestamps,stdout],textlines[dynamic] " & # "-d:chronicles_enabled_topics:integration:TRACE" +task testNatPortMapping, "Run UPnP NAT integration test (requires miniupnpd container)": + buildBinary "storage", + outName = "storage", + params = "-d:chronicles_runtime_filtering -d:chronicles_log_level=TRACE" + putEnv("STORAGE_INTEGRATION_TEST_INCLUDES", "integration/1_minute/testnatupnp.nim") + test "testIntegration", outName = "testIntegrationNat" + +# Used to build the testing binarie in Docker +task buildNatPortMappingBinaries, "Build UPnP NAT test binaries without running them": + buildBinary "storage", + outName = "storage", + params = "-d:chronicles_runtime_filtering -d:chronicles_log_level=TRACE" + putEnv("STORAGE_INTEGRATION_TEST_INCLUDES", "integration/1_minute/testnatupnp.nim") + buildBinary "testIntegration", outName = "testIntegrationNat", srcDir = "tests/" + task build, "build Logos Storage binary": storageTask() diff --git a/library/storage_thread_requests/requests/node_debug_request.nim b/library/storage_thread_requests/requests/node_debug_request.nim index 1925e05b..a72d7143 100644 --- a/library/storage_thread_requests/requests/node_debug_request.nim +++ b/library/storage_thread_requests/requests/node_debug_request.nim @@ -63,21 +63,10 @@ proc getDebug( "announceAddresses": node.discovery.announceAddrs, "table": table, "nat": { - "reachability": - if storage[].autonatService.isSome: - $storage[].autonatService.get.networkReachability - else: - "unknown", + "reachability": reachabilityStr(storage[].autonatService), "relayRunning": storage[].autoRelayService.isSome and storage[].autoRelayService.get.isRunning, - "portMapping": - if storage[].natMapper.isNone or - storage[].natMapper.get.portMappingType == NoMapping: - "none" - elif storage[].natMapper.get.portMappingType == UpnpMapping: - "upnp" - else: - "pmp", + "portMapping": portMappingStr(storage[].natMapper), }, } diff --git a/storage/conf.nim b/storage/conf.nim index f50bd29e..54b89d41 100644 --- a/storage/conf.nim +++ b/storage/conf.nim @@ -341,6 +341,24 @@ type name: "nat-max-relays" .}: int + natPortMappingDiscoverTimeout* {. + desc: "Timeout in milliseconds for UPnP/NAT-PMP/PCP device discovery", + defaultValue: 500, + name: "nat-port-mapping-discover-timeout" + .}: int + + natPortMappingTimeout* {. + desc: "Timeout in milliseconds for creating a port mapping on the router", + defaultValue: 500, + name: "nat-port-mapping-timeout" + .}: int + + natPortMappingRecheckPeriod* {. + desc: "Period in milliseconds between rechecks of existing port mappings", + defaultValue: 300000, + name: "nat-port-mapping-recheck-period" + .}: int + natSimulation* {. desc: "Simulate NAT filtering behavior for testing: endpoint-independent, address-dependent, address-and-port-dependent", diff --git a/storage/nat.nim b/storage/nat.nim index 0cf06e9c..ce3fea5a 100644 --- a/storage/nat.nim +++ b/storage/nat.nim @@ -12,10 +12,10 @@ import std/[options, net] import results import pkg/chronos -import pkg/chronos/threadsync import pkg/chronicles import pkg/libp2p import pkg/libp2p/services/autorelayservice +import pkg/libp2p/protocols/connectivity/autonatv2/service import ./utils import ./utils/natutils @@ -25,105 +25,82 @@ import ./discovery logScope: topics = "nat" -const NatPortMappingTimeout = 5.seconds - type NatConfig* = object case hasExtIp*: bool of true: extIp*: IpAddress of false: nat*: NatStrategy -type PortMappingType* = enum - NoMapping - UpnpMapping - PmpMapping - type NatMapper* = ref object of RootObj natConfig*: NatConfig tcpPort*: Port discoveryPort*: Port - portMappingType*: PortMappingType - -type MapNatPortsCtx = object - natConfig: NatConfig - tcpPort: Port - discoveryPort: Port - signal: ThreadSignalPtr - result: Option[(Port, Port)] - portMappingType: PortMappingType - -proc mapNatPortsThread(ctx: ptr MapNatPortsCtx) {.thread.} = - if ctx.natConfig.hasExtIp: - discard ctx.signal.fireSync() - return - - # Devices are recreated on each call: discover() costs ~200ms but only fires - # when AutoNAT reports NotReachable, which is exactly when we want a fresh scan. - let upnpRes = UpnpDevice.init() - if upnpRes.isOk: - let ports = upnpRes.value.mapPorts(ctx.tcpPort, ctx.discoveryPort) - if ports.isSome: - ctx.portMappingType = UpnpMapping - ctx.result = ports - discard ctx.signal.fireSync() - return - - let pmpRes = PmpDevice.init() - if pmpRes.isOk: - let ports = pmpRes.value.mapPorts(ctx.tcpPort, ctx.discoveryPort) - if ports.isSome: - ctx.portMappingType = PmpMapping - ctx.result = ports - - discard ctx.signal.fireSync() + discoverTimeout*: int + mappingTimeout*: int + recheckPeriod*: int + tcpMappingId: Option[cint] + udpMappingId: Option[cint] + activeMappingProtocol*: Option[MappingProtocol] + activeTcpPort: Option[Port] + activeUdpPort: Option[Port] + plumInitialized: bool method mapNatPorts*( m: NatMapper -): Future[Option[(Port, Port)]] {.async: (raises: [CancelledError]), base, gcsafe.} = - let signal = ThreadSignalPtr.new().valueOr: - warn "Failed to create ThreadSignalPtr for NAT port mapping" - return none((Port, Port)) +): Future[Option[(Port, Port, MappingProtocol)]] {. + async: (raises: [CancelledError]), base, gcsafe +.} = + if m.natConfig.hasExtIp: + return none((Port, Port, MappingProtocol)) - var ctx = cast[ptr MapNatPortsCtx](createShared(MapNatPortsCtx)) - ctx[] = MapNatPortsCtx( - natConfig: m.natConfig, - tcpPort: m.tcpPort, - discoveryPort: m.discoveryPort, - signal: signal, - ) + # 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): + return some((m.activeTcpPort.get, m.activeUdpPort.get, m.activeMappingProtocol.get)) - var thread: Thread[ptr MapNatPortsCtx] - var threadStarted = false - defer: - if threadStarted: - # Blocking the event loop here is acceptable: UPnP discover() is bounded - # by UPNP_TIMEOUT (200ms), so the worst-case stall is ~200ms. - joinThread(thread) - # Always sync hasUpnpMapping back, even on timeout or cancellation. - # If the thread mapped ports just after the timeout, close() will - # still clean them up on the router. - if ctx.portMappingType != NoMapping: - m.portMappingType = ctx.portMappingType - freeShared(ctx) - discard signal.close() + if not m.plumInitialized: + # 5s matches the old NatPortMappingTimeout used with miniupnpc/libnatpmp. + let res = init( + discoverTimeout = m.discoverTimeout, + mappingTimeout = m.mappingTimeout, + recheckPeriod = m.recheckPeriod, + ) + if res.isErr: + warn "Failed to initialize plum", msg = res.error + return none((Port, Port, MappingProtocol)) + m.plumInitialized = true - try: - createThread(thread, mapNatPortsThread, ctx) - threadStarted = true - except ValueError, ResourceExhaustedError: - warn "Failed to create thread for NAT port mapping" - return none((Port, Port)) + # If there is only one mapping, something went wrong somewhere + # so we delete the mappings to recreate them. + if m.tcpMappingId.isSome: + destroyMapping(m.tcpMappingId.get) + m.tcpMappingId = none(cint) - try: - if not await signal.wait().withTimeout(NatPortMappingTimeout): - warn "NAT port mapping thread timed out" - return none((Port, Port)) - except CancelledError as exc: - raise exc - except AsyncError as exc: - warn "Error waiting for NAT port mapping thread", error = exc.msg - return none((Port, Port)) + if m.udpMappingId.isSome: + destroyMapping(m.udpMappingId.get) + m.udpMappingId = none(cint) - return ctx.result + m.activeMappingProtocol = none(MappingProtocol) + m.activeTcpPort = none(Port) + m.activeUdpPort = none(Port) + + let tcpRes = await createMapping(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) + if udpRes.isErr: + warn "UDP port mapping failed", msg = udpRes.error + destroyMapping(tcpRes.value.id) + return none((Port, Port, MappingProtocol)) + + m.tcpMappingId = some(tcpRes.value.id) + m.udpMappingId = some(udpRes.value.id) + m.activeMappingProtocol = some(tcpRes.value.mapping.mappingProtocol) + m.activeTcpPort = some(Port(tcpRes.value.mapping.externalPort)) + m.activeUdpPort = some(Port(udpRes.value.mapping.externalPort)) + + some((m.activeTcpPort.get, m.activeUdpPort.get, m.activeMappingProtocol.get)) method handleNatStatus*( m: NatMapper, @@ -158,7 +135,7 @@ method handleNatStatus*( if dialBackAddr.isNone: warn "Got empty dialback address in AutoNat when node is NotReachable" else: - debug "Node is not reachable trying UPnP / PMP now" + debug "Node is not reachable trying port mapping now" # Here we should check first that a mapping exists. # If it does exist but Autonat still report as Not Reachable @@ -166,9 +143,9 @@ method handleNatStatus*( let maybePorts = await m.mapNatPorts() if maybePorts.isSome: - let (tcpPort, udpPort) = maybePorts.get() + let (tcpPort, udpPort, protocol) = maybePorts.get() - info "Port mapping created successfully", tcpPort, udpPort + info "Port mapping created successfully", tcpPort, udpPort, protocol let announceAddress = dialBackAddr.get.remapAddr(port = some(tcpPort)) @@ -195,25 +172,32 @@ method handleNatStatus*( else: debug "AutoRelayService started" -proc close*(m: NatMapper, device = UpnpDevice()) = - # UPnP mappings are permanent (leaseDuration=0) and must be deleted explicitly. - # NAT-PMP mappings expire automatically after NATPMP_LIFETIME seconds. - if m.portMappingType != UpnpMapping: - return +proc close*(m: NatMapper) = + if m.tcpMappingId.isSome: + destroyMapping(m.tcpMappingId.get) + m.tcpMappingId = none(cint) + if m.udpMappingId.isSome: + destroyMapping(m.udpMappingId.get) + m.udpMappingId = none(cint) + if m.plumInitialized: + discard cleanup() + m.plumInitialized = false - # deletePortMapping requires the IGD control URL set during init - let deviceRes = device.init() - if deviceRes.isErr: - warn "UPnP reinit failed during cleanup, port mappings may remain", - msg = deviceRes.error - return +proc reachabilityStr*(autonat: Option[AutonatV2Service]): string = + if autonat.isSome: + $autonat.get.networkReachability + else: + "unknown" - for (port, proto) in [ - (m.tcpPort, NatIpProtocol.Tcp), (m.discoveryPort, NatIpProtocol.Udp) - ]: - let res = deviceRes.value.deletePortMapping(port, proto) - if res.isErr: - error "UPnP port mapping deletion failed", port, proto, msg = res.error +proc portMappingStr*(natMapper: Option[NatMapper]): string = + if natMapper.isNone or natMapper.get.activeMappingProtocol.isNone: + return "none" + case natMapper.get.activeMappingProtocol.get + of MappingProtocol.UPnP: "upnp" + of MappingProtocol.NatPmp: "pmp" + of MappingProtocol.PCP: "pcp" + of MappingProtocol.Direct: "direct" + of MappingProtocol.Unknown: "none" proc findReachableNodes*(bootstrapNodes: seq[SignedPeerRecord]): seq[SignedPeerRecord] = ## Returns the list of nodes known to be directly reachable. diff --git a/storage/rest/api.nim b/storage/rest/api.nim index 6acdfd61..c125c7fb 100644 --- a/storage/rest/api.nim +++ b/storage/rest/api.nim @@ -535,8 +535,8 @@ proc initNodeApi(node: StorageNodeRef, conf: StorageConf, router: var RestRouter ## to invoke peer discovery, if it succeeds ## the returned addresses will be used to dial ## - ## `addrs` the listening addresses of the peers to dial, which is - ## /ip4/0.0.0.0/tcp/, where port is specified with the + ## `addrs` the listening addresses of the peers to dial, which is + ## /ip4/0.0.0.0/tcp/, where port is specified with the ## `--listen-port` CLI flag. ## var headers = buildCorsHeaders("GET", allowedOrigin) @@ -591,20 +591,10 @@ proc initDebugApi( "table": table, "storage": {"version": $storageVersion, "revision": $storageRevision}, "nat": { - "reachability": - if autonat.isSome: - $autonat.get.networkReachability - else: - "unknown", + "reachability": reachabilityStr(autonat), "clientMode": node.discovery.protocol.clientMode, "relayRunning": autoRelay.isSome and autoRelay.get.isRunning, - "portMapping": - if natMapper.isNone or natMapper.get.portMappingType == NoMapping: - "none" - elif natMapper.get.portMappingType == UpnpMapping: - "upnp" - else: - "pmp", + "portMapping": portMappingStr(natMapper), }, } diff --git a/storage/storage.nim b/storage/storage.nim index 61e8edc1..0c975b5d 100644 --- a/storage/storage.nim +++ b/storage/storage.nim @@ -392,6 +392,9 @@ proc new*( natConfig: config.nat, tcpPort: config.listenPort, discoveryPort: config.discoveryPort, + discoverTimeout: config.natPortMappingDiscoverTimeout, + mappingTimeout: config.natPortMappingTimeout, + recheckPeriod: config.natPortMappingRecheckPeriod, ) ) let relayService = AutoRelayService.new( diff --git a/storage/utils/natutils.nim b/storage/utils/natutils.nim index 45abd178..f96d4579 100644 --- a/storage/utils/natutils.nim +++ b/storage/utils/natutils.nim @@ -1,205 +1,15 @@ {.push raises: [].} import std/[options, net] -import nat_traversal/[miniupnpc, natpmp] import pkg/chronicles import results +import libplum/plum +import libplum/libplum -export miniupnpc, natpmp, results, options, net +export plum, libplum, results, options, net logScope: topics = "nat" -const UPNP_TIMEOUT* = 200 # ms -const NATPMP_LIFETIME* = 60 * 60 # seconds - type NatStrategy* = enum NatAuto - -type NatIpProtocol* = enum - Tcp - Udp - -# Generic Nat device can be UPnP or PmP -type NatDevice* = ref object of RootObj - -type UpnpDevice* = ref object of NatDevice - upnp: Miniupnp - -type PmpDevice* = ref object of NatDevice - npmp: NatPmp - -# appPortMapping is specific to the type of Nat device -method addPortMapping*( - d: NatDevice, port: Port, proto: NatIpProtocol -): Result[Port, string] {.base, gcsafe.} = - return err("not implemented") - -# Creates the mapping the the router and -# returns the opened ports. -method mapPorts*( - d: NatDevice, tcpPort, udpPort: Port -): Option[(Port, Port)] {.base, gcsafe.} = - var extTcpPort, extUdpPort: Port - - for t in [(tcpPort, NatIpProtocol.Tcp), (udpPort, NatIpProtocol.Udp)]: - let (port, proto) = t - let pmres = d.addPortMapping(port, proto) - - if pmres.isErr: - error "port mapping failed", msg = pmres.error - return none((Port, Port)) - - case proto - of Tcp: - extTcpPort = pmres.value - of Udp: - extUdpPort = pmres.value - - return some((extTcpPort, extUdpPort)) - -method getSpecificPortMapping*( - d: UpnpDevice, externalPort: string, protocol: UPNPProtocol -): Result[PortMappingRes, cstring] {.base, gcsafe.} = - if d.upnp == nil: - return err(cstring("upnp not initialized")) - - d.upnp.getSpecificPortMapping(externalPort = externalPort, protocol = protocol) - -method discover*(d: UpnpDevice): Result[int, cstring] {.base, gcsafe.} = - if d.upnp == nil: - return err(cstring("upnp not initialized")) - - return d.upnp.discover() - -method selectIGD*(d: UpnpDevice): SelectIGDResult {.base, gcsafe.} = - if d.upnp == nil: - return IGDNotFound - - return d.upnp.selectIGD() - -proc init*(T: type UpnpDevice): Result[UpnpDevice, string] {.gcsafe.} = - UpnpDevice().init() - -# Init UPnP device and create miniupnp instance. -# It call "discover" to retrieve the UPnP devices on the network, -# and then "selectIGD" to select a suitable device. -proc init*(d: UpnpDevice): Result[UpnpDevice, string] {.gcsafe.} = - if d.upnp == nil: - d.upnp = newMiniupnp() - - d.upnp.discoverDelay = UPNP_TIMEOUT - - let dres = d.discover() - if dres.isErr: - debug "UPnP", msg = dres.error - return err($dres.error) - - case d.selectIGD() - of IGDNotFound: - debug "UPnP", msg = "Internet Gateway Device not found. Giving up." - return err("IGD not found") - of IGDFound: - debug "UPnP", msg = "Internet Gateway Device found." - of IGDNotConnected: - debug "UPnP", - msg = "Internet Gateway Device found but it's not connected. Trying anyway." - of NotAnIGD: - debug "UPnP", - msg = - "Some device found, but it's not recognised as an Internet Gateway Device. Trying anyway." - of IGDIpNotRoutable: - debug "UPnP", - msg = - "Internet Gateway Device found and is connected, but with a reserved or non-routable IP. Trying anyway." - - return ok(d) - -# For UPnP, the external port is the same as the application port. -# This should work for most of the case. -# We could change this by using addAnyPortMapping for IGD2 compatible routers -# if needed. -method addPortMapping*( - d: UpnpDevice, port: Port, proto: NatIpProtocol -): Result[Port, string] {.gcsafe.} = - if d.upnp == nil: - return err("upnp not initialized") - - let protocol = if proto == NatIpProtocol.Tcp: UPNPProtocol.TCP else: UPNPProtocol.UDP - let pmres = d.upnp.addPortMapping( - externalPort = $port, - protocol = protocol, - internalHost = d.upnp.lanAddr, - internalPort = $port, - desc = "logos-storage", - leaseDuration = 0, - ) - if pmres.isErr: - return err($pmres.error) - - let cres = d.getSpecificPortMapping(externalPort = $port, protocol = protocol) - if cres.isErr: - # Eventually, the check could fail on some router even if the router is successful. - # So we log a warning but we still want to continue because it is not sure it is a failure. - warn "UPnP port mapping check failed. Assuming the check itself is broken and the port mapping was done.", - msg = cres.error - - info "UPnP: added port mapping", externalPort = port, internalPort = port - - return ok(port) - -method deletePortMapping*( - d: UpnpDevice, port: Port, proto: NatIpProtocol -): Result[void, string] {.base, gcsafe.} = - if d.upnp == nil: - return err("upnp not initialized") - - let protocol = if proto == NatIpProtocol.Tcp: UPNPProtocol.TCP else: UPNPProtocol.UDP - let res = d.upnp.deletePortMapping(externalPort = $port, protocol = protocol) - if res.isErr: - return err($res.error) - - debug "UPnP: deleted port mapping", port, proto - - ok() - -proc init*(T: type PmpDevice): Result[PmpDevice, string] {.gcsafe.} = - PmpDevice().init() - -# Create a NatPmP instance. -proc init*(d: PmpDevice): Result[PmpDevice, string] {.gcsafe.} = - if d.npmp == nil: - d.npmp = newNatPmp() - - let res = d.npmp.init() - if res.isErr: - debug "NAT-PMP", msg = res.error - return err($res.error) - - return ok(d) - -# Add a port mapping on NAT-PMP device. -# The application port might not be the external port. -# The latter is returned. -method addPortMapping*( - d: PmpDevice, port: Port, proto: NatIpProtocol -): Result[Port, string] {.gcsafe.} = - if d.npmp == nil: - return err("npmp not initialized") - - let protocol = - if proto == NatIpProtocol.Tcp: NatPmpProtocol.TCP else: NatPmpProtocol.UDP - let pmres = d.npmp.addPortMapping( - eport = port.cushort, - iport = port.cushort, - protocol = protocol, - lifetime = NATPMP_LIFETIME, - ) - if pmres.isErr: - return err(pmres.error) - - let extPort = Port(pmres.value) - - info "NAT-PMP: added port mapping", externalPort = extPort, internalPort = port - - return ok(extPort) diff --git a/tests/integration/1_minute/testnat.nim b/tests/integration/1_minute/testnat.nim index b9c7079a..abf40928 100644 --- a/tests/integration/1_minute/testnat.nim +++ b/tests/integration/1_minute/testnat.nim @@ -1,39 +1,15 @@ -import std/json import std/options -import std/sequtils import pkg/chronos import pkg/questionable/results import ../multinodes import ../storageclient import ../storageconfig +import ../nathelper -const - DetectionTimeout = 15_000 - RelayTimeout = 30_000 - PollInterval = 1_000 +export nathelper -proc checkNatStatus*( - client: StorageClient, reachability: string, relayRunning: bool, clientMode: bool -) {.async.} = - check eventuallySafe( - block: - let info = (await client.info()).get - let nat = info["nat"] - let addrs = info["addrs"].getElems.mapIt(it.getStr) - nat["reachability"].getStr() == reachability and - nat["clientMode"].getBool() == clientMode and - nat["relayRunning"].getBool() == relayRunning and - addrs.anyIt("p2p-circuit" in it) == relayRunning, - timeout = RelayTimeout, - pollInterval = PollInterval, - ) - -proc checkNatStatus*(client: StorageClient, reachability: string) {.async.} = - let notReachable = reachability == "NotReachable" - await client.checkNatStatus( - reachability, relayRunning = notReachable, clientMode = notReachable - ) +const DetectionTimeout = 15_000 # Reminder: multinodesuite setup the first node as bootstrap node multinodesuite "AutoNAT detection": diff --git a/tests/integration/1_minute/testnatupnp.nim b/tests/integration/1_minute/testnatupnp.nim index 933e6204..747246a9 100644 --- a/tests/integration/1_minute/testnatupnp.nim +++ b/tests/integration/1_minute/testnatupnp.nim @@ -1,21 +1,15 @@ -import std/[envvars, json, strutils, sequtils] +import std/[json, strutils, sequtils] import pkg/chronos import pkg/questionable/results -import nat_traversal/miniupnpc import ../multinodes import ../storageclient import ../storageconfig -import ../../../storage/utils/natutils -from ./testnat.nim import checkNatStatus +import ../nathelper -const - DetectionTimeout = 15_000 - RelayTimeout = 30_000 - PollInterval = 1_000 +const DetectionTimeout = 15_000 -# Requires a real UPnP router on the network (NAT_TEST_UPNP=1) multinodesuite "AutoNAT UPnP port mapping": let upnpConfig = NodeConfigs( clients: StorageConfigs @@ -33,10 +27,6 @@ multinodesuite "AutoNAT UPnP port mapping": test "node behind NAT maps ports via UPnP and exposes mapping in debug info", upnpConfig: - if getEnv("NAT_TEST_UPNP") != "1": - skip() - return - let node2 = clients()[1] await node2.client.checkNatStatus( @@ -51,23 +41,13 @@ multinodesuite "AutoNAT UPnP port mapping": pollInterval = PollInterval, ) - # Ideally we should find a way to test that the node is Reachable now - await node2.client.checkNatStatus( "NotReachable", relayRunning = false, clientMode = true ) - # Extract mapped TCP port from announce addresses and verify it exists on the IGD let announceAddrs = (await node2.client.info()).get["announceAddresses"].getElems.mapIt(it.getStr) let tcpAddr = announceAddrs.filterIt(it.startsWith("/ip4/") and "/tcp/" in it) check tcpAddr.len > 0 - let mappedPort = tcpAddr[0].split("/")[4] - let device = UpnpDevice.init() - check device.isOk - check device.get.getSpecificPortMapping(mappedPort, UPNPProtocol.TCP).isOk - await node2.stop() - - check device.get.getSpecificPortMapping(mappedPort, UPNPProtocol.TCP).isErr diff --git a/tests/integration/nat/docker-entrypoint.sh b/tests/integration/nat/docker-entrypoint.sh new file mode 100644 index 00000000..1be6b923 --- /dev/null +++ b/tests/integration/nat/docker-entrypoint.sh @@ -0,0 +1,28 @@ +#!/bin/bash +set -euo pipefail + +RUNDIR=/tmp/miniupnpd +mkdir -p "$RUNDIR" + +LAN_IF=$(ip route show default | awk '/default/{print $5; exit}') + +ip link add plum-wan type dummy +ip addr add 1.2.3.4/24 dev plum-wan +ip link set plum-wan up + +cat > "$RUNDIR/miniupnpd.conf" << EOF +ext_ifname=plum-wan +listening_ip=$LAN_IF +enable_pcp_pmp=no +port=0 +allow 1024-65535 0.0.0.0/0 1024-65535 +EOF + +if [[ "${DEBUG:-0}" == "1" ]]; then + miniupnpd -d -f "$RUNDIR/miniupnpd.conf" & +else + miniupnpd -d -f "$RUNDIR/miniupnpd.conf" > /dev/null 2>&1 & +fi +sleep 1 + +/app/build/testIntegrationNat diff --git a/tests/integration/nat/miniupnpd_stub_rdr.c b/tests/integration/nat/miniupnpd_stub_rdr.c new file mode 100644 index 00000000..15e45b0a --- /dev/null +++ b/tests/integration/nat/miniupnpd_stub_rdr.c @@ -0,0 +1,169 @@ +/* Stub firewall backend for miniupnpd. + * Replaces iptcrdr.o + iptpinhole.o + nfct_get.o. + * All mapping operations succeed without touching the kernel. */ + +#include +#include + +/* commonrdr.h interface */ + +int init_redirect(void) { return 0; } +void shutdown_redirect(void) {} + +int get_redirect_rule_count(const char *ifname) +{ (void)ifname; return 0; } + +int get_redirect_rule(const char *ifname, unsigned short eport, int proto, + char *iaddr, int iaddrlen, unsigned short *iport, + char *desc, int desclen, + char *rhost, int rhostlen, + unsigned int *timestamp, + uint64_t *packets, uint64_t *bytes) +{ (void)ifname; (void)eport; (void)proto; (void)iaddr; (void)iaddrlen; + (void)iport; (void)desc; (void)desclen; (void)rhost; (void)rhostlen; + (void)timestamp; (void)packets; (void)bytes; return -1; } + +int get_redirect_rule_by_index(int index, + char *ifname, unsigned short *eport, + char *iaddr, int iaddrlen, unsigned short *iport, + int *proto, char *desc, int desclen, + char *rhost, int rhostlen, + unsigned int *timestamp, + uint64_t *packets, uint64_t *bytes) +{ (void)index; (void)ifname; (void)eport; (void)iaddr; (void)iaddrlen; + (void)iport; (void)proto; (void)desc; (void)desclen; (void)rhost; + (void)rhostlen; (void)timestamp; (void)packets; (void)bytes; return -1; } + +unsigned short *get_portmappings_in_range(unsigned short startport, + unsigned short endport, + int proto, unsigned int *number) +{ (void)startport; (void)endport; (void)proto; *number = 0; return 0; } + +int update_portmapping(const char *ifname, unsigned short eport, int proto, + unsigned short iport, const char *desc, + unsigned int timestamp) +{ (void)ifname; (void)eport; (void)proto; (void)iport; (void)desc; + (void)timestamp; return 0; } + +int update_portmapping_desc_timestamp(const char *ifname, + unsigned short eport, int proto, + const char *desc, unsigned int timestamp) +{ (void)ifname; (void)eport; (void)proto; (void)desc; (void)timestamp; + return 0; } + +/* iptcrdr.h interface */ + +int add_redirect_rule2(const char *ifname, + const char *rhost, unsigned short eport, + const char *iaddr, unsigned short iport, int proto, + const char *desc, unsigned int timestamp) +{ (void)ifname; (void)rhost; (void)eport; (void)iaddr; (void)iport; + (void)proto; (void)desc; (void)timestamp; return 0; } + +int add_peer_redirect_rule2(const char *ifname, + const char *rhost, unsigned short rport, + const char *eaddr, unsigned short eport, + const char *iaddr, unsigned short iport, int proto, + const char *desc, unsigned int timestamp) +{ (void)ifname; (void)rhost; (void)rport; (void)eaddr; (void)eport; + (void)iaddr; (void)iport; (void)proto; (void)desc; (void)timestamp; + return 0; } + +int add_filter_rule2(const char *ifname, + const char *rhost, const char *iaddr, + unsigned short eport, unsigned short iport, + int proto, const char *desc) +{ (void)ifname; (void)rhost; (void)iaddr; (void)eport; (void)iport; + (void)proto; (void)desc; return 0; } + +int delete_redirect_and_filter_rules(unsigned short eport, int proto) +{ (void)eport; (void)proto; return 0; } + +int delete_filter_rule(const char *ifname, unsigned short port, int proto) +{ (void)ifname; (void)port; (void)proto; return 0; } + +int add_peer_dscp_rule2(const char *ifname, + const char *rhost, unsigned short rport, + unsigned char dscp, + const char *iaddr, unsigned short iport, int proto, + const char *desc, unsigned int timestamp) +{ (void)ifname; (void)rhost; (void)rport; (void)dscp; (void)iaddr; + (void)iport; (void)proto; (void)desc; (void)timestamp; return 0; } + +int get_peer_rule_by_index(int index, + char *ifname, unsigned short *eport, + char *iaddr, int iaddrlen, unsigned short *iport, + int *proto, char *desc, int desclen, + char *rhost, int rhostlen, unsigned short *rport, + unsigned int *timestamp, + uint64_t *packets, uint64_t *bytes) +{ (void)index; (void)ifname; (void)eport; (void)iaddr; (void)iaddrlen; + (void)iport; (void)proto; (void)desc; (void)desclen; (void)rhost; + (void)rhostlen; (void)rport; (void)timestamp; (void)packets; (void)bytes; + return -1; } + +int get_nat_redirect_rule(const char *nat_chain_name, const char *ifname, + unsigned short eport, int proto, + char *iaddr, int iaddrlen, unsigned short *iport, + char *desc, int desclen, + char *rhost, int rhostlen, + unsigned int *timestamp, + uint64_t *packets, uint64_t *bytes) +{ (void)nat_chain_name; (void)ifname; (void)eport; (void)proto; (void)iaddr; + (void)iaddrlen; (void)iport; (void)desc; (void)desclen; (void)rhost; + (void)rhostlen; (void)timestamp; (void)packets; (void)bytes; return -1; } + +int list_redirect_rule(const char *ifname) +{ (void)ifname; return 0; } + +/* commonrdr.h USE_NETFILTER interface */ + +int set_rdr_name(int param, const char *string) +{ (void)param; (void)string; return 0; } + +/* nfct_get.c interface */ + +int get_nat_ext_addr(struct sockaddr *src, struct sockaddr *dst, uint8_t proto, + struct sockaddr *ret_ext) +{ (void)src; (void)dst; (void)proto; (void)ret_ext; return -1; } + +/* iptpinhole.h interface */ + +int find_pinhole(const char *ifname, + const char *rem_host, unsigned short rem_port, + const char *int_client, unsigned short int_port, + int proto, char *desc, int desc_len, unsigned int *timestamp) +{ (void)ifname; (void)rem_host; (void)rem_port; (void)int_client; + (void)int_port; (void)proto; (void)desc; (void)desc_len; (void)timestamp; + return -1; } + +int add_pinhole(const char *ifname, + const char *rem_host, unsigned short rem_port, + const char *int_client, unsigned short int_port, + int proto, const char *desc, unsigned int timestamp) +{ (void)ifname; (void)rem_host; (void)rem_port; (void)int_client; + (void)int_port; (void)proto; (void)desc; (void)timestamp; return 0; } + +int update_pinhole(unsigned short uid, unsigned int timestamp) +{ (void)uid; (void)timestamp; return 0; } + +int delete_pinhole(unsigned short uid) +{ (void)uid; return 0; } + +int get_pinhole_info(unsigned short uid, + char *rem_host, int rem_hostlen, unsigned short *rem_port, + char *int_client, int int_clientlen, + unsigned short *int_port, + int *proto, char *desc, int desclen, + unsigned int *timestamp, + uint64_t *packets, uint64_t *bytes) +{ (void)uid; (void)rem_host; (void)rem_hostlen; (void)rem_port; + (void)int_client; (void)int_clientlen; (void)int_port; (void)proto; + (void)desc; (void)desclen; (void)timestamp; (void)packets; (void)bytes; + return -1; } + +int get_pinhole_uid_by_index(int index) +{ (void)index; return -1; } + +int clean_pinhole_list(unsigned int *next_timestamp) +{ (void)next_timestamp; return 0; } diff --git a/tests/integration/nathelper.nim b/tests/integration/nathelper.nim new file mode 100644 index 00000000..68f21176 --- /dev/null +++ b/tests/integration/nathelper.nim @@ -0,0 +1,34 @@ +import std/json +import std/sequtils +import pkg/chronos +import pkg/questionable/results + +import ./multinodes +import ./storageclient +import ./storageconfig + +const + RelayTimeout* = 30_000 + PollInterval* = 1_000 + +proc checkNatStatus*( + client: StorageClient, reachability: string, relayRunning: bool, clientMode: bool +) {.async.} = + check eventuallySafe( + block: + let info = (await client.info()).get + let nat = info["nat"] + let addrs = info["addrs"].getElems.mapIt(it.getStr) + nat["reachability"].getStr() == reachability and + nat["clientMode"].getBool() == clientMode and + nat["relayRunning"].getBool() == relayRunning and + addrs.anyIt("p2p-circuit" in it) == relayRunning, + timeout = RelayTimeout, + pollInterval = PollInterval, + ) + +proc checkNatStatus*(client: StorageClient, reachability: string) {.async.} = + let notReachable = reachability == "NotReachable" + await client.checkNatStatus( + reachability, relayRunning = notReachable, clientMode = notReachable + ) diff --git a/tests/integration/storageconfig.nim b/tests/integration/storageconfig.nim index 3508b31e..b509c0dd 100644 --- a/tests/integration/storageconfig.nim +++ b/tests/integration/storageconfig.nim @@ -330,6 +330,30 @@ proc withNatScheduleInterval*( config.addCliOption("--nat-schedule-interval", $scheduleInterval) return startConfig +proc withNatPortMappingDiscoverTimeout*( + self: StorageConfigs, timeout: int +): StorageConfigs {.raises: [StorageConfigError].} = + var startConfig = self + for config in startConfig.configs.mitems: + config.addCliOption("--nat-port-mapping-discover-timeout", $timeout) + return startConfig + +proc withNatPortMappingTimeout*( + self: StorageConfigs, timeout: int +): StorageConfigs {.raises: [StorageConfigError].} = + var startConfig = self + for config in startConfig.configs.mitems: + config.addCliOption("--nat-port-mapping-timeout", $timeout) + return startConfig + +proc withNatPortMappingRecheckPeriod*( + self: StorageConfigs, timeout: int +): StorageConfigs {.raises: [StorageConfigError].} = + var startConfig = self + for config in startConfig.configs.mitems: + config.addCliOption("--nat-port-mapping-recheck-period", $timeout) + return startConfig + proc withExtIp*( self: StorageConfigs, idx: int, ip = "127.0.0.1" ): StorageConfigs {.raises: [StorageConfigError].} = diff --git a/tests/storage/testnat.nim b/tests/storage/testnat.nim index d4c14f23..c1af8eaf 100644 --- a/tests/storage/testnat.nim +++ b/tests/storage/testnat.nim @@ -1,4 +1,4 @@ -import std/[net, importutils] +import std/[net] import pkg/chronos import pkg/libp2p/[multiaddress, multihash, multicodec] import pkg/libp2p/protocols/connectivity/autonat/types @@ -14,54 +14,16 @@ import ../../storage/discovery import ../../storage/rng import ../../storage/utils -privateAccess(NatMapper) - -type MockUpnpDevice = ref object of UpnpDevice - deletedPorts: seq[(Port, NatIpProtocol)] - -method discover*(d: MockUpnpDevice): Result[int, cstring] {.gcsafe.} = - ok(1) - -method selectIGD*(d: MockUpnpDevice): SelectIGDResult {.gcsafe.} = - IGDFound - -method deletePortMapping*( - d: MockUpnpDevice, port: Port, proto: NatIpProtocol -): Result[void, string] {.gcsafe.} = - d.deletedPorts.add((port, proto)) - ok() - type MockNatMapper = ref object of NatMapper - mappedPorts: Option[(Port, Port)] + mappedPorts: Option[(Port, Port, MappingProtocol)] method mapNatPorts*( m: MockNatMapper -): Future[Option[(Port, Port)]] {.async: (raises: [CancelledError]), gcsafe.} = +): Future[Option[(Port, Port, MappingProtocol)]] {. + async: (raises: [CancelledError]), gcsafe +.} = m.mappedPorts -suite "NAT - NatMapper.close": - test "does nothing when no upnp mapping": - let mapper = MockNatMapper( - natConfig: NatConfig(hasExtIp: false, nat: NatAuto), - tcpPort: Port(8080), - discoveryPort: Port(8090), - ) - let device = MockUpnpDevice() - mapper.close(device) - check device.deletedPorts.len == 0 - - test "deletes tcp and udp ports when upnp mapping exists": - let mapper = MockNatMapper( - natConfig: NatConfig(hasExtIp: false, nat: NatAuto), - tcpPort: Port(8080), - discoveryPort: Port(8090), - ) - mapper.portMappingType = UpnpMapping - let device = MockUpnpDevice() - mapper.close(device) - check device.deletedPorts == - @[(Port(8080), NatIpProtocol.Tcp), (Port(8090), NatIpProtocol.Udp)] - asyncchecksuite "NAT - handleNatStatus": var sw: Switch var key: PrivateKey @@ -86,7 +48,8 @@ asyncchecksuite "NAT - handleNatStatus": test "handleNatStatus announces mapped address when NotReachable and UPnP succeeds": let dialBack = MultiAddress.init("/ip4/1.2.3.4/tcp/8080").expect("valid") - let mapper = MockNatMapper(mappedPorts: some((Port(9000), Port(9001)))) + let mapper = + MockNatMapper(mappedPorts: some((Port(9000), Port(9001), MappingProtocol.UPnP))) await mapper.handleNatStatus( NotReachable, Opt.some(dialBack), discoveryPort, disc, sw, autoRelay @@ -98,7 +61,7 @@ asyncchecksuite "NAT - handleNatStatus": check disc.protocol.clientMode test "handleNatStatus starts autoRelay when NotReachable and UPnP failed": - let mapper = MockNatMapper(mappedPorts: none((Port, Port))) + let mapper = MockNatMapper(mappedPorts: none((Port, Port, MappingProtocol))) await mapper.handleNatStatus( NotReachable, Opt.none(MultiAddress), discoveryPort, disc, sw, autoRelay @@ -109,7 +72,7 @@ asyncchecksuite "NAT - handleNatStatus": test "handleNatStatus starts autoRelay when NotReachable and mapping fails": let dialBack = MultiAddress.init("/ip4/1.2.3.4/tcp/8080").expect("valid") - let mapper = MockNatMapper(mappedPorts: none((Port, Port))) + let mapper = MockNatMapper(mappedPorts: none((Port, Port, MappingProtocol))) await mapper.handleNatStatus( NotReachable, Opt.some(dialBack), discoveryPort, disc, sw, autoRelay @@ -120,7 +83,7 @@ asyncchecksuite "NAT - handleNatStatus": check disc.protocol.clientMode test "handleNatStatus does not announce address when Reachable and no dialBackAddr": - let mapper = MockNatMapper(mappedPorts: none((Port, Port))) + let mapper = MockNatMapper(mappedPorts: none((Port, Port, MappingProtocol))) await mapper.handleNatStatus( Reachable, Opt.none(MultiAddress), discoveryPort, disc, sw, autoRelay @@ -132,7 +95,7 @@ asyncchecksuite "NAT - handleNatStatus": test "handleNatStatus stops relay and announces dialBackAddr when Reachable": let dialBack = MultiAddress.init("/ip4/1.2.3.4/tcp/8080").expect("valid") - let mapper = MockNatMapper(mappedPorts: none((Port, Port))) + let mapper = MockNatMapper(mappedPorts: none((Port, Port, MappingProtocol))) discard await autorelayservice.setup(autoRelay, sw) await mapper.handleNatStatus( diff --git a/tests/storage/testnatutils.nim b/tests/storage/testnatutils.nim deleted file mode 100644 index 10de4d18..00000000 --- a/tests/storage/testnatutils.nim +++ /dev/null @@ -1,93 +0,0 @@ -import std/[options, net] -import nat_traversal/[miniupnpc, natpmp] -import pkg/results -import ../asynctest -import ../../storage/utils/natutils - -type MockUpnpDev = ref object of UpnpDevice - discoverOk: bool - igdResult: SelectIGDResult - addPortMappingOk: bool - failOnProto: Option[NatIpProtocol] - -type MockPmpDev = ref object of PmpDevice - addPortMappingOk: bool - mappedPort: Port - -method discover*(d: MockUpnpDev): Result[int, cstring] {.gcsafe.} = - if d.discoverOk: - ok(1) - else: - err(cstring("discover failed")) - -method selectIGD*(d: MockUpnpDev): SelectIGDResult {.gcsafe.} = - d.igdResult - -method addPortMapping*( - d: MockUpnpDev, port: Port, proto: NatIpProtocol -): Result[Port, string] {.gcsafe.} = - if d.failOnProto == some(proto): - err("mapping failed") - elif d.addPortMappingOk: - ok(port) - else: - err("mapping failed") - -method getSpecificPortMapping*( - d: MockUpnpDev, externalPort: string, protocol: UPNPProtocol -): Result[PortMappingRes, cstring] {.gcsafe.} = - ok(PortMappingRes()) - -method addPortMapping*( - d: MockPmpDev, port: Port, proto: NatIpProtocol -): Result[Port, string] {.gcsafe.} = - if d.addPortMappingOk: - ok(d.mappedPort) - else: - err("mapping failed") - -suite "NAT - UpnpDevice.init": - test "returns err when discover fails": - check MockUpnpDev(discoverOk: false).init().isErr - - test "returns err when IGD not found": - check MockUpnpDev(discoverOk: true, igdResult: IGDNotFound).init().isErr - - test "returns ok when IGD found": - check MockUpnpDev(discoverOk: true, igdResult: IGDFound).init().isOk - - test "returns ok when IGD not connected": - check MockUpnpDev(discoverOk: true, igdResult: IGDNotConnected).init().isOk - - test "returns ok when not an IGD": - check MockUpnpDev(discoverOk: true, igdResult: NotAnIGD).init().isOk - - test "returns ok when IP not routable": - check MockUpnpDev(discoverOk: true, igdResult: IGDIpNotRoutable).init().isOk - -suite "NAT - UpnpDevice.mapPorts": - test "returns none when addPortMapping fails": - check MockUpnpDev(addPortMappingOk: false).mapPorts(Port(8080), Port(8090)).isNone - - test "returns mapped ports": - let res = MockUpnpDev(addPortMappingOk: true).mapPorts(Port(8080), Port(8090)) - check res.isSome - check res.get() == (Port(8080), Port(8090)) - - test "returns none when tcp mapping fails": - let d = MockUpnpDev(addPortMappingOk: true, failOnProto: some(NatIpProtocol.Tcp)) - check d.mapPorts(Port(8080), Port(8090)).isNone - - test "returns none when udp mapping fails": - let d = MockUpnpDev(addPortMappingOk: true, failOnProto: some(NatIpProtocol.Udp)) - check d.mapPorts(Port(8080), Port(8090)).isNone - -suite "NAT - PmpDevice.mapPorts": - test "returns none when mapping fails": - check MockPmpDev(addPortMappingOk: false).mapPorts(Port(8080), Port(8090)).isNone - - test "returns assigned external ports": - let d = MockPmpDev(addPortMappingOk: true, mappedPort: Port(9000)) - let res = d.mapPorts(Port(8080), Port(8090)) - check res.isSome - check res.get() == (Port(9000), Port(9000))