Skip to content

Commit

Permalink
fix(libp2p): filter out dnsaddrs for different peers (#1954)
Browse files Browse the repository at this point in the history
Some multiaddrs like `/dnsaddr/bootstrap.ipfs.io` resolve to multiple
PeerIds.

E.g:

```console
% dig _dnsaddr.bootstrap.libp2p.io TXT

; <<>> DiG 9.10.6 <<>> _dnsaddr.bootstrap.libp2p.io TXT
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 53473
;; flags: qr rd ra; QUERY: 1, ANSWER: 4, AUTHORITY: 0, ADDITIONAL: 1

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 1232
;; QUESTION SECTION:
;_dnsaddr.bootstrap.libp2p.io.	IN	TXT

;; ANSWER SECTION:
_dnsaddr.bootstrap.libp2p.io. 261 IN	TXT	"dnsaddr=/dnsaddr/am6.bootstrap.libp2p.io/p2p/QmbLHAnMoJPWSCR5Zhtx6BHJX9KiKNN6tpvbUcqanj75Nb"
_dnsaddr.bootstrap.libp2p.io. 261 IN	TXT	"dnsaddr=/dnsaddr/ny5.bootstrap.libp2p.io/p2p/QmQCU2EcMqAqQPR2i9bChDtGNJchTbq5TbXJJ16u19uLTa"
_dnsaddr.bootstrap.libp2p.io. 261 IN	TXT	"dnsaddr=/dnsaddr/sg1.bootstrap.libp2p.io/p2p/QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt"
_dnsaddr.bootstrap.libp2p.io. 261 IN	TXT	"dnsaddr=/dnsaddr/sv15.bootstrap.libp2p.io/p2p/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN"
```

This can cause problems when dialing those addresses with a PeerId
since resolving the dnsaddr returns addresses with embedded PeerIds
that aren't for the node you are trying to dial so filter those out.

The errors these cause are non-fatal but do consume resources dialing
nodes we aren't going to be able to connect to.
  • Loading branch information
achingbrain authored Aug 13, 2023
1 parent 4c1a33b commit a31b420
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 4 deletions.
18 changes: 16 additions & 2 deletions packages/libp2p/src/connection-manager/dial-queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,22 @@ export class DialQueue {
))
.flat()

// filter out any multiaddrs that we do not have transports for
const filteredAddrs = resolvedAddresses.filter(addr => Boolean(this.transportManager.transportForMultiaddr(addr.multiaddr)))
const filteredAddrs = resolvedAddresses.filter(addr => {
// filter out any multiaddrs that we do not have transports for
if (this.transportManager.transportForMultiaddr(addr.multiaddr) == null) {
return false
}

// if the resolved multiaddr has a PeerID but it's the wrong one, ignore it
// - this can happen with addresses like bootstrap.libp2p.io that resolve
// to multiple different peers
const addrPeerId = addr.multiaddr.getPeerId()
if (peerId != null && addrPeerId != null) {
return peerId.equals(addrPeerId)
}

return true
})

// deduplicate addresses
const dedupedAddrs = new Map<string, Address>()
Expand Down
50 changes: 49 additions & 1 deletion packages/libp2p/test/connection-manager/dial-queue.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { mockConnection, mockDuplex, mockMultiaddrConnection } from '@libp2p/interface-compliance-tests/mocks'
import { createEd25519PeerId } from '@libp2p/peer-id-factory'
import { multiaddr } from '@multiformats/multiaddr'
import { multiaddr, resolvers } from '@multiformats/multiaddr'
import { expect } from 'aegir/chai'
import delay from 'delay'
import pDefer from 'p-defer'
Expand Down Expand Up @@ -280,4 +280,52 @@ describe('dial queue', () => {
// Dial attempt finished without connection
expect(signals['/ip4/127.0.0.1/tcp/1233']).to.have.property('aborted', true)
})

it('should ignore DNS addresses for other peers', async () => {
const remotePeer = await createEd25519PeerId()
const otherRemotePeer = await createEd25519PeerId()
const ma = multiaddr(`/dnsaddr/example.com/p2p/${remotePeer}`)
const maStr = `/ip4/123.123.123.123/tcp/2348/p2p/${remotePeer}`
const resolvedAddresses = [
`/ip4/234.234.234.234/tcp/4213/p2p/${otherRemotePeer}`,
maStr
]

let resolvedDNSAddrs = false
let dialedBadAddress = false

// simulate a DNSAddr that resolves to multiple different peers like
// bootstrap.libp2p.io
resolvers.set('dnsaddr', async (addr) => {
if (addr.equals(ma)) {
resolvedDNSAddrs = true
return resolvedAddresses
}

return []
})

dialer = new DialQueue(components, {
maxParallelDials: 50
})
components.transportManager.transportForMultiaddr.returns(stubInterface<Transport>())

const connection = mockConnection(mockMultiaddrConnection(mockDuplex(), remotePeer))

components.transportManager.dial.callsFake(async (ma, opts = {}) => {
if (ma.toString() === maStr) {
await delay(100)
return connection
}

dialedBadAddress = true
throw new Error('Could not dial address')
})

await expect(dialer.dial(ma)).to.eventually.equal(connection)
expect(resolvedDNSAddrs).to.be.true('Did not resolve DNSAddrs')
expect(dialedBadAddress).to.be.false('Dialed address with wrong peer id')

resolvers.delete('dnsaddr')
})
})
2 changes: 1 addition & 1 deletion packages/libp2p/test/connection-manager/direct.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ describe('dialing (direct, WebSockets)', () => {

const remotePeerId = peerIdFromString(remoteAddr.getPeerId() ?? '')
await localComponents.peerStore.patch(remotePeerId, {
multiaddrs: Array.from({ length: 11 }, (_, i) => multiaddr(`/ip4/127.0.0.1/tcp/1500${i}/ws/p2p/12D3KooWHFKTMzwerBtsVmtz4ZZEQy2heafxzWw6wNn5PPYkBxJ5`))
multiaddrs: Array.from({ length: 11 }, (_, i) => multiaddr(`/ip4/127.0.0.1/tcp/1500${i}/ws/p2p/${remotePeerId.toString()}`))
})

await expect(connectionManager.openConnection(remotePeerId))
Expand Down

0 comments on commit a31b420

Please sign in to comment.