Stevehose#259 checklist bugs and an enhancement #556

stevehose · 2019-12-18T21:43:53Z

Pull Request (PR) description:
This pull request includes a pile of bug fixes for the New-StigChecklist function:

Fixed #427: Windows 10 Rule V-63373 fails to apply settings to system drive
Fixed #514: Feature request: additional support for servicerule properties
Fixed #521: Organizational setting warning should include Stig name
Fixed #259: Checklist .ckl file fails XML validation in Stig Viewer 2.8.
Fixed #527: Checklist is not using manualcheckfile when using DscResult.
Fixed #548: Target/host data is blank when creating a new checklist.
Fixed #546: Typecast causing an issue when trying to generate checklist using New-StigChecklist function.
Fixed #401: Checklists generated by New-StigChecklist do not provide finding details.

It also includes the following enhancement

Update to PowerSTIG to show duplicate rule status matching in a checklist: #257. With this update, duplicate rules will show the same status as the original rule when added to a newly-generated checklist. This will boost the number of STIGs that show as processed by PowerSTIG.
This Pull Request (PR) fixes the following issues:

Task list:

Change details added to Unreleased section of CHANGELOG.md (Not required for Convert modules)?
Added/updated documentation, comment-based help and descriptions where appropriate?
Examples appropriately updated?
New/changed code adheres to Style Guidelines?
Unit and (optional) Integration tests created/updated where possible?

This change is

…525) * Added property based on community request * updated issue version

* added support for 2012R2 MS/DC 2.17/2.18 * added new line on the xccdf * removed tab from processed xml.

* Initial Convert and update IIS 8.5 1.9 * remove n-2 STIGs * removed quotes * added newline to raw xccdf * updated based on PR feedback

