EvotecIT / Testimo

Testimo is a PowerShell module for running health checks for Active Directory against a bunch of different tests
MIT License
534 stars 56 forks source link

Password Compexity Test #62

Open JasonCook599 opened 4 years ago

JasonCook599 commented 4 years ago

I have a few thoughts about the Password Complexity test and wanted to hear thoughts and comments from others. Broadly speaking, I am proposing this test falls in line with Microsoft's Security Baseline for Windows 1903.

  1. Lockout Threshold Currently, the test passes when the threshold is greater than five attempts. I suggest the test should pass is the value is less than or equal to five attempts as to not fail tests that are too secure.

  2. Password Expiration Currently, the test passes if the password age is less than or equal to 60. Microsoft has dropped this requirement and give the rationale in the blog post linked above. I suggest this test should be dropped.

  3. Minimum Password Lenght Currently, the test passes if the password length is greater than eight characters. Microsoft's latest recommendation is 14 characters.

  4. Minimum Password Age Currently, the test passes if the minimum age is less than or equal to one day. I suggest the test passes if the age is greater than or equal to one day. A value of less than one allows a password to be changed immediately thus negating password reuse policy.

  5. Password History Count Currently, the test passes if the number of previous passwords remembered is greater than or equal to 10. Microsoft's recommendation is 24.

What are your thoughts on this issue? I am happy to make these changes myself but wanted to find if these changes would be appreciated first.

I also wonder if there is a warning feature? Perhaps a pass if it matches Microsoft's guidelines, a warning if it doesn't, and fails if it doesn't pass the current test.

JasonCook599 commented 4 years ago

https://github.com/EvotecIT/Testimo/blob/7eb2cc9b9127e25a0a5f729ad2d298f9eb2f3004/Private/SourcesDomain/PasswordComplexity.ps1#L22-L105

PrzemyslawKlys commented 4 years ago

My thoughts are this:

I understand where you're coming from thou. My idea for this would be to implement multiple tests for the same value. Something like for ComplexityEnabled below where Parameters/Details are filled in with the different text, risk level. I also mentioned that possibly there should be more states https://github.com/EvotecIT/Testimo/issues/40#issuecomment-531939756. Null/True/False seems to be not enough.

$PasswordComplexity = @{
    Enable = $true
    Source = @{
        Name    = 'Password Complexity Requirements'
        Data    = {
            # Imports all commands / including private ones from PSWinDocumentation.AD
            $ADModule = Import-Module PSWinDocumentation.AD -PassThru
            & $ADModule { param($Domain); Get-WinADDomainDefaultPasswordPolicy -Domain $Domain } $Domain
        }
        Details = [ordered] @{
            Area        = ''
            Category    = ''
            Severity    = ''
            RiskLevel   = 0
            Description = ''
            Resolution  = ''
            Resources   = @(

            )
        }
    }
    Tests  = [ordered] @{
        ComplexityEnabled             = @{
            Enable                = $true
            Name                  = 'Complexity Enabled'
            Details               = [ordered] @{
                Area        = ''
                Category    = ''
                Severity    = ''
                RiskLevel   = 0
                Description = ''
                Resolution  = ''
                Resources   = @(

                )
            }
            Parameters            = @{
                Property      = 'Complexity Enabled'
                ExpectedValue = $true
                OperationType = 'eq'
            }
            DetailsRecommended    = [ordered] @{
                Area        = ''
                Category    = ''
                Severity    = ''
                RiskLevel   = 0
                Description = ''
                Resolution  = ''
                Resources   = @(

                )
            }
            ParametersRecommended = @{
                Property      = 'Complexity Enabled'
                ExpectedValue = $true
                OperationType = 'eq'
            }
        }
        'LockoutDuration'             = @{
            Enable     = $true
            Name       = 'Lockout Duration'
            Parameters = @{
                Property      = 'Lockout Duration'
                ExpectedValue = 30
                OperationType = 'ge'
            }
        }
        'LockoutObservationWindow'    = @{
            Enable     = $true
            Name       = 'Lockout Observation Window'
            Parameters = @{
                Property      = 'Lockout Observation Window'
                ExpectedValue = 30
                OperationType = 'ge'
            }
        }
        'LockoutThreshold'            = @{
            Enable     = $true
            Name       = 'Lockout Threshold'
            Parameters = @{
                Property      = 'Lockout Threshold'
                ExpectedValue = 5
                OperationType = 'gt'
            }
        }
        'MaxPasswordAge'              = @{
            Enable     = $true
            Name       = 'Max Password Age'
            Parameters = @{
                Property      = 'Max Password Age'
                ExpectedValue = 60
                OperationType = 'le'
            }
        }
        'MinPasswordLength'           = @{
            Enable     = $true
            Name       = 'Min Password Length'
            Parameters = @{
                Property      = 'Min Password Length'
                ExpectedValue = 8
                OperationType = 'gt'
            }
        }
        'MinPasswordAge'              = @{
            Enable     = $true
            Name       = 'Min Password Age'
            Parameters = @{
                Property      = 'Min Password Age'
                ExpectedValue = 1
                OperationType = 'le'
            }
        }
        'PasswordHistoryCount'        = @{
            Enable     = $true
            Name       = 'Password History Count'
            Parameters = @{
                Property      = 'Password History Count'
                ExpectedValue = 10
                OperationType = 'ge'
            }
        }
        'ReversibleEncryptionEnabled' = @{
            Enable     = $true
            Name       = 'Reversible Encryption Enabled'
            Parameters = @{
                Property      = 'Reversible Encryption Enabled'
                ExpectedValue = $false
                OperationType = 'eq'
            }
        }
    }
}

