Azure / bicep-registry-modules

Bicep registry modules
MIT License
509 stars 355 forks source link

Set default values to a secure value - Logic Apps #2428

Open elanzel opened 2 years ago

elanzel commented 2 years ago

All default values should comply with a security baseline, e.g. NIST 800

The build-in policies of Azure can be used as a reference.

The task would be to scan over each of the following policies and make sure, that the module is per default complying to them.

The following policies are the NIST 800 ones: \built-in-policies\policyDefinitions\Logic Apps\LogicApps_AuditDiagnosticLog_Audit.json \built-in-policies\policyDefinitions\Logic Apps\LogicApps_ISEWithCustomerManagedKey_AuditDeny.json \built-in-policies\policyDefinitions\Azure Government\Logic Apps\LogicApps_ISEWithCustomerManagedKey_AuditDeny.json

microsoft-github-policy-service[bot] commented 5 months ago

[!IMPORTANT] The "Needs: Triage :mag:" label must be removed once the triage process is complete!

[!TIP] For additional guidance on how to triage this issue/PR, see the BRM Issue Triage documentation.

AlexanderSehr commented 5 months ago

Hey @lsnoddy , I just migrated this issue over from CARML. Please take a look and triage if still relevant :)

microsoft-github-policy-service[bot] commented 5 months ago

[!WARNING] Tagging the AVM Core Team (@Azure/avm-core-team-technical-bicep) due to a module owner or contributor having not responded to this issue within 3 business days. The AVM Core Team will attempt to contact the module owners/contributors directly.

[!TIP]

  • To prevent further actions to take effect, the "Status: Response Overdue 🚩" label must be removed, once this issue has been responded to.
  • To avoid this rule being (re)triggered, the ""Needs: Triage :mag:" label must be removed as part of the triage process (when the issue is first responded to)!
lsnoddy commented 5 months ago

Will investigate.

lsnoddy commented 2 days ago

https://github.com/Azure/azure-policy/blob/master/built-in-policies/policyDefinitions/Logic%20Apps/AuditDiagnosticLog_Audit.json only applies if diagnostic settings are provided. Diagnostic settings are optional.

https://github.com/Azure/azure-policy/blob/master/built-in-policies/policyDefinitions/Logic%20Apps/ISEWithCustomerManagedKey_AuditDeny.json only applies if an integration service environment is provided. ISE is optional.

@AlexanderSehr , a default deployment of this module is non-compliant with this policy: https://github.com/Azure/azure-policy/blob/master/built-in-policies/policyDefinitions/Logic%20Apps/LogicAppsInISE_AuditDeny.json

However, I'm not sure if the module should be changed to require the ISE parameter. Can I get your opinion on this? Are we trying to enforce compliance with NIST 800 related policies by default?

AlexanderSehr commented 19 hours ago

Hey @lsnoddy, no in that case we're good. The WAF-aligned test should likely consider deploying an ISE & private networking and a Logic App into the same. However, as we're talking about an extra resource deployment here, I don't think the Logic App module itself should enforce nor deploy an ISE itself. The Set default values to a secure value effort should focus primarily on default values for parameters the module provides to its users to enable/disable specific settings.

That being said - if there's nothing to set here that does not require an extra user input, I'd consider this issue resolved.