From c33644532d709019f9aa64ceb0a3238a79dcb560 Mon Sep 17 00:00:00 2001 From: Pieter-Jan Briers Date: Fri, 21 Jun 2024 00:13:02 +0200 Subject: [PATCH] Rate limit ahelps (#29219) * Make chat rate limits a general-purpose system. Intending to use this with ahelps next. * Rate limt ahelps Fixes #28762 * Review comments --- .../Administration/Systems/BwoinkSystem.cs | 23 ++ .../Chat/Managers/ChatManager.RateLimit.cs | 85 ++---- Content.Server/Chat/Managers/ChatManager.cs | 10 +- Content.Server/Chat/Managers/IChatManager.cs | 4 +- Content.Server/Chat/Systems/ChatSystem.cs | 5 +- Content.Server/Entry/EntryPoint.cs | 3 + Content.Server/IoC/ServerContentIoC.cs | 3 + .../RateLimiting/PlayerRateLimitManager.cs | 254 ++++++++++++++++++ Content.Shared.Database/LogType.cs | 10 +- Content.Shared/CCVar/CCVars.cs | 19 ++ .../Locale/en-US/administration/bwoink.ftl | 2 + 11 files changed, 344 insertions(+), 74 deletions(-) create mode 100644 Content.Server/Players/RateLimiting/PlayerRateLimitManager.cs diff --git a/Content.Server/Administration/Systems/BwoinkSystem.cs b/Content.Server/Administration/Systems/BwoinkSystem.cs index a07115544b..0a797aa02a 100644 --- a/Content.Server/Administration/Systems/BwoinkSystem.cs +++ b/Content.Server/Administration/Systems/BwoinkSystem.cs @@ -9,6 +9,7 @@ using Content.Server.Administration.Managers; using Content.Server.Afk; using Content.Server.Discord; using Content.Server.GameTicking; +using Content.Server.Players.RateLimiting; using Content.Shared.Administration; using Content.Shared.CCVar; using Content.Shared.Mind; @@ -27,6 +28,8 @@ namespace Content.Server.Administration.Systems [UsedImplicitly] public sealed partial class BwoinkSystem : SharedBwoinkSystem { + private const string RateLimitKey = "AdminHelp"; + [Dependency] private readonly IPlayerManager _playerManager = default!; [Dependency] private readonly IAdminManager _adminManager = default!; [Dependency] private readonly IConfigurationManager _config = default!; @@ -35,6 +38,7 @@ namespace Content.Server.Administration.Systems [Dependency] private readonly GameTicker _gameTicker = default!; [Dependency] private readonly SharedMindSystem _minds = default!; [Dependency] private readonly IAfkManager _afkManager = default!; + [Dependency] private readonly PlayerRateLimitManager _rateLimit = default!; [GeneratedRegex(@"^https://discord\.com/api/webhooks/(\d+)/((?!.*/).*)$")] private static partial Regex DiscordRegex(); @@ -80,6 +84,22 @@ namespace Content.Server.Administration.Systems SubscribeLocalEvent(OnGameRunLevelChanged); SubscribeNetworkEvent(OnClientTypingUpdated); + + _rateLimit.Register( + RateLimitKey, + new RateLimitRegistration + { + CVarLimitPeriodLength = CCVars.AhelpRateLimitPeriod, + CVarLimitCount = CCVars.AhelpRateLimitCount, + PlayerLimitedAction = PlayerRateLimitedAction + }); + } + + private void PlayerRateLimitedAction(ICommonSession obj) + { + RaiseNetworkEvent( + new BwoinkTextMessage(obj.UserId, default, Loc.GetString("bwoink-system-rate-limited"), playSound: false), + obj.Channel); } private void OnOverrideChanged(string obj) @@ -395,6 +415,9 @@ namespace Content.Server.Administration.Systems return; } + if (_rateLimit.CountAction(eventArgs.SenderSession, RateLimitKey) != RateLimitStatus.Allowed) + return; + var escapedText = FormattedMessage.EscapeText(message.Text); string bwoinkText; diff --git a/Content.Server/Chat/Managers/ChatManager.RateLimit.cs b/Content.Server/Chat/Managers/ChatManager.RateLimit.cs index cf87ab6322..45e7d2e20d 100644 --- a/Content.Server/Chat/Managers/ChatManager.RateLimit.cs +++ b/Content.Server/Chat/Managers/ChatManager.RateLimit.cs @@ -1,84 +1,41 @@ -using System.Runtime.InteropServices; +using Content.Server.Players.RateLimiting; using Content.Shared.CCVar; using Content.Shared.Database; -using Robust.Shared.Enums; using Robust.Shared.Player; -using Robust.Shared.Timing; namespace Content.Server.Chat.Managers; internal sealed partial class ChatManager { - private readonly Dictionary _rateLimitData = new(); + private const string RateLimitKey = "Chat"; - public bool HandleRateLimit(ICommonSession player) + private void RegisterRateLimits() { - ref var datum = ref CollectionsMarshal.GetValueRefOrAddDefault(_rateLimitData, player, out _); - var time = _gameTiming.RealTime; - if (datum.CountExpires < time) - { - // Period expired, reset it. - var periodLength = _configurationManager.GetCVar(CCVars.ChatRateLimitPeriod); - datum.CountExpires = time + TimeSpan.FromSeconds(periodLength); - datum.Count = 0; - datum.Announced = false; - } - - var maxCount = _configurationManager.GetCVar(CCVars.ChatRateLimitCount); - datum.Count += 1; - - if (datum.Count <= maxCount) - return true; - - // Breached rate limits, inform admins if configured. - if (_configurationManager.GetCVar(CCVars.ChatRateLimitAnnounceAdmins)) - { - if (datum.NextAdminAnnounce < time) + _rateLimitManager.Register(RateLimitKey, + new RateLimitRegistration { - SendAdminAlert(Loc.GetString("chat-manager-rate-limit-admin-announcement", ("player", player.Name))); - var delay = _configurationManager.GetCVar(CCVars.ChatRateLimitAnnounceAdminsDelay); - datum.NextAdminAnnounce = time + TimeSpan.FromSeconds(delay); - } - } - - if (!datum.Announced) - { - DispatchServerMessage(player, Loc.GetString("chat-manager-rate-limited"), suppressLog: true); - _adminLogger.Add(LogType.ChatRateLimited, LogImpact.Medium, $"Player {player} breached chat rate limits"); - - datum.Announced = true; - } - - return false; + CVarLimitPeriodLength = CCVars.ChatRateLimitPeriod, + CVarLimitCount = CCVars.ChatRateLimitCount, + CVarAdminAnnounceDelay = CCVars.ChatRateLimitAnnounceAdminsDelay, + PlayerLimitedAction = RateLimitPlayerLimited, + AdminAnnounceAction = RateLimitAlertAdmins, + AdminLogType = LogType.ChatRateLimited, + }); } - private void PlayerStatusChanged(object? sender, SessionStatusEventArgs e) + private void RateLimitPlayerLimited(ICommonSession player) { - if (e.NewStatus == SessionStatus.Disconnected) - _rateLimitData.Remove(e.Session); + DispatchServerMessage(player, Loc.GetString("chat-manager-rate-limited"), suppressLog: true); } - private struct RateLimitDatum + private void RateLimitAlertAdmins(ICommonSession player) { - /// - /// Time stamp (relative to ) this rate limit period will expire at. - /// - public TimeSpan CountExpires; - - /// - /// How many messages have been sent in the current rate limit period. - /// - public int Count; - - /// - /// Have we announced to the player that they've been blocked in this rate limit period? - /// - public bool Announced; + if (_configurationManager.GetCVar(CCVars.ChatRateLimitAnnounceAdmins)) + SendAdminAlert(Loc.GetString("chat-manager-rate-limit-admin-announcement", ("player", player.Name))); + } - /// - /// Time stamp (relative to ) of the - /// next time we can send an announcement to admins about rate limit breach. - /// - public TimeSpan NextAdminAnnounce; + public RateLimitStatus HandleRateLimit(ICommonSession player) + { + return _rateLimitManager.CountAction(player, RateLimitKey); } } diff --git a/Content.Server/Chat/Managers/ChatManager.cs b/Content.Server/Chat/Managers/ChatManager.cs index 79683db641..6bb552d976 100644 --- a/Content.Server/Chat/Managers/ChatManager.cs +++ b/Content.Server/Chat/Managers/ChatManager.cs @@ -5,18 +5,17 @@ using Content.Server.Administration.Logs; using Content.Server.Administration.Managers; using Content.Server.Administration.Systems; using Content.Server.MoMMI; +using Content.Server.Players.RateLimiting; using Content.Server.Preferences.Managers; using Content.Shared.Administration; using Content.Shared.CCVar; using Content.Shared.Chat; using Content.Shared.Database; using Content.Shared.Mind; -using Robust.Server.Player; using Robust.Shared.Configuration; using Robust.Shared.Network; using Robust.Shared.Player; using Robust.Shared.Replays; -using Robust.Shared.Timing; using Robust.Shared.Utility; namespace Content.Server.Chat.Managers @@ -43,8 +42,7 @@ namespace Content.Server.Chat.Managers [Dependency] private readonly IConfigurationManager _configurationManager = default!; [Dependency] private readonly INetConfigurationManager _netConfigManager = default!; [Dependency] private readonly IEntityManager _entityManager = default!; - [Dependency] private readonly IGameTiming _gameTiming = default!; - [Dependency] private readonly IPlayerManager _playerManager = default!; + [Dependency] private readonly PlayerRateLimitManager _rateLimitManager = default!; /// /// The maximum length a player-sent message can be sent @@ -64,7 +62,7 @@ namespace Content.Server.Chat.Managers _configurationManager.OnValueChanged(CCVars.OocEnabled, OnOocEnabledChanged, true); _configurationManager.OnValueChanged(CCVars.AdminOocEnabled, OnAdminOocEnabledChanged, true); - _playerManager.PlayerStatusChanged += PlayerStatusChanged; + RegisterRateLimits(); } private void OnOocEnabledChanged(bool val) @@ -206,7 +204,7 @@ namespace Content.Server.Chat.Managers /// The type of message. public void TrySendOOCMessage(ICommonSession player, string message, OOCChatType type) { - if (!HandleRateLimit(player)) + if (HandleRateLimit(player) != RateLimitStatus.Allowed) return; // Check if message exceeds the character limit diff --git a/Content.Server/Chat/Managers/IChatManager.cs b/Content.Server/Chat/Managers/IChatManager.cs index c8c057a1ad..15d1288ee2 100644 --- a/Content.Server/Chat/Managers/IChatManager.cs +++ b/Content.Server/Chat/Managers/IChatManager.cs @@ -1,4 +1,6 @@ using System.Diagnostics.CodeAnalysis; +using Content.Server.Players; +using Content.Server.Players.RateLimiting; using Content.Shared.Administration; using Content.Shared.Chat; using Robust.Shared.Network; @@ -50,6 +52,6 @@ namespace Content.Server.Chat.Managers /// /// The player sending a chat message. /// False if the player has violated rate limits and should be blocked from sending further messages. - bool HandleRateLimit(ICommonSession player); + RateLimitStatus HandleRateLimit(ICommonSession player); } } diff --git a/Content.Server/Chat/Systems/ChatSystem.cs b/Content.Server/Chat/Systems/ChatSystem.cs index 65977c74f5..55beaf1f7f 100644 --- a/Content.Server/Chat/Systems/ChatSystem.cs +++ b/Content.Server/Chat/Systems/ChatSystem.cs @@ -6,6 +6,7 @@ using Content.Server.Administration.Managers; using Content.Server.Chat.Managers; using Content.Server.Examine; using Content.Server.GameTicking; +using Content.Server.Players.RateLimiting; using Content.Server.Speech.Components; using Content.Server.Speech.EntitySystems; using Content.Server.Station.Components; @@ -183,7 +184,7 @@ public sealed partial class ChatSystem : SharedChatSystem return; } - if (player != null && !_chatManager.HandleRateLimit(player)) + if (player != null && _chatManager.HandleRateLimit(player) != RateLimitStatus.Allowed) return; // Sus @@ -272,7 +273,7 @@ public sealed partial class ChatSystem : SharedChatSystem if (!CanSendInGame(message, shell, player)) return; - if (player != null && !_chatManager.HandleRateLimit(player)) + if (player != null && _chatManager.HandleRateLimit(player) != RateLimitStatus.Allowed) return; // It doesn't make any sense for a non-player to send in-game OOC messages, whereas non-players may be sending diff --git a/Content.Server/Entry/EntryPoint.cs b/Content.Server/Entry/EntryPoint.cs index e5b2338c5b..3a9d07126e 100644 --- a/Content.Server/Entry/EntryPoint.cs +++ b/Content.Server/Entry/EntryPoint.cs @@ -14,8 +14,10 @@ using Content.Server.Info; using Content.Server.IoC; using Content.Server.Maps; using Content.Server.NodeContainer.NodeGroups; +using Content.Server.Players; using Content.Server.Players.JobWhitelist; using Content.Server.Players.PlayTimeTracking; +using Content.Server.Players.RateLimiting; using Content.Server.Preferences.Managers; using Content.Server.ServerInfo; using Content.Server.ServerUpdates; @@ -108,6 +110,7 @@ namespace Content.Server.Entry _updateManager.Initialize(); _playTimeTracking.Initialize(); IoCManager.Resolve().Initialize(); + IoCManager.Resolve().Initialize(); } } diff --git a/Content.Server/IoC/ServerContentIoC.cs b/Content.Server/IoC/ServerContentIoC.cs index c6dfcadd38..858ad2fe26 100644 --- a/Content.Server/IoC/ServerContentIoC.cs +++ b/Content.Server/IoC/ServerContentIoC.cs @@ -13,8 +13,10 @@ using Content.Server.Info; using Content.Server.Maps; using Content.Server.MoMMI; using Content.Server.NodeContainer.NodeGroups; +using Content.Server.Players; using Content.Server.Players.JobWhitelist; using Content.Server.Players.PlayTimeTracking; +using Content.Server.Players.RateLimiting; using Content.Server.Preferences.Managers; using Content.Server.ServerInfo; using Content.Server.ServerUpdates; @@ -63,6 +65,7 @@ namespace Content.Server.IoC IoCManager.Register(); IoCManager.Register(); IoCManager.Register(); + IoCManager.Register(); } } } diff --git a/Content.Server/Players/RateLimiting/PlayerRateLimitManager.cs b/Content.Server/Players/RateLimiting/PlayerRateLimitManager.cs new file mode 100644 index 0000000000..59f086f9c3 --- /dev/null +++ b/Content.Server/Players/RateLimiting/PlayerRateLimitManager.cs @@ -0,0 +1,254 @@ +using System.Runtime.InteropServices; +using Content.Server.Administration.Logs; +using Content.Shared.Database; +using Robust.Server.Player; +using Robust.Shared.Configuration; +using Robust.Shared.Enums; +using Robust.Shared.Player; +using Robust.Shared.Timing; +using Robust.Shared.Utility; + +namespace Content.Server.Players.RateLimiting; + +/// +/// General-purpose system to rate limit actions taken by clients, such as chat messages. +/// +/// +/// +/// Different categories of rate limits must be registered ahead of time by calling . +/// Once registered, you can simply call to count a rate-limited action for a player. +/// +/// +/// This system is intended for rate limiting player actions over short periods, +/// to ward against spam that can cause technical issues such as admin client load. +/// It should not be used for in-game actions or similar. +/// +/// +/// Rate limits are reset when a client reconnects. +/// This should not be an issue for the reasonably short rate limit periods this system is intended for. +/// +/// +/// +public sealed class PlayerRateLimitManager +{ + [Dependency] private readonly IAdminLogManager _adminLog = default!; + [Dependency] private readonly IGameTiming _gameTiming = default!; + [Dependency] private readonly IConfigurationManager _cfg = default!; + [Dependency] private readonly IPlayerManager _playerManager = default!; + + private readonly Dictionary _registrations = new(); + private readonly Dictionary> _rateLimitData = new(); + + /// + /// Count and validate an action performed by a player against rate limits. + /// + /// The player performing the action. + /// The key string that was previously used to register a rate limit category. + /// Whether the action counted should be blocked due to surpassing rate limits or not. + /// + /// is not a connected player + /// OR is not a registered rate limit category. + /// + /// + public RateLimitStatus CountAction(ICommonSession player, string key) + { + if (player.Status == SessionStatus.Disconnected) + throw new ArgumentException("Player is not connected"); + if (!_registrations.TryGetValue(key, out var registration)) + throw new ArgumentException($"Unregistered key: {key}"); + + var playerData = _rateLimitData.GetOrNew(player); + ref var datum = ref CollectionsMarshal.GetValueRefOrAddDefault(playerData, key, out _); + var time = _gameTiming.RealTime; + if (datum.CountExpires < time) + { + // Period expired, reset it. + datum.CountExpires = time + registration.LimitPeriod; + datum.Count = 0; + datum.Announced = false; + } + + datum.Count += 1; + + if (datum.Count <= registration.LimitCount) + return RateLimitStatus.Allowed; + + // Breached rate limits, inform admins if configured. + if (registration.AdminAnnounceDelay is { } cvarAnnounceDelay) + { + if (datum.NextAdminAnnounce < time) + { + registration.Registration.AdminAnnounceAction!(player); + datum.NextAdminAnnounce = time + cvarAnnounceDelay; + } + } + + if (!datum.Announced) + { + registration.Registration.PlayerLimitedAction(player); + _adminLog.Add( + registration.Registration.AdminLogType, + LogImpact.Medium, + $"Player {player} breached '{key}' rate limit "); + + datum.Announced = true; + } + + return RateLimitStatus.Blocked; + } + + /// + /// Register a new rate limit category. + /// + /// + /// The key string that will be referred to later with . + /// Must be unique and should probably just be a constant somewhere. + /// + /// The data specifying the rate limit's parameters. + /// has already been registered. + /// is invalid. + public void Register(string key, RateLimitRegistration registration) + { + if (_registrations.ContainsKey(key)) + throw new InvalidOperationException($"Key already registered: {key}"); + + var data = new RegistrationData + { + Registration = registration, + }; + + if ((registration.AdminAnnounceAction == null) != (registration.CVarAdminAnnounceDelay == null)) + { + throw new ArgumentException( + $"Must set either both {nameof(registration.AdminAnnounceAction)} and {nameof(registration.CVarAdminAnnounceDelay)} or neither"); + } + + _cfg.OnValueChanged( + registration.CVarLimitCount, + i => data.LimitCount = i, + invokeImmediately: true); + _cfg.OnValueChanged( + registration.CVarLimitPeriodLength, + i => data.LimitPeriod = TimeSpan.FromSeconds(i), + invokeImmediately: true); + + if (registration.CVarAdminAnnounceDelay != null) + { + _cfg.OnValueChanged( + registration.CVarLimitCount, + i => data.AdminAnnounceDelay = TimeSpan.FromSeconds(i), + invokeImmediately: true); + } + + _registrations.Add(key, data); + } + + /// + /// Initialize the manager's functionality at game startup. + /// + public void Initialize() + { + _playerManager.PlayerStatusChanged += PlayerManagerOnPlayerStatusChanged; + } + + private void PlayerManagerOnPlayerStatusChanged(object? sender, SessionStatusEventArgs e) + { + if (e.NewStatus == SessionStatus.Disconnected) + _rateLimitData.Remove(e.Session); + } + + private sealed class RegistrationData + { + public required RateLimitRegistration Registration { get; init; } + public TimeSpan LimitPeriod { get; set; } + public int LimitCount { get; set; } + public TimeSpan? AdminAnnounceDelay { get; set; } + } + + private struct RateLimitDatum + { + /// + /// Time stamp (relative to ) this rate limit period will expire at. + /// + public TimeSpan CountExpires; + + /// + /// How many actions have been done in the current rate limit period. + /// + public int Count; + + /// + /// Have we announced to the player that they've been blocked in this rate limit period? + /// + public bool Announced; + + /// + /// Time stamp (relative to ) of the + /// next time we can send an announcement to admins about rate limit breach. + /// + public TimeSpan NextAdminAnnounce; + } +} + +/// +/// Contains all data necessary to register a rate limit with . +/// +public sealed class RateLimitRegistration +{ + /// + /// CVar that controls the period over which the rate limit is counted, measured in seconds. + /// + public required CVarDef CVarLimitPeriodLength { get; init; } + + /// + /// CVar that controls how many actions are allowed in a single rate limit period. + /// + public required CVarDef CVarLimitCount { get; init; } + + /// + /// An action that gets invoked when this rate limit has been breached by a player. + /// + /// + /// This can be used for informing players or taking administrative action. + /// + public required Action PlayerLimitedAction { get; init; } + + /// + /// CVar that controls the minimum delay between admin notifications, measured in seconds. + /// This can be omitted to have no admin notification system. + /// + /// + /// If set, must be set too. + /// + public CVarDef? CVarAdminAnnounceDelay { get; init; } + + /// + /// An action that gets invoked when a rate limit was breached and admins should be notified. + /// + /// + /// If set, must be set too. + /// + public Action? AdminAnnounceAction { get; init; } + + /// + /// Log type used to log rate limit violations to the admin logs system. + /// + public LogType AdminLogType { get; init; } = LogType.RateLimited; +} + +/// +/// Result of a rate-limited operation. +/// +/// +public enum RateLimitStatus : byte +{ + /// + /// The action was not blocked by the rate limit. + /// + Allowed, + + /// + /// The action was blocked by the rate limit. + /// + Blocked, +} diff --git a/Content.Shared.Database/LogType.cs b/Content.Shared.Database/LogType.cs index f486a7416c..33a5d30c6a 100644 --- a/Content.Shared.Database/LogType.cs +++ b/Content.Shared.Database/LogType.cs @@ -96,5 +96,13 @@ public enum LogType ChatRateLimited = 87, AtmosTemperatureChanged = 88, DeviceNetwork = 89, - StoreRefund = 90 + StoreRefund = 90, + + /// + /// User was rate-limited for some spam action. + /// + /// + /// This is a default value used by PlayerRateLimitManager, though users can use different log types. + /// + RateLimited = 91, } diff --git a/Content.Shared/CCVar/CCVars.cs b/Content.Shared/CCVar/CCVars.cs index f20ea21491..1a1a7f0226 100644 --- a/Content.Shared/CCVar/CCVars.cs +++ b/Content.Shared/CCVar/CCVars.cs @@ -883,6 +883,25 @@ namespace Content.Shared.CCVar public static readonly CVarDef AdminBypassMaxPlayers = CVarDef.Create("admin.bypass_max_players", true, CVar.SERVERONLY); + /* + * AHELP + */ + + /// + /// Ahelp rate limit values are accounted in periods of this size (seconds). + /// After the period has passed, the count resets. + /// + /// + public static readonly CVarDef AhelpRateLimitPeriod = + CVarDef.Create("ahelp.rate_limit_period", 2, CVar.SERVERONLY); + + /// + /// How many ahelp messages are allowed in a single rate limit period. + /// + /// + public static readonly CVarDef AhelpRateLimitCount = + CVarDef.Create("ahelp.rate_limit_count", 10, CVar.SERVERONLY); + /* * Explosions */ diff --git a/Resources/Locale/en-US/administration/bwoink.ftl b/Resources/Locale/en-US/administration/bwoink.ftl index 474af89c26..3a92f58ad1 100644 --- a/Resources/Locale/en-US/administration/bwoink.ftl +++ b/Resources/Locale/en-US/administration/bwoink.ftl @@ -14,3 +14,5 @@ bwoink-system-typing-indicator = {$players} {$count -> admin-bwoink-play-sound = Bwoink? bwoink-title-none-selected = None selected + +bwoink-system-rate-limited = System: you are sending messages too quickly. -- 2.51.2