invictus-ir / Microsoft-Extractor-Suite

A PowerShell module for acquisition of data from Microsoft 365 and Azure for Incident Response and Cyber Security purposes.
https://microsoft-365-extractor-suite.readthedocs.io/en/latest/
GNU General Public License v2.0
481 stars 68 forks source link

[Enhancement\Feature Request] - Alignment of code accross modules and Get-All #57

Closed angry-bender closed 4 months ago

angry-bender commented 8 months ago

Hey Team,

Just thought id raise an issue for longer term tracking.

I've noticed some of the scripts key functions are not consistent accross the available modules. It might be good to get a review in to ensure each module behaves the same for the following pieces i can think of off the top of my head:

  1. Authentication - GraphApi (When does a user consent? what flags are required, or do they do it before?). Has a user successfully signed into modules before using commands like search-unifiedauditlogs (Noting admins have to grant view-onlyauditlogs in both AzureAD and M365). Where they have it assigned in azure, but not M365, the search-unifiedauditlogs module may not be recognised
  2. Error Handling and checking - Some modules have error handling on the flags that are passed, some do not.
  3. File conventions - Some outputfiles have $date for de-duplication, some do not
  4. Fields - Might be an opportunity to collect fields in the same order between the GraphAPI modules and legacy Get-AzureAD modules

Once these are aligned, it would be awesome to have a get-all script that wraps together the collections (Perhaps with a legacy option for when a user cannot get Graph-API Access approved)

I note this is a big one, and i can try to tap away on it longer term, but unsure if i have the capacity for a bigger undertaking like this :-)

JoeyInvictus commented 8 months ago

Hi, thank you for the valuable feedback. This is indeed a big one, going require some time and effort. We can try to work on this and chip away at it over the longer term. Mainly, it's just me maintaining this project. So, it's sometimes hard to find time during my regular work. Since it doesn't bring in any money, it often gets less priority than other tasks. However, I will try my best to address the listed issues when I have some available time. Feel free to contribute as well, your pull requests have been great so far! :)

Regarding the points you've raised:

Authentication is indeed a complex issue, especially when dealing with multiple modules requiring authentication like Graph API, AzureAD, ExchangeOnline, and Azaccounts. Excluding Graph API, we attempt a quick check at the start to determine if we can execute the command or any command within the module. If this is not possible, we print a warning indicating that you must authenticate to that module before running the function. We've attempted more in-depth error handling, but encountered challenges, such as:

If you forget to connect to Connect-ExchangeOnline and then attempt to run search-UnifiedAuditLog, you will encounter the following error message:

search-UnifiedAuditLog: The term 'search-UnifiedAuditLog' is not recognized as the name of a cmdlet, function, script file, or operable program.

However, it's important to note that you may also encounter the same error if you have connected but are using an account that lacks the necessary permissions.

So, more in-depth handling of errors seems difficult to do. Therefore, we try to specify in the documentation which modules are required and give a warning when the script can't run the module.

For the Graph API, we attempt to connect with the required scopes for each function separately. We can create a connection script like with the others, but then we would need to import a lot of scopes, which might not be relevant for a user who only needs a subset of the functions. Thinking from the least privileged principle. What would be cool is to create some code that deploys an "acquisition" application in the target tenant with the required permissions and then use this application to acquire all relevant evidence. But this might also be a little overkill if you are just trying to get user info or MFA status.

On this page, we have an overview of each module and what roles/scopes are required: https://microsoft-365-extractor-suite.readthedocs.io/en/latest/installation/Prerequisites.html

It might be good to also include this information in the documentation for each step.

The error handling and checking suggestion is a good one and one we should look into. Over time, people requested new parameters and flags, so it got a little messy (my fault). I am currently testing error handling for the output directory on all functions we have. I will try to do it later for the rest of the flags as well.

For the file convention, I agree and think it would be good to add this to all files to keep it consistent.

For the fields around Graph API modules and the legacy Get-AzureAD modules, I will take a look and try to use the same order. This sounds like something logical to do, haha.

The "get-all" script is something we've been thinking about for a long time. It still feels like something we should create at some point. However, currently, new tasks are being added to the list quicker than I can process them. So, I might have to spend a weekend for this one in the future.

angry-bender commented 8 months ago

Hi, thank you for the valuable feedback. This is indeed a big one, going require some time and effort. We can try to work on this and chip away at it over the longer term. Mainly, it's just me maintaining this project. So, it's sometimes hard to find time during my regular work. Since it doesn't bring in any money, it often gets less priority than other tasks. However, I will try my best to address the listed issues when I have some available time. Feel free to contribute as well, your pull requests have been great so far! :)

Regarding the points you've raised:

Authentication is indeed a complex issue, especially when dealing with multiple modules requiring authentication like Graph API, AzureAD, ExchangeOnline, and Azaccounts. Excluding Graph API, we attempt a quick check at the start to determine if we can execute the command or any command within the module. If this is not possible, we print a warning indicating that you must authenticate to that module before running the function. We've attempted more in-depth error handling, but encountered challenges, such as:

If you forget to connect to Connect-ExchangeOnline and then attempt to run search-UnifiedAuditLog, you will encounter the following error message:

search-UnifiedAuditLog: The term 'search-UnifiedAuditLog' is not recognized as the name of a cmdlet, function, script file, or operable program.

However, it's important to note that you may also encounter the same error if you have connected but are using an account that lacks the necessary permissions.

So, more in-depth handling of errors seems difficult to do. Therefore, we try to specify in the documentation which modules are required and give a warning when the script can't run the module.

For the Graph API, we attempt to connect with the required scopes for each function separately. We can create a connection script like with the others, but then we would need to import a lot of scopes, which might not be relevant for a user who only needs a subset of the functions. Thinking from the least privileged principle. What would be cool is to create some code that deploys an "acquisition" application in the target tenant with the required permissions and then use this application to acquire all relevant evidence. But this might also be a little overkill if you are just trying to get user info or MFA status.

On this page, we have an overview of each module and what roles/scopes are required: https://microsoft-365-extractor-suite.readthedocs.io/en/latest/installation/Prerequisites.html

It might be good to also include this information in the documentation for each step.

The error handling and checking suggestion is a good one and one we should look into. Over time, people requested new parameters and flags, so it got a little messy (my fault). I am currently testing error handling for the output directory on all functions we have. I will try to do it later for the rest of the flags as well.

For the file convention, I agree and think it would be good to add this to all files to keep it consistent.

For the fields around Graph API modules and the legacy Get-AzureAD modules, I will take a look and try to use the same order. This sounds like something logical to do, haha.

The "get-all" script is something we've been thinking about for a long time. It still feels like something we should create at some point. However, currently, new tasks are being added to the list quicker than I can process them. So, I might have to spend a weekend for this one in the future.

Thanks Joey,

Very similar scenario here, where I can see a need / feature requirement though, happy to help out where possible. Found one such opportunity today where a bigger tenancy crashed. But completely understand where your coming from on maintenance being a fellow DFIR practitioner.

JoeyInvictus commented 8 months ago

In our recent update, we aligned some code across the modules:

JoeyInvictus commented 4 months ago

Hi,

We just released the update V2.0.0. I will close this issue for now, but please feel free to reopen it at any time or reach out with any suggestions or feedback.

The Get-all option is still on my Todo...🫣