fix: update record selection rules (#134)

Perform the [same validation as go-ipns](https://github.com/ipfs/go-ipns/blob/a2d4e93f7e8ffc9f996471eb1a24ff12c8484120/ipns.go#L325-L362).

- If a record has a v2 sig and the other does not, prefer that record
- If the sequence numbers are not equal, prefer the record with the higher sequence number
- If the sequence numbers are equal, prefer the record with the longer validity
- Otherwise prefer the first record

Also validates that embedded keys, where present, match the PeerId from the
IPNS Record.

Also, also: runs the type checker over the tests.

BREAKING CHANGE: extractPublicKey is now async

src/errors.js

@@ -9,3 +9,4 @@ exports.ERR_PEER_ID_FROM_PUBLIC_KEY = 'ERR_PEER_ID_FROM_PUBLIC_KEY'
 exports.ERR_PUBLIC_KEY_FROM_ID = 'ERR_PUBLIC_KEY_FROM_ID'
 exports.ERR_UNDEFINED_PARAMETER = 'ERR_UNDEFINED_PARAMETER'
 exports.ERR_INVALID_RECORD_DATA = 'ERR_INVALID_RECORD_DATA'
+exports.ERR_INVALID_EMBEDDED_KEY = 'ERR_INVALID_EMBEDDED_KEY'

src/index.js

@@ -83,6 +83,7 @@ const createWithExpiration = (privateKey, value, seq, expiration) => {
  * @param {number} validityType
  * @param {NanoDate} expirationDate
  * @param {bigint} ttl
+ * @returns {Promise<IPNSEntry>}
  */
 const _create = async (privateKey, value, seq, validityType, expirationDate, ttl) => {
   seq = BigInt(seq)
@@ -277,34 +278,44 @@ const embedPublicKey = async (publicKey, entry) => {
 }
 
 /**
- * Extracts a public key matching `pid` from the ipns record.
+ * Extracts a public key from the passed PeerId, falling
+ * back to the pubKey embedded in the ipns record.
  *
  * @param {PeerId} peerId - peer identifier object.
  * @param {IPNSEntry} entry - ipns entry record.
  */
-const extractPublicKey = (peerId, entry) => {
+const extractPublicKey = async (peerId, entry) => {
   if (!entry || !peerId) {
     const error = new Error('one or more of the provided parameters are not defined')
 
     log.error(error)
     throw errCode(error, ERRORS.ERR_UNDEFINED_PARAMETER)
   }
 
+  let pubKey
+
   if (entry.pubKey) {
-    let pubKey
     try {
       pubKey = crypto.keys.unmarshalPublicKey(entry.pubKey)
     } catch (err) {
       log.error(err)
       throw err
     }
-    return pubKey
+
+    const otherId = await PeerId.createFromPubKey(entry.pubKey)
+
+    if (!otherId.equals(peerId)) {
+      throw errCode(new Error('Embedded public key did not match PeerID'), ERRORS.ERR_INVALID_EMBEDDED_KEY)
+    }
+  } else if (peerId.pubKey) {
+    pubKey = peerId.pubKey
   }
 
-  if (peerId.pubKey) {
-    return peerId.pubKey
+  if (pubKey) {
+    return pubKey
   }
-  throw Object.assign(new Error('no public key is available'), { code: ERRORS.ERR_UNDEFINED_PARAMETER })
+
+  throw errCode(new Error('no public key is available'), ERRORS.ERR_UNDEFINED_PARAMETER)
 }
 
 /**
@@ -443,7 +454,9 @@ const unmarshal = (buf) => {
     validity: object.validity,
     sequence: Object.hasOwnProperty.call(object, 'sequence') ? BigInt(`${object.sequence}`) : 0n,
     pubKey: object.pubKey,
-    ttl: Object.hasOwnProperty.call(object, 'ttl') ? BigInt(`${object.ttl}`) : undefined
+    ttl: Object.hasOwnProperty.call(object, 'ttl') ? BigInt(`${object.ttl}`) : undefined,
+    signatureV2: object.signatureV2,
+    data: object.data
   }
 }
 
@@ -458,11 +471,12 @@ const validator = {
     const peerId = PeerId.createFromBytes(bufferId)
 
     // extract public key
-    const pubKey = extractPublicKey(peerId, receivedEntry)
+    const pubKey = await extractPublicKey(peerId, receivedEntry)
 
     // Record validation
     await validate(pubKey, receivedEntry)
   },
+
   /**
    * @param {Uint8Array} dataA
    * @param {Uint8Array} dataB
@@ -471,7 +485,25 @@ const validator = {
     const entryA = unmarshal(dataA)
     const entryB = unmarshal(dataB)
 
-    return entryA.sequence > entryB.sequence ? 0 : 1
+    // having a newer signature version is better than an older signature version
+    if (entryA.signatureV2 && !entryB.signatureV2) {
+      return 0
+    } else if (entryB.signatureV2 && !entryA.signatureV2) {
+      return 1
+    }
+
+    // choose later sequence number
+    if (entryA.sequence > entryB.sequence) {
+      return 0
+    } else if (entryA.sequence < entryB.sequence) {
+      return 1
+    }
+
+    // choose longer lived record if sequence numbers the same
+    const entryAValidityDate = parseRFC3339(uint8ArrayToString(entryA.validity))
+    const entryBValidityDate = parseRFC3339(uint8ArrayToString(entryB.validity))
+
+    return entryBValidityDate.getTime() > entryAValidityDate.getTime() ? 1 : 0
   }
 }
 

src/types.d.ts

@@ -8,7 +8,7 @@ export interface IPNSEntry {
   validity: Uint8Array // expiration datetime for the record in RFC3339 format
   sequence: bigint // number representing the version of the record
   ttl?: bigint // ttl in nanoseconds
-  pubKey?: Uint8Array
+  pubKey?: Uint8Array // the public portion of the key that signed this record (only present if it was not embedded in the IPNS key)
   signatureV2?: Uint8Array // the v2 signature of the record
   data?: Uint8Array // extensible data
 }

test/index.spec.js

@@ -3,6 +3,7 @@
 
 const { expect } = require('aegir/utils/chai')
 const { base58btc } = require('multiformats/bases/base58')
+const { base64urlpad } = require('multiformats/bases/base64')
 const { fromString: uint8ArrayFromString } = require('uint8arrays/from-string')
 const { concat: uint8ArrayConcat } = require('uint8arrays/concat')
 const PeerId = require('peer-id')
@@ -21,13 +22,18 @@ describe('ipns', function () {
   let ipfsId
   /** @type {import('libp2p-crypto').keys.supportedKeys.rsa.RsaPrivateKey} */
   let rsa
+  /** @type {import('libp2p-crypto').keys.supportedKeys.rsa.RsaPrivateKey} */
+  let rsa2
 
   before(async () => {
     rsa = await crypto.keys.generateKeyPair('RSA', 2048)
+    rsa2 = await crypto.keys.generateKeyPair('RSA', 2048)
+
+    const peerId = await PeerId.createFromPubKey(rsa.public.bytes)
 
     ipfsId = {
-      id: 'QmQ73f8hbM4hKwRYBqeUsPtiwfE2x6WPv9WnzaYt4nYcXf',
-      publicKey: 'CAASpgIwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDUOR0AJ2/yO0S/JIkKmYV/QdHzQXi1nrTCCXtEbUDVW5mXZfNf9bKeNDfW3UIIOwVzV6/sRhJqq/8sQAhmzURj1q2onCKgSLzjdePSLtykolQeQGSD+JO7rcxOLx+sTdIyJiclP/tkK2gfo2nrI6pjFTKNzR8VSoJx7gfiqY1N9LBgDsD4WjaOM2pBgzgVUlXpk27Aqvcd+htSWi6JuIZaBhPY/IzEvXwntGH9k7F8VkT6nUBilhqFFSWnz8cNKToCHjyhoozKfqN89S7EGMiNvG4cX4Dc/nVXlZRTAi4PNNewutimujROy2/tNEquC2uAlcAzhRAcLL/ujhEjJYP1AgMBAAE='
+      id: peerId.toB58String(),
+      publicKey: base64urlpad.encode(rsa.public.bytes)
     }
   })
 
@@ -43,6 +49,8 @@ describe('ipns', function () {
     expect(entry).to.have.property('validity')
     expect(entry).to.have.property('signature')
     expect(entry).to.have.property('validityType')
+    expect(entry).to.have.property('signatureV2')
+    expect(entry).to.have.property('data')
   })
 
   it('should be able to create a record with a fixed expiration', async () => {
@@ -121,6 +129,19 @@ describe('ipns', function () {
     expect(entryDataCreated.validityType).to.equal(unmarshalledData.validityType)
     expect(entryDataCreated.signature).to.equalBytes(unmarshalledData.signature)
     expect(entryDataCreated.sequence).to.equal(unmarshalledData.sequence)
+    expect(entryDataCreated.ttl).to.equal(unmarshalledData.ttl)
+
+    if (!unmarshalledData.signatureV2) {
+      throw new Error('No v2 sig found')
+    }
+
+    expect(entryDataCreated.signatureV2).to.equalBytes(unmarshalledData.signatureV2)
+
+    if (!unmarshalledData.data) {
+      throw new Error('No v2 data found')
+    }
+
+    expect(entryDataCreated.data).to.equalBytes(unmarshalledData.data)
 
     return ipns.validate(rsa.public, unmarshalledData)
   })
@@ -206,13 +227,13 @@ describe('ipns', function () {
     expect.fail('Expected ERR_UNDEFINED_PARAMETER')
   })
 
-  it('should be able to export a previously embed public key from an ipns record', async () => {
+  it('should be able to export a previously embedded public key from an ipns record', async () => {
     const sequence = 0
     const validity = 1000000
 
     const entry = await ipns.create(rsa, cid, sequence, validity)
     await ipns.embedPublicKey(rsa.public, entry)
-    const publicKey = ipns.extractPublicKey(PeerId.createFromB58String(ipfsId.id), entry)
+    const publicKey = await ipns.extractPublicKey(PeerId.createFromB58String(ipfsId.id), entry)
     expect(publicKey.bytes).to.equalBytes(rsa.public.bytes)
   })
 
@@ -245,22 +266,63 @@ describe('ipns', function () {
     const keyBytes = base58btc.decode(`z${ipfsId.id}`)
     const key = uint8ArrayConcat([uint8ArrayFromString('/ipns/'), keyBytes])
 
-    try {
-      await ipns.validator.validate(marshalledData, key)
-    } catch (err) {
-      expect(err).to.exist()
-      expect(err).to.include({
-        code: ERRORS.ERR_SIGNATURE_VERIFICATION
-      })
-    }
+    await expect(ipns.validator.validate(marshalledData, key))
+      .to.eventually.be.rejected().with.property('code', ERRORS.ERR_SIGNATURE_VERIFICATION)
   })
 
-  it('should use validator.select to select the record with the highest sequence number', async () => {
+  it('should use validator.validate to verify that a record is not valid when it is passed with the wrong IPNS key', async () => {
     const sequence = 0
     const validity = 1000000
 
     const entry = await ipns.create(rsa, cid, sequence, validity)
-    const newEntry = await ipns.create(rsa, cid, (sequence + 1), validity)
+    await ipns.embedPublicKey(rsa.public, entry)
+    const marshalledData = ipns.marshal(entry)
+
+    const keyBytes = (await PeerId.createFromPrivKey(rsa2.bytes)).toBytes()
+    const key = uint8ArrayConcat([uint8ArrayFromString('/ipns/'), keyBytes])
+
+    await expect(ipns.validator.validate(marshalledData, key))
+      .to.eventually.be.rejected().with.property('code', ERRORS.ERR_INVALID_EMBEDDED_KEY)
+  })
+
+  it('should use validator.validate to verify that a record is not valid when the wrong key is embedded', async () => {
+    const sequence = 0
+    const validity = 1000000
+
+    const entry = await ipns.create(rsa, cid, sequence, validity)
+    await ipns.embedPublicKey(rsa2.public, entry)
+    const marshalledData = ipns.marshal(entry)
+
+    const keyBytes = (await PeerId.createFromPrivKey(rsa.bytes)).toBytes()
+    const key = uint8ArrayConcat([uint8ArrayFromString('/ipns/'), keyBytes])
+
+    await expect(ipns.validator.validate(marshalledData, key))
+      .to.eventually.be.rejected().with.property('code', ERRORS.ERR_INVALID_EMBEDDED_KEY)
+  })
+
+  it('should use validator.select to select the record with the highest sequence number', async () => {
+    const sequence = 0
+    const lifetime = 1000000
+
+    const entry = await ipns.create(rsa, cid, sequence, lifetime)
+    const newEntry = await ipns.create(rsa, cid, (sequence + 1), lifetime)
+
+    const marshalledData = ipns.marshal(entry)
+    const marshalledNewData = ipns.marshal(newEntry)
+
+    let valid = ipns.validator.select(marshalledNewData, marshalledData)
+    expect(valid).to.equal(0) // new data is the selected one
+
+    valid = ipns.validator.select(marshalledData, marshalledNewData)
+    expect(valid).to.equal(1) // new data is the selected one
+  })
+
+  it('should use validator.select to select the record with the longest validity', async () => {
+    const sequence = 0
+    const lifetime = 1000000
+
+    const entry = await ipns.create(rsa, cid, sequence, lifetime)
+    const newEntry = await ipns.create(rsa, cid, sequence, (lifetime + 1))
 
     const marshalledData = ipns.marshal(entry)
     const marshalledNewData = ipns.marshal(newEntry)
@@ -271,4 +333,23 @@ describe('ipns', function () {
     valid = ipns.validator.select(marshalledData, marshalledNewData)
     expect(valid).to.equal(1) // new data is the selected one
   })
+
+  it('should use validator.select to select an older record with a v2 sig when the newer record only uses v1', async () => {
+    const sequence = 0
+    const lifetime = 1000000
+
+    const entry = await ipns.create(rsa, cid, sequence, lifetime)
+
+    const newEntry = await ipns.create(rsa, cid, sequence + 1, lifetime)
+    delete newEntry.signatureV2
+
+    const marshalledData = ipns.marshal(entry)
+    const marshalledNewData = ipns.marshal(newEntry)
+
+    let valid = ipns.validator.select(marshalledNewData, marshalledData)
+    expect(valid).to.equal(1) // old data is the selected one
+
+    valid = ipns.validator.select(marshalledData, marshalledNewData)
+    expect(valid).to.equal(0) // old data is the selected one
+  })
 })

tsconfig.json

@@ -4,10 +4,10 @@
     "outDir": "dist"
   },
   "include": [
-    "src"
+    "src",
+    "test"
   ],
   "exclude": [
-    "src/pb/ipns.js",
-    "test"
+    "src/pb/ipns.js"
   ]
 }