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

MS.AAD.6.1 passes when at least one custom domain is configured correctly #594

Open
jed-exotic opened this issue Dec 18, 2024 · 6 comments

Comments

@jed-exotic
Copy link

jed-exotic commented Dec 18, 2024

The scenario that I am encountering involves multiple custom domains in an Entra ID tenant. One of which was unverified, the other two verified. Only one of the two verified domains is configured correctly.

Upon reviewing the results, the test appears to have passed. Inspecting the Test-MtCisaPasswordExpiration.ps1 revealed to me the reason behind this:

https://github.com/maester365/maester/blob/main/powershell/public/cisa/entra/Test-MtCisaPasswordExpiration.ps1#L40-L50

The code implies that if a single custom domain is configured correctly, the behavior of the test is to pass.

What is the rationale in only evaluating compliance for one of the results that match the test? I would be more comfortable seeing a warning here, or even failure.

The National Institute of Standards and Technology (NIST), OMB, and Microsoft have published guidance indicating mandated periodic password changes make user accounts less secure. For example, OMB-22-09 states, "Password policies must not require use of special characters or regular rotation."

Nothing about this seems to imply checking only one of the results from Get-MgDomain

@jed-exotic
Copy link
Author

I see in the comments above (on the latest commit):
https://github.com/maester365/maester/blob/main/powershell/public/cisa/entra/Test-MtCisaPasswordExpiration.ps1#L11

Returns true if at least 1 domain has password expiration of 100 years or greater

So this appears to be the expected behavior/implementation, however I would like to understand the justification for not verifying all domains.

@jed-exotic
Copy link
Author

jed-exotic commented Dec 26, 2024

This can be adjusted to be a stronger test by modifying the $testResult accordingly:

$testResult = ($result | Measure-Object).Count - ($managedDomains|Measure-Object).Count -eq 0

@jed-exotic
Copy link
Author

Alternatively, if the intention is to only check the Primary/Default domain we may disregard the previous suggestion and instead supply an additional condition here:

    $managedDomains = $result | Where-Object {`
        $_.authenticationType -eq "Managed" -and `
        $_.PasswordValidityPeriodInDays -ge 36500 -and `
        $_.isDefault
    }

@merill
Copy link
Contributor

merill commented Jan 7, 2025

@jed-exotic thanks for the discussion and the PR, I believe the implementation is based on SCUBA Gear but I agree that we should check all managed domains are not doing password expiry to have this rule pass.

In the PR you are filtering out managed domains that are not verified, I believe they should be included in the check too (e.g. there could be a tenant with no verified domains).

Are you okay to update the PR?

@soulemike Let us know if you think otherwise.

@jed-exotic
Copy link
Author

In the PR you are filtering out managed domains that are not verified, I believe they should be included in the check too (e.g. there could be a tenant with no verified domains).

I'm not super familiar on the topic of domain verification in Entra, but I don't believe you can configure password expiry on an unverified domain. If that's a possible configuration, then I can incorporate the suggestion.

@soulemike
Copy link
Contributor

soulemike commented Jan 7, 2025

I agree more details here would be good. Likely returning each domain in a table with the configuration and pass state, similar to other tests. Something like the below. With the test result validating all verified managed and federated domains are valid.

Domain (Default) Verified Type Validation
contoso.com (✔️) Verified Managed ✅ Pass
sub.contoso.com () Verified Managed ❌ Fail
contoso.onmicrosoft.com () Verified Managed ✅ Pass
oldbrand.com () Verified Federated 🗄️ Skipped
google.com () Unverified Managed 🗄️ Skipped

Reviewing the CISA REGO it looks like they do something similar and measure the aggregate, so just looks like something I missed when wiring this up. https://github.com/cisagov/ScubaGear/blob/main/PowerShell/ScubaGear/Rego/AADConfig.rego#L681

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

No branches or pull requests

3 participants