diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b1a63798..268683bb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## [Unreleased] -* Update PowerSTIG to increase code coverage of unit tests: [#737](https://github.com/microsoft/PowerStig/issues/737) +* Update PowerSTIG to Increase Code Coverage of Unit Tests: [#737](https://github.com/microsoft/PowerStig/issues/737) * Update PowerSTIG with new SkipRuleSeverity Parameter to skip entire STIG Category/Severity Level(s): [711](https://github.com/microsoft/PowerStig/issues/711) ## [4.5.0] - 2020-09-01 diff --git a/Tests/Unit/Module/.tests.header.ps1 b/Tests/Unit/Module/.tests.header.ps1 index af9bedc4b..222b0b701 100644 --- a/Tests/Unit/Module/.tests.header.ps1 +++ b/Tests/Unit/Module/.tests.header.ps1 @@ -108,7 +108,8 @@ if ($global:moduleName -ne 'STIG.Checklist' -and $global:moduleName -ne 'STIG.Do } else { - import-module $script:moduleRoot\Module\Common\Common.psm1 + $commonModulePath = Join-Path -Path $script:moduleRoot -ChildPath 'Module\Common\Common.psm1' + Import-Module -Name $commonModulePath } <# diff --git a/Tests/Unit/Module/PermissionRule.tests.ps1 b/Tests/Unit/Module/PermissionRule.tests.ps1 index d730862ae..d2a3f1d61 100644 --- a/Tests/Unit/Module/PermissionRule.tests.ps1 +++ b/Tests/Unit/Module/PermissionRule.tests.ps1 @@ -40,7 +40,7 @@ try The default location is the "%SystemRoot%\SYSTEM32\WINEVT\LOGS" directory. They may have been moved to another folder. If the permissions for these files are not as restrictive as the ACLs listed, this is a finding.' - } + }, @{ Path = 'HKLM:\SYSTEM\CurrentControlSet\Control\SecurePipeServers\winreg\' AccessControlEntry = @( @@ -84,7 +84,7 @@ try Administrators - Full Control - This key and subkeys Backup Operators - Read - This key only LOCAL SERVICE - Read - This key and subkeys' - } + }, @{ # Windows 10 STIG V-63593 Path = 'HKLM:\SECURITY' @@ -118,6 +118,154 @@ try permission. If the defaults have not been changed, these are not a finding.' + }, + @{ + Path = '%windir%\sysvol' + AccessControlEntry = @( + [pscustomobject]@{ + Rights = 'ReadAndExecute' + Inheritance = 'This folder subfolders and files' + Principal = 'Authenticated Users' + ForcePrincipal = $false + Type = 'Allow' + }, + [pscustomobject]@{ + Rights = 'ReadAndExecute' + Inheritance = 'This folder subfolders and files' + Principal = 'Server Operators' + ForcePrincipal = $false + Type = 'Allow' + }, + [pscustomobject]@{ + Rights = 'AppendData,ChangePermissions,CreateDirectories,CreateFiles,Delete,DeleteSubdirectoriesAndFiles,ExecuteFile,ListDirectory,Modify,Read,ReadAndExecute,ReadAttributes,ReadData,ReadExtendedAttributes,ReadPermissions,Synchronize,TakeOwnership,Traverse,Write,WriteAttributes,WriteData,WriteExtendedAttributes' + Inheritance = 'This folder only' + Principal = 'Administrators' + ForcePrincipal = $false + Type = 'Allow' + }, + [pscustomobject]@{ + Rights = 'FullControl' + Inheritance = 'Subfolders and files only' + Principal = 'CREATOR OWNER' + ForcePrincipal = $false + Type = 'Allow' + }, + [pscustomobject]@{ + Rights = 'FullControl' + Inheritance = 'Subfolders and files only' + Principal = 'Administrators' + ForcePrincipal = $false + Type = 'Allow' + }, + [pscustomobject]@{ + Rights = 'FullControl' + Inheritance = 'This folder subfolders and files' + Principal = 'SYSTEM' + ForcePrincipal = $false + Type = 'Allow' + } + ) + Force = $true + OrganizationValueRequired = $false + CheckContent = "Verify the permissions on the SYSVOL directory. + + Open a command prompt. + Run `"net share`". + Make note of the directory location of the SYSVOL share. + + By default this will be \Windows\SYSVOL\sysvol. For this requirement, permissions will be verified at the first SYSVOL directory level. + + Open File Explorer. + Navigate to \Windows\SYSVOL (or the directory noted previously if different). + Right click the directory and select properties. + Select the Security tab. + Click Advanced. + + If any standard user accounts or groups have greater than read & execute permissions, this is a finding. The default permissions noted below meet this requirement. + + Type - Allow + Principal - Authenticated Users + Access - Read & execute + Inherited from - None + Applies to - This folder, subfolder and files + + Type - Allow + Principal - Server Operators + Access - Read & execute + Inherited from - None + Applies to - This folder, subfolder and files + + Type - Allow + Principal - Administrators + Access - Special + Inherited from - None + Applies to - This folder only + (Access - Special - Basic Permissions: all selected except Full control) + + Type - Allow + Principal - CREATOR OWNER + Access - Full control + Inherited from - None + Applies to - Subfolders and files only + + Type - Allow + Principal - Administrators + Access - Full control + Inherited from - None + Applies to - Subfolders and files only + + Type - Allow + Principal - SYSTEM + Access - Full control + Inherited from - None + Applies to - This folder, subfolders and files + + Alternately, use Icacls.exe to view the permissions of the SYSVOL directory. + Open a command prompt. + Run `"icacls c:\Windows\SYSVOL + The following results should be displayed: + + NT AUTHORITY\Authenticated Users:(RX) + NT AUTHORITY\Authenticated Users:(OI)(CI)(IO)(GR,GE) + BUILTIN\Server Operators:(RX) + BUILTIN\Server Operators:(OI)(CI)(IO)(GR,GE) + BUILTIN\Administrators:(M,WDAC,WO) + BUILTIN\Administrators:(OI)(CI)(IO)(F) + NT AUTHORITY\SYSTEM:(F) + NT AUTHORITY\SYSTEM:(OI)(CI)(IO)(F) + BUILTIN\Administrators:(M,WDAC,WO) + CREATOR OWNER:(OI)(CI)(IO)(F) + + (RX) - Read & execute + Run `"icacls /help`" to view definitions of other permission codes." + }, + @{ + Path = '%windir%\NTDS\*.*' + AccessControlEntry = @( + [pscustomobject]@{ + Rights = 'FullControl' + Inheritance = '' + Principal = 'NT AUTHORITY\SYSTEM' + ForcePrincipal = $false + } + ) + Force = $true + OrganizationValueRequired = $false + CheckContent = 'Verify the permissions on the content of the NTDS directory. + Open the registry editor (regedit). + Navigate to HKEY_LOCAL_MACHINE\System\CurrentControlSet\Services\NTDS\Parameters. + Note the directory locations in the values for: + Database log files path + DSA Database file + By default they will be \Windows\NTDS. If the locations are different, the following will need to be run for each. + Open an elevated command prompt (Win+x, Command Prompt (Admin)). + Navigate to the NTDS directory (\Windows\NTDS by default). + Run "icacls *.*". + If the permissions on each file are not at least as restrictive as the following, this is a finding. + NT AUTHORITY\SYSTEM:(I)(F) + (I) - permission inherited from parent container + (F) - full access + Do not use File Explorer to attempt to view permissions of the NTDS folder. Accessing the folder through File Explorer will change the permissions on the folder.' } ) #endregion diff --git a/Tests/Unit/Module/RegistryRule.tests.ps1 b/Tests/Unit/Module/RegistryRule.tests.ps1 index 27f6f1d2c..63e98f20c 100644 --- a/Tests/Unit/Module/RegistryRule.tests.ps1 +++ b/Tests/Unit/Module/RegistryRule.tests.ps1 @@ -186,6 +186,88 @@ try HKCU\Software\Policies\Microsoft\Office\16.0\excel\security\fileblock Criteria: If the value XL4Workbooks is REG_DWORD = 2, this is not a finding.' + }, + @{ + Key = 'HKEY_LOCAL_MACHINE\Software\Policies\Microsoft\Windows\TCPIP\v6Transition' + ValueName = 'Force_Tunneling' + ValueData = 'Enabled' + ValueType = 'String' + Ensure = 'Present' + OrganizationValueRequired = $false + CheckContent = 'If the following registry value does not exist or is not configured as specified, this is a finding: + + Registry Hive: HKEY_LOCAL_MACHINE + Registry Path: \Software\Policies\Microsoft\Windows\TCPIP\v6Transition\ + + Value Name: Force_Tunneling + + Type: REG_SZ + Value: Enabled' + }, + @{ + Key = 'HKEY_LOCAL_MACHINE\Software\Policies\Microsoft\Windows\LocationAndSensors' + ValueName = 'DisableLocation' + ValueData = '1' + ValueType = 'Dword' + Ensure = 'Present' + OrganizationValueRequired = $false + CheckContent = 'If the following registry value does not exist or is not configured as specified, this is a finding: + + Registry Hive: HKEY_LOCAL_MACHINE + Registry Path: \Software\Policies\Microsoft\Windows\LocationAndSensors\ + + Value Name: DisableLocation + + Type: REG_DWORD + Value: 1 (Enabled) + + If location services are approved for the system by the organization, this may be set to "Disabled" (0). This must be documented with the ISSO.' + }, + @{ + Key = 'HKEY_LOCAL_MACHINE\System\CurrentControlSet\Control\SecurePipeServers\Winreg\AllowedExactPaths' + ValueName = 'Machine' + ValueData = 'System\CurrentControlSet\Control\ProductOptions;System\CurrentControlSet\Control\Server Applications;Software\Microsoft\Windows NT\CurrentVersion' + ValueType = 'MultiString' + Ensure = 'Present' + OrganizationValueRequired = $false + CheckContent = "If the following registry value does not exist or is not configured as specified, this is a finding: + +Registry Hive: HKEY_LOCAL_MACHINE +Registry Path: \System\CurrentControlSet\Control\SecurePipeServers\Winreg\AllowedExactPaths\ + +Value Name: Machine + +Value Type: REG_MULTI_SZ +Value: see below +System\CurrentControlSet\Control\ProductOptions +System\CurrentControlSet\Control\Server Applications +Software\Microsoft\Windows NT\CurrentVersion + +Legitimate applications may add entries to this registry value. If an application requires these entries to function properly and is documented with the ISSO, this would not be a finding. +Documentation must contain supporting information from the vendor's instructions." + }, + @{ + Key = 'HKEY_LOCAL_MACHINE\System\CurrentControlSet\Services\LanManServer\Parameters' + ValueName = 'NullSessionPipes' + ValueData = 'netlogon;samr;lsarpc' + ValueType = 'MultiString' + Ensure = 'Present' + OrganizationValueRequired = $false + CheckContent = "If the following registry value does not exist or is not configured as specified, this is a finding: + + Registry Hive: HKEY_LOCAL_MACHINE + Registry Path: \System\CurrentControlSet\Services\LanManServer\Parameters\ + + Value Name: NullSessionPipes + + Value Type: REG_MULTI_SZ + Value: netlogon, samr, lsarpc + + The default configuration of systems promoted to domain controllers may include a blank entry in the first line prior to `"netlogon`", `"samr`", and `"lsarpc`". This will appear in the registry as a blank + entry when viewing the registry key summary; however the value data for `"NullSessionPipes`" will contain the default entries. + + Legitimate applications may add entries to this registry value. If an application requires these entries to function properly and is documented with the ISSO, this would not be a finding. + Documentation must contain supporting information from the vendor's instructions." } ) #endregion @@ -300,6 +382,7 @@ try } } } + Describe 'Match Static method' { $stringsToTest = @( @@ -317,6 +400,7 @@ try [RegistryRuleConvert]::Match($stringsToTest.string) | Should -Be $false } } + Describe 'Test-RegistryValueDataContainsRange' { $rangeStrings = @( diff --git a/Tests/Unit/Module/SqlScriptQueryRule.tests.ps1 b/Tests/Unit/Module/SqlScriptQueryRule.tests.ps1 index 307d659af..b6f188667 100644 --- a/Tests/Unit/Module/SqlScriptQueryRule.tests.ps1 +++ b/Tests/Unit/Module/SqlScriptQueryRule.tests.ps1 @@ -160,6 +160,153 @@ try FixText = "Remove the publicly available `"AdventureWorks`" database from SQL Server by running the following query: DROP DATABASE AdventureWorks" } + SaAccountRename = @{ + GetScript = "SELECT name FROM sys.server_principals WHERE TYPE = 'S' and name not like '%##%'" + SetScript = "alter login sa with name = [`$(saAccountName)]" + TestScript = "SELECT name FROM sys.server_principals WHERE TYPE = 'S' and name = 'sa'" + CheckContent = "Verify the SQL Server default 'sa' account name has been changed. + Navigate to SQL Server Management Studio >> Object Explorer >> <'SQL Server name'> >> Security >> Logins. + If SQL Server default 'sa' account name is in the 'Logins' list, this is a finding." + FixText = "Navigate to SQL Server Management Studio >> Object Explorer >> <'SQL Server name'> >> Security >> Logins >> click 'sa' account name. + Hit <F2> while the name is highlighted in order to edit the name. + Rename the 'sa' account." + } + TraceFileLimit = @{ + GetScript = "SELECT * FROM ::fn_trace_getinfo(NULL)" + SetScript = "DECLARE @new_trace_id INT; DECLARE @maxsize bigint DECLARE @maxRolloverFiles int DECLARE @traceId int DECLARE @traceFilePath nvarchar(500) SET @traceFilePath = N'`$(TraceFilePath)' SET @traceId = (Select Id from sys.traces where path LIKE (@traceFilePath + '%')) SET @maxsize = `$(MaxTraceFileSize) SET @maxRolloverFiles = `$(MaxRollOverFileCount) EXEC sp_trace_setstatus @traceid, @status = 2 EXECUTE master.dbo.sp_trace_create @new_trace_id OUTPUT, 6, @traceFilePath, @maxsize, NULL, @maxRolloverFiles " + TestScript = "DECLARE @traceFilePath nvarchar(500) DECLARE @desiredFileSize bigint DECLARE @desiredMaxFiles int DECLARE @currentFileSize bigint DECLARE @currentMaxFiles int SET @traceFilePath = N'`$(TraceFilePath)' SET @currentFileSize = (SELECT max_size from sys.traces where path LIKE (@traceFilePath + '%')) SET @currentMaxFiles = (SELECT max_files from sys.traces where path LIKE (@traceFilePath + '%')) IF (@currentFileSize != `$(MaxTraceFileSize)) BEGIN PRINT 'file size not in desired state' SELECT max_size from sys.traces where path LIKE (@traceFilePath + '%') END IF (@currentMaxFiles != `$(MaxRollOverFileCount)) BEGIN PRINT 'max files not in desired state'SELECT max_files from sys.traces where path LIKE (@traceFilePath + '%') END" + CheckContent = "Check the SQL Server audit setting on the maximum number of files of the trace used for the auditing requirement. + Select * from sys.traces. Determine the audit being used to fulfill the overall auditing requirement. Examine the max_files and max_size parameters. SQL will overwrite the oldest files when the max_files parameter has been exceeded. Care must be taken to ensure that this does not happen, or data will be lost. + The amount of space determined for logging by SQL Server is calculated by multiplying the maximum number of files by the maximum file size. + If auditing will outgrow the space reserved for logging before being overwritten, this is a finding." + FixText = "Configure the maximum number of audit log files that are to be generated, staying within the number of logs the system was sized to support. + Update the max_files parameter of the audits to ensure the correct number of files is defined." + } + ShutdownOnError = @{ + GetScript = "SELECT * FROM ::fn_trace_getinfo(NULL)" + SetScript = "DECLARE @new_trace_id INT; DECLARE @traceid INT; SET @traceId = (SELECT traceId FROM ::fn_trace_getinfo(NULL) WHERE Value = 6) EXECUTE master.dbo.sp_trace_create @results = @new_trace_id OUTPUT, @options = 6, @traceFilePath = N'`$(TraceFilePath)'" + TestScript = "DECLARE @traceId int SET @traceId = (SELECT traceId FROM ::fn_trace_getinfo(NULL) WHERE Value = 6) IF (@traceId IS NULL) SELECT traceId FROM ::fn_trace_getinfo(NULL) ELSE Print NULL" + CheckContent = "From the query prompt: + SELECT DISTINCT traceid FROM sys.fn_trace_getinfo(0); + All currently defined traces for the SQL Server instance will be listed. If no traces are returned, this is a finding. + Determine the trace being used for the auditing requirement. Replace # in the following code with a traceid being used for the auditing requirements. + From the query prompt, determine whether the trace options include the value 4, which means SHUTDOWN_ON_ERROR: + SELECT CAST(value AS INT) + FROM sys.fn_trace_getinfo(#) + where property = 1; + If the query does not return a value, this is a finding. + If a value is returned but is not 4 or 6, this is a finding. + (6 represents the combination of values 2 and 4. 2 means TRACE_FILE_ROLLOVER.) + NOTE: Microsoft has flagged the trace techniques and tools used in this STIG as deprecated. They will be removed at some point after SQL Server 2014. The replacement feature is Extended Events. If Extended Events are in use and configured to satisfy this requirement, this is not a finding. The following code can be used to check Extended Events settings. + /********************************** + Check to verify shutdown on failure is set. + The following settings are what should be returned: + name = <name of audit> + on_failure = 1 + on_failure_desc = SHUTDOWN SERVER INSTANCE + **********************************/ + SELECT name, on_failure, on_failure_desc + FROM sys.server_audits " + FixText = "If a trace does not exist, create a trace specification that complies with requirements. + If a trace exists, but is not set to SHUTDOWN_ON_ERROR, modify the SQL Server audit setting to immediately shutdown the database in the event of an audit failure by setting property 1 to a value of 4 or 6 for the audit. + (See the SQL Server Help page for sys.sp_trace_create for implementation details.)" + } + ViewAnyDatabase = @{ + GetScript = "SELECT who.name AS [Principal Name], who.type_desc AS [Principal Type], who.is_disabled AS [Principal Is Disabled], what.state_desc AS [Permission State], what.permission_name AS [Permission Name] FROM sys.server_permissions what INNER JOIN sys.server_principals who ON who.principal_id = what.grantee_principal_id WHERE what.permission_name = 'View any database' AND who.type_desc = 'SERVER_ROLE' ORDER BY who.name" + SetScript = "REVOKE External access assembly TO '`$(ViewAnyDbUser)'" + TestScript = "SELECT who.name AS [Principal Name], who.type_desc AS [Principal Type], who.is_disabled AS [Principal Is Disabled], what.state_desc AS [Permission State], what.permission_name AS [Permission Name] FROM sys.server_permissions what INNER JOIN sys.server_principals who ON who.principal_id = what.grantee_principal_id WHERE what.permission_name = 'View any database' AND who.type_desc = 'SERVER_ROLE' AND who.name != '`$(ViewAnyDbUser)' ORDER BY who.name" + CheckContent = "Obtain the list of roles that are authorized for the SQL Server 'View any database' permission and what 'Grant', 'Grant With', and/or 'Deny' privilege is authorized. Obtain the list of roles with that permission by running the following query: + SELECT + who.name AS [Principal Name], + who.type_desc AS [Principal Type], + who.is_disabled AS [Principal Is Disabled], + what.state_desc AS [Permission State], + what.permission_name AS [Permission Name] + FROM + sys.server_permissions what + INNER JOIN sys.server_principals who + ON who.principal_id = what.grantee_principal_id + WHERE + what.permission_name = 'View any database' + AND who.type_desc = 'SERVER_ROLE' + ORDER BY + who.name + ; + GO + If any role has 'Grant', 'With Grant' or 'Deny' privileges on this permission and users with that role are not authorized to have the permission, this is a finding. + Alternatively, to provide a combined list for all requirements of this type: + SELECT + what.permission_name AS [Permission Name], + what.state_desc AS [Permission State], + who.name AS [Principal Name], + who.type_desc AS [Principal Type], + who.is_disabled AS [Principal Is Disabled] + FROM + sys.server_permissions what + INNER JOIN sys.server_principals who + ON who.principal_id = what.grantee_principal_id + WHERE + what.permission_name IN + ( + 'Administer bulk operations', + 'Alter any availability group', + 'Alter any connection', + 'Alter any credential', + 'Alter any database', + 'Alter any endpoint ', + 'Alter any event notification ', + 'Alter any event session ', + 'Alter any linked server', + 'Alter any login', + 'Alter any server audit', + 'Alter any server role', + 'Alter resources', + 'Alter server state ', + 'Alter Settings ', + 'Alter trace', + 'Authenticate server ', + 'Control server', + 'Create any database ', + 'Create availability group', + 'Create DDL event notification', + 'Create endpoint', + 'Create server role', + 'Create trace event notification', + 'External access assembly', + 'Shutdown', + 'Unsafe Assembly', + 'View any database', + 'View any definition', + 'View server state' + ) + AND who.type_desc = 'SERVER_ROLE' + ORDER BY + what.permission_name, + who.name + ; + GO + " + FixText = "Remove the `"View any database`" permission access from the role that is not authorized by executing the following query: + REVOKE View any database TO <'role name'>" + } + ChangeDatabaseOwner= @{ + GetScript = "select suser_sname(owner_sid) AS 'Owner' from sys.databases where name = `$(Database)" + SetScript = "ALTER AUTHORIZATION ON DATABASE::`$(Database) to `$(DatabaseOwner)" + TestScript = "SELECT suser_sname(owner_sid) AS 'Owner' FROM sys.databases WHERE name = N'`$(Database)' and suser_sname(owner_sid) != N'`$(DatabaseOwner)';" + Variable = "DatabaseOwner={0}" + CheckContent = "Review system documentation to identify SQL Server accounts authorized to own database objects. + If the SQL Server database ownership list does not exist or needs to be updated, this is a finding. + Run the following SQL query to determine SQL Server ownership of all database objects: + SELECT name AS 'Database name' + , SUSER_SNAME(owner_sid) AS 'Database Owner' + , state_desc AS 'Database state' + FROM sys.databases" + FixText = "Add and/or update system documentation to include any accounts authorized for object ownership and remove any account not authorized. + Reassign database ownership to authorized database owner account: + Navigate to SQL Server Management Studio >> Object Explorer >> <'SQL Server name'> >> Databases >> right click <'database name'> >> Properties >> Files. + Select new database `"Owner`": + Navigate to click on […] >> Select new Database Owner >> Browse… >> click on box to indicate account >> <'OK'> >> <'OK'> >> <'OK'>" + } } #endregion #region Method Tests diff --git a/build.yaml b/build.yaml index 9ffc410a7..548009e17 100644 --- a/build.yaml +++ b/build.yaml @@ -55,7 +55,7 @@ Pester: - tests/Unit ExcludeTag: Tag: - CodeCoverageThreshold: 55 # Set to 0 to bypass + CodeCoverageThreshold: 80 # Set to 0 to bypass CodeCoverageOutputFile: JaCoCo_coverage.xml CodeCoverageOutputFileEncoding: ascii diff --git a/source/Module/Rule.SqlScriptQuery/Convert/Methods.ps1 b/source/Module/Rule.SqlScriptQuery/Convert/Methods.ps1 index 843129981..d101a0e26 100644 --- a/source/Module/Rule.SqlScriptQuery/Convert/Methods.ps1 +++ b/source/Module/Rule.SqlScriptQuery/Convert/Methods.ps1 @@ -1144,7 +1144,7 @@ function Get-ShutdownOnErrorSetScript $CheckContent ) - $setScript += "DECLARE @new_trace_id INT; " + $setScript = "DECLARE @new_trace_id INT; " $setScript += "DECLARE @traceid INT; " $setScript += "SET @traceId = (SELECT traceId FROM ::fn_trace_getinfo(NULL) WHERE Value = 6) " $setScript += "EXECUTE master.dbo.sp_trace_create " @@ -1754,7 +1754,7 @@ function Get-SqlRuleType <# .SYNOPSIS - Determines if a SQL rule requires a variable to + Determines if a SQL rule requires a variable to #> function Test-VariableRequired {