Skip to content

Commit 406b391

Browse files
achingbrainachingbrain
authored
fix: support IPv6 with IPv4 (#2864)
Upgrades to latest nat-port-mapper for better IPv6 support.
1 parent 97978b9 commit 406b391

File tree

7 files changed

+101
-61
lines changed

7 files changed

+101
-61
lines changed

‎packages/upnp-nat/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
"test:electron-main": "aegir test -t electron-main"
5151
},
5252
"dependencies": {
53-
"@achingbrain/nat-port-mapper": "^3.0.2",
53+
"@achingbrain/nat-port-mapper": "^4.0.0",
5454
"@chainsafe/is-ip": "^2.0.2",
5555
"@libp2p/interface": "^2.2.1",
5656
"@libp2p/interface-internal": "^2.1.1",

‎packages/upnp-nat/src/constants.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,2 @@
1-
export const DEFAULT_PORT_MAPPING_TTL = 720_000
21
export const DEFAULT_GATEWAY_SEARCH_TIMEOUT = 60_000
32
export const DEFAULT_GATEWAY_SEARCH_INTERVAL = 300_000

‎packages/upnp-nat/src/gateway-finder.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,13 @@ export class GatewayFinder extends TypedEventEmitter<GatewayFinderEvents> {
3434

3535
// every five minutes, search for network gateways for one minute
3636
this.findGateways = repeatingTask(async (options) => {
37-
for await (const gateway of this.portMappingClient.findGateways(options)) {
38-
if (this.gateways.some(g => g.id === gateway.id)) {
37+
for await (const gateway of this.portMappingClient.findGateways({
38+
...options,
39+
searchInterval: 10000
40+
})) {
41+
if (this.gateways.some(g => {
42+
return g.id === gateway.id && g.family === gateway.family
43+
})) {
3944
// already seen this gateway
4045
continue
4146
}

‎packages/upnp-nat/src/index.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,13 @@ export interface UPnPNATInit {
8686
*/
8787
portMappingAutoRefresh?: boolean
8888

89+
/**
90+
* How long before a port mapping expires to refresh it in ms
91+
*
92+
* @default 60_000
93+
*/
94+
portMappingRefreshThreshold?: number
95+
8996
/**
9097
* A preconfigured instance of a NatAPI client can be passed as an option,
9198
* otherwise one will be created

‎packages/upnp-nat/src/upnp-nat.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { upnpNat } from '@achingbrain/nat-port-mapper'
22
import { serviceCapabilities, setMaxListeners, start, stop } from '@libp2p/interface'
33
import { debounce } from '@libp2p/utils/debounce'
4-
import { DEFAULT_PORT_MAPPING_TTL } from './constants.js'
54
import { GatewayFinder } from './gateway-finder.js'
65
import { UPnPPortMapper } from './upnp-port-mapper.js'
76
import type { UPnPNATComponents, UPnPNATInit, UPnPNAT as UPnPNATInterface } from './index.js'
@@ -29,8 +28,9 @@ export class UPnPNAT implements Startable, UPnPNATInterface {
2928

3029
this.portMappingClient = init.portMappingClient ?? upnpNat({
3130
description: init.portMappingDescription ?? `${components.nodeInfo.name}@${components.nodeInfo.version} ${components.peerId.toString()}`,
32-
ttl: init.portMappingTTL ?? DEFAULT_PORT_MAPPING_TTL,
33-
autoRefresh: init.portMappingAutoRefresh ?? true
31+
ttl: init.portMappingTTL,
32+
autoRefresh: init.portMappingAutoRefresh,
33+
refreshThreshold: init.portMappingRefreshThreshold
3434
})
3535

3636
// trigger update when our addresses change

‎packages/upnp-nat/src/upnp-port-mapper.ts

Lines changed: 40 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
import { isIPv4, isIPv6 } from '@chainsafe/is-ip'
1+
import { isIPv4 } from '@chainsafe/is-ip'
22
import { InvalidParametersError, start, stop } from '@libp2p/interface'
3+
import { isLinkLocal } from '@libp2p/utils/multiaddr/is-link-local'
34
import { isLoopback } from '@libp2p/utils/multiaddr/is-loopback'
45
import { isPrivate } from '@libp2p/utils/multiaddr/is-private'
56
import { isPrivateIp } from '@libp2p/utils/private-ip'
67
import { QUICV1, TCP, WebSockets, WebSocketsSecure, WebTransport } from '@multiformats/multiaddr-matcher'
78
import { dynamicExternalAddress } from './check-external-address.js'
8-
import { DoubleNATError, InvalidIPAddressError } from './errors.js'
9+
import { DoubleNATError } from './errors.js'
910
import type { ExternalAddress } from './check-external-address.js'
1011
import type { Gateway } from '@achingbrain/nat-port-mapper'
1112
import type { ComponentLogger, Logger } from '@libp2p/interface'
@@ -98,12 +99,20 @@ export class UPnPPortMapper {
9899
/**
99100
* Return any eligible multiaddrs that are not mapped on the detected gateway
100101
*/
101-
private getUnmappedAddresses (multiaddrs: Multiaddr[], ipType: 4 | 6): Multiaddr[] {
102+
private getUnmappedAddresses (multiaddrs: Multiaddr[], publicAddresses: string[]): Multiaddr[] {
102103
const output: Multiaddr[] = []
103104

104105
for (const ma of multiaddrs) {
105-
// ignore public addresses
106-
if (!isPrivate(ma)) {
106+
const stringTuples = ma.stringTuples()
107+
const address = `${stringTuples[0][1]}`
108+
109+
// ignore public IPv4 addresses
110+
if (isIPv4(address) && !isPrivate(ma)) {
111+
continue
112+
}
113+
114+
// ignore any addresses that match the interface on the network gateway
115+
if (publicAddresses.includes(address)) {
107116
continue
108117
}
109118

@@ -112,6 +121,11 @@ export class UPnPPortMapper {
112121
continue
113122
}
114123

124+
// ignore link-local addresses
125+
if (isLinkLocal(ma)) {
126+
continue
127+
}
128+
115129
// only IP based addresses
116130
if (!(
117131
TCP.exactMatch(ma) ||
@@ -123,13 +137,9 @@ export class UPnPPortMapper {
123137
continue
124138
}
125139

126-
const { port, host, family, transport } = ma.toOptions()
140+
const { port, transport } = ma.toOptions()
127141

128-
if (family !== ipType) {
129-
continue
130-
}
131-
132-
if (this.mappedPorts.has(`${host}-${port}-${transport}`)) {
142+
if (this.mappedPorts.has(`${port}-${transport}`)) {
133143
continue
134144
}
135145

@@ -143,62 +153,49 @@ export class UPnPPortMapper {
143153
try {
144154
const externalHost = await this.externalAddress.getPublicIp()
145155

146-
let ipType: 4 | 6 = 4
147-
148-
if (isIPv4(externalHost)) {
149-
ipType = 4
150-
} else if (isIPv6(externalHost)) {
151-
ipType = 6
152-
} else {
153-
throw new InvalidIPAddressError(`Public address ${externalHost} was not an IPv4 address`)
154-
}
155-
156156
// filter addresses to get private, non-relay, IP based addresses that we
157157
// haven't mapped yet
158-
const addresses = this.getUnmappedAddresses(this.addressManager.getAddresses(), ipType)
158+
const addresses = this.getUnmappedAddresses(this.addressManager.getAddresses(), [externalHost])
159159

160160
if (addresses.length === 0) {
161161
this.log('no private, non-relay, unmapped, IP based addresses found')
162162
return
163163
}
164164

165-
this.log('%s public IP %s', this.externalAddress != null ? 'using configured' : 'discovered', externalHost)
165+
this.log('discovered public IP %s', externalHost)
166166

167167
this.assertNotBehindDoubleNAT(externalHost)
168168

169169
for (const addr of addresses) {
170170
// try to open uPnP ports for each thin waist address
171-
const { family, host, port, transport } = addr.toOptions()
171+
const { port, host, transport, family } = addr.toOptions()
172172

173-
if (family === 6) {
174-
// only support IPv4 addresses
173+
// don't try to open port on IPv6 host via IPv4 gateway
174+
if (family === 4 && this.gateway.family !== 'IPv4') {
175175
continue
176176
}
177177

178-
if (this.mappedPorts.has(`${host}-${port}-${transport}`)) {
179-
// already mapped this port
178+
// don't try to open port on IPv4 host via IPv6 gateway
179+
if (family === 6 && this.gateway.family !== 'IPv6') {
180180
continue
181181
}
182182

183-
try {
184-
const key = `${host}-${port}-${transport}`
185-
this.log('creating mapping of key %s', key)
183+
const key = `${host}-${port}-${transport}`
186184

187-
const externalPort = await this.gateway.map(port, {
188-
localAddress: host,
189-
protocol: transport === 'tcp' ? 'tcp' : 'udp'
190-
})
185+
if (this.mappedPorts.has(key)) {
186+
// already mapped this port
187+
continue
188+
}
191189

192-
this.mappedPorts.set(key, {
193-
externalHost,
194-
externalPort
190+
try {
191+
const mapping = await this.gateway.map(port, host, {
192+
protocol: transport === 'tcp' ? 'TCP' : 'UDP'
195193
})
196-
197-
this.log('created mapping of %s:%s to %s:%s', externalHost, externalPort, host, port)
198-
199-
this.addressManager.addPublicAddressMapping(host, port, externalHost, externalPort, transport === 'tcp' ? 'tcp' : 'udp')
194+
this.mappedPorts.set(key, mapping)
195+
this.addressManager.addPublicAddressMapping(mapping.internalHost, mapping.internalPort, mapping.externalHost, mapping.externalPort, transport === 'tcp' ? 'tcp' : 'udp')
196+
this.log('created mapping of %s:%s to %s:%s for protocol %s', mapping.internalHost, mapping.internalPort, mapping.externalHost, mapping.externalPort, transport)
200197
} catch (err) {
201-
this.log.error('failed to create mapping of %s:%s - %e', host, port, err)
198+
this.log.error('failed to create mapping for %s:%d for protocol - %e', host, port, transport, err)
202199
}
203200
}
204201
} catch (err: any) {

‎packages/upnp-nat/test/index.spec.ts

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ describe('UPnP NAT (TCP)', () => {
3535
events: new TypedEventEmitter()
3636
}
3737

38-
gateway = stubInterface<Gateway>()
38+
gateway = stubInterface<Gateway>({
39+
family: 'IPv4'
40+
})
3941
client = stubInterface<UPnPNATClient>({
4042
findGateways: async function * (options) {
4143
yield gateway
@@ -71,20 +73,35 @@ describe('UPnP NAT (TCP)', () => {
7173
components
7274
} = await createNatManager()
7375

74-
gateway.externalIp.resolves('82.3.1.5')
76+
const internalHost = '192.168.1.12'
77+
const internalPort = 4002
78+
79+
const externalHost = '82.3.1.5'
80+
const externalPort = 4003
81+
82+
gateway.externalIp.resolves(externalHost)
7583

7684
components.addressManager.getAddresses.returns([
7785
multiaddr('/ip4/127.0.0.1/tcp/4002'),
78-
multiaddr('/ip4/192.168.1.12/tcp/4002')
86+
multiaddr(`/ip4/${internalHost}/tcp/${internalPort}`)
7987
])
8088

89+
gateway.map.withArgs(internalPort, internalHost).resolves({
90+
internalHost,
91+
internalPort,
92+
externalHost,
93+
externalPort,
94+
protocol: 'TCP'
95+
})
96+
8197
await start(natManager)
8298
await natManager.mapIpAddresses()
8399

84100
expect(gateway.map.called).to.be.true()
85-
expect(gateway.map.getCall(0).args[0]).to.equal(4002)
86-
expect(gateway.map.getCall(0).args[1]).to.include({
87-
protocol: 'tcp'
101+
expect(gateway.map.getCall(0).args[0]).to.equal(internalPort)
102+
expect(gateway.map.getCall(0).args[1]).to.equal(internalHost)
103+
expect(gateway.map.getCall(0).args[2]).to.include({
104+
protocol: 'TCP'
88105
})
89106
expect(components.addressManager.addPublicAddressMapping.called).to.be.true()
90107
})
@@ -95,20 +112,35 @@ describe('UPnP NAT (TCP)', () => {
95112
components
96113
} = await createNatManager()
97114

98-
gateway.externalIp.resolves('82.3.1.5')
115+
const internalHost = '192.168.1.12'
116+
const internalPort = 4002
117+
118+
const externalHost = '82.3.1.5'
119+
const externalPort = 4003
120+
121+
gateway.externalIp.resolves(externalHost)
99122

100123
components.addressManager.getAddresses.returns([
101124
multiaddr('/ip4/127.0.0.1/tcp/4002'),
102-
multiaddr('/ip4/192.168.1.12/tcp/4002')
125+
multiaddr(`/ip4/${internalHost}/tcp/${internalPort}`)
103126
])
104127

128+
gateway.map.withArgs(internalPort, internalHost).resolves({
129+
internalHost,
130+
internalPort,
131+
externalHost,
132+
externalPort,
133+
protocol: 'TCP'
134+
})
135+
105136
await start(natManager)
106137
await natManager.mapIpAddresses()
107138

108139
expect(gateway.map.called).to.be.true()
109-
expect(gateway.map.getCall(0).args[0]).to.equal(4002)
110-
expect(gateway.map.getCall(0).args[1]).to.include({
111-
protocol: 'tcp'
140+
expect(gateway.map.getCall(0).args[0]).to.equal(internalPort)
141+
expect(gateway.map.getCall(0).args[1]).to.equal(internalHost)
142+
expect(gateway.map.getCall(0).args[2]).to.include({
143+
protocol: 'TCP'
112144
})
113145
expect(components.addressManager.addPublicAddressMapping.called).to.be.true()
114146
})

0 commit comments

Comments
 (0)