Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: map ports via uPnP when addresses change #1798

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix: map ports via uPnP when addresses change #1798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 39 additions & 20 deletions packages/libp2p/src/upnp-nat/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ import isPrivateIp from 'private-ip'
import { isBrowser } from 'wherearewe'
import { codes } from '../errors.js'
import * as pkg from '../version.js'
import type { PeerId } from '@libp2p/interface/peer-id'
import type { Startable } from '@libp2p/interface/startable'
import type { AddressManager } from '@libp2p/interface-internal/address-manager'
import type { Libp2pEvents } from '@libp2p/interface'
import type { PeerId } from '@libp2p/interface-peer-id'
import type { TransportManager } from '@libp2p/interface-internal/transport-manager'
import type { EventEmitter } from '@libp2p/interface/events'
import type { Startable } from '@libp2p/interface/startable'

const log = logger('libp2p:upnp-nat')
const DEFAULT_TTL = 7200
Expand Down Expand Up @@ -62,6 +64,7 @@ export interface UPnPNATComponents {
peerId: PeerId
transportManager: TransportManager
addressManager: AddressManager
events: EventEmitter<Libp2pEvents>
}

class UPnPNAT implements Startable {
Expand Down Expand Up @@ -89,36 +92,34 @@ class UPnPNAT implements Startable {
if (this.ttl < DEFAULT_TTL) {
throw new CodeError(`NatManager ttl should be at least ${DEFAULT_TTL} seconds`, codes.ERR_INVALID_PARAMETERS)
}

// try to map external ports when our address list changes
this.components.events.addEventListener('self:peer:update', () => {
this.mapAddresses()
.catch(err => {
log.error('error mapping external addresses', err)
})
})
}

isStarted (): boolean {
return this.started
}

start (): void {
// #TODO: is there a way to remove this? Seems like a hack
}

/**
* Attempt to use uPnP to configure port mapping using the current gateway.
*
* Run after start to ensure the transport manager has all addresses configured.
*/
afterStart (): void {
if (isBrowser || this.started) {
return
}

this.started = true

// done async to not slow down startup
void this._start().catch((err) => {
// hole punching errors are non-fatal
log.error(err)
})
}

async _start (): Promise<void> {
/**
* Attempt to use uPnP to configure port mapping using the current gateway.
*
* Run after start to ensure the transport manager has all addresses configured.
*/
async mapAddresses (): Promise<void> {
const addrs = this.components.transportManager.getAddrs()

for (const addr of addrs) {
Expand All @@ -128,21 +129,31 @@ class UPnPNAT implements Startable {
if (!addr.isThinWaistAddress() || transport !== 'tcp') {
// only bare tcp addresses
// eslint-disable-next-line no-continue
log.trace('skipping %a as it is not a think waist address', addr)
continue
}

if (isLoopback(addr)) {
// eslint-disable-next-line no-continue
log.trace('skipping %a as it is a loopback address', addr)
continue
}

if (family !== 4) {
// ignore ipv6
// eslint-disable-next-line no-continue
log.trace('skipping %a as it is not an ip4 address', addr)
continue
}

const client = this._getClient()

if (client.openPorts.map(p => p.localPort).includes(port)) {
// skip ports we have already mapped
log.trace('skipping %a as it is already mapped', addr)
continue
}

const publicIp = this.externalAddress ?? await client.externalIp()
const isPrivate = isPrivateIp(publicIp)

Expand All @@ -154,6 +165,10 @@ class UPnPNAT implements Startable {
throw new Error(`${publicIp} is not an IP address`)
}

if (!this.isStarted()) {
return
}

const publicPort = highPort()

log(`opening uPnP connection from ${publicIp}:${publicPort} to ${host}:${port}`)
Expand All @@ -165,11 +180,15 @@ class UPnPNAT implements Startable {
protocol: transport.toUpperCase() === 'TCP' ? 'TCP' : 'UDP'
})

this.components.addressManager.addObservedAddr(fromNodeAddress({
const externalAddress = fromNodeAddress({
family: 4,
address: publicIp,
port: publicPort
}, transport))
}, transport)

log('mapped external uPnP address %a', externalAddress)

this.components.addressManager.addObservedAddr(externalAddress)
}
}

Expand Down
29 changes: 22 additions & 7 deletions packages/libp2p/test/upnp-nat/upnp-nat.node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ import { createEd25519PeerId } from '@libp2p/peer-id-factory'
import { tcp } from '@libp2p/tcp'
import { multiaddr } from '@multiformats/multiaddr'
import { expect } from 'aegir/chai'
import delay from 'delay'
import { pEvent } from 'p-event'
import pWaitFor from 'p-wait-for'
import sinon from 'sinon'
import { type StubbedInstance, stubInterface } from 'sinon-ts'
import { DefaultAddressManager } from '../../src/address-manager/index.js'
import { defaultComponents, type Components } from '../../src/components.js'
Expand All @@ -18,25 +19,26 @@ import { DefaultTransportManager } from '../../src/transport-manager.js'
import { uPnPNATService } from '../../src/upnp-nat/index.js'
import type { NatAPI } from '@achingbrain/nat-port-mapper'
import type { PeerUpdate } from '@libp2p/interface'
import type { PeerId } from '@libp2p/interface/peer-id'
import type { PeerId } from '@libp2p/interface-peer-id'
import type { PeerData, PeerStore } from '@libp2p/interface/peer-store'

const DEFAULT_ADDRESSES = [
'/ip4/127.0.0.1/tcp/0',
'/ip4/0.0.0.0/tcp/0'
]

describe('UPnP NAT (TCP)', () => {
describe.only('UPnP NAT (TCP)', () => {
const teardown: Array<() => Promise<void>> = []
let client: StubbedInstance<NatAPI>

async function createNatManager (addrs = DEFAULT_ADDRESSES, natManagerOptions = {}): Promise<{ natManager: any, components: Components }> {
const events = new EventEmitter()
const peerStore: StubbedInstance<PeerStore> = stubInterface<PeerStore>()
const components: any = defaultComponents({
peerId: await createEd25519PeerId(),
upgrader: mockUpgrader({ events }),
events,
peerStore: stubInterface<PeerStore>()
peerStore
})

components.peerStore.patch.callsFake(async (peerId: PeerId, details: PeerData) => {
Expand All @@ -59,6 +61,7 @@ describe('UPnP NAT (TCP)', () => {
})(components)

client = stubInterface<NatAPI>()
client.openPorts = []

natManager._getClient = () => {
return client
Expand All @@ -67,11 +70,11 @@ describe('UPnP NAT (TCP)', () => {
components.transportManager.add(tcp()())

await start(components)
await components.transportManager.listen(components.addressManager.getListenAddrs())

teardown.push(async () => {
await stop(natManager)
await components.transportManager.removeAll()
await stop(components)
})

return {
Expand All @@ -95,7 +98,17 @@ describe('UPnP NAT (TCP)', () => {

await start(natManager)

await delay(100)
const addObservedAddr = sinon.spy(components.addressManager, 'addObservedAddr')

components.events.safeDispatchEvent('self:peer:update', {
detail: {
peer: {}
}
})

await pWaitFor(() => addObservedAddr.called, {
interval: 100
})

observed = components.addressManager.getObservedAddrs().map(ma => ma.toString())
expect(observed).to.not.be.empty()
Expand All @@ -108,6 +121,8 @@ describe('UPnP NAT (TCP)', () => {

expect(client.map.called).to.be.true()

console.info('internalPorts', internalPorts)

internalPorts.forEach(port => {
expect(client.map.getCall(0).args[0]).to.include({
localPort: port,
Expand Down Expand Up @@ -135,7 +150,7 @@ describe('UPnP NAT (TCP)', () => {
let observed = components.addressManager.getObservedAddrs().map(ma => ma.toString())
expect(observed).to.be.empty()

await expect(natManager._start()).to.eventually.be.rejectedWith(/double NAT/)
await expect(natManager.mapAddresses()).to.eventually.be.rejectedWith(/double NAT/)

observed = components.addressManager.getObservedAddrs().map(ma => ma.toString())
expect(observed).to.be.empty()
Expand Down
Loading