fix(crowdsec): send a payload LAPI accepts (500 on alerts, 401 on auth) (#2553)
## Problem
Contributing a ban to CrowdSec failed against a real LAPI — `POST /v1/alerts` answered **500**, and depending on the shard's locale, auth answered **401**. Three independent defects, each sufficient on its own.
## Fixes
**`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 — 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 — 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, not cosmetic. It is now an `internal const` carrying that reason.
Also fixed, same root cause as the timestamp bug: the login-expiry parse used a bare `DateTime.TryParse` on LAPI's RFC3339 `expire`. Under a mismatched culture that silently fails 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.
## Note on scope
The two 500 causes are independent. On an `en-US` shard only the missing scenario fields were biting; the date bug was latent and would have surfaced as an unexplained regression the first time someone ran a shard under a European locale.
## Verification
The emitted payload is field-for-field identical to a hand-verified request that a live LAPI accepts:
```json
[
{
"scenario": "modernuo/rate-limit",
"scenario_hash": "",
"scenario_version": "1.0",
"message": "ModernUO rate-limit ban for 192.0.2.123",
"events_count": 1,
"start_at": "2026-07-27T15:04:05.123Z",
"stop_at": "2026-07-27T15:04:05.123Z",
"capacity": 1,
"leakspeed": "0s",
"simulated": false,
"events": [],
"remediation": true,
"source": { "scope": "Ip", "value": "192.0.2.123" },
"decisions": [
{
"origin": "modernuo",
"type": "ban",
"scope": "Ip",
"value": "192.0.2.123",
"duration": "300s",
"scenario": "modernuo/rate-limit"
}
]
}
]
```
Regression tests assert the required scenario fields on the **serialized JSON** rather than the DTO — the DTO is not what goes on the wire — and cover the timestamp as a `[Theory]` across `fi-FI`/`th-TH`/`ar-SA`.
`dotnet test --filter "FullyQualifiedName~CrowdSec"` → **21/21 passed**, build clean with 0 warnings.
This commit is contained in:
Kamron Batman
GitHub
parent
294dcd94a0
commit
967ddf48fa
5 changed files with 111 additions and 4 deletions
|
|
@ -13,6 +13,7 @@
|
|||
* along with this program. If not, see <http://www.gnu.org/licenses/>. *
|
||||
*************************************************************************/
|
||||
|
||||
using System;
|
||||
using System.Net;
|
||||
using Server.Network.Bans.CrowdSec;
|
||||
using Xunit;
|
||||
|
|
@ -21,6 +22,16 @@ namespace Server.Tests.Network.Bans;
|
|||
|
||||
public class CrowdSecAlertClientTests
|
||||
{
|
||||
/// <summary>
|
||||
/// LAPI's default watcher profile matches the <c>crowdsec/</c> prefix and answers 401 without it, so
|
||||
/// this is a protocol constraint rather than a cosmetic product string.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void UserAgent_IsPrefixedForTheWatcherProfile()
|
||||
{
|
||||
Assert.StartsWith("crowdsec/", CrowdSecAlertClient.UserAgent, StringComparison.Ordinal);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuildDeleteQuery_EscapesOrigin()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
[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<JsonElement>().ValueKind);
|
||||
Assert.Equal(JsonValueKind.String, payload["scenario_version"]!.GetValue<JsonElement>().ValueKind);
|
||||
Assert.Equal(1, payload["capacity"]!.GetValue<int>());
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// ':' 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.
|
||||
/// </summary>
|
||||
[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;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>A non-UTC input must still be stamped as UTC — the trailing 'Z' is a literal, not a claim.</summary>
|
||||
[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()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -21,11 +21,24 @@ namespace Server.Network.Bans.CrowdSec;
|
|||
public sealed class CrowdSecAlert
|
||||
{
|
||||
[JsonPropertyName("scenario")] public string Scenario { get; set; }
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
[JsonPropertyName("scenario_hash")] public string ScenarioHash { get; set; } = "";
|
||||
|
||||
/// <summary>Required alongside <see cref="ScenarioHash"/>; same 500-on-missing behavior.</summary>
|
||||
[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; }
|
||||
|
||||
/// <summary>Bucket capacity. One decision per alert, so 1 — a leaky-bucket capacity of 0 is nonsense.</summary>
|
||||
[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; } = [];
|
||||
|
|
|
|||
|
|
@ -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 };
|
||||
|
||||
/// <summary>
|
||||
/// The <c>crowdsec/</c> 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.
|
||||
/// </summary>
|
||||
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<CrowdSecLoginResponse>(_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) =>
|
||||
|
|
|
|||
|
|
@ -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<CrowdSecAlert>(byIp.Count);
|
||||
|
||||
foreach (var (value, item) in byIp)
|
||||
|
|
@ -390,6 +391,17 @@ public sealed class CrowdSecReporter : IBanReporter
|
|||
return alerts;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// ISO8601/RFC3339 UTC timestamp for <c>start_at</c>/<c>stop_at</c>. LAPI parses these with Go's
|
||||
/// <c>time.RFC3339</c> 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.
|
||||
/// </summary>
|
||||
internal static string FormatTimestamp(DateTime time) =>
|
||||
(time.Kind == DateTimeKind.Utc ? time : time.ToUniversalTime())
|
||||
.ToString("yyyy-MM-ddTHH:mm:ss.fffZ", CultureInfo.InvariantCulture);
|
||||
|
||||
/// <summary>CrowdSec accepts Go durations; whole seconds are unambiguous and sufficient.</summary>
|
||||
internal static string FormatDuration(TimeSpan ttl)
|
||||
{
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue