From abc5f22a628f8ffbbad8cf261fe3e21e1005f842 Mon Sep 17 00:00:00 2001 From: tabcat Date: Thu, 16 Jul 2026 02:07:35 +0700 Subject: [PATCH 1/3] fix(gossipsub): validate the message key for all peer id types The message key was checked against the from peer id by comparing public keys directly, which only applies to peer ids that inline their public key. It now derives the peer id from the key and compares that, so the check applies to every peer id type. --- .../gossipsub/src/utils/buildRawMessage.ts | 7 ++-- .../test/unit/buildRawMessage.spec.ts | 41 ++++++++++++++++++- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/packages/gossipsub/src/utils/buildRawMessage.ts b/packages/gossipsub/src/utils/buildRawMessage.ts index bfd19dbd3e..1060b5effc 100644 --- a/packages/gossipsub/src/utils/buildRawMessage.ts +++ b/packages/gossipsub/src/utils/buildRawMessage.ts @@ -1,5 +1,5 @@ import { publicKeyFromProtobuf } from '@libp2p/crypto/keys' -import { peerIdFromMultihash } from '@libp2p/peer-id' +import { peerIdFromMultihash, peerIdFromPublicKey } from '@libp2p/peer-id' import * as Digest from 'multiformats/hashes/digest' import { concat as uint8ArrayConcat } from 'uint8arrays/concat' import { fromString as uint8ArrayFromString } from 'uint8arrays/from-string' @@ -139,8 +139,9 @@ export async function validateToRawMessage ( let publicKey: PublicKey if (msg.key != null) { publicKey = publicKeyFromProtobuf(msg.key) - // TODO: Should `fromPeerId.pubKey` be optional? - if (fromPeerId.publicKey !== undefined && !publicKey.equals(fromPeerId.publicKey)) { + // derive the peer id from the key and compare, so this applies to peer + // ids that do not inline their public key (RSA) as well as those that do + if (!peerIdFromPublicKey(publicKey).equals(fromPeerId)) { return { valid: false, error: ValidateError.InvalidPeerId } } } else { diff --git a/packages/gossipsub/test/unit/buildRawMessage.spec.ts b/packages/gossipsub/test/unit/buildRawMessage.spec.ts index e00ec39e7b..df413eb264 100644 --- a/packages/gossipsub/test/unit/buildRawMessage.spec.ts +++ b/packages/gossipsub/test/unit/buildRawMessage.spec.ts @@ -3,8 +3,9 @@ import { peerIdFromPrivateKey } from '@libp2p/peer-id' import { expect } from 'aegir/chai' import { fromString as uint8ArrayFromString } from 'uint8arrays/from-string' import { StrictSign } from '../../src/index.ts' -import { buildRawMessage } from '../../src/utils/buildRawMessage.ts' +import { buildRawMessage, validateToRawMessage } from '../../src/utils/buildRawMessage.ts' import { getPublishConfigFromPeerId } from '../../src/utils/publishConfig.ts' +import type { PrivateKey } from '@libp2p/interface' describe('buildRawMessage', () => { describe('Signing seqno', () => { @@ -28,4 +29,42 @@ describe('buildRawMessage', () => { } }) }) + + describe('RSA author key binding', () => { + const topic = 'test-topic' + const data = uint8ArrayFromString('hello') + let victimKey: PrivateKey + + before(async function () { + // RSA key generation is slow + this.timeout(30_000) + victimKey = await generateKeyPair('RSA', 2048) + }) + + it('accepts a message signed by the genuine RSA author', async () => { + const victim = peerIdFromPrivateKey(victimKey) + const config = getPublishConfigFromPeerId(StrictSign, victim, victimKey) + const { raw } = await buildRawMessage(config, topic, data, data) + + const result = await validateToRawMessage(StrictSign, raw) + expect(result.valid).to.equal(true) + }) + + it('rejects a message whose signing key does not derive to the RSA author', async () => { + const victim = peerIdFromPrivateKey(victimKey) + const attackerKey = await generateKeyPair('Ed25519') + + // claim the victim RSA peer id as author but sign with the attacker's key. + // getPublishConfigFromPeerId does not check that the peer id matches the + // private key, so `from` is the victim while the signature and `key` field + // are the attacker's + const forgedConfig = getPublishConfigFromPeerId(StrictSign, victim, attackerKey) + const { raw } = await buildRawMessage(forgedConfig, topic, data, data) + + expect(raw.from).to.deep.equal(victim.toMultihash().bytes) + + const result = await validateToRawMessage(StrictSign, raw) + expect(result.valid).to.equal(false) + }) + }) }) From abc7a941c16590a092458c974ce2e5eb93c745d2 Mon Sep 17 00:00:00 2001 From: tabcat Date: Fri, 17 Jul 2026 02:02:23 +0700 Subject: [PATCH 2/3] fix(gossipsub): compare the derived multihash for all key types peerIdFromPublicKey throws UnsupportedKeyTypeError for ECDSA keys, which escaped the validator and skipped the invalid-message score penalty. Comparing the multihash the key derives to is defined for every key type. --- packages/gossipsub/src/utils/buildRawMessage.ts | 8 ++++---- packages/gossipsub/test/unit/buildRawMessage.spec.ts | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/gossipsub/src/utils/buildRawMessage.ts b/packages/gossipsub/src/utils/buildRawMessage.ts index 1060b5effc..fca6ee7679 100644 --- a/packages/gossipsub/src/utils/buildRawMessage.ts +++ b/packages/gossipsub/src/utils/buildRawMessage.ts @@ -1,5 +1,5 @@ import { publicKeyFromProtobuf } from '@libp2p/crypto/keys' -import { peerIdFromMultihash, peerIdFromPublicKey } from '@libp2p/peer-id' +import { peerIdFromMultihash } from '@libp2p/peer-id' import * as Digest from 'multiformats/hashes/digest' import { concat as uint8ArrayConcat } from 'uint8arrays/concat' import { fromString as uint8ArrayFromString } from 'uint8arrays/from-string' @@ -139,9 +139,9 @@ export async function validateToRawMessage ( let publicKey: PublicKey if (msg.key != null) { publicKey = publicKeyFromProtobuf(msg.key) - // derive the peer id from the key and compare, so this applies to peer - // ids that do not inline their public key (RSA) as well as those that do - if (!peerIdFromPublicKey(publicKey).equals(fromPeerId)) { + + // the message key must derive to the `from` peer id + if (!fromPeerId.equals(publicKey.toMultihash().bytes)) { return { valid: false, error: ValidateError.InvalidPeerId } } } else { diff --git a/packages/gossipsub/test/unit/buildRawMessage.spec.ts b/packages/gossipsub/test/unit/buildRawMessage.spec.ts index df413eb264..544603cef0 100644 --- a/packages/gossipsub/test/unit/buildRawMessage.spec.ts +++ b/packages/gossipsub/test/unit/buildRawMessage.spec.ts @@ -3,6 +3,7 @@ import { peerIdFromPrivateKey } from '@libp2p/peer-id' import { expect } from 'aegir/chai' import { fromString as uint8ArrayFromString } from 'uint8arrays/from-string' import { StrictSign } from '../../src/index.ts' +import { ValidateError } from '../../src/types.ts' import { buildRawMessage, validateToRawMessage } from '../../src/utils/buildRawMessage.ts' import { getPublishConfigFromPeerId } from '../../src/utils/publishConfig.ts' import type { PrivateKey } from '@libp2p/interface' @@ -64,7 +65,7 @@ describe('buildRawMessage', () => { expect(raw.from).to.deep.equal(victim.toMultihash().bytes) const result = await validateToRawMessage(StrictSign, raw) - expect(result.valid).to.equal(false) + expect(result).to.deep.equal({ valid: false, error: ValidateError.InvalidPeerId }) }) }) }) From 900e2c6f478fefea9e95dbdec5f4e2427ed7cff1 Mon Sep 17 00:00:00 2001 From: tabcat Date: Sat, 18 Jul 2026 01:28:03 +0700 Subject: [PATCH 3/3] refactor(gossipsub): reuse the verified key instead of re-decoding msg.key The success return re-decoded msg.key a second time; publicKey already holds that exact value, so reuse it and drop the redundant RSA DER parse. --- packages/gossipsub/src/utils/buildRawMessage.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gossipsub/src/utils/buildRawMessage.ts b/packages/gossipsub/src/utils/buildRawMessage.ts index fca6ee7679..498cc0b986 100644 --- a/packages/gossipsub/src/utils/buildRawMessage.ts +++ b/packages/gossipsub/src/utils/buildRawMessage.ts @@ -177,7 +177,7 @@ export async function validateToRawMessage ( sequenceNumber: BigInt(`0x${uint8ArrayToString(msg.seqno, 'base16')}`), topic: msg.topic, signature: msg.signature, - key: msg.key != null ? publicKeyFromProtobuf(msg.key) : publicKey + key: publicKey } } }