From 7d3c86b35c2fac2d2eaf92b1219a37fc08c18291 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Mon, 17 Mar 2025 17:36:52 +0100 Subject: [PATCH] fix: only check for non-wildcard ports If we are passed a wildcard port, start a UDP mux listener on a ephemeral port, otherwise check the existing listeners before trying to start a new one. To be reused an existing listener must be owned by the current peer and be of the other IP family (e.g. you cannot listen on `[ip4-host]:1234` twice but you can listen on `[ip4-host:1234]` and `[ip6-host]:1234`. Fixes #3049 --- .../src/private-to-public/listener.ts | 70 +++++++++---------- .../transport-webrtc/test/transport.spec.ts | 70 ++++++++++++++++++- 2 files changed, 103 insertions(+), 37 deletions(-) diff --git a/packages/transport-webrtc/src/private-to-public/listener.ts b/packages/transport-webrtc/src/private-to-public/listener.ts index 4127169245..b643d11f3a 100644 --- a/packages/transport-webrtc/src/private-to-public/listener.ts +++ b/packages/transport-webrtc/src/private-to-public/listener.ts @@ -1,5 +1,5 @@ -import { isIPv4, isIPv6 } from '@chainsafe/is-ip' -import { InvalidPeerIdError, TypedEventEmitter } from '@libp2p/interface' +import { isIPv4 } from '@chainsafe/is-ip' +import { InvalidParametersError, InvalidPeerIdError, TypedEventEmitter } from '@libp2p/interface' import { getThinWaistAddresses } from '@libp2p/utils/get-thin-waist-addresses' import { multiaddr, fromStringTuples } from '@multiformats/multiaddr' import { WebRTCDirect } from '@multiformats/multiaddr-matcher' @@ -83,53 +83,53 @@ export class WebRTCDirectListener extends TypedEventEmitter impl } async listen (ma: Multiaddr): Promise { - const { host, port } = ma.toOptions() - - // have to do this before any async work happens so starting two listeners - // for the same port concurrently (e.g. ipv4/ipv6 both port 0) results in a - // single mux listener. This is necessary because libjuice binds to all - // interfaces for a given port so we we need to key on just the port number - // not the host + the port number - let existingServer = UDP_MUX_LISTENERS.find(s => s.port === port) - - // if the server has not been started yet, or the port is a wildcard port - // and there is already a wildcard port for this address family, start a new - // UDP mux server - const wildcardPorts = port === 0 && existingServer?.port === 0 - const sameAddressFamily = (existingServer?.isIPv4 === true && isIPv4(host)) || (existingServer?.isIPv6 === true && isIPv6(host)) - let createdMuxServer = false - - if (existingServer == null || (wildcardPorts && sameAddressFamily)) { - this.log('starting UDP mux server on %s:%p', host, port) - existingServer = this.startUDPMuxServer(host, port) - UDP_MUX_LISTENERS.push(existingServer) - createdMuxServer = true - } + const { host, port, family } = ma.toOptions() + + let udpMuxServer: UDPMuxServer | undefined + + if (port !== 0) { + // libjuice binds to all interfaces (IPv4/IPv6) for a given port so if we + // want to listen on a specific port, and there's already a mux listener + // for that port for the other family started by this node, we should + // reuse it + udpMuxServer = UDP_MUX_LISTENERS.find(s => s.port === port) + + // make sure the port is free for the given family + if (udpMuxServer != null && ((udpMuxServer.isIPv4 && family === 4) || (udpMuxServer.isIPv6 && family === 6))) { + throw new InvalidParametersError(`There is already a listener for ${host}:${port}`) + } - if (!existingServer.peerId.equals(this.components.peerId)) { - // this would have to be another in-process peer so we are likely in a - // testing environment - throw new InvalidPeerIdError(`Another peer is already performing UDP mux on ${host}:${existingServer.port}`) + // check that we own the mux server + if (udpMuxServer != null && !udpMuxServer.peerId.equals(this.components.peerId)) { + throw new InvalidPeerIdError(`Another peer is already performing UDP mux on ${host}:${port}`) + } } - this.stunServer = await existingServer.server - const address = this.stunServer.address() + // start the mux server if we don't have one already + if (udpMuxServer == null) { + this.log('starting UDP mux server on %s:%p', host, port) + udpMuxServer = this.startUDPMuxServer(host, port, family) + UDP_MUX_LISTENERS.push(udpMuxServer) + } - if (!createdMuxServer) { - this.log('reused existing UDP mux server on %s:%p', host, address.port) + if (family === 4) { + udpMuxServer.isIPv4 = true + } else if (family === 6) { + udpMuxServer.isIPv6 = true } + this.stunServer = await udpMuxServer.server this.listeningMultiaddr = ma this.safeDispatchEvent('listening') } - private startUDPMuxServer (host: string, port: number): UDPMuxServer { + private startUDPMuxServer (host: string, port: number, family: 4 | 6): UDPMuxServer { return { peerId: this.components.peerId, owner: this, port, - isIPv4: isIPv4(host), - isIPv6: isIPv6(host), + isIPv4: family === 4, + isIPv6: family === 6, server: Promise.resolve() .then(async (): Promise => { // ensure we have a certificate diff --git a/packages/transport-webrtc/test/transport.spec.ts b/packages/transport-webrtc/test/transport.spec.ts index f55773004d..1fd806887a 100644 --- a/packages/transport-webrtc/test/transport.spec.ts +++ b/packages/transport-webrtc/test/transport.spec.ts @@ -114,8 +114,8 @@ describe('WebRTCDirect Transport', () => { return this.skip() } - const ipv4 = multiaddr('/ip4/127.0.0.1/udp/0') - const ipv6 = multiaddr('/ip6/::1/udp/0') + const ipv4 = multiaddr('/ip4/127.0.0.1/udp/37287') + const ipv6 = multiaddr('/ip6/::1/udp/37287') await Promise.all([ listener.listen(ipv4), @@ -208,4 +208,70 @@ describe('WebRTCDirect Transport', () => { expect(ma.toString()).to.include('/udp/12346/webrtc-direct/certhash/u', 'did not add certhash to all WebRTC Direct addresses') } }) + + it('can start listeners for two nodes on wildcard socket addresses', async function () { + if ((!isNode && !isElectronMain) || !supportsIpV6()) { + return this.skip() + } + + const otherTransport = new WebRTCDirectTransport({ + ...components, + peerId: peerIdFromPrivateKey(await generateKeyPair('Ed25519')) + }) + const otherTransportIp4Listener = otherTransport.createListener({ + upgrader + }) + const otherTransportIp6Listener = otherTransport.createListener({ + upgrader + }) + + const ip6Listener = transport.createListener({ + upgrader + }) + + const ipv4 = multiaddr('/ip4/0.0.0.0/udp/0') + const ipv6 = multiaddr('/ip6/::/udp/0') + + await Promise.all([ + listener.listen(ipv4), + otherTransportIp4Listener.listen(ipv4), + ip6Listener.listen(ipv6), + otherTransportIp6Listener.listen(ipv6) + ]) + + assertAllMultiaddrsHaveSamePort(listener.getAddrs()) + assertAllMultiaddrsHaveSamePort(otherTransportIp4Listener.getAddrs()) + assertAllMultiaddrsHaveSamePort(otherTransportIp6Listener.getAddrs()) + assertAllMultiaddrsHaveSamePort(ip6Listener.getAddrs()) + + await listener.close() + await otherTransportIp4Listener.close() + await otherTransportIp6Listener.close() + await ip6Listener.close() + }) + + it('can start multiple wildcard listeners', async function () { + if ((!isNode && !isElectronMain) || !supportsIpV6()) { + return this.skip() + } + + const otherListener = transport.createListener({ + upgrader + }) + + const ipv4 = multiaddr('/ip4/0.0.0.0/udp/0') + + await Promise.all([ + listener.listen(ipv4), + otherListener.listen(ipv4) + ]) + + assertAllMultiaddrsHaveSamePort(listener.getAddrs()) + assertAllMultiaddrsHaveSamePort(otherListener.getAddrs()) + + expect(listener.getAddrs()[0].toOptions().port).to.not.equal(otherListener.getAddrs()[0].toOptions().port, 'wildcard listeners did not listen on different ports') + + await listener.close() + await otherListener.close() + }) })