Since "source" is the most consuming part of the test, testing something twice shouldn't be a problem. We would just need to make sure to define proper/final configuration

JasonCook599 commented 4 years ago

That solution does make sense to me. Should adding the recommended values, as shown above, wait until the multiple states feature is implemented? I'm not sure whether or not adding them now will affect the way the test runs.

PrzemyslawKlys commented 4 years ago

Well, I am not sure if the choice/configuration is final - I am not sure what should be in Area/Category/Severity - what kind of values/states there should be for all fields. All that needs definition, then one would need to kind of define this and modify Testimo to respect this.

While you could modify the configuration now for Password Complexity it will be ignored and later on, you will have to fix it again anyways. We can leave this issue open till Testimo is ready to accept that change.

So first someone needs to sit down, define kind of "RFC" and then slowly build it up :) It needs to be very flexible so it can be extended to all tests, and possibly even other types like Exchange, VM or whatever.

SUBnet192 commented 4 years ago
  • Minimum password age of 1 is good but also a pain because if the user changes the password and wants to change it again (for whatever reason) they need to wait 1 day

That's to prevent users from cycling through passwords to override the password history in a single day and come back to the same initial password they had...

SUBnet192 commented 4 years ago

I would suggest we change the values to what the NIST is recommending nowadays as a baseline. I understand your point about customizing it for your personal use, but like everything else in Testimo, it's baseline based/best practices, so we should at least have the base recommended values.

PrzemyslawKlys commented 4 years ago

My problem is NIST is nowhere near security requirements as far as I can tell. For me, ISO 27002 or something European would be better choice.

I don't agree with not imposing some rules on users. I have a few Clients and a big headache with users using Company2019.

But again, fully agree something has to be agreed on what is the recommended way to go for Testimo. I've chosen defaults that I would expect to see for a Client, rather than looking on best practice.

SUBnet192 commented 4 years ago

I agree with you about users using "Company2019" or whatever. But unfortunately, there is nothing built-in with Windows to prevent this.

My preferences are 10 characters minimum, encourage people to use passphrases (again, can't enforce), no expiration (useless and dumb users will either write it down somewhere or have an incremental counter). The mixed case/digits enforcement ends up with people doing "P@ssw0rd2020!" which isn't, as most password tools are smart enough to deal with character substitution.

So I agree with the base settings of the NIST as a baseline recommendation. I always supplement it (when the client agrees) with a 3rd party tool to add controls like Anixis.

For Testimo, if we could base it on some factual recommendation/guideline vs personal preferences as default values would be the best option. So this way nobody challenges it. And like you said and beautifully implemented, it can be customized to your own liking.

PrzemyslawKlys commented 4 years ago

You can enforce using password filters. For example https://github.com/lithnet/ad-password-protection

SUBnet192 commented 4 years ago

Agreed, third party, not native, just like Anixis. So we can't base Testimo rules expecting people to respect them based on 3rd party tools. Do you have any formal guidelines from an EU standard that shows exactly what their recommendations are like the NIST (PS: I'm Canadian, so it's not an US American bias) :)

PrzemyslawKlys commented 4 years ago

To be honest I have no clue. Unfortunately EU is not like the US - each country has its own rules :/

SUBnet192 commented 4 years ago

So can we agree to use NIST as a baseline since it's the only official recognized entity that published such recommendations?

PrzemyslawKlys commented 4 years ago

I am unsure. Somehow it feels wrong :)