feat(crypto): add publicKeyForPeer to bind keys to peer ids

packages/crypto/src/keys/index.ts

@@ -8,7 +8,8 @@
  * For encryption / decryption support, RSA keys should be used.
  */
 
-import { InvalidParametersError, UnsupportedKeyTypeError } from '@libp2p/interface'
+import { InvalidParametersError, InvalidPublicKeyError, UnsupportedKeyTypeError } from '@libp2p/interface'
+import { equals as uint8ArrayEquals } from 'uint8arrays/equals'
 import { ECDSAPrivateKey as ECDSAPrivateKeyClass } from './ecdsa/ecdsa.ts'
 import { ECDSA_P_256_OID, ECDSA_P_384_OID, ECDSA_P_521_OID } from './ecdsa/index.ts'
 import { generateECDSAKeyPair, pkiMessageToECDSAPrivateKey, pkiMessageToECDSAPublicKey, unmarshalECDSAPrivateKey, unmarshalECDSAPublicKey } from './ecdsa/utils.ts'
@@ -21,7 +22,7 @@ import { pkcs1ToRSAPrivateKey, pkixToRSAPublicKey, generateRSAKeyPair, pkcs1Mess
 import { privateKeyLength as secp256k1PrivateKeyLength, publicKeyLength as secp256k1PublicKeyLength } from './secp256k1/index.ts'
 import { generateSecp256k1KeyPair, unmarshalSecp256k1PrivateKey, unmarshalSecp256k1PublicKey } from './secp256k1/utils.ts'
 import type { Curve } from './ecdsa/index.ts'
-import type { PrivateKey, PublicKey, KeyType, RSAPrivateKey, Secp256k1PrivateKey, Ed25519PrivateKey, Secp256k1PublicKey, Ed25519PublicKey, ECDSAPrivateKey, ECDSAPublicKey } from '@libp2p/interface'
+import type { AbortOptions, PeerId, PrivateKey, PublicKey, KeyType, RSAPrivateKey, Secp256k1PrivateKey, Ed25519PrivateKey, Secp256k1PublicKey, Ed25519PublicKey, ECDSAPrivateKey, ECDSAPublicKey } from '@libp2p/interface'
 import type { MultihashDigest } from 'multiformats'
 import type { Digest } from 'multiformats/hashes/digest'
 
@@ -158,6 +159,40 @@ export function publicKeyToProtobuf (key: PublicKey): Uint8Array {
   })
 }
 
+/**
+ * Returns a public key for the peer that is safe for verifying its signatures.
+ *
+ * A supplied key must derive to the peer id, otherwise `InvalidPublicKeyError`
+ * is thrown, even if the peer id embeds a key. With no supplied key the key
+ * inlined in the peer id is returned, or `InvalidPublicKeyError` is thrown if
+ * there is none. Malformed key bytes also throw `InvalidPublicKeyError`.
+ */
+export async function publicKeyForPeer (peer: Pick<PeerId, 'publicKey' | 'toMultihash'>, key?: Uint8Array | PublicKey, options?: AbortOptions): Promise<PublicKey> {
+  options?.signal?.throwIfAborted()
+
+  if (key != null) {
+    let publicKey: PublicKey
+
+    try {
+      publicKey = key instanceof Uint8Array ? publicKeyFromProtobuf(key) : key
+    } catch (err: any) {
+      throw new InvalidPublicKeyError(`Could not decode public key - ${err.message}`)
+    }
+
+    if (!uint8ArrayEquals(publicKey.toMultihash().bytes, peer.toMultihash().bytes)) {
+      throw new InvalidPublicKeyError('Public key does not match peer id')
+    }
+
+    return publicKey
+  }
+
+  if (peer.publicKey == null) {
+    throw new InvalidPublicKeyError('No public key available for peer id')
+  }
+
+  return peer.publicKey
+}
+
 /**
  * Converts a protobuf serialized private key into its representative object
  */

packages/crypto/test/keys/public-key-for-peer.spec.ts

