From 2be79d054a4898cd5a1c4e9dd7172ccb86f7f2db Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Sat, 25 Jul 2026 01:03:46 -0700 Subject: [PATCH] docs(network): document the connection-filter seam and the UInt128 IP wart Writes up IConnectionFilter for content authors: the accept-path contract (allocation-free, non-blocking, side effects owned by the filter), registration order and short-circuiting, the unregister-on-throw policy, and why this must not be routed through EventSink.InvokeSocketConnect. Also records the IPAddress <-> UInt128 normalization quirk. Addresses are normalized to IPv6 form, so a v4 address round-tripped through UInt128 can come back as InterNetworkV6 with IsIPv4MappedToIPv6 set. That is what the seemingly redundant clause in ToUInt128 is defending, not a stray condition. Noted as a follow-up rather than churned mid-feature: the normalization would read better as an explicit "to canonical v6 bits" step that never needs the family check. Co-Authored-By: Claude Opus 4.8 --- Projects/Server/Utilities/IPAddressUtility.cs | 7 ++- dev-docs/networking-packets.md | 59 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/Projects/Server/Utilities/IPAddressUtility.cs b/Projects/Server/Utilities/IPAddressUtility.cs index 757dfd99c..ef9a390e5 100644 --- a/Projects/Server/Utilities/IPAddressUtility.cs +++ b/Projects/Server/Utilities/IPAddressUtility.cs @@ -28,7 +28,12 @@ namespace Server; /// public static class IPAddressUtility { - // Converts an IPAddress to a UInt128 in IPv6 format + // Converts an IPAddress to a UInt128 in IPv6 format. + // The IsIPv4MappedToIPv6 clause below looks redundant (the BCL only ever sets it on InterNetworkV6), + // but it guards the v4 -> UInt128 -> IPAddress round-trip, which can return a mapped v6 address for + // what is really a v4 one. + //TODO Rework as an explicit "to canonical v6 bits" step that needs no family check + // (see dev-docs/networking-packets.md, "IP Address Normalization") public static UInt128 ToUInt128(this IPAddress ip) { if (ip.AddressFamily == AddressFamily.InterNetwork && !ip.IsIPv4MappedToIPv6) diff --git a/dev-docs/networking-packets.md b/dev-docs/networking-packets.md index 43d4aec15..aae52439d 100644 --- a/dev-docs/networking-packets.md +++ b/dev-docs/networking-packets.md @@ -473,6 +473,60 @@ ns.SendMovementRej(int sequence, Mobile m); 6. **Big-endian by default** -- only use `WriteLE`/`ReadLE` when the protocol requires it 7. **Function pointers** (`&Handler`) for incoming packet registration (no delegate allocation) +## Connection Filtering (Accept Path) + +Every inbound socket is checked before it becomes a `NetState`. The check runs on the game loop once +per accepted connection -- this is the path that has to survive a DDoS -- so it must be allocation-free +and non-blocking. + +Gates plug in through `IConnectionFilter`, registered with `ConnectionFilters.Register()` during the +Configure sweep: + +```csharp +public sealed class MyFilter : IConnectionFilter +{ + public string Name => "my-filter"; + public void Configure() { /* read config, no I/O */ } + public void Start(CancellationToken token) { /* background hydration */ } + public void Stop() { } + public bool ShouldDeny(IPAddress address) => /* allocation-free membership test */; +} + +// In a static Configure() so the sweep finds it: +ConnectionFilters.Register(new MyFilter()); +``` + +Rules: + +- `ShouldDeny` must be **allocation-free**, O(log n) at worst, no I/O, no blocking. Anything expensive + (parsing, reloading, contributing to an external service) belongs off the loop or behind a bounded, + non-blocking enqueue. +- Side effects a hit implies (reporting to `BanChannel`, promoting to an OS firewall, suppressing + duplicate reports) are the **filter's** business, not the accept path's. +- Filters are consulted in registration order and the first denial short-circuits, so register the + cheapest and most selective first. Core registers before content is swept. +- A filter that throws is **unregistered** and the connection fails open. A filter that faults once + faults for every connection, so leaving it registered would mean an exception per accept. + +Built-in filters: `firewall` (core, admin-curated, mutable at runtime) and `blocklist` (UOContent, +file-sourced, millions of entries, demand-pages hits to CrowdSec). Do **not** route this kind of check +through `EventSink.InvokeSocketConnect` -- that fires later and allocates a `SocketConnectEventArgs` +per connection, which is exactly what the accept path avoids for rejected traffic. + +### IP Address Normalization (`IPAddressUtility`) + +Addresses are normalized to `UInt128` in **IPv6 form** so a single comparison/index works for both +families. An IPv4 address becomes its v4-mapped-v6 value (`::ffff:a.b.c.d`), which is why round-tripping +matters: `IPv4 -> UInt128 -> IPAddress` can come back as `InterNetworkV6` with `IsIPv4MappedToIPv6` +set, even though it is "really" a v4 address. Code that switches on `AddressFamily` alone will mis-handle +those, so the helpers check both. + +> **Known wart / follow-up:** `ToUInt128` guards with `AddressFamily == InterNetwork && !IsIPv4MappedToIPv6`. +> Per BCL semantics `IsIPv4MappedToIPv6` is only ever true for `InterNetworkV6`, so the second clause +> reads as redundant -- it is really defending the round-trip described above. The normalization would be +> clearer as an explicit "to canonical v6 bits" step that never needs the family check at all. Deliberately +> left as-is; to be revisited in a follow-up PR rather than churned mid-feature. + ## Key File References | File | Description | @@ -492,3 +546,8 @@ ns.SendMovementRej(int sequence, Mobile m); | `Projects/Server/Network/Packets/OutgoingAccountPackets.cs` | Account packets | | `Projects/Server/Network/Packets/OutgoingContainerPackets.cs` | Container packets | | `Projects/Server/Network/PacketHandler.cs` | PacketHandler class | +| `Projects/Server/Network/IConnectionFilter.cs` | Accept-path gate contract | +| `Projects/Server/Network/ConnectionFilters.cs` | Filter registry + lifecycle | +| `Projects/Server/Network/Firewall/Firewall.cs` | Admin-curated firewall set | +| `Projects/Server/Utilities/IPAddressUtility.cs` | IPAddress <-> UInt128 normalization | +| `Projects/UOContent/Misc/Blocklist/BlocklistFilter.cs` | File-sourced blocklist filter |