SaDisabled checks #882

dataplat · Apr 27, 2022 · 52c70bc · 52c70bc
1 parent 363d990
commit 52c70bc
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 5 deletions.
diff --git a/Perf Testing pesterv5.ps1 b/Perf Testing pesterv5.ps1
@@ -14,10 +14,18 @@ ipmo ./dbachecks.psd1
 
 # 
 
-$Checks = 'SaRenamed','DefaultFilePath','AdHocDistributedQueriesEnabled','AdHocWorkload',  'DefaultTrace', 'OleAutomationProceduresDisabled', 'CrossDBOwnershipChaining', 'ScanForStartupProceduresDisabled', 'RemoteAccessDisabled', 'SQLMailXPsDisabled', 'DAC', 'OLEAutomation'
-$Checks = 'SaRenamed'
+$Checks = 'SaDisabled','SaRenamed','DefaultFilePath','AdHocDistributedQueriesEnabled','AdHocWorkload',  'DefaultTrace', 'OleAutomationProceduresDisabled', 'CrossDBOwnershipChaining', 'ScanForStartupProceduresDisabled', 'RemoteAccessDisabled', 'SQLMailXPsDisabled', 'DAC', 'OLEAutomation'
+$Checks = 'SaDisabled'
 Compare-CheckRuns -Checks $checks
 
+<#
+When there are default skips (some of the CIS checks) we need to set the configs and check
+
+Set-DbcConfig skip.security.sadisabled -Value $false
+Set-DbcConfig skip.security.sadisabled -Value $true
+Get-DbcConfigValue skip.security.sadisabled
+#>
+
 # Load the function below and then you can keep running the checks defined above in v4 and v5 and compare the performance
 # You can keep updating the .Tests.ps1 files and rerunning the function without needing to re-import hte module
 

diff --git a/Validate v4 adn v5.ps1 b/Validate v4 adn v5.ps1
@@ -14,10 +14,18 @@ ipmo ./dbachecks.psd1
 
 # 
 
-$Checks = 'SaRenamed','DefaultFilePath','AdHocDistributedQueriesEnabled','AdHocWorkload',  'DefaultTrace', 'OleAutomationProceduresDisabled', 'CrossDBOwnershipChaining', 'ScanForStartupProceduresDisabled', 'RemoteAccessDisabled', 'SQLMailXPsDisabled', 'DAC', 'OLEAutomation'
-$Checks = 'SaRenamed'
+$Checks = 'SaDisabled','SaRenamed','DefaultFilePath','AdHocDistributedQueriesEnabled','AdHocWorkload',  'DefaultTrace', 'OleAutomationProceduresDisabled', 'CrossDBOwnershipChaining', 'ScanForStartupProceduresDisabled', 'RemoteAccessDisabled', 'SQLMailXPsDisabled', 'DAC', 'OLEAutomation'
+$Checks = 'SaDisabled'
 Compare-v4andv5Results -Checks $Checks
 
+<#
+When there are default skips (some of the CIS checks) we need to set the configs and check
+
+Set-DbcConfig skip.security.sadisabled -Value $false
+Set-DbcConfig skip.security.sadisabled -Value $true
+Get-DbcConfigValue skip.security.sadisabled
+#>
+
 # Load the function below and then you can keep running the checks defined above in v4 and v5 and compare the performance
 # You can keep updating the .Tests.ps1 files and rerunning the function without needing to re-import hte module
 
@@ -28,7 +36,8 @@ Compare-v4andv5Results -Checks $Checks
 # If you get odd results - or you dont get any checks run
 
 # run the import module and the Invoke Dbc Check with Verbose and that might show you New-Json messing 
-# with your files or that you are lookign in PSMOdulePath instead of Git Repo path (run Reset-dbcConfig to fix that)
+# with your files or that you are looking in PSModulePath instead of Git Repo path (run Reset-dbcConfig to fix that)
+
 
 function Compare-v4andv5Results {
     param($Checks)
@@ -136,4 +145,6 @@ $cred = New-Object -TypeName System.Management.Automation.PSCredential -Argument
 $Sqlinstances = 'localhost,7401', 'localhost,7402', 'localhost,7403'
 Invoke-DbcCheck -SqlInstance $Sqlinstances -SqlCredential $cred -Check $Checks -legacy $false
 Invoke-DbcCheck -SqlInstance $Sqlinstances -SqlCredential $cred -Check $Checks -legacy $true 
+
+$smo = $Instance = Connect-DbaInstance -Sqlinstance $SqlInstances[0] -SqlCredential $cred
 #>
diff --git a/checks/Instancev5.Tests.ps1 b/checks/Instancev5.Tests.ps1
@@ -133,10 +133,19 @@ Describe "Default File Path" -Tag DefaultFilePath, Instance -ForEach $InstancesT
 }
 
 Describe "SA Login Renamed" -Tag SaRenamed, DISA, CIS, Medium, Instance -ForEach $InstancesToTest {
+    $skip = Get-DbcConfigValue skip.instance.SaRenamed
     Context "Checking that sa login has been renamed on <_.Name>" {
         It "sa login has been renamed on <_.Name>" {
             ($PsItem.Logins.Name)  | Should -Not -BeIn 'sa' -Because "Renaming the sa account is a requirement"
         }
     }
 }
 
+Describe "SA Login Disabled" -Tag SaDisabled, DISA, CIS, Medium, Instance -ForEach $InstancesToTest {
+    $skip = Get-DbcConfigValue skip.security.sadisabled
+    Context "Checking that sa login has been disabled on <_.Name>" {
+        It "sa login is disabled on <_.Name>" -Skip:$Skip {
+            ($PsItem.Logins | Where-Object ID -eq 1).IsDisabled | Should -Be $true -Because "We expected the original sa login to be disabled"
+        }
+    }
+}
diff --git a/internal/functions/NewGet-AllInstanceInfo.ps1 b/internal/functions/NewGet-AllInstanceInfo.ps1
@@ -83,6 +83,11 @@ function NewGet-AllInstanceInfo {
         'SaRenamed' {
 
         }
+        'SaDisabled' {
+            $LoginInitFields.Add("IsDisabled") | Out-Null # so we can check if sa is disabled
+            $LoginInitFields.Add("ID") | Out-Null # so we can check if sa is disabled even if it has been renamed
+            $Instance.SetDefaultInitFields([Microsoft.SqlServer.Management.Smo.Settings], $LoginInitFields)
+        }
 
         Default { }
     }