@@ -0,0 +1,101 @@
+/* eslint-env mocha */
+import { expect } from 'aegir/chai'
+import { identity } from 'multiformats/hashes/identity'
+import { fromString as uint8ArrayFromString } from 'uint8arrays/from-string'
+import { generateKeyPair, publicKeyForPeer, publicKeyToProtobuf } from '../../src/keys/index.ts'
+import type { PeerId, PublicKey } from '@libp2p/interface'
+
+type Peer = Pick<PeerId, 'publicKey' | 'toMultihash'>
+
+// a peer id as parsed from the wire: an inlined key for Ed25519/secp256k1, no
+// key for RSA
+function peerFor (key: PublicKey): Peer {
+  return {
+    publicKey: key.type === 'RSA' ? undefined : key,
+    toMultihash: () => key.toMultihash()
+  } as any
+}
+
+describe('publicKeyForPeer', function () {
+  this.timeout(20 * 1000)
+
+  it('returns the key inlined in an Ed25519 peer id', async () => {
+    const key = (await generateKeyPair('Ed25519')).publicKey
+
+    await expect(publicKeyForPeer(peerFor(key))).to.eventually.equal(key)
+  })
+
+  it('accepts a supplied key that derives to an Ed25519 peer id', async () => {
+    const key = (await generateKeyPair('Ed25519')).publicKey
+
+    const result = await publicKeyForPeer(peerFor(key), publicKeyToProtobuf(key))
+
+    expect(result.toMultihash().bytes).to.equalBytes(key.toMultihash().bytes)
+  })
+
+  it('rejects a supplied key that does not derive to an Ed25519 peer id', async () => {
+    const victim = (await generateKeyPair('Ed25519')).publicKey
+    const attacker = (await generateKeyPair('Ed25519')).publicKey
+
+    await expect(publicKeyForPeer(peerFor(victim), attacker)).to.eventually.be.rejected
+      .with.property('name', 'InvalidPublicKeyError')
+  })
+
+  it('rejects a supplied key that does not derive to a secp256k1 peer id', async () => {
+    const victim = (await generateKeyPair('secp256k1')).publicKey
+    const attacker = (await generateKeyPair('Ed25519')).publicKey
+
+    await expect(publicKeyForPeer(peerFor(victim), attacker)).to.eventually.be.rejected
+      .with.property('name', 'InvalidPublicKeyError')
+  })
+
+  it('rejects malformed key bytes', async () => {
+    const key = (await generateKeyPair('Ed25519')).publicKey
+
+    await expect(publicKeyForPeer(peerFor(key), uint8ArrayFromString('not a key'))).to.eventually.be.rejected
+      .with.property('name', 'InvalidPublicKeyError')
+  })
+
+  describe('RSA peer id without a key', () => {
+    let key: PublicKey
+
+    before(async () => {
+      key = (await generateKeyPair('RSA', 2048)).publicKey
+    })
+
+    it('accepts a supplied key that derives to the peer id', async () => {
+      const result = await publicKeyForPeer(peerFor(key), publicKeyToProtobuf(key))
+
+      expect(result.toMultihash().bytes).to.equalBytes(key.toMultihash().bytes)
+    })
+
+    it('rejects a supplied key that does not derive to the peer id', async () => {
+      const attacker = (await generateKeyPair('Ed25519')).publicKey
+
+      await expect(publicKeyForPeer(peerFor(key), attacker)).to.eventually.be.rejected
+        .with.property('name', 'InvalidPublicKeyError')
+    })
+
+    it('rejects when no key is supplied', async () => {
+      await expect(publicKeyForPeer(peerFor(key))).to.eventually.be.rejected
+        .with.property('name', 'InvalidPublicKeyError')
+    })
+  })
+
+  it('rejects a URL peer id', async () => {
+    const url: Peer = {
+      publicKey: undefined,
+      toMultihash: () => identity.digest(uint8ArrayFromString('https://example.com'))
+    } as any
+
+    await expect(publicKeyForPeer(url)).to.eventually.be.rejected
+      .with.property('name', 'InvalidPublicKeyError')
+  })
+
+  it('rejects when aborted', async () => {
+    const key = (await generateKeyPair('Ed25519')).publicKey
+
+    await expect(publicKeyForPeer(peerFor(key), undefined, { signal: AbortSignal.abort() })).to.eventually.be.rejected
+      .with.property('name', 'AbortError')
+  })
+})

packages/floodsub/src/sign.ts

@@ -1,3 +1,4 @@
+import { publicKeyForPeer } from '@libp2p/crypto/keys'
 import { peerIdFromPrivateKey } from '@libp2p/peer-id'
 import { concat as uint8ArrayConcat } from 'uint8arrays/concat'
 import { fromString as uint8ArrayFromString } from 'uint8arrays/from-string'
