From e431ec6dd341de205f4d818c0d8d5d4213434586 Mon Sep 17 00:00:00 2001 From: Jeff Johnson Date: Sat, 4 Dec 2021 18:29:28 -0700 Subject: [PATCH] Bug fixes - User name whitelist edit distance was not used correctly - When log file config other than path/mask was changed, the changes were not propagated to the appropriate log file scanner --- IPBan/IPBan.csproj | 6 +-- IPBanCore/Core/IPBan/IPBanConfig.cs | 13 ++++- IPBanCore/Core/IPBan/IPBanLogFileManager.cs | 53 +++++++++++++------- IPBanCore/Core/IPBan/IPBanLogFileScanner.cs | 32 ++++++++++-- IPBanCore/Core/IPBan/IPBanService_Private.cs | 15 ++++-- IPBanCore/IPBanCore.csproj | 8 +-- IPBanTests/IPBanBanTests.cs | 16 ++++-- IPBanTests/IPBanLogFileParserTests.cs | 4 +- IPBanTests/IPBanTests.csproj | 6 +-- 9 files changed, 110 insertions(+), 43 deletions(-) diff --git a/IPBan/IPBan.csproj b/IPBan/IPBan.csproj index 2c784528..12b41882 100644 --- a/IPBan/IPBan.csproj +++ b/IPBan/IPBan.csproj @@ -4,9 +4,9 @@ Exe net5.0 AnyCPU - 1.6.0 - 1.6.0 - 1.6.0 + 1.6.1 + 1.6.1 + 1.6.1 Digital Ruby, LLC (c) 2010 Digital Ruby, LLC false diff --git a/IPBanCore/Core/IPBan/IPBanConfig.cs b/IPBanCore/Core/IPBan/IPBanConfig.cs index 42c8a4fe..93d06cd6 100644 --- a/IPBanCore/Core/IPBan/IPBanConfig.cs +++ b/IPBanCore/Core/IPBan/IPBanConfig.cs @@ -448,6 +448,12 @@ public static string CleanMultilineString(string text) return sb.ToString().Trim(); } + /// + public override string ToString() + { + return Xml; + } + /// /// Get a value from configuration manager app settings /// @@ -610,14 +616,17 @@ public bool UserNameFailsUserNameWhitelistRegex(string userName) /// If the user name whitelist is empty, this method returns true. /// /// User name + /// Whether a user name whitelist is defined /// True if within max edit distance of any whitelisted user name, false otherwise. - public bool IsUserNameWithinMaximumEditDistanceOfUserNameWhitelist(string userName) + public bool IsUserNameWithinMaximumEditDistanceOfUserNameWhitelist(string userName, out bool hasUserNameWhitelist) { if (userNameWhitelist.Count == 0) { - return true; + hasUserNameWhitelist = false; + return false; } + hasUserNameWhitelist = true; userName = userName.Normalize().ToUpperInvariant().Trim(); foreach (string userNameToCheckAgainst in userNameWhitelist) { diff --git a/IPBanCore/Core/IPBan/IPBanLogFileManager.cs b/IPBanCore/Core/IPBan/IPBanLogFileManager.cs index cb43d30f..6e16d16b 100644 --- a/IPBanCore/Core/IPBan/IPBanLogFileManager.cs +++ b/IPBanCore/Core/IPBan/IPBanLogFileManager.cs @@ -94,28 +94,43 @@ private void UpdateLogFiles(IPBanConfig newConfig) if (!string.IsNullOrWhiteSpace(pathAndMask)) { // if we don't have this log file and the platform matches, add it - bool noMatchingLogFile = logFilesToParse.FirstOrDefault(f => f.PathAndMask == pathAndMask) is null; + IPBanLogFileScanner existingScanner = logFilesToParse.FirstOrDefault(f => f.PathAndMask == pathAndMask); + + IPBanIPAddressLogFileScannerOptions options = new() + { + Dns = service.DnsLookup, + LoginHandler = service, + MaxFileSizeBytes = newFile.MaxFileSize, + PathAndMask = pathAndMask, + PingIntervalMilliseconds = (service.ManualCycle ? 0 : newFile.PingInterval), + RegexFailure = IPBanConfig.ParseRegex(newFile.FailedLoginRegex, true), + RegexSuccess = IPBanConfig.ParseRegex(newFile.SuccessfulLoginRegex, true), + RegexFailureTimestampFormat = newFile.FailedLoginRegexTimestampFormat, + RegexSuccessTimestampFormat = newFile.SuccessfulLoginRegexTimestampFormat, + Source = newFile.Source, + FailedLoginThreshold = newFile.FailedLoginThreshold, + FailedLogLevel = newFile.FailedLoginLogLevel, + SuccessfulLogLevel = newFile.SuccessfulLoginLogLevel + }; + + // if we have an existing log file scanner, but it does not match the new configuration, remove the old log file scanner + // and we will add a new one with updated config + if (existingScanner is not null && !existingScanner.MatchesOptions(options)) + { + // TODO: Add unit/integration test for this case + Logger.Info("Log file options changed for path/mask {0}", pathAndMask); + logFilesToParse.RemoveWhere(f => f.PathAndMask == pathAndMask); + existingScanner.Dispose(); + existingScanner = null; + } + + // make sure we match the platform before potentially making a new log file scanner bool platformMatches = !string.IsNullOrWhiteSpace(newFile.PlatformRegex) && Regex.IsMatch(OSUtility.Description, newFile.PlatformRegex.ToString().Trim(), RegexOptions.IgnoreCase | RegexOptions.CultureInvariant); - if (noMatchingLogFile && platformMatches) + + if (existingScanner is null && platformMatches) { // log files use a timer internally and do not need to be updated regularly - IPBanIPAddressLogFileScannerOptions options = new() - { - Dns = service.DnsLookup, - LoginHandler = service, - MaxFileSizeBytes = newFile.MaxFileSize, - PathAndMask = pathAndMask, - PingIntervalMilliseconds = (service.ManualCycle ? 0 : newFile.PingInterval), - RegexFailure = newFile.FailedLoginRegex, - RegexSuccess = newFile.SuccessfulLoginRegex, - RegexFailureTimestampFormat = newFile.FailedLoginRegexTimestampFormat, - RegexSuccessTimestampFormat = newFile.SuccessfulLoginRegexTimestampFormat, - Source = newFile.Source, - FailedLoginThreshold = newFile.FailedLoginThreshold, - FailedLogLevel = newFile.FailedLoginLogLevel, - SuccessfulLogLevel = newFile.SuccessfulLoginLogLevel - }; IPBanLogFileScanner scanner = new(options); logFilesToParse.Add(scanner); Logger.Info("Adding log file to parse: {0}", pathAndMask); @@ -123,7 +138,7 @@ private void UpdateLogFiles(IPBanConfig newConfig) else { Logger.Trace("Ignoring log file path {0}, regex: {1}, no matching file: {2}, platform match: {3}", - pathAndMask, newFile.PlatformRegex, noMatchingLogFile, platformMatches); + pathAndMask, newFile.PlatformRegex, existingScanner is null, platformMatches); } } } diff --git a/IPBanCore/Core/IPBan/IPBanLogFileScanner.cs b/IPBanCore/Core/IPBan/IPBanLogFileScanner.cs index 7e3de14d..79fb893d 100644 --- a/IPBanCore/Core/IPBan/IPBanLogFileScanner.cs +++ b/IPBanCore/Core/IPBan/IPBanLogFileScanner.cs @@ -76,13 +76,37 @@ public IPBanLogFileScanner(IPBanIPAddressLogFileScannerOptions options) : base(o this.loginHandler = options.LoginHandler; this.dns = options.Dns; - this.regexFailure = IPBanConfig.ParseRegex(options.RegexFailure, true); + this.regexFailure = options.RegexFailure; this.regexFailureTimestampFormat = options.RegexFailureTimestampFormat; - this.regexSuccess = IPBanConfig.ParseRegex(options.RegexSuccess, true); + this.regexSuccess = options.RegexSuccess; this.regexSuccessTimestampFormat = options.RegexSuccessTimestampFormat; } + /// + /// Check if this log file scanner matches all the provided options + /// + /// Options + /// True if matches options, false otherwise + public bool MatchesOptions(IPBanIPAddressLogFileScannerOptions options) + { + if (options is null) + { + return false; + } + + return Source == options.Source && + FailedLoginThreshold == options.FailedLoginThreshold && + FailedLogLevel == options.FailedLogLevel && + SuccessfulLogLevel == options.SuccessfulLogLevel && + this.loginHandler == options.LoginHandler && + this.dns == options.Dns && + this.regexFailure?.ToString() == options.RegexFailure?.ToString() && + this.regexFailureTimestampFormat == options.RegexFailureTimestampFormat && + this.regexSuccess?.ToString() == options.RegexSuccess?.ToString() && + this.regexSuccessTimestampFormat == options.RegexSuccessTimestampFormat; + } + /// protected override void OnProcessText(string text) { @@ -148,7 +172,7 @@ public class IPBanIPAddressLogFileScannerOptions /// Regular expression for failed logins, should at minimum have an ipaddress group, but can also have /// a timestamp group, source group and username group. /// - public string RegexFailure { get; set; } + public Regex RegexFailure { get; set; } /// /// Optional date/time format if RegexFailure has a timestamp group @@ -158,7 +182,7 @@ public class IPBanIPAddressLogFileScannerOptions /// /// Regular expression for successful logins, see RegexFailure for regex group names. /// - public string RegexSuccess { get; set; } + public Regex RegexSuccess { get; set; } /// /// Optional date/time format if RegexSuccess has a timestamp group diff --git a/IPBanCore/Core/IPBan/IPBanService_Private.cs b/IPBanCore/Core/IPBan/IPBanService_Private.cs index 9d5ad18e..74838d4f 100644 --- a/IPBanCore/Core/IPBan/IPBanService_Private.cs +++ b/IPBanCore/Core/IPBan/IPBanService_Private.cs @@ -179,7 +179,12 @@ internal async Task UpdateConfiguration() if (configChanged) { Logger.Info("Config file changed"); - } // else config was force refreshed but no actual change + } + else + { + Logger.Debug("Config file force reloaded"); + } + Logger.Debug("New config: " + Config.Xml); } } catch (Exception ex) @@ -328,7 +333,10 @@ private async Task ProcessPendingFailedLogins(IReadOnlyList i else { int maxFailedLoginAttempts; - if (Config.IsWhitelisted(userName)) + bool hasUserNameWhitelist = false; + bool userNameWhitelisted = Config.IsWhitelisted(userName) || + Config.IsUserNameWithinMaximumEditDistanceOfUserNameWhitelist(userName, out hasUserNameWhitelist); + if (userNameWhitelisted) { maxFailedLoginAttempts = Config.FailedLoginAttemptsBeforeBanUserNameWhitelist; } @@ -344,7 +352,8 @@ private async Task ProcessPendingFailedLogins(IReadOnlyList i bool ipBlacklisted = Config.BlacklistFilter.IsFiltered(ipAddress); bool userBlacklisted = (!ipBlacklisted && Config.BlacklistFilter.IsFiltered(userName)); bool userFailsWhitelistRegex = (!userBlacklisted && Config.UserNameFailsUserNameWhitelistRegex(userName)); - bool editDistanceBlacklisted = (!ipBlacklisted && !userBlacklisted && !userFailsWhitelistRegex && !Config.IsUserNameWithinMaximumEditDistanceOfUserNameWhitelist(userName)); + bool editDistanceBlacklisted = (!ipBlacklisted && !userBlacklisted && !userFailsWhitelistRegex && + (hasUserNameWhitelist && !userNameWhitelisted)); bool configBlacklisted = ipBlacklisted || userBlacklisted || userFailsWhitelistRegex || editDistanceBlacklisted; // if the event came in with a count of 0 that means it is an automatic ban diff --git a/IPBanCore/IPBanCore.csproj b/IPBanCore/IPBanCore.csproj index 028920c0..f3c3232d 100644 --- a/IPBanCore/IPBanCore.csproj +++ b/IPBanCore/IPBanCore.csproj @@ -6,10 +6,10 @@ true true DigitalRuby.IPBanCore - 1.6.0 - 1.6.0 - 1.6.0 - Jeff Johnson + 1.6.1 + 1.6.1 + 1.6.1 + Jeff Johnson Digital Ruby, LLC IPBan IPBan is the leading solution to block hackers and botnets. Easily monitor events for all kinds of sources and put the bad ip addresses in the firewall automatically. diff --git a/IPBanTests/IPBanBanTests.cs b/IPBanTests/IPBanBanTests.cs index 522fc281..f6d27d01 100644 --- a/IPBanTests/IPBanBanTests.cs +++ b/IPBanTests/IPBanBanTests.cs @@ -387,15 +387,25 @@ public async Task TestUserNameWhitelistRegexBan() } [Test] - public void TestUserNameWhitelistBan() + public async Task TestUserNameWhitelistBan() { using IPBanConfig.TempConfigChanger configChanger = new(service, xml => { return IPBanConfig.ChangeConfigAppSetting(xml, "UserNameWhitelist", "OnlyMe"); }, out string newConfig); - // TODO: ensure non OnlyMe users are banned immediately - // TODO: ensure OnlyMe user gets 20 failed logins before ban + service.AddIPAddressLogEvents(new IPAddressLogEvent[] + { + // should ban, we have a user name whitelist + new IPAddressLogEvent("99.99.99.90", "ftp_1", "RDP", 1, IPAddressEventType.FailedLogin), + + // should not ban after 19 attempts, user is whitelisted + new IPAddressLogEvent("99.99.99.99", "onlyme", "RDP", 19, IPAddressEventType.FailedLogin) + }); + await service.RunCycleAsync(); + + Assert.IsTrue(service.Firewall.IsIPAddressBlocked("99.99.99.90", out _)); + Assert.IsFalse(service.Firewall.IsIPAddressBlocked("99.99.99.99", out _)); } [Test] diff --git a/IPBanTests/IPBanLogFileParserTests.cs b/IPBanTests/IPBanLogFileParserTests.cs index ef248abe..b7664556 100644 --- a/IPBanTests/IPBanLogFileParserTests.cs +++ b/IPBanTests/IPBanLogFileParserTests.cs @@ -295,9 +295,9 @@ private LogFileScanner SetupLogFileScanner(string failureRegex = "", LoginHandler = this, Source = source, PathAndMask = pathAndMask, - RegexFailure = failureRegex, + RegexFailure = IPBanConfig.ParseRegex(failureRegex, true), RegexFailureTimestampFormat = failureRegexTimestampFormat, - RegexSuccess = successRegex, + RegexSuccess = IPBanConfig.ParseRegex(successRegex, true), RegexSuccessTimestampFormat = successRegexTimestampFormat }; LogFileScanner scanner = new IPBanLogFileScanner(options); diff --git a/IPBanTests/IPBanTests.csproj b/IPBanTests/IPBanTests.csproj index 9fbf4b03..4de8de60 100644 --- a/IPBanTests/IPBanTests.csproj +++ b/IPBanTests/IPBanTests.csproj @@ -8,9 +8,9 @@ DigitalRuby.IPBanTests true latest - 1.6.0 - 1.6.0 - 1.6.0 + 1.6.1 + 1.6.1 + 1.6.1 Off false