fix: signature compliant with spec

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": {

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'
 }

src/record/envelope/index.js

@@ -6,9 +6,10 @@ 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 varint = require('varint')
 
+const { codes } = require('../../errors')
 const Protobuf = require('./envelope.proto')
 
 /**
@@ -69,36 +70,47 @@ class Envelope {
   /**
    * Validate envelope data signature for the given domain.
    * @param {string} domain
-   * @return {Promise}
+   * @return {Promise<boolean>}
    */
-  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())
-
-  return Buffer.concat([domainBuffer, payloadTypeBuffer, payload])
+  // 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
+
+  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
+  ])
 }
 
+Envelope.createSignData = createSignData
+
 /**
  * Unmarshal a serialized Envelope protobuf message.
  * @param {Buffer} data
@@ -126,7 +138,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 +161,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
 }

src/record/peer-record/consts.js

@@ -15,4 +15,4 @@ module.exports.ENVELOPE_DOMAIN_PEER_RECORD = '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]

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

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)
   })
 })

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)
+  })
 })