From 882443711b49332eaeadcc74df18c414f1e0e582 Mon Sep 17 00:00:00 2001 From: Archi Date: Mon, 23 Aug 2021 16:49:19 +0200 Subject: [PATCH] Optimize ApiAuthenticationMiddleware for bruteforcing We can favour bruteforcers by checking first if the client is even eligible for talking with us, this will (in a very negligible way) improve defense against common DoS. Also rewrite Timer initialization while at it. This is internal class and we don't expect this middleware to be initialized more than once anyway. --- .../ApiAuthenticationMiddleware.cs | 26 ++++++------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/ArchiSteamFarm/IPC/Integration/ApiAuthenticationMiddleware.cs b/ArchiSteamFarm/IPC/Integration/ApiAuthenticationMiddleware.cs index d9e028fe5..125703aa2 100644 --- a/ArchiSteamFarm/IPC/Integration/ApiAuthenticationMiddleware.cs +++ b/ArchiSteamFarm/IPC/Integration/ApiAuthenticationMiddleware.cs @@ -47,8 +47,7 @@ namespace ArchiSteamFarm.IPC.Integration { private static readonly SemaphoreSlim AuthorizationSemaphore = new(1, 1); private static readonly ConcurrentDictionary FailedAuthorizations = new(); - - private static Timer? ClearFailedAuthorizationsTimer; + private static readonly Timer ClearFailedAuthorizationsTimer = new(ClearFailedAuthorizations); private readonly ForwardedHeadersOptions ForwardedHeadersOptions; private readonly RequestDelegate Next; @@ -63,12 +62,7 @@ namespace ArchiSteamFarm.IPC.Integration { ForwardedHeadersOptions = forwardedHeadersOptions.Value ?? throw new InvalidOperationException(nameof(forwardedHeadersOptions)); lock (FailedAuthorizations) { - ClearFailedAuthorizationsTimer ??= new Timer( - _ => FailedAuthorizations.Clear(), - null, - TimeSpan.FromHours(FailedAuthorizationsCooldownInHours), // Delay - TimeSpan.FromHours(FailedAuthorizationsCooldownInHours) // Period - ); + ClearFailedAuthorizationsTimer.Change(TimeSpan.FromHours(FailedAuthorizationsCooldownInHours), TimeSpan.FromHours(FailedAuthorizationsCooldownInHours)); } } @@ -101,21 +95,23 @@ namespace ArchiSteamFarm.IPC.Integration { await context.Response.WriteJsonAsync(new GenericResponse(false, statusCodeResponse), jsonOptions.Value.SerializerSettings).ConfigureAwait(false); } + private static void ClearFailedAuthorizations(object? state = null) => FailedAuthorizations.Clear(); + private async Task<(HttpStatusCode StatusCode, bool Permanent)> GetAuthenticationStatus(HttpContext context) { if (context == null) { throw new ArgumentNullException(nameof(context)); } - if (ClearFailedAuthorizationsTimer == null) { - throw new InvalidOperationException(nameof(ClearFailedAuthorizationsTimer)); - } - IPAddress? clientIP = context.Connection.RemoteIpAddress; if (clientIP == null) { throw new InvalidOperationException(nameof(clientIP)); } + if (FailedAuthorizations.TryGetValue(clientIP, out byte attempts) && (attempts >= MaxFailedAuthorizationAttempts)) { + return (HttpStatusCode.Forbidden, false); + } + string? ipcPassword = ASF.GlobalConfig != null ? ASF.GlobalConfig.IPCPassword : GlobalConfig.DefaultIPCPassword; if (string.IsNullOrEmpty(ipcPassword)) { @@ -138,12 +134,6 @@ namespace ArchiSteamFarm.IPC.Integration { return (ForwardedHeadersOptions.KnownNetworks.Any(network => network.Contains(clientIP)) ? HttpStatusCode.OK : HttpStatusCode.Forbidden, true); } - if (FailedAuthorizations.TryGetValue(clientIP, out byte attempts)) { - if (attempts >= MaxFailedAuthorizationAttempts) { - return (HttpStatusCode.Forbidden, false); - } - } - if (!context.Request.Headers.TryGetValue(HeadersField, out StringValues passwords) && !context.Request.Query.TryGetValue("password", out passwords)) { return (HttpStatusCode.Unauthorized, true); }