Skip to content
This repository was archived by the owner on Aug 23, 2019. It is now read-only.

Commit a62a72b

Browse files
jacobheunjacobheun
committed
fix: improve erroring around invalid peers
docs: add some comments chore: update deps test: simplify identify test
1 parent b6600da commit a62a72b

File tree

6 files changed

+238
-53
lines changed

6 files changed

+238
-53
lines changed

‎package.json‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,9 @@
3737
"npm": ">=3.0.0"
3838
},
3939
"devDependencies": {
40-
"aegir": "^13.1.0",
41-
"buffer-loader": "0.0.1",
40+
"aegir": "^15.0.0",
4241
"chai": "^4.1.2",
4342
"dirty-chai": "^2.0.1",
44-
"gulp": "^3.9.1",
4543
"libp2p-mplex": "~0.7.0",
4644
"libp2p-pnet": "~0.1.0",
4745
"libp2p-secio": "~0.10.0",
@@ -50,7 +48,6 @@
5048
"libp2p-webrtc-star": "~0.14.0",
5149
"libp2p-websockets": "~0.12.0",
5250
"peer-book": "~0.7.0",
53-
"pull-goodbye": "0.0.2",
5451
"sinon": "^5.0.2",
5552
"webrtcsupport": "^2.2.0"
5653
},
@@ -62,7 +59,7 @@
6259
"interface-connection": "~0.3.2",
6360
"ip-address": "^5.8.9",
6461
"libp2p-circuit": "~0.2.0",
65-
"libp2p-identify": "~0.7.1",
62+
"libp2p-identify": "~0.7.2",
6663
"lodash.includes": "^4.3.0",
6764
"moving-average": "^1.0.0",
6865
"multiaddr": "^5.0.0",

‎src/connection.js‎

