Skip to content

feat: exchange signed peer records in identify #682

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vasco-santos merged 6 commits into from
Jul 15, 2020
Merged

feat: exchange signed peer records in identify #682

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5023bce
feat: exchange signed peer records in identify
vasco-santos Jun 20, 2020
eb0b020
feat: create self peer record in identify
vasco-santos Jun 24, 2020
c2fa11a
chore: address review
vasco-santos Jun 24, 2020
d67190b
fix: revert new identify protocol versions
vasco-santos Jul 15, 2020
931b71b
chore: apply suggestions from code review
vasco-santos Jul 15, 2020
d935021
chore: do not get signed peer record n times
vasco-santos Jul 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
chore: address review
  • Loading branch information
@vasco-santos
vasco-santos committed Jul 15, 2020
commit c2fa11a0aa173df1e47f6505d325265f252aeaf0
4 changes: 2 additions & 2 deletions src/identify/consts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ module.exports.MULTICODEC_IDENTIFY = '/p2p/id/1.1.0'
module.exports.MULTICODEC_IDENTIFY_PUSH = '/p2p/id/push/1.1.0'
Copy link
Contributor

@jacobheun jacobheun Jul 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are the versions listed, I was having trouble finding the updates. Is this in the specs repo or in go-libp2p?

Copy link
Member Author

@vasco-santos vasco-santos Jul 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit added them: libp2p/go-libp2p@077a818#diff-98be6dba669aa4123b7ef190fe7113e8

However, now that I looked further, the same protocol is now being used and go reverted this change per libp2p/go-libp2p#907 (comment)

I am doing the same


// Legacy
module.exports.MULTICODEC_IDENTIFY_LEGACY = '/ipfs/id/1.0.0'
module.exports.MULTICODEC_IDENTIFY_PUSH_LEGACY = '/ipfs/id/push/1.0.0'
module.exports.MULTICODEC_IDENTIFY_1_0_0 = '/ipfs/id/1.0.0'
module.exports.MULTICODEC_IDENTIFY_PUSH_1_0_0 = '/ipfs/id/push/1.0.0'
135 changes: 34 additions & 101 deletions src/identify/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ const PeerRecord = require('../record/peer-record')

