feat(network): pluggable connection filters; file blocklist + contribute-first CrowdSec (#2542)
Reshapes IP banning around one idea: **core owns the question, content owns every answer.**
Core gains a single accept-path seam — `IConnectionFilter` — and loses everything that used to implement one. The firewall moves to UOContent, a new file-backed blocklist joins it there, and CrowdSec is repositioned from an in-app enforcer to a contribute-first reporter.
## The seam
```csharp
public interface IConnectionFilter
{
string Name { get; }
void Configure();
void Start(CancellationToken token);
void Stop();
bool ShouldDeny(IPAddress address);
}
```
The accept path went from hardcoded branches to one question:
```csharp
else if (ConnectionFilters.ShouldDeny(remoteIP, out var deniedBy))
{
logger.Debug("{Address} denied by connection filter '{Filter}'", remoteIP, deniedBy);
}
```
Filters register during the Configure sweep. The registry is a plain array walked by an indexed loop — no enumerator, no closure, no allocation — and the first denial short-circuits. An interface dispatch is noise next to the `accept()` syscall, so pluggability costs nothing measurable on the path that has to survive a DDoS.
Whatever a hit implies — persisting, promoting to an OS bouncer, contributing to the ban channel — is the filter's business, not the accept path's.
A filter that throws is **unregistered and the connection fails open**. A filter that faults once faults for every subsequent connection, so leaving it registered means an exception and a log line per accept — exactly the amplification an attacker wants — and a broken filter must not be able to deny everyone either.
This deliberately does **not** reuse `EventSink.InvokeSocketConnect`: that fires later and allocates a `SocketConnectEventArgs` per connection, which is what the accept path avoids for rejected traffic.
## What ships behind it
**`firewall`** (UOContent) — the existing admin-curated set. Collapsed from `Firewall` + `AdminFirewall` + a threaded enforcer into one single-threaded store with **zero concurrency primitives**: the accept path, admin gump, TTL expiry and boot load all run on the game loop. Persists to `Configuration/firewall.json` with automatic migration from the legacy `firewall.cfg`. No behavior change for operators — same namespace, same gump, same commands.
**`blocklist`** (UOContent) — new. Holds a millions-strong list in-app and **demand-pages** hits up to CrowdSec, which promotes them to the OS firewall.
The motivation is concrete: CrowdSec's Windows bouncer cannot load the ~3.9M IPs that 91 community feeds produce, but it handles ~100k fine. So the millions live in-process behind a binary search, and only addresses that *actually connect* get promoted. A `PromotedGuard` suppresses re-reporting an address until the bouncer picks it up.
The list is parsed straight from UTF-8 file bytes with no per-line string allocation, off the game loop, and published as an immutable snapshot swapped through a single `volatile` reference. Reloads yield to world saves.
**`tools/Export-IpBlocklist.ps1`** — the producer. Requires PowerShell 7 and runs on Windows, Linux and macOS; Windows PowerShell 5.1 is refused up front via `#requires`. Merges a thin, non-overlapping feed set into one de-duplicated, bogon-filtered file. Parsing runs in a compiled `Add-Type` hot loop (~1s for ~4M lines instead of minutes). Written to a `.tmp` sibling and swapped with `File.Replace`, so the shard never reads a half-written list, and a total feed outage refuses to overwrite a good list with an empty one. Re-running is idempotent — it exits without downloading anything while the list on disk is younger than `-MinInterval` (default 2h, the anchor feed's own refresh period), so a misconfigured scheduler can't hammer upstream.
## CrowdSec: contribute-first
`IBanReporter` + `BanChannel` fan locally-decided bans out to external systems. `CrowdSecReporter` (UOContent) posts to LAPI `POST /v1/alerts` and retracts via `DELETE /v1/decisions`.
Reporting is **enqueue-only** on the accept path: a bounded, coalescing channel drained off-loop with bounded retry, counted drops on overflow, and a flush on shutdown. Under a DDoS the accept path never does synchronous or lock-contending per-IP work.
### Why not pull decisions from CrowdSec?
The original design streamed decisions into an in-app snapshot and enforced them at the accept gate. That's the wrong layer: by the time the shard sees the connection, the TCP handshake and socket setup are already paid for. `cs-firewall-bouncer` drops the same traffic **at the kernel**, and it's what CrowdSec is built to do. So the shard now contributes what it uniquely knows (rate-limit trips, blocklist hits from real connection attempts) and lets the OS enforce.
The one thing the OS can't do — hold millions of entries on Windows — is exactly what the in-app blocklist covers, and it feeds the same pipeline.
## Threading policy
`CLAUDE.md` rule #3 is rewritten as an explicit three-part policy, with rule #10 restated in tandem:
- Anything touching game state runs **only** on the main loop.
- Heavy work that *needs* game state must be **chunked** across ticks, never threaded.
- Heavy work that does *not* need game state (large-file parse, external I/O) **must** run off-loop **and must yield to world saves**.
Results come back via an immutable snapshot swapped through a single `volatile` reference, or `Core.LoopContext.Post` — never by letting the scheduler decide where heavy work runs. Both new subsystems follow it.
## Shared primitives
`SortedRangeIndex<T> where T : IBinaryInteger<T>` — coalesced disjoint interval arrays plus a binary search. The firewall, the blocklist, and (as of this PR) core's reserved-network tables all use it.
Coalescing is a correctness requirement, not an optimization: multi-feed lists nest CIDRs (`/24` containing a `/32`), and a search that inspects only the rightmost run whose minimum is ≤ the value is sound **only** over disjoint runs. That bug was caught in review and is covered by regression tests.
`IPAddressUtility` collects the allocation-free `IPAddress` ↔ `UInt128` conversions and CIDR parsing that were previously scattered or duplicated.
## Config
| File | Owner | Keys |
|---|---|---|
| `Configuration/bans.json` | core | `reportRateLimitTrips`, `autoBanDuration` |
| `Configuration/blocklist.json` | content | `file`, `reloadInterval`, `reportHits`, `banDuration`, `promoteSuppression` |
| `Configuration/crowdsec.json` | content | `lapiUrl`, `machineId`, `password`, `origin`, `manualBanDuration`, `flushInterval`, `maxQueue` |
| `Configuration/firewall.json` | content | persisted firewall entries (migrated from `firewall.cfg`) |
Everything is inert by default. CrowdSec self-disables without credentials; the blocklist self-disables until its file exists. A shard that changes nothing sees no behavior change.
## Notes for review
- **Core no longer references `Firewall` or `IFirewallEntry` anywhere.** `NetworkUtilities` used to build its reserved-network tables out of `CidrFirewallEntry`, which coupled core to the firewall for something unrelated to banning; those are now a `SortedRangeIndex<UInt128>`, same semantics and public API.
- **`BanChannel.Stop()` no longer persists the firewall** — a contribution coordinator has no business saving an enforcement store. That's the firewall filter's `Stop()`.
- **A dead `whitelisted` parameter was dropped** from the blocklist gate: it was hardcoded `false` at its only call site, and no whitelist concept exists in core.
- **The blocklist filter is an instance, not a static.** The static version forced its tests onto the sequential collection with a reset hook; they now run in parallel.
- `dev-docs/networking-packets.md` documents the seam for content authors, plus a known wart in the `IPAddress` ↔ `UInt128` normalization flagged for a follow-up PR.
- The generator was verified on Linux, macOS and Windows under a temporary CI matrix (since removed). It caught two portability bugs — a Windows-only path separator, and a culture-sensitive duration parse that read `2.5` as `25` on comma-decimal locales and *silently* turned a 2.5h cooldown into 25h — plus a third that made the script unparseable on Windows PowerShell 5.1. The source is ASCII-only for that last reason: `#requires` is only honored once a file parses, so non-ASCII in a BOM-less script produces parse errors instead of the version message.
## Tests
**1344 pass** (782 `Server.Tests`, 562 `UOContent.Tests`). New coverage: filter registry (registration, short-circuit, fault-disable), blocklist parsing/CIDR/coalescing, snapshot reload markers, promote-guard TTL, ban-channel fan-out, CrowdSec alert building/dedup/flush-on-stop, and the generator's output-format contract pinned against the reader.
This commit is contained in:
Kamron Batman
GitHub
parent
bec4cfa910
commit
c39454137e
52 changed files with 4655 additions and 547 deletions
|
|
@ -473,6 +473,65 @@ 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. Order affects only how quickly a denial is reached, never whether
|
||||
one happens.
|
||||
- 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.
|
||||
|
||||
Core owns the question; **every implementation lives in UOContent**. The two that ship are `firewall`
|
||||
(admin-curated, mutable at runtime, persisted to `Configuration/firewall.json`) and `blocklist`
|
||||
(file-sourced, millions of entries, demand-pages hits to CrowdSec). A shard that fronts its server with
|
||||
an upstream proxy or edge scrubbing can drop both and register nothing.
|
||||
|
||||
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 +551,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/UOContent/Misc/Firewall/Firewall.cs` | Admin-curated firewall set |
|
||||
| `Projects/Server/Utilities/IPAddressUtility.cs` | IPAddress <-> UInt128 normalization, CIDR parsing |
|
||||
| `Projects/UOContent/Misc/Blocklist/BlocklistFilter.cs` | File-sourced blocklist filter |
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue