From 4ae2407809c77842dfaaf6d2e71c46e545a3c7cf Mon Sep 17 00:00:00 2001 From: Michael Van Leeuwen Date: Sun, 6 Nov 2022 12:13:01 -0800 Subject: [PATCH 1/7] Add AvoidUsingAllowUnencryptedAuthentication rule --- ...voidUsingAllowUnencryptedAuthentication.cs | 117 ++++++++++++++++++ Rules/Strings.Designer.cs | 36 ++++++ Rules/Strings.resx | 12 ++ 3 files changed, 165 insertions(+) create mode 100644 Rules/AvoidUsingAllowUnencryptedAuthentication.cs diff --git a/Rules/AvoidUsingAllowUnencryptedAuthentication.cs b/Rules/AvoidUsingAllowUnencryptedAuthentication.cs new file mode 100644 index 000000000..955de8113 --- /dev/null +++ b/Rules/AvoidUsingAllowUnencryptedAuthentication.cs @@ -0,0 +1,117 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Management.Automation.Language; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +#if !CORECLR +using System.ComponentModel.Composition; +#endif +using System.Globalization; + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ + /// + /// AvoidUsingAllowUnencryptedAuthentication: Avoid sending credentials and secrets over unencrypted connections. + /// +#if !CORECLR +[Export(typeof(IScriptRule))] +#endif + public class AvoidUsingAllowUnencryptedAuthentication : AvoidParameterGeneric + { + /// + /// Condition on the cmdlet that must be satisfied for the error to be raised + /// + /// + /// + public override bool CommandCondition(CommandAst CmdAst) + { + return true; + } + + /// + /// Condition on the parameter that must be satisfied for the error to be raised. + /// + /// + /// + /// + public override bool ParameterCondition(CommandAst CmdAst, CommandElementAst CeAst) + { + return CeAst is CommandParameterAst && String.Equals((CeAst as CommandParameterAst).ParameterName, "AllowUnencryptedAuthentication", StringComparison.OrdinalIgnoreCase); + } + + /// + /// Retrieves the error message + /// + /// + /// + /// + public override string GetError(string fileName, CommandAst cmdAst) + { + return String.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingAllowUnencryptedAuthenticationError); + } + + /// + /// GetName: Retrieves the name of this rule. + /// + /// The name of this rule + public override string GetName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.AvoidUsingAllowUnencryptedAuthenticationName); + } + + /// + /// GetCommonName: Retrieves the common name of this rule. + /// + /// The common name of this rule + public override string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingAllowUnencryptedAuthenticationCommonName); + } + + /// + /// GetDescription: Retrieves the description of this rule. + /// + /// The description of this rule + public override string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingAllowUnencryptedAuthenticationDescription); + } + + /// + /// GetSourceType: Retrieves the type of the rule: builtin, managed or module. + /// + public override SourceType GetSourceType() + { + return SourceType.Builtin; + } + + /// + /// GetSeverity: Retrieves the severity of the rule: error, warning or information. + /// + /// + public override RuleSeverity GetSeverity() + { + return RuleSeverity.Warning; + } + + /// + /// DiagnosticSeverity: Retrieves the severity of the rule of type DiagnosticSeverity: error, warning or information. + /// + /// + public override DiagnosticSeverity GetDiagnosticSeverity() + { + return DiagnosticSeverity.Warning; + } + + /// + /// GetSourceName: Retrieves the module/assembly name the rule is from. + /// + public override string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + } +} diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 30d0a7321..13cb02712 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -762,6 +762,42 @@ internal static string AvoidUsernameAndPasswordParamsName { } } + /// + /// Looks up a localized string similar to Avoid AllowUnencryptedAuthentication Switch. + /// + internal static string AvoidUsingAllowUnencryptedAuthenticationCommonName { + get { + return ResourceManager.GetString("AvoidUsingAllowUnencryptedAuthenticationCommonName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Avoid sending credentials and secrets over unencrypted connections.. + /// + internal static string AvoidUsingAllowUnencryptedAuthenticationDescription { + get { + return ResourceManager.GetString("AvoidUsingAllowUnencryptedAuthenticationDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The insecure AllowUsingUnencryptedAuthentication switch was used. This should be avoided except for compatability with legacy systems.. + /// + internal static string AvoidUsingAllowUnencryptedAuthenticationError { + get { + return ResourceManager.GetString("AvoidUsingAllowUnencryptedAuthenticationError", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to AvoidUsingAllowUnencryptedAuthentication. + /// + internal static string AvoidUsingAllowUnencryptedAuthenticationName { + get { + return ResourceManager.GetString("AvoidUsingAllowUnencryptedAuthenticationName", resourceCulture); + } + } + /// /// Looks up a localized string similar to Avoid Using Broken Hash Algorithms. /// diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 20b04712d..686140ecf 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -1191,4 +1191,16 @@ Parameter '{0}' of function/cmdlet '{1}' does not match its exact casing '{2}'. + + Avoid AllowUnencryptedAuthentication Switch + + + Avoid sending credentials and secrets over unencrypted connections. + + + The insecure AllowUsingUnencryptedAuthentication switch was used. This should be avoided except for compatability with legacy systems. + + + AvoidUsingAllowUnencryptedAuthentication + \ No newline at end of file From 827c2cf8dcf66df42c00758c216f63d16a924041 Mon Sep 17 00:00:00 2001 From: Michael Van Leeuwen Date: Sun, 6 Nov 2022 12:14:05 -0800 Subject: [PATCH 2/7] Add AvoidUsingAllowUnencryptedAuthentication docs and tests --- Tests/Engine/GetScriptAnalyzerRule.tests.ps1 | 2 +- ...ngAllowUnencryptedAuthentication.tests.ps1 | 44 +++++++++++++++++++ ...voidUsingAllowUnencryptedAuthentication.md | 33 ++++++++++++++ docs/Rules/README.md | 1 + 4 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 Tests/Rules/AvoidUsingAllowUnencryptedAuthentication.tests.ps1 create mode 100644 docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index 0ca6e569f..960a2fcd5 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -63,7 +63,7 @@ Describe "Test Name parameters" { It "get Rules with no parameters supplied" { $defaultRules = Get-ScriptAnalyzerRule - $expectedNumRules = 68 + $expectedNumRules = 69 if ($PSVersionTable.PSVersion.Major -le 4) { # for PSv3 PSAvoidGlobalAliases is not shipped because diff --git a/Tests/Rules/AvoidUsingAllowUnencryptedAuthentication.tests.ps1 b/Tests/Rules/AvoidUsingAllowUnencryptedAuthentication.tests.ps1 new file mode 100644 index 000000000..a3068b128 --- /dev/null +++ b/Tests/Rules/AvoidUsingAllowUnencryptedAuthentication.tests.ps1 @@ -0,0 +1,44 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +BeforeAll { + $settings = @{ + IncludeRules = @('PSAvoidUsingAllowUnencryptedAuthentication') + Rules = @{ + PSAvoidUsingAllowUnencryptedAuthentication = @{ + Enable = $true + } + } + } + + $violationMessage = [regex]::Escape("The insecure AllowUsingUnencryptedAuthentication switch was used. This should be avoided except for compatability with legacy systems.") +} + +Describe "AvoidUsingAllowUnencryptedAuthentication" { + Context "When there are violations" { + It "detects unencrypted authentication violations" { + (Invoke-ScriptAnalyzer -ScriptDefinition 'Invoke-WebRequest foo -AllowUnencryptedAuthentication' -Settings $settings).Count | Should -Be 1 + (Invoke-ScriptAnalyzer -ScriptDefinition 'Invoke-RestMethod foo -AllowUnencryptedAuthentication' -Settings $settings).Count | Should -Be 1 + (Invoke-ScriptAnalyzer -ScriptDefinition 'iwr foo -AllowUnencryptedAuthentication' -Settings $settings).Count | Should -Be 1 + } + + It "has the correct description message" { + (Invoke-ScriptAnalyzer -ScriptDefinition 'Invoke-WebRequest foo -AllowUnencryptedAuthentication' -Settings $settings).Message | Should -Match $violationMessage + } + + It "detects arbitrary cmdlets" { + (Invoke-ScriptAnalyzer -ScriptDefinition 'Invoke-CustomWebRequest foo -AllowUnencryptedAuthentication' -Settings $settings).Count | Should -Be 1 + } + + } + + Context "When there are no violations" { + It "does not flag safe usage" { + (Invoke-ScriptAnalyzer -ScriptDefinition 'Invoke-WebRequest foo' -Settings $settings).Count | Should -Be 0 + } + + It "does not flag cases with unrelated parameters" { + (Invoke-ScriptAnalyzer -ScriptDefinition 'Invoke-WebRequest foo -Method Get' -Settings $settings).Count | Should -Be 0 + } + } +} \ No newline at end of file diff --git a/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md b/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md new file mode 100644 index 000000000..c4d3f255a --- /dev/null +++ b/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md @@ -0,0 +1,33 @@ +--- +description: Avoid sending credentials and secrets over unencrypted connections +ms.custom: PSSA v1.22.0 +ms.date: 11/06/2022 +ms.topic: reference +title: AvoidUsingAllowUnencryptedAuthentication +--- +# AvoidUsingAllowUnencryptedAuthentication + +**Severity Level: Warning** + +## Description + +Avoid using the AllowUnencryptedAuthentication switch, which sends credentials and secrets over unencrypted connections. +This should be avoided except for compatability with legacy systems. + +## How + +Avoid using the AllowUnencryptedAuthentication switch. + +## Example 1 + +### Wrong + +```powershell +Invoke-WebRequest foo -AllowUnencryptedAuthentication +``` + +### Correct + +```powershell +Invoke-WebRequest foo +``` \ No newline at end of file diff --git a/docs/Rules/README.md b/docs/Rules/README.md index aaac74f66..e2e424a4e 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -26,6 +26,7 @@ The PSScriptAnalyzer contains the following rule definitions. | [AvoidSemicolonsAsLineTerminators](./AvoidSemicolonsAsLineTerminators.md) | Warning | No | | | [AvoidShouldContinueWithoutForce](./AvoidShouldContinueWithoutForce.md) | Warning | Yes | | | [AvoidTrailingWhitespace](./AvoidTrailingWhitespace.md) | Warning | Yes | | +| [AvoidUsingAllowUnencryptedAuthentication](./AvoidUsingAllowUnencryptedAuthentication.md) | Warning | Yes | | | [AvoidUsingBrokenHashAlgorithms](./AvoidUsingBrokenHashAlgorithms.md) | Warning | Yes | | | [AvoidUsingCmdletAliases](./AvoidUsingCmdletAliases.md) | Warning | Yes | Yes2 | | [AvoidUsingComputerNameHardcoded](./AvoidUsingComputerNameHardcoded.md) | Error | Yes | | From 4ebdcb957f04a18fb6758b2fc3d70aeca02b1f20 Mon Sep 17 00:00:00 2001 From: Michael Van Leeuwen Date: Mon, 7 Nov 2022 08:02:12 -0800 Subject: [PATCH 3/7] Update docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md Co-authored-by: Christoph Bergmeister --- docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md b/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md index c4d3f255a..70855b74f 100644 --- a/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md +++ b/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md @@ -11,7 +11,7 @@ title: AvoidUsingAllowUnencryptedAuthentication ## Description -Avoid using the AllowUnencryptedAuthentication switch, which sends credentials and secrets over unencrypted connections. +Avoid using the `AllowUnencryptedAuthentication` switch on `Invoke-WebRequest`, which sends credentials and secrets over unencrypted connections. This should be avoided except for compatability with legacy systems. ## How From c489ba4ed5c20a882463bce7c78d7e32444110db Mon Sep 17 00:00:00 2001 From: Michael Van Leeuwen Date: Mon, 7 Nov 2022 08:11:01 -0800 Subject: [PATCH 4/7] Fix code review suggestions --- .../AvoidUsingAllowUnencryptedAuthentication.tests.ps1 | 8 +------- docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md | 4 +++- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/Tests/Rules/AvoidUsingAllowUnencryptedAuthentication.tests.ps1 b/Tests/Rules/AvoidUsingAllowUnencryptedAuthentication.tests.ps1 index a3068b128..ca89b280c 100644 --- a/Tests/Rules/AvoidUsingAllowUnencryptedAuthentication.tests.ps1 +++ b/Tests/Rules/AvoidUsingAllowUnencryptedAuthentication.tests.ps1 @@ -10,8 +10,6 @@ BeforeAll { } } } - - $violationMessage = [regex]::Escape("The insecure AllowUsingUnencryptedAuthentication switch was used. This should be avoided except for compatability with legacy systems.") } Describe "AvoidUsingAllowUnencryptedAuthentication" { @@ -22,10 +20,6 @@ Describe "AvoidUsingAllowUnencryptedAuthentication" { (Invoke-ScriptAnalyzer -ScriptDefinition 'iwr foo -AllowUnencryptedAuthentication' -Settings $settings).Count | Should -Be 1 } - It "has the correct description message" { - (Invoke-ScriptAnalyzer -ScriptDefinition 'Invoke-WebRequest foo -AllowUnencryptedAuthentication' -Settings $settings).Message | Should -Match $violationMessage - } - It "detects arbitrary cmdlets" { (Invoke-ScriptAnalyzer -ScriptDefinition 'Invoke-CustomWebRequest foo -AllowUnencryptedAuthentication' -Settings $settings).Count | Should -Be 1 } @@ -36,7 +30,7 @@ Describe "AvoidUsingAllowUnencryptedAuthentication" { It "does not flag safe usage" { (Invoke-ScriptAnalyzer -ScriptDefinition 'Invoke-WebRequest foo' -Settings $settings).Count | Should -Be 0 } - + It "does not flag cases with unrelated parameters" { (Invoke-ScriptAnalyzer -ScriptDefinition 'Invoke-WebRequest foo -Method Get' -Settings $settings).Count | Should -Be 0 } diff --git a/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md b/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md index 70855b74f..8c4dd6ccf 100644 --- a/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md +++ b/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md @@ -11,9 +11,11 @@ title: AvoidUsingAllowUnencryptedAuthentication ## Description -Avoid using the `AllowUnencryptedAuthentication` switch on `Invoke-WebRequest`, which sends credentials and secrets over unencrypted connections. +Avoid using the `AllowUnencryptedAuthentication` switch on `Invoke-WebRequest`, `Invoke-RestMethod`, and other webrequest cmdlets, which sends credentials and secrets over unencrypted connections. This should be avoided except for compatability with legacy systems. +For more details, see the documentation warning [here](https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.utility/invoke-webrequest?view=powershell-7.2#-allowunencryptedauthentication). + ## How Avoid using the AllowUnencryptedAuthentication switch. From 8ee0d15c765b878b44d27d78614a30142c985c93 Mon Sep 17 00:00:00 2001 From: Michael Van Leeuwen Date: Mon, 7 Nov 2022 08:14:54 -0800 Subject: [PATCH 5/7] Fix md code styling --- docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md b/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md index 8c4dd6ccf..13b79267a 100644 --- a/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md +++ b/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md @@ -18,7 +18,7 @@ For more details, see the documentation warning [here](https://learn.microsoft.c ## How -Avoid using the AllowUnencryptedAuthentication switch. +Avoid using the `AllowUnencryptedAuthentication` switch. ## Example 1 From da58e92ac8df83ec9c77a2681be4a3325c345521 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Thu, 1 Feb 2024 12:42:14 +0000 Subject: [PATCH 6/7] bump rule count in tests again --- Tests/Engine/GetScriptAnalyzerRule.tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index 960a2fcd5..93824060a 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -63,7 +63,7 @@ Describe "Test Name parameters" { It "get Rules with no parameters supplied" { $defaultRules = Get-ScriptAnalyzerRule - $expectedNumRules = 69 + $expectedNumRules = 70 if ($PSVersionTable.PSVersion.Major -le 4) { # for PSv3 PSAvoidGlobalAliases is not shipped because From 4e7449ae171670f0aac9c94a85fa776d4281ebf9 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Thu, 1 Feb 2024 17:49:06 +0000 Subject: [PATCH 7/7] Update docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md --- docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md b/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md index 13b79267a..c30b69844 100644 --- a/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md +++ b/docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md @@ -14,7 +14,7 @@ title: AvoidUsingAllowUnencryptedAuthentication Avoid using the `AllowUnencryptedAuthentication` switch on `Invoke-WebRequest`, `Invoke-RestMethod`, and other webrequest cmdlets, which sends credentials and secrets over unencrypted connections. This should be avoided except for compatability with legacy systems. -For more details, see the documentation warning [here](https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.utility/invoke-webrequest?view=powershell-7.2#-allowunencryptedauthentication). +For more details, see the documentation warning [here](https://learn.microsoft.com/powershell/module/microsoft.powershell.utility/invoke-webrequest#-allowunencryptedauthentication). ## How