LMBQ-267: Fixing that only a single security scan could run at the same time, extending configurability, adjusting full scan config

Lombiq.Tests.UI.Shortcuts/Controllers/ErrorController.cs

@@ -11,6 +11,5 @@ public class ErrorController : Controller
     public const string ExceptionMessage = "This action intentionally causes an exception!";
 
     [AllowAnonymous]
-    public IActionResult Index() =>
-        throw new InvalidOperationException(ExceptionMessage);
+    public IActionResult Index() => throw new InvalidOperationException(ExceptionMessage);
 }

Lombiq.Tests.UI/Docs/SecurityScanning.md

@@ -41,9 +41,10 @@ This is because the Docker installation is configured to use Windows images, whi
 
 - Most common alerts in Orchard Core can be resolved by [using the extension method in Lombiq Helpful Libraries](https://github.com/Lombiq/Helpful-Libraries/blob/dev/Lombiq.HelpfulLibraries.OrchardCore/Docs/Security.md) like this: `orchardCoreBuilder.ConfigureSecurityDefaults(allowInlineStyle: true)`.
 - If you're unsure what happens in a scan, run the [ZAP desktop app](https://www.zaproxy.org/download/) and load the Automation Framework plan's YAML file into it. If you use the default scans, then these will be available under the build output directory (like _bin/Debug_) under _SecurityScanning/AutomationFrameworkPlans_. Then, you can open and run them as demonstrated [in this video](https://youtu.be/PnCbIAnauD8?si=u0vi63Uvv9wZINzb&t=1173).
-- If an alert is a false positive, follow [the official docs](https://www.zaproxy.org/faq/how-do-i-handle-a-false-positive/). You can use the [`alertFilter` job](https://www.zaproxy.org/docs/desktop/addons/alert-filters/automation/) to ignore alerts in very specific conditions. You can also access this via the .NET configuration API.
+- If an alert is a false positive, follow [the official docs](https://www.zaproxy.org/faq/how-do-i-handle-a-false-positive/). You can use the [`alertFilter` job](https://www.zaproxy.org/docs/desktop/addons/alert-filters/automation/) to ignore alerts in very specific conditions. You can also access this via the .NET configuration API via `SecurityScanConfiguration.MarkScanRuleAsFalsePositiveForUrlWithRegex()`.
 - ZAP didn't find everything in your app? By default, ZAP has a crawl depth of 5 for its standard spider and 10 for its AJAX spider. Set `maxDepth` (and `maxChildren`) [for `spider`](https://www.zaproxy.org/docs/desktop/addons/automation-framework/job-spider/) and `maxCrawlDepth` [for `spiderAjax`](https://www.zaproxy.org/docs/desktop/addons/ajax-spider/automation/).
 - Do you sometimes get slightly different scan results? This is normal, and ZAP can be inconsistent/appear random within limits, see [the official docs page](https://www.zaproxy.org/faq/why-can-zap-scans-be-inconsistent/).
 - Is the active scan too slow?
   - You can find out which rules take the most time by adding a script displaying each rules' runtime with `YamlDocumentExtensions.AddDisplayActiveScanRuleRuntimesScriptAfterActiveScan()`.
-  - The ["Cross Site Scripting (DOM Based)" active scan rule](https://www.zaproxy.org/docs/desktop/addons/dom-xss-active-scan-rule/), unlike other rules, launches browsers and thus will take 1-2 orders of magnitude more time than other scans, usually causing the bulk of an active scan's runtime. Also see [the official docs](https://www.zaproxy.org/docs/desktop/addons/dom-xss-active-scan-rule/). You can tune it so it completes faster but still produces acceptable results for your app. You can do this from the Automation Framework plan's YAML file (see the samples on how you can use a custom one), or with `SecurityScanConfiguration.ConfigureActiveScanRule()`.
+  - The ["Cross Site Scripting (DOM Based)" active scan rule](https://www.zaproxy.org/docs/desktop/addons/dom-xss-active-scan-rule/), unlike other rules, launches browsers and thus will take 1-2 orders of magnitude more time than other scans, usually causing the bulk of an active scan's runtime. Also see [the official docs](https://www.zaproxy.org/docs/desktop/addons/dom-xss-active-scan-rule/). You can tune it so it completes faster but still produces acceptable results for your app. You can do this from the Automation Framework plan's YAML file (see the samples on how you can use a custom one), or with `SecurityScanConfiguration.ConfigureXssActiveScanRule()`.
+  - In CI workflows, you might want to restrict how many scans run in parallel, if you have more than one. You can use [xUnit's `[Collection]` attributes](https://xunit.net/docs/running-tests-in-parallel#parallelism-in-test-frameworks) to have e.g. only two collections for such tests, thus allowing only two parallel scans.

Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml

@@ -16,21 +16,24 @@ env:
       parameters: {}
     technology:
       exclude:
-      - "C"
-      - "IBM DB2"
-      - "PHP"
       - "CouchDB"
-      - "Oracle"
-      - "JSP/Servlet"
       - "Firebird"
       - "HypersonicSQL"
-      - "SAP MaxDB"
-      - "Ruby"
+      - "IBM DB2"
       - "Microsoft Access"
-      - "Java"
-      - "Tomcat"
+      - "MongoDB"
+      - "Oracle"
+      - "SAP MaxDB"
       - "Sybase"
+      - "Java"
+      - "C"
+      - "JSP/Servlet"
+      - "Java"
+      - "PHP"
       - "Python"
+      - "Ruby"
+      - "Apache"
+      - "Tomcat"
   parameters:
     failOnError: true
     failOnWarning: false

Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml

@@ -16,21 +16,24 @@ env:
       parameters: {}
     technology:
       exclude:
-      - "C"
-      - "IBM DB2"
-      - "PHP"
       - "CouchDB"
-      - "Oracle"
-      - "JSP/Servlet"
       - "Firebird"
       - "HypersonicSQL"
-      - "SAP MaxDB"
-      - "Ruby"
+      - "IBM DB2"
       - "Microsoft Access"
-      - "Java"
-      - "Tomcat"
+      - "MongoDB"
+      - "Oracle"
+      - "SAP MaxDB"
       - "Sybase"
+      - "Java"
+      - "C"
+      - "JSP/Servlet"
+      - "Java"
+      - "PHP"
       - "Python"
+      - "Ruby"
+      - "Apache"
+      - "Tomcat"
   parameters:
     failOnError: true
     failOnWarning: false
@@ -62,6 +65,26 @@ jobs:
     threshold: "high"
   name: "passiveScan-config"
   type: "passiveScan-config"
+- alertFilters:
+  # Mistakes a "system-property('xsl:vendor')" XSLT injection attempt as successful due to "Microsoft" being there on
+  # the login screen at all times for External Login. Might happen in similar cases with other brand names too.
+  - ruleId: "90017"
+    ruleName: "XSLT Injection (90017)"
+    context: ""
+    newRisk: "False Positive"
+    parameter: ""
+    parameterRegex: false
+    url: ".*/(ChangePassword|Account/LinkLogin|Account/ExternalLogin|Users/LogOff).*"
+    urlRegex: true
+    attack: ""
+    attackRegex: false
+    evidence: ""
+    evidenceRegex: false
+    methods: []
+  parameters:
+    deleteGlobalAlerts: false
+  name: "alertFilter"
+  type: "alertFilter"
 - parameters: {}
   name: "spider"
   type: "spider"

Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs

@@ -1,5 +1,6 @@
 using Lombiq.HelpfulLibraries.OrchardCore.Mvc;
 using Lombiq.Tests.UI.Constants;
+using Lombiq.Tests.UI.Extensions;
 using Lombiq.Tests.UI.Services;
 using Lombiq.Tests.UI.Shortcuts.Controllers;
 using System;
@@ -33,6 +34,8 @@ public class SecurityScanConfiguration
     public Uri StartUri { get; private set; }
     public bool AjaxSpiderIsUsed { get; private set; }
     public string SignInUserName { get; private set; }
+    public bool AdminIsExcluded { get; private set; }
+    public bool UnusedDatabaseTechnologiesAreExcluded { get; private set; } = true;
 
     /// <summary>
     /// Gets a value indicating whether the security scan should not visit the <see cref="ErrorController"/> to test
@@ -100,6 +103,33 @@ public SecurityScanConfiguration ExcludeUrlWithRegex(string excludedUrlRegex)
         return this;
     }
 
+    /// <summary>
+    /// Excludes the Orchard Core admin area (dashboard), under /Admin by default, from the scan, or disables the
+    /// exclusion.
+    /// </summary>
+    /// <param name="adminIsExcluded">
+    /// Indicates whether the Orchard Core admin area (dashboard) should be excluded from the scan.
+    /// </param>
+    public SecurityScanConfiguration ExcludeAdmin(bool adminIsExcluded = true)
+    {
+        AdminIsExcluded = adminIsExcluded;
+        return this;
+    }
+
+    /// <summary>
+    /// Excludes those database engines from the technologies ZAP tailors attacks for that aren't currently used for
+    /// running the app.
+    /// </summary>
+    /// <param name="unusedDatabaseTechnologiesAreExcluded">
+    /// Indicates whether those database engines are excluded from the technologies ZAP tailors attacks for that aren't
+    /// currently used for running the app.
+    /// </param>
+    public SecurityScanConfiguration ExcludeUnusedDatabaseTechnologies(bool unusedDatabaseTechnologiesAreExcluded = true)
+    {
+        UnusedDatabaseTechnologiesAreExcluded = unusedDatabaseTechnologiesAreExcluded;
+        return this;
+    }
+
     /// <summary>
     /// Disable a certain active scan rule for the whole scan. If you only want to disable a rule for specific pages
     /// matched by a regex, use <see cref="DisableScanRuleForUrlWithRegex(string, int, string)"/> instead.
@@ -137,6 +167,25 @@ public SecurityScanConfiguration ConfigureActiveScanRule(int id, ScanRuleThresho
         return this;
     }
 
+    /// <summary>
+    /// Configures the <see href="https://www.zaproxy.org/docs/alerts/40026/">Cross Site Scripting (DOM Based)</see>
+    /// active scan rule for the whole scan. Since this scan takes usually the most time of an active scan, you may want
+    /// to reduce its strength at least, depending on your specific requirements.
+    /// </summary>
+    /// <param name="threshold">
+    /// Controls how likely ZAP is to report potential vulnerabilities. See <see
+    /// href="https://www.zaproxy.org/docs/desktop/ui/dialogs/scanpolicy/#threshold">the official docs</see>.
+    /// </param>
+    /// <param name="strength">
+    /// Controls the number of attacks that ZAP will perform. See <see
+    /// href="https://www.zaproxy.org/docs/desktop/ui/dialogs/scanpolicy/#strength">the official docs</see>.
+    /// </param>
+    public SecurityScanConfiguration ConfigureXssActiveScanRule(ScanRuleThreshold threshold, ScanRuleStrength strength) // #spell-check-ignore-line
+    {
+        ConfigureActiveScanRule(40026, threshold, strength, "Cross Site Scripting (DOM Based)");
+        return this;
+    }
+
     /// <summary>
     /// Disable a certain passive scan rule for the whole scan. If you only want to disable a rule for specific pages
     /// matched by a regex, use <see cref="DisableScanRuleForUrlWithRegex(string, int, string)"/> instead.
@@ -247,7 +296,10 @@ internal async Task ApplyToPlanAsync(YamlDocument yamlDocument, UITestContext co
     {
         yamlDocument.SetStartUrl(StartUri);
 
-        foreach (var uri in _additionalUris) yamlDocument.AddUrl(uri);
+        foreach (var uri in _additionalUris)
+        {
+            yamlDocument.AddUrl(uri.IsAbsoluteUri ? uri : context.GetAbsoluteUri(uri.OriginalString));
+        }
 
         if (AjaxSpiderIsUsed) yamlDocument.AddSpiderAjaxAfterSpider();
 
@@ -279,6 +331,31 @@ internal async Task ApplyToPlanAsync(YamlDocument yamlDocument, UITestContext co
 #pragma warning disable S3878 // Arrays should not be created for params parameters
         yamlDocument.AddExcludePathsRegex([.. _excludedUrlRegexPatterns]);
 #pragma warning restore S3878 // Arrays should not be created for params parameters
+
+        if (AdminIsExcluded) yamlDocument.AddExcludePathsRegex($".*{context.AdminUrlPrefix}.*");
+
+        if (UnusedDatabaseTechnologiesAreExcluded)
+        {
+            var excludes = yamlDocument
+                .GetCurrentContext()
+                .GetOrAddNode<YamlMappingNode>("technology")
+                .GetOrAddNode<YamlSequenceNode>("exclude");
+
+            if (context.Configuration.UseSqlServer)
+            {
+                excludes.Add(new YamlScalarNode("MySQL"));
+                excludes.Add(new YamlScalarNode("PostgreSQL"));
+                excludes.Add(new YamlScalarNode("SQLite"));
+            }
+            else
+            {
+                // Assuming SQLite.
+                excludes.Add(new YamlScalarNode("Microsoft SQL Server"));
+                excludes.Add(new YamlScalarNode("MySQL"));
+                excludes.Add(new YamlScalarNode("PostgreSQL"));
+            }
+        }
+
         foreach (var rule in _disabledActiveScanRules) yamlDocument.DisableActiveScanRule(rule.Id, rule.Name);
 
         foreach (var ruleConfiguration in _configuredActiveScanRules)

Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs

@@ -72,7 +72,7 @@ public static Task RunAndConfigureAndAssertFullSecurityScanForContinuousIntegrat
         // Ignore some validation errors that only happen during security tests.
         context.Configuration.UseAssertAppLogsForSecurityScan();
 
-        // This takes over 10 minutes and the session will certainly time out with retries.
+        // This can take even over 10 minutes and the CI session would certainly time out with retries.
         context.Configuration.MaxRetryCount = 0;
 
         return context.RunAndAssertFullSecurityScanAsync(

Lombiq.Tests.UI/SecurityScanning/ZapManager.cs

@@ -33,12 +33,21 @@ public sealed class ZapManager : IAsyncDisposable
 
     private static readonly SemaphoreSlim _pullSemaphore = new(1, 1);
     private static readonly CliProgram _docker = new("docker");
+    private static readonly PortLeaseManager _portLeaseManager;
 
     private readonly ITestOutputHelper _testOutputHelper;
     private readonly CancellationTokenSource _cancellationTokenSource = new();
 
     private static bool _wasPulled;
 
+    private int _zapPort;
+
+    static ZapManager()
+    {
+        var agentIndexTimesHundred = TestConfigurationManager.GetAgentIndexOrDefault() * 100;
+        _portLeaseManager = new PortLeaseManager(15000 + agentIndexTimesHundred, 15099 + agentIndexTimesHundred);
+    }
+
     internal ZapManager(ITestOutputHelper testOutputHelper) => _testOutputHelper = testOutputHelper;
 
     /// <summary>
@@ -122,6 +131,11 @@ public async Task<SecurityScanResult> RunSecurityScanAsync(
             cliParameters.Add("localhost:host-gateway");
         }
 
+        // Using a different port than the default 8080 is necessary so ZAP doesn't clash with other web processes and
+        // to allow more than one security scan to run at the same time.
+        _zapPort = await _portLeaseManager.LeaseAvailableRandomPortAsync();
+        _testOutputHelper.WriteLineTimestampedAndDebug("Running ZAP on port {0}.", _zapPort);
+
         cliParameters.AddRange(new object[]
         {
             "--rm",
@@ -133,6 +147,8 @@ public async Task<SecurityScanResult> RunSecurityScanAsync(
             "-cmd",
             "-autorun",
             _zapWorkingDirectoryPath + yamlFileName,
+            "-port",
+            _zapPort,
         });
 
         var stdErrBuffer = new StringBuilder();
@@ -182,6 +198,8 @@ public async ValueTask DisposeAsync()
             await _cancellationTokenSource.CancelAsync();
             _cancellationTokenSource.Dispose();
         }
+
+        await _portLeaseManager.StopLeaseAsync(_zapPort);
     }
 
     private async Task EnsureInitializedAsync()

Lombiq.Tests.UI/Services/OrchardCoreUITestExecutorConfiguration.cs

@@ -247,6 +247,9 @@ public static Func<IWebApplicationInstance, Task> CreateAppLogAssertionForSecuri
             // One way to verify correct error handling is to navigate to ~/Lombiq.Tests.UI.Shortcuts/Error/Index, which
             // always throws an exception. This also gets logged but it's expected, so it should be ignored.
             ErrorController.ExceptionMessage,
+            // Thrown from Microsoft.AspNetCore.Authentication.AuthenticationService.ChallengeAsync() when ZAP sends
+            // invalid authentication challenges.
+            "System.InvalidOperationException: No authentication handler is registered for the scheme",
         };
 
         permittedErrorLines.AddRange(additionalPermittedErrorLines);