From 1152068d2f99489037c613816e8fe3b16a8fd49f Mon Sep 17 00:00:00 2001 From: Luke Evanson Date: Thu, 17 Sep 2026 16:02:05 -0700 Subject: [PATCH 1/3] Fix message acknowledgment logic and signature handling --- src/server/chat.js | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/server/chat.js b/src/server/chat.js index 2258fdc68..f2013e713 100644 --- a/src/server/chat.js +++ b/src/server/chat.js @@ -6,6 +6,11 @@ const messageExpireTime = 300000 // 5 min (ms) const { mojangPublicKeyPem } = require('./constants') class VerificationError extends Error {} + +function signaturesEqual (a, b) { + if (Buffer.isBuffer(a) && Buffer.isBuffer(b)) return a.equals(b) + return a === b +} function validateLastMessages (pending, lastSeen, lastRejected) { if (lastRejected) { const rejectedTime = pending.get(lastRejected.sender, lastRejected.signature) @@ -19,7 +24,7 @@ function validateLastMessages (pending, lastSeen, lastRejected) { for (const { messageSender, messageSignature } of lastSeen) { if (pending.previouslyAcknowledged(messageSender, messageSignature)) continue - const ts = pending.get(messageSender)(messageSignature) + const ts = pending.get(messageSender, messageSignature) if (!ts) { throw new VerificationError(`Client saw a message that we never sent from '${messageSender}'`) } else if (lastTimestamp && (ts < lastTimestamp)) { @@ -242,20 +247,20 @@ class Pending extends Array { this.push([sender, signature]) } - acknowledge (sender, username) { - delete this.m[sender][username] - this.splice(this.findIndex(([a, b]) => a === sender && b === username), 1) + acknowledge (sender, signature) { + if (this.m[sender]) delete this.m[sender][signature] + const index = this.findIndex(([a, b]) => a === sender && signaturesEqual(b, signature)) + if (index !== -1) this.splice(index, 1) } acknowledgePrior (sender, signature) { - for (let i = 0; i < this.length; i++) { + const index = this.findIndex(([a, b]) => a === sender && signaturesEqual(b, signature)) + if (index === -1) return + for (let i = 0; i <= index; i++) { const [a, b] = this[i] - delete this.m[a] - if (a === sender && b === signature) { - this.splice(0, i) - break - } + if (this.m[a]) delete this.m[a][b] } + this.splice(0, index + 1) } // Once we've acknowledged that the client has saw the messages we sent, @@ -264,10 +269,12 @@ class Pending extends Array { // we need to store it in memory to allow those entries to be approved again without // erroring about a message we never sent in the next serverbound message packet we get. setPreviouslyAcknowledged (lastSeen, lastRejected = {}) { - this.lastSeen = lastSeen.map(e => Object.values(e)).push(Object.values(lastRejected)) + this.lastSeen = lastSeen.map(e => Object.values(e)) + const rejected = Object.values(lastRejected) + if (rejected.length) this.lastSeen.push(rejected) } previouslyAcknowledged (sender, signature) { - return this.lastSeen.some(([a, b]) => a === sender && b === signature) + return this.lastSeen.some(([a, b]) => a === sender && signaturesEqual(b, signature)) } } From 3fb009bd66cfa2b4930e4415e2ec896cb4506aac Mon Sep 17 00:00:00 2001 From: Luke Evanson Date: Thu, 17 Sep 2026 16:02:36 -0700 Subject: [PATCH 2/3] test(server): regression tests for Pending lastSeen acknowledgements --- test/chatPendingTest.js | 54 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 test/chatPendingTest.js diff --git a/test/chatPendingTest.js b/test/chatPendingTest.js new file mode 100644 index 000000000..96bf81de2 --- /dev/null +++ b/test/chatPendingTest.js @@ -0,0 +1,54 @@ +/* eslint-env mocha */ + +const EventEmitter = require('events') +const assert = require('power-assert') +const injectChat = require('../src/server/chat') + +const SENDER_A = '11111111-1111-1111-1111-111111111111' +const SENDER_B = '22222222-2222-2222-2222-222222222222' + +function makeServer () { + const client = new EventEmitter() + client.supportFeature = (feature) => feature === 'chainedChatWithHashing' // 1.19.1/1.19.2: Pending path + client.settings = {} + client.socket = { address: () => '127.0.0.1' } + const ended = [] + const errors = [] + client.end = (reason) => ended.push(reason) + client.on('error', (err) => errors.push(err)) + injectChat(client, { features: { signedChat: true } }, { enforceSecureProfile: true, hideErrors: true }) + client.verifyMessage = () => true // injectChat installs the real verifier; stub it after injection + return { client, ended, errors } +} + +function chat (previousMessages, lastRejectedMessage) { + return { timestamp: BigInt(Date.now()), previousMessages, lastRejectedMessage } +} + +function seen (sender, signature) { + return { messageSender: sender, messageSignature: Buffer.from(signature) } +} + +describe('server chat Pending lastSeen bookkeeping', () => { + it('validates a chain of acknowledgements across chat packets', () => { + const { client, ended, errors } = makeServer() + client.logSentMessageFromPeer({ senderUuid: SENDER_A, signature: Buffer.from('sig-a1'), timestamp: 1n }) + client.logSentMessageFromPeer({ senderUuid: SENDER_B, signature: Buffer.from('sig-b1'), timestamp: 2n }) + client.logSentMessageFromPeer({ senderUuid: SENDER_A, signature: Buffer.from('sig-a2'), timestamp: 3n }) + + client.emit('chat_message', chat([seen(SENDER_A, 'sig-a1'), seen(SENDER_B, 'sig-b1')])) + // B's entry repeats (re-parsed buffer, same bytes); A's second message is new + client.emit('chat_message', chat([seen(SENDER_B, 'sig-b1'), seen(SENDER_A, 'sig-a2')])) + + assert.deepStrictEqual(ended, []) + assert.deepStrictEqual(errors, []) + }) + + it('still rejects a message the server never sent', () => { + const { client, ended } = makeServer() + client.logSentMessageFromPeer({ senderUuid: SENDER_A, signature: Buffer.from('sig-a1'), timestamp: 1n }) + client.emit('chat_message', chat([seen(SENDER_A, 'sig-a1')])) + client.emit('chat_message', chat([seen(SENDER_B, 'sig-unknown')])) + assert.deepStrictEqual(ended, ['multiplayer.disconnect.chat_validation_failed']) + }) +}) From 415a12ab667c739cc263cc82f40bf0701c6efdef Mon Sep 17 00:00:00 2001 From: Luke Evanson Date: Sat, 3 Oct 2026 12:42:05 -0700 Subject: [PATCH 3/3] test: use real 1.19.2 features for Pending acknowledgements Use minecraft-data feature support after the signedChat logging change. Include 1.19.2v in the suite title so normal version-selected CI runs the regression cases. --- test/chatPendingTest.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/chatPendingTest.js b/test/chatPendingTest.js index 96bf81de2..6159415ea 100644 --- a/test/chatPendingTest.js +++ b/test/chatPendingTest.js @@ -3,20 +3,21 @@ const EventEmitter = require('events') const assert = require('power-assert') const injectChat = require('../src/server/chat') +const mcData = require('minecraft-data')('1.19.2') const SENDER_A = '11111111-1111-1111-1111-111111111111' const SENDER_B = '22222222-2222-2222-2222-222222222222' function makeServer () { const client = new EventEmitter() - client.supportFeature = (feature) => feature === 'chainedChatWithHashing' // 1.19.1/1.19.2: Pending path + client.supportFeature = mcData.supportFeature client.settings = {} client.socket = { address: () => '127.0.0.1' } const ended = [] const errors = [] client.end = (reason) => ended.push(reason) client.on('error', (err) => errors.push(err)) - injectChat(client, { features: { signedChat: true } }, { enforceSecureProfile: true, hideErrors: true }) + injectChat(client, mcData, { enforceSecureProfile: true, hideErrors: true }) client.verifyMessage = () => true // injectChat installs the real verifier; stub it after injection return { client, ended, errors } } @@ -29,7 +30,7 @@ function seen (sender, signature) { return { messageSender: sender, messageSignature: Buffer.from(signature) } } -describe('server chat Pending lastSeen bookkeeping', () => { +describe('1.19.2v server chat Pending lastSeen bookkeeping', () => { it('validates a chain of acknowledgements across chat packets', () => { const { client, ended, errors } = makeServer() client.logSentMessageFromPeer({ senderUuid: SENDER_A, signature: Buffer.from('sig-a1'), timestamp: 1n })