diff --git a/packages/peer-discovery-mdns/package.json b/packages/peer-discovery-mdns/package.json index 1e5b43e6f4..525edaeb1a 100644 --- a/packages/peer-discovery-mdns/package.json +++ b/packages/peer-discovery-mdns/package.json @@ -47,6 +47,7 @@ "@libp2p/interface": "^0.1.2", "@libp2p/logger": "^3.0.2", "@libp2p/peer-id": "^3.0.2", + "@libp2p/utils": "^4.0.2", "@multiformats/multiaddr": "^12.1.5", "@types/multicast-dns": "^7.2.1", "dns-packet": "^5.4.0", diff --git a/packages/peer-discovery-mdns/src/index.ts b/packages/peer-discovery-mdns/src/index.ts index 4f2f36ab89..7424131c06 100644 --- a/packages/peer-discovery-mdns/src/index.ts +++ b/packages/peer-discovery-mdns/src/index.ts @@ -6,6 +6,7 @@ import * as query from './query.js' import { stringGen } from './utils.js' import type { PeerDiscovery, PeerDiscoveryEvents } from '@libp2p/interface/peer-discovery' import type { PeerInfo } from '@libp2p/interface/peer-info' +import type { Startable } from '@libp2p/interface/src/startable.js' import type { AddressManager } from '@libp2p/interface-internal/address-manager' const log = logger('libp2p:mdns') @@ -23,7 +24,7 @@ export interface MulticastDNSComponents { addressManager: AddressManager } -class MulticastDNS extends EventEmitter implements PeerDiscovery { +class MulticastDNS extends EventEmitter implements PeerDiscovery, Startable { public mdns?: multicastDNS.MulticastDNS private readonly broadcast: boolean @@ -50,9 +51,10 @@ class MulticastDNS extends EventEmitter implements PeerDisc this.port = init.port ?? 5353 this.components = components this._queryInterval = null - this._onPeer = this._onPeer.bind(this) this._onMdnsQuery = this._onMdnsQuery.bind(this) this._onMdnsResponse = this._onMdnsResponse.bind(this) + this._onMdnsWarning = this._onMdnsWarning.bind(this) + this._onMdnsError = this._onMdnsError.bind(this) } readonly [peerDiscovery] = this @@ -76,6 +78,8 @@ class MulticastDNS extends EventEmitter implements PeerDisc this.mdns = multicastDNS({ port: this.port, ip: this.ip }) this.mdns.on('query', this._onMdnsQuery) this.mdns.on('response', this._onMdnsResponse) + this.mdns.on('warning', this._onMdnsWarning) + this.mdns.on('error', this._onMdnsError) this._queryInterval = query.queryLAN(this.mdns, this.serviceTag, this.interval) } @@ -113,14 +117,12 @@ class MulticastDNS extends EventEmitter implements PeerDisc } } - _onPeer (evt: CustomEvent): void { - if (this.mdns == null) { - return - } + _onMdnsWarning (err: Error): void { + log.error('mdns warning', err) + } - this.dispatchEvent(new CustomEvent('peer', { - detail: evt.detail - })) + _onMdnsError (err: Error): void { + log.error('mdns error', err) } /** @@ -135,6 +137,8 @@ class MulticastDNS extends EventEmitter implements PeerDisc this.mdns.removeListener('query', this._onMdnsQuery) this.mdns.removeListener('response', this._onMdnsResponse) + this.mdns.removeListener('warning', this._onMdnsWarning) + this.mdns.removeListener('error', this._onMdnsError) if (this._queryInterval != null) { clearInterval(this._queryInterval) diff --git a/packages/peer-discovery-mdns/src/query.ts b/packages/peer-discovery-mdns/src/query.ts index 2bc97b262c..bb6aa8ba30 100644 --- a/packages/peer-discovery-mdns/src/query.ts +++ b/packages/peer-discovery-mdns/src/query.ts @@ -1,6 +1,7 @@ import { logger } from '@libp2p/logger' import { peerIdFromString } from '@libp2p/peer-id' -import { multiaddr, type Multiaddr } from '@multiformats/multiaddr' +import { isPrivate } from '@libp2p/utils/multiaddr/is-private' +import { multiaddr, type Multiaddr, protocols } from '@multiformats/multiaddr' import type { PeerInfo } from '@libp2p/interface/peer-info' import type { Answer, StringAnswer, TxtAnswer } from 'dns-packet' import type { MulticastDNS, QueryPacket, ResponsePacket } from 'multicast-dns' @@ -9,7 +10,7 @@ const log = logger('libp2p:mdns:query') export function queryLAN (mdns: MulticastDNS, serviceTag: string, interval: number): ReturnType { const query = (): void => { - log('query', serviceTag) + log.trace('query', serviceTag) mdns.query({ questions: [{ @@ -40,6 +41,16 @@ export function gotResponse (rsp: ResponsePacket, localPeerName: string, service } }) + // according to the spec, peer details should be in the additional records, + // not the answers though it seems go-libp2p at least ignores this? + // https://github.com/libp2p/specs/blob/master/discovery/mdns.md#response + rsp.additionals.forEach((answer) => { + switch (answer.type) { + case 'TXT': txtAnswers.push(answer); break + default: break + } + }) + if (answerPTR == null || answerPTR?.name !== serviceTag || txtAnswers.length === 0 || @@ -63,7 +74,7 @@ export function gotResponse (rsp: ResponsePacket, localPeerName: string, service return { id: peerIdFromString(peerId), - multiaddrs, + multiaddrs: multiaddrs.map(addr => addr.decapsulateCode(protocols('p2p').code)), protocols: [] } } catch (e) { @@ -92,20 +103,45 @@ export function gotQuery (qry: QueryPacket, mdns: MulticastDNS, peerName: string data: peerName + '.' + serviceTag }) - multiaddrs.forEach((addr) => { - // spec mandates multiaddr contains peer id - if (addr.getPeerId() != null) { + multiaddrs + // mDNS requires link-local addresses only + // https://github.com/libp2p/specs/blob/master/discovery/mdns.md#issues + .filter(isLinkLocal) + .forEach((addr) => { + const data = 'dnsaddr=' + addr.toString() + + // TXT record fields have a max data length of 255 bytes + // see 6.1 - https://www.ietf.org/rfc/rfc6763.txt + if (data.length > 255) { + log('multiaddr %a is too long to use in mDNS query response', addr) + return + } + + // spec mandates multiaddr contains peer id + if (addr.getPeerId() == null) { + log('multiaddr %a did not have a peer ID so cannot be used in mDNS query response', addr) + return + } + answers.push({ name: peerName + '.' + serviceTag, type: 'TXT', class: 'IN', ttl: 120, - data: 'dnsaddr=' + addr.toString() + data }) - } - }) + }) - log('responding to query') + log.trace('responding to query') mdns.respond(answers) } } + +function isLinkLocal (ma: Multiaddr): boolean { + // match private ip4/ip6 & loopback addresses + if (isPrivate(ma)) { + return true + } + + return false +} diff --git a/packages/peer-discovery-mdns/test/compliance.spec.ts b/packages/peer-discovery-mdns/test/compliance.spec.ts index a823f9ed35..b24d590d91 100644 --- a/packages/peer-discovery-mdns/test/compliance.spec.ts +++ b/packages/peer-discovery-mdns/test/compliance.spec.ts @@ -1,6 +1,7 @@ /* eslint-env mocha */ import { CustomEvent } from '@libp2p/interface/events' +import { isStartable } from '@libp2p/interface/startable' import tests from '@libp2p/interface-compliance-tests/peer-discovery' import { createEd25519PeerId } from '@libp2p/peer-id-factory' import { multiaddr } from '@multiformats/multiaddr' @@ -32,16 +33,21 @@ describe('compliance tests', () => { }) // Trigger discovery - const maStr = '/ip4/127.0.0.1/tcp/15555/ws/p2p-webrtc-star/p2p/QmcgpsyWgH8Y8ajJz1Cu72KnS5uo2Aa2LpzU7kinSooo2d' - - // @ts-expect-error not a PeerDiscovery field - intervalId = setInterval(() => discovery._onPeer(new CustomEvent('peer', { - detail: { - id: peerId2, - multiaddrs: [multiaddr(maStr)], - protocols: [] + const maStr = '/ip4/127.0.0.1/tcp/15555/ws/p2p-webrtc-star' + + intervalId = setInterval(() => { + if (isStartable(discovery) && !discovery.isStarted()) { + return } - })), 1000) + + discovery.dispatchEvent(new CustomEvent('peer', { + detail: { + id: peerId2, + multiaddrs: [multiaddr(maStr)], + protocols: [] + } + })) + }, 1000) return discovery }, diff --git a/packages/peer-discovery-mdns/test/multicast-dns.spec.ts b/packages/peer-discovery-mdns/test/multicast-dns.spec.ts index 3ddf5c8ea9..9955002dd7 100644 --- a/packages/peer-discovery-mdns/test/multicast-dns.spec.ts +++ b/packages/peer-discovery-mdns/test/multicast-dns.spec.ts @@ -38,25 +38,25 @@ describe('MulticastDNS', () => { ]) aMultiaddrs = [ - multiaddr('/ip4/127.0.0.1/tcp/20001'), + multiaddr('/ip4/192.168.1.142/tcp/20001'), multiaddr('/dns4/webrtc-star.discovery.libp2p.io/tcp/443/wss/p2p-webrtc-star'), multiaddr('/dns4/discovery.libp2p.io/tcp/8443') ] bMultiaddrs = [ - multiaddr('/ip4/127.0.0.1/tcp/20002'), - multiaddr('/ip6/::1/tcp/20002'), + multiaddr('/ip4/192.168.1.143/tcp/20002'), + multiaddr('/ip6/2604:1380:4602:5c00::3/tcp/20002'), multiaddr('/dnsaddr/discovery.libp2p.io') ] cMultiaddrs = [ - multiaddr('/ip4/127.0.0.1/tcp/20003'), - multiaddr('/ip4/127.0.0.1/tcp/30003/ws'), + multiaddr('/ip4/192.168.1.144/tcp/20003'), + multiaddr('/ip4/192.168.1.144/tcp/30003/ws'), multiaddr('/dns4/discovery.libp2p.io') ] dMultiaddrs = [ - multiaddr('/ip4/127.0.0.1/tcp/30003/ws') + multiaddr('/ip4/192.168.1.145/tcp/30003/ws') ] }) @@ -110,7 +110,8 @@ describe('MulticastDNS', () => { await pWaitFor(() => peers.has(expectedPeer)) mdnsA.removeEventListener('peer', foundPeer) - expect(peers.get(expectedPeer).multiaddrs.length).to.equal(3) + // everything except loopback + expect(peers.get(expectedPeer).multiaddrs.length).to.equal(2) await stop(mdnsA, mdnsB, mdnsD) }) @@ -141,15 +142,6 @@ describe('MulticastDNS', () => { await stop(mdnsC) }) - it('should start and stop with go-libp2p-mdns compat', async () => { - const mdnsA = mdns({ - port: 50004 - })(getComponents(pA, aMultiaddrs)) - - await start(mdnsA) - await stop(mdnsA) - }) - it('should not emit undefined peer ids', async () => { const mdnsA = mdns({ port: 50004 @@ -219,4 +211,80 @@ describe('MulticastDNS', () => { await stop(mdnsA, mdnsB) }) + + it('only includes link-local addresses', async function () { + this.timeout(40 * 1000) + + // these are not link-local addresses + const publicAddress = '/ip4/48.52.76.32/tcp/1234' + const relayDnsAddress = `/dnsaddr/example.org/tcp/1234/p2p/${pD.toString()}/p2p-circuit` + const dnsAddress = '/dns4/example.org/tcp/1234' + + // this address is too long to fit in a TXT record + const longAddress = `/ip4/192.168.1.142/udp/4001/quic-v1/webtransport/certhash/uEiDils3hWFJmsWOJIoMPxAcpzlyFNxTDZpklIoB8643ddw/certhash/uEiAM4BGr4OMK3O9cFGwfbNc4J7XYnsKE5wNPKKaTLa4fkw/p2p/${pD.toString()}/p2p-circuit` + + // these are link-local addresses + const relayAddress = `/ip4/192.168.1.142/tcp/1234/p2p/${pD.toString()}/p2p-circuit` + const localAddress = '/ip4/192.168.1.123/tcp/1234' + const localWsAddress = '/ip4/192.168.1.123/tcp/1234/ws' + + // these are not link-local but go-libp2p advertises loopback addresses even + // though you shouldn't for mDNS + const loopbackAddress = '/ip4/127.0.0.1/tcp/1234' + const loopbackAddress6 = '/ip6/::1/tcp/1234' + + const mdnsA = mdns({ + broadcast: false, // do not talk to ourself + port: 50005, + ip: '224.0.0.252' + })(getComponents(pA, aMultiaddrs)) + + const mdnsB = mdns({ + port: 50005, // port must be the same + ip: '224.0.0.252' // ip must be the same + })(getComponents(pB, [ + multiaddr(publicAddress), + multiaddr(relayAddress), + multiaddr(relayDnsAddress), + multiaddr(localAddress), + multiaddr(loopbackAddress), + multiaddr(loopbackAddress6), + multiaddr(dnsAddress), + multiaddr(longAddress), + multiaddr(localWsAddress) + ])) + + await start(mdnsA, mdnsB) + + const { detail: { id, multiaddrs } } = await new Promise>((resolve) => { + mdnsA.addEventListener('peer', resolve, { + once: true + }) + }) + + expect(pB.toString()).to.eql(id.toString()) + + ;[ + publicAddress, + relayDnsAddress, + dnsAddress, + longAddress + ].forEach(addr => { + expect(multiaddrs.map(ma => ma.toString())) + .to.not.include(addr) + }) + + ;[ + relayAddress, + localAddress, + localWsAddress, + loopbackAddress, + loopbackAddress6 + ].forEach(addr => { + expect(multiaddrs.map(ma => ma.toString())) + .to.include(addr) + }) + + await stop(mdnsA, mdnsB) + }) })