From bd44ef4026db7e5d595e56a2ca58201d90930833 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Mon, 27 Jul 2026 23:30:32 -0700 Subject: [PATCH] fix(crowdsec): send a payload LAPI accepts (500 on alerts, 401 on auth) Contributing a ban to CrowdSec failed against a real LAPI. Three independent defects, each sufficient on its own: scenario_hash / scenario_version were never serialized. LAPI dereferences both unconditionally when persisting an alert, so omitting them is a nil deref and a 500 rather than a validation error. Both are now emitted with the values a watcher without a hub scenario is expected to send ("" and "1.0"). start_at/stop_at were formatted without an IFormatProvider. ':' is the time separator *specifier* in a custom .NET format string, not a literal, so a shard running under a culture like fi-FI emitted "T15.04.05.123Z" -- which Go's time.RFC3339 rejects, producing another 500. Non-Gregorian cultures (th-TH, ar-SA) would also shift the year. Formatting is now pinned to InvariantCulture in FormatTimestamp, which additionally converts non-UTC input, since the trailing 'Z' is a literal and was previously an unchecked claim. The User-Agent was a plain product string. LAPI's default watcher profile matches the "crowdsec/" prefix and answers 401 without it, so the header is a protocol constraint; it is now an internal const carrying that reason. Also fixes the same culture bug in the login-expiry parse: a bare DateTime.TryParse on LAPI's RFC3339 expire silently fails under a mismatched culture and falls back to a fabricated UtcNow+1h, pushing re-auth past the real expiry and costing a 401-relogin round trip on every send. capacity now defaults to 1 instead of 0, matching the one-decision-per-alert shape actually being sent. The resulting payload is field-for-field identical to a hand-verified request that LAPI accepts. Regression tests assert the required scenario fields on the serialized JSON rather than the DTO, since the DTO is not what goes on the wire, and cover the timestamp across fi-FI/th-TH/ar-SA. Co-Authored-By: Claude Opus 5 (1M context) --- .../Network/Bans/CrowdSecAlertClientTests.cs | 11 ++++ .../Network/Bans/CrowdSecReporterTests.cs | 56 +++++++++++++++++++ .../UOContent/Misc/CrowdSec/CrowdSecAlert.cs | 15 ++++- .../Misc/CrowdSec/CrowdSecAlertClient.cs | 19 ++++++- .../Misc/CrowdSec/CrowdSecReporter.cs | 14 ++++- 5 files changed, 111 insertions(+), 4 deletions(-) diff --git a/Projects/UOContent.Tests/Tests/Network/Bans/CrowdSecAlertClientTests.cs b/Projects/UOContent.Tests/Tests/Network/Bans/CrowdSecAlertClientTests.cs index f8a990f25..756e19b1d 100644 --- a/Projects/UOContent.Tests/Tests/Network/Bans/CrowdSecAlertClientTests.cs +++ b/Projects/UOContent.Tests/Tests/Network/Bans/CrowdSecAlertClientTests.cs @@ -13,6 +13,7 @@ * along with this program. If not, see . * *************************************************************************/ +using System; using System.Net; using Server.Network.Bans.CrowdSec; using Xunit; @@ -21,6 +22,16 @@ namespace Server.Tests.Network.Bans; public class CrowdSecAlertClientTests { + /// + /// LAPI's default watcher profile matches the crowdsec/ prefix and answers 401 without it, so + /// this is a protocol constraint rather than a cosmetic product string. + /// + [Fact] + public void UserAgent_IsPrefixedForTheWatcherProfile() + { + Assert.StartsWith("crowdsec/", CrowdSecAlertClient.UserAgent, StringComparison.Ordinal); + } + [Fact] public void BuildDeleteQuery_EscapesOrigin() { diff --git a/Projects/UOContent.Tests/Tests/Network/Bans/CrowdSecReporterTests.cs b/Projects/UOContent.Tests/Tests/Network/Bans/CrowdSecReporterTests.cs index a8daf3c44..6ad850f34 100644 --- a/Projects/UOContent.Tests/Tests/Network/Bans/CrowdSecReporterTests.cs +++ b/Projects/UOContent.Tests/Tests/Network/Bans/CrowdSecReporterTests.cs @@ -15,7 +15,9 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.Net; +using System.Text.Json; using System.Threading; using System.Threading.Tasks; using Server.Network.Bans.CrowdSec; @@ -81,6 +83,60 @@ public class CrowdSecReporterTests Assert.Equal("modernuo/blocklist", decision.Scenario); } + /// + /// LAPI dereferences scenario_hash/scenario_version unconditionally when persisting an alert, so an + /// omitted field is a 500, not a validation error. Asserted on the serialized payload rather than the + /// DTO because that is what actually goes on the wire. + /// + [Fact] + public void BuildAlerts_SerializedPayload_CarriesRequiredScenarioFields() + { + var alerts = CrowdSecReporter.BuildAlerts( + [new(IPAddress.Parse("9.9.9.9"), TimeSpan.FromHours(1), "rate-limit", false)], + Settings(), + DateTime.UnixEpoch); + + var payload = JsonSerializer.SerializeToNode(alerts)!.AsArray()[0]!.AsObject(); + + Assert.True(payload.ContainsKey("scenario_hash")); + Assert.True(payload.ContainsKey("scenario_version")); + Assert.Equal(JsonValueKind.String, payload["scenario_hash"]!.GetValue().ValueKind); + Assert.Equal(JsonValueKind.String, payload["scenario_version"]!.GetValue().ValueKind); + Assert.Equal(1, payload["capacity"]!.GetValue()); + } + + /// + /// ':' is the time-separator specifier in a custom .NET format string, so a shard under fi-FI used to + /// emit "T00.00.00.000Z" — which Go's time.RFC3339 rejects, and LAPI answers 500 for. th-TH additionally + /// shifts the year via the Buddhist calendar. + /// + [Theory] + [InlineData("fi-FI")] + [InlineData("th-TH")] + [InlineData("ar-SA")] + public void FormatTimestamp_IsIso8601_RegardlessOfCulture(string culture) + { + var previous = CultureInfo.CurrentCulture; + try + { + CultureInfo.CurrentCulture = new CultureInfo(culture); + Assert.Equal("1970-01-01T00:00:00.000Z", CrowdSecReporter.FormatTimestamp(DateTime.UnixEpoch)); + } + finally + { + CultureInfo.CurrentCulture = previous; + } + } + + /// A non-UTC input must still be stamped as UTC — the trailing 'Z' is a literal, not a claim. + [Fact] + public void FormatTimestamp_ConvertsNonUtcInput() + { + var local = new DateTimeOffset(1970, 1, 1, 2, 0, 0, TimeSpan.FromHours(2)).LocalDateTime; + + Assert.Equal("1970-01-01T00:00:00.000Z", CrowdSecReporter.FormatTimestamp(local)); + } + [Fact] public void FormatDuration_UsesSeconds_FloorsAtOne() { diff --git a/Projects/UOContent/Misc/CrowdSec/CrowdSecAlert.cs b/Projects/UOContent/Misc/CrowdSec/CrowdSecAlert.cs index db0d8d80f..773cf50ec 100644 --- a/Projects/UOContent/Misc/CrowdSec/CrowdSecAlert.cs +++ b/Projects/UOContent/Misc/CrowdSec/CrowdSecAlert.cs @@ -21,11 +21,24 @@ namespace Server.Network.Bans.CrowdSec; public sealed class CrowdSecAlert { [JsonPropertyName("scenario")] public string Scenario { get; set; } + + /// + /// Required by LAPI. It dereferences the field unconditionally when persisting the alert, so + /// omitting it answers 500 rather than a validation error. Empty is the accepted value for a + /// watcher that isn't shipping a hub scenario. + /// + [JsonPropertyName("scenario_hash")] public string ScenarioHash { get; set; } = ""; + + /// Required alongside ; same 500-on-missing behavior. + [JsonPropertyName("scenario_version")] public string ScenarioVersion { get; set; } = "1.0"; + [JsonPropertyName("message")] public string Message { get; set; } [JsonPropertyName("events_count")] public int EventsCount { get; set; } = 1; [JsonPropertyName("start_at")] public string StartAt { get; set; } [JsonPropertyName("stop_at")] public string StopAt { get; set; } - [JsonPropertyName("capacity")] public int Capacity { get; set; } + + /// Bucket capacity. One decision per alert, so 1 — a leaky-bucket capacity of 0 is nonsense. + [JsonPropertyName("capacity")] public int Capacity { get; set; } = 1; [JsonPropertyName("leakspeed")] public string LeakSpeed { get; set; } = "0s"; [JsonPropertyName("simulated")] public bool Simulated { get; set; } [JsonPropertyName("events")] public object[] Events { get; set; } = []; diff --git a/Projects/UOContent/Misc/CrowdSec/CrowdSecAlertClient.cs b/Projects/UOContent/Misc/CrowdSec/CrowdSecAlertClient.cs index af853244a..b91d0ebe5 100644 --- a/Projects/UOContent/Misc/CrowdSec/CrowdSecAlertClient.cs +++ b/Projects/UOContent/Misc/CrowdSec/CrowdSecAlertClient.cs @@ -15,6 +15,7 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.Net; using System.Net.Http; using System.Net.Http.Headers; @@ -40,6 +41,12 @@ public sealed class CrowdSecAlertClient : ICrowdSecAlertClient { private static readonly JsonSerializerOptions _jsonOptions = new() { PropertyNameCaseInsensitive = true }; + /// + /// The crowdsec/ prefix is load-bearing: LAPI's default watcher profile matches on it and + /// answers 401 for anything else, so this cannot be a plain product string. + /// + internal const string UserAgent = "crowdsec/ModernUO-watcher-1.0"; + private readonly HttpClient _http; private readonly string _machineId; private readonly string _password; @@ -51,7 +58,7 @@ public sealed class CrowdSecAlertClient : ICrowdSecAlertClient { var baseUri = new Uri(settings.LapiUrl, UriKind.Absolute); // fails loud on malformed url _http = new HttpClient { BaseAddress = baseUri, Timeout = TimeSpan.FromSeconds(30) }; - _http.DefaultRequestHeaders.Add("User-Agent", "ModernUO-watcher/1.0"); + _http.DefaultRequestHeaders.Add("User-Agent", UserAgent); _machineId = settings.MachineId; _password = settings.Password; } @@ -71,7 +78,15 @@ public sealed class CrowdSecAlertClient : ICrowdSecAlertClient var login = await response.Content.ReadFromJsonAsync(_jsonOptions, token) .ConfigureAwait(false); _token = login?.Token ?? throw new InvalidOperationException("CrowdSec login returned no token."); - _tokenExpiresUtc = DateTime.TryParse(login.Expire, out var exp) ? exp.ToUniversalTime() : DateTime.UtcNow.AddHours(1); + + // LAPI returns an RFC3339 expiry. Parse it invariantly for the same reason we format invariantly: + // the current culture must not decide whether a machine-readable timestamp is understood. + _tokenExpiresUtc = DateTime.TryParse( + login.Expire, + CultureInfo.InvariantCulture, + DateTimeStyles.AdjustToUniversal, + out var exp + ) ? exp : DateTime.UtcNow.AddHours(1); } private void Authorize(HttpRequestMessage message) => diff --git a/Projects/UOContent/Misc/CrowdSec/CrowdSecReporter.cs b/Projects/UOContent/Misc/CrowdSec/CrowdSecReporter.cs index 306d4ab53..e38aa17a6 100644 --- a/Projects/UOContent/Misc/CrowdSec/CrowdSecReporter.cs +++ b/Projects/UOContent/Misc/CrowdSec/CrowdSecReporter.cs @@ -15,6 +15,7 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.Net; using System.Threading; using System.Threading.Channels; @@ -357,7 +358,7 @@ public sealed class CrowdSecReporter : IBanReporter byIp[item.Ip.ToString()] = item; } - var timestamp = nowUtc.ToString("yyyy-MM-ddTHH:mm:ss.fffZ"); + var timestamp = FormatTimestamp(nowUtc); var alerts = new List(byIp.Count); foreach (var (value, item) in byIp) @@ -390,6 +391,17 @@ public sealed class CrowdSecReporter : IBanReporter return alerts; } + /// + /// ISO8601/RFC3339 UTC timestamp for start_at/stop_at. LAPI parses these with Go's + /// time.RFC3339 and answers 500 when the parse fails, so the format must be culture-independent: + /// ':' is the *time separator* specifier in a custom .NET format string, and a shard running under a + /// culture like fi-FI would otherwise emit "T12.34.56.789Z". InvariantCulture also pins the Gregorian + /// calendar, which non-Gregorian cultures (th-TH, ar-SA) would otherwise shift the year for. + /// + internal static string FormatTimestamp(DateTime time) => + (time.Kind == DateTimeKind.Utc ? time : time.ToUniversalTime()) + .ToString("yyyy-MM-ddTHH:mm:ss.fffZ", CultureInfo.InvariantCulture); + /// CrowdSec accepts Go durations; whole seconds are unambiguous and sufficient. internal static string FormatDuration(TimeSpan ttl) {