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

feat(azure): Add check defender_auto_provisioning_log_analytics_agent_vms_on #3322

Merged

Conversation

puchy22
Copy link
Contributor

@puchy22 puchy22 commented Jan 26, 2024

Context

Add new check to ensure Auto Provisioning Log Analytics in VMs is On

Description

Add the new check for Defender service with its respective unit tests

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@puchy22 puchy22 requested a review from a team as a code owner January 26, 2024 09:47
Copy link
Member

@jfagoagas jfagoagas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's really good work @puchy22 👏

I left some comments just to clarify some parts, but the overall is great!

"CLI": "https://www.trendmicro.com/cloudoneconformity-staging/knowledge-base/azure/SecurityCenter/automatic-provisioning-of-monitoring-agent.html",
"NativeIaC": "",
"Other": "",
"Terraform": "https://docs.bridgecrew.io/docs/ensure-that-azure-defender-is-set-to-on-for-app-service#terraform"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this right?

Comment on lines 60 to 62
logger.error(f"Subscription name: {subscription}")
logger.error(
f"{error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
logger.error(f"Subscription name: {subscription}")
logger.error(
f"{error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
logger.error(
f"Subscription name: {subscription} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"

report = Check_Report_Azure(self.metadata())
report.status = "PASS"
report.subscription = subscription
report.resource_name = "Defender Auto Provisioning Log Analytics Agents On"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resource_name maybe should be the auto_provisioning_settings.name, what do you think?


from prowler.providers.azure.services.defender.defender_service import AutoProvisioningSetting

AZURE_SUSCRIPTION = str(uuid4())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use this value from the tests/prowler/providers/azure/azure_fixtures.py. Pedro included that in the previous PR.

puchy22 and others added 6 commits January 26, 2024 11:12
…ioning_log_analytics_agent_vms_on/defender_auto_provisioning_log_analytics_agent_vms_on.metadata.json

Co-authored-by: Pepe Fagoaga <pepe@verica.io>
…ioning_log_analytics_agent_vms_on/defender_auto_provisioning_log_analytics_agent_vms_on.metadata.json

Co-authored-by: Pepe Fagoaga <pepe@verica.io>
…ioning_log_analytics_agent_vms_on/defender_auto_provisioning_log_analytics_agent_vms_on.py

Co-authored-by: Pepe Fagoaga <pepe@verica.io>
…ioning_log_analytics_agent_vms_on/defender_auto_provisioning_log_analytics_agent_vms_on.py

Co-authored-by: Pepe Fagoaga <pepe@verica.io>
…prowler into azure-checks-microsoft-defender
@jfagoagas jfagoagas added the provider/azure Issues/PRs related with the Azure provider label Jan 26, 2024
@jfagoagas jfagoagas changed the title feat(azure): Add new check "defender_auto_provisioning_log_analytics_agent_vms_on" feat(azure): Add check "defender_auto_provisioning_log_analytics_agent_vms_on" Jan 26, 2024
@jfagoagas jfagoagas self-requested a review January 26, 2024 13:43
Copy link
Member

@jfagoagas jfagoagas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not sure if we should include a condition in the check just to only audit the default resource because some clients can have different or custom things in Azure and in that case we will generate findings maybe not having to. My vote is to include that filter for now to be 100% sure.

Also this check is going to be deprecated in August 2024 per https://learn.microsoft.com/en-us/azure/azure-monitor/agents/log-analytics-agent

Let's talk with @sergargar on Monday.

report = Check_Report_Azure(self.metadata())
report.status = "PASS"
report.subscription = subscription_name
report.resource_name = auto_provisioning_settings["default"].resource_name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not sure if we should include here a condition just to review the default resource. Let's talk with @sergargar on Monday.

"RelatedUrl": "https://www.trendmicro.com/cloudoneconformity-staging/knowledge-base/azure/SecurityCenter/automatic-provisioning-of-monitoring-agent.html",
"Remediation": {
"Code": {
"CLI": "https://www.trendmicro.com/cloudoneconformity-staging/knowledge-base/azure/SecurityCenter/automatic-provisioning-of-monitoring-agent.html",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put this link in "Other", please

@sergargar sergargar self-requested a review January 29, 2024 09:30
@sergargar sergargar changed the title feat(azure): Add check "defender_auto_provisioning_log_analytics_agent_vms_on" feat(azure): Add check defender_auto_provisioning_log_analytics_agent_vms_on Jan 29, 2024
sergargar
sergargar previously approved these changes Jan 29, 2024
Copy link
Member

@sergargar sergargar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome job!! 🚀

@sergargar sergargar self-requested a review January 29, 2024 10:02
@sergargar sergargar merged commit a8edd03 into prowler-cloud:master Jan 29, 2024
6 checks passed
@puchy22 puchy22 deleted the azure-checks-microsoft-defender branch January 31, 2024 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
provider/azure Issues/PRs related with the Azure provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants