From 2a1b04214bab9d5fe2530aafaf57ac4f23a1efb1 Mon Sep 17 00:00:00 2001 From: rootdarkarchon Date: Sat, 5 Nov 2022 14:51:59 +0100 Subject: [PATCH] adjust authentication handler --- .../Authentication/FailedAuthorization.cs | 14 +- .../SecretKeyAuthenticationHandler.cs | 162 +++++++----------- 2 files changed, 62 insertions(+), 114 deletions(-) diff --git a/MareSynchronosServer/MareSynchronosServices/Authentication/FailedAuthorization.cs b/MareSynchronosServer/MareSynchronosServices/Authentication/FailedAuthorization.cs index 54fd806..c6ad45a 100644 --- a/MareSynchronosServer/MareSynchronosServices/Authentication/FailedAuthorization.cs +++ b/MareSynchronosServer/MareSynchronosServices/Authentication/FailedAuthorization.cs @@ -4,23 +4,11 @@ using System.Threading.Tasks; namespace MareSynchronosServices.Authentication; -internal class FailedAuthorization : IDisposable +internal class FailedAuthorization { private int failedAttempts = 1; public int FailedAttempts => failedAttempts; public Task ResetTask { get; set; } - public CancellationTokenSource? ResetCts { get; set; } - - public void Dispose() - { - try - { - ResetCts?.Cancel(); - ResetCts?.Dispose(); - } - catch { } - } - public void IncreaseFailedAttempts() { Interlocked.Increment(ref failedAttempts); diff --git a/MareSynchronosServer/MareSynchronosServices/Authentication/SecretKeyAuthenticationHandler.cs b/MareSynchronosServer/MareSynchronosServices/Authentication/SecretKeyAuthenticationHandler.cs index cdd3579..e3eb740 100644 --- a/MareSynchronosServer/MareSynchronosServices/Authentication/SecretKeyAuthenticationHandler.cs +++ b/MareSynchronosServer/MareSynchronosServices/Authentication/SecretKeyAuthenticationHandler.cs @@ -1,9 +1,9 @@ using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Security.Cryptography; using System.Text; -using System.Threading; using System.Threading.Tasks; using MareSynchronosShared.Data; using MareSynchronosShared.Metrics; @@ -16,164 +16,124 @@ namespace MareSynchronosServices.Authentication; public class SecretKeyAuthenticationHandler { - private readonly ILogger logger; - private readonly MareMetrics metrics; + private readonly ILogger _logger; + private readonly MareMetrics _metrics; private const string Unauthorized = "Unauthorized"; - private readonly Dictionary authorizations = new(); - private readonly Dictionary failedAuthorizations = new(); - private readonly object authDictLock = new(); - private readonly object failedAuthLock = new(); + private readonly ConcurrentDictionary _cachedAuthorizations = new(); + private readonly ConcurrentDictionary _failedAuthorizations = new(); private readonly int _failedAttemptsForTempBan; private readonly int _tempBanMinutes; - private List _whitelistedIps = new(); + private readonly List _whitelistedIps = new(); public void ClearUnauthorizedUsers() { - lock (authDictLock) + foreach (var item in _cachedAuthorizations.ToArray()) { - foreach (var item in authorizations.ToArray()) + if (item.Value == Unauthorized) { - if (item.Value == Unauthorized) - { - authorizations[item.Key] = string.Empty; - } + _cachedAuthorizations[item.Key] = string.Empty; } } } public void RemoveAuthentication(string uid) { - lock (authDictLock) + var authorization = _cachedAuthorizations.Where(u => u.Value == uid); + if (authorization.Any()) { - var authorization = authorizations.Where(u => u.Value == uid); - if (authorization.Any()) - { - authorizations.Remove(authorization.First().Key); - } + _cachedAuthorizations.Remove(authorization.First().Key, out _); } } public async Task AuthenticateAsync(MareDbContext mareDbContext, string ip, string secretKey) { - metrics.IncCounter(MetricsAPI.CounterAuthenticationRequests); + _metrics.IncCounter(MetricsAPI.CounterAuthenticationRequests); if (string.IsNullOrEmpty(secretKey)) { - metrics.IncCounter(MetricsAPI.CounterAuthenticationFailures); + _metrics.IncCounter(MetricsAPI.CounterAuthenticationFailures); return new AuthReply() { Success = false, Uid = new UidMessage() { Uid = string.Empty } }; } - lock (failedAuthLock) + if (_failedAuthorizations.TryGetValue(ip, out var existingFailedAuthorization) && existingFailedAuthorization.FailedAttempts > _failedAttemptsForTempBan) { - if (failedAuthorizations.TryGetValue(ip, out var existingFailedAuthorization) && existingFailedAuthorization.FailedAttempts > _failedAttemptsForTempBan) - { - metrics.IncCounter(MetricsAPI.CounterAuthenticationFailures); + _metrics.IncCounter(MetricsAPI.CounterAuthenticationCacheHits); + _metrics.IncCounter(MetricsAPI.CounterAuthenticationFailures); + + if (existingFailedAuthorization.ResetTask == null) + { + _logger.LogWarning("TempBan {ip} for authorization spam", ip); - existingFailedAuthorization.ResetCts?.Cancel(); - existingFailedAuthorization.ResetCts?.Dispose(); - existingFailedAuthorization.ResetCts = new CancellationTokenSource(); - var token = existingFailedAuthorization.ResetCts.Token; existingFailedAuthorization.ResetTask = Task.Run(async () => { - await Task.Delay(TimeSpan.FromMinutes(_tempBanMinutes), token).ConfigureAwait(false); - if (token.IsCancellationRequested) return; - FailedAuthorization? failedAuthorization; - lock (failedAuthLock) - { - failedAuthorizations.Remove(ip, out failedAuthorization); - } - failedAuthorization?.Dispose(); - }, token); + await Task.Delay(TimeSpan.FromMinutes(_tempBanMinutes)).ConfigureAwait(false); - logger.LogWarning("TempBan {ip} for authorization spam", ip); - return new AuthReply() { Success = false, Uid = new UidMessage() { Uid = string.Empty } }; + }).ContinueWith((t) => + { + _failedAuthorizations.Remove(ip, out _); + }); } + return new AuthReply() { Success = false, Uid = new UidMessage() { Uid = string.Empty } }; } using var sha256 = SHA256.Create(); var hashedHeader = BitConverter.ToString(sha256.ComputeHash(Encoding.UTF8.GetBytes(secretKey))).Replace("-", ""); - string uid; - lock (authDictLock) + bool fromCache = _cachedAuthorizations.TryGetValue(hashedHeader, out string uid); + + if (fromCache) { - if (authorizations.TryGetValue(hashedHeader, out uid)) + _metrics.IncCounter(MetricsAPI.CounterAuthenticationCacheHits); + + if (uid == Unauthorized) { - if (uid == Unauthorized) - { - metrics.IncCounter(MetricsAPI.CounterAuthenticationFailures); - - logger.LogWarning("Failed authorization from {ip}", ip); - - lock (failedAuthLock) - { - if (!_whitelistedIps.Any(w => ip.Contains(w))) - { - if (failedAuthorizations.TryGetValue(ip, out var auth)) - { - auth.IncreaseFailedAttempts(); - } - else - { - failedAuthorizations[ip] = new FailedAuthorization(); - } - } - } - - return new AuthReply() { Success = false, Uid = new UidMessage() { Uid = string.Empty } }; - } - - metrics.IncCounter(MetricsAPI.CounterAuthenticationCacheHits); + return AuthenticationFailure(ip); } } - - if (string.IsNullOrEmpty(uid)) + else { uid = (await mareDbContext.Auth.AsNoTracking() .FirstOrDefaultAsync(m => m.HashedKey == hashedHeader).ConfigureAwait(false))?.UserUID; if (uid == null) { - lock (authDictLock) - { - authorizations[hashedHeader] = Unauthorized; - } + _cachedAuthorizations[hashedHeader] = Unauthorized; - logger.LogWarning("Failed authorization from {ip}", ip); - lock (failedAuthLock) - { - if (!_whitelistedIps.Any(w => ip.Contains(w))) - { - if (failedAuthorizations.TryGetValue(ip, out var auth)) - { - auth.IncreaseFailedAttempts(); - } - else - { - failedAuthorizations[ip] = new FailedAuthorization(); - } - } - - } - - metrics.IncCounter(MetricsAPI.CounterAuthenticationFailures); - return new AuthReply() { Success = false, Uid = new UidMessage() { Uid = string.Empty } }; + return AuthenticationFailure(ip); } - lock (authDictLock) - { - authorizations[hashedHeader] = uid; - } + _cachedAuthorizations[hashedHeader] = uid; } - metrics.IncCounter(MetricsAPI.CounterAuthenticationSuccesses); + _metrics.IncCounter(MetricsAPI.CounterAuthenticationSuccesses); return new AuthReply() { Success = true, Uid = new UidMessage() { Uid = uid } }; } + private AuthReply AuthenticationFailure(string ip) + { + _metrics.IncCounter(MetricsAPI.CounterAuthenticationFailures); + + _logger.LogWarning("Failed authorization from {ip}", ip); + if (!_whitelistedIps.Any(w => ip.Contains(w))) + { + if (_failedAuthorizations.TryGetValue(ip, out var auth)) + { + auth.IncreaseFailedAttempts(); + } + else + { + _failedAuthorizations[ip] = new FailedAuthorization(); + } + } + + return new AuthReply() { Success = false, Uid = new UidMessage() { Uid = string.Empty } }; + } + public SecretKeyAuthenticationHandler(IConfiguration configuration, ILogger logger, MareMetrics metrics) { - this.logger = logger; - this.metrics = metrics; + this._logger = logger; + this._metrics = metrics; var config = configuration.GetRequiredSection("MareSynchronos"); _failedAttemptsForTempBan = config.GetValue("FailedAuthForTempBan", 5); logger.LogInformation("FailedAuthForTempBan: {num}", _failedAttemptsForTempBan);