diff --git a/.claude/settings.local.json b/.claude/settings.local.json new file mode 100644 index 00000000..dabd2a58 --- /dev/null +++ b/.claude/settings.local.json @@ -0,0 +1,13 @@ +{ + "permissions": { + "allow": [ + "Bash", + "Powershell", + "Python", + "Write", + "Edit", + "MultiEdit" + ], + "defaultMode": "dontAsk" + } +} diff --git a/.gitignore b/.gitignore index bb7b8f9e..c2104f79 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,9 @@ bin/ obj/ package/ packages/ +results/ +report/ +.nuget-packages/ *.suo *.cachefile *.user diff --git a/IPBan/IPBan.csproj b/IPBan/IPBan.csproj index f2cea38f..8295da1c 100644 --- a/IPBan/IPBan.csproj +++ b/IPBan/IPBan.csproj @@ -26,6 +26,9 @@ false partial true + + + $(NoWarn);IL2104;IL2026 @@ -45,6 +48,6 @@ - + diff --git a/IPBanCore/Core/IPBan/IPBanConfig.cs b/IPBanCore/Core/IPBan/IPBanConfig.cs index 5cd733bf..4d2e309c 100644 --- a/IPBanCore/Core/IPBan/IPBanConfig.cs +++ b/IPBanCore/Core/IPBan/IPBanConfig.cs @@ -42,6 +42,7 @@ namespace DigitalRuby.IPBanCore /// /// Configuration for ip ban app /// + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "Configuration XML models are runtime-deserialized and preserved by IPBanCore usage patterns.")] public sealed class IPBanConfig : IIsWhitelisted { /// @@ -134,6 +135,7 @@ public void Dispose() private readonly string processToRunOnUnban = string.Empty; private readonly bool useDefaultBannedIPAddressHandler; private readonly string getUrlUpdate = string.Empty; + private readonly string getUrlUpdateSha256 = string.Empty; private readonly string getUrlStart = string.Empty; private readonly string getUrlStop = string.Empty; private readonly string getUrlConfig = string.Empty; @@ -238,6 +240,7 @@ private IPBanConfig(XmlDocument doc, IDnsLookup dns = null, IDnsServerList dnsLi TryGetConfig("UserNameWhitelistMinimumEditDistance", ref userNameWhitelistMaximumEditDistance); TryGetConfig("FailedLoginAttemptsBeforeBanUserNameWhitelist", ref failedLoginAttemptsBeforeBanUserNameWhitelist); TryGetConfig("GetUrlUpdate", ref getUrlUpdate); + TryGetConfig("GetUrlUpdateSha256", ref getUrlUpdateSha256); TryGetConfig("GetUrlStart", ref getUrlStart); TryGetConfig("GetUrlStop", ref getUrlStop); TryGetConfig("GetUrlConfig", ref getUrlConfig); @@ -260,8 +263,11 @@ private string GetAppSettingsValue(string key, bool logMissing = true) { if (string.IsNullOrWhiteSpace(key)) { - // bad key - Logger.Warn("Ignoring null/empty key"); + if (logMissing) + { + // bad key + Logger.Debug("Ignoring null/empty key"); + } return null; } @@ -269,7 +275,7 @@ private string GetAppSettingsValue(string key, bool logMissing = true) { if (logMissing) { - Logger.Warn("Ignoring key {0}, not found in appSettings", key); + Logger.Debug("Ignoring key {0}, not found in appSettings", key); } return null; // skip trying to convert } @@ -638,7 +644,7 @@ public bool IsUserNameWithinMaximumEditDistanceOfUserNameWhitelist(string userNa foreach (string userNameToCheckAgainst in userNameWhitelist) { int distance = LevenshteinUnsafe.Distance(userName, userNameToCheckAgainst); - if (distance <= userNameWhitelistMaximumEditDistance) + if (distance >= 0 && distance <= userNameWhitelistMaximumEditDistance) { return true; } @@ -1152,6 +1158,14 @@ public static string ValidateFirewallUriRules(string firewallUriRules) /// public string GetUrlUpdate { get { return getUrlUpdate; } } + /// + /// Expected SHA-256 hash (hex, case-insensitive) of the binary returned by GetUrlUpdate. + /// If empty, the auto-update download is fetched but NOT executed — this is the safe default + /// and protects against a malicious/MITMed update server. Operators must explicitly set this + /// hash to opt in to automated update execution. + /// + public string GetUrlUpdateSha256 { get { return getUrlUpdateSha256; } } + /// /// A url to get when the service starts, empty for none. See ReplaceUrl of IPBanService for place-holders. /// diff --git a/IPBanCore/Core/IPBan/IPBanDB.cs b/IPBanCore/Core/IPBan/IPBanDB.cs index a84f0f8a..7409c6a3 100644 --- a/IPBanCore/Core/IPBan/IPBanDB.cs +++ b/IPBanCore/Core/IPBan/IPBanDB.cs @@ -139,18 +139,24 @@ private static long GetInt64(object value) /// public static IPAddressEntry ParseIPAddressEntry(SqliteDataReader reader) { - string ipAddress = reader.GetString(0); - long lastFailedLogin = reader.GetInt64(1); - long failedLoginCount = reader.GetInt64(2); + // Older DB schemas (created before BanEndDate / UserName / Source columns were added) + // can return NULL even though the column declarations now have defaults — read with + // IsDBNull guards so a stale schema doesn't crash every query that hits a legacy row. + string ipAddress = reader.IsDBNull(0) ? string.Empty : reader.GetString(0); + long lastFailedLogin = reader.IsDBNull(1) ? 0L : reader.GetInt64(1); + long failedLoginCount = reader.IsDBNull(2) ? 0L : reader.GetInt64(2); object banDateObj = reader.GetValue(3); - IPAddressState state = (IPAddressState)(int)reader.GetInt32(4); + IPAddressState state = reader.IsDBNull(4) ? IPAddressState.Active : (IPAddressState)(int)reader.GetInt32(4); object banEndDateObj = reader.GetValue(5); - string userName = reader.GetString(6); - string source = reader.GetString(7); + string userName = reader.IsDBNull(6) ? string.Empty : reader.GetString(6); + string source = reader.IsDBNull(7) ? string.Empty : reader.GetString(7); long banDateLong = GetInt64(banDateObj); long banEndDateLong = GetInt64(banEndDateObj); DateTime? banDate = (banDateLong == 0 ? (DateTime?)null : banDateLong.ToDateTimeUnixMilliseconds()); - DateTime? banEndDate = (banDateLong == 0 ? (DateTime?)null : banEndDateLong.ToDateTimeUnixMilliseconds()); + // Each ban-date column is independent: BanDate may be set while BanEndDate is NULL + // (legacy rows from before BanEndDate existed, or in-flight transitions). Each guard + // must check its own column. + DateTime? banEndDate = (banEndDateLong == 0 ? (DateTime?)null : banEndDateLong.ToDateTimeUnixMilliseconds()); DateTime lastFailedLoginDt = lastFailedLogin.ToDateTimeUnixMilliseconds(); return new IPAddressEntry { diff --git a/IPBanCore/Core/IPBan/IPBanFirewallUtility.cs b/IPBanCore/Core/IPBan/IPBanFirewallUtility.cs index b0a734a0..4cfe706a 100644 --- a/IPBanCore/Core/IPBan/IPBanFirewallUtility.cs +++ b/IPBanCore/Core/IPBan/IPBanFirewallUtility.cs @@ -25,6 +25,7 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE using System; using System.Collections.Generic; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.IO.Pipelines; using System.Linq; @@ -57,6 +58,7 @@ private static void AppendRange(StringBuilder b, PortRange range) /// Rule prefix or null for default /// Current firewall /// Firewall + [UnconditionalSuppressMessage("Trimming", "IL2072", Justification = "Firewall implementations are selected and activated dynamically at runtime by design.")] public static IIPBanFirewall CreateFirewall(IReadOnlyCollection allTypes, string rulePrefix = null, IIPBanFirewall previousFirewall = null) @@ -611,6 +613,11 @@ public static int RunProcess(string program, object input, object output, params inputStream.CopyTo(p.StandardInput.BaseStream); } } + catch (IOException) + { + // the process may have already exited and closed stdin (broken pipe); + // feeding stdin is best-effort, so ignore the write failure + } finally { try { p.StandardInput.Close(); } catch { /* ignore */ } diff --git a/IPBanCore/Core/IPBan/IPBanIPThreatUploader.cs b/IPBanCore/Core/IPBan/IPBanIPThreatUploader.cs index 57f9405b..738fd992 100644 --- a/IPBanCore/Core/IPBan/IPBanIPThreatUploader.cs +++ b/IPBanCore/Core/IPBan/IPBanIPThreatUploader.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; +using System.Diagnostics.CodeAnalysis; using System.Threading; using System.Threading.Tasks; @@ -31,6 +32,7 @@ public void Dispose() } /// + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "Anonymous payload shape is fixed and used only for IPThreat API upload.")] public async Task Update(CancellationToken cancelToken = default) { // ready to run? @@ -104,12 +106,22 @@ await service.RequestMaker.MakeRequestAsync(ipThreatReportApiUri, /// public void AddIPAddressLogEvents(IEnumerable events) { - lock (events) + // Run the filter outside the lock — the predicate calls into service.Config which we + // don't want to hold the events lock across. Only the AddRange happens inside. + var filtered = events.Where(e => e.Type == IPAddressEventType.Blocked && + e.Count > 0 && + !e.External && + !service.Config.IsWhitelisted(e.IPAddress, out _)).ToArray(); + if (filtered.Length == 0) + { + return; + } + // Qualify with `this.` so the lock targets the field — the parameter is also named + // `events` and would otherwise shadow it, locking an unrelated caller-supplied object + // while the field itself stayed unprotected. + lock (this.events) { - this.events.AddRange(events.Where(e => e.Type == IPAddressEventType.Blocked && - e.Count > 0 && - !e.External && - !service.Config.IsWhitelisted(e.IPAddress, out _))); + this.events.AddRange(filtered); } } } diff --git a/IPBanCore/Core/IPBan/IPBanLogManager.cs b/IPBanCore/Core/IPBan/IPBanLogManager.cs index 02205758..f5c98b29 100644 --- a/IPBanCore/Core/IPBan/IPBanLogManager.cs +++ b/IPBanCore/Core/IPBan/IPBanLogManager.cs @@ -57,7 +57,6 @@ public IPBanLogManager(IIPBanService service) /// public Task Update(CancellationToken cancelToken) { - UpdateLogFiles(service.Config); if (service.ManualCycle) { foreach (var scanner in logsToParse) diff --git a/IPBanCore/Core/IPBan/IPBanMemoryFirewall.cs b/IPBanCore/Core/IPBan/IPBanMemoryFirewall.cs index a66a0e98..9a828d91 100644 --- a/IPBanCore/Core/IPBan/IPBanMemoryFirewall.cs +++ b/IPBanCore/Core/IPBan/IPBanMemoryFirewall.cs @@ -695,7 +695,7 @@ public override string GetPorts(string ruleName) { return ruleRanges.Ports; } - else if (!allowRuleRanges.TryGetValue(ruleName, out ruleRanges)) + else if (allowRuleRanges.TryGetValue(ruleName, out ruleRanges)) { return ruleRanges.Ports; } diff --git a/IPBanCore/Core/IPBan/IPBanService.cs b/IPBanCore/Core/IPBan/IPBanService.cs index 43998e9d..6ae559aa 100644 --- a/IPBanCore/Core/IPBan/IPBanService.cs +++ b/IPBanCore/Core/IPBan/IPBanService.cs @@ -153,11 +153,12 @@ public void AddIPAddressLogEvents(IEnumerable events) } /// - /// Write a new config file + /// Write a new config file. Virtual so tests can intercept the call without touching + /// the on-disk config. /// /// Xml of the new config file /// Task - public async Task WriteConfigAsync(string xml) + public virtual async Task WriteConfigAsync(string xml) { // Ensure valid xml before writing the file XmlDocument doc = new(); @@ -488,6 +489,13 @@ public static T CreateAndStartIPBanTestService(string directory = null, strin ExtensionMethods.FileWriteAllTextWithRetry(configFileOverridePath, configFileOverrideText); T service = IPBanService.CreateService(); service.ConfigFilePath = configFilePath; + service.ConfigReaderWriter.UseFile = false; + service.ConfigReaderWriter.GlobalConfigString = configFileText; + service.ConfigOverrideReaderWriter.UseFile = false; + service.ConfigOverrideReaderWriter.GlobalConfigString = configFileOverrideText; + service.LocalIPAddressString = "127.0.0.1"; + service.RemoteIPAddressString = "127.0.0.1"; + service.OtherIPAddressesString = "127.0.0.1"; service.MultiThreaded = false; service.ManualCycle = true; service.DnsList = null; // too slow for tests, turn off @@ -552,7 +560,6 @@ public static void DisposeIPBanTestService(IPBanService service) Directory.Delete(appDataCache, true); } service.Firewall.Truncate(); - service.RunCycleAsync().Sync(); service.IPBanDelegate = null; service.Dispose(); IPBanService.CleanupIPBanTestFiles(); diff --git a/IPBanCore/Core/IPBan/IPBanService_Private.cs b/IPBanCore/Core/IPBan/IPBanService_Private.cs index ecfb7a4d..9f190732 100644 --- a/IPBanCore/Core/IPBan/IPBanService_Private.cs +++ b/IPBanCore/Core/IPBan/IPBanService_Private.cs @@ -31,6 +31,7 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE using System.Linq; using System.Net; using System.Runtime.InteropServices; +using System.Security.Cryptography; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -205,7 +206,10 @@ private async Task SetNetworkInfo(CancellationToken cancelToken) } // request new config file - await GetUrl(UrlType.Config, cancelToken); + if (!string.IsNullOrWhiteSpace(Config.GetUrlConfig)) + { + await GetUrl(UrlType.Config, cancelToken); + } } private async Task ProcessPendingFailedLogins(IReadOnlyList ipAddresses, CancellationToken cancelToken) @@ -813,6 +817,17 @@ protected virtual void OnFirewallDisposing() { } /// Task protected virtual Task OnUpdate(CancellationToken cancelToken) => Task.CompletedTask; + /// + /// Launch the verified update binary. Virtual so tests can intercept the actual + /// process launch without having to spawn a real subprocess on the host. + /// + /// Path to the (already-written, hash-verified) update binary. + /// Command-line arguments to pass to the binary. + protected virtual void LaunchUpdateBinary(string tempFile, string args) + { + ProcessUtility.CreateDetachedProcess(tempFile, args); + } + /// /// Get url from config /// @@ -857,14 +872,45 @@ protected virtual async Task GetUrl(UrlType urlType, CancellationToken can // if the update url sends bytes, we assume a software update, and run the result as an .exe if (bytes.Length != 0) { - var tempFile = Path.Combine(TempFile.TempDirectory, "IPBanServiceUpdate.exe"); - File.WriteAllBytes(tempFile, bytes); - - // however you are doing the update, you must allow -c and -d parameters - // pass -c to tell the update executable to delete itself when done - // pass -d for a directory which tells the .exe where this service lives - string args = "-c \"-d=" + AppContext.BaseDirectory + "\""; - ProcessUtility.CreateDetachedProcess(tempFile, args); + // Verify the downloaded binary against an operator-configured SHA-256 + // hash before executing it. The auto-update channel runs the result as + // a privileged process (service account on Windows, root on Linux), so + // any attacker who can MITM this URL or influence the config-supplied + // GetUrlUpdate value would otherwise get arbitrary code execution. If + // no hash is configured the bytes are skipped — execution requires an + // explicit operator opt-in. + string expectedHash = (Config.GetUrlUpdateSha256 ?? string.Empty).Trim(); + if (string.IsNullOrWhiteSpace(expectedHash)) + { + Logger.Warn("Auto-update download from {0} skipped — no GetUrlUpdateSha256 " + + "is configured. Set GetUrlUpdateSha256 to the SHA-256 hex of the expected " + + "update binary to opt in to automatic execution.", url); + } + else + { + byte[] actualHashBytes = SHA256.HashData(bytes); + string actualHash = Convert.ToHexString(actualHashBytes); + if (!string.Equals(actualHash, expectedHash.Replace(" ", string.Empty), + StringComparison.OrdinalIgnoreCase)) + { + Logger.Error("Auto-update download from {0} REJECTED — hash mismatch. " + + "Expected {1}, got {2}. The binary will not be executed.", + url, expectedHash, actualHash); + } + else + { + var tempFile = Path.Combine(TempFile.TempDirectory, "IPBanServiceUpdate.exe"); + File.WriteAllBytes(tempFile, bytes); + + // however you are doing the update, you must allow -c and -d parameters + // pass -c to tell the update executable to delete itself when done + // pass -d for a directory which tells the .exe where this service lives + string args = "-c \"-d=" + AppContext.BaseDirectory + "\""; + Logger.Warn("Auto-update download from {0} verified (sha256 {1}); executing.", + url, actualHash); + LaunchUpdateBinary(tempFile, args); + } + } } } else if (urlType == UrlType.Config && bytes.Length != 0) @@ -883,7 +929,8 @@ protected virtual async Task GetUrl(UrlType urlType, CancellationToken can private async Task UpdateUpdaters(CancellationToken cancelToken) { // hit start url if first time, if not first time will be ignored - if (!(await GetUrl(UrlType.Start, cancelToken))) + if ((!string.IsNullOrWhiteSpace(Config.GetUrlStart) || !string.IsNullOrWhiteSpace(Config.GetUrlUpdate)) && + !(await GetUrl(UrlType.Start, cancelToken))) { // send update await GetUrl(UrlType.Update, cancelToken); diff --git a/IPBanCore/Core/Utility/AsyncQueue.cs b/IPBanCore/Core/Utility/AsyncQueue.cs index 04e14823..e66dc41b 100644 --- a/IPBanCore/Core/Utility/AsyncQueue.cs +++ b/IPBanCore/Core/Utility/AsyncQueue.cs @@ -1,4 +1,4 @@ -/* +/* MIT License Copyright (c) 2012-present Digital Ruby, LLC - https://ipban.com @@ -38,41 +38,74 @@ public class AsyncQueue : IDisposable { private readonly SemaphoreSlim locker = new(0); private readonly ConcurrentQueue queue = new(); + private int disposed; // 0 = live, 1 = disposed (Interlocked-managed) /// - /// Dispose + /// Whether has been called. Public for testability. + /// + public bool IsDisposed => Volatile.Read(ref disposed) != 0; + + /// + /// Dispose. Idempotent and safe to call concurrently with Enqueue/Dequeue — + /// post-dispose Enqueue calls become no-ops, post-dispose Dequeue returns (false, default). /// public void Dispose() { - GC.SuppressFinalize(this); - locker.Dispose(); + // Use Interlocked.Exchange so concurrent Dispose calls don't double-dispose the + // SemaphoreSlim (which would throw on the second call). + if (Interlocked.Exchange(ref disposed, 1) == 0) + { + GC.SuppressFinalize(this); + try { locker.Dispose(); } catch { /* tolerate races with concurrent dequeue */ } + } } /// - /// Add an item to the queue, causing TryDequeueAsync to return an item + /// Add an item to the queue, causing TryDequeueAsync to return an item. + /// No-op after Dispose. /// - /// - /// Task public ValueTask EnqueueAsync(T item) { + if (IsDisposed) + { + return new(); + } queue.Enqueue(item); - locker.Release(1); + try { locker.Release(1); } + catch (ObjectDisposedException) { /* raced with Dispose; the item is on the queue but no consumer can wake */ } return new(); } /// - /// Add many items to the queue, causing TryDequeueAsync to return for each item + /// Add many items to the queue, causing TryDequeueAsync to return for each item. /// - /// Source public ValueTask EnqueueRangeAsync(IEnumerable source) { + if (IsDisposed || source is null) + { + return new(); + } + // Track how many items we actually enqueued and release that many in a finally — if + // the source enumerator throws partway through, we still need the semaphore count to + // match the queue contents, otherwise dequeuers would either block forever or wake + // for items that aren't there. int count = 0; - foreach (var item in source) + try + { + foreach (var item in source) + { + queue.Enqueue(item); + count++; + } + } + finally { - queue.Enqueue(item); - count++; + if (count > 0) + { + try { locker.Release(count); } + catch (ObjectDisposedException) { /* raced with Dispose */ } + } } - locker.Release(count); return new(); } diff --git a/IPBanCore/Core/Utility/AsyncReaderWriterLock.cs b/IPBanCore/Core/Utility/AsyncReaderWriterLock.cs index 2b6a0f33..d14b6df1 100644 --- a/IPBanCore/Core/Utility/AsyncReaderWriterLock.cs +++ b/IPBanCore/Core/Utility/AsyncReaderWriterLock.cs @@ -156,13 +156,21 @@ private void ReleaseReaderLock() } } + private int disposed; // 0 = live, 1 = disposed (Interlocked-managed) + /// - /// Dispose of all resources + /// Dispose of all resources. Idempotent — safe to call multiple times. The Interlocked + /// gate prevents a second call from re-disposing already-disposed semaphores, and the + /// per-semaphore try/catch tolerates races with in-flight Wait/Release operations. /// public void Dispose() { - _writeSemaphore.Dispose(); - _readSemaphore.Dispose(); + if (Interlocked.Exchange(ref disposed, 1) != 0) + { + return; + } + try { _writeSemaphore.Dispose(); } catch { /* best effort */ } + try { _readSemaphore.Dispose(); } catch { /* best effort */ } } } } diff --git a/IPBanCore/Core/Utility/IOUtility.cs b/IPBanCore/Core/Utility/IOUtility.cs index 411b5ed0..d55e6771 100644 --- a/IPBanCore/Core/Utility/IOUtility.cs +++ b/IPBanCore/Core/Utility/IOUtility.cs @@ -22,7 +22,7 @@ public static string[] GetLines(string fileNameOrUrl, int maxBytes = 0) Uri.TryCreate(fileNameOrUrl, UriKind.Absolute, out var url)) { using var client = new System.Net.Http.HttpClient() { Timeout = TimeSpan.FromSeconds(10) }; - var response = client.GetAsync(url).Sync(); + using var response = client.GetAsync(url, System.Net.Http.HttpCompletionOption.ResponseHeadersRead).Sync(); if (response.IsSuccessStatusCode) { if (maxBytes > 0 && response.Content.Headers.ContentLength > maxBytes) diff --git a/IPBanCore/Core/Utility/IPAddressProcessExecutor.cs b/IPBanCore/Core/Utility/IPAddressProcessExecutor.cs index 576c8a09..1ed0620c 100644 --- a/IPBanCore/Core/Utility/IPAddressProcessExecutor.cs +++ b/IPBanCore/Core/Utility/IPAddressProcessExecutor.cs @@ -25,10 +25,104 @@ void Execute(string programToRun, IReadOnlyCollection ipAddre /// public sealed class IPAddressProcessExecutor : IIPAddressProcessExecutor { + /// + /// Tokenize an argument template into individual argv tokens, respecting double-quoted spans. + /// We tokenize the *template* (config-supplied) once, then replace placeholders inside each + /// token. This means an attacker-controlled username embedded as ###USERNAME### becomes a + /// single argv entry no matter what characters it contains — it can never inject extra args. + /// Public for testability; this is a pure function. + /// + public static string[] TokenizeArguments(string args) + { + if (string.IsNullOrEmpty(args)) + { + return Array.Empty(); + } + var result = new List(); + var current = new StringBuilder(); + bool inQuotes = false; + foreach (var c in args) + { + if (c == '"') + { + inQuotes = !inQuotes; + } + else if (char.IsWhiteSpace(c) && !inQuotes) + { + if (current.Length > 0) + { + result.Add(current.ToString()); + current.Clear(); + } + } + else + { + current.Append(c); + } + } + if (current.Length > 0) + { + result.Add(current.ToString()); + } + return [.. result]; + } + + /// + /// Sanitize log data so embedded quotes / newlines can't disrupt downstream consumers. + /// Public for testability — pure function. + /// + public static string CleanLogData(string raw) => + (raw ?? string.Empty) + .Replace("\"", string.Empty) + .Replace("'", string.Empty) + .Replace("\\", "/") + .Replace("\n", " ") + .Replace("\r", " ") + .Replace("\t", " ") + .Trim(); + + /// + /// Build a for the given IP/template combo. + /// Each pre-tokenized argument template element becomes one entry in ArgumentList after + /// placeholder substitution, so attacker-controlled values cannot inject extra argv slots. + /// Public for testability; this is a pure function. + /// + public static System.Diagnostics.ProcessStartInfo BuildStartInfo( + string programFullPath, string[] argTokens, IPAddressLogEvent ipAddress, string appName) + { + var psi = new System.Diagnostics.ProcessStartInfo + { + FileName = programFullPath, + WorkingDirectory = System.IO.Path.GetDirectoryName(programFullPath), + UseShellExecute = false, + }; + string logData = CleanLogData(ipAddress.LogData); + foreach (var token in argTokens) + { + // replacements happen *inside* the token so substituted values stay + // inside their own argv slot — argv injection is impossible. + psi.ArgumentList.Add(token + .Replace("###IPADDRESS###", ipAddress.IPAddress) + .Replace("###SOURCE###", ipAddress.Source ?? string.Empty) + .Replace("###USERNAME###", ipAddress.UserName ?? string.Empty) + .Replace("###APP###", appName) + .Replace("###COUNT###", ipAddress.Count.ToStringInvariant()) + .Replace("###LOG###", logData) + .Replace("###OSNAME###", OSUtility.Name) + .Replace("###OSVERSION###", OSUtility.Version)); + } + return psi; + } + /// public void Execute(string programToRun, IReadOnlyCollection ipAddresses, string appName, Action taskRunner) { + // Defensive snapshot. The taskRunner may execute the closure asynchronously, and the + // caller is allowed to mutate the original collection after Execute returns. Without + // a snapshot the task thread could enumerate a list while it's being modified. + var ipAddressSnapshot = ipAddresses.ToArray(); + foreach (string process in programToRun.Split('\n')) { string[] pieces = process.Trim().Split('|', StringSplitOptions.TrimEntries); @@ -44,40 +138,19 @@ public void Execute(string programToRun, IReadOnlyCollection string programFullPath = System.IO.Path.GetFullPath(pieces[0]); string programArgs = pieces[1]; - foreach (var ipAddress in ipAddresses) - { - // log data cleanup - var logData = (ipAddress.LogData ?? string.Empty) - .Replace("\"", string.Empty) - .Replace("'", string.Empty) - .Replace("\\", "/") - .Replace("\n", " ") - .Replace("\r", " ") - .Replace("\t", " ") - .Trim(); - - string replacedArgs = programArgs.Replace("###IPADDRESS###", ipAddress.IPAddress) - .Replace("###SOURCE###", ipAddress.Source ?? string.Empty) - .Replace("###USERNAME###", ipAddress.UserName ?? string.Empty) - .Replace("###APP###", appName) - .Replace("###COUNT###", ipAddress.Count.ToStringInvariant()) - .Replace("###LOG###", logData) - .Replace("###OSNAME###", OSUtility.Name) - .Replace("###OSVERSION###", OSUtility.Version); + // pre-tokenize the template once + string[] argTokens = TokenizeArguments(programArgs); + foreach (var ipAddress in ipAddressSnapshot) + { try { - System.Diagnostics.ProcessStartInfo psi = new() - { - FileName = programFullPath, - WorkingDirectory = System.IO.Path.GetDirectoryName(programFullPath), - Arguments = replacedArgs - }; + var psi = BuildStartInfo(programFullPath, argTokens, ipAddress, appName); using var p = System.Diagnostics.Process.Start(psi); } catch (Exception ex) { - Logger.Error(ex, "Failed to execute process {0} {1}", programFullPath, replacedArgs); + Logger.Error(ex, "Failed to execute process {0} {1}", programFullPath, programArgs); } } }); diff --git a/IPBanCore/Core/Utility/IPAddressRange.cs b/IPBanCore/Core/Utility/IPAddressRange.cs index 7a2a4d59..1df33baa 100644 --- a/IPBanCore/Core/Utility/IPAddressRange.cs +++ b/IPBanCore/Core/Utility/IPAddressRange.cs @@ -410,37 +410,37 @@ public bool Chomp(IPAddressRange range, out IPAddressRange left, out IPAddressRa } else if (cmpLeft > 0 && cmpRight < 0) { - // middle chomp, left and right will be set - if (!range.Begin.TryDecrement(out IPAddress end)) + // Middle chomp — both sides have remainder, except at the IP space boundaries + // (0.0.0.0 / ::, 255.255.255.255 / ffff:…) where decrement/increment legitimately + // has no representable result. In those cases the corresponding side stays null. + if (range.Begin.TryDecrement(out IPAddress end)) { - throw new ApplicationException("Unexpected failed decrement of " + range.Begin); + left = new IPAddressRange(Begin, end); } - left = new IPAddressRange(Begin, end); - if (!range.End.TryIncrement(out IPAddress start)) + if (range.End.TryIncrement(out IPAddress start)) { - throw new ApplicationException("Unexpected failed increment of " + range.End); + right = new IPAddressRange(start, End); } - right = new IPAddressRange(start, End); return true; } else if (cmpRight < 0 && range.End.CompareTo(Begin) >= 0) { // chomp with only right piece remaining - if (!range.End.TryIncrement(out IPAddress start)) + if (range.End.TryIncrement(out IPAddress start)) { - throw new ApplicationException("Unexpected failed increment of " + range.End); + right = new IPAddressRange(start, End); } - right = new IPAddressRange(start, End); + // if increment fails (range.End == max IP), there is no right piece — still a valid chomp return true; } else if (cmpLeft > 0 && range.Begin.CompareTo(End) <= 0) { // chomp with only left piece remaining - if (!range.Begin.TryDecrement(out IPAddress end)) + if (range.Begin.TryDecrement(out IPAddress end)) { - throw new ApplicationException("Unexpected failed decrement of " + range.Begin); + left = new IPAddressRange(Begin, end); } - left = new IPAddressRange(Begin, end); + // if decrement fails (range.Begin == 0.0.0.0 / ::), there is no left piece return true; } @@ -901,7 +901,7 @@ private bool TryGetValue(IEnumerable> items, string bool IReadOnlyDictionary.TryGetValue(string key, out string value) => TryGetValue(key, out value); /// - IEnumerator> IEnumerable>.GetEnumerator() => (IEnumerator>)GetDictionaryItems().GetEnumerator(); + IEnumerator> IEnumerable>.GetEnumerator() => ((IEnumerable>)GetDictionaryItems()).GetEnumerator(); /// /// Compare to another ip address range diff --git a/IPBanCore/Core/Utility/JsonSerializationHelper.cs b/IPBanCore/Core/Utility/JsonSerializationHelper.cs index 50862859..fee27c15 100644 --- a/IPBanCore/Core/Utility/JsonSerializationHelper.cs +++ b/IPBanCore/Core/Utility/JsonSerializationHelper.cs @@ -4,6 +4,7 @@ using System.Text; using System.Text.Json; using System.Text.Json.Serialization; +using System.Diagnostics.CodeAnalysis; using System.Threading; using System.Threading.Tasks; @@ -27,6 +28,7 @@ public static class JsonSerializationHelper /// Type /// Json text /// Object + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "IPBanCore controls serialized models for these paths.")] public static T Deserialize(string json) => JsonSerializer.Deserialize(json, Options)!; /// @@ -35,6 +37,7 @@ public static class JsonSerializationHelper /// Type /// Object /// Json text + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "IPBanCore controls serialized models for these paths.")] public static string Serialize(T obj) => JsonSerializer.Serialize(obj, Options); /// @@ -44,6 +47,7 @@ public static class JsonSerializationHelper /// Stream /// Object /// Stream is null + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "IPBanCore controls serialized models for these paths.")] public static T Deserialize(Stream stream) { ArgumentNullException.ThrowIfNull(stream); @@ -57,6 +61,7 @@ public static T Deserialize(Stream stream) /// Type /// Object /// Stream is null + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "Type is explicitly provided by caller and controlled by IPBanCore.")] public static object Deserialize(Stream stream, Type type) { ArgumentNullException.ThrowIfNull(stream); @@ -70,6 +75,7 @@ public static object Deserialize(Stream stream, Type type) /// Object /// Stream /// Stream is null + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "IPBanCore controls serialized models for these paths.")] public static void Serialize(T obj, Stream stream) { ArgumentNullException.ThrowIfNull(stream); @@ -84,6 +90,7 @@ public static void Serialize(T obj, Stream stream) /// Cancel token /// /// Stream is null + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "IPBanCore controls serialized models for these paths.")] public static async Task DeserializeAsync(Stream stream, CancellationToken cancel = default) { ArgumentNullException.ThrowIfNull(stream); @@ -100,6 +107,7 @@ public static async Task DeserializeAsync(Stream stream, CancellationToken /// Cancel token /// Task /// Stream is null + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "IPBanCore controls serialized models for these paths.")] public static Task SerializeAsync(T obj, Stream stream, CancellationToken cancel = default) { ArgumentNullException.ThrowIfNull(stream); diff --git a/IPBanCore/Core/Utility/LevenshteinUnsafe.cs b/IPBanCore/Core/Utility/LevenshteinUnsafe.cs index ab4236c4..dc0865b5 100644 --- a/IPBanCore/Core/Utility/LevenshteinUnsafe.cs +++ b/IPBanCore/Core/Utility/LevenshteinUnsafe.cs @@ -31,16 +31,35 @@ namespace DigitalRuby.IPBanCore public static class LevenshteinUnsafe { /// - /// Compares the two values to find the minimum Levenshtein distance. + /// Maximum input length accepted for either string. Inputs longer than this are rejected + /// to prevent unbounded stackalloc / CPU consumption on attacker-controlled values. + /// + public const int MaxInputLength = 1024; + + /// + /// Threshold (in elements) below which we use stackalloc; at or above this we fall back + /// to a heap-allocated buffer so we never blow the stack. + /// + private const int StackAllocThreshold = 256; + + /// + /// Compares the two values to find the minimum Levenshtein distance. /// Thread safe. /// - /// Difference. 0 complete match. -1 if either value1 or value2 is null, -2 if value2 is too large. + /// Difference. 0 complete match. -1 if either value1 or value2 is null, -2 if either value is too large. public static unsafe int Distance(string value1, string value2) { if (value1 is null || value2 is null) { return -1; } + else if (value1.Length > MaxInputLength || value2.Length > MaxInputLength) + { + // Bound the input length. The inner loop allocates `int[value2.Length]` either + // on the stack or the heap; an attacker-supplied string longer than this cap + // would either blow the stack (uncatchable in .NET) or chew memory. + return -2; + } else if (value2.Length == 0) { return value1.Length; @@ -50,7 +69,25 @@ public static unsafe int Distance(string value1, string value2) return value2.Length; } - int* costs = stackalloc int[value2.Length]; + // small inputs go on the stack (fast path); larger inputs use a heap buffer pinned + // for the duration of the call so the unsafe pointer math stays valid. + if (value2.Length < StackAllocThreshold) + { + int* costs = stackalloc int[value2.Length]; + return DistanceCore(value1, value2, costs); + } + else + { + int[] heapCosts = new int[value2.Length]; + fixed (int* costs = heapCosts) + { + return DistanceCore(value1, value2, costs); + } + } + } + + private static unsafe int DistanceCore(string value1, string value2, int* costs) + { int* costsEnd = costs + value2.Length, ptr; int i = 0, j, insertionCost, cost, additionCost; char value1Char; diff --git a/IPBanCore/Core/Utility/LockedEnumerable.cs b/IPBanCore/Core/Utility/LockedEnumerable.cs index dac3e79b..e205e0ce 100644 --- a/IPBanCore/Core/Utility/LockedEnumerable.cs +++ b/IPBanCore/Core/Utility/LockedEnumerable.cs @@ -36,6 +36,7 @@ public class LockedEnumerable : IEnumerator { private readonly SemaphoreSlim locker = new(1, 1); private readonly IEnumerator e; + private int disposed; // Interlocked-managed; 0 = live, 1 = disposed /// /// Constructor @@ -45,7 +46,19 @@ public LockedEnumerable(IEnumerable obj) { obj.ThrowIfNull(); locker.Wait(); - e = obj.GetEnumerator(); + try + { + // We've already taken the semaphore — if GetEnumerator throws we must release + // it on the way out, otherwise the LockedEnumerable becomes GC-eligible while + // still holding the lock and any future caller deadlocks waiting on it. + e = obj.GetEnumerator(); + } + catch + { + locker.Release(); + locker.Dispose(); + throw; + } } /// @@ -57,8 +70,17 @@ public LockedEnumerable(IEnumerable obj) /// public void Dispose() { + // Idempotent — Release+Dispose each underlying resource exactly once. The + // SemaphoreSlim must be both released (to allow re-entry on the same instance, + // though we don't use it that way) and disposed (we own it and need to free it). + if (Interlocked.Exchange(ref disposed, 1) != 0) + { + return; + } GC.SuppressFinalize(this); - locker.Release(); + try { e?.Dispose(); } catch { /* best effort */ } + try { locker.Release(); } catch { /* tolerate already-released semaphore */ } + try { locker.Dispose(); } catch { /* tolerate already-disposed */ } } /// diff --git a/IPBanCore/Core/Utility/LogFileScanner.cs b/IPBanCore/Core/Utility/LogFileScanner.cs index a5dabe9c..e05f694e 100644 --- a/IPBanCore/Core/Utility/LogFileScanner.cs +++ b/IPBanCore/Core/Utility/LogFileScanner.cs @@ -189,6 +189,10 @@ public static IReadOnlyCollection GetFiles(string pathAndMask, bool // pull out the directory portion of the path/mask, accounting for * syntax in the folder name string replacedPathAndMask = ReplacePathVars(pathAndMask); NormalizeGlob(replacedPathAndMask, out string dirPortion, out string globPortion); + if (!string.IsNullOrWhiteSpace(dirPortion) && !Directory.Exists(dirPortion)) + { + return files; + } // create a matcher to match glob or regular file syntax Matcher fileMatcher = new Matcher(StringComparison.OrdinalIgnoreCase).AddInclude(globPortion); diff --git a/IPBanCore/Core/Utility/Logger.cs b/IPBanCore/Core/Utility/Logger.cs index e91aa97a..1ba3d907 100644 --- a/IPBanCore/Core/Utility/Logger.cs +++ b/IPBanCore/Core/Utility/Logger.cs @@ -256,7 +256,7 @@ public static void ResetConfigFile() #endif - Console.WriteLine("Creating default nlog.config file"); + Console.WriteLine("Creating default nlog.config file. Check logfile.txt for output."); // storing this as a resource fails to use correct string in precompiled .exe with .net core, bug with Microsoft I think string defaultNLogConfig = $@" diff --git a/IPBanCore/Core/Utility/OSUtility.cs b/IPBanCore/Core/Utility/OSUtility.cs index e687ce2d..0e590231 100644 --- a/IPBanCore/Core/Utility/OSUtility.cs +++ b/IPBanCore/Core/Utility/OSUtility.cs @@ -179,13 +179,12 @@ public static IReadOnlyDictionary GetUserAccounts() private static readonly Lock locker = new(); - private static PerformanceCounter windowsCpuCounter; private static PerformanceCounter windowsDiskIopsCounter; - //private static Process linuxCpuCounter; - //private static Process linuxDiskIopsCounter; - private static float windowsCpuPercent; private static float windowsDiskIopsPercent; private static float networkUsage = -1.0f; + private static long cpuIdleTime; + private static long cpuTotalTime; + private static int windowsDiskIopsCounterStarted; private static bool isWindows; private static bool isLinux; @@ -405,6 +404,19 @@ private static void PopulateUsersLinux(Dictionary newUsers) /// String public static string OSInfo => $"Name: {Name}, Version: {Version}, Friendly Name: {FriendlyName}, Description: {Description}"; + internal static string CreateFullyQualifiedDomainName(string hostName, string domainName, string fallbackServerName) + { + string fqdn = (string.IsNullOrWhiteSpace(hostName) ? fallbackServerName : hostName)?.Trim().TrimEnd('.') ?? string.Empty; + domainName = domainName?.Trim().Trim('.'); + if (!string.IsNullOrWhiteSpace(domainName) && + !fqdn.Equals(domainName, StringComparison.OrdinalIgnoreCase) && + !fqdn.EndsWith("." + domainName, StringComparison.OrdinalIgnoreCase)) + { + fqdn += "." + domainName; + } + return string.IsNullOrWhiteSpace(fqdn) ? fallbackServerName : fqdn; + } + private static string fqdn; /// /// Fully qualified domain name @@ -431,12 +443,7 @@ public static string FQDN } try { - fqdn = System.Net.Dns.GetHostName(); - if (!string.IsNullOrWhiteSpace(domainName) && - !fqdn.StartsWith(domainName + ".", StringComparison.OrdinalIgnoreCase)) - { - fqdn = domainName + "." + fqdn; - } + fqdn = CreateFullyQualifiedDomainName(System.Net.Dns.GetHostName(), domainName, serverName); } catch { @@ -472,6 +479,22 @@ public MEMORYSTATUSEX() [DllImport("kernel32.dll", CharSet = CharSet.Auto, SetLastError = true)] private static extern bool GlobalMemoryStatusEx([In, Out] ref MEMORYSTATUSEX lpBuffer); + [StructLayout(LayoutKind.Sequential)] + private readonly struct SystemFileTime + { + private readonly uint lowDateTime; + private readonly uint highDateTime; + + public long ToInt64() + { + return ((long)highDateTime << 32) | lowDateTime; + } + } + + [return: MarshalAs(UnmanagedType.Bool)] + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool GetSystemTimes(out SystemFileTime idleTime, out SystemFileTime kernelTime, out SystemFileTime userTime); + /// /// Get memory usage /// @@ -550,9 +573,7 @@ public static bool GetDiskUsage(out long totalStorage, out long availableStorage } /// - /// Attempt to determine current cpu usage. For Linux, mpstat must be installed first. - /// Ubuntu/Debian: apt install -y sysstat - /// Rest: yum install -y sysstat + /// Attempt to determine current cpu usage. /// /// Percent of all cpu resources currently in use, 0.0-1.0 /// True if cpu usage could be determined, false otherwise @@ -561,62 +582,85 @@ public static bool GetCpuUsage(out float percentUsed) percentUsed = 0.0f; if (isWindows) { - if (windowsCpuCounter is null) + if (GetSystemTimes(out SystemFileTime idleTime, out SystemFileTime kernelTime, out SystemFileTime userTime)) + { + long idle = idleTime.ToInt64(); + long total = kernelTime.ToInt64() + userTime.ToInt64(); + return TryCalculateCpuUsage(idle, total, out percentUsed); + } + } + else if (isLinux) + { + return TryGetLinuxCpuUsage(out percentUsed); + } + return false; + } + + private static bool TryGetLinuxCpuUsage(out float percentUsed) + { + percentUsed = 0.0f; + try + { + // /proc/stat cpu fields: + // cpu user nice system idle iowait irq softirq steal guest guest_nice + // Example: + // cpu 7705 0 6085 8068477 2224 0 444 0 0 0 + string line = File.ReadLines("/proc/stat").FirstOrDefault(x => x.StartsWith("cpu ", StringComparison.Ordinal)); + if (string.IsNullOrWhiteSpace(line)) + { + return false; + } + + string[] pieces = line.Split(' ', StringSplitOptions.RemoveEmptyEntries); + if (pieces.Length < 5) { - lock (locker) + return false; + } + + long idle = 0L; + long total = 0L; + for (int i = 1; i < pieces.Length; i++) + { + if (!long.TryParse(pieces[i], NumberStyles.Integer, CultureInfo.InvariantCulture, out long value)) { - if (windowsCpuCounter is null) - { - windowsCpuCounter = new PerformanceCounter("Processor Information", "% Processor Utility", "_Total"); + return false; + } - // capture windows cpu in background - System.Threading.Tasks.Task.Run(async () => - { - windowsCpuPercent = Math.Clamp(windowsCpuCounter.NextValue() * 0.01f, 0.0f, 1.0f); - while (!Environment.HasShutdownStarted) - { - await System.Threading.Tasks.Task.Delay(1000); - windowsCpuPercent = Math.Clamp(windowsCpuCounter.NextValue() * 0.01f, 0.0f, 1.0f); - } - }); - } + total += value; + if (i == 4 || i == 5) + { + idle += value; } } - percentUsed = windowsCpuPercent; - //string output = StartProcessAndWait(60000, "wmic", "cpu get loadpercentage", out _, LogLevel.Trace); - //string[] lines = output.Split('\n', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); - //if (lines.Length > 1 && float.TryParse(lines[^1], NumberStyles.None, CultureInfo.InvariantCulture, out percentUsed)) - //{ - //percentUsed *= 0.01f; - //return true; - //} - return true; + return TryCalculateCpuUsage(idle, total, out percentUsed); } - else if (isLinux) + catch { - // Linux 5.10.102.1-microsoft-standard-WSL2 (MACHINE_NAME) 04/24/22 _x86_64_ (24 CPU) - // - // 14:19:43 CPU %usr %nice %sys %iowait %irq %soft %steal %guest %gnice %idle - // 14:19:43 all 0.00 0.00 0.02 0.00 0.00 0.00 0.00 0.00 0.00 99.97 - string output = StartProcessAndWait(60000, "mpstat", "1 1", out _, LogLevel.Trace); - string[] lines = output.Split('\n', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); - if (lines.Length > 1) + return false; + } + } + + private static bool TryCalculateCpuUsage(long idle, long total, out float percentUsed) + { + percentUsed = 0.0f; + if (total <= 0L || idle < 0L) + { + return false; + } + + lock (locker) + { + long idleDelta = (cpuIdleTime == 0L ? idle : idle - cpuIdleTime); + long totalDelta = (cpuTotalTime == 0L ? total : total - cpuTotalTime); + cpuIdleTime = idle; + cpuTotalTime = total; + if (totalDelta > 0L) { - string lastLine = lines[^1]; - int pos = lastLine.LastIndexOf(' '); - if (pos > 0) - { - string piece = lastLine[pos..].Trim(); - if (pos > 0 && float.TryParse(piece, NumberStyles.AllowDecimalPoint, CultureInfo.InvariantCulture, out percentUsed)) - { - percentUsed = Math.Clamp(1.0f - (0.01f * percentUsed), 0.0f, 1.0f); - return true; - } - } + percentUsed = Math.Clamp(1.0f - ((float)idleDelta / totalDelta), 0.0f, 1.0f); } } - return false; + return true; } /// @@ -698,26 +742,27 @@ public static bool GetDiskIopsUsage(out float percentUsed) percentUsed = 0.0f; if (isWindows) { - if (windowsDiskIopsCounter is null) + if (Interlocked.CompareExchange(ref windowsDiskIopsCounterStarted, 1, 0) == 0) { - lock (locker) + // PerformanceCounter construction can block for seconds on some hosts. + // Keep the first call non-blocking and let the sampler populate the cache. + System.Threading.Tasks.Task.Run(async () => { - if (windowsDiskIopsCounter is null) + try { windowsDiskIopsCounter = new PerformanceCounter("PhysicalDisk", "% Disk Time", "_Total"); - - // capture disk io in background - System.Threading.Tasks.Task.Run(async () => + windowsDiskIopsPercent = Math.Clamp(windowsDiskIopsCounter.NextValue() * 0.01f, 0.0f, 1.0f); + while (!Environment.HasShutdownStarted) { + await System.Threading.Tasks.Task.Delay(1000); windowsDiskIopsPercent = Math.Clamp(windowsDiskIopsCounter.NextValue() * 0.01f, 0.0f, 1.0f); - while (!Environment.HasShutdownStarted) - { - await System.Threading.Tasks.Task.Delay(1000); - windowsDiskIopsPercent = Math.Clamp(windowsDiskIopsCounter.NextValue() * 0.01f, 0.0f, 1.0f); - } - }); + } } - } + catch + { + Interlocked.Exchange(ref windowsDiskIopsCounterStarted, 0); + } + }); } percentUsed = windowsDiskIopsPercent; @@ -853,20 +898,20 @@ public static string StartProcessAndWait(int timeoutMilliseconds, string program })); thread.Start(); int timeout = (timeoutMilliseconds < 1 ? Timeout.Infinite : timeoutMilliseconds + 5000); + if (!thread.Join(timeout)) + { + throw new ApplicationException("Timed out waiting for process result"); + } exitCode = _exitCode; if (_ex is not null) { throw _ex; } - else if (!thread.Join(timeout)) - { - throw new ApplicationException("Timed out waiting for process result"); - } return output.ToString(); } private static Dictionary users = new(StringComparer.OrdinalIgnoreCase); // user name, enabled - private static DateTime usersExpire = IPBanService.UtcNow; + private static DateTime usersExpire = DateTime.MinValue; /// /// The amount of time to cache the users found in the method. diff --git a/IPBanCore/Core/Utility/ProcessUtility.cs b/IPBanCore/Core/Utility/ProcessUtility.cs index cc887d3f..b64d323b 100644 --- a/IPBanCore/Core/Utility/ProcessUtility.cs +++ b/IPBanCore/Core/Utility/ProcessUtility.cs @@ -203,21 +203,80 @@ public static void CreateDetachedProcess(string fileName, string arguments) } else { - // ensure process is executable - OSUtility.StartProcessAndWait("sudo", "chmod +x \"" + fileName + "\""); + // chmod via ArgumentList — no shell, so fileName goes through as a single argv + // slot regardless of what characters it contains. + var chmod = new ProcessStartInfo + { + FileName = "sudo", + UseShellExecute = false, + }; + chmod.ArgumentList.Add("chmod"); + chmod.ArgumentList.Add("+x"); + chmod.ArgumentList.Add(fileName); + using (var p = Process.Start(chmod)) + { + p?.WaitForExit(); + } + + // Pipe the launch command into `at` via stdin. Without `-f`, at reads its job + // body from stdin and copies it into the at spool — no temp file is written and + // nothing needs cleanup. + string commandBody = BuildAtJobBody(fileName, arguments); - // use Linux at, should have been installed earlier - ProcessStartInfo info = new() + ProcessStartInfo atInfo = new() { - Arguments = "-c \"echo sudo \\\"" + fileName + "\\\" " + arguments.Replace("\"", "\\\"") + " | at now\"", + FileName = "at", CreateNoWindow = true, - FileName = "/bin/bash", UseShellExecute = false, + RedirectStandardInput = true, WindowStyle = ProcessWindowStyle.Hidden, WorkingDirectory = Path.GetDirectoryName(fileName) }; - using var detachedProcess = Process.Start(info); + atInfo.ArgumentList.Add("now"); + using var detachedProcess = Process.Start(atInfo); + if (detachedProcess is not null) + { + detachedProcess.StandardInput.Write(commandBody); + detachedProcess.StandardInput.Close(); + } + } + } + + /// + /// Build the shell command body that gets piped into `at` (or any compatible scheduler) + /// to launch via sudo. fileName is wrapped in single quotes + /// so any spaces, semicolons, backticks, or `$()` in the path are inert literals when + /// /bin/sh later runs the spooled job. Public for testability — pure function with no + /// side effects, safe to call cross-platform. + /// + public static string BuildAtJobBody(string fileName, string arguments) + { + var command = new StringBuilder(); + command.Append("sudo ").Append(BashEscape(fileName)); + if (!string.IsNullOrEmpty(arguments)) + { + // arguments is operator-supplied (a shell-formed argument string by contract). + // If callers ever start passing user-influenced data, switch to tokenized + // arguments and BashEscape each one. + command.Append(' ').Append(arguments); + } + command.Append('\n'); + return command.ToString(); + } + + /// + /// Shell-escape a value for safe inclusion inside a single-quoted bash string. + /// Wraps the value in single quotes and replaces any embedded single quote with '\'' + /// (close-quote, escaped quote, reopen-quote) — the canonical bash idiom. + /// Public for testability; this is a pure function. + /// + public static string BashEscape(string value) + { + if (value is null) + { + return "''"; } + return "'" + value.Replace("'", "'\\''") + "'"; } } } diff --git a/IPBanCore/Core/Utility/TempFile.cs b/IPBanCore/Core/Utility/TempFile.cs index 2282fa15..288f7427 100644 --- a/IPBanCore/Core/Utility/TempFile.cs +++ b/IPBanCore/Core/Utility/TempFile.cs @@ -30,7 +30,11 @@ static TempFile() } } - var exeLoc = System.Reflection.Assembly.GetEntryAssembly()?.Location ?? Guid.NewGuid().ToString("N"); + var exeLoc = AppContext.BaseDirectory; + if (string.IsNullOrWhiteSpace(exeLoc)) + { + exeLoc = Guid.NewGuid().ToString("N"); + } // generate sub dir name from exeLoc using hash var hashBytes = MD5.HashData(System.Text.Encoding.UTF8.GetBytes(exeLoc)); diff --git a/IPBanCore/Core/Utility/XmlCData.cs b/IPBanCore/Core/Utility/XmlCData.cs index 7d3e2822..3394ae1a 100644 --- a/IPBanCore/Core/Utility/XmlCData.cs +++ b/IPBanCore/Core/Utility/XmlCData.cs @@ -91,7 +91,7 @@ public System.Xml.Schema.XmlSchema GetSchema() /// Reader public void ReadXml(System.Xml.XmlReader reader) { - value = reader.ReadElementString(); + value = (reader.ReadElementString() ?? string.Empty).Trim(); } /// @@ -106,7 +106,7 @@ public void WriteXml(System.Xml.XmlWriter writer) } else { - writer.WriteCData("\n" + value + "\n"); + writer.WriteCData(value); } } } diff --git a/IPBanCore/IPBanCore.csproj b/IPBanCore/IPBanCore.csproj index 35a5c235..d91d49f3 100644 --- a/IPBanCore/IPBanCore.csproj +++ b/IPBanCore/IPBanCore.csproj @@ -43,7 +43,7 @@ - + diff --git a/IPBanCore/IPBanResources.Designer.cs b/IPBanCore/IPBanResources.Designer.cs index 80fddc46..c02cf7d5 100644 --- a/IPBanCore/IPBanResources.Designer.cs +++ b/IPBanCore/IPBanResources.Designer.cs @@ -19,7 +19,7 @@ namespace DigitalRuby.IPBanCore { // class via a tool like ResGen or Visual Studio. // To add or remove a member, edit your .ResX file then rerun ResGen // with the /str option, or rebuild your VS project. - [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "17.0.0.0")] + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "18.0.0.0")] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] public class IPBanResources { @@ -78,6 +78,15 @@ public static string AccessDuration { } } + /// + /// Looks up a localized string similar to Activity. + /// + public static string Activity { + get { + return ResourceManager.GetString("Activity", resourceCulture); + } + } + /// /// Looks up a localized string similar to Add. /// @@ -221,6 +230,24 @@ public static string AsnBlacklistWhitelist { return ResourceManager.GetString("AsnBlacklistWhitelist", resourceCulture); } } + + /// + /// Looks up a localized string similar to Restrict firewall monitor to IPBan Pro rules. + /// + public static string RestrictToIPBanProEvents { + get { + return ResourceManager.GetString("RestrictToIPBanProEvents", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to When checked, only IPBan Pro rules are reported. When off, all firewall rule events are reported, which can greatly increase network traffic and events to the web admin. + /// + public static string RestrictToIPBanProEventsTooltip { + get { + return ResourceManager.GetString("RestrictToIPBanProEventsTooltip", resourceCulture); + } + } /// /// Looks up a localized string similar to Whitelist instead of blacklist. diff --git a/IPBanCore/IPBanResources.resx b/IPBanCore/IPBanResources.resx index a592b756..89335a0d 100644 --- a/IPBanCore/IPBanResources.resx +++ b/IPBanCore/IPBanResources.resx @@ -1180,6 +1180,12 @@ For block rules, port list is ignored ports, so if you wanted to block port 3389 Whitelist instead of blacklist + + Restrict firewall monitor to IPBan Pro rules + + + When checked, only IPBan Pro rules are reported. When off, all firewall rule events are reported, which can greatly increase network traffic and events to the web admin. + BlacklisterWhitelister @@ -1201,4 +1207,7 @@ For block rules, port list is ignored ports, so if you wanted to block port 3389 Reset Banned IPs + + Activity + \ No newline at end of file diff --git a/IPBanCore/Linux/IPBanLinuxBaseFirewallIPTables.cs b/IPBanCore/Linux/IPBanLinuxBaseFirewallIPTables.cs index a82a26e4..e23a0850 100644 --- a/IPBanCore/Linux/IPBanLinuxBaseFirewallIPTables.cs +++ b/IPBanCore/Linux/IPBanLinuxBaseFirewallIPTables.cs @@ -416,7 +416,13 @@ public override Task Update(CancellationToken cancelToken) /// public override IEnumerable GetRuleNames(string ruleNamePrefix = null) { - string prefix = " " + matchSetDirectiveNoHyphens + " " + RulePrefix + (ruleNamePrefix ?? string.Empty); + // prepend RulePrefix for a bare prefix (e.g. "MyRule"), but leave an already-qualified + // prefix (e.g. RulePrefix + "EXTRA_") alone so it is not doubled up + if (!string.IsNullOrWhiteSpace(ruleNamePrefix) && !ruleNamePrefix.StartsWith(RulePrefix, StringComparison.OrdinalIgnoreCase)) + { + ruleNamePrefix = RulePrefix + ruleNamePrefix; + } + string prefix = " " + matchSetDirectiveNoHyphens + " " + (ruleNamePrefix ?? RulePrefix); using var tmp = new TempFile(); IPBanFirewallUtility.RunProcess(IpTablesProcess, null, tmp, "-L", "-n"); using var reader = new StreamReader(tmp); diff --git a/IPBanCore/Linux/IPBanLinuxFirewallD.cs b/IPBanCore/Linux/IPBanLinuxFirewallD.cs index a6ee4a74..b53645f5 100644 --- a/IPBanCore/Linux/IPBanLinuxFirewallD.cs +++ b/IPBanCore/Linux/IPBanLinuxFirewallD.cs @@ -302,6 +302,12 @@ public override string GetPorts(string ruleName) /// public override IEnumerable GetRuleNames(string ruleNamePrefix = null) { + // a caller passing a bare prefix (e.g. "MyRule") means the rule named RulePrefix + "MyRule"; + // prepend RulePrefix to match the Windows firewall behavior, but leave already-qualified prefixes alone + if (!string.IsNullOrWhiteSpace(ruleNamePrefix) && !ruleNamePrefix.StartsWith(RulePrefix)) + { + ruleNamePrefix = RulePrefix + ruleNamePrefix; + } var rules = IPBanLinuxIPSetFirewallD.GetSetNames(ruleNamePrefix ?? RulePrefix); return rules; } diff --git a/IPBanCore/Linux/NetFilterJsonConverters.cs b/IPBanCore/Linux/NetFilterJsonConverters.cs index 9e878cd2..5a08ebf5 100644 --- a/IPBanCore/Linux/NetFilterJsonConverters.cs +++ b/IPBanCore/Linux/NetFilterJsonConverters.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Text.Json; using System.Text.Json.Serialization; @@ -16,6 +17,7 @@ internal sealed class JsonStringEnumLowercaseConverter : JsonConverterFactory public override bool CanConvert(Type typeToConvert) => typeToConvert.IsEnum; /// + [UnconditionalSuppressMessage("Trimming", "IL2071", Justification = "Enum converter type is constructed dynamically from the runtime enum type.")] public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options) { var convType = typeof(LowercaseEnumConverter<>).MakeGenericType(typeToConvert); @@ -123,6 +125,7 @@ internal sealed class JsonStringEnumFlagsLowercaseConverter : JsonConverterFacto public override bool CanConvert(Type typeToConvert) => typeToConvert.IsEnum; /// + [UnconditionalSuppressMessage("Trimming", "IL2071", Justification = "Flags enum converter type is constructed dynamically from the runtime enum type.")] public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options) { var convType = typeof(FlagsLowercaseEnumConverter<>).MakeGenericType(typeToConvert); diff --git a/IPBanCore/Linux/NetFilterRuleset.cs b/IPBanCore/Linux/NetFilterRuleset.cs index 57829a81..2ff0c099 100644 --- a/IPBanCore/Linux/NetFilterRuleset.cs +++ b/IPBanCore/Linux/NetFilterRuleset.cs @@ -341,6 +341,7 @@ void IJsonOnDeserialized.OnDeserialized() } /// + [System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "NetFilter model graph is explicitly defined in this assembly.")] public override string ToString() { return System.Text.Json.JsonSerializer.Serialize(this, options: serializeOptions); @@ -402,6 +403,7 @@ private void LinkSetsAndRules() public sealed class NetFilterEntry { /// + [System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "NetFilter model graph is explicitly defined in this assembly.")] public override string ToString() { return System.Text.Json.JsonSerializer.Serialize(this, options: NetFilterRuleset.serializeOptions); @@ -439,6 +441,7 @@ public override string ToString() public sealed class NetFilterMeta { /// + [System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "NetFilter model graph is explicitly defined in this assembly.")] public override string ToString() { return System.Text.Json.JsonSerializer.Serialize(this, options: NetFilterRuleset.serializeOptions); @@ -466,6 +469,7 @@ public override string ToString() public sealed class NetFilterTable { /// + [System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "NetFilter model graph is explicitly defined in this assembly.")] public override string ToString() { return System.Text.Json.JsonSerializer.Serialize(this, options: NetFilterRuleset.serializeOptions); @@ -493,6 +497,7 @@ public override string ToString() public sealed class NetFilterChain { /// + [System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "NetFilter model graph is explicitly defined in this assembly.")] public override string ToString() { return System.Text.Json.JsonSerializer.Serialize(this, options: NetFilterRuleset.serializeOptions); @@ -545,6 +550,7 @@ public override string ToString() public sealed class NetFilterRule { /// + [System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "NetFilter model graph is explicitly defined in this assembly.")] public override string ToString() { return System.Text.Json.JsonSerializer.Serialize(this, options: NetFilterRuleset.serializeOptions); @@ -711,6 +717,7 @@ public bool Allow public sealed class NetFilterSet { /// + [System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "NetFilter model graph is explicitly defined in this assembly.")] public override string ToString() { return System.Text.Json.JsonSerializer.Serialize(this, options: NetFilterRuleset.serializeOptions); diff --git a/IPBanCore/Windows/COM/ComHelper.cs b/IPBanCore/Windows/COM/ComHelper.cs index fdaa6b6b..843b0899 100644 --- a/IPBanCore/Windows/COM/ComHelper.cs +++ b/IPBanCore/Windows/COM/ComHelper.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics.CodeAnalysis; using System.Runtime.InteropServices; #pragma warning disable IL2072 @@ -9,6 +10,7 @@ namespace DigitalRuby.IPBanCore.Windows.COM internal static class ComHelper { [System.Diagnostics.CodeAnalysis.SuppressMessage("Interoperability", "CA1416:Validate platform compatibility", Justification = "jjxtra")] + [UnconditionalSuppressMessage("Trimming", "IL2072", Justification = "COM activation is dynamic by design and resolved via COM registration at runtime.")] public static T CreateInstance() { if (!IsSupported()) diff --git a/IPBanCore/Windows/IPBanWindowsFirewall.cs b/IPBanCore/Windows/IPBanWindowsFirewall.cs index 9d67ce50..0c10f4dd 100644 --- a/IPBanCore/Windows/IPBanWindowsFirewall.cs +++ b/IPBanCore/Windows/IPBanWindowsFirewall.cs @@ -60,14 +60,86 @@ public class IPBanWindowsFirewall : IPBanBaseFirewall // DO NOT CHANGE THESE CONST AND READONLY FIELDS! *********************************************************************************** private const string clsidFwPolicy2 = "{E2B3C97F-6AE1-41AC-817A-F6F92166D7DD}"; private const string clsidFwRule = "{2C5BC43E-3369-4C33-AB0C-BE9469677AF4}"; + private static readonly INetFwPolicy2 policy = CreateComPolicy(); + private static readonly INetFwMgr manager = CreateComManager(); + + [UnconditionalSuppressMessage("Trimming", "IL2072", Justification = "Windows COM firewall policy activation is runtime COM-based.")] [System.Diagnostics.CodeAnalysis.SuppressMessage("Interoperability", "CA1416:Validate platform compatibility", Justification = "jjxtra")] - private static readonly INetFwPolicy2 policy = Activator.CreateInstance(Type.GetTypeFromCLSID(new Guid(clsidFwPolicy2))) as INetFwPolicy2; + private static INetFwPolicy2 CreateComPolicy() => + Activator.CreateInstance(Type.GetTypeFromCLSID(new Guid(clsidFwPolicy2))) as INetFwPolicy2; + + [UnconditionalSuppressMessage("Trimming", "IL2072", Justification = "Windows COM firewall manager activation is runtime COM-based.")] [System.Diagnostics.CodeAnalysis.SuppressMessage("Interoperability", "CA1416:Validate platform compatibility", Justification = "jjxtra")] - private static readonly INetFwMgr manager = (INetFwMgr)Activator.CreateInstance(Type.GetTypeFromProgID("HNetCfg.FwMgr")); - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] + private static INetFwMgr CreateComManager() => + (INetFwMgr)Activator.CreateInstance(Type.GetTypeFromProgID("HNetCfg.FwMgr")); + + [UnconditionalSuppressMessage("Trimming", "IL2072", Justification = "COM type resolved by CLSID at runtime; parameterless constructor is guaranteed by COM registration.")] [System.Diagnostics.CodeAnalysis.SuppressMessage("Interoperability", "CA1416:Validate platform compatibility", Justification = "jjxtra")] - private static readonly Type ruleType = Type.GetTypeFromCLSID(new Guid(clsidFwRule)); + private static INetFwRule CreateComFwRule() => + Activator.CreateInstance(Type.GetTypeFromCLSID(new Guid(clsidFwRule))) as INetFwRule; private static readonly char[] firewallEntryDelimiters = ['/', '-']; + + // Dedicated lock object for serializing access to the COM policy and its Rules + // collection. We don't lock on `policy` directly because (a) it's a COM RCW so locking + // on it is fragile if the COM init ever fails and the field is null, and (b) using a + // plain object makes the synchronization boundary explicit. Every policy.Rules access + // in this class must happen inside lock(policyLock). + private static readonly object policyLock = new(); + // ********************************************************************************************************************************** + + /// + /// Release a COM RCW reference for a fetched INetFwRule. Any rule obtained via + /// policy.Rules.Item(name) or by iterating policy.Rules carries a managed wrapper + /// over a COM object — failing to release it leaves the underlying RPC handle in + /// the COM apartment until GC. At firewall scales (thousands of rules iterated each + /// cycle) those handles accumulate until the firewall service refuses new calls and + /// the process must be restarted. + /// Public for testability — handles null and non-COM objects safely. + /// + [System.Diagnostics.CodeAnalysis.SuppressMessage("Interoperability", "CA1416:Validate platform compatibility", Justification = "Windows firewall COM object release is only used in Windows firewall implementation.")] + public static void ReleaseRule(INetFwRule rule) + { + if (rule is null) + { + return; + } + try + { + if (Marshal.IsComObject(rule)) + { + Marshal.FinalReleaseComObject(rule); + } + } + catch + { + // best effort — RCW may have already been released by another path + } + } + + /// + /// A list of INetFwRule COM objects that releases each RCW on Dispose. + /// Callers MUST dispose (via using or explicit Dispose) to avoid leaking. + /// Public for testability. + /// + public sealed class RuleList : List, IDisposable + { + private bool disposed; + + /// + public void Dispose() + { + if (disposed) + { + return; + } + disposed = true; + for (int i = 0; i < Count; i++) + { + ReleaseRule(this[i]); + } + Clear(); + } + } // ********************************************************************************************************************************** private static string CreateRuleStringForIPAddresses(IReadOnlyList ipAddresses, int index, int count) @@ -98,6 +170,7 @@ private static string CreateRuleStringForIPAddresses(IReadOnlyList ipAdd return b.ToString(); } + [UnconditionalSuppressMessage("Trimming", "IL2072", Justification = "COM rule object activation is runtime COM-based.")] private static bool GetOrCreateRule(string ruleName, string remoteIPAddresses, NetFwAction action, IEnumerable allowedPorts = null, string description = null) { @@ -105,10 +178,10 @@ private static bool GetOrCreateRule(string ruleName, string remoteIPAddresses, N bool emptyIPAddressString = string.IsNullOrWhiteSpace(remoteIPAddresses) || remoteIPAddresses == "*"; bool ruleNeedsToBeAdded = false; - lock (policy) + lock (policyLock) { - recreateRule: INetFwRule rule = null; + try_again: try { rule = policy.Rules.Item(ruleName); @@ -122,7 +195,7 @@ private static bool GetOrCreateRule(string ruleName, string remoteIPAddresses, N var disabled = description is not null && description.Contains("###DISABLED###"); description = description?.Replace("###DISABLED###", string.Empty); var ruleDescription = description ?? "Automatically created by IPBan"; - rule = Activator.CreateInstance(ruleType) as INetFwRule; + rule = CreateComFwRule(); rule.Name = ruleName; rule.Description = ruleDescription; rule.Enabled = !disabled; @@ -168,7 +241,11 @@ private static bool GetOrCreateRule(string ruleName, string remoteIPAddresses, N if (!ruleNeedsToBeAdded) { policy.Rules.Remove(ruleName); - goto recreateRule; + // release the stale rule before re-fetching to avoid leaking its COM RCW + ReleaseRule(rule); + rule = null; + ruleNeedsToBeAdded = false; + goto try_again; } } } @@ -198,7 +275,10 @@ private static bool GetOrCreateRule(string ruleName, string remoteIPAddresses, N { policy.Rules.Add(rule); } - return (rule != null); + bool created = (rule != null); + // release our local COM reference; if we Added, the policy holds its own reference + ReleaseRule(rule); + return created; } } @@ -218,15 +298,17 @@ private static void CreateAllowRule(IReadOnlyList ipAddresses, int index private void MigrateOldDefaultRuleNames() { - // migrate old default rule names to new names + // migrate old default rule names to new names — release each fetched rule's RCW + // after we're done renaming it, otherwise a rolling migration leaks one COM handle + // per rule per service start. INetFwRule rule; for (int i = 0; ; i += MaxIpAddressesPerRule) { - lock (policy) + rule = null; + lock (policyLock) { try { - rule = null; try { // migrate really old style @@ -243,11 +325,14 @@ private void MigrateOldDefaultRuleNames() catch { // ignore exception, assume does not exist + ReleaseRule(rule); break; } } + ReleaseRule(rule); } - lock (policy) + rule = null; + lock (policyLock) { try { @@ -259,15 +344,17 @@ private void MigrateOldDefaultRuleNames() // ignore exception, assume does not exist } } + ReleaseRule(rule); } private static bool DeleteRules(string ruleNamePrefix, int startIndex = 0) { try { - lock (policy) + using var matchingRules = EnumerateRulesMatchingPrefix(ruleNamePrefix); + lock (policyLock) { - foreach (INetFwRule rule in EnumerateRulesMatchingPrefix(ruleNamePrefix).ToArray()) + foreach (INetFwRule rule in matchingRules) { try { @@ -282,6 +369,7 @@ private static bool DeleteRules(string ruleNamePrefix, int startIndex = 0) } } } + // matchingRules.Dispose() releases all enumerated INetFwRule COM RCWs return true; } catch (Exception ex) @@ -291,32 +379,63 @@ private static bool DeleteRules(string ruleNamePrefix, int startIndex = 0) } } - private static List EnumerateRulesMatchingPrefix(string ruleNamePrefix) + // Returns a snapshot of rules whose name starts with the prefix. The caller MUST + // dispose the returned RuleList (use `using var rules = ...`) so each INetFwRule's + // COM RCW is released — otherwise every call leaks one handle per matching rule. + // The policy lock is held for the entire enumeration because the underlying COM + // enumerator is not thread-safe; a concurrent Add/Remove from another thread can + // crash the iterator mid-loop. + [System.Diagnostics.CodeAnalysis.SuppressMessage("Interoperability", "CA1416:Validate platform compatibility", Justification = "Windows firewall COM enumerator release is only used in Windows firewall implementation.")] + private static RuleList EnumerateRulesMatchingPrefix(string ruleNamePrefix) { // powershell example // (New-Object -ComObject HNetCfg.FwPolicy2).rules | Where-Object { $_.Name -match '^prefix' } | ForEach-Object { Write-Output "$($_.Name)" } // TODO: Revisit COM interface in .NET core 3.0 - List rules = []; - var e = policy.Rules.GetEnumeratorVariant(); - object[] results = new object[64]; - int count; + RuleList rules = new(); bool matchAll = (string.IsNullOrWhiteSpace(ruleNamePrefix) || ruleNamePrefix == "*"); IntPtr bufferLengthPointer = Marshal.AllocCoTaskMem(Marshal.SizeOf()); + object[] results = new object[64]; + int count; try { - do + lock (policyLock) { - e.Next(results.Length, results, bufferLengthPointer); - count = Marshal.ReadInt32(bufferLengthPointer); - foreach (object o in results) + var e = policy.Rules.GetEnumeratorVariant(); + try + { + do + { + e.Next(results.Length, results, bufferLengthPointer); + count = Marshal.ReadInt32(bufferLengthPointer); + for (int i = 0; i < count; i++) + { + object o = results[i]; + if (o is INetFwRule rule) + { + if (matchAll || rule.Name.StartsWith(ruleNamePrefix, StringComparison.OrdinalIgnoreCase)) + { + rules.Add(rule); + } + else + { + // we own this RCW since enumeration handed it to us — release the ones we filter out + ReleaseRule(rule); + } + } + // null entries are returned at the end of the buffer; nothing to release + results[i] = null; + } + } + while (count == results.Length); + } + finally { - if ((o is INetFwRule rule) && (matchAll || rule.Name.StartsWith(ruleNamePrefix, StringComparison.OrdinalIgnoreCase))) + if (e is not null && Marshal.IsComObject(e)) { - rules.Add(rule); + try { Marshal.FinalReleaseComObject(e); } catch { /* best effort */ } } } } - while (count == results.Length); } finally { @@ -496,7 +615,7 @@ public override IPBanMemoryFirewall Compile() try { - lock (policy) + lock (policyLock) { foreach (INetFwRule rule in policy.Rules) { @@ -530,6 +649,13 @@ public override IPBanMemoryFirewall Compile() Logger.Error(inner); } } + finally + { + // Each rule from this foreach is a fresh COM RCW that we own; release + // it before the next iteration so a Compile over a large firewall + // doesn't accumulate handles until exhaustion. + ReleaseRule(rule); + } } } } @@ -577,29 +703,32 @@ public override Task BlockIPAddressesDelta(string ruleNamePrefix, IEnumera string prefix = (string.IsNullOrWhiteSpace(ruleNamePrefix) ? BlockRulePrefix : RulePrefix + ruleNamePrefix).TrimEnd('_') + "_"; int ruleIndex; - INetFwRule[] rules = [.. EnumerateRulesMatchingPrefix(prefix)]; + // Snapshot just the rule names and remote-address strings into managed memory, then + // dispose the RuleList immediately. The rest of the algorithm only needs the strings; + // holding live COM references across it would pin one RCW per rule for the whole + // delta computation. + string[] ruleNames; List> remoteIPAddresses = []; List ruleChanges = []; - for (int i = 0; i < rules.Length; i++) + using (var matchedRules = EnumerateRulesMatchingPrefix(prefix)) { - string[] ipList = rules[i].RemoteAddresses.Split(','); - HashSet ipSet = []; - foreach (string ip in ipList) + ruleNames = new string[matchedRules.Count]; + for (int i = 0; i < matchedRules.Count; i++) { - // trim out submask - int pos = ip.IndexOfAny(firewallEntryDelimiters); - if (pos >= 0) - { - ipSet.Add(ip[..pos]); - } - else + ruleNames[i] = matchedRules[i].Name; + string[] ipList = (matchedRules[i].RemoteAddresses ?? string.Empty).Split(','); + HashSet ipSet = []; + foreach (string ip in ipList) { - ipSet.Add(ip); + // trim out submask + int pos = ip.IndexOfAny(firewallEntryDelimiters); + ipSet.Add(pos >= 0 ? ip[..pos] : ip); } + remoteIPAddresses.Add(ipSet); + ruleChanges.Add(false); } - remoteIPAddresses.Add(ipSet); - ruleChanges.Add(false); } + // matchedRules disposed here — all INetFwRule RCWs released List deltas = ipAddresses.ToList(); int deltasCount = deltas.Count; for (int deltaIndex = deltas.Count - 1; deltaIndex >= 0; deltaIndex--) @@ -657,7 +786,7 @@ public override Task BlockIPAddressesDelta(string ruleNamePrefix, IEnumera { if (ruleChanges[i]) { - string name = (i < rules.Length ? rules[i].Name : prefix + ruleIndex.ToStringInvariant()); + string name = (i < ruleNames.Length ? ruleNames[i] : prefix + ruleIndex.ToStringInvariant()); GetOrCreateRule(name, string.Join(',', remoteIPAddresses[i]), NetFwAction.Block, allowedPorts); } ruleIndex += MaxIpAddressesPerRule; @@ -700,94 +829,103 @@ public override Task AllowIPAddresses(string ruleNamePrefix, IEnumerable GetRuleNames(string ruleNamePrefix = null) { string prefix = (string.IsNullOrWhiteSpace(ruleNamePrefix) ? RulePrefix : RulePrefix + ruleNamePrefix); - return EnumerateRulesMatchingPrefix(prefix).OrderBy(r => r.Name).Select(r => r.Name); + // Materialize the names with ToList() before the using block disposes the RuleList. + // A lazy LINQ chain would be evaluated only when the consumer iterated it — by then + // the COM rules would already be released and Name reads would crash. + using var matchedRules = EnumerateRulesMatchingPrefix(prefix); + return matchedRules.OrderBy(r => r.Name).Select(r => r.Name).ToList(); } /// public override bool DeleteRule(string ruleName) { + INetFwRule rule = null; try { - INetFwRule rule = policy.Rules.Item(ruleName); - policy.Rules.Remove(rule.Name); + lock (policyLock) + { + rule = policy.Rules.Item(ruleName); + policy.Rules.Remove(rule.Name); + } return true; } catch { + return false; + } + finally + { + ReleaseRule(rule); } - return false; } - /// - public override IEnumerable EnumerateBannedIPAddresses(string ruleNamePrefix = null) + // Read RemoteAddresses for one rule under the policy lock, releasing the COM RCW + // before returning. Yields a null string when the rule does not exist. + private static string ReadRemoteAddressesForRule(string ruleName) { - int i = 0; - INetFwRule rule; - string prefix = (ruleNamePrefix is null ? BlockRulePrefix : ruleNamePrefix); - while (true) + INetFwRule rule = null; + try { - string ruleName = prefix + i.ToString(CultureInfo.InvariantCulture); - try + lock (policyLock) { - rule = policy.Rules.Item(ruleName); + try + { + rule = policy.Rules.Item(ruleName); + } + catch + { + return null; // does not exist + } if (rule is null) { - break; + return null; } + return rule.RemoteAddresses ?? string.Empty; } - catch + } + finally + { + ReleaseRule(rule); + } + } + + /// + public override IEnumerable EnumerateBannedIPAddresses(string ruleNamePrefix = null) + { + string prefix = (ruleNamePrefix is null ? BlockRulePrefix : ruleNamePrefix); + for (int i = 0; ; i += MaxIpAddressesPerRule) + { + string ruleName = prefix + i.ToString(CultureInfo.InvariantCulture); + string remoteAddresses = ReadRemoteAddressesForRule(ruleName); + if (remoteAddresses is null) { - // does not exist - break; + yield break; } - foreach (string ip in rule.RemoteAddresses.Split(',')) + foreach (string ip in remoteAddresses.Split(',')) { int pos = ip.IndexOfAny(firewallEntryDelimiters); - if (pos < 0) - { - yield return ip; - } - else - { - yield return ip[..pos]; - } + yield return pos < 0 ? ip : ip[..pos]; } - i += MaxIpAddressesPerRule; } } /// public override IEnumerable EnumerateAllowedIPAddresses(string ruleNamePrefix = null) { - INetFwRule rule; string prefix = (ruleNamePrefix is null ? AllowRulePrefix : ruleNamePrefix); for (int i = 0; ; i += MaxIpAddressesPerRule) { - try - { - rule = policy.Rules.Item(prefix + i.ToString(CultureInfo.InvariantCulture)); - if (rule is null) - { - break; - } - } - catch + string ruleName = prefix + i.ToString(CultureInfo.InvariantCulture); + string remoteAddresses = ReadRemoteAddressesForRule(ruleName); + if (remoteAddresses is null) { - // OK, rule does not exist yield break; } - foreach (string ip in rule.RemoteAddresses.Split(',')) + foreach (string ip in remoteAddresses.Split(',')) { int pos = ip.IndexOfAny(firewallEntryDelimiters); - if (pos < 0) - { - yield return ip; - } - else - { - yield return ip[..pos]; - } + yield return pos < 0 ? ip : ip[..pos]; } } } @@ -796,32 +934,46 @@ public override IEnumerable EnumerateAllowedIPAddresses(string ruleNameP public override IEnumerable EnumerateIPAddresses(string ruleNamePrefix = null) { string prefix = (string.IsNullOrWhiteSpace(ruleNamePrefix) ? RulePrefix : RulePrefix + ruleNamePrefix); - foreach (INetFwRule rule in EnumerateRulesMatchingPrefix(prefix)) + + // Snapshot the RemoteAddresses strings while the RuleList is still live, then dispose + // the rules and yield from managed memory. Yielding from inside the using block would + // either keep COM RCWs alive across the consumer's iteration (which may be slow or + // unbounded) or, worse, dispose them while the consumer is still reading. + List ipLists = []; + using (var matchedRules = EnumerateRulesMatchingPrefix(prefix)) { - string ipList = rule.RemoteAddresses; - if (!string.IsNullOrWhiteSpace(ipList) && ipList != "*") + foreach (INetFwRule rule in matchedRules) { - string[] ips = ipList.Split(','); - foreach (string ip in ips) + string ipList = rule.RemoteAddresses; + if (!string.IsNullOrWhiteSpace(ipList) && ipList != "*") { - if (IPAddressRange.TryParse(ip, out IPAddressRange range)) - { - yield return range; - } - // else // should never happen + ipLists.Add(ipList); } } } + + foreach (string ipList in ipLists) + { + foreach (string ip in ipList.Split(',')) + { + if (IPAddressRange.TryParse(ip, out IPAddressRange range)) + { + yield return range; + } + // else // should never happen + } + } } /// public override string GetPorts(string ruleName) { + INetFwRule rule = null; try { - lock (policy) + lock (policyLock) { - INetFwRule rule = policy.Rules.Item(ruleName); + rule = policy.Rules.Item(ruleName); return rule.LocalPorts ?? string.Empty; } } @@ -829,16 +981,20 @@ public override string GetPorts(string ruleName) { // not exist, no way to determine this without throwing } + finally + { + ReleaseRule(rule); + } return null; - } /// public override void Truncate() { - foreach (INetFwRule rule in EnumerateRulesMatchingPrefix(RulePrefix).ToArray()) + using var matchedRules = EnumerateRulesMatchingPrefix(RulePrefix); + foreach (INetFwRule rule in matchedRules) { - lock (policy) + lock (policyLock) { try { diff --git a/IPBanCore/ipban.config b/IPBanCore/ipban.config index 913fe341..109d4766 100644 --- a/IPBanCore/ipban.config +++ b/IPBanCore/ipban.config @@ -1099,6 +1099,8 @@ ###APP### will be replaced with the app name and version. ###COUNT### will be replaced with the number of events. ###LOG### will be replaced with matching log text that triggered the event, if known. + ###OSNAME### will be replaced with the operating system name. + ###OSVERSION### will be replaced with the operating system version. Can run multiple processes by delimiting each with a newline --> @@ -1112,6 +1114,8 @@ ###APP### will be replaced with the app name and version. ###COUNT### will be replaced with the number of events. ###LOG### will be replaced with matching log text that triggered the event, if known. + ###OSNAME### will be replaced with the operating system name. + ###OSVERSION### will be replaced with the operating system version. Can run multiple processes by delimiting each with a newline --> @@ -1167,9 +1171,15 @@ + --> + + + + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + diff --git a/IPBanTests/IPBanUInt128Tests.cs b/IPBanTests/IPBanUInt128Tests.cs new file mode 100644 index 00000000..de3613d7 --- /dev/null +++ b/IPBanTests/IPBanUInt128Tests.cs @@ -0,0 +1,298 @@ +/* +MIT License + +Copyright (c) 2012-present Digital Ruby, LLC - https://ipban.com + +Tests for the custom UInt128 struct used as the IPv6 numeric backing for +IPAddressRange. Bugs in arithmetic, comparison, or BigInteger round-trip would +silently break v6 range matching across the codebase. +*/ + +using System.Numerics; + +using DigitalRuby.IPBanCore; + +using NUnit.Framework; +using NUnit.Framework.Legacy; + +namespace DigitalRuby.IPBanTests +{ + [TestFixture] + public sealed class IPBanUInt128Tests + { + // -------------------- constants and constructors -------------------- + + [Test] + public void Constants_HaveExpectedValues() + { + ClassicAssert.AreEqual(0UL, UInt128.Zero.LeastSignificant); + ClassicAssert.AreEqual(0UL, UInt128.Zero.MostSignificant); + + ClassicAssert.AreEqual(1UL, UInt128.One.LeastSignificant); + ClassicAssert.AreEqual(0UL, UInt128.One.MostSignificant); + + ClassicAssert.AreEqual(0UL, UInt128.MinValue.LeastSignificant); + ClassicAssert.AreEqual(0UL, UInt128.MinValue.MostSignificant); + + ClassicAssert.AreEqual(ulong.MaxValue, UInt128.MaxValue.LeastSignificant); + ClassicAssert.AreEqual(ulong.MaxValue, UInt128.MaxValue.MostSignificant); + } + + [Test] + public void Construct_FromTwoLongs_StoresAsExpected() + { + var v = new UInt128(0x1234_5678_9ABC_DEF0UL, 0x0FED_CBA9_8765_4321UL); + ClassicAssert.AreEqual(0x1234_5678_9ABC_DEF0UL, v.MostSignificant); + ClassicAssert.AreEqual(0x0FED_CBA9_8765_4321UL, v.LeastSignificant); + } + + [Test] + public void Construct_FromUlong_LeavesUpperZero() + { + UInt128 v = 42UL; // implicit conversion + ClassicAssert.AreEqual(42UL, v.LeastSignificant); + ClassicAssert.AreEqual(0UL, v.MostSignificant); + } + + // -------------------- BigInteger round-trip -------------------- + + [Test] + public void BigInteger_RoundTrips_AcrossSignificantValues() + { + BigInteger[] cases = + { + BigInteger.Zero, + BigInteger.One, + 42, + ulong.MaxValue, // exactly 64 bits + BigInteger.Pow(2, 64), // first value needing the upper word + BigInteger.Pow(2, 64) + 1, + BigInteger.Pow(2, 127), // top bit set + (BigInteger.One << 128) - 1, // UInt128.MaxValue + }; + + foreach (var bi in cases) + { + UInt128 v = (UInt128)bi; + BigInteger back = v; // implicit conversion via operator + ClassicAssert.AreEqual(bi, back, $"round-trip failed for {bi}"); + } + } + + // -------------------- equality + comparison -------------------- + + [Test] + public void Equality_TwoIdenticalValuesAreEqual() + { + var a = new UInt128(7, 11); + var b = new UInt128(7, 11); + + ClassicAssert.IsTrue(a == b); + ClassicAssert.IsFalse(a != b); + ClassicAssert.IsTrue(a.Equals(b)); + ClassicAssert.AreEqual(a.GetHashCode(), b.GetHashCode()); + } + + [Test] + public void Equality_DifferentMostSignificantNotEqual() + { + ClassicAssert.IsFalse(new UInt128(1, 0) == new UInt128(2, 0)); + } + + [Test] + public void Equality_DifferentLeastSignificantNotEqual() + { + ClassicAssert.IsFalse(new UInt128(0, 1) == new UInt128(0, 2)); + } + + [Test] + public void Comparison_OrderMatchesNumericValue() + { + // a < b < c — verify each comparison operator agrees. + UInt128 a = 1UL; + UInt128 b = ulong.MaxValue; // 2^64 - 1 + UInt128 c = new(1UL, 0UL); // 2^64 + + // Use a copy for the reflexive checks so the compiler doesn't complain about + // self-comparison (CS1718). Semantically identical — verifies <= / >= return true + // for two equal values. + UInt128 aCopy = 1UL; + UInt128 cCopy = new(1UL, 0UL); + + ClassicAssert.IsTrue(a < b); ClassicAssert.IsTrue(b < c); + ClassicAssert.IsTrue(c > b); ClassicAssert.IsTrue(b > a); + ClassicAssert.IsTrue(a <= aCopy); ClassicAssert.IsTrue(a <= b); + ClassicAssert.IsTrue(c >= cCopy); ClassicAssert.IsTrue(c >= b); + + ClassicAssert.AreEqual(-1, a.CompareTo(b)); + ClassicAssert.AreEqual( 1, c.CompareTo(b)); + ClassicAssert.AreEqual( 0, b.CompareTo(b)); + } + + [Test] + public void Comparison_AcrossUpperWordBoundary() + { + // Two values where MSB differs by 1; the bigger must compare greater regardless of + // LSB ordering — confirms the comparator orders MSB-then-LSB, not LSB-only. + UInt128 a = new(1UL, ulong.MaxValue); + UInt128 b = new(2UL, 0UL); + ClassicAssert.IsTrue(b > a); + } + + // -------------------- arithmetic -------------------- + + [Test] + public void Add_SimpleSumWithinLowerWord() + { + UInt128 a = 100UL; + UInt128 b = 23UL; + ClassicAssert.AreEqual((UInt128)123UL, a + b); + } + + [Test] + public void Add_CarryFromLowerToUpperWord() + { + // 0xFFFF_FFFF_FFFF_FFFF + 1 should produce a carry into the upper word. + UInt128 v = ulong.MaxValue; + UInt128 result = v + 1UL; + ClassicAssert.AreEqual(0UL, result.LeastSignificant); + ClassicAssert.AreEqual(1UL, result.MostSignificant); + } + + [Test] + public void Add_OverflowWrapsAroundLikeNativeUnsigned() + { + // MaxValue + 1 wraps to 0 (modular semantics for unsigned 128-bit). + UInt128 result = UInt128.MaxValue + UInt128.One; + ClassicAssert.AreEqual(UInt128.Zero, result); + } + + [Test] + public void Subtract_SimpleDifferenceWithinLowerWord() + { + ClassicAssert.AreEqual((UInt128)77UL, (UInt128)100UL - (UInt128)23UL); + } + + [Test] + public void Subtract_BorrowFromUpperWord() + { + // (1, 0) - 1 = (0, 0xFFFF_FFFF_FFFF_FFFF) + UInt128 result = new UInt128(1UL, 0UL) - 1UL; + ClassicAssert.AreEqual(0UL, result.MostSignificant); + ClassicAssert.AreEqual(ulong.MaxValue, result.LeastSignificant); + } + + [Test] + public void Subtract_UnderflowWrapsAroundLikeNativeUnsigned() + { + // 0 - 1 wraps to MaxValue. + UInt128 result = UInt128.Zero - UInt128.One; + ClassicAssert.AreEqual(UInt128.MaxValue, result); + } + + // -------------------- bitwise -------------------- + + [Test] + public void BitwiseAnd_MasksAcrossBothWords() + { + UInt128 a = new(0xFFFF_0000_FFFF_0000UL, 0xFFFF_0000_FFFF_0000UL); + UInt128 b = new(0x0000_FFFF_0000_FFFFUL, 0x0F0F_0F0F_0F0F_0F0FUL); + UInt128 r = a & b; + ClassicAssert.AreEqual(0x0000_0000_0000_0000UL, r.MostSignificant); + ClassicAssert.AreEqual(0x0F0F_0000_0F0F_0000UL, r.LeastSignificant); + } + + [Test] + public void BitwiseOr_UnionsBothWords() + { + UInt128 a = new(0xAAAA_AAAA_AAAA_AAAAUL, 0x5555_5555_5555_5555UL); + UInt128 b = new(0x5555_5555_5555_5555UL, 0xAAAA_AAAA_AAAA_AAAAUL); + UInt128 r = a | b; + ClassicAssert.AreEqual(ulong.MaxValue, r.MostSignificant); + ClassicAssert.AreEqual(ulong.MaxValue, r.LeastSignificant); + } + + [Test] + public void LeftShift_WithinLowerWord() + { + UInt128 v = 1UL; + UInt128 r = v << 4; + ClassicAssert.AreEqual(16UL, r.LeastSignificant); + ClassicAssert.AreEqual(0UL, r.MostSignificant); + } + + [Test] + public void LeftShift_AcrossWordBoundary() + { + // 1 << 64 should land entirely in the upper word. + UInt128 r = (UInt128)1UL << 64; + ClassicAssert.AreEqual(0UL, r.LeastSignificant); + ClassicAssert.AreEqual(1UL, r.MostSignificant); + } + + [Test] + public void RightShift_AcrossWordBoundary() + { + // High bit (1 << 64) shifted right by 64 returns to the lower word. + UInt128 v = new(1UL, 0UL); + UInt128 r = v >> 64; + ClassicAssert.AreEqual(1UL, r.LeastSignificant); + ClassicAssert.AreEqual(0UL, r.MostSignificant); + } + + // -------------------- parsing and formatting -------------------- + + [Test] + public void Parse_DecimalRoundTrip() + { + // Pick a value that uses both 64-bit words. + BigInteger expected = (BigInteger.One << 100) + 12345; + UInt128 v = (UInt128)expected; + + string s = v.ToString(); + ClassicAssert.AreEqual(expected.ToString(), s); + + UInt128 back = UInt128.Parse(s); + ClassicAssert.AreEqual(v, back); + } + + [Test] + public void Parse_MaxValueRoundTrip() + { + string s = UInt128.MaxValue.ToString(); + ClassicAssert.AreEqual(UInt128.MaxValue, UInt128.Parse(s)); + } + + [Test] + public void Parse_ZeroRoundTrip() + { + ClassicAssert.AreEqual(UInt128.Zero, UInt128.Parse("0")); + ClassicAssert.AreEqual("0", UInt128.Zero.ToString()); + } + + [Test] + public void TryParse_RejectsGarbage() + { + ClassicAssert.IsFalse(UInt128.TryParse("not-a-number", out _)); + ClassicAssert.IsFalse(UInt128.TryParse(string.Empty, out _)); + ClassicAssert.IsFalse(UInt128.TryParse(null, out _)); + } + + [Test] + public void TryParse_AcceptsNormalInput() + { + ClassicAssert.IsTrue(UInt128.TryParse("123456789", out var v)); + ClassicAssert.AreEqual((UInt128)123456789UL, v); + } + + // -------------------- explicit conversions -------------------- + + [Test] + public void ExplicitToUlong_TruncatesUpperBits() + { + // (1, 42) → cast to ulong returns the low 64 bits (42), upper bits are dropped. + UInt128 v = new(1UL, 42UL); + ClassicAssert.AreEqual(42UL, (ulong)v); + } + } +} diff --git a/IPBanTests/IPBanUpdateChannelTests.cs b/IPBanTests/IPBanUpdateChannelTests.cs new file mode 100644 index 00000000..e5c33a7f --- /dev/null +++ b/IPBanTests/IPBanUpdateChannelTests.cs @@ -0,0 +1,254 @@ +/* +MIT License + +Copyright (c) 2012-present Digital Ruby, LLC - https://ipban.com + +Tests for the auto-update channel — verify the SHA-256 gate on the downloaded +update binary. Each branch (no hash configured, hash mismatch, hash match) must +produce the documented behavior before the binary is executed. +*/ + +using System; +using System.Collections.Generic; +using System.Security.Cryptography; +using System.Text; +using System.Threading; +using System.Threading.Tasks; + +using DigitalRuby.IPBanCore; + +using NUnit.Framework; +using NUnit.Framework.Legacy; + +namespace DigitalRuby.IPBanTests +{ + /// + /// End-to-end tests for IPBanService.GetUrl(UrlType.Update): + /// - empty config hash → binary fetched but NOT executed (safe default) + /// - mismatched hash → binary rejected, NOT executed + /// - matching hash → binary executed + /// + /// We swap in a fake IHttpRequestMaker so no real network traffic flies, and we + /// subclass IPBanService to override LaunchUpdateBinary so no real subprocess starts. + /// + [TestFixture] + public sealed class IPBanUpdateChannelTests + { + // -------- test doubles -------- + + /// Captures-and-returns a fixed payload for any URL. + private sealed class FakeHttpRequestMaker : IHttpRequestMaker + { + public byte[] Payload { get; } + public List Calls { get; } = []; + + public FakeHttpRequestMaker(byte[] payload) { Payload = payload; } + + public Task MakeRequestAsync(Uri uri, + byte[] postJson = null, + IEnumerable> headers = null, + string method = null, + CancellationToken cancelToken = default) + { + Calls.Add(uri); + return Task.FromResult(Payload); + } + } + + /// Subclass that records LaunchUpdateBinary calls and exposes GetUrl. + public sealed class CapturingIPBanService : IPBanService + { + public List<(string TempFile, string Args)> LaunchedBinaries { get; } = []; + + protected override void LaunchUpdateBinary(string tempFile, string args) + { + // intentionally do NOT call base — we don't want to spawn anything in tests + LaunchedBinaries.Add((tempFile, args)); + } + + public Task CallGetUrl(UrlType urlType, CancellationToken cancelToken = default) + => GetUrl(urlType, cancelToken); + } + + // -------- helpers -------- + + private const string TestUpdateUrl = "http://localhost/update"; + + private static string Sha256Hex(byte[] bytes) => + Convert.ToHexString(SHA256.HashData(bytes)); + + private static CapturingIPBanService CreateService(string updateUrl, string sha256Hash) + { + string updateLine = ""; + string hashLine = ""; + + return IPBanServiceTestConfigHelper.CreateServiceWithConfig( + cfg => cfg + .Replace("", updateLine) + .Replace("", hashLine)); + } + + // -------- the actual tests -------- + + [Test] + public async Task EmptyHashConfig_FetchesButDoesNotExecute() + { + // safe default: if the operator has not opted in by setting GetUrlUpdateSha256, + // we fetch the bytes (so the operator can inspect them) but do NOT exec them. + byte[] payload = Encoding.UTF8.GetBytes("fake-update-binary"); + var service = CreateService(TestUpdateUrl, sha256Hash: string.Empty); + try + { + var http = new FakeHttpRequestMaker(payload); + service.RequestMaker = http; + + await service.CallGetUrl(IPBanService.UrlType.Update, CancellationToken.None); + + ClassicAssert.AreEqual(1, http.Calls.Count, "should have fetched once"); + CollectionAssert.IsEmpty(service.LaunchedBinaries, + "must NOT execute when no hash is configured (safe default)"); + } + finally + { + service.Dispose(); + } + } + + [Test] + public async Task WrongHash_FetchesButRejects() + { + // hash mismatch must reject the binary, log an error, and NOT execute it. + byte[] payload = Encoding.UTF8.GetBytes("fake-update-binary"); + string wrongHash = new string('0', 64); // 64 hex chars — definitely not the real hash + var service = CreateService(TestUpdateUrl, sha256Hash: wrongHash); + try + { + var http = new FakeHttpRequestMaker(payload); + service.RequestMaker = http; + + await service.CallGetUrl(IPBanService.UrlType.Update, CancellationToken.None); + + ClassicAssert.AreEqual(1, http.Calls.Count); + CollectionAssert.IsEmpty(service.LaunchedBinaries, + "must NOT execute when the configured hash does not match the payload"); + } + finally + { + service.Dispose(); + } + } + + [Test] + public async Task MatchingHash_ExecutesTheBinary() + { + // the happy path — operator configured the correct hash, the bytes match, exec proceeds. + byte[] payload = Encoding.UTF8.GetBytes("fake-update-binary-v2"); + string correctHash = Sha256Hex(payload); + var service = CreateService(TestUpdateUrl, sha256Hash: correctHash); + try + { + var http = new FakeHttpRequestMaker(payload); + service.RequestMaker = http; + + await service.CallGetUrl(IPBanService.UrlType.Update, CancellationToken.None); + + ClassicAssert.AreEqual(1, http.Calls.Count); + ClassicAssert.AreEqual(1, service.LaunchedBinaries.Count, + "matching hash must execute the binary exactly once"); + StringAssert.EndsWith("IPBanServiceUpdate.exe", + service.LaunchedBinaries[0].TempFile); + } + finally + { + service.Dispose(); + } + } + + [Test] + public async Task HashCompareIsCaseInsensitive() + { + // SHA-256 hex is conventionally either case; the gate must accept both. + byte[] payload = Encoding.UTF8.GetBytes("test-bytes"); + string lowerHash = Sha256Hex(payload).ToLowerInvariant(); + var service = CreateService(TestUpdateUrl, sha256Hash: lowerHash); + try + { + var http = new FakeHttpRequestMaker(payload); + service.RequestMaker = http; + + await service.CallGetUrl(IPBanService.UrlType.Update, CancellationToken.None); + + ClassicAssert.AreEqual(1, service.LaunchedBinaries.Count, + "lowercase hex must compare equal to uppercase output of HashData"); + } + finally + { + service.Dispose(); + } + } + + [Test] + public async Task EmptyResponseBody_NoExecution() + { + // server returns nothing — there's no binary to execute regardless of hash config. + byte[] payload = []; + var service = CreateService(TestUpdateUrl, sha256Hash: Sha256Hex(payload)); + try + { + var http = new FakeHttpRequestMaker(payload); + service.RequestMaker = http; + + await service.CallGetUrl(IPBanService.UrlType.Update, CancellationToken.None); + + CollectionAssert.IsEmpty(service.LaunchedBinaries, + "empty response must never trigger an execution"); + } + finally + { + service.Dispose(); + } + } + } + + /// + /// Lightweight tests for the new setting. + /// + [TestFixture] + public sealed class IPBanConfigSha256Tests + { + [Test] + public void DefaultIsEmptyString() + { + // default config has an empty auto-update hash -> safe-by-default (binary not executed) + var service = IPBanServiceTestConfigHelper.CreateServiceWithConfig(); + try + { + ClassicAssert.IsNotNull(service.Config.GetUrlUpdateSha256); + ClassicAssert.AreEqual(string.Empty, service.Config.GetUrlUpdateSha256); + } + finally + { + service.Dispose(); + } + } + + [Test] + public void ConfigValueRoundTrips() + { + // when the operator sets GetUrlUpdateSha256 in config, the property exposes that exact value + const string expectedHash = "ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789"; + var service = IPBanServiceTestConfigHelper.CreateServiceWithConfig( + cfg => cfg.Replace( + "", + "")); + try + { + ClassicAssert.AreEqual(expectedHash, service.Config.GetUrlUpdateSha256); + } + finally + { + service.Dispose(); + } + } + } +} diff --git a/IPBanTests/IPBanUriFirewallRuleTests.cs b/IPBanTests/IPBanUriFirewallRuleTests.cs index 15fbfb07..e9efd4b2 100644 --- a/IPBanTests/IPBanUriFirewallRuleTests.cs +++ b/IPBanTests/IPBanUriFirewallRuleTests.cs @@ -30,6 +30,7 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE using System; using System.Collections.Generic; using System.IO; +using System.IO.Compression; using System.Linq; using System.Text; using System.Threading; @@ -132,5 +133,70 @@ public Task RunFirewallTask(Func { return action(state, memoryFirewall, default); } + + [Test] + public void ToString_ContainsPrefixIntervalAndUri() + { + using IPBanUriFirewallRule rule = new(memoryFirewall, this, this, this, "TestPrefix", + TimeSpan.FromMinutes(5.0), new Uri("http://example.com/list.txt")); + string s = rule.ToString(); + StringAssert.Contains("TestPrefix", s); + StringAssert.Contains("00:05:00", s); + StringAssert.Contains("example.com", s); + } + + [Test] + public void Equals_AndHashCode_BasedOnPrefixIntervalUri() + { + using IPBanUriFirewallRule a = new(memoryFirewall, this, this, this, "P", + TimeSpan.FromMinutes(5.0), new Uri("http://example.com/list.txt")); + using IPBanUriFirewallRule b = new(memoryFirewall, this, this, this, "P", + TimeSpan.FromMinutes(5.0), new Uri("http://example.com/list.txt")); + using IPBanUriFirewallRule c = new(memoryFirewall, this, this, this, "Other", + TimeSpan.FromMinutes(5.0), new Uri("http://example.com/list.txt")); + ClassicAssert.IsTrue(a.Equals(b)); + ClassicAssert.IsFalse(a.Equals(c)); + ClassicAssert.IsFalse(a.Equals("not a rule")); + ClassicAssert.AreEqual(a.GetHashCode(), b.GetHashCode()); + } + + [Test] + public async Task TestGzipFile() + { + string tempFile = Path.GetTempFileName(); + string gzPath = tempFile + ".gz"; + try + { + File.Delete(tempFile); + // Write a gzipped version of the test file content. + using (var fs = File.Create(gzPath)) + using (var gz = new GZipStream(fs, CompressionMode.Compress)) + using (var sw = new StreamWriter(gz)) + { + sw.Write(GetTestFile()); + } + Uri uriObj = new("file://" + gzPath.Replace("\\", "/")); + using IPBanUriFirewallRule rule = new(memoryFirewall, this, this, this, + "GzPrefix", TimeSpan.FromMinutes(1.0), uriObj); + await rule.Update(); + ClassicAssert.AreEqual(1, memoryFirewall.GetRuleNames("GzPrefix").ToArray().Length); + } + finally + { + try { File.Delete(gzPath); } catch { /* best effort */ } + } + } + + [Test] + public async Task DeleteRule_RemovesAllRulesUnderPrefix() + { + using IPBanUriFirewallRule rule = new(memoryFirewall, this, this, this, "ToDelete", + TimeSpan.FromMinutes(5.0), new Uri("http://example.com/list.txt")); + await rule.Update(); + // Some rule(s) should now exist for this prefix + ClassicAssert.IsTrue(memoryFirewall.GetRuleNames("ToDelete").Any()); + rule.DeleteRule(); + ClassicAssert.IsFalse(memoryFirewall.GetRuleNames("ToDelete").Any()); + } } } diff --git a/IPBanTests/IPBanWindowsFirewallTests.cs b/IPBanTests/IPBanWindowsFirewallTests.cs new file mode 100644 index 00000000..a47c174e --- /dev/null +++ b/IPBanTests/IPBanWindowsFirewallTests.cs @@ -0,0 +1,101 @@ +/* +MIT License + +Copyright (c) 2012-present Digital Ruby, LLC - https://ipban.com + +Tests for the IPBanWindowsFirewall COM-handling helpers — ReleaseRule and +RuleList. End-to-end firewall behavior is covered by IPBanFirewallTests +against a real firewall on the host; this fixture exercises the helpers +that prevent COM-handle leakage during rule iteration. +*/ + +using DigitalRuby.IPBanCore; +using DigitalRuby.IPBanCore.Windows.COM; + +using NUnit.Framework; +using NUnit.Framework.Legacy; + +using System; + +namespace DigitalRuby.IPBanTests +{ + /// + /// Unit tests for IPBanWindowsFirewall.ReleaseRule and IPBanWindowsFirewall.RuleList. + /// These tests exercise the helpers without instantiating the firewall itself, so they + /// run cross-platform as long as the COM interop *types* compile (which they do — only + /// runtime instantiation of policy is Windows-only). + /// + [TestFixture] + public sealed class IPBanWindowsFirewallTests + { + [Test] + public void ReleaseRule_NullIsNoOp() + { + // The helper is called from finally blocks in many paths; passing null must be + // a no-op rather than NRE'ing on Marshal.FinalReleaseComObject(null). + Assert.DoesNotThrow(() => IPBanWindowsFirewall.ReleaseRule(null)); + } + + [Test] + public void RuleList_DisposeIsIdempotent() + { + // RuleList.Dispose iterates and releases. Second Dispose must be a no-op. + using var list = new IPBanWindowsFirewall.RuleList(); + list.Add(null); // null entries must not crash the iteration + list.Dispose(); + Assert.DoesNotThrow(() => list.Dispose()); + Assert.DoesNotThrow(() => list.Dispose()); + } + + [Test] + public void RuleList_DisposeClearsTheList() + { + // After dispose, the list is empty — defensive against accidental re-use. + var list = new IPBanWindowsFirewall.RuleList(); + list.Add(null); + list.Add(null); + ClassicAssert.AreEqual(2, list.Count); + list.Dispose(); + ClassicAssert.AreEqual(0, list.Count); + } + + [Test] + public void RuleList_DisposeToleratesNullEntries() + { + // RuleList.Dispose() walks each element and calls ReleaseRule. Null entries + // (which can happen if a COM enumeration buffer fills with nulls at the end) + // must not cause Dispose to throw. + using var list = new IPBanWindowsFirewall.RuleList(); + for (int i = 0; i < 10; i++) + { + list.Add(null); + } + Assert.DoesNotThrow(() => list.Dispose()); + } + + [Test] + public void RuleList_UsingPatternReleasesEvenOnException() + { + // The whole point of using `using var rules = ...` is that exceptions thrown + // during processing still trigger Dispose. Verify that contract. + bool disposed = false; + try + { + using var list = new IPBanWindowsFirewall.RuleList(); + list.Add(null); + throw new InvalidOperationException("simulated mid-iteration failure"); + // unreachable — but Dispose runs via using's finally +#pragma warning disable CS0162 + disposed = true; +#pragma warning restore CS0162 + } + catch (InvalidOperationException) + { + // expected + } + // We can't directly observe the dispose without exposing internal state, but the + // assertion is "no second exception escaped" — i.e. Dispose tolerated the null. + ClassicAssert.IsFalse(disposed, "control flow should have skipped the post-throw line"); + } + } +} diff --git a/IPBanTests/IPBanXmlCDataTests.cs b/IPBanTests/IPBanXmlCDataTests.cs new file mode 100644 index 00000000..b0cd3d11 --- /dev/null +++ b/IPBanTests/IPBanXmlCDataTests.cs @@ -0,0 +1,93 @@ +/* +MIT License + +Copyright (c) 2012-present Digital Ruby, LLC - https://ipban.com + +Coverage tests for the XmlCData CDATA wrapper. +*/ + +using System.IO; +using System.Xml.Serialization; + +using DigitalRuby.IPBanCore; + +using NUnit.Framework; +using NUnit.Framework.Legacy; + +namespace DigitalRuby.IPBanTests +{ + [TestFixture] + public sealed class IPBanXmlCDataTests + { + [Test] + public void Construct_WithValue_TrimsAndStores() + { + var c = new XmlCData(" hello "); + ClassicAssert.AreEqual("hello", c.ToString()); + } + + [Test] + public void Construct_Default_StoresEmpty() + { + var c = new XmlCData(); + ClassicAssert.AreEqual(string.Empty, c.ToString()); + } + + [Test] + public void ImplicitOperator_FromString_Wraps() + { + XmlCData c = "value"; + ClassicAssert.AreEqual("value", c.ToString()); + } + + [Test] + public void ImplicitOperator_ToString_Unwraps() + { + var c = new XmlCData("v"); + string s = c; + ClassicAssert.AreEqual("v", s); + + string s2 = (XmlCData)null; + ClassicAssert.IsNull(s2); + } + + [Test] + public void GetSchema_ReturnsNull() + { + var c = new XmlCData("v"); + ClassicAssert.IsNull(c.GetSchema()); + } + + [Test] + public void XmlSerialize_RoundTripsValue() + { + var ser = new XmlSerializer(typeof(CDataWrap)); + var orig = new CDataWrap { Body = new XmlCData("hello there") }; + using var sw = new StringWriter(); + ser.Serialize(sw, orig); + string xml = sw.ToString(); + StringAssert.Contains("CDATA", xml); + + using var sr = new StringReader(xml); + var back = (CDataWrap)ser.Deserialize(sr); + // ReadXml trims surrounding whitespace from the CDATA payload. + ClassicAssert.AreEqual("hello there", back.Body.ToString()); + } + + [Test] + public void XmlSerialize_EmptyValueWritesEmptyString() + { + var ser = new XmlSerializer(typeof(CDataWrap)); + var orig = new CDataWrap { Body = new XmlCData("") }; + using var sw = new StringWriter(); + ser.Serialize(sw, orig); + string xml = sw.ToString(); + ClassicAssert.IsFalse(xml.Contains("CDATA")); + } + + public class CDataWrap + { + public XmlCData Body { get; set; } + } + } +} diff --git a/IPBanTests/TestData/EventViewer/Files/EventViewerInput4.txt b/IPBanTests/TestData/EventViewer/Files/EventViewerInput4.txt new file mode 100644 index 00000000..456815ad --- /dev/null +++ b/IPBanTests/TestData/EventViewer/Files/EventViewerInput4.txt @@ -0,0 +1,41 @@ + + + + 4625 + 0 + 0 + 12544 + 0 + 0x8010000000000000 + + 228965137 + + + Security + THEMACHINE + + + + S-1-0-0 + - + - + 0x0 + S-1-0-0 + THEUSER + + 0xc000006d + %%2313 + 0xc0000064 + 3 + NtLmSsp + NTLM + - + - + - + 0 + 0x0 + - + 55.11.1.2 + 0 + + \ No newline at end of file diff --git a/IPBanTests/TestData/EventViewer/Files/EventViewerInput5.txt b/IPBanTests/TestData/EventViewer/Files/EventViewerInput5.txt new file mode 100644 index 00000000..af2430df --- /dev/null +++ b/IPBanTests/TestData/EventViewer/Files/EventViewerInput5.txt @@ -0,0 +1,51 @@ + +4625001254400x80100000000000001062358SecurityCPU123S-1-0-0--0x0S-1-0-0administrator-0xc000006d%%23130xc000006a3NtLmSsp NTLMjade-hp--00x0-27.1.2.30An account failed to log on. + +Subject: + Security ID: S-1-0-0 + Account Name: - + Account Domain: - + Logon ID: 0x0 + +Logon Type: 3 + +Account For Which Logon Failed: + Security ID: S-1-0-0 + Account Name: administrator + Account Domain: - + +Failure Information: + Failure Reason: Unknown user name or bad password. + Status: 0xC000006D + Sub Status: 0xC000006A + +Process Information: + Caller Process ID: 0x0 + Caller Process Name: - + +Network Information: + Workstation Name: thestation + Source Network Address: 27.1.2.3 + Source Port: 0 + +Detailed Authentication Information: + Logon Process: NtLmSsp + Authentication Package: NTLM + Transited Services: - + Package Name (NTLM only): - + Key Length: 0 + +This event is generated when a logon request fails. It is generated on the computer where access was attempted. + +The Subject fields indicate the account on the local system which requested the logon. This is most commonly a service such as the Server service, or a local process such as Winlogon.exe or Services.exe. + +The Logon Type field indicates the kind of logon that was requested. The most common types are 2 (interactive) and 3 (network). + +The Process Information fields indicate which account and process on the system requested the logon. + +The Network Information fields indicate where a remote logon request originated. Workstation name is not always available and may be left blank in some cases. + +The authentication information fields provide detailed information about this specific logon request. + - Transited services indicate which intermediate services have participated in this logon request. + - Package name indicates which sub-protocol was used among the NTLM protocols. + - Key length indicates the length of the generated session key. This will be 0 if no session key was requested.InformationLogonInfoSecurityMicrosoft Windows security auditing.Audit Failure \ No newline at end of file diff --git a/IPBanTests/TestData/EventViewer/Files/EventViewerInput6.txt b/IPBanTests/TestData/EventViewer/Files/EventViewerInput6.txt new file mode 100644 index 00000000..8a6071a6 --- /dev/null +++ b/IPBanTests/TestData/EventViewer/Files/EventViewerInput6.txt @@ -0,0 +1,47 @@ + + + + 4624 + 2 + 0 + 12544 + 0 + 0x8020000000000000 + + 19428992 + + + Security + vmi2134045 + + + + S-1-5-18 + VMI2134045$ + WORKGROUP + 0x3e7 + S-1-5-21-3713639508-3051920844-4088787598-1000 + MyUser + VMI2134045 + 0x3d3309b4 + 7 + User32 + Negotiate + VMI2134045 + {00000000-0000-0000-0000-000000000000} + - + - + 0 + 0x788 + C:\Windows\System32\svchost.exe + 100.1.2.3 + 0 + %%1833 + - + - + - + %%1843 + 0x3d33096b + %%1843 + + \ No newline at end of file diff --git a/IPBanTests/TestData/EventViewer/Files/EventViewerTests.txt b/IPBanTests/TestData/EventViewer/Files/EventViewerTests.txt index 431975f1..5fc8ee13 100644 --- a/IPBanTests/TestData/EventViewer/Files/EventViewerTests.txt +++ b/IPBanTests/TestData/EventViewer/Files/EventViewerTests.txt @@ -7,6 +7,24 @@ # FailedLogin, SuccessfulLogin or None [newline] # --- [newline] # +$file:./TestData/EventViewer/Files/EventViewerInput6.txt +100.1.2.3 +MyUser +RDP +SuccessfulLogin +--- +$file:./TestData/EventViewer/Files/EventViewerInput5.txt +27.1.2.3 +administrator +RDP +FailedLogin +--- +$file:./TestData/EventViewer/Files/EventViewerInput4.txt +55.11.1.2 +THEUSER +RDP +FailedLogin +--- $file:./TestData/EventViewer/Files/EventViewerInput3.txt 5.15.55.111 Admin diff --git a/Recipes/Windows/EventViewer/RDP_FailedLogin_312.xml b/Recipes/Windows/EventViewer/RDP_FailedLogin_312.xml new file mode 100644 index 00000000..b9a416da --- /dev/null +++ b/Recipes/Windows/EventViewer/RDP_FailedLogin_312.xml @@ -0,0 +1,29 @@ + + + RDP + 0x4000000000000000 + Microsoft-Windows-TerminalServices-Gateway/Operational + 0 + + + //EventID + ^312$ + + + //Username + + .+) + ]]> + + + + //IpAddress + + .+) + ]]> + + + + diff --git a/Recipes/Windows/LogFile/Mediawiki.xml b/Recipes/Windows/LogFile/Mediawiki.xml new file mode 100644 index 00000000..9cf1c289 --- /dev/null +++ b/Recipes/Windows/LogFile/Mediawiki.xml @@ -0,0 +1,32 @@ + + + MediaWiki + E:/MediaWikiLogs/auth.log + + \d{4}-\d{2}-\d{2}\s\d{2}:\d{2}:\d{2})\s.*?:\s+(?:FAIL|ATTEMPT)\s+user=(?\S*)\s+ip=(?\S+) + ]]> + + yyyy-MM-dd HH:mm:ss + + \d{4}-\d{2}-\d{2}\s\d{2}:\d{2}:\d{2})\s.*?:\s+SUCCESS\s+user=(?\S*)\s+ip=(?\S+) + ]]> + + yyyy-MM-dd HH:mm:ss + Windows + 10000 + 0 + 0 + diff --git a/azure-pipelines.yml b/azure-pipelines.yml index f5e3e3d2..4c5df22f 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -19,7 +19,7 @@ steps: displayName: 'Install .Net SDK' inputs: packageType: 'sdk' - version: '9.x' + version: '10.x' includePreviewVersions: false installationPath: $(Agent.ToolsDirectory)/dotnet_latest diff --git a/coverage/coverlet.runsettings b/coverage/coverlet.runsettings new file mode 100644 index 00000000..eb2f63ce --- /dev/null +++ b/coverage/coverlet.runsettings @@ -0,0 +1,43 @@ + + + + + + + + cobertura,opencover,lcov,json + + + [DigitalRuby.IPBanCore]* + + + + [DigitalRuby.IPBanTests]*, + [*.Tests]*, + [DigitalRuby.IPBanCore]DigitalRuby.IPBanCore.Windows.COM.*, + [DigitalRuby.IPBanCore]*.IPBanResources, + [DigitalRuby.IPBanCore]*.AssemblyInfo, + [*]*.Designer + + + + **/obj/**,**/bin/**,**/*.Designer.cs,**/IPBanResources.Designer.cs,**/Windows/COM/*.cs + + + + Obsolete,GeneratedCodeAttribute,CompilerGeneratedAttribute,ExcludeFromCodeCoverageAttribute + + + + false + false + false + + + + + diff --git a/coverage/run.ps1 b/coverage/run.ps1 new file mode 100644 index 00000000..b1cca444 --- /dev/null +++ b/coverage/run.ps1 @@ -0,0 +1,99 @@ +#!/usr/bin/env pwsh +<# +.SYNOPSIS + Runs the IPBanTests test suite under coverlet and produces a raw + HTML coverage report. + +.DESCRIPTION + PowerShell port of coverage/run.sh. Invokes `dotnet test` with the XPlat Code Coverage + collector against IPBanTests/IPBanTests.csproj, prints a summary derived from the + cobertura XML, and (if reportgenerator is installed) renders an HTML report. + +.PARAMETER Filter + Optional NUnit / dotnet-test filter expression passed through to `dotnet test --filter`. + Example: -Filter "Category!=LinuxIntegrationSlow". + +.EXAMPLE + .\coverage\run.ps1 + .\coverage\run.ps1 -Filter "FullyQualifiedName~IPBanFirewallTests" +#> +param( + [string]$Filter = "" +) + +$ErrorActionPreference = "Stop" + +$scriptDir = $PSScriptRoot +$repoRoot = Split-Path -Parent $scriptDir +Set-Location $repoRoot + +# Install reportgenerator on demand +if (-not (Get-Command reportgenerator -ErrorAction SilentlyContinue)) { + if (Get-Command dotnet -ErrorAction SilentlyContinue) { + Write-Host ">> installing dotnet-reportgenerator-globaltool" -ForegroundColor Cyan + try { dotnet tool install -g dotnet-reportgenerator-globaltool | Out-Null } catch { } + $toolsDir = Join-Path $env:USERPROFILE ".dotnet\tools" + if (Test-Path $toolsDir -and ($env:Path -notlike "*$toolsDir*")) { + $env:Path = "$env:Path;$toolsDir" + } + } +} + +$resultsDir = Join-Path $repoRoot "coverage/results" +$reportDir = Join-Path $repoRoot "coverage/report" + +# Wipe old results so we don't aggregate across runs +Remove-Item -Recurse -Force $resultsDir, $reportDir -ErrorAction SilentlyContinue +New-Item -ItemType Directory -Force -Path $resultsDir | Out-Null + +Write-Host ">> running tests with coverage collection" -ForegroundColor Cyan +$testArgs = @( + "IPBanTests/IPBanTests.csproj", + "--collect:XPlat Code Coverage", + "--settings", (Join-Path $scriptDir "coverlet.runsettings"), + "--results-directory", $resultsDir, + "-c", "Release", + "--logger", "console;verbosity=minimal" +) +if ($Filter) { $testArgs += @("--filter", $Filter) } + +dotnet test @testArgs + +# Locate the cobertura xml the collector wrote +$cobertura = Get-ChildItem -Path $resultsDir -Recurse -Filter "coverage.cobertura.xml" | + Select-Object -First 1 -ExpandProperty FullName +if (-not $cobertura) { + Write-Error ">> no coverage.cobertura.xml produced — check that coverlet.collector is referenced in IPBanTests.csproj" + exit 1 +} +Write-Host ">> raw report: $cobertura" -ForegroundColor Green + +# Print a summary using summary.py (matches run.sh output) — fall back to inline parse if python is missing +$summaryScript = Join-Path $scriptDir "summary.py" +$python = $null +foreach ($candidate in @("python3", "python", "py")) { + if (Get-Command $candidate -ErrorAction SilentlyContinue) { $python = $candidate; break } +} + +if ($python -and (Test-Path $summaryScript)) { + & $python $summaryScript $cobertura +} else { + Write-Host "" + Write-Host "=== Coverage summary ===" -ForegroundColor Yellow + [xml]$xml = Get-Content $cobertura + $lineRate = [double]$xml.coverage.'line-rate' * 100 + $branchRate = [double]$xml.coverage.'branch-rate' * 100 + " line coverage: {0,6:N2}%" -f $lineRate + " branch coverage: {0,6:N2}%" -f $branchRate +} + +# Render HTML if reportgenerator is available +if (Get-Command reportgenerator -ErrorAction SilentlyContinue) { + Write-Host ">> generating HTML report at coverage/report/index.html" -ForegroundColor Cyan + reportgenerator ` + -reports:"$cobertura" ` + -targetdir:"$reportDir" ` + -reporttypes:"Html;Badges;TextSummary" | Out-Null + Write-Host ">> open coverage/report/index.html in a browser" -ForegroundColor Green +} else { + Write-Host ">> reportgenerator not on PATH — skipped HTML render" -ForegroundColor Yellow +} diff --git a/coverage/run.sh b/coverage/run.sh new file mode 100644 index 00000000..785e0a20 --- /dev/null +++ b/coverage/run.sh @@ -0,0 +1,121 @@ +#!/usr/bin/env bash +# Run the IPBan test suite under coverlet and produce raw + HTML coverage reports. +# Usage: +# coverage/run.sh # all tests +# coverage/run.sh "Category!=LinuxIntegrationSlow" # pass-through filter +# +# Outputs (all under the coverage/ folder so the repo root stays clean): +# coverage/results//coverage.cobertura.xml raw report +# coverage/report/index.html HTML report (if reportgenerator is installed) + +set -euo pipefail + +# Resolve repo root regardless of where the script is invoked from. +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +cd "$REPO_ROOT" + +FILTER="${1:-}" + +# install reportgenerator if missing +if ! command -v reportgenerator >/dev/null 2>&1; then + if command -v dotnet >/dev/null 2>&1; then + echo ">> installing dotnet-reportgenerator-globaltool" + dotnet tool install -g dotnet-reportgenerator-globaltool || true + export PATH="$PATH:$HOME/.dotnet/tools" + fi +fi + +# clear old results so we don't aggregate across runs +rm -rf coverage/results coverage/report +mkdir -p coverage/results + +echo ">> running tests with coverage collection" +TEST_ARGS=( + "IPBanTests/IPBanTests.csproj" + "--collect:XPlat Code Coverage" + "--settings" "$SCRIPT_DIR/coverlet.runsettings" + "--results-directory" "$REPO_ROOT/coverage/results" + "-c" "Release" + "--logger" "console;verbosity=minimal" +) +if [ -n "$FILTER" ]; then + TEST_ARGS+=("--filter" "$FILTER") +fi +dotnet test "${TEST_ARGS[@]}" + +# locate the cobertura xml the collector wrote +COBERTURA="$(find coverage/results -name 'coverage.cobertura.xml' | head -n1)" +if [ -z "$COBERTURA" ]; then + echo ">> ERROR: no coverage.cobertura.xml produced — check that coverlet.collector is referenced in IPBanTests.csproj" + exit 1 +fi +echo ">> raw report: $COBERTURA" + +# print a quick text summary using the cobertura XML +python3 - "$COBERTURA" <<'PY' +import sys, xml.etree.ElementTree as ET +path = sys.argv[1] +tree = ET.parse(path) +root = tree.getroot() +line_rate = float(root.attrib.get('line-rate', 0)) * 100 +branch_rate = float(root.attrib.get('branch-rate', 0)) * 100 +print() +print(f"=== Coverage summary ===") +print(f" line coverage: {line_rate:6.2f}%") +print(f" branch coverage: {branch_rate:6.2f}%") +print() +# print bottom 15 files by line coverage so gaps are obvious +# Aggregate by file (cobertura emits one per type, so a file with multiple types +# shows up multiple times — we collapse by filename). +agg = {} # filename -> [covered_lines, total_lines] +for cls in root.iter('class'): + name = cls.attrib.get('filename', cls.attrib.get('name', '?')) + lines = cls.find('lines') + if lines is None: continue + line_list = lines.findall('line') + total = len(line_list) + covered = sum(1 for l in line_list if int(l.attrib.get('hits', 0)) > 0) + if name not in agg: + agg[name] = [0, 0] + agg[name][0] += covered + agg[name][1] += total + +# Strip the long github.com prefix if SourceLink is enabled +def short(p): + for marker in ('IPBanCore/', '/IPBanCore/'): + i = p.rfind(marker) + if i >= 0: return p[i + (1 if marker.startswith('/') else 0):] + return p + +# 20 files with the lowest line coverage (>= 10 lines, to skip trivial cases) +ranked = sorted( + ((cov / total * 100 if total else 0, total, short(name)) for name, (cov, total) in agg.items() if total >= 10), + key=lambda x: (x[0], -x[1])) +print("=== 20 files with the lowest line coverage (>=10 lines) ===") +print(f" {'cov%':>6} {'lines':>5} file") +for rate, total, name in ranked[:20]: + print(f" {rate:6.2f} {total:5d} {name}") +# Also show files with low coverage AND many lines — these are the highest-leverage targets +ranked_by_uncovered = sorted( + (((total - cov), cov / total * 100 if total else 0, total, short(name)) for name, (cov, total) in agg.items() if total >= 30), + key=lambda x: -x[0]) +print() +print("=== 15 files with the most uncovered lines (>=30 total) ===") +print(f" {'uncov':>5} {'cov%':>6} {'lines':>5} file") +for uncov, rate, total, name in ranked_by_uncovered[:15]: + print(f" {uncov:5d} {rate:6.2f} {total:5d} {name}") +PY + +# render HTML if reportgenerator is available +if command -v reportgenerator >/dev/null 2>&1; then + echo ">> generating HTML report at coverage/report/index.html" + reportgenerator \ + -reports:"$COBERTURA" \ + -targetdir:coverage/report \ + -reporttypes:"Html;Badges;TextSummary" \ + >/dev/null + echo ">> open coverage/report/index.html in a browser" +else + echo ">> reportgenerator not on PATH — skipped HTML render" +fi diff --git a/coverage/summary.py b/coverage/summary.py new file mode 100644 index 00000000..34566b31 --- /dev/null +++ b/coverage/summary.py @@ -0,0 +1,51 @@ +import sys, xml.etree.ElementTree as ET, glob, os +path = sys.argv[1] if len(sys.argv) > 1 else None +if not path: + matches = glob.glob('coverage/results/**/coverage.cobertura.xml', recursive=True) + path = matches[0] if matches else None +if not path: + print('no coverage.cobertura.xml found') + sys.exit(1) +print(f"reading: {path}") +tree = ET.parse(path) +root = tree.getroot() +line_rate = float(root.attrib.get('line-rate', 0)) * 100 +branch_rate = float(root.attrib.get('branch-rate', 0)) * 100 +print(f"=== Coverage summary ===") +print(f" line coverage: {line_rate:6.2f}%") +print(f" branch coverage: {branch_rate:6.2f}%") +print() +agg = {} +for cls in root.iter('class'): + name = cls.attrib.get('filename', cls.attrib.get('name', '?')) + lines = cls.find('lines') + if lines is None: continue + line_list = lines.findall('line') + total = len(line_list) + covered = sum(1 for l in line_list if int(l.attrib.get('hits', 0)) > 0) + if name not in agg: + agg[name] = [0, 0] + agg[name][0] += covered + agg[name][1] += total + +def short(p): + for marker in ('IPBanCore\\', 'IPBanCore/', '/IPBanCore/'): + i = p.rfind(marker) + if i >= 0: return p[i + (1 if marker.startswith('/') else 0):] + return p + +ranked = sorted( + ((cov / total * 100 if total else 0, total, short(name)) for name, (cov, total) in agg.items() if total >= 10), + key=lambda x: (x[0], -x[1])) +print("=== 40 files with the lowest line coverage (>=10 lines) ===") +print(f" {'cov%':>6} {'lines':>5} file") +for rate, total, name in ranked[:40]: + print(f" {rate:6.2f} {total:5d} {name}") +ranked_by_uncovered = sorted( + (((total - cov), cov / total * 100 if total else 0, total, short(name)) for name, (cov, total) in agg.items() if total >= 30), + key=lambda x: -x[0]) +print() +print("=== 30 files with the most uncovered lines (>=30 total) ===") +print(f" {'uncov':>5} {'cov%':>6} {'lines':>5} file") +for uncov, rate, total, name in ranked_by_uncovered[:30]: + print(f" {uncov:5d} {rate:6.2f} {total:5d} {name}")