docs/JAMULUS_PROTOCOL.md: message reference, directory flows, small fixes

[mcfnord]

One file. Fills the main gaps in the protocol doc, verified against src/protocol.cpp / src/socket.cpp / src/serverlist.cpp on current main:

• How a receiver tells protocol packets from audio packets (they share one port; parse-or-audio in CSocket::ProcessPacket()), the CRC parameters, and how acknowledge/retransmit actually works — previously the doc said only "must be acknowledged".
• A complete message-ID reference table for both message classes, with the legacy IDs called out. This is the piece a Wireshark user or protocol reader currently has to reconstruct from protocol.h.
• A new section on directories: registration and refresh/timeout intervals, server-list responses (why both reduced and full lists are sent), and the CLM_SEND_EMPTY_MESSAGE/CLM_EMPTY_MESSAGE NAT hole-punch flow — previously undocumented here despite being a third of the connectionless messages.
• Fixes message names that drifted from protocol.h (REQ_CHANNEL_INFOS, CHANNEL_INFOS, REQ_CONN_CLIENTS_LIST).

CHANGELOG: SKIP

🤖 Generated with Claude Code

[softins]

This looks good. Just a few comments and suggested changes.

[ann0see]

Please squash the commits here.

[pljones]

Mm, leaves me asking how big an exploit surface that is -- I don't think it's that large as there aren't very many protocol messages that a malicious client could send to a server after initiating a connection that require a response that could have the ACKN unsent. But a spoofable sender address would open this.

[mcfnord]

🤖 AI: Traced it. CreateAndSendMessage() (protocol.cpp:577) is the only entry point onto the retransmit-until-ACK queue (SendMessQueue), used by 21 distinct message types — jitter-buffer size, client ID, channel gain/pan, mute state, connected-client list, channel info, chat text, network-transport props, split-message support, raw-audio support, licence-required, opus-supported, channel-level-list request, version/OS, recorder state. Audio doesn't go through this path — it's UDP fire-and-forget, separate from the ACK'd control-message queue.

SendMessQueue (protocol.h:321, a std::list) has no size cap — nothing like MAX_SIZE_MESS_QUEUE exists. There's also no dedup: EnqueueMessage() (protocol.cpp:519) unconditionally push_back()s on every call, with no check for an existing entry of the same iID. ACK handling only pops SendMessQueue.front() when its iCnt and iID match the incoming ACKN (protocol.cpp:709-719) — strict FIFO, one message retransmitted at a time regardless of queue depth.

So repeated calls to the same setter do pile up, and there's a legitimate path that does exactly that: CAudioMixerBoard::UpdateGainValue() (audiomixerboard.cpp:1858) emits ChangeChanGain on every unsuppressed fader update, unthrottled, which reaches CreateChanGainMes() via CChannel::SetRemoteChanGain() (channel.h:134). Measured directly with a minimal headless harness linking the unmodified protocol.cpp/.h and leaving MessReadyForSending unconnected, so no ACKN is ever processed: 200 calls to CreateChanGainMes() on one channel grow SendMessQueue by exactly one entry per call, zero pops, ending at depth 200; repeated at 50000 calls, still perfectly linear with no cap encountered. Combined with the unbounded retransmit window from the earlier reply, a connection whose ACKs are being dropped or spoofed accumulates one queue entry per UI action indefinitely, not just per handshake step.

[pljones]

Ah, so every connection-oriented message, regardless of whether the recipient expects it (e.g. Client should not send something to Server...) get an ACKN back anyway?

[pljones]

Anyway, it needs fixing.

[mcfnord]

🤖 AI: Yes, and it is wider than the message type. For a connected channel the acknowledgement is unconditional for anything that passes the frame and CRC check: CreateAndImmSendAcknMess sits outside the switch ( iRecIDModified ) and outside the if ( bEvaluateMessage ) guard, so no branch inside either can suppress it.

Now it is measured, against a server built from an unmodified main, over loopback. Each of these was sent to a connected channel 10 times and acknowledged all 10:

• PROTMESSID_REQ_JITT_BUF_SIZE, handled — the control
• PROTMESSID_DISCONNECTION, defined but with no case
• ID 99, not defined at all
• PROTMESSID_CLIENT_ID, which only ever travels server to client
• PROTMESSID_JITT_BUF_SIZE with a wrong-sized body
• one part of a 3-part split message, so never reassembled or evaluated

The last two are the ones worth noting: a body the evaluator rejects on size still gets acknowledged, and so does a split fragment that is never evaluated at all — the sender is told the message arrived and was accepted when nothing has read it.

Negative controls, same rig, 0/10 each: an PROTMESSID_ACKN itself, a frame with a corrupted CRC, and a connectionless ID.

One bound on it — this needs an established channel. An unconnected peer sending the same undefined ID gets 0/10, because the protocol path calls FindChannel without allowing a new channel, so there is nothing to parse into.

[pljones]

the sender is told the message arrived and was accepted when nothing has read it.

That part is explicitly intentional in the design, I believe.

The only issue here is the "forever" part, really.