const {
MULTICODEC_IDENTIFY,
MULTICODEC_IDENTIFY_LEGACY,
MULTICODEC_IDENTIFY_1_0_0,
MULTICODEC_IDENTIFY_PUSH,
MULTICODEC_IDENTIFY_PUSH_LEGACY,
MULTICODEC_IDENTIFY_PUSH_1_0_0,
AGENT_VERSION,
PROTOCOL_VERSION
} = require('./consts')
Expand Down Expand Up @@ -97,26 +97,12 @@ class IdentifyService {
push (connections) {
const pushes = connections.map(async connection => {
try {
const { protocol, stream } = await connection.newStream([MULTICODEC_IDENTIFY_PUSH, MULTICODEC_IDENTIFY_PUSH_LEGACY])

// Handle Legacy
if (protocol === MULTICODEC_IDENTIFY_PUSH_LEGACY) {
return pipe(
[{
listenAddrs: this._libp2p.multiaddrs.map((ma) => ma.buffer),
protocols: Array.from(this._protocols.keys())
}],
pb.encode(Message),
stream,
consume
)
}

const envelope = await this._getSelfPeerRecord()
const signedPeerRecord = envelope.marshal()
const { stream } = await connection.newStream([MULTICODEC_IDENTIFY_PUSH, MULTICODEC_IDENTIFY_PUSH_1_0_0])
const signedPeerRecord = await this._getSelfPeerRecord()

await pipe(
[{
listenAddrs: this._libp2p.multiaddrs.map((ma) => ma.buffer),
signedPeerRecord,
protocols: Array.from(this._protocols.keys())
}],
Expand Down Expand Up @@ -159,7 +145,7 @@ class IdentifyService {
* @returns {Promise<void>}
*/
async identify (connection) {
const { protocol, stream } = await connection.newStream([MULTICODEC_IDENTIFY, MULTICODEC_IDENTIFY_LEGACY])
const { protocol, stream } = await connection.newStream([MULTICODEC_IDENTIFY, MULTICODEC_IDENTIFY_1_0_0])
const [data] = await pipe(
[],
stream,
Expand Down Expand Up @@ -198,7 +184,7 @@ class IdentifyService {
observedAddr = IdentifyService.getCleanMultiaddr(observedAddr)

// LEGACY: differentiate message with SignedPeerRecord
if (protocol === MULTICODEC_IDENTIFY_LEGACY) {
if (protocol === MULTICODEC_IDENTIFY_1_0_0) {
Copy link
Contributor

@jacobheun jacobheun Jul 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to check the protocol, I'd just check the existence of signedPeerRecord. I think it might be better to catch and log the errors unmarshalling the signed peer record and then fallback to adding the listenAddrs. If for some reason the payload is malformed this will allow us to easily fallback.

// Update peers data in PeerStore
this.peerStore.addressBook.set(id, listenAddrs.map((addr) => multiaddr(addr)))
this.peerStore.protoBook.set(id, protocols)
Expand Down Expand Up @@ -249,13 +235,11 @@ class IdentifyService {
handleMessage ({ connection, stream, protocol }) {
switch (protocol) {
case MULTICODEC_IDENTIFY:
case MULTICODEC_IDENTIFY_1_0_0:
return this._handleIdentify({ connection, stream })
case MULTICODEC_IDENTIFY_LEGACY:
return this._handleIdentifyLegacy({ connection, stream })
case MULTICODEC_IDENTIFY_PUSH:
case MULTICODEC_IDENTIFY_PUSH_1_0_0:
return this._handlePush({ connection, stream })
case MULTICODEC_IDENTIFY_PUSH_LEGACY:
return this._handlePushLegacy({ connection, stream })
default:
log.error('cannot handle unknown protocol %s', protocol)
}
Expand All @@ -275,45 +259,14 @@ class IdentifyService {
publicKey = this.peerId.pubKey.bytes
}

const envelope = await this._getSelfPeerRecord()
const signedPeerRecord = envelope.marshal()

const message = Message.encode({
protocolVersion: PROTOCOL_VERSION,
agentVersion: AGENT_VERSION,
publicKey,
signedPeerRecord,
observedAddr: connection.remoteAddr.buffer,
protocols: Array.from(this._protocols.keys())
})

pipe(
[message],
lp.encode(),
stream,
consume
)
}

/**
* Sends the `Identify` response with listen addresses (LEGACY)
* to the requesting peer over the given `connection`
* @private
* @param {object} options
* @param {*} options.stream
* @param {Connection} options.connection
*/
_handleIdentifyLegacy ({ connection, stream }) {
let publicKey = Buffer.alloc(0)
if (this.peerId.pubKey) {
publicKey = this.peerId.pubKey.bytes
}
const signedPeerRecord = await this._getSelfPeerRecord()

const message = Message.encode({
protocolVersion: PROTOCOL_VERSION,
agentVersion: AGENT_VERSION,
publicKey,
listenAddrs: this._libp2p.multiaddrs.map((ma) => ma.buffer),
signedPeerRecord,
observedAddr: connection.remoteAddr.buffer,
protocols: Array.from(this._protocols.keys())
})
Expand Down Expand Up @@ -353,6 +306,22 @@ class IdentifyService {
return log.error('received invalid message', err)
}

const id = connection.remotePeer

// Legacy
if (!message.signedPeerRecord) {
try {
this.peerStore.addressBook.set(id, message.listenAddrs.map((addr) => multiaddr(addr)))
} catch (err) {
return log.error('received invalid listen addrs', err)
}

// Update the protocols
this.peerStore.protoBook.set(id, message.protocols)

return
}

// Open envelope and verify if is authenticated
let envelope
try {
Expand All @@ -372,7 +341,6 @@ class IdentifyService {
}

// Update peers data in PeerStore
const id = connection.remotePeer
try {
// TODO: Store as certified record

Expand All @@ -386,45 +354,8 @@ class IdentifyService {
}

/**
* Reads the Identify Push message from the given `connection`
* with listen addresses (LEGACY)
* @private
* @param {object} options
* @param {*} options.stream
* @param {Connection} options.connection
*/
async _handlePushLegacy ({ connection, stream }) {
const [data] = await pipe(
[],
stream,
lp.decode(),
take(1),
toBuffer,
collect
)

let message
try {
message = Message.decode(data)
} catch (err) {
return log.error('received invalid message', err)
}

// Update peers data in PeerStore
const id = connection.remotePeer
try {
this.peerStore.addressBook.set(id, message.listenAddrs.map((addr) => multiaddr(addr)))
} catch (err) {
return log.error('received invalid listen addrs', err)
}

// Update the protocols
this.peerStore.protoBook.set(id, message.protocols)
}

/**
* Get self signed peer record envelope.
* @return {Envelope}
* Get self signed peer record raw envelope.
* @return {Buffer}
*/
async _getSelfPeerRecord () {
// TODO: Verify if updated
Expand All @@ -436,7 +367,9 @@ class IdentifyService {
peerId: this.peerId,
multiaddrs: this._libp2p.multiaddrs
})
this._selfRecord = await Envelope.seal(peerRecord, this.peerId)
const envelope = await Envelope.seal(peerRecord, this.peerId)

this._selfRecord = envelope.marshal()

return this._selfRecord
}
Expand All @@ -449,8 +382,8 @@ module.exports.IdentifyService = IdentifyService
*/
module.exports.multicodecs = {
IDENTIFY: MULTICODEC_IDENTIFY,
IDENTIFY_LEGACY: MULTICODEC_IDENTIFY_LEGACY,
IDENTIFY_1_0_0: MULTICODEC_IDENTIFY_1_0_0,
IDENTIFY_PUSH: MULTICODEC_IDENTIFY_PUSH,
IDENTIFY_PUSH_LEGACY: MULTICODEC_IDENTIFY_PUSH_LEGACY
IDENTIFY_PUSH_1_0_0: MULTICODEC_IDENTIFY_PUSH_1_0_0
}
module.exports.Message = Message
22 changes: 11 additions & 11 deletions test/identify/index.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ const protocols = new Map([
])

const protocolsLegacy = new Map([
[multicodecs.IDENTIFY_LEGACY, () => { }],
[multicodecs.IDENTIFY_PUSH_LEGACY, () => { }]
[multicodecs.IDENTIFY_1_0_0, () => { }],
[multicodecs.IDENTIFY_PUSH_1_0_0, () => { }]
])

describe('Identify', () => {
Expand All @@ -49,7 +49,7 @@ describe('Identify', () => {
sinon.restore()
})

it('should be able to identify another peer with legacy protocol', async () => {
it('should be able to identify another peer with 1.0.0 legacy protocol', async () => {
const localIdentify = new IdentifyService({
libp2p: {
peerId: localPeer,
Expand Down Expand Up @@ -81,7 +81,7 @@ describe('Identify', () => {
const remoteConnectionMock = { remoteAddr: observedAddr }

const [local, remote] = duplexPair()
sinon.stub(localConnectionMock, 'newStream').returns({ stream: local, protocol: multicodecs.IDENTIFY_LEGACY })
sinon.stub(localConnectionMock, 'newStream').returns({ stream: local, protocol: multicodecs.IDENTIFY_1_0_0 })

sinon.spy(localIdentify.peerStore.addressBook, 'set')
sinon.spy(localIdentify.peerStore.protoBook, 'set')
Expand All @@ -92,7 +92,7 @@ describe('Identify', () => {
remoteIdentify.handleMessage({
connection: remoteConnectionMock,
stream: remote,
protocol: multicodecs.IDENTIFY_LEGACY
protocol: multicodecs.IDENTIFY_1_0_0
})
])

Expand Down Expand Up @@ -214,7 +214,7 @@ describe('Identify', () => {
})

describe('push', () => {
it('should be able to push identify updates to another peer with legacy protocol', async () => {
it('should be able to push identify updates to another peer with 1.0.0 legacy protocols', async () => {
const connectionManager = new EventEmitter()
connectionManager.getConnection = () => {}

Expand All @@ -225,8 +225,8 @@ describe('Identify', () => {
multiaddrs: listenMaddrs
},
protocols: new Map([
[multicodecs.IDENTIFY_LEGACY],
[multicodecs.IDENTIFY_PUSH_LEGACY],
[multicodecs.IDENTIFY_1_0_0],
[multicodecs.IDENTIFY_PUSH_1_0_0],
['/echo/1.0.0']
])
})
Expand All @@ -247,12 +247,12 @@ describe('Identify', () => {
})

// Setup peer protocols and multiaddrs
const localProtocols = new Set([multicodecs.IDENTIFY_LEGACY, multicodecs.IDENTIFY_PUSH_LEGACY, '/echo/1.0.0'])
const localProtocols = new Set([multicodecs.IDENTIFY_1_0_0, multicodecs.IDENTIFY_PUSH_1_0_0, '/echo/1.0.0'])
const localConnectionMock = { newStream: () => {} }
const remoteConnectionMock = { remotePeer: localPeer }

const [local, remote] = duplexPair()
sinon.stub(localConnectionMock, 'newStream').returns({ stream: local, protocol: multicodecs.IDENTIFY_PUSH_LEGACY })
sinon.stub(localConnectionMock, 'newStream').returns({ stream: local, protocol: multicodecs.IDENTIFY_PUSH_1_0_0 })

sinon.spy(remoteIdentify.peerStore.addressBook, 'set')
sinon.spy(remoteIdentify.peerStore.protoBook, 'set')
Expand All @@ -263,7 +263,7 @@ describe('Identify', () => {
remoteIdentify.handleMessage({
connection: remoteConnectionMock,
stream: remote,
protocol: multicodecs.IDENTIFY_PUSH_LEGACY
protocol: multicodecs.IDENTIFY_PUSH_1_0_0
})
])

Expand Down