Add packet capturing to the connections and make the analyzer reusable - #897
Open
sven-n wants to merge 3 commits into
Open
Add packet capturing to the connections and make the analyzer reusable#897sven-n wants to merge 3 commits into
sven-n wants to merge 3 commits into
Conversation
This is the infrastructure for the network analyzer page of the admin panel: it allows to watch the decrypted traffic of a connection which is handled by one of our servers, without an external proxy. All three server types create their connections through the same Listener, so one hook in the Connection covers the connect server, the game server and the chat server. Both directions are captured unencrypted: the incoming packets are read after the decryptor, and the Output writes into the encryptor, so it still sees plain text. * IPacketCaptureSink gets the complete, decrypted data packets of a connection. Sinks are registered with IConnection.AddCaptureSink and removed again with RemoveCaptureSink. As long as no sink is registered, a connection captures nothing - it just checks a field for null per packet and direction. * Because a write to the pipe writer isn't necessarily one data packet, the written data is buffered by the OutgoingPacketCollector and split into packets again, based on the packet header. * IConnection has an Id now, so a connection can be addressed by the admin panel and correlated in the logs. The PacketAnalyzer got three changes to be usable for more than the WinForms tool: * The client version is a parameter of the extraction methods instead of a property. One instance can therefore analyze the traffic of connections with different client versions at the same time. * The packet definitions are selected by their Direction element and a PacketDefinitionSet, instead of one file per direction. That's what allows to analyze the traffic of the connect server and the chat server, too - their definitions are included in the output now. * The definition files are only watched for changes when it's requested. That's a development feature, so only the WinForms tool uses it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pb82LmoaUVdZtBtQs7xrtA
Deploying openmudocs with
|
| Latest commit: |
7a41fa2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6aa26b81.openmudocs.pages.dev |
| Branch Preview URL: | https://claude-network-packet-captur.openmudocs.pages.dev |
Two improvements over the first version of the capture infrastructure: * The outgoing data is no longer split into packets by a hand written collector. Instead, it's copied into an own pipe, which is read by the CapturedPacketReader - a PacketPipeReaderBase, like every other packet source of the network layer. That reuses the packet splitting we already have, including the handling of malformed data: when the captured data is malformed, the reader stops and detaches itself, without affecting the connection. The data is copied when it's advanced, and flushed to the capture when the connection flushes, so the capture never blocks the connection. * The registered sinks are kept in an ImmutableArray which is updated with ImmutableInterlocked, instead of copying arrays under a lock. The lock is gone, and the array keeps the iteration in the hot path allocation free. The creation of the ExtendedPipeWriter is atomic now, so the capture is always attached to the same instance which is used by the Output. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pb82LmoaUVdZtBtQs7xrtA
The capture of a connection was kept running until it disconnected, because a capture may only start or end at a packet boundary - otherwise the captured data would begin or end in the middle of a data packet. The ExtendedPipeWriter knows where those boundaries are, so it applies a requested change itself now: a pending capture writer is applied when nothing is written to the target yet, or right after a flush. Nothing is copied anymore when nobody is watching, and the capture can be started again later. The lifecycle of the capture is guarded by a lock now. It's only held when a capture is started or stopped, which is a rare operation and never happens on the path of a data packet - the sinks themselves stay lock-free. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pb82LmoaUVdZtBtQs7xrtA
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 1 of #895, on top of #896.
This is the infrastructure for the analyzer page: watching the decrypted traffic of a connection which is already handled by one of our servers, without an external proxy. Nothing uses it yet — the capture service and the page follow in the next phases.
The capture hook
All three server types create their connections through the same
Listener, so one hook inConnectioncovers connect server, game server and chat server. Both directions are captured unencrypted: incoming packets are read after the decryptor, andOutputwrites into the encryptor, so it still sees plain text.IPacketCaptureSinkgets the complete, decrypted data packets. Sinks are registered withIConnection.AddCaptureSinkand removed withRemoveCaptureSink. They are kept in anImmutableArray, updated withImmutableInterlocked— so there is no lock, and iterating them per packet stays allocation free.CapturedPacketReader— aPacketPipeReaderBase, like every other packet source of the network layer. The data is copied when it's advanced (before the target can recycle the buffer) and flushed to the capture when the connection flushes.ExtendedPipeWriterknows where those boundaries are, so it applies a requested change itself: when nothing is written to the target yet, or right after a flush.AddCaptureSinkstarts a fresh capture.IConnectionhas anIdnow, so a connection can be addressed from the admin panel and correlated in the logs.Deviation from the plan in #895: it described a settable
CaptureSinkproperty plus a separate multiplexing sink.AddCaptureSink/RemoveCaptureSinkwith the immutable array inside the connection turned out nicer: attaching and detaching is thread-safe without bookkeeping on the caller's side.About the one remaining lock: the sinks are lock-free, but the lifecycle of the capture (starting and stopping it) is guarded by a lock. It's only held for that rare operation, never on the path of a data packet. A lock-free variant of the lifecycle would have a small window in which a capture is started and immediately torn down by a concurrent removal, so this seemed like the better trade.
The analyzer
Three changes so it can serve more than the WinForms tool:
ExtractInformation/ExtractShortInformationinstead of a property, so one instance can analyze connections with different client versions concurrently. It had to be threaded through the private extraction chain as well, becauseDetermineDynamicStructLengthuses it to decide the item data size of the extended season 6 client.Directionelement and aPacketDefinitionSet, instead of "one file per direction". Every packet in every definition file has aDirection, so this is equivalent for the game server, and it's what makes the connect server and chat server definitions usable — those files contain both directions (and oneBidirectionalpacket). Both files are copied to the output now.Tests
14 new tests for the capture (
PacketCaptureTest) covering: a received packet, a sent packet, a packet written in two chunks, two packets in one write, aC2packet with a two byte length, an incomplete trailing packet that is only reported once completed, several sinks and their removal, capturing being stopped and started again, an exception in a sink not breaking the connection, malformed data stopping the capture without breaking the connection, and — with and without a sink — that the written data still arrives at the target pipe unchanged.6 new tests for the analyzer (
PacketAnalyzerTest), guarding the refactoring: the same packet code resolving toInstantMoveRequestorObjectMoveddepending on the direction,AddNpcsToScope075vsAddNpcsToScopedepending on the client version passed per call, the connect server set resolvingServerListRequest, a game server packet not being found in the connect server set, and theBidirectionalchatChatMessagebeing found in both directions.They live in
MUnique.OpenMU.Network.Tests, which now references the analyzer library — that also confirms the packet definition xml files flow transitively into a referencing project's output. Say the word if you'd rather have a separate test project for the analyzer.Verification
dotnet build MUnique.OpenMU.sln -p:ci=true→ 0 errors; no warning originates in a file this PR adds or changes.MUnique.OpenMU.Network.Testsis at 51 passed, 4 skipped, up from 31 passed).🤖 Generated with Claude Code
https://claude.ai/code/session_01Pb82LmoaUVdZtBtQs7xrtA