Skip to content

Conversation

@AvarianKnight
Copy link

The current flow for connection handling of mumble client puts a bunch of un-needed strain on the server as we have to go through the entire list of UDP sockets that aren't yet authenticated.

This instead allows us to send a partially unencrypted packet with a session_id that the calling client is trying to authenticate for, and an encrypted Audio packet that the server will use to verify that they are actually the client.

Checks

The current flow for connection handling of mumble client puts a bunch of
un-needed strain on the server as we have to go through the entire list of
UDP sockets that aren't yet authenticated.

This instead allows us to send a partially unencrypted packet with a `session_id`
that the calling client is trying to authenticate for, and an encrypted Audio
packet that the server will use to verify that they are actually the client.
@AvarianKnight AvarianKnight marked this pull request as draft December 1, 2025 05:13
@coderabbitai
Copy link

coderabbitai bot commented Dec 1, 2025

Walkthrough

This PR updates the Mumble VoIP protocol documentation and protobuf definitions to version 1.5.X. Documentation changes include renaming the voice data section to UDP Protocol, expanding the introduction to cover UDP-based audio transport, and detailing dual packet format support (Protobuf and Legacy). The protobuf schema additions include a new udp_ping_interval field in ServerConfig to configure UDP ping timing and a new Authenticate message in MumbleUDP.proto containing session identification and audio buffer fields. These changes formalize the UDP protocol handling and client authentication mechanisms.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding a UDP variant of the Authenticate message to the protocol definitions, which is the primary focus reflected across the protobuf and documentation modifications.
Description check ✅ Passed The description clearly explains the motivation (reducing server strain on unauthenticated UDP socket iteration) and the proposed solution (sending a packet with session_id and encrypted Audio for verification), meeting the core requirements.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/dev/network-protocol/voice_data.md (1)

271-290: Fix grammar error and add structured documentation for the Authenticate packet format.

Grammar issue (Line 276): "should send repeat the packet back" should be "should repeat the packet back" or "should send the packet back."

Missing documentation: The UDP connectivity checks section documents the Ping packet structure (lines 103–122 with a header/data table), but the new Authenticate packet is not similarly documented. Add a structured section describing:

  • Authenticate packet header format
  • Field layout (session_id, buffer)
  • Example or wire format

This will align with the existing pattern and clarify the packet structure for implementers.

Apply this diff to fix the grammar error:

-When the server receives the *Authenticate packet* and the server successfully
-decrypts the packets voice data it should send repeat the packet back to the client.
+When the server receives the *Authenticate packet* and successfully
+decrypts the packet's voice data, it should repeat the packet back to the client.

For the structured documentation, add a section similar to the Ping packet section:

### Authenticate packet

The Authenticate packet is used to initiate UDP connectivity checks and authenticate the client to a specific session. The server will echo back the same packet if decryption is successful.

| Field           | Type     | Description                                  |
| --------------- | -------- | -------------------------------------------- |
| `Header`        | `byte`   | Common audio packet header (TBD: specific value) |
| `Session ID`    | `varint` | Session the client is attempting to authenticate for |
| `Audio buffer`  | `Audio`  | Encrypted audio buffer for identity verification |
🧹 Nitpick comments (1)
docs/dev/network-protocol/voice_data.md (1)

1-26: Address grammar and style issues in the UDP Protocol introduction.

Two issues flagged by static analysis:

  1. Line ~12: Three successive sentences begin with "For". Reword one or two sentences to vary the structure. For example, combine lines 11-12 or restructure line 13.

  2. Line ~14: Use a hyphen to join "4-byte header" (compound adjective modifying "header").

As per static analysis hints from LanguageTool, these improve readability and adhere to style conventions.

Apply this diff to fix the grammar issues:

-For build v1.5.517 and newer you are expected use protocol buffers: [`MumbleUDP.proto`](https://github.com/mumble-voip/mumble/blob/master/src/MumbleUDP.proto).
+For build v1.5.517 and newer, protocol buffers are expected: [`MumbleUDP.proto`](https://github.com/mumble-voip/mumble/blob/master/src/MumbleUDP.proto).
-For build v1.5.516 and older you are expected use the Legacy Packet format as described below.
+For build v1.5.516 and older, use the Legacy Packet format as described below.
-For both the Protobuf packet format and the Legacy Packet format:
-- All UDP packets must be sent with a 4 byte header, even if they're unencrypted (like `Ping` and the UDP variant of `Authentication`).
+Both the Protobuf and Legacy Packet formats share these constraints:
+- All UDP packets must be sent with a 4-byte header, even if they're unencrypted (like `Ping` and the UDP variant of `Authentication`).
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 483992f and 89831de.

📒 Files selected for processing (4)
  • docs/dev/network-protocol/README.md (1 hunks)
  • docs/dev/network-protocol/voice_data.md (2 hunks)
  • src/Mumble.proto (1 hunks)
  • src/MumbleUDP.proto (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/dev/network-protocol/voice_data.md

[style] ~12-~12: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...gacy Packet format as described below. For both the Protobuf packet format and the...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[grammar] ~14-~14: Use a hyphen to join words.
Context: ... - All UDP packets must be sent with a 4 byte header, even if they're unencrypted...

(QB_NEW_EN_HYPHEN)

🔇 Additional comments (3)
src/MumbleUDP.proto (1)

87-97: Verify proto3 optional field handling in authentication flow.

The Authenticate message uses proto3, where all fields are implicitly optional by default. Both session_id and buffer appear semantically required for the authentication flow. In proto3, unset fields are not transmitted, which could lead to incomplete packets if a client fails to populate either field.

Confirm that:

  1. Clients are required to set both fields before transmission
  2. Servers validate that both fields are present before attempting decryption
  3. The implicit optionality doesn't introduce security or interoperability gaps (e.g., ambiguity between "field not set" and "field set to default value")
src/Mumble.proto (1)

606-607: Good addition to ServerConfig.

The udp_ping_interval field is correctly added with appropriate type, tag numbering, and documentation. This allows servers to configure UDP ping timing, which aligns well with the UDP connectivity updates described in the documentation changes.

docs/dev/network-protocol/README.md (1)

4-5: LGTM – Documentation version updates are clear.

The updated version references (Mumble VoIP 1.5.X and Mumble 1.5.857 client) provide clarity on the protocol state being documented. The specific client version grounds the documentation while the generic protocol version allows some flexibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant