formal internal review of AAD baseline

[tkol2022]

Scope

This PR is designed to facilitate a formal review of the revised AAD baseline from CISA Scuba principals and the Scuba development team. Following this review the AAD baseline should be in a state for sending to the CISA agency formal review process.

Note

We have active development issues against this baseline at the moment, including new policy implementations and tweaks to existing policies. If any changes occur during this baseline review which materially affect the ScubaGear code, it may require a notification to the affected developers.

✅ Pre-approval checklist

• This PR has an informative and human-readable title.
• Changes are limited to a single goal - eschew scope creep!
• All future TODOs are captured in issues, which are referenced
  in code comments.
• All relevant type-of-change labels have been added.
• All relevant repo and/or project documentation has been updated
  to reflect the changes in this PR.

✅ Pre-merge checklist

• Revert dependencies to default branches.
• Finalize version.

[gdasher]

I reviewed the content of the AAD baseline on the emerald branch. It is approved from my point of view with these changes:

• MS.AAD.7.* controls: Clarify that Microsoft AAD PIM qualifies as a pam system and that a non-PIM PAM may be used if it meets the same set of requirements. Terminology from PIM is sprinkled throughout these controls (e.g. permanent active), implying that PIM must be used. Overall, my suggestion is to be explicit that PIM terminology is used through and that another PAM system may be used assuming it meets the requirements, but also that PIM is acceptable to meet the pam requirement in MS.AAD.7.5v1
• (nit) MS.AAD.4.1v1: please used fixed width font for the log types so that they stand out from the surrounding text

[tkol2022]

@nanda-katikaneni @schrolla @ssatyapal123 Kindly let me know if you have any review comments. thx.

[schrolla]

No technical issues or findings with the policies themselves, mostly recommendations to bring the baseline document in line with the style guide or conventions used in other baselines OR clarify the intent of the policies.

[tkol2022]

I reviewed the content of the AAD baseline on the emerald branch. It is approved from my point of view with these changes:

• MS.AAD.7. controls*: Clarify that Microsoft AAD PIM qualifies as a pam system and that a non-PIM PAM may be used if it meets the same set of requirements. Terminology from PIM is sprinkled throughout these controls (e.g. permanent active), implying that PIM must be used. Overall, my suggestion is to be explicit that PIM terminology is used through and that another PAM system may be used assuming it meets the requirements, but also that PIM is acceptable to meet the pam requirement in MS.AAD.7.5v1
• (nit) MS.AAD.4.1v1: please used fixed width font for the log types so that they stand out from the surrounding text

Added the back ticks so that the logs stand out.

[tkol2022]

Per discussion at stand-up, removed the policy bullets from Appendix A. Left the reference to the Hybrid Identity Solutions document in tact and fixed the URL since it was wrong.

[tkol2022]

@gdasher @schrolla @nanda-katikaneni Made the requested revisions. Take a look if you like and kindly perform the GitHub approval if you are satisfied. Thanks for the diligent review.

[gdasher]

I skimmed through one more time. Looks good with one small nit.

[ssatyapal123]

Thanks for the opportunity to review. It looks great.
Small suggestion in Policy
MS.AAD.2.1v1
Users detected as high risk SHALL be blocked
Rationale: By blocking users determined as high risk, compromised accounts are prevented from accessing the tenant.

Suggest changing Rationale to:
Rationale: By blocking users determined as high risk, compromised accounts can be prevented from accessing the tenant.

[tkol2022]

Thanks for the opportunity to review. It looks great. Small suggestion in Policy MS.AAD.2.1v1 Users detected as high risk SHALL be blocked Rationale: By blocking users determined as high risk, compromised accounts are prevented from accessing the tenant.

Suggest changing Rationale to: Rationale: By blocking users determined as high risk, compromised accounts can be prevented from accessing the tenant.

Fixed.

[schrolla]

Resolved all of my remaining issues and a quick look shows the baseline is in good shape and looks ready for merge pending resolution of a few minor comments from others.

[nanda-katikaneni]

Looks good to me.