Azure / Enterprise-Scale

The Azure Landing Zones (Enterprise-Scale) architecture provides prescriptive guidance coupled with Azure best practices, and it follows design principles across the critical design areas for organizations to define their Azure architecture
https://aka.ms/alz
MIT License
1.69k stars 963 forks source link

Deploy-ASC-SecurityContacts is always non-compliantBug Report #1627

Closed NucLabs closed 3 months ago

NucLabs commented 5 months ago

Describe the bug It appears to me that the same problem as #1477 is back. Not only Deploy-ASC-SecurityContacts isn't evaluating correctly anymore: the same problem seems to be the case with the builtin policy Email notification for high severity alerts should be enabled (6e2593d9-add6-4083-9c9b-4b7d2188c899) that is non-compliant in all our subscriptions.

Steps to reproduce

  1. Assign the policy definition Deploy-ASC-SecurityContacts to a subscription and define the Security contacts email address, Effect and Minimal severity (High) parameters
  2. Wait for evaluation
  3. Remediate, if necesarry
  4. Wait for evaluation, the resource (subscription will be non-compliant. The reason reported is that Microsoft.Security/securityContacts/alertNotifications.minimalSeverity has no value while the Target value is "High"

Compare the screenshot below with the output of the api where minimalSeverity is set to High:

(Invoke-AzRestMethod -Method 'Get' -Path ('/subscriptions/9fa****************/providers/Microsoft.Security/securityContacts?api-version=2020-01-01-preview')).Content|ConvertFrom-Json -Depth 10|ConvertTo-Json -Depth 10
{
  "properties": {
    "alertNotifications": {
      "state": "On",
      "minimalSeverity": "High"
    },
    "notificationsByRole": {
      "state": "On",
      "roles": [
        "Owner"
      ]
    },
    "emails": "(redacted)",
    "phone": ""
  },
  "id": "/subscriptions/9fa**********/providers/Microsoft.Security/securityContacts/default",
  "name": "default",
  "type": "Microsoft.Security/securityContacts",
  "etag": "\"4300***************\"",
  "location": "West Europe"
}

Screenshots image

Springstone commented 5 months ago

@NucLabs I can confirm the same from my side. Can I ask that you open a support ticket for the built-in policy you mentioned above, as it involves the same logic that is failing.

Essentially:

Microsoft.Security/securityContacts/alertNotifications.minimalSeverity

is not evaluating properly in policy. We'll investigate further, and try find someone in PG to help sort this out, but opening a support ticket might be a quicker path.

NucLabs commented 5 months ago

Support ticket #2404260050000291

donk-msft commented 5 months ago

@NucLabs I can confirm the same from my side as well.

Springstone commented 5 months ago

Just to update all on this issue. I've opened an internal support ticket for the same. It is a confirmed issue that has been escalated to the Defender team, as that is where the issue is (due to API changes affecting properties we are querying). Nothing we can do, but will keep this issue open for tracking.

itzkhayer commented 4 months ago

Same issue I have had for the past 2 weeks, got MS ticket opened and engineer provided me a script where it does show compliance but it doesn't actually do the job of deploying the security contact email. whats the point of that lol.

here the json script incase it helps anyone to figure it out

{ "mode": "All", "policyRule": { "if": { "allOf": [ { "field": "type", "equals": "Microsoft.Resources/subscriptions" } ] }, "then": { "effect": "[parameters('effect')]", "details": { "type": "Microsoft.Security/securityContacts", "deploymentScope": "subscription", "existenceScope": "subscription", "roleDefinitionIds": [ "/providers/Microsoft.Authorization/roleDefinitions/fb1c8493-542b-48eb-b624-b4c8fea62acd" ], "existenceCondition": { "not": { "allOf": [ { "field": "Microsoft.Security/securityContacts/alertsToAdmins", "equals": "Off" }, { "field": "Microsoft.Security/securityContacts/email", "notcontains": "[parameters('emailSecurityContact')]" }, { "anyOf": [ { "field": "Microsoft.Security/securityContacts/alertNotifications.minimalSeverity", "equals": "High" }, { "count": { "field": "Microsoft.Security/securityContacts/notificationsSources[]", "where": { "allOf": [ { "field": "Microsoft.Security/securityContacts/notificationsSources[].sourceType", "equals": "Alert" }, { "field": "Microsoft.Security/securityContacts/notificationsSources[*].Alert.minimalSeverity", "equals": "[parameters('minimalSeverity')]" } ] } }, "equals": 1 } ] } ] } }, "deployment": { "location": "northeurope", "properties": { "mode": "incremental", "parameters": { "emailSecurityContact": { "value": "[parameters('emailSecurityContact')]" }, "minimalSeverity": { "value": "[parameters('minimalSeverity')]" } }, "template": { "$schema": "https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#", "contentVersion": "1.0.0.0", "parameters": { "emailSecurityContact": { "type": "string", "metadata": { "description": "Security contacts email address" } }, "minimalSeverity": { "type": "string", "metadata": { "description": "Minimal severity level reported" } } }, "variables": {}, "resources": [ { "type": "Microsoft.Security/securityContacts", "name": "default", "apiVersion": "2020-01-01-preview", "properties": { "emails": "[parameters('emailSecurityContact')]", "notificationsByRole": { "state": "On", "roles": [ "Owner" ] }, "alertNotifications": { "state": "On", "minimalSeverity": "[parameters('minimalSeverity')]" } } } ], "outputs": {} } } } } } }, "parameters": { "emailSecurityContact": { "type": "String", "metadata": { "displayName": "Security contacts email address", "description": "Provide email address for Azure Security Center contact details" } }, "effect": { "type": "String", "metadata": { "displayName": "Effect", "description": "Enable or disable the execution of the policy" }, "allowedValues": [ "DeployIfNotExists", "Disabled" ], "defaultValue": "DeployIfNotExists" }, "minimalSeverity": { "type": "String", "metadata": { "displayName": "Minimal severity", "description": "Defines the minimal alert severity which will be sent as email notifications" }, "allowedValues": [ "High", "Medium", "Low" ], "defaultValue": "High" } } }

itzkhayer commented 4 months ago

I fixed it myself!!! I changed environment setting email and alert purposely.. then deleted policy. remade new policy with my own script and it deployed email and alert and compliance status approved!!

{ "mode": "All", "policyRule": { "if": { "allOf": [ { "field": "type", "equals": "Microsoft.Resources/subscriptions" } ] }, "then": { "effect": "[parameters('effect')]", "details": { "type": "Microsoft.Security/securityContacts", "deploymentScope": "subscription", "existenceScope": "subscription", "roleDefinitionIds": [ "/providers/Microsoft.Authorization/roleDefinitions/fb1c8493-542b-48eb-b624-b4c8fea62acd" ], "existenceCondition": { "allOf": [ { "field": "Microsoft.Security/securityContacts/alertsToAdmins", "equals": "On" }, { "field": "Microsoft.Security/securityContacts/email", "contains": "[parameters('emailSecurityContact')]" }, { "anyOf": [ { "field": "Microsoft.Security/securityContacts/alertNotifications.minimalSeverity", "equals": "High" }, { "count": { "field": "Microsoft.Security/securityContacts/notificationsSources[]", "where": { "allOf": [ { "field": "Microsoft.Security/securityContacts/notificationsSources[].sourceType", "equals": "Alert" }, { "field": "Microsoft.Security/securityContacts/notificationsSources[*].Alert.minimalSeverity", "equals": "[parameters('minimalSeverity')]" } ] } }, "equals": 1 } ] } ] }, "deployment": { "location": "northeurope", "properties": { "mode": "incremental", "parameters": { "emailSecurityContact": { "value": "[parameters('emailSecurityContact')]" }, "minimalSeverity": { "value": "[parameters('minimalSeverity')]" } }, "template": { "$schema": "https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#", "contentVersion": "1.0.0.0", "parameters": { "emailSecurityContact": { "type": "string", "metadata": { "description": "Security contacts email address" } }, "minimalSeverity": { "type": "string", "metadata": { "description": "Minimal severity level reported" } } }, "variables": {}, "resources": [ { "type": "Microsoft.Security/securityContacts", "name": "default", "apiVersion": "2020-01-01-preview", "properties": { "emails": "[parameters('emailSecurityContact')]", "notificationsByRole": { "state": "On", "roles": [ "Owner" ] }, "alertNotifications": { "state": "On", "minimalSeverity": "[parameters('minimalSeverity')]" } } } ], "outputs": {} } } } } } }, "parameters": { "emailSecurityContact": { "type": "String", "metadata": { "displayName": "Security contacts email address", "description": "Provide email address for Azure Security Center contact details" } }, "effect": { "type": "String", "metadata": { "displayName": "Effect", "description": "Enable or disable the execution of the policy" }, "allowedValues": [ "DeployIfNotExists", "Disabled" ], "defaultValue": "DeployIfNotExists" }, "minimalSeverity": { "type": "String", "metadata": { "displayName": "Minimal severity", "description": "Defines the minimal alert severity which will be sent as email notifications" }, "allowedValues": [ "High", "Medium", "Low" ], "defaultValue": "High" } } }

itzkhayer commented 4 months ago

I might try work on and get this setting turned on ''Notify about attack paths with the following risk level (or higher):''

itzkhayer commented 4 months ago

Any news on when ''Email notification for high severity alerts should be enabled'' audit will be solved? I could create custom and solve it but would rather have this auto solved as it falls in CIS initiative

Springstone commented 4 months ago

Any news on when ''Email notification for high severity alerts should be enabled'' audit will be solved? I could create custom and solve it but would rather have this auto solved as it falls in CIS initiative

According to PG, they have published an updated version of that policy that should always work. If it doesn't, PLEASE open a support ticket!!

NucLabs commented 4 months ago

@Springstone From my holiday address in Italy I can confirm that the builtin policy is fixed indeed.

neok-g commented 4 months ago

@Springstone I can confirm that the built-in policy Email notification for high severity alerts should be enabled (6e2593d9-add6-4083-9c9b-4b7d2188c899) is now compliant. However the Enterprise Scale custom policy Deploy Microsoft Defender for Cloud Security Contacts (Deploy-ASC-SecurityContacts) is still non-compliant.

Springstone commented 3 months ago

Hi all, this is correct. Looks like PG went and made some fundamental changes to the API, and published a new version, deprecating "alertNotifications" and replacing it with "NotificationSources" as per the latest API documentation here: https://learn.microsoft.com/en-us/rest/api/defenderforcloud/security-contacts/create?view=rest-defenderforcloud-2023-12-01-preview&tabs=HTTP

Now that we've completed the policy refresh, we'll address the issue for our custom policy asap.

Springstone commented 3 months ago

@neok-g @NucLabs @itzkhayer @donk-msft I've worked on updating this and ask you to please confirm my positive results. Latest version of the policy is here in this PR : https://github.com/Azure/Enterprise-Scale/pull/1663

donk-msft commented 3 months ago

@Springstone I left some comments in the PR, but once those issues are resolved the policy is working for me.

Springstone commented 3 months ago

@donk-msft can't find your comments in the PR, could you share here or on Teams :D

donk-msft commented 3 months ago

@Springstone that's strange... My comments are:

Springstone commented 3 months ago

For 126, well spotted! The [[ is required due to how we package the policies into a single ARM template, so its by design. Many thanks for your input!

NucLabs commented 3 months ago

@Springstone I copy/pasted the code into the policy editor the portal. The editor complained about a few parameter types that are defined als lowercase "string", but should be pascal-cased "String"

"minimalSeverity": { "type": "string",

"minimalSeverity": { "type": "String",

I also had to fix the double [['s, I already found out one (long) day that it is some kind of escape sequence :-)

Despite the 'isEnable' issue spotted by @donk-msft the evaluation and remediation are working OK for me

Springstone commented 3 months ago

Fixed in PR #1663

neok-g commented 3 months ago

Great! When will these changes be synced to ALZ-bicep repo?

Springstone commented 3 months ago

@neok-g should be arriving sometime next week.