diff --git a/Projects/UOContent.Tests/Tests/Network/AutoDenylist/AutoDenylistTests.cs b/Projects/UOContent.Tests/Tests/Network/AutoDenylist/AutoDenylistTests.cs index 3931eb3e4..ae926794d 100644 --- a/Projects/UOContent.Tests/Tests/Network/AutoDenylist/AutoDenylistTests.cs +++ b/Projects/UOContent.Tests/Tests/Network/AutoDenylist/AutoDenylistTests.cs @@ -20,8 +20,8 @@ using Xunit; namespace Server.Tests.Network.AutoDenylists; -// Static store, so every test resets it first. Addresses come from TEST-NET-2 (198.51.100.0/24). -// Sequential: the cap tests reach Sweep, which rents from STArrayPool, which is not thread-safe. +// Static store, so every test resets it first and none may run alongside another. +// Addresses come from TEST-NET-2 (198.51.100.0/24). [Collection("Sequential UOContent Tests")] public class AutoDenylistTests { @@ -62,8 +62,11 @@ public class AutoDenylistTests Assert.Equal(0, AutoDenylist.Count); } + // Not refreshed on purpose: it is what keeps insertion order equal to expiry order, so retiring lapsed + // entries costs the number expiring instead of the number held. A flooder whose hold lapses trips the + // rate limiter on its next attempt -- which runs ahead of the connection filters -- and is held again. [Fact] - public void Repeat_detection_extends_the_hold() + public void Repeat_detection_does_not_extend_the_hold() { Reset(); var ip = IPAddress.Parse("198.51.100.13"); @@ -71,8 +74,72 @@ public class AutoDenylistTests AutoDenylist.Hold(ip, BanReasons.SilentConnect, Now); AutoDenylist.Hold(ip, BanReasons.SilentConnect, Now + DurationMs - 1); - Assert.True(AutoDenylist.IsDenied(ip, Now + DurationMs + 1)); // would have lapsed without the second - Assert.Equal(1, AutoDenylist.Count); // and did not add a duplicate + Assert.Equal(1, AutoDenylist.Count); // no duplicate + Assert.True(AutoDenylist.IsDenied(ip, Now + DurationMs - 1)); + Assert.False(AutoDenylist.IsDenied(ip, Now + DurationMs + 1)); // lapses from the FIRST detection + } + + // The ring carries the expiry and the set carries membership; if they ever disagree, an address is + // either denied forever or retired early. + [Fact] + public void Ring_and_set_stay_in_step() + { + Reset(maxEntries: 4); + + for (var i = 0; i < 8; i++) + { + AutoDenylist.Hold(IPAddress.Parse($"198.51.100.{70 + i}"), BanReasons.InvalidSeed, Now); + } + + Assert.Equal(4, AutoDenylist.Count); + Assert.Equal(AutoDenylist.Count, AutoDenylist.RingCount); + + AutoDenylist.Release(IPAddress.Parse("198.51.100.71")); + Assert.Equal(3, AutoDenylist.Count); + Assert.Equal(AutoDenylist.Count, AutoDenylist.RingCount); + + AutoDenylist.Drain(Now + DurationMs + 1); + Assert.Equal(0, AutoDenylist.Count); + Assert.Equal(0, AutoDenylist.RingCount); + } + + // The ring grows in doublings but is capped at maxEntries, which is not a power of two. Filling exactly + // to it must land on the last slot rather than off the end. + [Fact] + public void Ring_fills_exactly_to_a_non_power_of_two_cap() + { + Reset(maxEntries: 100); + + for (var i = 0; i < 120; i++) + { + AutoDenylist.Hold(IPAddress.Parse($"198.51.100.{i}"), BanReasons.InvalidSeed, Now); + } + + Assert.Equal(100, AutoDenylist.Count); + Assert.Equal(100, AutoDenylist.RingCount); + + // And the whole ring still drains, so no slot was stranded by a wrapped write. + AutoDenylist.Drain(Now + DurationMs + 1); + Assert.Equal(0, AutoDenylist.Count); + Assert.Equal(0, AutoDenylist.RingCount); + } + + // Releasing leaves no ring record behind, so a re-detection is not retired by the old one. + [Fact] + public void Release_then_re_hold_is_not_retired_by_the_stale_record() + { + Reset(); + var ip = IPAddress.Parse("198.51.100.15"); + + AutoDenylist.Hold(ip, BanReasons.RateLimit, Now); + AutoDenylist.Release(ip); + + var later = Now + DurationMs - 1; + AutoDenylist.Hold(ip, BanReasons.RateLimit, later); + + // The first hold's expiry has passed; the second must survive it. + Assert.True(AutoDenylist.IsDenied(ip, Now + DurationMs + 1)); + Assert.Equal(1, AutoDenylist.RingCount); } [Fact] diff --git a/Projects/UOContent.Tests/Tests/Network/BanExemptionsTests.cs b/Projects/UOContent.Tests/Tests/Network/BanExemptionsTests.cs index 7e930fbdf..b38364373 100644 --- a/Projects/UOContent.Tests/Tests/Network/BanExemptionsTests.cs +++ b/Projects/UOContent.Tests/Tests/Network/BanExemptionsTests.cs @@ -28,15 +28,15 @@ public class BanExemptionsTests private static readonly IPAddress _listed = IPAddress.Parse("192.0.2.10"); private static readonly IPAddress _unlisted = IPAddress.Parse("192.0.2.11"); - private static void WithFileAllowlist(string contents) => - FileAllowlist.LoadForTesting(BlocklistSnapshot.Build(Encoding.ASCII.GetBytes(contents), out _, out _)); + private static void WithManualAllowlist(string contents) => + ManualAllowlist.LoadForTesting(BlocklistSnapshot.Build(Encoding.ASCII.GetBytes(contents), out _, out _)); - private static void WithEmptyFileAllowlist() => FileAllowlist.LoadForTesting(BlocklistSnapshot.Empty); + private static void WithEmptyManualAllowlist() => ManualAllowlist.LoadForTesting(BlocklistSnapshot.Empty); [Fact] - public void File_allowlist_exempts_behavioral_contributions() + public void Manual_allowlist_exempts_behavioral_contributions() { - WithFileAllowlist("192.0.2.10"); + WithManualAllowlist("192.0.2.10"); // Subtracting from the blocklist does nothing for behavioural detections, which never consult it. Assert.True(BanExemptions.IsExempt(_listed, BanReasons.ForeignProtocol, NeverCalled)); @@ -46,10 +46,10 @@ public class BanExemptionsTests } [Fact] - public void File_allowlist_covers_cidr_entries() + public void Manual_allowlist_covers_cidr_entries() { // Carve-outs are CIDRs, so a shared-CGNAT player is only covered if ranges work here. - WithFileAllowlist("192.0.2.0/24"); + WithManualAllowlist("192.0.2.0/24"); Assert.True(BanExemptions.IsExempt(_listed, BanReasons.RateLimit, NeverCalled)); Assert.True(BanExemptions.IsExempt(IPAddress.Parse("192.0.2.254"), BanReasons.RateLimit, NeverCalled)); @@ -57,9 +57,9 @@ public class BanExemptionsTests } [Fact] - public void Manual_bans_are_never_exempt_even_when_file_allowlisted() + public void Manual_bans_are_never_exempt_even_when_allowlisted() { - WithFileAllowlist("192.0.2.10"); + WithManualAllowlist("192.0.2.10"); // An explicit decision outranks the operator's own carve-out, and must not cost a strike. Assert.False(BanExemptions.IsExempt(_listed, BanReasons.Manual, NeverCalled)); @@ -68,16 +68,16 @@ public class BanExemptionsTests [Fact] public void Unopted_reasons_are_never_exempt() { - WithFileAllowlist("192.0.2.10"); + WithManualAllowlist("192.0.2.10"); Assert.False(BanExemptions.IsExempt(_listed, BanReasons.Blocklist, NeverCalled)); Assert.False(BanExemptions.IsExempt(_listed, "some-future-reason", NeverCalled)); } [Fact] - public void File_allowlist_does_not_spend_the_earned_lists_strikes() + public void Manual_allowlist_does_not_spend_the_earned_lists_strikes() { - WithFileAllowlist("192.0.2.10"); + WithManualAllowlist("192.0.2.10"); // Unconditional, so the revocable list must not be consulted -- that would burn a strike. Assert.True(BanExemptions.IsExempt(_listed, BanReasons.RateLimit, NeverCalled)); @@ -86,7 +86,7 @@ public class BanExemptionsTests [Fact] public void Falls_through_to_the_login_allowlist_when_not_file_listed() { - WithEmptyFileAllowlist(); + WithEmptyManualAllowlist(); var consulted = 0; @@ -107,7 +107,7 @@ public class BanExemptionsTests [Fact] public void Null_address_is_never_exempt() { - WithEmptyFileAllowlist(); + WithEmptyManualAllowlist(); Assert.False(BanExemptions.IsExempt(null, BanReasons.RateLimit, NeverCalled)); } diff --git a/Projects/UOContent.Tests/Tests/Network/Bans/Blocklist/BlocklistConfigurationTests.cs b/Projects/UOContent.Tests/Tests/Network/Bans/Blocklist/BlocklistConfigurationTests.cs index 6464a1bed..3d81c8d87 100644 --- a/Projects/UOContent.Tests/Tests/Network/Bans/Blocklist/BlocklistConfigurationTests.cs +++ b/Projects/UOContent.Tests/Tests/Network/Bans/Blocklist/BlocklistConfigurationTests.cs @@ -30,6 +30,7 @@ public class BlocklistConfigurationTests { var original = new BlocklistSettings { + Enabled = true, File = "D:/shared/ip-blocklist.txt", ReloadInterval = TimeSpan.FromMinutes(5), ReportHits = false, @@ -39,6 +40,7 @@ public class BlocklistConfigurationTests var json = JsonConfig.Serialize(original); + Assert.Contains("\"enabled\"", json); Assert.Contains("\"file\"", json); Assert.Contains("\"reloadInterval\"", json); Assert.Contains("\"reportHits\"", json); @@ -48,6 +50,7 @@ public class BlocklistConfigurationTests var restored = JsonSerializer.Deserialize(json, JsonConfig.DefaultOptions); Assert.NotNull(restored); + Assert.Equal(original.Enabled, restored.Enabled); Assert.Equal(original.File, restored.File); Assert.Equal(original.ReloadInterval, restored.ReloadInterval); Assert.Equal(original.ReportHits, restored.ReportHits); @@ -55,6 +58,13 @@ public class BlocklistConfigurationTests Assert.Equal(original.PromoteSuppression, restored.PromoteSuppression); } + // The point of the flag: a shard that never opts in must not start the reload poll. + [Fact] + public void Blocklist_is_off_by_default() + { + Assert.False(new BlocklistSettings().Enabled); + } + // The generator (tools/Export-IpBlocklist.ps1) writes to this path by default; if one side moves // without the other, a shard silently enforces nothing. [Fact] diff --git a/Projects/UOContent.Tests/Tests/Network/ManualAllowlist/ManualAllowlistConfigurationTests.cs b/Projects/UOContent.Tests/Tests/Network/ManualAllowlist/ManualAllowlistConfigurationTests.cs new file mode 100644 index 000000000..1146414b5 --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Network/ManualAllowlist/ManualAllowlistConfigurationTests.cs @@ -0,0 +1,66 @@ +/************************************************************************* + * ModernUO * + * Copyright 2019-2026 - ModernUO Development Team * + * Email: hi@modernuo.com * + * File: ManualAllowlistConfigurationTests.cs * + * * + * This program is free software: you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation, either version 3 of the License, or * + * (at your option) any later version. * + * * + * You should have received a copy of the GNU General Public License * + * along with this program. If not, see . * + *************************************************************************/ + +using System; +using System.Text.Json; +using Server.Json; +using Server.Network.Bans; +using Xunit; + +namespace Server.Tests.Network.ManualAllowlists; + +public class ManualAllowlistConfigurationTests +{ + // Locks the JsonConfig casing contract: JsonConfig's options are case-SENSITIVE, so every settings + // member must carry an explicit [JsonPropertyName("camelCase")] or it silently binds nothing. + [Fact] + public void ManualAllowlistSettings_RoundTripsThroughJsonConfig() + { + var original = new ManualAllowlistSettings + { + Enabled = true, + Files = ["D:/shared/ip-allowlist*.txt"], + ReloadInterval = TimeSpan.FromMinutes(5) + }; + + var json = JsonConfig.Serialize(original); + + Assert.Contains("\"enabled\"", json); + Assert.Contains("\"files\"", json); + Assert.Contains("\"reloadInterval\"", json); + + var restored = JsonSerializer.Deserialize(json, JsonConfig.DefaultOptions); + + Assert.NotNull(restored); + Assert.Equal(original.Enabled, restored.Enabled); + Assert.Equal(original.Files, restored.Files); + Assert.Equal(original.ReloadInterval, restored.ReloadInterval); + } + + // The point of the flag: a shard that never opts in must not start the reload poll. + [Fact] + public void Manual_allowlist_is_off_by_default() + { + Assert.False(new ManualAllowlistSettings().Enabled); + } + + // The generator creates ip-allowlist.txt beside the blocklist; the wildcard is what picks up a + // carve-out file (-RefreshCarveouts writes ip-allowlist-starlink.txt) with no config edit. + [Fact] + public void Default_pattern_matches_the_generator_output_path() + { + Assert.Equal(["Configuration/ip-allowlist*.txt"], new ManualAllowlistSettings().Files); + } +} diff --git a/Projects/UOContent/Network/AutoDenylist/AutoDenylist.cs b/Projects/UOContent/Network/AutoDenylist/AutoDenylist.cs index b9b4dd7c1..7b3c44ae4 100644 --- a/Projects/UOContent/Network/AutoDenylist/AutoDenylist.cs +++ b/Projects/UOContent/Network/AutoDenylist/AutoDenylist.cs @@ -32,12 +32,26 @@ namespace Server.Network; /// no bouncer, which is the default. Not persisted, by design: a holding pen that survives restarts is a ban /// without a ban's review. Only verdicts are held. /// +/// +/// A hold is never refreshed, so every expiry is insertion + duration and the ring is sorted by +/// construction. Retiring lapsed entries is therefore the number expiring rather than the number held, which +/// is what lets the cap be sized for the flood instead of for a scan. +/// public static class AutoDenylist { private static readonly ILogger logger = LogFactory.GetLogger(typeof(AutoDenylist)); - // Address (normalized v6 bits) -> Core.TickCount at which the hold lapses. Loop-only. - private static readonly Dictionary _held = []; + // Membership only (normalized v6 bits). Loop-only. The expiry lives beside the key in the ring, so + // there is exactly one copy of it and the two cannot disagree. + private static readonly HashSet _held = []; + + // The same keys in expiry order. Parallel arrays rather than an array of structs: UInt128 forces + // 16-byte alignment, so a packed (key, expiry) struct costs 32 bytes where these cost 24 -- and the + // drain reads only the long[], 8 sequential bytes per entry. + private static UInt128[] _ringKeys = []; + private static long[] _ringExpiry = []; + private static int _ringHead; + private static int _ringCount; private static bool _enabled; private static long _durationMs; @@ -46,6 +60,9 @@ public static class AutoDenylist public static int Count => _held.Count; + // Test seam: the ring and the set hold the same entries, and nothing else may assume it. + internal static int RingCount => _ringCount; + public static void Configure() { AutoDenylistConfiguration.Load(); @@ -77,35 +94,47 @@ public static class AutoDenylist return false; } + Drain(nowTicks); + var key = address.ToUInt128(); - // An address already held is just extended, so no cap check is needed. - if (!_held.ContainsKey(key) && _held.Count >= _maxEntries) + // Deliberately not refreshed: the first detection sets the expiry and later ones leave it alone. + // That keeps insertion order equal to expiry order, which is why the drain can stop at the first + // live record. A flooder whose hold lapses trips the rate limiter on its next attempt -- which + // runs ahead of the connection filters -- and is held again. + if (!_held.Add(key)) { - Sweep(nowTicks); - - if (_held.Count >= _maxEntries) - { - if (!_warnedFull) - { - _warnedFull = true; - logger.Warning( - "Auto-denylist is full at {Max} addresses; further detections are disconnected but not held", - _maxEntries - ); - } - - return false; - } + return true; } - _held[key] = nowTicks + _durationMs; + // Drain already reclaimed everything reclaimable, so being over now means genuinely full. + if (_held.Count > _maxEntries) + { + _held.Remove(key); + + if (!_warnedFull) + { + _warnedFull = true; + logger.Warning( + "Auto-denylist is full at {Max} addresses; further detections are disconnected but not held", + _maxEntries + ); + } + + return false; + } + + Push(key, nowTicks + _durationMs); return true; } public static bool IsDenied(IPAddress address) => IsDenied(address, Core.TickCount); - /// The pure decision, split out so the accept-path policy can be tested without a clock. + /// + /// The accept-path decision, split out so the policy can be tested without a clock. Drains first: the + /// expiry lives in the ring, not beside the membership, so a lapsed hold has to be retired here rather + /// than expired on read. One array read when nothing has lapsed. + /// internal static bool IsDenied(IPAddress address, long nowTicks) { if (!_enabled || address == null) @@ -113,46 +142,136 @@ public static class AutoDenylist return false; } - // Decided on read, so a lapsed hold cannot deny even before the sweep. Subtraction: TickCount wraps. - return _held.TryGetValue(address.ToUInt128(), out var expires) && expires - nowTicks > 0; + Drain(nowTicks); + return _held.Contains(address.ToUInt128()); } /// Releases an address early, e.g. when an operator retracts a ban. public static void Release(IPAddress address) { - if (_enabled && address != null) - { - _held.Remove(address.ToUInt128()); - } - } - - internal static void Sweep(long nowTicks) - { - if (_held.Count == 0) + if (!_enabled || address == null) { return; } - var lapsed = 0; - - foreach (var (address, expires) in _held) + var key = address.ToUInt128(); + if (_held.Remove(key)) { - if (expires - nowTicks <= 0) - { - _held.Remove(address); - lapsed++; - } + // The ring record has to go too. Nothing records that this key was released, so if it were + // detected again before the old record lapsed, that record would retire the new hold early. + // O(n), but this is an operator retraction, not the accept path. + PurgeRing(key); + } + } + + /// + /// Retires everything that has lapsed. Expiries only ever increase along the ring, so the first live + /// record ends the scan and the cost is the number actually expiring, not the number held. + /// + internal static void Drain(long nowTicks) + { + var before = _ringCount; + + // Subtraction, never a direct compare: tick counts wrap. See dev-docs/tick-counts.md. + while (_ringCount > 0 && _ringExpiry[_ringHead] - nowTicks <= 0) + { + _held.Remove(_ringKeys[_ringHead]); + _ringHead = _ringHead + 1 == _ringKeys.Length ? 0 : _ringHead + 1; + _ringCount--; } - if (lapsed > 0) + if (_ringCount != before) { _warnedFull = false; } } + private static void Push(UInt128 key, long expiry) + { + if (_ringCount == _ringKeys.Length) + { + Grow(); + } + + var tail = _ringHead + _ringCount; + if (tail >= _ringKeys.Length) + { + tail -= _ringKeys.Length; + } + + _ringKeys[tail] = key; + _ringExpiry[tail] = expiry; + _ringCount++; + } + + private static void Grow() + { + // Capped at the entry cap: Push only runs below it, so the ring never needs more, and doubling + // past it would reserve roughly twice the slots it can ever use. + var size = Math.Min(Math.Max(64, _ringKeys.Length * 2), _maxEntries); + var keys = new UInt128[size]; + var expiry = new long[size]; + + for (var i = 0; i < _ringCount; i++) + { + var from = _ringHead + i; + if (from >= _ringKeys.Length) + { + from -= _ringKeys.Length; + } + + keys[i] = _ringKeys[from]; + expiry[i] = _ringExpiry[from]; + } + + _ringKeys = keys; + _ringExpiry = expiry; + _ringHead = 0; + } + + private static void PurgeRing(UInt128 key) + { + var capacity = _ringKeys.Length; + + for (var i = 0; i < _ringCount; i++) + { + var at = _ringHead + i; + if (at >= capacity) + { + at -= capacity; + } + + if (_ringKeys[at] != key) + { + continue; + } + + // Close the gap so the ring stays contiguous and expiry-ordered. + for (var j = i; j < _ringCount - 1; j++) + { + var to = _ringHead + j; + if (to >= capacity) + { + to -= capacity; + } + + var from = to + 1 == capacity ? 0 : to + 1; + _ringKeys[to] = _ringKeys[from]; + _ringExpiry[to] = _ringExpiry[from]; + } + + _ringCount--; + return; + } + } + internal static void LoadForTesting(bool enabled, long durationMs, int maxEntries) { _held.Clear(); + _ringKeys = []; + _ringExpiry = []; + _ringHead = 0; + _ringCount = 0; _enabled = enabled; _durationMs = durationMs; _maxEntries = maxEntries; @@ -163,6 +282,8 @@ public static class AutoDenylist /// Accept-path gate for . public sealed class AutoDenylistFilter : IConnectionFilter { + private Timer _sweepTimer; + public string Name => "auto-denylist"; public void Register() @@ -171,12 +292,20 @@ public sealed class AutoDenylistFilter : IConnectionFilter public void Start(CancellationToken token) { - // Only an optimisation: IsDenied expires on read. - Timer.DelayCall(TimeSpan.FromMinutes(1), TimeSpan.FromMinutes(1), () => AutoDenylist.Sweep(Core.TickCount)); + // Only reclaims memory: Hold and IsDenied both drain, so this matters on a shard that has gone + // quiet after a flood and would otherwise hold the ring until someone next connects. + _sweepTimer = Timer.DelayCall( + TimeSpan.FromMinutes(1), + TimeSpan.FromMinutes(1), + () => AutoDenylist.Drain(Core.TickCount) + ); } public void Stop() { + // Recurring, so an uncancelled sweep survives Stop and the next Start adds a second one. + _sweepTimer?.Stop(); + _sweepTimer = null; } public bool ShouldDeny(IPAddress address) => AutoDenylist.IsDenied(address); diff --git a/Projects/UOContent/Network/AutoDenylist/AutoDenylistConfiguration.cs b/Projects/UOContent/Network/AutoDenylist/AutoDenylistConfiguration.cs index 4d45da350..fd13ef003 100644 --- a/Projects/UOContent/Network/AutoDenylist/AutoDenylistConfiguration.cs +++ b/Projects/UOContent/Network/AutoDenylist/AutoDenylistConfiguration.cs @@ -21,9 +21,8 @@ using Server.Json; namespace Server.Network; /// -/// Loads the from Configuration/auto-denylist.json (matching the -/// per-feature JSON config pattern used by BlocklistConfiguration). Loaded once; a missing file writes -/// a template so operators have something to edit. +/// Loads the from Configuration/auto-denylist.json. Loaded once; +/// a missing file writes a template so operators have something to edit. /// public static class AutoDenylistConfiguration { @@ -76,6 +75,14 @@ public record AutoDenylistSettings /// stops it becoming the exhaustion it prevents. At the cap new addresses are not tracked, but are still /// disconnected by whichever gate detected them. /// + /// + /// A HashSet capacity, not a round number. Grown from empty it steps 36,353 → 75,431 → 156,437 + /// → 324,449, so this fills one exactly instead of stranding slots: 65,536 sat just past a resize and + /// left 9,895 of them unusable. Sized to cover the 50k–250k distinct-source floods seen in practice, + /// for ~19 MB — 36 bytes a set slot plus 24 for the ring record. Raising it is bounded by memory + /// rather than by a scan, since holds are retired from the ring in expiry order; a flood past it wants + /// upstream scrubbing rather than a larger cap. + /// [JsonPropertyName("maxEntries")] - public int MaxEntries { get; set; } = 65536; + public int MaxEntries { get; set; } = 324_449; } diff --git a/Projects/UOContent/Network/BanExemptions.cs b/Projects/UOContent/Network/BanExemptions.cs index 5566bd5e7..c5f6b726a 100644 --- a/Projects/UOContent/Network/BanExemptions.cs +++ b/Projects/UOContent/Network/BanExemptions.cs @@ -20,7 +20,7 @@ using Server.Network.Bans; namespace Server.Network; /// -/// Combines and into the one answer +/// Combines and into the one answer /// asks for, so neither source has to know about the other. /// public static class BanExemptions @@ -51,7 +51,7 @@ public static class BanExemptions } // Deliberate and unconditional, so it wins and must not spend the earned list's strikes. - if (FileAllowlist.Contains(address)) + if (ManualAllowlist.Contains(address)) { return true; } diff --git a/Projects/UOContent/Network/Blocklist/BlocklistConfiguration.cs b/Projects/UOContent/Network/Blocklist/BlocklistConfiguration.cs index 8a2df5ca6..19541636f 100644 --- a/Projects/UOContent/Network/Blocklist/BlocklistConfiguration.cs +++ b/Projects/UOContent/Network/Blocklist/BlocklistConfiguration.cs @@ -21,9 +21,8 @@ using Server.Json; namespace Server.Network.Bans; /// -/// Loads the from Configuration/blocklist.json (matching the -/// per-feature JSON config pattern used by AssistantConfiguration). Loaded once; a missing file -/// writes a template so operators have something to edit. +/// Loads the from Configuration/blocklist.json. Loaded once; a +/// missing file writes a template so operators have something to edit. /// public static class BlocklistConfiguration { @@ -53,12 +52,19 @@ public static class BlocklistConfiguration } /// -/// Bound configuration for . The filter is inert unless -/// points at a list that actually exists, so the shipped defaults are safe on a shard that never runs -/// the generator. +/// Bound configuration for . The filter is inert unless +/// is set and points at a list that exists, so a shard that never runs the generator +/// pays nothing for the defaults. /// public record BlocklistSettings { + /// + /// Whether the accept-path gate runs at all. Off by default: the reload poll runs for the whole + /// uptime, which no shard should pay before an operator has chosen to run a blocklist. + /// + [JsonPropertyName("enabled")] + public bool Enabled { get; set; } + /// /// Path to the blocklist. A relative path resolves against ; an /// absolute path is used as-is (handy when several shards share one generated list). Set to @@ -67,19 +73,6 @@ public record BlocklistSettings [JsonPropertyName("file")] public string File { get; set; } = "Configuration/ip-blocklist.txt"; - /// - /// Addresses that must never be blocked and never escalated, in the blocklist's own format. The same - /// files tools/Export-IpBlocklist.ps1 subtracts at generation time; the shard reads them so an - /// entry also suppresses ban contributions, which the generator alone cannot do. See - /// . - /// - /// - /// The filename may contain wildcards, which is how the default picks up a carve-out an admin adds - /// without anyone editing this file. - /// - [JsonPropertyName("allowlistFiles")] - public string[] AllowlistFiles { get; set; } = ["Configuration/ip-allowlist*.txt"]; - /// How often the file is checked for changes. Reloads only happen when it actually changed. [JsonPropertyName("reloadInterval")] public TimeSpan ReloadInterval { get; set; } = TimeSpan.FromSeconds(60); diff --git a/Projects/UOContent/Network/Blocklist/BlocklistFilter.cs b/Projects/UOContent/Network/Blocklist/BlocklistFilter.cs index 36354160b..030997803 100644 --- a/Projects/UOContent/Network/Blocklist/BlocklistFilter.cs +++ b/Projects/UOContent/Network/Blocklist/BlocklistFilter.cs @@ -25,8 +25,8 @@ namespace Server.Network.Bans; /// /// Accept-path gate for a large, file-sourced IP blocklist, hydrated from the file a generator /// (tools/Export-IpBlocklist.ps1) writes on a schedule. Holds an immutable snapshot swapped -/// atomically by an off-loop reload poll, so accept-path reads are lock-free. Inert when no file is -/// configured or present. +/// atomically by an off-loop reload poll, so accept-path reads are lock-free. Opt-in via +/// blocklist.json's enabled; inert when off, or when no file is configured or present. /// /// /// This is the demand-paging half of the design: an OS firewall cannot hold millions of entries on @@ -38,12 +38,13 @@ public sealed class BlocklistFilter : IConnectionFilter { private static readonly ILogger logger = LogFactory.GetLogger(typeof(BlocklistFilter)); - // Written by the reload poll (off-loop), read by the accept path (game loop): a single volatile - // reference swap is the whole synchronization story — readers see the old or the new snapshot, whole. + // Written by the reload poll (off-loop), read by the accept path (game loop). One volatile reference + // swap is the whole synchronization story: readers see the old or the new snapshot, whole. private volatile BlocklistSnapshot _snapshot = BlocklistSnapshot.Empty; private readonly PromotedGuard _guard = new(); + private bool _enabled; private string _path; private TimeSpan _interval; private bool _reportHits; @@ -52,6 +53,7 @@ public sealed class BlocklistFilter : IConnectionFilter private string _lastGenerated; private DateTime _lastWriteUtc; private CancellationTokenSource _cts; + private Timer _sweepTimer; public string Name => "blocklist"; @@ -68,6 +70,7 @@ public sealed class BlocklistFilter : IConnectionFilter var s = BlocklistConfiguration.Settings; _path = ResolvePath(s.File); + _enabled = s.Enabled && _path != null; _interval = s.ReloadInterval <= TimeSpan.Zero ? TimeSpan.FromSeconds(60) : s.ReloadInterval; _reportHits = s.ReportHits; _banDuration = s.BanDuration; @@ -91,16 +94,26 @@ public sealed class BlocklistFilter : IConnectionFilter public void Start(CancellationToken token) { - if (_path == null) + if (!_enabled) { - logger.Information("Blocklist disabled (\"file\" empty in blocklist.json)"); + LogWhyDisabled(); return; } + // The operator's override on this gate, opted into separately. Without it only the generator's + // subtraction covers carve-outs, and that does not cover ban contributions. + if (!ManualAllowlist.Enabled) + { + logger.Warning( + "Blocklist is on but the manual allowlist is not; set \"enabled\" in ip-allowlist.json so a " + + "carve-out also suppresses ban contributions" + ); + } + _cts = CancellationTokenSource.CreateLinkedTokenSource(token); - // A missing file is the shipped default, not an error: the gate stays inert until the poll picks - // up whatever the generator first writes. No restart needed. + // A missing file is not an error: the gate stays inert until the poll picks up whatever the + // generator first writes. No restart needed. if (File.Exists(_path)) { Reload(); // synchronous prime; empty on failure (fail-open) @@ -110,17 +123,47 @@ public sealed class BlocklistFilter : IConnectionFilter logger.Information("Blocklist inert: no list at \"{Path}\"; polling every {Interval}", _path, _interval); } - // Sweep the promote-guard so a distinct-IP flood cannot grow it unbounded. - Timer.DelayCall(TimeSpan.FromMinutes(1), TimeSpan.FromMinutes(1), SweepGuard); + // Sweep the promote-guard so a distinct-IP flood cannot grow it unbounded. Only marked when hits + // are reported, so there is nothing to sweep otherwise. + if (_reportHits) + { + _sweepTimer = Timer.DelayCall(TimeSpan.FromMinutes(1), TimeSpan.FromMinutes(1), SweepGuard); + } _ = Task.Run(() => PollLoop(_cts.Token), _cts.Token); } + private void LogWhyDisabled() + { + if (_path == null) + { + logger.Information("Blocklist disabled (\"file\" empty in blocklist.json)"); + } + else if (File.Exists(_path)) + { + // An upgraded shard has a list on disk but no "enabled" key, so say so rather than silently + // dropping a gate it was relying on. + logger.Warning( + "Blocklist is off (\"enabled\" false in blocklist.json) but a list is present at \"{Path}\"; " + + "no addresses will be denied", + _path + ); + } + else + { + logger.Information("Blocklist disabled (\"enabled\" false in blocklist.json)"); + } + } + public void Stop() { _cts?.Cancel(); _cts?.Dispose(); _cts = null; + + // Recurring, so an uncancelled sweep survives Stop and the next Start adds a second one. + _sweepTimer?.Stop(); + _sweepTimer = null; } public bool ShouldDeny(IPAddress address) @@ -153,9 +196,9 @@ public sealed class BlocklistFilter : IConnectionFilter } // Both are asked only once the list has matched, so they cost the common accept nothing. The file - // list is usually redundant because the generator subtracts it — except right after an operator adds - // an entry without regenerating, which is exactly when someone is waiting to get back in. - if (FileAllowlist.Contains(address)) + // list is usually redundant because the generator subtracts it — except right after an operator + // adds an entry without regenerating, which is when someone is waiting to get back in. + if (ManualAllowlist.Contains(address)) { return false; } @@ -248,9 +291,8 @@ public sealed class BlocklistFilter : IConnectionFilter private void Reload() { - // Capture the mtime/header BEFORE Load() so the markers describe the version being parsed, not - // one the producer swapped in mid-parse. Stale markers only cost an extra reload next poll; - // capturing after could skip a version entirely. + // Capture the mtime/header BEFORE Load() so they describe the version being parsed. Capturing + // after could skip a version the producer swapped in mid-parse; stale markers only cost a reload. var writeUtc = default(DateTime); try { diff --git a/Projects/UOContent/Network/Blocklist/BlocklistSnapshot.cs b/Projects/UOContent/Network/Blocklist/BlocklistSnapshot.cs index 29b2e5176..d62df0752 100644 --- a/Projects/UOContent/Network/Blocklist/BlocklistSnapshot.cs +++ b/Projects/UOContent/Network/Blocklist/BlocklistSnapshot.cs @@ -46,8 +46,7 @@ public sealed class BlocklistSnapshot /// Parses a blocklist directly from its UTF-8/ASCII file bytes — one line at a time, splitting on /// '\n' with no per-line string allocation. IPv4 singles and CIDRs are parsed straight from the /// byte span; IPv6 (the rare path) decodes the single address token and defers to the framework parser. - /// Malformed lines increment and never throw. Build-time intermediates use - /// the multithreaded pool because this runs off the game loop on the reload/bootstrap thread. + /// Malformed lines increment and never throw. /// public static BlocklistSnapshot Build(ReadOnlySpan data, out int parsed, out int skipped) { @@ -183,7 +182,7 @@ public sealed class BlocklistSnapshot } /// - /// Plain set membership, for callers whose set is an ALLOWlist (see ) and for + /// Plain set membership, for callers whose set is an ALLOWlist (see ) and for /// whom would read backwards. The interval machinery is direction-agnostic. /// public bool Contains(IPAddress ip) => IsBanned(ip); diff --git a/Projects/UOContent/Network/CrowdSec/CrowdSecConfiguration.cs b/Projects/UOContent/Network/CrowdSec/CrowdSecConfiguration.cs index b1376b6a0..5855633f9 100644 --- a/Projects/UOContent/Network/CrowdSec/CrowdSecConfiguration.cs +++ b/Projects/UOContent/Network/CrowdSec/CrowdSecConfiguration.cs @@ -21,9 +21,8 @@ using Server.Json; namespace Server.Network.Bans.CrowdSec; /// -/// Loads the from Configuration/crowdsec.json (matching the -/// per-feature JSON config pattern used by AssistantConfiguration). Loaded once; a missing file -/// writes a disabled-by-default template so operators have something to edit. +/// Loads the from Configuration/crowdsec.json. Loaded once; a +/// missing file writes a disabled-by-default template so operators have something to edit. /// public static class CrowdSecConfiguration { diff --git a/Projects/UOContent/Network/CrowdSec/CrowdSecReporter.cs b/Projects/UOContent/Network/CrowdSec/CrowdSecReporter.cs index 385534d2a..c900eb90f 100644 --- a/Projects/UOContent/Network/CrowdSec/CrowdSecReporter.cs +++ b/Projects/UOContent/Network/CrowdSec/CrowdSecReporter.cs @@ -304,12 +304,10 @@ public sealed class CrowdSecReporter : IBanReporter } /// - /// Sends with up to 3 attempts total (1 initial + 2 retries), backing off 1s then 2s between - /// attempts, for transient LAPI failures (network blips, 5xx). Backoff uses - /// so it never blocks the thread; a cancellation during backoff propagates as - /// so the drain loop exits cleanly. Returns false (never - /// throws for a send failure) once attempts are exhausted, so the caller can count the drop and keep - /// draining instead of losing the rest of the batch/queue. + /// Up to 3 attempts (1 initial + 2 retries) backing off 1s then 2s, for transient LAPI failures + /// (network blips, 5xx). Backoff uses so it never blocks the thread, and a + /// cancellation during it propagates so the drain loop exits cleanly. Returns false rather than + /// throwing once attempts are exhausted, so the caller counts the drop and keeps draining. /// private static async ValueTask SendWithBoundedRetryAsync(Func send, CancellationToken token) { diff --git a/Projects/UOContent/Network/Firewall/Firewall.cs b/Projects/UOContent/Network/Firewall/Firewall.cs index 075b47ec3..36c11bbf9 100644 --- a/Projects/UOContent/Network/Firewall/Firewall.cs +++ b/Projects/UOContent/Network/Firewall/Firewall.cs @@ -28,10 +28,10 @@ namespace Server.Network; public static class Firewall { // Single-threaded: the accept path, admin gump/command, TTL expiry timer, and boot load all run on - // the main game loop. No locks, caches, or version counters are needed. See the ban-channel design doc. - // _entries is the authoritative store (gump/persistence/TTL/command all work against it); _index is a - // derived, rebuild-on-demand SortedRangeIndex used only for the accept-path IsBlocked lookup, shared - // with the same sorted-range binary-search primitive the blocklist uses (see BlocklistSnapshot). + // the main game loop, so no locks, caches, or version counters are needed. _entries is the + // authoritative store; _index is a derived, rebuild-on-demand SortedRangeIndex used only for the + // accept-path IsBlocked lookup, over the same primitive the blocklist uses (see BlocklistSnapshot). + // See dev-docs/ip-bans-and-allowlists.md. private static readonly List _entries = []; // Entries with a TTL: entry -> absolute expiry tick (Core.TickCount). Permanent entries are absent. @@ -152,7 +152,7 @@ public static class Firewall } /// - /// Removes every entry whose TTL has elapsed. Called from the main-thread maintenance timer (Task 2). + /// Removes every entry whose TTL has elapsed. Called from the main-thread maintenance timer. /// internal static void ExpireEntries(long nowTicks) { diff --git a/Projects/UOContent/Network/LoginAllowlist/LoginAllowlist.cs b/Projects/UOContent/Network/LoginAllowlist/LoginAllowlist.cs index c44ceb33d..23075f7ff 100644 --- a/Projects/UOContent/Network/LoginAllowlist/LoginAllowlist.cs +++ b/Projects/UOContent/Network/LoginAllowlist/LoginAllowlist.cs @@ -19,6 +19,7 @@ using System.Globalization; using System.IO; using System.Net; using System.Text; +using System.Threading; using System.Threading.Tasks; using Server.Logging; using Server.Network.Bans; @@ -30,16 +31,11 @@ namespace Server.Network; /// blocked and a flaky connection cannot get one globally banned. /// /// -/// -/// Consulted only after the blocklist has already matched, and again before a ban is contributed, so a -/// normal accept pays nothing for it. An entry is evidence rather than a licence: enough strikes inside the -/// window revokes it. It cannot bootstrap, so it hedges stable addresses and does not replace -/// . See dev-docs/ip-bans-and-allowlists.md. -/// -/// -/// Both dictionaries are game-loop state. Only the file write runs off-loop, over a snapshot taken on the -/// loop. -/// +/// Consulted only after the blocklist has already matched, so a normal accept pays nothing for it. An entry +/// is evidence rather than a licence: enough strikes inside the window revokes it. It cannot bootstrap, so +/// it does not replace . Both dictionaries are game-loop state; only the file +/// write runs off-loop, over a snapshot taken on the loop. +/// See dev-docs/ip-bans-and-allowlists.md. /// public static class LoginAllowlist { @@ -52,6 +48,11 @@ public static class LoginAllowlist // _allowed and cannot be grown by an attacker. private static readonly Dictionary _strikes = []; + // Reused: past ~5,300 entries a fresh UInt128[] is an LOH allocation, once per flush. Grown + // geometrically, never shrunk. + private static UInt128[] _addressBuffer = []; + private static long[] _stampBuffer = []; + private static bool _enabled; private static string _path; private static long _ttlSeconds; @@ -59,6 +60,10 @@ public static class LoginAllowlist private static long _strikeWindowSeconds; private static bool _dirty; + // Loop-only. The writer owns the buffers until it posts completion back, so a flush landing mid-write + // waits rather than overwriting them. + private static bool _writing; + public static int Count => _allowed.Count; private struct Strike @@ -97,10 +102,14 @@ public static class LoginAllowlist var interval = LoginAllowlistConfiguration.Settings.FlushInterval; if (interval <= TimeSpan.Zero) { - interval = TimeSpan.FromMinutes(1); + interval = TimeSpan.FromHours(1); } Timer.DelayCall(interval, interval, Flush); + + // HandleClosed skips InvokeShutdown when the server crashed, so the crash path needs its own. + EventSink.Shutdown += OnShutdown; + EventSink.ServerCrashed += OnCrashed; } /// @@ -197,6 +206,8 @@ public static class LoginAllowlist { _allowed.Clear(); _strikes.Clear(); + _writing = false; + _dirty = false; _enabled = enabled; _ttlSeconds = ttlSeconds; _escalateAfterStrikes = escalateAfterStrikes; @@ -208,28 +219,87 @@ public static class LoginAllowlist private static void Flush() { - if (!_enabled || !_dirty) - { - return; - } - // A save owns the disk and nothing here is urgent. _dirty stays set, so skipping loses nothing. // See the threading policy in CLAUDE.md (rules #3 and #10). - if (World.Saving || World.WorldState == WorldState.PendingSave) + if (!_enabled || !_dirty || _writing || World.Saving || World.WorldState == WorldState.PendingSave) { return; } + var count = Snapshot(out var dropped); + var addresses = _addressBuffer; + var stamps = _stampBuffer; + var path = _path; + + _dirty = false; + _writing = true; + + _ = Task.Run( + () => + { + var written = Write(path, addresses, stamps, count, dropped); + + // _writing and _dirty are loop state, so the writer hands the release back. Rule #10. + Core.LoopContext.Post( + () => + { + _writing = false; + if (!written) + { + _dirty = true; // nothing reached disk; the next flush retries + } + } + ); + } + ); + } + + /// + /// A crash is the case the flush interval cannot cover, so write on the way down. Runs on whichever + /// thread faulted, and the dictionaries are loop state, so it only writes when that is the loop. + /// + private static void OnCrashed(ServerCrashedEventArgs e) + { + if (Thread.CurrentThread == Core.Thread) + { + OnShutdown(); + } + } + + /// Synchronous: nothing schedules after this, so a handed-off write would reach no disk. + private static void OnShutdown() + { + // A write already in flight holds the buffers and has all but the last moments of the list. + if (!_enabled || !_dirty || _writing) + { + return; + } + + var count = Snapshot(out var dropped); + _dirty = false; + + Write(_path, _addressBuffer, _stampBuffer, count, dropped); + } + + /// + /// Prunes expired entries and copies what survives into the shared buffers. Returns the live count; the + /// buffers run longer and everything past it is stale. + /// + private static int Snapshot(out int dropped) + { var nowUnix = ToUnixSeconds(Core.Now); var cutoff = nowUnix - _ttlSeconds; - // Prune and snapshot in one loop-side pass; the writer only sees private copies. Not pooled: - // STArrayPool is single-threaded and these escape to another thread. - var addresses = new UInt128[_allowed.Count]; - var stamps = new long[_allowed.Count]; - var count = 0; + if (_addressBuffer.Length < _allowed.Count) + { + // Geometric so a shard adding addresses one at a time does not reallocate every flush. + var size = Math.Max(_allowed.Count, Math.Max(64, _addressBuffer.Length * 2)); + _addressBuffer = new UInt128[size]; + _stampBuffer = new long[size]; + } - var dropped = 0; + var count = 0; + dropped = 0; foreach (var (address, stamp) in _allowed) { @@ -241,19 +311,13 @@ public static class LoginAllowlist continue; } - addresses[count] = address; - stamps[count] = stamp; + _addressBuffer[count] = address; + _stampBuffer[count] = stamp; count++; } PruneStaleStrikes(nowUnix); - - _dirty = false; - - var path = _path; - var total = count; - - _ = Task.Run(() => Write(path, addresses, stamps, total, dropped)); + return count; } /// Drops tallies whose window has closed. @@ -273,7 +337,8 @@ public static class LoginAllowlist } } - private static void Write(string path, UInt128[] addresses, long[] stamps, int count, int dropped) + /// Writes the list out. Returns false when nothing reached disk, so the caller can retry. + private static bool Write(string path, UInt128[] addresses, long[] stamps, int count, int dropped) { try { @@ -309,11 +374,14 @@ public static class LoginAllowlist { logger.Information("Login allowlist wrote {Count} entr(ies), dropped {Dropped} past TTL", count, dropped); } + + return true; } catch (Exception e) { // Recoverable: entries are still in memory and the next flush retries. logger.Warning(e, "Could not write the login allowlist to \"{Path}\"", path); + return false; } } diff --git a/Projects/UOContent/Network/LoginAllowlist/LoginAllowlistConfiguration.cs b/Projects/UOContent/Network/LoginAllowlist/LoginAllowlistConfiguration.cs index 08f04ecf2..5c673eb10 100644 --- a/Projects/UOContent/Network/LoginAllowlist/LoginAllowlistConfiguration.cs +++ b/Projects/UOContent/Network/LoginAllowlist/LoginAllowlistConfiguration.cs @@ -21,9 +21,8 @@ using Server.Json; namespace Server.Network; /// -/// Loads the from Configuration/login-allowlist.json (matching -/// the per-feature JSON config pattern used by BlocklistConfiguration). Loaded once; a missing file -/// writes a template so operators have something to edit. +/// Loads the from Configuration/login-allowlist.json. Loaded +/// once; a missing file writes a template so operators have something to edit. /// public static class LoginAllowlistConfiguration { @@ -82,11 +81,12 @@ public record LoginAllowlistSettings public TimeSpan Ttl { get; set; } = TimeSpan.FromDays(90); /// - /// How often a changed list is written out. A crash loses at most this much, and an entry is re-earned by - /// the next login. + /// How often a changed list is written out. A clean shutdown always writes, so this only bounds what a + /// crash loses — and an entry is re-earned by the next login. Hourly against a 90-day TTL, because each + /// flush walks the whole list on the game loop. /// [JsonPropertyName("flushInterval")] - public TimeSpan FlushInterval { get; set; } = TimeSpan.FromMinutes(1); + public TimeSpan FlushInterval { get; set; } = TimeSpan.FromHours(1); /// /// How many suppressed contributions inside revoke an address's entry. Past diff --git a/Projects/UOContent/Network/Blocklist/FileAllowlist.cs b/Projects/UOContent/Network/ManualAllowlist/ManualAllowlist.cs similarity index 83% rename from Projects/UOContent/Network/Blocklist/FileAllowlist.cs rename to Projects/UOContent/Network/ManualAllowlist/ManualAllowlist.cs index 0d5582713..20e351457 100644 --- a/Projects/UOContent/Network/Blocklist/FileAllowlist.cs +++ b/Projects/UOContent/Network/ManualAllowlist/ManualAllowlist.cs @@ -2,7 +2,7 @@ * ModernUO * * Copyright 2019-2026 - ModernUO Development Team * * Email: hi@modernuo.com * - * File: FileAllowlist.cs * + * File: ManualAllowlist.cs * * * * This program is free software: you can redistribute it and/or modify * * it under the terms of the GNU General Public License as published by * @@ -32,15 +32,15 @@ namespace Server.Network.Bans; /// Behavioural detections never consult the blocklist, so without reading the files here a carve-out is /// quietly routed around: one scanner behind a shared CGNAT address is enough to get the whole address /// contributed and firewalled. Reading them also means an entry applies on the next reload rather than the -/// next regeneration. Unconditional, unlike , but still no shield against a -/// manual ban — see . +/// next regeneration. Opt-in via ip-allowlist.json's enabled, since the poll runs for the +/// whole uptime; no shield against a manual ban either — see . /// -public static class FileAllowlist +public static class ManualAllowlist { - private static readonly ILogger logger = LogFactory.GetLogger(typeof(FileAllowlist)); + private static readonly ILogger logger = LogFactory.GetLogger(typeof(ManualAllowlist)); - // Written by the reload poll (off-loop), read by the accept path (game loop): a single volatile - // reference swap is the whole synchronization story — readers see the old or the new snapshot, whole. + // Written by the reload poll (off-loop), read by the accept path (game loop). One volatile reference + // swap is the whole synchronization story: readers see the old or the new snapshot, whole. private static volatile BlocklistSnapshot _snapshot = BlocklistSnapshot.Empty; private static string[] _patterns = []; @@ -53,24 +53,51 @@ public static class FileAllowlist /// True when an operator listed this address. Safe before . public static bool Contains(IPAddress address) => address != null && _snapshot.Contains(address); + /// True when the shard is reading allowlist files. Safe before . + public static bool Enabled { get; private set; } + public static void Initialize() { - // BlocklistFilter.Register ran during the Configure sweep, so the settings are populated. - var settings = BlocklistConfiguration.Settings; + ManualAllowlistConfiguration.Load(); + var settings = ManualAllowlistConfiguration.Settings; if (settings == null) { return; } - _patterns = ResolvePaths(settings.AllowlistFiles); + _patterns = ResolvePaths(settings.Files); _interval = settings.ReloadInterval <= TimeSpan.Zero ? TimeSpan.FromSeconds(60) : settings.ReloadInterval; - if (_patterns.Length == 0) + if (!settings.Enabled) { - logger.Information("File allowlist disabled (\"allowlistFiles\" empty in blocklist.json)"); + // The generator still subtracts these files, so a carve-out an operator already wrote looks + // like it works right up until a behavioural detection contributes the address anyway. + var present = ExpandPaths().Length; + _patterns = []; + + if (present > 0) + { + logger.Warning( + "Manual allowlist is off (\"enabled\" false in ip-allowlist.json) but {Count} allowlist file(s) " + + "are present; those carve-outs will not suppress ban contributions", + present + ); + } + else + { + logger.Information("Manual allowlist disabled (\"enabled\" false in ip-allowlist.json)"); + } + return; } + if (_patterns.Length == 0) + { + logger.Information("Manual allowlist disabled (\"files\" empty in ip-allowlist.json)"); + return; + } + + Enabled = true; Reload(); _cts = CancellationTokenSource.CreateLinkedTokenSource(Core.ClosingTokenSource.Token); @@ -197,7 +224,7 @@ public static class FileAllowlist } catch (Exception e) { - logger.Warning(e, "File allowlist reload check failed; keeping last snapshot ({Count})", Count); + logger.Warning(e, "Manual allowlist reload check failed; keeping last snapshot ({Count})", Count); } } } @@ -246,7 +273,7 @@ public static class FileAllowlist _lastStamp = stamp; logger.Information( - "File allowlist loaded {Count} range(s) from {Files} file(s)", + "Manual allowlist loaded {Count} range(s) from {Files} file(s)", next.Count, files ); diff --git a/Projects/UOContent/Network/ManualAllowlist/ManualAllowlistConfiguration.cs b/Projects/UOContent/Network/ManualAllowlist/ManualAllowlistConfiguration.cs new file mode 100644 index 000000000..dd21fe97c --- /dev/null +++ b/Projects/UOContent/Network/ManualAllowlist/ManualAllowlistConfiguration.cs @@ -0,0 +1,83 @@ +/************************************************************************* + * ModernUO * + * Copyright 2019-2026 - ModernUO Development Team * + * Email: hi@modernuo.com * + * File: ManualAllowlistConfiguration.cs * + * * + * This program is free software: you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation, either version 3 of the License, or * + * (at your option) any later version. * + * * + * You should have received a copy of the GNU General Public License * + * along with this program. If not, see . * + *************************************************************************/ + +using System; +using System.IO; +using System.Text.Json.Serialization; +using Server.Json; + +namespace Server.Network.Bans; + +/// +/// Loads the from Configuration/ip-allowlist.json. Loaded once; +/// a missing file writes a template so operators have something to edit. +/// +public static class ManualAllowlistConfiguration +{ + private const string _path = "Configuration/ip-allowlist.json"; + + public static ManualAllowlistSettings Settings { get; private set; } + + public static void Load() + { + var path = Path.Join(Core.BaseDirectory, _path); + + if (File.Exists(path)) + { + Settings = JsonConfig.Deserialize(path); + } + else + { + Settings = new ManualAllowlistSettings(); + Save(); + } + } + + private static void Save() + { + JsonConfig.Serialize(Path.Join(Core.BaseDirectory, _path), Settings); + } +} + +/// +/// Bound configuration for . Its own file rather than a corner of +/// blocklist.json: the blocklist is only one of two consumers, and the other +/// () works on a shard that runs no blocklist at all. +/// +public record ManualAllowlistSettings +{ + /// + /// Whether the shard reads at all. Off by default: reading them costs a poll for + /// the whole uptime, which no shard should pay before an operator has written a carve-out. + /// + [JsonPropertyName("enabled")] + public bool Enabled { get; set; } + + /// + /// Addresses that must never be blocked and never escalated, in the blocklist's own format. The same + /// files tools/Export-IpBlocklist.ps1 subtracts at generation time; the shard reads them so an + /// entry also suppresses ban contributions, which the generator alone cannot do. + /// + /// + /// The filename may contain wildcards, which is how the default picks up a carve-out an admin adds + /// without anyone editing this file. + /// + [JsonPropertyName("files")] + public string[] Files { get; set; } = ["Configuration/ip-allowlist*.txt"]; + + /// How often the files are checked for changes. Reloads only happen when one actually changed. + [JsonPropertyName("reloadInterval")] + public TimeSpan ReloadInterval { get; set; } = TimeSpan.FromSeconds(60); +} diff --git a/dev-docs/ip-bans-and-allowlists.md b/dev-docs/ip-bans-and-allowlists.md index 5588bb19e..40320efe5 100644 --- a/dev-docs/ip-bans-and-allowlists.md +++ b/dev-docs/ip-bans-and-allowlists.md @@ -24,7 +24,7 @@ Filters are consulted in registration order, first denial wins: | Filter | Source | Scope | |---|---|---| | `firewall` | `Configuration/firewall.json`, mutable in-game | Admin-curated, permanent | -| `blocklist` | `Configuration/ip-blocklist.txt` (millions of entries) | Reputation feeds | +| `blocklist` | `Configuration/ip-blocklist.txt` (millions of entries, opt-in) | Reputation feeds | | `auto-denylist` | In-memory, 15 min | What this shard just caught misbehaving | ### Contributing a ban @@ -38,7 +38,7 @@ Two, with different authority: | List | Source | Revocable? | Covers | |---|---|---|---| -| `FileAllowlist` | every `ip-allowlist*.txt` | No — an operator said so | Blocking **and** escalation | +| `ManualAllowlist` | every `ip-allowlist*.txt` (opt-in) | No — an operator said so | Blocking **and** escalation | | `LoginAllowlist` | Earned by authenticating, 90-day TTL | Yes — 10 strikes/hour | Blocking **and** escalation | Both are consulted **only after the blocklist has already matched**, so a normal accept — the one an @@ -66,16 +66,22 @@ If none of those match, they may be inside a **CIDR** in the blocklist, or held ### 2. Add them to the allowlist -One entry per line in `Distribution/Configuration/ip-allowlist.txt` — a bare address or a CIDR. This file -is yours; the generator creates it once and never rewrites it. +Set `"enabled": true` in `Configuration/ip-allowlist.json` first — it is off by default, so a shard that +has never written a carve-out does not poll for one. The shard logs a warning at startup if allowlist files +are present while the flag is off. + +Then one entry per line in `Distribution/Configuration/ip-allowlist.txt` — a bare address or a CIDR. This +file is yours; the generator creates it once and never rewrites it. ``` 203.0.113.42 # shard owner, listed via a shared upstream address 198.51.100.0/24 # a whole range if the ISP rotates within it ``` -The shard reloads within `reloadInterval` (60s default). **No restart, and no need to re-run the -generator.** From that point the address is neither blocked nor contributed. +With the flag on, the shard reloads within `reloadInterval` (60s default). **No restart, and no need to +re-run the generator.** From that point the address is neither blocked nor contributed. With the flag off +the generator still subtracts the file at generation time, so the address stops being *blocked* — but a +behavioural detection can still contribute it, which is the case the flag exists to cover. ### 3. Clear any ban that already exists @@ -123,8 +129,10 @@ where your players actually are, and a carve-out names a real network, so you bu ``` That writes `ip-allowlist-starlink.txt` beside the blocklist, and every `ip-allowlist*.txt` there is -subtracted — both by the generator and by the shard, with no config edit. Starlink costs about 0.1% of the -list. Blank a file (keep the file) to reputation-block that network again; delete it to drop the carve-out. +subtracted by the generator with no config edit. For the shard to read them too — which is what also stops +a carve-out address being *contributed* by a behavioural detection — set `enabled` in `ip-allowlist.json`; +it is off by default so no shard polls for files it never wrote. Starlink costs about 0.1% of the list. +Blank a file (keep the file) to reputation-block that network again; delete it to drop the carve-out. Carve-out files carry an `asn=` marker in their header, which is how `-RefreshCarveouts` finds them. A hand-written allowlist has no marker and is never rewritten. @@ -155,6 +163,12 @@ Escalation is **immediate**, on the first detection: a 15-minute local hold plus (4h) contribution. There is no N-connection threshold; the strike counter governs only revoking a `LoginAllowlist` entry. +The local hold runs 15 minutes from the **first** detection and is never extended by later ones, so an +address that keeps trying is released on schedule rather than held indefinitely. It does not get a free +run: the rate limiter sits *ahead* of the connection filters, so a flooder is re-reported and re-held on +its next attempt. Not refreshing is what keeps the holds in expiry order, which is what makes retiring +lapsed ones cost the number expiring rather than the number held. + ### What is deliberately NOT detected **Do not add rules based on arrival framing.** TCP has no message boundaries, so the network, the OS or a @@ -177,21 +191,27 @@ firewalled off. Shortening the 5s handshake window has been tried and broke real - **An allowlist cannot bootstrap.** A `LoginAllowlist` entry is only earned by getting in, so it can never repair an existing false positive, and it is weakest on rotating CGNAT — a player whose lease moved is a - stranger again. `FileAllowlist` is the fix for that, which is why it is manual. + stranger again. `ManualAllowlist` is the fix for that, which is why it is manual — and opt-in, via + `ip-allowlist.json`. - **A never-logged-in player on a shared address can still be caught**, for up to `badConnectDuration`, if a co-tenant misbehaves. Accepted: it is 4h and self-healing. The cheapest lever is `badConnectDuration`. - **`MaxConnections` (4096) is a hard ceiling.** The accept gate runs *after* the kernel completed the TCP handshake, so a blocklist match saves the socket setup and the `NetState` slot but never the connection itself. Only an upstream L4 proxy or edge scrubbing moves that cost off the shard. +- **The `auto-denylist` stops tracking at `maxEntries`.** Past it a detection still disconnects the + connection, but the address is not held, so it pays full detection cost on every reconnect instead of a + cheap accept-gate deny. The default is sized for the 50k–250k distinct-source floods seen in practice; a + flood past it wants upstream scrubbing rather than a larger cap, which only buys a longer on-loop scan. ## Configuration | File | Controls | |---|---| | `bans.json` | `reportRateLimitTrips`, `autoBanDuration`, `reportBadConnects`, `badConnectDuration` | -| `blocklist.json` | `file`, `allowlistFiles` (wildcards allowed), `reloadInterval`, `reportHits`, `banDuration`, `promoteSuppression` | +| `blocklist.json` | `enabled` (default `false`), `file`, `reloadInterval`, `reportHits`, `banDuration`, `promoteSuppression` | +| `ip-allowlist.json` | `enabled` (default `false`), `files` (wildcards allowed), `reloadInterval` | | `login-allowlist.json` | `enabled`, `file`, `ttl`, `flushInterval`, `escalateAfterStrikes`, `strikeWindow` | -| `auto-denylist.json` | `enabled`, `duration`, `maxEntries` | +| `auto-denylist.json` | `enabled`, `duration`, `maxEntries` (default `324,449` — sized for the floods seen in practice; see the remark on the setting before raising it) | | `crowdsec.json` | `lapiUrl`, `machineId`, `password`, `origin`, `manualBanDuration`, `flushInterval`, `maxQueue` | | `firewall.json` | Admin-curated entries | @@ -208,7 +228,7 @@ A shard fronted by an upstream proxy can disable all of it and register nothing. | `Projects/Server/Network/Bans/BanReasons.cs` | Reason slugs + the behavioural opt-in set | | `Projects/UOContent/Network/BanExemptions.cs` | Combines both allowlists into one answer | | `Projects/UOContent/Network/Blocklist/BlocklistFilter.cs` | File-sourced blocklist filter | -| `Projects/UOContent/Network/Blocklist/FileAllowlist.cs` | Operator carve-outs, read from the allowlist files | +| `Projects/UOContent/Network/Blocklist/ManualAllowlist.cs` | Operator carve-outs, read from the allowlist files | | `Projects/UOContent/Network/LoginAllowlist/LoginAllowlist.cs` | Allowlist earned by authenticating | | `Projects/UOContent/Network/AutoDenylist/AutoDenylist.cs` | Short-lived local hold | | `Projects/UOContent/Network/CrowdSec/CrowdSecReporter.cs` | LAPI contribution sink | diff --git a/dev-docs/networking-packets.md b/dev-docs/networking-packets.md index af47a1e06..639963cfd 100644 --- a/dev-docs/networking-packets.md +++ b/dev-docs/networking-packets.md @@ -511,9 +511,9 @@ Rules: Core owns the question; **every implementation lives in UOContent**. The three that ship are `firewall` (admin-curated, mutable at runtime, persisted to `Configuration/firewall.json`), `blocklist` (file-sourced, -millions of entries, demand-pages hits to CrowdSec) and `auto-denylist` (in-memory, short-lived, fed by the -shard's own behavioural detections). A shard that fronts its server with an upstream proxy or edge scrubbing -can drop all of them and register nothing. +millions of entries, demand-pages hits to CrowdSec, **opt-in**) and `auto-denylist` (in-memory, +short-lived, fed by the shard's own behavioural detections). A shard that fronts its server with an +upstream proxy or edge scrubbing can drop all of them and register nothing. The allowlists, ban contribution, behavioural detection and the operator process for exempting a false-positive address are covered separately in