…ance STIG - Ver 1, Rel 7 (#544) * fixed and updated SQL Instance STIGs * updated sqlserver composite and removed tabs * updated sqlserver composite. * removed tabs

* updated JRE rule V-66941.a to be a org settings * updated name of processed STIG * update changelog * added space to TS build issue.

…rocessed STIGs (#550) * updated Win2016DC failed converts and added tests * removed V-73517 from MS-1.9 as the rule no longer exist.

…6 V1R1 (#553) * added Office-System2013 STIG support. * reconverted xccdfs, corrected issues in some processed stigs. * added OfficeSystem 2016 V1R1 STIG * updated changelog.md

* Update to fix checklist bugs * Fixed bug in checklist parameter ManualCheckFile * Updated Checklist Pester tests * Updates based upon PR comments * Updated changelog.md

* Added helper function And test to verify module versions * Added tests to assert dependant module versions. * Removed commented code * Removed whitespace

* added support for 2019 MS modified hardcoded parser rule ids to support 2019 MS * added support for 2019 DC STIG; parser update to address failed AD permission rules (ActiveDirectoryAuditRule) which isn't currently implemented * Added ProcessMitigation to WindowsServer composite * regenerated all xccdfs, 6 were corrected/modified * updated changelog.md * appveyor build issue - space insert

…soft/PowerStig into stevehose#259ChecklistBugs

# Conflicts: # CHANGELOG.md # Module/STIG/Functions.Checklist.ps1 # StigData/Processed/SqlServer-2016-Instance-1.3.xml

codecov-io · 2019-12-21T19:35:53Z

Codecov Report

Merging #556 into 4.3.0 will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##            4.3.0     #556   +/-   ##
=======================================
  Coverage   77.83%   77.83%           
=======================================
  Files          16       16           
  Lines         203      203           
  Branches        3        3           
=======================================
  Hits          158      158           
  Misses         42       42           
  Partials        3        3

Impacted Files	Coverage Δ
PowerStig.psm1	`100% <ø> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0357067...5f3bd7c. Read the comment docs.

jcwalker

Reviewed 11 of 12 files at r1.
Reviewable status: 11 of 12 files reviewed, 27 unresolved discussions (waiting on @bcwilhite, @jcwalker, and @stevehose)

Module/Common/Functions.XccdfXml.ps1, line 38 at r1 (raw file):

    )

    Begin

Begin,Process, End are keywords so they need to be lowercase per the styleguidelines.

Module/Common/Functions.XccdfXml.ps1, line 40 at r1 (raw file):

    Begin
    {
        $CurrentVerbosePreference = $global:VerbosePreference

$CurrentVerbosePreference should be camel case.

Module/Common/Functions.XccdfXml.ps1, line 50 at r1 (raw file):

    {
        # Get the raw xccdf xml to pull additional details from the root node.
        [xml] $msStig = Get-Content -Path $path

$path should be pascal case.

Module/Common/Functions.XccdfXml.ps1, line 58 at r1 (raw file):

        # Remove DC and Core settings from the MS xml
        Write-Information -MessageData "Removing Domain Controller and Core settings from Member Server STIG"

Lets put an empty line between Write-Information and foreach throughout this file.

Module/Common/Functions.XccdfXml.ps1, line 110 at r1 (raw file):

        }

        $FilePath = "$Destination\$(Split-Path -Path $path -Leaf)"

$FilePath should be camel case.

Module/Common/Functions.XccdfXml.ps1, line 125 at r1 (raw file):

<#
    .SYNOPSIS

Lets put a blank line after each section in the comment based help to make it consistent with the existing code.

Module/Common/Functions.XccdfXml.ps1, line 161 at r1 (raw file):

        [int] $stigProcessedCounter = 1

        # Global added so that the stig rule can be referenced later

Lets end this sentence with a period to be consistent.

Module/Common/Functions.XccdfXml.ps1, line 183 at r1 (raw file):

            foreach ($correction in $StigGroupListChangeLog[$stigRule.Id])
            {
                # If the logfile contains a single * as the OldText, treat it as replacing everything with the newText value

Lets end this sentence with a period.

Module/Common/Functions.XccdfXml.ps1, line 195 at r1 (raw file):

                }
            }
            $rules = [ConvertFactory]::Rule($stigRule)

Styleguidelines recommend a newline after a closing brace.

Module/Common/Functions.XccdfXml.ps1, line 228 at r1 (raw file):

                }
            }
            $stigProcessedCounter ++

Styleguidelines recommend a newline after a closing brace.

Module/Common/Functions.XccdfXml.ps1, line 233 at r1 (raw file):

    end
    {
        $global:stigSettings

Let put a return statement here. I know it's not needed but it will be consistent with C# and the current code.

Module/Common/Functions.XccdfXml.ps1, line 239 at r1 (raw file):

<#
    .SYNOPSIS
        Creates the file name to create from the xccdf content

Lets put a blank line after each section in the comment based help to make it consistent with the existing code.

Module/Common/Functions.XccdfXml.ps1, line 278 at r1 (raw file):

    else
    {
        #$Destination = "$(Split-Path -Path (Split-Path -Path (Split-Path -Path $PSScriptRoot)))\StigData\Processed"

Remove commented code.

Module/Common/Functions.XccdfXml.ps1, line 291 at r1 (raw file):

<#
    .SYNOPSIS

Lets put a blank line after each section in the comment based help to make it consistent with the existing code.

Module/Common/Functions.XccdfXml.ps1, line 487 at r1 (raw file):

            -split "(Release:)(.*?)(Benchmark)" )[2].trim()

    "$($StigDetails.Benchmark.version).$revision"

lets put a return statement here

Module/STIG/Functions.Checklist.ps1, line 101 at r1 (raw file):

        $TargetNode = $DscResult.PSComputerName
    }

Whitepsace

Module/STIG/Functions.Checklist.ps1, line 217 at r1 (raw file):

    $fileList = Get-PowerStigFileList -StigDetails $xccdfBenchmark
    $processedFileName = $fileList.Settings.FullName
    [xml]$processed = get-content -Path $processedFileName

Lets Pascal case the Get-Content cmdlet

Module/STIG/Functions.Checklist.ps1, line 219 at r1 (raw file):

    [xml]$processed = get-content -Path $processedFileName

    $vulnerabilities = Get-VulnerabilityList -XccdfBenchmark $xccdfBenchmarkContent

Lets put an empty line above the foreach

Module/STIG/Functions.Checklist.ps1, line 281 at r1 (raw file):

        elseif ($PSCmdlet.ParameterSetName -eq 'result')
        {
            $manualCheck = $manualCheckData | Where-Object {$_.VulID -eq $VID}

Lets use name parameters, (-FileterScript)

Module/STIG/Functions.Checklist.ps1, line 287 at r1 (raw file):

                $findingDetails = $manualCheck.Details
                $comments = $manualCheck.Comments
            } else {

else needs to be on it's own line.

Module/STIG/Functions.Checklist.ps1, line 316 at r1 (raw file):

        # Test to see if this rule is managed as a duplicate
        $convertedRule = $processed.SelectSingleNode("//Rule[@id='$vid']")

Lets put a blank line after $convertedRule...etc...

Module/STIG/Functions.Checklist.ps1, line 561 at r1 (raw file):

    }
}
function Get-FindingDetailsString

All the functions need comment based help.

Module/STIG/Functions.Checklist.ps1, line 618 at r1 (raw file):

            return 'MACAddress'
        }

Whitepsace

Module/STIG/Functions.Checklist.ps1, line 626 at r1 (raw file):

            return 'IPv4Address'
        }

Whitepsace

Module/STIG/Functions.Checklist.ps1, line 634 at r1 (raw file):

            return 'IPv6Address'
        }

Whitespace

Module/STIG/Convert/Functions.PowerStigXml.ps1, line 98 at r1 (raw file):

    }

    Get-RegistryRuleExpressions -Path $Path -StigBenchmarkXml $stigBenchmarkXml

Does this function return 2 different objects?

Module/STIG/Convert/Functions.PowerStigXml.ps1, line 130 at r1 (raw file):

    )

    Begin

Begin, Process, End should be lowercase.

stevehose

Reviewable status: 11 of 12 files reviewed, 27 unresolved discussions (waiting on @bcwilhite, @jcwalker, and @stevehose)

Module/Common/Functions.XccdfXml.ps1, line 38 at r1 (raw file):