diff --git a/packages/crypto/src/keys/index.ts b/packages/crypto/src/keys/index.ts index 255e6e2524..8bb072d0b5 100644 --- a/packages/crypto/src/keys/index.ts +++ b/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, key?: Uint8Array | PublicKey, options?: AbortOptions): Promise { + 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 */ diff --git a/packages/crypto/test/keys/public-key-for-peer.spec.ts b/packages/crypto/test/keys/public-key-for-peer.spec.ts new file mode 100644 index 0000000000..7bad0384c1 --- /dev/null +++ b/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 + +// 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') + }) +}) diff --git a/packages/floodsub/src/sign.ts b/packages/floodsub/src/sign.ts index e5cc1b39bf..9565336402 100644 --- a/packages/floodsub/src/sign.ts +++ b/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 { // @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 { 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) } diff --git a/packages/floodsub/test/forgery.spec.ts b/packages/floodsub/test/forgery.spec.ts new file mode 100644 index 0000000000..2754b3d18a --- /dev/null +++ b/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 { + 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(), + 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 { + // @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) + }) +}) diff --git a/packages/floodsub/test/sign.spec.ts b/packages/floodsub/test/sign.spec.ts index b5ef3c3ac8..0f2e8e768a 100644 --- a/packages/floodsub/test/sign.spec.ts +++ b/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) }) }) }) diff --git a/packages/gossipsub/src/utils/buildRawMessage.ts b/packages/gossipsub/src/utils/buildRawMessage.ts index 890f78e0a6..fbae59258b 100644 --- a/packages/gossipsub/src/utils/buildRawMessage.ts +++ b/packages/gossipsub/src/utils/buildRawMessage.ts @@ -1,4 +1,4 @@ -import { publicKeyFromProtobuf } from '@libp2p/crypto/keys' +import { publicKeyForPeer, publicKeyFromProtobuf } from '@libp2p/crypto/keys' import { peerIdFromMultihash } from '@libp2p/peer-id' import * as Digest from 'multiformats/hashes/digest' import { concat as uint8ArrayConcat } from 'uint8arrays/concat' @@ -129,32 +129,13 @@ export async function validateToRawMessage ( return { valid: false, error: ValidateError.InvalidPeerId } } - // - check from defined - // - transform source to PeerId - // - parse signature - // - get .key, else from source - // - check key == source if present - // - verify sig - let publicKey: PublicKey - if (msg.key != null) { - // malformed key bytes must reject rather than throw, so an attacker - // sending them is still scored as delivering an invalid message - try { - publicKey = publicKeyFromProtobuf(msg.key) - } catch { - return { valid: false, error: ValidateError.InvalidPeerId } - } - - // the message key must derive to the `from` peer id - if (!fromPeerId.equals(publicKey.toMultihash().bytes)) { - return { valid: false, error: ValidateError.InvalidPeerId } - } - } else { - if (fromPeerId.publicKey == null) { - return { valid: false, error: ValidateError.InvalidPeerId } - } - publicKey = fromPeerId.publicKey + try { + publicKey = await publicKeyForPeer(fromPeerId, msg.key) + } catch { + // mismatched, missing or malformed keys are invalid, not thrown, so the + // peer is still scored + return { valid: false, error: ValidateError.InvalidPeerId } } const rpcMsgPreSign: RPC.Message = { diff --git a/packages/gossipsub/test/unit/buildRawMessage.spec.ts b/packages/gossipsub/test/unit/buildRawMessage.spec.ts index 9b81f146e9..bea2c923c2 100644 --- a/packages/gossipsub/test/unit/buildRawMessage.spec.ts +++ b/packages/gossipsub/test/unit/buildRawMessage.spec.ts @@ -101,6 +101,24 @@ describe('buildRawMessage', () => { const result = await validateToRawMessage(StrictSign, raw) expect(result).to.deep.equal({ valid: false, error: ValidateError.InvalidPeerId }) }) + + it('rejects a message from an RSA author without a key', async () => { + const victim = peerIdFromPrivateKey(victimKey) + const { raw } = await buildRawMessage(getPublishConfigFromPeerId(StrictSign, victim, victimKey), topic, data, data) + raw.key = undefined + + await expect(validateToRawMessage(StrictSign, raw)).to.eventually.deep.equal({ valid: false, error: ValidateError.InvalidPeerId }) + }) + }) + + describe('Ed25519 author key binding', () => { + it('rejects a message whose signing key does not derive to the Ed25519 author', async () => { + const victim = peerIdFromPrivateKey(await generateKeyPair('Ed25519')) + const attackerKey = await generateKeyPair('Ed25519') + const { raw } = await buildRawMessage(getPublishConfigFromPeerId(StrictSign, victim, attackerKey), 'test-topic', uint8ArrayFromString('hello'), uint8ArrayFromString('hello')) + + await expect(validateToRawMessage(StrictSign, raw)).to.eventually.deep.equal({ valid: false, error: ValidateError.InvalidPeerId }) + }) }) describe('malformed message fields', () => { diff --git a/packages/libp2p/src/libp2p.ts b/packages/libp2p/src/libp2p.ts index 7a88b7b8c7..1d7e562daf 100644 --- a/packages/libp2p/src/libp2p.ts +++ b/packages/libp2p/src/libp2p.ts @@ -1,5 +1,5 @@ -import { publicKeyFromProtobuf } from '@libp2p/crypto/keys' -import { contentRoutingSymbol, peerDiscoverySymbol, peerRoutingSymbol, InvalidParametersError, InvalidPublicKeyError } from '@libp2p/interface' +import { publicKeyForPeer } from '@libp2p/crypto/keys' +import { contentRoutingSymbol, peerDiscoverySymbol, peerRoutingSymbol, InvalidParametersError } from '@libp2p/interface' import { defaultLogger } from '@libp2p/logger' import { PeerSet } from '@libp2p/peer-collections' import { peerIdFromString } from '@libp2p/peer-id' @@ -359,12 +359,8 @@ export class Libp2p extends TypedEventEmitter // search any available content routing methods const bytes = await this.contentRouting.get(peerKey, options) - const publicKey = publicKeyFromProtobuf(bytes) - // ensure the returned key belongs to the requested peer - if (!peer.equals(publicKey.toMultihash().bytes)) { - throw new InvalidPublicKeyError('public key did not match requested peer') - } + const publicKey = await publicKeyForPeer(peer, bytes, options) await this.peerStore.patch(peer, { publicKey diff --git a/packages/libp2p/test/core/get-public-key.spec.ts b/packages/libp2p/test/core/get-public-key.spec.ts index dcf9ec5ff3..322e758fbc 100644 --- a/packages/libp2p/test/core/get-public-key.spec.ts +++ b/packages/libp2p/test/core/get-public-key.spec.ts @@ -88,7 +88,7 @@ describe('getPublicKey', () => { // key is fetched from content routing const otherPeerWithoutPublicKey = peerIdFromMultihash(otherPeer.toMultihash()) - await expect(node.getPublicKey(otherPeerWithoutPublicKey)).to.eventually.be.rejectedWith(/did not match requested peer/) + await expect(node.getPublicKey(otherPeerWithoutPublicKey)).to.eventually.be.rejected.with.property('name', 'InvalidPublicKeyError') expect(patchSpy.called).to.be.false() }) })