Lines changed: 66 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -43,59 +43,77 @@ class ConnectionManager {
4343
// 2. call getPeerInfo
4444
// 3. add this conn to the pool
4545
if (this.switch.identify) {
46-
// overload peerInfo to use Identify instead
47-
conn.getPeerInfo = (callback) => {
48-
const conn = muxedConn.newStream()
49-
const ms = new multistream.Dialer()
50-
callback = once(callback)
51-
52-
waterfall([
53-
(cb) => ms.handle(conn, cb),
54-
(cb) => ms.select(identify.multicodec, cb),
55-
(conn, cb) => identify.dialer(conn, cb),
56-
(peerInfo, observedAddrs, cb) => {
57-
observedAddrs.forEach((oa) => {
58-
this.switch._peerInfo.multiaddrs.addSafe(oa)
59-
})
60-
cb(null, peerInfo)
61-
}
62-
], (err, peerInfo) => {
63-
if (peerInfo) {
64-
conn.setPeerInfo(peerInfo)
65-
}
66-
callback(err, peerInfo)
67-
})
68-
}
69-
70-
conn.getPeerInfo((err, peerInfo) => {
71-
if (err) {
72-
return log('Identify not successful')
46+
// Get the peer info from the crypto exchange
47+
conn.getPeerInfo((err, cryptoPI) => {
48+
if (err || !cryptoPI) {
49+
log('crypto peerInfo wasnt found')
7350
}
74-
const b58Str = peerInfo.id.toB58String()
75-
76-
this.switch.muxedConns[b58Str] = { muxer: muxedConn }
77-
78-
if (peerInfo.multiaddrs.size > 0) {
79-
// with incomming conn and through identify, going to pick one
80-
// of the available multiaddrs from the other peer as the one
81-
// I'm connected to as we really can't be sure at the moment
82-
// TODO add this consideration to the connection abstraction!
83-
peerInfo.connect(peerInfo.multiaddrs.toArray()[0])
84-
} else {
85-
// for the case of websockets in the browser, where peers have
86-
// no addr, use just their IPFS id
87-
peerInfo.connect(`/ipfs/${b58Str}`)
51+
52+
// overload peerInfo to use Identify instead
53+
conn.getPeerInfo = (callback) => {
54+
const conn = muxedConn.newStream()
55+
const ms = new multistream.Dialer()
56+
callback = once(callback)
57+
58+
waterfall([
59+
(cb) => ms.handle(conn, cb),
60+
(cb) => ms.select(identify.multicodec, cb),
61+
// run identify and verify the peer has the same info from crypto
62+
(conn, cb) => identify.dialer(conn, cryptoPI, cb),
63+
(peerInfo, observedAddrs, cb) => {
64+
observedAddrs.forEach((oa) => {
65+
this.switch._peerInfo.multiaddrs.addSafe(oa)
66+
})
67+
cb(null, peerInfo)
68+
}
69+
], (err, peerInfo) => {
70+
if (err) {
71+
return muxedConn.end(() => {
72+
if (peerInfo) {
73+
setImmediate(() => this.switch.emit('peer-mux-closed', peerInfo))
74+
}
75+
callback(err, null)
76+
})
77+
}
78+
79+
if (peerInfo) {
80+
conn.setPeerInfo(peerInfo)
81+
}
82+
callback(err, peerInfo)
83+
})
8884
}
89-
peerInfo = this.switch._peerBook.put(peerInfo)
9085

91-
muxedConn.on('close', () => {
92-
delete this.switch.muxedConns[b58Str]
93-
peerInfo.disconnect()
86+
conn.getPeerInfo((err, peerInfo) => {
87+
if (err) {
88+
return log('identify not successful')
89+
}
90+
const b58Str = peerInfo.id.toB58String()
91+
92+
this.switch.muxedConns[b58Str] = { muxer: muxedConn }
93+
94+
if (peerInfo.multiaddrs.size > 0) {
95+
// with incomming conn and through identify, going to pick one
96+
// of the available multiaddrs from the other peer as the one
97+
// I'm connected to as we really can't be sure at the moment
98+
// TODO add this consideration to the connection abstraction!
99+
peerInfo.connect(peerInfo.multiaddrs.toArray()[0])
100+
} else {
101+
// for the case of websockets in the browser, where peers have
102+
// no addr, use just their IPFS id
103+
peerInfo.connect(`/ipfs/${b58Str}`)
104+
}
94105
peerInfo = this.switch._peerBook.put(peerInfo)
95-
setImmediate(() => this.switch.emit('peer-mux-closed', peerInfo))
96-
})
97106

98-
setImmediate(() => this.switch.emit('peer-mux-established', peerInfo))
107+
muxedConn.on('close', () => {
108+
delete this.switch.muxedConns[b58Str]
109+
peerInfo.disconnect()
110+
peerInfo = this.switch._peerBook.put(peerInfo)
111+
log(`closed connection to ${b58Str}`)
112+
setImmediate(() => this.switch.emit('peer-mux-closed', peerInfo))
113+
})
114+
115+
setImmediate(() => this.switch.emit('peer-mux-established', peerInfo))
116+
})
99117
})
100118
}
101119

‎src/dial.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,7 @@ class Dialer {
317317
delete this.switch.muxedConns[b58Id]
318318
this.peerInfo.disconnect()
319319
this.switch._peerInfo.disconnect()
320+
log(`closed connection to ${b58Id}`)
320321
setImmediate(() => this.switch.emit('peer-mux-closed', this.peerInfo))
321322
})
322323

‎test/identify.node.js‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/* eslint-env mocha */
2+
'use strict'
3+
4+
const chai = require('chai')
5+
const dirtyChai = require('dirty-chai')
6+
const expect = chai.expect
7+
chai.use(dirtyChai)
8+
const parallel = require('async/parallel')
9+
const TCP = require('libp2p-tcp')
10+
const multiplex = require('libp2p-mplex')
11+
const pull = require('pull-stream')
12+
const secio = require('libp2p-secio')
13+
const PeerBook = require('peer-book')
14+
const identify = require('libp2p-identify')
15+
const lp = require('pull-length-prefixed')
16+
17+
const utils = require('./utils')
18+
const createInfos = utils.createInfos
19+
const Switch = require('../src')
20+
21+
describe('Identify', () => {
22+
let switchA
23+
let switchB
24+
let switchC
25+
26+
before((done) => createInfos(3, (err, infos) => {
27+
expect(err).to.not.exist()
28+
29+
const peerA = infos[0]
30+
const peerB = infos[1]
31+
const peerC = infos[2]
32+
33+
peerA.multiaddrs.add('/ip4/127.0.0.1/tcp/9001')
34+
peerB.multiaddrs.add('/ip4/127.0.0.1/tcp/9002')
35+
peerC.multiaddrs.add('/ip4/127.0.0.1/tcp/9003')
36+
37+
switchA = new Switch(peerA, new PeerBook())
38+
switchB = new Switch(peerB, new PeerBook())
39+
switchC = new Switch(peerC, new PeerBook())
40+
41+
switchA.transport.add('tcp', new TCP())
42+
switchB.transport.add('tcp', new TCP())
43+
switchC.transport.add('tcp', new TCP())
44+
45+
switchA.connection.crypto(secio.tag, secio.encrypt)
46+
switchB.connection.crypto(secio.tag, secio.encrypt)
47+
switchC.connection.crypto(secio.tag, secio.encrypt)
48+
49+
switchA.connection.addStreamMuxer(multiplex)
50+
switchB.connection.addStreamMuxer(multiplex)
51+
switchC.connection.addStreamMuxer(multiplex)
52+
53+
switchA.connection.reuse()
54+
switchB.connection.reuse()
55+
switchC.connection.reuse()
56+
57+
parallel([
58+
(cb) => switchA.transport.listen('tcp', {}, null, cb),
59+
(cb) => switchB.transport.listen('tcp', {}, null, cb),
60+
(cb) => switchC.transport.listen('tcp', {}, null, cb)
61+
], done)
62+
}))
63+
64+
after(function (done) {
65+
this.timeout(3 * 1000)
66+
parallel([
67+
(cb) => switchA.stop(cb),
68+
(cb) => switchB.stop(cb),
69+
(cb) => switchC.stop(cb)
70+
], done)
71+
})
72+
73+
afterEach(function (done) {
74+
// Hangup everything
75+
parallel([
76+
(cb) => switchA.hangUp(switchB._peerInfo, cb),
77+
(cb) => switchA.hangUp(switchC._peerInfo, cb),
78+
(cb) => switchB.hangUp(switchA._peerInfo, cb),
79+
(cb) => switchB.hangUp(switchC._peerInfo, cb),
80+
(cb) => switchC.hangUp(switchA._peerInfo, cb),
81+
(cb) => switchC.hangUp(switchB._peerInfo, cb)
82+
], done)
83+
})
84+
85+
it('should identify a good peer', (done) => {
86+
switchA.handle('/id-test/1.0.0', (protocol, conn) => pull(conn, conn))
87+
switchB.dial(switchA._peerInfo, '/id-test/1.0.0', (err, conn) => {
88+
expect(err).to.not.exist()
89+
let data = Buffer.from('data that cant be had')
90+
pull(
91+
pull.values([data]),
92+
conn,
93+
pull.collect((err, values) => {
94+
expect(err).to.not.exist()
95+
expect(values).to.deep.equal([data])
96+
done()
97+
})
98+
)
99+
})
100+
})
101+
102+
it('should require crypto and identify to have the same peerId', (done) => {
103+
identify.listener = (conn) => {
104+
conn.getObservedAddrs((err, observedAddrs) => {
105+
if (err) { return }
106+
observedAddrs = observedAddrs[0]
107+
108+
// pretend to be another peer
109+
let publicKey = switchC._peerInfo.id.pubKey.bytes
110+
111+
const msgSend = identify.message.encode({
112+
protocolVersion: 'ipfs/0.1.0',
113+
agentVersion: 'na',
114+
publicKey: publicKey,
115+
listenAddrs: switchC._peerInfo.multiaddrs.toArray().map((ma) => ma.buffer),
116+
observedAddr: observedAddrs ? observedAddrs.buffer : Buffer.from('')
117+
})
118+
119+
pull(
120+
pull.values([msgSend]),
121+
lp.encode(),
122+
conn
123+
)
124+
})
125+
}
126+
127+
switchA.handle('/id-test/1.0.0', (protocol, conn) => pull(conn, conn))
128+
switchB.dial(switchA._peerInfo, '/id-test/1.0.0', (err, conn) => {
129+
expect(err).to.not.exist()
130+
pull(
131+
pull.values([Buffer.from('data that cant be had')]),
132+
conn,
133+
pull.collect((err, values) => {
134+
expect(err).to.exist()
135+
expect(values).to.have.length(0)
136+
done()
137+
})
138+
)
139+
})
140+
})
141+
})

‎test/node.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,6 @@ require('./secio.node')
77
require('./swarm-no-muxing.node')
88
require('./swarm-muxing.node')
99
require('./circuit-relay.node')
10+
require('./identify.node')
1011
require('./limit-dialer.node')
1112
require('./stats.node')

‎test/utils.js‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,30 @@ exports.tryEcho = (conn, callback) => {
4747
})
4848
)
4949
}
50+
51+
/**
52+
* A utility method for calling done multiple times to help with async
53+
* testing
54+
*
55+
* @param {Number} n The number of times done will be called
56+
* @param {Function} willFinish An optional callback for cleanup before done is called
57+
* @param {Function} done
58+
* @returns {void}
59+
*/
60+
exports.doneAfter = (n, willFinish, done) => {
61+
if (!done) {
62+
done = willFinish
63+
willFinish = undefined
64+
}
65+
66+
let count = 0
67+
let errors = []
68+
return (err) => {
69+
count++
70+
if (err) errors.push(err)
71+
if (count >= n) {
72+
if (willFinish) willFinish()
73+
done(errors.length > 0 ? errors : null)
74+
}
75+
}
76+
}

0 commit comments

Comments
 (0)