From 816c476d8a72e0a53a23be057a4c9c1b4e73b8e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Tue, 26 Dec 2023 13:26:08 +0100 Subject: [PATCH 01/87] Update baseline rule exception. --- .../SecurityScanning/AutomationFrameworkPlans/Baseline.yml | 5 +++++ .../SecurityScanning/AutomationFrameworkPlans/FullScan.yml | 5 +++++ .../SecurityScanning/AutomationFrameworkPlans/GraphQL.yml | 5 +++++ .../SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml | 5 +++++ 4 files changed, 20 insertions(+) diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml index 3ebde1b96..51052fe72 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml @@ -45,6 +45,11 @@ jobs: - id: 10035 name: "Strict-Transport-Security Header" threshold: "off" + # Some stock Orchard Core shapes and pages (including the setup page) contain inline script blocks, so this directive + # must be permitted for OC to work at all. + - id: 10055 + name: "script-src includes unsafe-inline" + threshold: "off" name: "passiveScan-config" type: "passiveScan-config" - parameters: {} diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml index 19bf336b2..204883d31 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml @@ -45,6 +45,11 @@ jobs: - id: 10035 name: "Strict-Transport-Security Header" threshold: "off" + # Some stock Orchard Core shapes and pages (including the setup page) contain inline script blocks, so this directive + # must be permitted for OC to work at all. + - id: 10055 + name: "script-src includes unsafe-inline" + threshold: "off" name: "passiveScan-config" type: "passiveScan-config" - parameters: {} diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml index 24f14d902..70c1b1e92 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml @@ -45,6 +45,11 @@ jobs: - id: 10035 name: "Strict-Transport-Security Header" threshold: "off" + # Some stock Orchard Core shapes and pages (including the setup page) contain inline script blocks, so this directive + # must be permitted for OC to work at all. + - id: 10055 + name: "script-src includes unsafe-inline" + threshold: "off" name: "passiveScan-config" type: "passiveScan-config" - parameters: diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml index 9327dc09a..836bfc53a 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml @@ -45,6 +45,11 @@ jobs: - id: 10035 name: "Strict-Transport-Security Header" threshold: "off" + # Some stock Orchard Core shapes and pages (including the setup page) contain inline script blocks, so this directive + # must be permitted for OC to work at all. + - id: 10055 + name: "script-src includes unsafe-inline" + threshold: "off" name: "passiveScan-config" type: "passiveScan-config" - parameters: {} From 7856594cd2a265cb4b7033f981e6e8b4596aad00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Tue, 26 Dec 2023 14:53:33 +0100 Subject: [PATCH 02/87] The baseline scan should visit at least one page that throws an exception. --- .../SecurityScanningUITestContextExtensions.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs index 95c5c790c..7c8276ea0 100644 --- a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs +++ b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs @@ -1,6 +1,8 @@ +using Lombiq.HelpfulLibraries.OrchardCore.Mvc; using Lombiq.Tests.UI.Exceptions; using Lombiq.Tests.UI.Extensions; using Lombiq.Tests.UI.Services; +using Lombiq.Tests.UI.Shortcuts.Controllers; using Microsoft.CodeAnalysis.Sarif; using System; using System.Threading.Tasks; @@ -25,7 +27,14 @@ public static Task RunAndAssertBaselineSecurityScanAsync( Action assertSecurityScanResult = null) => context.RunAndAssertSecurityScanAsync( AutomationFrameworkPlanPaths.BaselinePlanPath, - configure, + configuration => + { + // Make sure to visit at least one page that throws an exception. + var errorPageRelativeUrl = context.GetRelativeUrlOfAction(error => error.Index()); + configuration.ModifyZapPlan(plan => plan.AddUrl(context.GetAbsoluteUri(errorPageRelativeUrl))); + + configure?.Invoke(configuration); + }, assertSecurityScanResult); /// From 222be4759f2a50907aea8ac7d09f71624e08d5b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Tue, 26 Dec 2023 17:48:23 +0100 Subject: [PATCH 03/87] Disable app log assertion for the duration of the security scan. --- .../SecurityScanningUITestContextExtensions.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs index 7c8276ea0..9e4a4d2fd 100644 --- a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs +++ b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs @@ -4,6 +4,7 @@ using Lombiq.Tests.UI.Services; using Lombiq.Tests.UI.Shortcuts.Controllers; using Microsoft.CodeAnalysis.Sarif; +using Shouldly; using System; using System.Threading.Tasks; @@ -116,6 +117,11 @@ public static async Task RunAndAssertSecurityScanAsync( Action configure = null, Action assertSecurityScanResult = null) { + // Verify that the app logs are fine right now, then disable app log assertion for the duration of this scan. + await context.Configuration.AssertAppLogsAsync(context.Application); + var assertAppLogsAsync = context.Configuration.AssertAppLogsAsync; + context.Configuration.AssertAppLogsAsync = _ => Task.CompletedTask; + var configuration = context.Configuration.SecurityScanningConfiguration; SecurityScanResult result = null; @@ -133,6 +139,10 @@ public static async Task RunAndAssertSecurityScanAsync( if (result != null) context.AppendDirectoryToFailureDump(result.ReportsDirectoryPath); throw new SecurityScanningAssertionException(ex); } + + // Clear app logs before app log assertion is restored. + context.ClearLogs(); + context.Configuration.AssertAppLogsAsync = assertAppLogsAsync; } /// From 6cfb21ddcad7813483457974ff09a30bac9f7983 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Tue, 26 Dec 2023 18:32:31 +0100 Subject: [PATCH 04/87] unusing --- .../SecurityScanning/SecurityScanningUITestContextExtensions.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs index 9e4a4d2fd..350834efe 100644 --- a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs +++ b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs @@ -4,7 +4,6 @@ using Lombiq.Tests.UI.Services; using Lombiq.Tests.UI.Shortcuts.Controllers; using Microsoft.CodeAnalysis.Sarif; -using Shouldly; using System; using System.Threading.Tasks; From b60dfe0f8eb07f561fe6eb591ec626d10a07c19c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Tue, 26 Dec 2023 20:10:16 +0100 Subject: [PATCH 05/87] Expect custom error page. --- .../SecurityScanning/AutomationFrameworkPlans/Baseline.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml index 51052fe72..e7fab4e3e 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml @@ -50,6 +50,9 @@ jobs: - id: 10055 name: "script-src includes unsafe-inline" threshold: "off" + - id: 90022 + name: "Application Error Disclosure" + threshold: "low" name: "passiveScan-config" type: "passiveScan-config" - parameters: {} From eea23bd977f5cdbcfd76169864d78257ca78c816 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Tue, 26 Dec 2023 22:16:30 +0100 Subject: [PATCH 06/87] Disable rule 10037. --- .../SecurityScanning/AutomationFrameworkPlans/Baseline.yml | 7 ++++--- .../SecurityScanning/AutomationFrameworkPlans/FullScan.yml | 4 ++++ .../SecurityScanning/AutomationFrameworkPlans/GraphQL.yml | 4 ++++ .../SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml | 4 ++++ 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml index e7fab4e3e..b269322af 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml @@ -50,9 +50,10 @@ jobs: - id: 10055 name: "script-src includes unsafe-inline" threshold: "off" - - id: 90022 - name: "Application Error Disclosure" - threshold: "low" + # This is a low-risk alert, enforcing it is undesirable for branding. + - id: 10037 + name: "Server Leaks Information via 'X-Powered-By' HTTP Response Header Field(s)" + threshold: "off" name: "passiveScan-config" type: "passiveScan-config" - parameters: {} diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml index 204883d31..a87e4d2d3 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml @@ -50,6 +50,10 @@ jobs: - id: 10055 name: "script-src includes unsafe-inline" threshold: "off" + # This is a low-risk alert, enforcing it is undesirable for branding. + - id: 10037 + name: "Server Leaks Information via 'X-Powered-By' HTTP Response Header Field(s)" + threshold: "off" name: "passiveScan-config" type: "passiveScan-config" - parameters: {} diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml index 70c1b1e92..f88add172 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml @@ -50,6 +50,10 @@ jobs: - id: 10055 name: "script-src includes unsafe-inline" threshold: "off" + # This is a low-risk alert, enforcing it is undesirable for branding. + - id: 10037 + name: "Server Leaks Information via 'X-Powered-By' HTTP Response Header Field(s)" + threshold: "off" name: "passiveScan-config" type: "passiveScan-config" - parameters: diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml index 836bfc53a..43912d188 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml @@ -50,6 +50,10 @@ jobs: - id: 10055 name: "script-src includes unsafe-inline" threshold: "off" + # This is a low-risk alert, enforcing it is undesirable for branding. + - id: 10037 + name: "Server Leaks Information via 'X-Powered-By' HTTP Response Header Field(s)" + threshold: "off" name: "passiveScan-config" type: "passiveScan-config" - parameters: {} From 71f189d3016e6978a29b22c653f3cb692881bb6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Wed, 27 Dec 2023 21:51:42 +0100 Subject: [PATCH 07/87] Fix disabled rules, shouldn't have been dictionary. --- .../SecurityScanConfiguration.cs | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs b/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs index 4aad3938e..79bd71e52 100644 --- a/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs +++ b/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs @@ -32,7 +32,8 @@ public class SecurityScanConfiguration public IDictionary ConfiguredActiveScanRules { get; } = new Dictionary(); public IList DisabledPassiveScanRules { get; } = new List(); - public IDictionary DisabledRulesForUrls { get; } = new Dictionary(); + public IList<(string Url, int Id, string RuleName)> DisabledRulesForUrls { get; } = new List<(string Url, int Id, string RuleName)>(); + public IList<(string Url, int Id, string Justification)> FalsePositives { get; } = new List<(string Url, int Id, string Justification)>(); public IList> ZapPlanModifiers { get; } = new List>(); internal SecurityScanConfiguration() @@ -161,7 +162,30 @@ public SecurityScanConfiguration DisablePassiveScanRule(int id, string name = "" /// public SecurityScanConfiguration DisableScanRuleForUrlWithRegex(string urlRegex, int ruleId, string ruleName = "") { - DisabledRulesForUrls[urlRegex] = new ScanRule(ruleId, ruleName); + DisabledRulesForUrls.Add((urlRegex, ruleId, ruleName)); + return this; + } + + /// + /// Marks a rule (can be any rule, including e.g. both active or passive scan rules) for just URLs matching the + /// given regular expression pattern. + /// + /// + /// The regex pattern to match URLs against. It will be matched against the whole absolute URL, e.g., ".*blog.*" + /// will match https://example.com/blog, https://example.com/blog/my-post, etc. + /// + /// The ID of the rule. In the scan report, this is usually displayed as "Plugin Id". + /// + /// A human-readable explanation of why the alert is false positive. + /// + public SecurityScanConfiguration MarkScanRuleAsFalsePositiveForUrlWithRegex(string urlRegex, int ruleId, string justification) + { + if (string.IsNullOrWhiteSpace(justification)) + { + throw new InvalidOperationException("Please provide a justification for disabling this alert!"); + } + + FalsePositives.Add((urlRegex, ruleId, justification)); return this; } @@ -247,7 +271,8 @@ internal async Task ApplyToPlanAsync(YamlDocument yamlDocument, UITestContext co } foreach (var rule in DisabledPassiveScanRules) yamlDocument.DisablePassiveScanRule(rule.Id, rule.Name); - foreach (var urlToRule in DisabledRulesForUrls) yamlDocument.AddAlertFilter(urlToRule.Key, urlToRule.Value.Id, urlToRule.Value.Name); + foreach (var (url, id, name) in DisabledRulesForUrls) yamlDocument.AddAlertFilter(url, id, name); + foreach (var (url, id, justification) in FalsePositives) yamlDocument.AddAlertFilter(url, id, justification, isFalsePositive: true); foreach (var modifier in ZapPlanModifiers) await modifier(yamlDocument); } From e0115687cce9a572c92ae5d2651a8e8e5307220a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Wed, 27 Dec 2023 21:59:06 +0100 Subject: [PATCH 08/87] Turn all the collections into private, because we have methods to handle them in a specific restricted manner so exposing the original collections is only asking for shenanigans. --- .../SecurityScanConfiguration.cs | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs b/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs index 79bd71e52..2bc2f93f0 100644 --- a/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs +++ b/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs @@ -23,18 +23,18 @@ namespace Lombiq.Tests.UI.SecurityScanning; /// public class SecurityScanConfiguration { + private readonly List _additionalUris = new(); + private readonly List _excludedUrlRegexPatterns = new(); + private readonly List _disabledActiveScanRules = new(); + private readonly Dictionary _configuredActiveScanRules = new(); + private readonly List _disabledPassiveScanRules = new(); + private readonly List<(string Url, int Id, string RuleName)> _disabledRulesForUrls = new(); + private readonly List<(string Url, int Id, string Justification)> _falsePositives = new(); + private readonly List> _zapPlanModifiers = new(); + public Uri StartUri { get; private set; } - public IList AdditionalUris { get; } = new List(); public bool AjaxSpiderIsUsed { get; private set; } public string SignInUserName { get; private set; } - public IList ExcludedUrlRegexPatterns { get; } = new List(); - public IList DisabledActiveScanRules { get; } = new List(); - public IDictionary ConfiguredActiveScanRules { get; } = - new Dictionary(); - public IList DisabledPassiveScanRules { get; } = new List(); - public IList<(string Url, int Id, string RuleName)> DisabledRulesForUrls { get; } = new List<(string Url, int Id, string RuleName)>(); - public IList<(string Url, int Id, string Justification)> FalsePositives { get; } = new List<(string Url, int Id, string Justification)>(); - public IList> ZapPlanModifiers { get; } = new List>(); internal SecurityScanConfiguration() { @@ -57,7 +57,7 @@ public SecurityScanConfiguration StartAtUri(Uri startUri) /// The under the app to also cover during the scan. public SecurityScanConfiguration AddAdditionalUri(Uri additionalUri) { - AdditionalUris.Add(additionalUri); + _additionalUris.Add(additionalUri); return this; } @@ -91,7 +91,7 @@ public SecurityScanConfiguration SignIn(string userName = DefaultUser.UserName) /// public SecurityScanConfiguration ExcludeUrlWithRegex(string excludedUrlRegex) { - ExcludedUrlRegexPatterns.Add(excludedUrlRegex); + _excludedUrlRegexPatterns.Add(excludedUrlRegex); return this; } @@ -106,7 +106,7 @@ public SecurityScanConfiguration ExcludeUrlWithRegex(string excludedUrlRegex) /// public SecurityScanConfiguration DisableActiveScanRule(int id, string name = "") { - DisabledActiveScanRules.Add(new ScanRule(id, name)); + _disabledActiveScanRules.Add(new ScanRule(id, name)); return this; } @@ -128,7 +128,7 @@ public SecurityScanConfiguration DisableActiveScanRule(int id, string name = "") /// public SecurityScanConfiguration ConfigureActiveScanRule(int id, ScanRuleThreshold threshold, ScanRuleStrength strength, string name = "") { - ConfiguredActiveScanRules.Add(new ScanRule(id, name), (threshold, strength)); + _configuredActiveScanRules.Add(new ScanRule(id, name), (threshold, strength)); return this; } @@ -143,7 +143,7 @@ public SecurityScanConfiguration ConfigureActiveScanRule(int id, ScanRuleThresho /// public SecurityScanConfiguration DisablePassiveScanRule(int id, string name = "") { - DisabledPassiveScanRules.Add(new ScanRule(id, name)); + _disabledPassiveScanRules.Add(new ScanRule(id, name)); return this; } @@ -162,7 +162,7 @@ public SecurityScanConfiguration DisablePassiveScanRule(int id, string name = "" /// public SecurityScanConfiguration DisableScanRuleForUrlWithRegex(string urlRegex, int ruleId, string ruleName = "") { - DisabledRulesForUrls.Add((urlRegex, ruleId, ruleName)); + _disabledRulesForUrls.Add((urlRegex, ruleId, ruleName)); return this; } @@ -185,7 +185,7 @@ public SecurityScanConfiguration MarkScanRuleAsFalsePositiveForUrlWithRegex(stri throw new InvalidOperationException("Please provide a justification for disabling this alert!"); } - FalsePositives.Add((urlRegex, ruleId, justification)); + _falsePositives.Add((urlRegex, ruleId, justification)); return this; } @@ -200,7 +200,7 @@ public SecurityScanConfiguration MarkScanRuleAsFalsePositiveForUrlWithRegex(stri /// public SecurityScanConfiguration ModifyZapPlan(Func modifyPlan) { - ZapPlanModifiers.Add(modifyPlan); + _zapPlanModifiers.Add(modifyPlan); return this; } @@ -215,7 +215,7 @@ public SecurityScanConfiguration ModifyZapPlan(Func modifyPl /// public SecurityScanConfiguration ModifyZapPlan(Action modifyPlan) { - ZapPlanModifiers.Add(yamlDocument => + _zapPlanModifiers.Add(yamlDocument => { modifyPlan(yamlDocument); return Task.CompletedTask; @@ -228,7 +228,7 @@ 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); if (AjaxSpiderIsUsed) yamlDocument.AddSpiderAjaxAfterSpider(); @@ -258,10 +258,10 @@ internal async Task ApplyToPlanAsync(YamlDocument yamlDocument, UITestContext co // pollPostData: "" } - yamlDocument.AddExcludePathsRegex(ExcludedUrlRegexPatterns.ToArray()); - foreach (var rule in DisabledActiveScanRules) yamlDocument.DisableActiveScanRule(rule.Id, rule.Name); + yamlDocument.AddExcludePathsRegex(_excludedUrlRegexPatterns.ToArray()); + foreach (var rule in _disabledActiveScanRules) yamlDocument.DisableActiveScanRule(rule.Id, rule.Name); - foreach (var ruleConfiguration in ConfiguredActiveScanRules) + foreach (var ruleConfiguration in _configuredActiveScanRules) { yamlDocument.ConfigureActiveScanRule( ruleConfiguration.Key.Id, @@ -270,10 +270,10 @@ internal async Task ApplyToPlanAsync(YamlDocument yamlDocument, UITestContext co ruleConfiguration.Key.Name); } - foreach (var rule in DisabledPassiveScanRules) yamlDocument.DisablePassiveScanRule(rule.Id, rule.Name); - foreach (var (url, id, name) in DisabledRulesForUrls) yamlDocument.AddAlertFilter(url, id, name); - foreach (var (url, id, justification) in FalsePositives) yamlDocument.AddAlertFilter(url, id, justification, isFalsePositive: true); - foreach (var modifier in ZapPlanModifiers) await modifier(yamlDocument); + foreach (var rule in _disabledPassiveScanRules) yamlDocument.DisablePassiveScanRule(rule.Id, rule.Name); + foreach (var (url, id, name) in _disabledRulesForUrls) yamlDocument.AddAlertFilter(url, id, name); + foreach (var (url, id, justification) in _falsePositives) yamlDocument.AddAlertFilter(url, id, justification, isFalsePositive: true); + foreach (var modifier in _zapPlanModifiers) await modifier(yamlDocument); } public class ScanRule From f8219f8bee09e3331873f1219e27125c825b0bfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Wed, 27 Dec 2023 23:22:22 +0100 Subject: [PATCH 09/87] Some YAML extensio DRYing. --- .../YamlDocumentExtensions.cs | 57 ++++++++++--------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs b/Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs index 9966e63ad..da08e77ab 100644 --- a/Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs +++ b/Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs @@ -108,9 +108,7 @@ public static YamlDocument AddExcludePathsRegex(this YamlDocument yamlDocument, { var currentContext = yamlDocument.GetCurrentContext(); - if (!currentContext.Children.ContainsKey("excludePaths")) currentContext.Add("excludePaths", new YamlSequenceNode()); - - var excludePaths = (YamlSequenceNode)currentContext["excludePaths"]; + var excludePaths = currentContext.GetOrAddNode("excludePaths"); foreach (var pattern in excludePathsRegexPatterns) { excludePaths.Add(pattern); @@ -134,18 +132,12 @@ public static YamlDocument AddExcludePathsRegex(this YamlDocument yamlDocument, /// public static YamlDocument DisablePassiveScanRule(this YamlDocument yamlDocument, int id, string name = "") { - var passiveScanConfigJob = yamlDocument.GetPassiveScanConfigJobOrThrow(); - - if (!passiveScanConfigJob.Children.ContainsKey("rules")) passiveScanConfigJob.Add("rules", new YamlSequenceNode()); - - var newRule = new YamlMappingNode + yamlDocument.GetPassiveScanConfigJobOrThrow().AddRuleToRulesNode(new YamlMappingNode { { "id", id.ToTechnicalString() }, { "name", name }, { "threshold", "off" }, - }; - - ((YamlSequenceNode)passiveScanConfigJob["rules"]).Add(newRule); + }); return yamlDocument; } @@ -204,19 +196,13 @@ public static YamlDocument ConfigureActiveScanRule( throw new ArgumentException("The \"activeScan\" job should contain a policyDefinition."); } - var policyDefinition = (YamlMappingNode)activeScanConfigJob["policyDefinition"]; - - if (!policyDefinition.Children.ContainsKey("rules")) policyDefinition.Add("rules", new YamlSequenceNode()); - - var newRule = new YamlMappingNode + activeScanConfigJob["policyDefinition"].AddRuleToRulesNode(new YamlMappingNode { { "id", id.ToTechnicalString() }, { "name", name }, { "threshold", threshold.ToString() }, { "strength", strength.ToString() }, - }; - - ((YamlSequenceNode)policyDefinition["rules"]).Add(newRule); + }); return yamlDocument; } @@ -261,18 +247,14 @@ public static YamlDocument AddAlertFilter( jobs.Children.Insert(passiveScanConfigIndex + 1, alertFilterJob); } - if (!alertFilterJob.Children.ContainsKey("alertFilters")) alertFilterJob.Add("alertFilters", new YamlSequenceNode()); - - var newRule = new YamlMappingNode + alertFilterJob.GetOrAddNode("alertFilters").Add(new YamlMappingNode { { "ruleId", ruleId.ToTechnicalString() }, { "ruleName", ruleName }, { "url", urlMatchingRegexPattern }, { "urlRegex", "true" }, { "newRisk", isFalsePositive ? "False Positive" : "Info" }, - }; - - ((YamlSequenceNode)alertFilterJob["alertFilters"]).Add(newRule); + }); return yamlDocument; } @@ -309,6 +291,20 @@ public static YamlDocument AddRequestor(this YamlDocument yamlDocument, string u /// public static YamlMappingNode GetRootNode(this YamlDocument yamlDocument) => (YamlMappingNode)yamlDocument.RootNode; + /// + /// Tries to access the child node by the name of . If it doesn't exists, it's created. + /// + public static T GetOrAddNode(this YamlNode yamlNode, string key) + where T : YamlNode, new() + { + if (yamlNode is YamlMappingNode mappingNode && !mappingNode.Children.ContainsKey(key)) + { + mappingNode.Children.Add(key, new T()); + } + + return (T)yamlNode[key]; + } + /// /// Gets the "jobs" section of the ZAP Automation Framework plan. /// @@ -341,9 +337,7 @@ public static YamlSequenceNode GetUrls(this YamlDocument yamlDocument) { var currentContext = yamlDocument.GetCurrentContext(); - if (!currentContext.Children.ContainsKey("urls")) currentContext.Add("urls", new YamlSequenceNode()); - - return (YamlSequenceNode)currentContext["urls"]; + return currentContext.GetOrAddNode("urls"); } /// @@ -419,4 +413,11 @@ public static YamlDocument MergeFrom(this YamlDocument baseDocument, YamlDocumen return baseDocument; } + + /// + /// Ensures that the has a rules child and then adds the to it. + /// + public static void AddRuleToRulesNode(this YamlNode parentOfRules, YamlNode newRule) => + parentOfRules.GetOrAddNode("rules").Add(newRule); } From c61a55c48aacfdc310097168436770dd3cfd3c97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Thu, 28 Dec 2023 00:17:57 +0100 Subject: [PATCH 10/87] By default ignore /vendor/ or /vendors/ URLs. --- .../SecurityScanningUITestContextExtensions.cs | 4 ++++ .../SecurityScanning/YamlDocumentExtensions.cs | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs index 350834efe..8fa3e86bf 100644 --- a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs +++ b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs @@ -165,6 +165,10 @@ public static Task RunSecurityScanAsync( configuration.StartAtUri(context.GetCurrentUri()); + // By default ignore /vendor/ or /vendors/ URLs. This is case-insensitive. We have no control over them, and + // they may contain several false positives (e.g. in font-awesome).. + configuration.ExcludeUrlWithRegex(@".*/vendors?/.*"); + if (context.Configuration.SecurityScanningConfiguration.ZapAutomationFrameworkPlanModifier != null) { configuration.ModifyZapPlan(async plan => diff --git a/Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs b/Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs index da08e77ab..f5f5ad72a 100644 --- a/Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs +++ b/Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs @@ -207,6 +207,15 @@ public static YamlDocument ConfigureActiveScanRule( return yamlDocument; } + /// + /// Adds a regular expression used to exclude paths in the current context. + /// + public static YamlDocument AddGlobalExcludePath(this YamlDocument yamlDocument, string pathRegex) + { + yamlDocument.GetCurrentContext().GetOrAddNode("excludePaths").Add(pathRegex); + return yamlDocument; + } + /// /// Adds an Alert Filter to the ZAP /// Automation Framework plan. From f6ea9fb22525f20b284ec0818643b29deccb29ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Thu, 28 Dec 2023 00:28:21 +0100 Subject: [PATCH 11/87] unusing --- Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs b/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs index 2bc2f93f0..e80161cdb 100644 --- a/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs +++ b/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs @@ -5,7 +5,6 @@ using Lombiq.Tests.UI.Shortcuts.Controllers; using System; using System.Collections.Generic; -using System.Linq; using System.Threading.Tasks; using YamlDotNet.RepresentationModel; From 32f03dd2af5be00df7d1018bf633dd117a6f08c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Thu, 28 Dec 2023 02:04:43 +0100 Subject: [PATCH 12/87] Fix sample. --- Lombiq.Tests.UI.Samples/Tests/SecurityScanningTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lombiq.Tests.UI.Samples/Tests/SecurityScanningTests.cs b/Lombiq.Tests.UI.Samples/Tests/SecurityScanningTests.cs index 14dbbf4e4..3ab1abbde 100644 --- a/Lombiq.Tests.UI.Samples/Tests/SecurityScanningTests.cs +++ b/Lombiq.Tests.UI.Samples/Tests/SecurityScanningTests.cs @@ -65,7 +65,7 @@ public Task SecurityScanWithCustomConfigurationShouldPass() => configuration => configuration ////.UseAjaxSpider() // This is quite slow so just showing you here but not running it. .ExcludeUrlWithRegex(".*blog.*") - .DisablePassiveScanRule(10037, "Server Leaks Information via \"X-Powered-By\" HTTP Response Header Field(s)") + .DisablePassiveScanRule(10020, "The response does not include either Content-Security-Policy with 'frame-ancestors' directive.") .DisableScanRuleForUrlWithRegex(".*/about", 10038, "Content Security Policy (CSP) Header Not Set") .SignIn(), sarifLog => sarifLog.Runs[0].Results.Count.ShouldBeLessThan(34))); From 445074d15a8e28400e47ff66384a9bf95f586861 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Thu, 28 Dec 2023 14:20:25 +0100 Subject: [PATCH 13/87] Why was this rule disabled in the first place? Just enable "OrchardCore.Https". --- .../SecurityScanning/AutomationFrameworkPlans/Baseline.yml | 3 --- .../SecurityScanning/AutomationFrameworkPlans/FullScan.yml | 3 --- .../SecurityScanning/AutomationFrameworkPlans/GraphQL.yml | 3 --- .../SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml | 3 --- 4 files changed, 12 deletions(-) diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml index b269322af..a17a2d0aa 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml @@ -42,9 +42,6 @@ jobs: enableTags: false disableAllRules: false rules: - - id: 10035 - name: "Strict-Transport-Security Header" - threshold: "off" # Some stock Orchard Core shapes and pages (including the setup page) contain inline script blocks, so this directive # must be permitted for OC to work at all. - id: 10055 diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml index a87e4d2d3..05bedaadf 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml @@ -42,9 +42,6 @@ jobs: enableTags: false disableAllRules: false rules: - - id: 10035 - name: "Strict-Transport-Security Header" - threshold: "off" # Some stock Orchard Core shapes and pages (including the setup page) contain inline script blocks, so this directive # must be permitted for OC to work at all. - id: 10055 diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml index f88add172..c256d68bc 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml @@ -42,9 +42,6 @@ jobs: enableTags: false disableAllRules: false rules: - - id: 10035 - name: "Strict-Transport-Security Header" - threshold: "off" # Some stock Orchard Core shapes and pages (including the setup page) contain inline script blocks, so this directive # must be permitted for OC to work at all. - id: 10055 diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml index 43912d188..6a5441fd4 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml @@ -42,9 +42,6 @@ jobs: enableTags: false disableAllRules: false rules: - - id: 10035 - name: "Strict-Transport-Security Header" - threshold: "off" # Some stock Orchard Core shapes and pages (including the setup page) contain inline script blocks, so this directive # must be permitted for OC to work at all. - id: 10055 From 1a6ad0c3b8ada0c6c8ea4f55ffbb04292ffb8672 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Thu, 28 Dec 2023 23:02:20 +0100 Subject: [PATCH 14/87] Update the comment. --- Lombiq.Tests.UI.Samples/Tests/SecurityScanningTests.cs | 4 ++-- .../SecurityScanningUITestContextExtensions.cs | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Lombiq.Tests.UI.Samples/Tests/SecurityScanningTests.cs b/Lombiq.Tests.UI.Samples/Tests/SecurityScanningTests.cs index 3ab1abbde..87d6dbece 100644 --- a/Lombiq.Tests.UI.Samples/Tests/SecurityScanningTests.cs +++ b/Lombiq.Tests.UI.Samples/Tests/SecurityScanningTests.cs @@ -45,8 +45,8 @@ public Task BasicSecurityScanShouldPass() => // usually not just unnecessary for a website that's not an SPA, but also slows the scan down by a lot. However, // if you have an SPA, you need to use it. // - Excludes certain URLs from the scan completely. Use this if you don't want ZAP to process certain URLs at all. - // - Disables the "Server Leaks Information via "X-Powered-By" HTTP Response Header Field(s)" alert of ZAP's passive - // scan for the whole scan. This is because by default, Orchard Core sends an "X-Powered-By: OrchardCore" header. + // - Disables the The response does not include either Content-Security-Policy with 'frame-ancestors' directive." + // alert of ZAP's passive scan for the whole scan. This is because by default, Orchard Core sends an "X-Powered-By: OrchardCore" header. // If you want airtight security, you might want to turn this off, but for the sake of example we just ignore the // alert here. // - Also disables the "Content Security Policy (CSP) Header Not Set" rule but only for the /about page. Use this to diff --git a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs index 8fa3e86bf..c5b40d868 100644 --- a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs +++ b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs @@ -29,7 +29,8 @@ public static Task RunAndAssertBaselineSecurityScanAsync( AutomationFrameworkPlanPaths.BaselinePlanPath, configuration => { - // Make sure to visit at least one page that throws an exception. + // Make sure to visit at least one page that throws an exception. This is to ensure that error handling + // is tested for security as well. var errorPageRelativeUrl = context.GetRelativeUrlOfAction(error => error.Index()); configuration.ModifyZapPlan(plan => plan.AddUrl(context.GetAbsoluteUri(errorPageRelativeUrl))); From 5dd76472c88a2cfe99c1f8a4994c8ad44ff0ea61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Thu, 28 Dec 2023 23:02:43 +0100 Subject: [PATCH 15/87] Update Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Zoltán Lehóczky --- Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs b/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs index e80161cdb..f0e948235 100644 --- a/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs +++ b/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs @@ -166,8 +166,8 @@ public SecurityScanConfiguration DisableScanRuleForUrlWithRegex(string urlRegex, } /// - /// Marks a rule (can be any rule, including e.g. both active or passive scan rules) for just URLs matching the - /// given regular expression pattern. + /// Marks a rule (can be any rule, including e.g. both active or passive scan rules) as false positivefor just URLs + /// matching the given regular expression pattern. /// /// /// The regex pattern to match URLs against. It will be matched against the whole absolute URL, e.g., ".*blog.*" From 2544f98d5e587a5044967be00f052ebdf33f7621 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Thu, 28 Dec 2023 23:06:54 +0100 Subject: [PATCH 16/87] Remove duplicate method. --- .../SecurityScanning/YamlDocumentExtensions.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs b/Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs index f5f5ad72a..da08e77ab 100644 --- a/Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs +++ b/Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs @@ -207,15 +207,6 @@ public static YamlDocument ConfigureActiveScanRule( return yamlDocument; } - /// - /// Adds a regular expression used to exclude paths in the current context. - /// - public static YamlDocument AddGlobalExcludePath(this YamlDocument yamlDocument, string pathRegex) - { - yamlDocument.GetCurrentContext().GetOrAddNode("excludePaths").Add(pathRegex); - return yamlDocument; - } - /// /// Adds an Alert Filter to the ZAP /// Automation Framework plan. From a97fd47c4a534e3eaf49a72165e887e0fb8df9cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Thu, 28 Dec 2023 23:13:11 +0100 Subject: [PATCH 17/87] More documentation. --- Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs | 4 ++++ Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs b/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs index f0e948235..113654396 100644 --- a/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs +++ b/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs @@ -177,6 +177,10 @@ public SecurityScanConfiguration DisableScanRuleForUrlWithRegex(string urlRegex, /// /// A human-readable explanation of why the alert is false positive. /// + /// + /// Marking a rule as false positive helps the development of ZAP by collecting which rules have the highest false + /// positive rate (see the FAQ). + /// public SecurityScanConfiguration MarkScanRuleAsFalsePositiveForUrlWithRegex(string urlRegex, int ruleId, string justification) { if (string.IsNullOrWhiteSpace(justification)) diff --git a/Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs b/Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs index da08e77ab..a30e0a71d 100644 --- a/Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs +++ b/Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs @@ -218,7 +218,8 @@ public static YamlDocument ConfigureActiveScanRule( /// /// /// The human-readable name of the rule. Not required to turn off the rule, and its value doesn't matter. It's just - /// useful for the readability of the method call. + /// useful for the readability of the method call. If is , + /// write the justification here as well. /// /// /// If you disable the rule because it's a false positive, then set this to . This helps the From 33810db528bd75736405e118edb1811ddccc7173 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Thu, 28 Dec 2023 23:17:22 +0100 Subject: [PATCH 18/87] Explain why disabling "Strict-Transport-Security Header" is necessary. --- .../SecurityScanning/AutomationFrameworkPlans/Baseline.yml | 5 +++++ .../SecurityScanning/AutomationFrameworkPlans/FullScan.yml | 5 +++++ .../SecurityScanning/AutomationFrameworkPlans/GraphQL.yml | 5 +++++ .../SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml | 5 +++++ 4 files changed, 20 insertions(+) diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml index a17a2d0aa..81fe3acd6 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml @@ -42,6 +42,11 @@ jobs: enableTags: false disableAllRules: false rules: + # This needs to be disabled during UI testing, because a local app needn't use HSTS. It's also something commonly + # configured outside the app, like in Cloudflare. + - id: 10035 + name: "Strict-Transport-Security Header" + threshold: "off" # Some stock Orchard Core shapes and pages (including the setup page) contain inline script blocks, so this directive # must be permitted for OC to work at all. - id: 10055 diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml index 05bedaadf..da30e5784 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml @@ -42,6 +42,11 @@ jobs: enableTags: false disableAllRules: false rules: + # This needs to be disabled during UI testing, because a local app needn't use HSTS. It's also something commonly + # configured outside the app, like in Cloudflare. + - id: 10035 + name: "Strict-Transport-Security Header" + threshold: "off" # Some stock Orchard Core shapes and pages (including the setup page) contain inline script blocks, so this directive # must be permitted for OC to work at all. - id: 10055 diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml index c256d68bc..97f98cb47 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml @@ -42,6 +42,11 @@ jobs: enableTags: false disableAllRules: false rules: + # This needs to be disabled during UI testing, because a local app needn't use HSTS. It's also something commonly + # configured outside the app, like in Cloudflare. + - id: 10035 + name: "Strict-Transport-Security Header" + threshold: "off" # Some stock Orchard Core shapes and pages (including the setup page) contain inline script blocks, so this directive # must be permitted for OC to work at all. - id: 10055 diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml index 6a5441fd4..d24a580d7 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml @@ -42,6 +42,11 @@ jobs: enableTags: false disableAllRules: false rules: + # This needs to be disabled during UI testing, because a local app needn't use HSTS. It's also something commonly + # configured outside the app, like in Cloudflare. + - id: 10035 + name: "Strict-Transport-Security Header" + threshold: "off" # Some stock Orchard Core shapes and pages (including the setup page) contain inline script blocks, so this directive # must be permitted for OC to work at all. - id: 10055 From a1e8dad62aacf882743eaa5f8fd7d03f19be9e85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Thu, 28 Dec 2023 23:56:18 +0100 Subject: [PATCH 19/87] Remove forced error generation because we are going to enable OrchardCore.Diagnostics by default anyway. --- .../SecurityScanningUITestContextExtensions.cs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs index c5b40d868..ba505a8dc 100644 --- a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs +++ b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs @@ -1,8 +1,6 @@ -using Lombiq.HelpfulLibraries.OrchardCore.Mvc; using Lombiq.Tests.UI.Exceptions; using Lombiq.Tests.UI.Extensions; using Lombiq.Tests.UI.Services; -using Lombiq.Tests.UI.Shortcuts.Controllers; using Microsoft.CodeAnalysis.Sarif; using System; using System.Threading.Tasks; @@ -27,15 +25,7 @@ public static Task RunAndAssertBaselineSecurityScanAsync( Action assertSecurityScanResult = null) => context.RunAndAssertSecurityScanAsync( AutomationFrameworkPlanPaths.BaselinePlanPath, - configuration => - { - // Make sure to visit at least one page that throws an exception. This is to ensure that error handling - // is tested for security as well. - var errorPageRelativeUrl = context.GetRelativeUrlOfAction(error => error.Index()); - configuration.ModifyZapPlan(plan => plan.AddUrl(context.GetAbsoluteUri(errorPageRelativeUrl))); - - configure?.Invoke(configuration); - }, + configure, assertSecurityScanResult); /// From e0c6d65925a8d77e7a60f51067bb085fa2c64c7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Fri, 29 Dec 2023 00:48:24 +0100 Subject: [PATCH 20/87] Tweak app log error handling during sercurity scan.. --- .../SecurityScanningUITestContextExtensions.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs index ba505a8dc..85b8b59b8 100644 --- a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs +++ b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs @@ -108,6 +108,8 @@ public static async Task RunAndAssertSecurityScanAsync( Action assertSecurityScanResult = null) { // Verify that the app logs are fine right now, then disable app log assertion for the duration of this scan. + // This way we see if security holes open when something goes wrong. It would be a problem if the site was only + // safe when everything worked well. await context.Configuration.AssertAppLogsAsync(context.Application); var assertAppLogsAsync = context.Configuration.AssertAppLogsAsync; context.Configuration.AssertAppLogsAsync = _ => Task.CompletedTask; @@ -130,9 +132,10 @@ public static async Task RunAndAssertSecurityScanAsync( throw new SecurityScanningAssertionException(ex); } - // Clear app logs before app log assertion is restored. - context.ClearLogs(); + // Now that the security scan has concluded, we can turn back app log assertion and verify if any errors have + // been accumulated during the security scan. context.Configuration.AssertAppLogsAsync = assertAppLogsAsync; + await context.Configuration.AssertAppLogsAsync(context.Application); } /// From 6fa5d43849c1b8bef9489437b611ad55e352ab87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Fri, 29 Dec 2023 02:51:03 +0100 Subject: [PATCH 21/87] Disable 10062. --- .../SecurityScanning/AutomationFrameworkPlans/Baseline.yml | 4 ++++ .../SecurityScanning/AutomationFrameworkPlans/FullScan.yml | 4 ++++ .../SecurityScanning/AutomationFrameworkPlans/GraphQL.yml | 4 ++++ .../SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml | 4 ++++ 4 files changed, 16 insertions(+) diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml index 81fe3acd6..39cd44476 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml @@ -56,6 +56,10 @@ jobs: - id: 10037 name: "Server Leaks Information via 'X-Powered-By' HTTP Response Header Field(s)" threshold: "off" + # This rule generates false positives on UUIDs or random numeric sequences. + - id: 10062 + name: "The response contains Personally Identifiable Information, such as CC number, SSN and similar sensitive data." + threshold: "off" name: "passiveScan-config" type: "passiveScan-config" - parameters: {} diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml index da30e5784..525084fa1 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml @@ -56,6 +56,10 @@ jobs: - id: 10037 name: "Server Leaks Information via 'X-Powered-By' HTTP Response Header Field(s)" threshold: "off" + # This rule generates false positives on UUIDs or random numeric sequences. + - id: 10062 + name: "The response contains Personally Identifiable Information, such as CC number, SSN and similar sensitive data." + threshold: "off" name: "passiveScan-config" type: "passiveScan-config" - parameters: {} diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml index 97f98cb47..a37b1261e 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml @@ -56,6 +56,10 @@ jobs: - id: 10037 name: "Server Leaks Information via 'X-Powered-By' HTTP Response Header Field(s)" threshold: "off" + # This rule generates false positives on UUIDs or random numeric sequences. + - id: 10062 + name: "The response contains Personally Identifiable Information, such as CC number, SSN and similar sensitive data." + threshold: "off" name: "passiveScan-config" type: "passiveScan-config" - parameters: diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml index d24a580d7..5a783abda 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml @@ -56,6 +56,10 @@ jobs: - id: 10037 name: "Server Leaks Information via 'X-Powered-By' HTTP Response Header Field(s)" threshold: "off" + # This rule generates false positives on UUIDs or random numeric sequences. + - id: 10062 + name: "The response contains Personally Identifiable Information, such as CC number, SSN and similar sensitive data." + threshold: "off" name: "passiveScan-config" type: "passiveScan-config" - parameters: {} From 5e3ed10a5ef530364e39664af33bc797e98d0566 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Fri, 29 Dec 2023 04:51:19 +0100 Subject: [PATCH 22/87] Typo. --- Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs b/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs index 113654396..963e82c59 100644 --- a/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs +++ b/Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs @@ -166,7 +166,7 @@ public SecurityScanConfiguration DisableScanRuleForUrlWithRegex(string urlRegex, } /// - /// Marks a rule (can be any rule, including e.g. both active or passive scan rules) as false positivefor just URLs + /// Marks a rule (can be any rule, including e.g. both active or passive scan rules) as false positive for just URLs /// matching the given regular expression pattern. /// /// From d1fd50d5af059cd079adb6bb1b3d3eb78a6af393 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Fri, 29 Dec 2023 21:30:00 +0100 Subject: [PATCH 23/87] Update coment regarding unsafe-inline. --- .../SecurityScanning/AutomationFrameworkPlans/Baseline.yml | 4 ++-- .../SecurityScanning/AutomationFrameworkPlans/FullScan.yml | 4 ++-- .../SecurityScanning/AutomationFrameworkPlans/GraphQL.yml | 4 ++-- .../SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml index 39cd44476..ac5cbe6ed 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml @@ -47,8 +47,8 @@ jobs: - id: 10035 name: "Strict-Transport-Security Header" threshold: "off" - # Some stock Orchard Core shapes and pages (including the setup page) contain inline script blocks, so this directive - # must be permitted for OC to work at all. + # This is required for