@@ -9,7 +10,7 @@ import type { PeerId, PrivateKey, PublicKey } from '@libp2p/interface'
 export const SignPrefix = uint8ArrayFromString('libp2p-pubsub:')
 
 /**
- * Signs the provided message with the given `peerId`
+ * Signs the provided message with the given private key
  */
 export async function signMessage (privateKey: PrivateKey, message: { from: PeerId, topic: string, data: Uint8Array, sequenceNumber: bigint }, encode: (rpc: PubSubRPCMessage) => Uint8Array): Promise<SignedMessage> {
   // @ts-expect-error signature field is missing, added below
@@ -34,7 +35,8 @@ export async function signMessage (privateKey: PrivateKey, message: { from: Peer
 }
 
 /**
- * Verifies the signature of the given message
+ * Verifies the signature of the given message, returning false if its key does
+ * not belong to the author
  */
 export async function verifySignature (message: SignedMessage, encode: (rpc: PubSubRPCMessage) => Uint8Array): Promise<boolean> {
   if (message.type !== 'signed') {
@@ -59,39 +61,19 @@ export async function verifySignature (message: SignedMessage, encode: (rpc: Pub
     }).subarray()
   ])
 
-  // Get the public key
-  const pubKey = messagePublicKey(message)
+  let pubKey: PublicKey
 
-  // verify the base message
-  return pubKey.verify(bytes, message.signature)
-}
+  try {
+    // only trust a supplied key if it derives to the author
+    pubKey = await publicKeyForPeer(message.from, message.key)
+  } catch (err: any) {
+    if (err.name === 'InvalidPublicKeyError') {
+      return false
+    }
 
-/**
- * Returns the PublicKey associated with the given message.
- * If no valid PublicKey can be retrieved an error will be returned.
- */
-export function messagePublicKey (message: SignedMessage): PublicKey {
-  if (message.type !== 'signed') {
-    throw new Error('Message type must be "signed" to have a public key')
+    throw err
   }
 
-  // should be available in the from property of the message (peer id)
-  if (message.from == null) {
-    throw new Error('Could not get the public key from the originator id')
-  }
-
-  // an Ed25519/secp256k1 peer id embeds its own public key, so a key recovered
-  // from `from` is provably the author's; trust it over the supplied message.key
-  if (message.from.publicKey != null) {
-    return message.from.publicKey
-  }
-
-  // otherwise the supplied key must derive to the from peer id, else a message
-  // could be signed by one key while claiming a different author
-  if (message.key != null && message.from.equals(message.key.toMultihash().bytes)) {
-    return message.key
-  }
-
-  // We couldn't validate pubkey is from the originator, error
-  throw new Error('Could not get the public key from the originator id')
+  // verify the base message
+  return pubKey.verify(bytes, message.signature)
 }

packages/floodsub/test/forgery.spec.ts

@@ -0,0 +1,125 @@
+/* eslint-env mocha */
+import { generateKeyPair, publicKeyToProtobuf } from '@libp2p/crypto/keys'
+import { start, stop } from '@libp2p/interface'
+import { defaultLogger } from '@libp2p/logger'
+import { peerIdFromPrivateKey } from '@libp2p/peer-id'
+import { expect } from 'aegir/chai'
+import { stubInterface } from 'sinon-ts'
+import { concat as uint8ArrayConcat } from 'uint8arrays/concat'
+import { fromString as uint8ArrayFromString } from 'uint8arrays/from-string'
+import { floodsub } from '../src/index.ts'
+import { RPC } from '../src/message/rpc.ts'
+import { PeerStreams } from '../src/peer-streams.ts'
+import { SignPrefix } from '../src/sign.ts'
+import { randomSeqno, toRpcMessage } from '../src/utils.ts'
+import type { PubSubRPCMessage } from '../src/floodsub.ts'
+import type { FloodSub } from '../src/index.ts'
+import type { PeerId, PrivateKey, PublicKey } from '@libp2p/interface'
+import type { Registrar } from '@libp2p/interface-internal'
+
+const topic = 'test-topic'
+const data = uint8ArrayFromString('hello')
+
+// a wire message claiming to be from `from`, signed by `signer` and carrying
+// `key` in its key field
+async function wireMessage (from: PeerId, signer: PrivateKey, key: PublicKey): Promise<PubSubRPCMessage> {
+  const unsigned = toRpcMessage({
+    type: 'signed',
+    from,
+    data,
+    sequenceNumber: randomSeqno(),
+    topic,
+    signature: new Uint8Array(0),
+    key
+  })
+  delete unsigned.signature
+  delete unsigned.key
+
+  const signature = await signer.sign(uint8ArrayConcat([SignPrefix, RPC.Message.encode(unsigned)]))
+
+  return { ...unsigned, signature, key: publicKeyToProtobuf(key) }
+}
+
+describe('forged messages over the wire', () => {
+  let pubsub: FloodSub
+  let sender: PeerStreams
+  let attackerKey: PrivateKey
+  let delivered: number
+
+  beforeEach(async () => {
+    const privateKey = await generateKeyPair('Ed25519')
+
+    pubsub = floodsub()({
+      peerId: peerIdFromPrivateKey(privateKey),
+      privateKey,
+      registrar: stubInterface<Registrar>(),
+      logger: defaultLogger()
+    })
+    await start(pubsub)
+    pubsub.subscribe(topic)
+
+    attackerKey = await generateKeyPair('Ed25519')
+    sender = new PeerStreams(peerIdFromPrivateKey(attackerKey))
+    delivered = 0
+    pubsub.addEventListener('message', () => { delivered++ })
+  })
+
+  afterEach(async () => {
+    await stop(pubsub)
+  })
+
+  async function receive (message: PubSubRPCMessage): Promise<void> {
+    // @ts-expect-error private method
+    await pubsub['processRpc'](sender, { subscriptions: [], messages: [message] })
+    // @ts-expect-error private field
+    await pubsub.queue.onIdle()
+  }
+
+  it('delivers a message signed by its author', async () => {
+    await receive(await wireMessage(sender.peerId, attackerKey, attackerKey.publicKey))
+
+    expect(delivered).to.equal(1)
+  })
+
+  it('delivers a message relayed from a different Ed25519 author', async () => {
+    const authorKey = await generateKeyPair('Ed25519')
+
+    await receive(await wireMessage(peerIdFromPrivateKey(authorKey), authorKey, authorKey.publicKey))
+
+    expect(delivered).to.equal(1)
+  })
+
+  it('delivers a message from an RSA author carrying its key', async () => {
+    const authorKey = await generateKeyPair('RSA', 2048)
+
+    await receive(await wireMessage(peerIdFromPrivateKey(authorKey), authorKey, authorKey.publicKey))
+
+    expect(delivered).to.equal(1)
+  })
+
+  it('drops a message signed by the attacker claiming an Ed25519 author', async () => {
+    const victim = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
+
+    await receive(await wireMessage(victim, attackerKey, attackerKey.publicKey))
+
+    expect(delivered).to.equal(0)
+  })
+
+  // dropped as unsigned by `isSigned` before signature verification, since the
+  // key does not derive to the author
+  it('drops a message signed by the attacker claiming an RSA author', async () => {
+    const victim = peerIdFromPrivateKey(await generateKeyPair('RSA', 2048))
+
+    await receive(await wireMessage(victim, attackerKey, attackerKey.publicKey))
+
+    expect(delivered).to.equal(0)
+  })
+
+  it('drops a message signed by its Ed25519 author that carries a different key', async () => {
+    const authorKey = await generateKeyPair('Ed25519')
+
+    await receive(await wireMessage(peerIdFromPrivateKey(authorKey), authorKey, attackerKey.publicKey))
+
+    expect(delivered).to.equal(0)
+  })
+})

packages/floodsub/test/sign.spec.ts

@@ -150,9 +150,8 @@ describe('author key binding', () => {
     await expect(verifySignature(forged, encodeMessage)).to.eventually.equal(false)
   })
 
-  // an RSA peer id does not carry its public key, so `from.publicKey` is
-  // undefined and verification goes through the derive-to-author check rather
-  // than the authoritative-key branch that the inlined types above exercise
+  // an RSA peer id does not carry its public key, so the message key is the only
+  // source and must derive to the author
   describe('peer id without an inline public key (RSA)', () => {
     let rsaKey: PrivateKey
     let author: PeerId
@@ -175,7 +174,7 @@ describe('author key binding', () => {
     it('rejects a message whose key does not derive to the author', async () => {
       const forged = await forge(author, await generateKeyPair('Ed25519'))
 
-      await expect(verifySignature(forged, encodeMessage)).to.be.rejectedWith('Could not get the public key from the originator id')
+      await expect(verifySignature(forged, encodeMessage)).to.eventually.equal(false)
     })
   })
 })