Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add AvoidUsingAllowUnencryptedAuthentication #1857

Merged
merged 8 commits into from
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 117 additions & 0 deletions Rules/AvoidUsingAllowUnencryptedAuthentication.cs
Original file line number Diff line number Diff line change
@@ -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
{
/// <summary>
/// AvoidUsingAllowUnencryptedAuthentication: Avoid sending credentials and secrets over unencrypted connections.
/// </summary>
#if !CORECLR
[Export(typeof(IScriptRule))]
#endif
public class AvoidUsingAllowUnencryptedAuthentication : AvoidParameterGeneric
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, didn't know there was this base class to make implementation so much simpler

{
/// <summary>
/// Condition on the cmdlet that must be satisfied for the error to be raised
/// </summary>
/// <param name="CmdAst"></param>
/// <returns></returns>
public override bool CommandCondition(CommandAst CmdAst)
{
return true;
}

/// <summary>
/// Condition on the parameter that must be satisfied for the error to be raised.
/// </summary>
/// <param name="CmdAst"></param>
/// <param name="CeAst"></param>
/// <returns></returns>
public override bool ParameterCondition(CommandAst CmdAst, CommandElementAst CeAst)
{
return CeAst is CommandParameterAst && String.Equals((CeAst as CommandParameterAst).ParameterName, "AllowUnencryptedAuthentication", StringComparison.OrdinalIgnoreCase);
}

/// <summary>
/// Retrieves the error message
/// </summary>
/// <param name="FileName"></param>
/// <param name="CmdAst"></param>
/// <returns></returns>
public override string GetError(string fileName, CommandAst cmdAst)
{
return String.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingAllowUnencryptedAuthenticationError);
}

/// <summary>
/// GetName: Retrieves the name of this rule.
/// </summary>
/// <returns>The name of this rule</returns>
public override string GetName()
{
return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.AvoidUsingAllowUnencryptedAuthenticationName);
}

/// <summary>
/// GetCommonName: Retrieves the common name of this rule.
/// </summary>
/// <returns>The common name of this rule</returns>
public override string GetCommonName()
{
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingAllowUnencryptedAuthenticationCommonName);
}

/// <summary>
/// GetDescription: Retrieves the description of this rule.
/// </summary>
/// <returns>The description of this rule</returns>
public override string GetDescription()
{
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingAllowUnencryptedAuthenticationDescription);
}

/// <summary>
/// GetSourceType: Retrieves the type of the rule: builtin, managed or module.
/// </summary>
public override SourceType GetSourceType()
{
return SourceType.Builtin;
}

/// <summary>
/// GetSeverity: Retrieves the severity of the rule: error, warning or information.
/// </summary>
/// <returns></returns>
public override RuleSeverity GetSeverity()
{
return RuleSeverity.Warning;
}

/// <summary>
/// DiagnosticSeverity: Retrieves the severity of the rule of type DiagnosticSeverity: error, warning or information.
/// </summary>
/// <returns></returns>
public override DiagnosticSeverity GetDiagnosticSeverity()
{
return DiagnosticSeverity.Warning;
}

/// <summary>
/// GetSourceName: Retrieves the module/assembly name the rule is from.
/// </summary>
public override string GetSourceName()
{
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
}
}
}
12 changes: 12 additions & 0 deletions Rules/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1206,4 +1206,16 @@
<data name="AvoidExclaimOperatorCorrectionDescription" xml:space="preserve">
<value>Replace ! with -not</value>
</data>
<data name="AvoidUsingAllowUnencryptedAuthenticationCommonName" xml:space="preserve">
<value>Avoid AllowUnencryptedAuthentication Switch</value>
</data>
<data name="AvoidUsingAllowUnencryptedAuthenticationDescription" xml:space="preserve">
<value>Avoid sending credentials and secrets over unencrypted connections.</value>
</data>
<data name="AvoidUsingAllowUnencryptedAuthenticationError" xml:space="preserve">
<value>The insecure AllowUsingUnencryptedAuthentication switch was used. This should be avoided except for compatability with legacy systems.</value>
</data>
<data name="AvoidUsingAllowUnencryptedAuthenticationName" xml:space="preserve">
<value>AvoidUsingAllowUnencryptedAuthentication</value>
</data>
</root>
2 changes: 1 addition & 1 deletion Tests/Engine/GetScriptAnalyzerRule.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 38 additions & 0 deletions Tests/Rules/AvoidUsingAllowUnencryptedAuthentication.tests.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.

BeforeAll {
$settings = @{
IncludeRules = @('PSAvoidUsingAllowUnencryptedAuthentication')
Rules = @{
PSAvoidUsingAllowUnencryptedAuthentication = @{
Enable = $true
}
}
}
}

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd be happier if these were separate test cases, but this is ok

(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 "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
}
}
}
35 changes: 35 additions & 0 deletions docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
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 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.
MJVL marked this conversation as resolved.
Show resolved Hide resolved

For more details, see the documentation warning [here](https://learn.microsoft.com/powershell/module/microsoft.powershell.utility/invoke-webrequest#-allowunencryptedauthentication).

## How

Avoid using the `AllowUnencryptedAuthentication` switch.

## Example 1

### Wrong

```powershell
Invoke-WebRequest foo -AllowUnencryptedAuthentication
```

### Correct

```powershell
Invoke-WebRequest foo
```
1 change: 1 addition & 0 deletions docs/Rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,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 | Yes<sup>2</sup> |
| [AvoidUsingComputerNameHardcoded](./AvoidUsingComputerNameHardcoded.md) | Error | Yes | |
Expand Down