From 7faf93bed888a603f4dbaacc4a55318ab0bf7233 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Fri, 29 Nov 2024 17:56:32 +0000 Subject: [PATCH] fix: support IPv6 with IPv4 Upgrades to latest nat-port-mapper for better IPv6 support. --- packages/upnp-nat/package.json | 2 +- packages/upnp-nat/src/constants.ts | 1 - packages/upnp-nat/src/gateway-finder.ts | 9 ++- packages/upnp-nat/src/index.ts | 7 ++ packages/upnp-nat/src/upnp-nat.ts | 6 +- packages/upnp-nat/src/upnp-port-mapper.ts | 83 +++++++++++------------ packages/upnp-nat/test/index.spec.ts | 54 ++++++++++++--- 7 files changed, 101 insertions(+), 61 deletions(-) diff --git a/packages/upnp-nat/package.json b/packages/upnp-nat/package.json index e6b4dd2167..3693b1a4db 100644 --- a/packages/upnp-nat/package.json +++ b/packages/upnp-nat/package.json @@ -50,7 +50,7 @@ "test:electron-main": "aegir test -t electron-main" }, "dependencies": { - "@achingbrain/nat-port-mapper": "^3.0.2", + "@achingbrain/nat-port-mapper": "^4.0.0", "@chainsafe/is-ip": "^2.0.2", "@libp2p/interface": "^2.2.1", "@libp2p/interface-internal": "^2.1.1", diff --git a/packages/upnp-nat/src/constants.ts b/packages/upnp-nat/src/constants.ts index ff8852c748..b8d9b0c1e8 100644 --- a/packages/upnp-nat/src/constants.ts +++ b/packages/upnp-nat/src/constants.ts @@ -1,3 +1,2 @@ -export const DEFAULT_PORT_MAPPING_TTL = 720_000 export const DEFAULT_GATEWAY_SEARCH_TIMEOUT = 60_000 export const DEFAULT_GATEWAY_SEARCH_INTERVAL = 300_000 diff --git a/packages/upnp-nat/src/gateway-finder.ts b/packages/upnp-nat/src/gateway-finder.ts index 3702b0fbbe..3b46cf6551 100644 --- a/packages/upnp-nat/src/gateway-finder.ts +++ b/packages/upnp-nat/src/gateway-finder.ts @@ -34,8 +34,13 @@ export class GatewayFinder extends TypedEventEmitter { // every five minutes, search for network gateways for one minute this.findGateways = repeatingTask(async (options) => { - for await (const gateway of this.portMappingClient.findGateways(options)) { - if (this.gateways.some(g => g.id === gateway.id)) { + for await (const gateway of this.portMappingClient.findGateways({ + ...options, + searchInterval: 10000 + })) { + if (this.gateways.some(g => { + return g.id === gateway.id && g.family === gateway.family + })) { // already seen this gateway continue } diff --git a/packages/upnp-nat/src/index.ts b/packages/upnp-nat/src/index.ts index 6ee6efdaa8..0f224750ab 100644 --- a/packages/upnp-nat/src/index.ts +++ b/packages/upnp-nat/src/index.ts @@ -86,6 +86,13 @@ export interface UPnPNATInit { */ portMappingAutoRefresh?: boolean + /** + * How long before a port mapping expires to refresh it in ms + * + * @default 60_000 + */ + portMappingRefreshThreshold?: number + /** * A preconfigured instance of a NatAPI client can be passed as an option, * otherwise one will be created diff --git a/packages/upnp-nat/src/upnp-nat.ts b/packages/upnp-nat/src/upnp-nat.ts index eb51f78b03..7439e1a399 100644 --- a/packages/upnp-nat/src/upnp-nat.ts +++ b/packages/upnp-nat/src/upnp-nat.ts @@ -1,7 +1,6 @@ import { upnpNat } from '@achingbrain/nat-port-mapper' import { serviceCapabilities, setMaxListeners, start, stop } from '@libp2p/interface' import { debounce } from '@libp2p/utils/debounce' -import { DEFAULT_PORT_MAPPING_TTL } from './constants.js' import { GatewayFinder } from './gateway-finder.js' import { UPnPPortMapper } from './upnp-port-mapper.js' import type { UPnPNATComponents, UPnPNATInit, UPnPNAT as UPnPNATInterface } from './index.js' @@ -29,8 +28,9 @@ export class UPnPNAT implements Startable, UPnPNATInterface { this.portMappingClient = init.portMappingClient ?? upnpNat({ description: init.portMappingDescription ?? `${components.nodeInfo.name}@${components.nodeInfo.version} ${components.peerId.toString()}`, - ttl: init.portMappingTTL ?? DEFAULT_PORT_MAPPING_TTL, - autoRefresh: init.portMappingAutoRefresh ?? true + ttl: init.portMappingTTL, + autoRefresh: init.portMappingAutoRefresh, + refreshThreshold: init.portMappingRefreshThreshold }) // trigger update when our addresses change diff --git a/packages/upnp-nat/src/upnp-port-mapper.ts b/packages/upnp-nat/src/upnp-port-mapper.ts index 203f0b0156..c1b2289bb7 100644 --- a/packages/upnp-nat/src/upnp-port-mapper.ts +++ b/packages/upnp-nat/src/upnp-port-mapper.ts @@ -1,11 +1,12 @@ -import { isIPv4, isIPv6 } from '@chainsafe/is-ip' +import { isIPv4 } from '@chainsafe/is-ip' import { InvalidParametersError, start, stop } from '@libp2p/interface' +import { isLinkLocal } from '@libp2p/utils/multiaddr/is-link-local' import { isLoopback } from '@libp2p/utils/multiaddr/is-loopback' import { isPrivate } from '@libp2p/utils/multiaddr/is-private' import { isPrivateIp } from '@libp2p/utils/private-ip' import { QUICV1, TCP, WebSockets, WebSocketsSecure, WebTransport } from '@multiformats/multiaddr-matcher' import { dynamicExternalAddress } from './check-external-address.js' -import { DoubleNATError, InvalidIPAddressError } from './errors.js' +import { DoubleNATError } from './errors.js' import type { ExternalAddress } from './check-external-address.js' import type { Gateway } from '@achingbrain/nat-port-mapper' import type { ComponentLogger, Logger } from '@libp2p/interface' @@ -98,12 +99,20 @@ export class UPnPPortMapper { /** * Return any eligible multiaddrs that are not mapped on the detected gateway */ - private getUnmappedAddresses (multiaddrs: Multiaddr[], ipType: 4 | 6): Multiaddr[] { + private getUnmappedAddresses (multiaddrs: Multiaddr[], publicAddresses: string[]): Multiaddr[] { const output: Multiaddr[] = [] for (const ma of multiaddrs) { - // ignore public addresses - if (!isPrivate(ma)) { + const stringTuples = ma.stringTuples() + const address = `${stringTuples[0][1]}` + + // ignore public IPv4 addresses + if (isIPv4(address) && !isPrivate(ma)) { + continue + } + + // ignore any addresses that match the interface on the network gateway + if (publicAddresses.includes(address)) { continue } @@ -112,6 +121,11 @@ export class UPnPPortMapper { continue } + // ignore link-local addresses + if (isLinkLocal(ma)) { + continue + } + // only IP based addresses if (!( TCP.exactMatch(ma) || @@ -123,13 +137,9 @@ export class UPnPPortMapper { continue } - const { port, host, family, transport } = ma.toOptions() + const { port, transport } = ma.toOptions() - if (family !== ipType) { - continue - } - - if (this.mappedPorts.has(`${host}-${port}-${transport}`)) { + if (this.mappedPorts.has(`${port}-${transport}`)) { continue } @@ -143,62 +153,49 @@ export class UPnPPortMapper { try { const externalHost = await this.externalAddress.getPublicIp() - let ipType: 4 | 6 = 4 - - if (isIPv4(externalHost)) { - ipType = 4 - } else if (isIPv6(externalHost)) { - ipType = 6 - } else { - throw new InvalidIPAddressError(`Public address ${externalHost} was not an IPv4 address`) - } - // filter addresses to get private, non-relay, IP based addresses that we // haven't mapped yet - const addresses = this.getUnmappedAddresses(this.addressManager.getAddresses(), ipType) + const addresses = this.getUnmappedAddresses(this.addressManager.getAddresses(), [externalHost]) if (addresses.length === 0) { this.log('no private, non-relay, unmapped, IP based addresses found') return } - this.log('%s public IP %s', this.externalAddress != null ? 'using configured' : 'discovered', externalHost) + this.log('discovered public IP %s', externalHost) this.assertNotBehindDoubleNAT(externalHost) for (const addr of addresses) { // try to open uPnP ports for each thin waist address - const { family, host, port, transport } = addr.toOptions() + const { port, host, transport, family } = addr.toOptions() - if (family === 6) { - // only support IPv4 addresses + // don't try to open port on IPv6 host via IPv4 gateway + if (family === 4 && this.gateway.family !== 'IPv4') { continue } - if (this.mappedPorts.has(`${host}-${port}-${transport}`)) { - // already mapped this port + // don't try to open port on IPv4 host via IPv6 gateway + if (family === 6 && this.gateway.family !== 'IPv6') { continue } - try { - const key = `${host}-${port}-${transport}` - this.log('creating mapping of key %s', key) + const key = `${host}-${port}-${transport}` - const externalPort = await this.gateway.map(port, { - localAddress: host, - protocol: transport === 'tcp' ? 'tcp' : 'udp' - }) + if (this.mappedPorts.has(key)) { + // already mapped this port + continue + } - this.mappedPorts.set(key, { - externalHost, - externalPort + try { + const mapping = await this.gateway.map(port, host, { + protocol: transport === 'tcp' ? 'TCP' : 'UDP' }) - - this.log('created mapping of %s:%s to %s:%s', externalHost, externalPort, host, port) - - this.addressManager.addPublicAddressMapping(host, port, externalHost, externalPort, transport === 'tcp' ? 'tcp' : 'udp') + this.mappedPorts.set(key, mapping) + this.addressManager.addPublicAddressMapping(mapping.internalHost, mapping.internalPort, mapping.externalHost, mapping.externalPort, transport === 'tcp' ? 'tcp' : 'udp') + this.log('created mapping of %s:%s to %s:%s for protocol %s', mapping.internalHost, mapping.internalPort, mapping.externalHost, mapping.externalPort, transport) } catch (err) { - this.log.error('failed to create mapping of %s:%s - %e', host, port, err) + this.log.error('failed to create mapping for %s:%d for protocol - %e', host, port, transport, err) } } } catch (err: any) { diff --git a/packages/upnp-nat/test/index.spec.ts b/packages/upnp-nat/test/index.spec.ts index b36388002d..63a539e248 100644 --- a/packages/upnp-nat/test/index.spec.ts +++ b/packages/upnp-nat/test/index.spec.ts @@ -35,7 +35,9 @@ describe('UPnP NAT (TCP)', () => { events: new TypedEventEmitter() } - gateway = stubInterface() + gateway = stubInterface({ + family: 'IPv4' + }) client = stubInterface({ findGateways: async function * (options) { yield gateway @@ -71,20 +73,35 @@ describe('UPnP NAT (TCP)', () => { components } = await createNatManager() - gateway.externalIp.resolves('82.3.1.5') + const internalHost = '192.168.1.12' + const internalPort = 4002 + + const externalHost = '82.3.1.5' + const externalPort = 4003 + + gateway.externalIp.resolves(externalHost) components.addressManager.getAddresses.returns([ multiaddr('/ip4/127.0.0.1/tcp/4002'), - multiaddr('/ip4/192.168.1.12/tcp/4002') + multiaddr(`/ip4/${internalHost}/tcp/${internalPort}`) ]) + gateway.map.withArgs(internalPort, internalHost).resolves({ + internalHost, + internalPort, + externalHost, + externalPort, + protocol: 'TCP' + }) + await start(natManager) await natManager.mapIpAddresses() expect(gateway.map.called).to.be.true() - expect(gateway.map.getCall(0).args[0]).to.equal(4002) - expect(gateway.map.getCall(0).args[1]).to.include({ - protocol: 'tcp' + expect(gateway.map.getCall(0).args[0]).to.equal(internalPort) + expect(gateway.map.getCall(0).args[1]).to.equal(internalHost) + expect(gateway.map.getCall(0).args[2]).to.include({ + protocol: 'TCP' }) expect(components.addressManager.addPublicAddressMapping.called).to.be.true() }) @@ -95,20 +112,35 @@ describe('UPnP NAT (TCP)', () => { components } = await createNatManager() - gateway.externalIp.resolves('82.3.1.5') + const internalHost = '192.168.1.12' + const internalPort = 4002 + + const externalHost = '82.3.1.5' + const externalPort = 4003 + + gateway.externalIp.resolves(externalHost) components.addressManager.getAddresses.returns([ multiaddr('/ip4/127.0.0.1/tcp/4002'), - multiaddr('/ip4/192.168.1.12/tcp/4002') + multiaddr(`/ip4/${internalHost}/tcp/${internalPort}`) ]) + gateway.map.withArgs(internalPort, internalHost).resolves({ + internalHost, + internalPort, + externalHost, + externalPort, + protocol: 'TCP' + }) + await start(natManager) await natManager.mapIpAddresses() expect(gateway.map.called).to.be.true() - expect(gateway.map.getCall(0).args[0]).to.equal(4002) - expect(gateway.map.getCall(0).args[1]).to.include({ - protocol: 'tcp' + expect(gateway.map.getCall(0).args[0]).to.equal(internalPort) + expect(gateway.map.getCall(0).args[1]).to.equal(internalHost) + expect(gateway.map.getCall(0).args[2]).to.include({ + protocol: 'TCP' }) expect(components.addressManager.addPublicAddressMapping.called).to.be.true() })