Skip to content

Commit

Permalink
Bug fixes
Browse files Browse the repository at this point in the history 
- 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
  • Loading branch information
@jjxtra
jjxtra committed Dec 5, 2021
1 parent ac0f2fb commit e431ec6
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 43 deletions.
6 changes: 3 additions & 3 deletions IPBan/IPBan.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
<OutputType>Exe</OutputType>
<TargetFrameworks>net5.0</TargetFrameworks>
<PlatformTarget>AnyCPU</PlatformTarget>
<Version>1.6.0</Version>
<AssemblyVersion>1.6.0</AssemblyVersion>
<FileVersion>1.6.0</FileVersion>
<Version>1.6.1</Version>
<AssemblyVersion>1.6.1</AssemblyVersion>
<FileVersion>1.6.1</FileVersion>
<Company>Digital Ruby, LLC</Company>
<Copyright>(c) 2010 Digital Ruby, LLC</Copyright>
<GenerateAssemblyInfo>false</GenerateAssemblyInfo>
Expand Down
13 changes: 11 additions & 2 deletions IPBanCore/Core/IPBan/IPBanConfig.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,12 @@ public static string CleanMultilineString(string text)
return sb.ToString().Trim();
}

/// <inheritdoc />
public override string ToString()
{
return Xml;
}

/// <summary>
/// Get a value from configuration manager app settings
/// </summary>
Expand Down Expand Up @@ -610,14 +616,17 @@ public bool UserNameFailsUserNameWhitelistRegex(string userName)
/// If the user name whitelist is empty, this method returns true.
/// </summary>
/// <param name="userName">User name</param>
/// <param name="hasUserNameWhitelist">Whether a user name whitelist is defined</param>
/// <returns>True if within max edit distance of any whitelisted user name, false otherwise.</returns>
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)
{
Expand Down
53 changes: 34 additions & 19 deletions IPBanCore/Core/IPBan/IPBanLogFileManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,36 +94,51 @@ 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);
}
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);
}
}
}
Expand Down
32 changes: 28 additions & 4 deletions IPBanCore/Core/IPBan/IPBanLogFileScanner.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/// <summary>
/// Check if this log file scanner matches all the provided options
/// </summary>
/// <param name="options">Options</param>
/// <returns>True if matches options, false otherwise</returns>
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;
}

/// <inheritdoc />
protected override void OnProcessText(string text)
{
Expand Down Expand Up @@ -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.
/// </summary>
public string RegexFailure { get; set; }
public Regex RegexFailure { get; set; }

/// <summary>
/// Optional date/time format if RegexFailure has a timestamp group
Expand All @@ -158,7 +182,7 @@ public class IPBanIPAddressLogFileScannerOptions
/// <summary>
/// Regular expression for successful logins, see RegexFailure for regex group names.
/// </summary>
public string RegexSuccess { get; set; }
public Regex RegexSuccess { get; set; }

/// <summary>
/// Optional date/time format if RegexSuccess has a timestamp group
Expand Down
15 changes: 12 additions & 3 deletions IPBanCore/Core/IPBan/IPBanService_Private.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -328,7 +333,10 @@ private async Task ProcessPendingFailedLogins(IReadOnlyList<IPAddressLogEvent> 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;
}
Expand All @@ -344,7 +352,8 @@ private async Task ProcessPendingFailedLogins(IReadOnlyList<IPAddressLogEvent> 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
Expand Down
8 changes: 4 additions & 4 deletions IPBanCore/IPBanCore.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
<PreserveCompilationContext>true</PreserveCompilationContext>
<PackageRequireLicenseAcceptance>true</PackageRequireLicenseAcceptance>
<PackageId>DigitalRuby.IPBanCore</PackageId>
<Version>1.6.0</Version>
<AssemblyVersion>1.6.0</AssemblyVersion>
<FileVersion>1.6.0</FileVersion>
<Authors>Jeff Johnson</Authors>
<Version>1.6.1</Version>
<AssemblyVersion>1.6.1</AssemblyVersion>
<FileVersion>1.6.1</FileVersion>
<Authors>Jeff Johnson</Authors>
<Company>Digital Ruby, LLC</Company>
<Product>IPBan</Product>
<Description>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.</Description>
Expand Down
16 changes: 13 additions & 3 deletions IPBanTests/IPBanBanTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
4 changes: 2 additions & 2 deletions IPBanTests/IPBanLogFileParserTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions IPBanTests/IPBanTests.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
<AssemblyName>DigitalRuby.IPBanTests</AssemblyName>
<PreserveCompilationContext>true</PreserveCompilationContext>
<LangVersion>latest</LangVersion>
<Version>1.6.0</Version>
<AssemblyVersion>1.6.0</AssemblyVersion>
<FileVersion>1.6.0</FileVersion>
<Version>1.6.1</Version>
<AssemblyVersion>1.6.1</AssemblyVersion>
<FileVersion>1.6.1</FileVersion>
<GenerateSerializationAssemblies>Off</GenerateSerializationAssemblies>
<IsTrimmable>false</IsTrimmable>
</PropertyGroup>
Expand Down

0 comments on commit e431ec6

Please sign in to comment.