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) <noreply@anthropic.com>
This commit is contained in:
parent
294dcd94a0
commit
bd44ef4026
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