Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Password Compexity Test #62

Open
JasonCook599 opened this issue Sep 18, 2019 · 13 comments
Open

Password Compexity Test #62

JasonCook599 opened this issue Sep 18, 2019 · 13 comments
Labels
enhancement New feature or request

Comments

@JasonCook599
Copy link
Contributor

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
Copy link
Contributor Author

Tests = [ordered] @{
ComplexityEnabled = @{
Enable = $true
Name = 'Complexity Enabled'
Parameters = @{
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'
}
}
}
}

@PrzemyslawKlys PrzemyslawKlys added enhancement New feature or request question Further information is requested labels Sep 18, 2019
@PrzemyslawKlys
Copy link
Member

My thoughts are this:

  • The tests are there to quickly asses current settings (for example for MSP to quickly check AD they don't know) and get results
  • Everyone can modify these settings per their company policy and run the checks with changed parameters
  • While I agree with MS on the passwords it's not Black/White. A lot of countries/areas have their own forced rules that do not care what is best practice but care what the law says.
  • 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
  • I often set password history count higher like 24, but leave minimum password age to 0. If the user tries to do that trick with changing his password to the same password we will see 10-24 events of changing the password for 1 user within a short period of time which should be a red flag.
  • I would only consider disabling password expiration if MFA is in place - otherwise, you end up as I did with 1500 users with weak passwords (Pa$$sw0rd is still hard password that matches "complicated guidance")

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 #40 (comment). 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
Copy link
Contributor Author

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
Copy link
Member

PrzemyslawKlys commented Sep 18, 2019

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.

@PrzemyslawKlys PrzemyslawKlys removed the question Further information is requested label Oct 2, 2019
@SUBnet192
Copy link
Contributor

  • 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
Copy link
Contributor

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.

  • Passwords must be at least 8 characters in length if chosen by the subscriber.

  • Verifiers should not impose other composition rules (e.g., requiring mixtures of different character types or prohibiting consecutively repeated characters) for passwords.

  • Verifiers should not require passwords to be changed arbitrarily (e.g., periodically). However, verifiers shall force a change if there is evidence of compromise of the password.

  • Verifiers shall implement a rate-limiting mechanism that effectively limits the number of failed authentication attempts that can be made on the subscriber’s account.

  • Verifiers shall store passwords in a form that is resistant to offline attacks. Passwords shall be salted and hashed using a suitable one-way key derivation function. Key derivation functions take a password, a salt, and a cost factor as inputs then generate a password hash.

@PrzemyslawKlys
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member

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

@SUBnet192
Copy link
Contributor

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
Copy link
Member

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

@SUBnet192
Copy link
Contributor

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

@PrzemyslawKlys
Copy link
Member

I am unsure. Somehow it feels wrong :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants