Skip to content
This repository has been archived by the owner on Jun 27, 2023. It is now read-only.

fix: allow key field to be unset #118

Merged
merged 2 commits into from
Jan 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 24 additions & 5 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { fromString as uint8ArrayFromString } from 'uint8arrays/from-string'
import { toString as uint8ArrayToString } from 'uint8arrays/to-string'
import { sha256 } from 'multiformats/hashes/sha2'
import type { Message, PubSubRPCMessage } from '@libp2p/interface-pubsub'
import { peerIdFromBytes } from '@libp2p/peer-id'
import { peerIdFromBytes, peerIdFromKeys } from '@libp2p/peer-id'
import { codes } from './errors.js'
import { CodeError } from '@libp2p/interfaces/errors'

Expand Down Expand Up @@ -66,12 +66,30 @@ export const ensureArray = function <T> (maybeArray: T | T[]) {
return maybeArray
}

export const toMessage = (message: PubSubRPCMessage): Message => {
const isSigned = async (message: PubSubRPCMessage): Promise<boolean> => {
if ((message.sequenceNumber == null) || (message.from == null) || (message.signature == null)) {
return false
}
// if a public key is present in the `from` field, the message should be signed
const fromID = peerIdFromBytes(message.from)
if (fromID.publicKey != null) {
return true
}

if (message.key != null) {
const signingID = await peerIdFromKeys(message.key)
return signingID.equals(fromID)
}

return false
}

export const toMessage = async (message: PubSubRPCMessage): Promise<Message> => {
if (message.from == null) {
throw new CodeError('RPC message was missing from', codes.ERR_MISSING_FROM)
}

if (message.sequenceNumber == null || message.from == null || message.signature == null || message.key == null) {
if (!await isSigned(message)) {
return {
type: 'unsigned',
topic: message.topic ?? '',
Expand All @@ -83,9 +101,10 @@ export const toMessage = (message: PubSubRPCMessage): Message => {
type: 'signed',
from: peerIdFromBytes(message.from),
topic: message.topic ?? '',
sequenceNumber: bigIntFromBytes(message.sequenceNumber),
sequenceNumber: bigIntFromBytes(message.sequenceNumber ?? new Uint8Array(0)),
data: message.data ?? new Uint8Array(0),
signature: message.signature,
signature: message.signature ?? new Uint8Array(0),
// @ts-expect-error key need not be defined
key: message.key
Comment on lines +107 to 108
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of ts-expect-error, please verify that the key is present and correct - e.g. it should come from the message or the peer id, and if it's present in both the message and the peer id, they should be the same.

Copy link
Contributor Author

@ckousik ckousik Jan 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. The @ts-expect-error has been retained since it should be nullable. The value of message.key is now verified in isSigned.

As far as secp256k1 or ed25519 keys are concerned, consider the current behaviour here. Since the public key is extractable, the msg.Key field is left unset, and the current js implementation considers this as an unsigned message.

}
}
Expand Down
35 changes: 35 additions & 0 deletions test/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as utils from '../src/utils.js'
import { fromString as uint8ArrayFromString } from 'uint8arrays/from-string'
import type { Message, PubSubRPCMessage } from '@libp2p/interface-pubsub'
import { peerIdFromBytes, peerIdFromString } from '@libp2p/peer-id'
import * as PeerIdFactory from '@libp2p/peer-id-factory'

describe('utils', () => {
it('randomSeqno', () => {
Expand Down Expand Up @@ -94,4 +95,38 @@ describe('utils', () => {
expect(utils.bigIntFromBytes(utils.bigIntToBytes(val))).to.equal(val)
})
})

it('ensures message is signed if public key is extractable', async () => {
const dummyPeerID = await PeerIdFactory.createRSAPeerId()

const cases: PubSubRPCMessage[] = [
{
from: (await PeerIdFactory.createSecp256k1PeerId()).toBytes(),
topic: 'test',
data: new Uint8Array(0),
sequenceNumber: utils.bigIntToBytes(1n),
signature: new Uint8Array(0)
},
{
from: peerIdFromString('QmPNdSYk5Rfpo5euNqwtyizzmKXMNHdXeLjTQhcN4yfX22').toBytes(),
topic: 'test',
data: new Uint8Array(0),
sequenceNumber: utils.bigIntToBytes(1n),
signature: new Uint8Array(0)
},
{
from: dummyPeerID.toBytes(),
topic: 'test',
data: new Uint8Array(0),
sequenceNumber: utils.bigIntToBytes(1n),
signature: new Uint8Array(0),
key: dummyPeerID.publicKey
}
]
const expected = ['signed', 'unsigned', 'signed']

const actual = (await Promise.all(cases.map(utils.toMessage))).map(m => m.type)

expect(actual).to.deep.equal(expected)
})
})