From 4ab125e0177bde0be11acd94b97fc6050d19222b Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Fri, 26 Jun 2020 17:37:53 +0200 Subject: [PATCH] fix: signature compliant with spec --- package.json | 1 + src/errors.js | 3 +- src/record/README.md | 18 ++-------- src/record/envelope/index.js | 61 ++++++++++++++++++++------------ src/record/peer-record/consts.js | 9 +---- src/record/peer-record/index.js | 1 - test/record/envelope.spec.js | 17 ++++----- test/record/peer-record.spec.js | 28 +++++++++++++-- 8 files changed, 80 insertions(+), 58 deletions(-) diff --git a/package.json b/package.json index d14012f8..7c1358ae 100644 --- a/package.json +++ b/package.json @@ -78,6 +78,7 @@ "sanitize-filename": "^1.6.3", "streaming-iterables": "^4.1.0", "timeout-abort-controller": "^1.0.0", + "varint": "^5.0.0", "xsalsa20": "^1.0.2" }, "devDependencies": { diff --git a/src/errors.js b/src/errors.js index c47043e6..18e600c6 100644 --- a/src/errors.js +++ b/src/errors.js @@ -29,5 +29,6 @@ exports.codes = { ERR_TRANSPORT_UNAVAILABLE: 'ERR_TRANSPORT_UNAVAILABLE', ERR_TRANSPORT_DIAL_FAILED: 'ERR_TRANSPORT_DIAL_FAILED', ERR_UNSUPPORTED_PROTOCOL: 'ERR_UNSUPPORTED_PROTOCOL', - ERR_INVALID_MULTIADDR: 'ERR_INVALID_MULTIADDR' + ERR_INVALID_MULTIADDR: 'ERR_INVALID_MULTIADDR', + ERR_SIGNATURE_NOT_VALID: 'ERR_SIGNATURE_NOT_VALID' } diff --git a/src/record/README.md b/src/record/README.md index 49008b14..f423f475 100644 --- a/src/record/README.md +++ b/src/record/README.md @@ -47,7 +47,7 @@ All libp2p nodes keep a `PeerStore`, that among other information stores a set o Libp2p peer records were created to enable the distribution of verifiable address records, which we can prove originated from the addressed peer itself. With such guarantees, libp2p can prioritize addresses based on their authenticity, with the most strict strategy being to only dial certified addresses. -A peer record contains the peers' publicly reachable listen addresses, and may be extended in the future to contain additional metadata relevant to routing. It also contains a `seq` field, so that we can order peer records by time and identify if a received record is more recent than the stored one. +A peer record contains the peers' publicly reachable listen addresses, and may be extended in the future to contain additional metadata relevant to routing. It also contains a `seqNumber` field, so that we can order peer records by time and identify if a received record is more recent than the stored one. You can read further about the Peer Record in [libp2p/specs#217](https://github.com/libp2p/specs/pull/217). @@ -92,7 +92,7 @@ When a libp2p node changes its listen addresses, the identify service should be Considering that a node can discover other peers' addresses from a variety of sources, Libp2p Peerstore should be able to differentiate the addresses that were obtained through a signed peer record. -Once a record is received and its signature properly validated, its envelope should be stored in the AddressBook on its byte representations. However, the `seq` number of the record must be compared with potentially stored records, so that we do not override correct data. +Once a record is received and its signature properly validated, its envelope should be stored in the AddressBook on its byte representations. However, the `seqNumber` number of the record must be compared with potentially stored records, so that we do not override correct data. The AddressBook Addresses must be updated with the content of the envelope with a certified property that allows other subsystems to identify that the known certified addresses of a peer. @@ -112,17 +112,3 @@ When a subsystem wants to provide a record, it should get it from the AddressBoo - Modular dialer? (taken from go PR notes) - With the modular dialer, users should easily be able to configure precedence. With dialer v1, anything we do to prioritise dials is gonna be spaghetti and adhoc. With the modular dialer, you’d be able to specify the order of dials when instantiating the pipeline. - Multiple parallel dials. We already have the issue where new addresses aren't added to existing dials. - -### Notes: - -- Possible design for AddressBook - -``` -addr_book_record - \_ peer_id: bytes - \_ signed_addrs: []AddrEntry - \_ unsigned_addrs: []AddrEntry - \_ certified_record - \_ seq: bytes - \_ raw: bytes -``` diff --git a/src/record/envelope/index.js b/src/record/envelope/index.js index b27e70e5..8db37051 100644 --- a/src/record/envelope/index.js +++ b/src/record/envelope/index.js @@ -5,14 +5,18 @@ const log = debug('libp2p:envelope') log.error = debug('libp2p:envelope:error') const errCode = require('err-code') -const crypto = require('libp2p-crypto') -const multicodec = require('multicodec') -const PeerId = require('peer-id') +const { Buffer } = require('buffer') +const crypto = require('libp2p-crypto') +const PeerId = require('peer-id') +const varint = require('varint') + +const { codes } = require('../../errors') const Protobuf = require('./envelope.proto') /** - * The Envelope is responsible for keeping arbitrary signed by a libp2p peer. + * The Envelope is responsible for keeping an arbitrary signed record + * by a libp2p peer. */ class Envelope { /** @@ -41,7 +45,7 @@ class Envelope { if (this._marshal) { return this._marshal } - // TODO: type for marshal (default: RSA) + const publicKey = crypto.keys.marshalPublicKey(this.peerId.pubKey) this._marshal = Protobuf.encode({ @@ -69,34 +73,43 @@ class Envelope { /** * Validate envelope data signature for the given domain. * @param {string} domain - * @return {Promise} + * @return {Promise} */ - async validate (domain) { + validate (domain) { const signData = createSignData(domain, this.payloadType, this.payload) - try { - await this.peerId.pubKey.verify(signData, this.signature) - } catch (_) { - log.error('record signature verification failed') - // TODO - throw errCode(new Error('record signature verification failed'), 'ERRORS.ERR_SIGNATURE_VERIFICATION') - } + return this.peerId.pubKey.verify(signData, this.signature) } } /** * Helper function that prepares a buffer to sign or verify a signature. * @param {string} domain - * @param {number} payloadType + * @param {Buffer} payloadType * @param {Buffer} payload * @return {Buffer} */ const createSignData = (domain, payloadType, payload) => { - // TODO: this should be compliant with the spec! - const domainBuffer = Buffer.from(domain) - const payloadTypeBuffer = Buffer.from(payloadType.toString()) + // When signing, a peer will prepare a buffer by concatenating the following: + // - The length of the domain separation string string in bytes + // - The domain separation string, encoded as UTF-8 + // - The length of the payload_type field in bytes + // - The value of the payload_type field + // - The length of the payload field in bytes + // - The value of the payload field - return Buffer.concat([domainBuffer, payloadTypeBuffer, payload]) + const domainLength = varint.encode(Buffer.byteLength(domain)) + const payloadTypeLength = varint.encode(payloadType.length) + const payloadLength = varint.encode(payload.length) + + return Buffer.concat([ + Buffer.from(domainLength), + Buffer.from(domain), + Buffer.from(payloadTypeLength), + payloadType, + Buffer.from(payloadLength), + payload + ]) } /** @@ -118,7 +131,7 @@ const unmarshalEnvelope = async (data) => { /** * Seal marshals the given Record, places the marshaled bytes inside an Envelope -* and signs with the given private key. +* and signs it with the given peerId's private key. * @async * @param {Record} record * @param {PeerId} peerId @@ -126,7 +139,7 @@ const unmarshalEnvelope = async (data) => { */ Envelope.seal = async (record, peerId) => { const domain = record.domain - const payloadType = Buffer.from(`${multicodec.print[record.codec]}${domain}`) + const payloadType = Buffer.from(record.codec) const payload = record.marshal() const signData = createSignData(domain, payloadType, payload) @@ -149,7 +162,11 @@ Envelope.seal = async (record, peerId) => { */ Envelope.openAndCertify = async (data, domain) => { const envelope = await unmarshalEnvelope(data) - await envelope.validate(domain) + const valid = await envelope.validate(domain) + + if (!valid) { + throw errCode(new Error('envelope signature is not valid for the given domain'), codes.ERR_SIGNATURE_NOT_VALID) + } return envelope } diff --git a/src/record/peer-record/consts.js b/src/record/peer-record/consts.js index 9f14e8d1..4a65e975 100644 --- a/src/record/peer-record/consts.js +++ b/src/record/peer-record/consts.js @@ -1,6 +1,5 @@ 'use strict' -// const { Buffer } = require('buffer') const multicodec = require('multicodec') // The domain string used for peer records contained in a Envelope. @@ -9,10 +8,4 @@ module.exports.ENVELOPE_DOMAIN_PEER_RECORD = 'libp2p-peer-record' // The type hint used to identify peer records in a Envelope. // Defined in https://github.com/multiformats/multicodec/blob/master/table.csv // with name "libp2p-peer-record" -// TODO -// const b = Buffer.aloc(2) -// b.writeInt16BE(multicodec.LIBP2P_PEER_RECORD) -// module.exports.ENVELOPE_PAYLOAD_TYPE_PEER_RECORD = b - -// const ENVELOPE_PAYLOAD_TYPE_PEER_RECORD = Buffer.aloc(2) -module.exports.ENVELOPE_PAYLOAD_TYPE_PEER_RECORD = multicodec.LIBP2P_PEER_RECORD +module.exports.ENVELOPE_PAYLOAD_TYPE_PEER_RECORD = multicodec.print[multicodec.LIBP2P_PEER_RECORD] diff --git a/src/record/peer-record/index.js b/src/record/peer-record/index.js index 1cb4d40f..8f127def 100644 --- a/src/record/peer-record/index.js +++ b/src/record/peer-record/index.js @@ -24,7 +24,6 @@ class PeerRecord extends Record { * @param {number} [params.seqNumber] monotonically-increasing sequence counter that's used to order PeerRecords in time. */ constructor ({ peerId, multiaddrs = [], seqNumber = Date.now() }) { - // TODO: verify domain/payload type super(ENVELOPE_DOMAIN_PEER_RECORD, ENVELOPE_PAYLOAD_TYPE_PEER_RECORD) this.peerId = peerId diff --git a/test/record/envelope.spec.js b/test/record/envelope.spec.js index c5661def..8f277394 100644 --- a/test/record/envelope.spec.js +++ b/test/record/envelope.spec.js @@ -6,18 +6,18 @@ chai.use(require('dirty-chai')) chai.use(require('chai-bytes')) const { expect } = chai -const multicodec = require('multicodec') - const Envelope = require('../../src/record/envelope') const Record = require('libp2p-interfaces/src/record') +const { codes: ErrorCodes } = require('../../src/errors') const peerUtils = require('../utils/creators/peer') -const domain = '/test-domain' +const domain = 'libp2p-testing' +const codec = '/libp2p/testdata' class TestRecord extends Record { constructor (data) { - super(domain, multicodec.LIBP2P_PEER_RECORD) + super(domain, codec) this.data = data } @@ -31,7 +31,7 @@ class TestRecord extends Record { } describe('Envelope', () => { - const payloadType = Buffer.from(`${multicodec.print[multicodec.LIBP2P_PEER_RECORD]}${domain}`) + const payloadType = Buffer.from(codec) let peerId let testRecord @@ -78,11 +78,12 @@ describe('Envelope', () => { expect(isEqual).to.eql(true) }) - it.skip('throw on open and verify when a different domain is used', async () => { + it('throw on open and verify when a different domain is used', async () => { const envelope = await Envelope.seal(testRecord, peerId) const rawEnvelope = envelope.marshal() - await expect(Envelope.openAndCertify(rawEnvelope, '/fake-domain')) - .to.eventually.rejected() + await expect(Envelope.openAndCertify(rawEnvelope, '/bad-domain')) + .to.eventually.be.rejected() + .and.to.have.property('code', ErrorCodes.ERR_SIGNATURE_NOT_VALID) }) }) diff --git a/test/record/peer-record.spec.js b/test/record/peer-record.spec.js index 4047433b..eda77a8e 100644 --- a/test/record/peer-record.spec.js +++ b/test/record/peer-record.spec.js @@ -5,9 +5,10 @@ const chai = require('chai') chai.use(require('dirty-chai')) const { expect } = chai +const tests = require('libp2p-interfaces/src/record/tests') const multiaddr = require('multiaddr') -const tests = require('libp2p-interfaces/src/record/tests') +const Envelope = require('../../src/record/envelope') const PeerRecord = require('../../src/record/peer-record') const peerUtils = require('../utils/creators/peer') @@ -113,5 +114,28 @@ describe('PeerRecord', () => { }) describe('PeerRecord inside Envelope', () => { - // TODO + let peerId + let peerRecord + + before(async () => { + [peerId] = await peerUtils.createPeerId() + const multiaddrs = [ + multiaddr('/ip4/127.0.0.1/tcp/2000') + ] + const seqNumber = Date.now() + peerRecord = new PeerRecord({ peerId, multiaddrs, seqNumber }) + }) + + it('creates an envelope with the PeerRecord and can unmarshal it', async () => { + const e = await Envelope.seal(peerRecord, peerId) + const byteE = e.marshal() + + const decodedE = await Envelope.openAndCertify(byteE, peerRecord.domain) + expect(decodedE).to.exist() + + const decodedPeerRecord = PeerRecord.createFromProtobuf(decodedE.payload) + + const isEqual = peerRecord.isEqual(decodedPeerRecord) + expect(isEqual).to.eql(true) + }) })