dataplat / dbachecks

✔ SQL Server Environmental Validation
https://dbachecks.readthedocs.io/en/latest/
MIT License
460 stars 141 forks source link

LoginAuditSuccessful Not working as described in CIS Benchmark #1017

Open GrumpyVader opened 10 months ago

GrumpyVader commented 10 months ago

Bug Report

General Troubleshooting steps

Does (Find-Module dbachecks).Version match (Get-Module dbachecks).Version.ToString() YES

image

Version Information

Steps to Reproduce

Invoke-DbcCheck -SqlInstance DBC02 -Check LoginAuditSuccessful

image

Description of Bug

Instance.Assertion : LoginAuditSuccessful does not return data specified by CIS Benchmark documentation.

Test 5.4 Ensure 'SQL Server Audit' is set to capture both 'failed' and 'successful logins' is supposed to check if a SQL Audit has been created to capture both Successful and Failed Logins.

image

Currently LoginAuditSuccessful executes the exact same command as LoginAuditFailed.

image

This check should execute the Audit Query that is outlined in the CIS Benchmark or any DbaTools equivalent.

Ant-Green commented 10 months ago

That particular check is in reference to the instance level errorlog reporting of logins failure/success for point 5.3 of the baseline.

There isn't a check at the moment which deals with 5.4.

Please feel free to add a new check using Get-DbaInstanceAudit and Get-DbaInstanceAuditSpecification to satisfy 5.4 into the Instance level checks.

GrumpyVader commented 10 months ago

Thanks Ant-Green, but I am going to disagree. LoginAuditFailed satisfies 5.3. While the naming of LoginAuditSuccessful does not point to 5.4, the description suggests that it is for 5.4

Expected 'All', because We expected the audit level to be set to capture all logins (successful and failed), but got Failure.

GrumpyVader commented 10 months ago

ok got it. thanks for the clarification.

Ant-Green commented 10 months ago

LoginAuditFailed is there to check that Security is set to "Failed logins only" LoginAuditSuccessful is there to check that Security is set to "Both Successful and Failed logins" (probably needs a wording tweak to say what it is actually checking) Capture1 Capture2

Ant-Green commented 10 months ago

Damn my PC looks like I deleted the other comment, thought I made it a strike through,

But yeah baselines keep changing, some checks need tweaks, some need to be written still, some need removing.

5.4 needs a check writing for it, at the moment it doesn't exist.

New checks need writing too.

GrumpyVader commented 10 months ago

If I get time in the new year, I can take a look at some of it. Seems like my work is going to force CIS/PCI compliance on me. Luckily all of my servers are over 90% compliant and there are some things that I just will not be able to implement but I will have to explain why.