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

azurerm_security_center_subscription_pricing auto-enables OnUploadMalwareScanning extension with DefenderForStorageV2 subplan #27338

Open
1 task done
PeterBennink opened this issue Sep 10, 2024 · 8 comments · May be fixed by #27805

Comments

@PeterBennink
Copy link

PeterBennink commented Sep 10, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment and review the contribution guide to help.

Terraform Version

1.5.7

AzureRM Provider Version

3.114.0

Affected Resource(s)/Data Source(s)

azurerm_security_center_subscription_pricing

Terraform Configuration Files

resource "azurerm_security_center_subscription_pricing" "ms_defender" {
  tier          = "Standard"
  resource_type = "StorageAccounts"
  subplan       = "DefenderForStorageV2"

  extension {
    name = "SensitiveDataDiscovery"
  }
}

Debug Output/Panic Output

n/a, they apply succeeds

Expected Behaviour

I would expect my subscription to be set to the new Defender for Storage plan, with SensitiveDataDiscovery enabled

Actual Behaviour

My subscription gets set to the new Defender for Storage plan, with SensitiveDataDiscovery and OnUploadMalwareScanning enabled. I don't want to use this feature, but I don't really have a way to disable it, because I can't explicitly disable it in Terraform, if I add it as an extension block it will also get enabled, at that point I can only tweak CapGBPerMonthPerStorageAccount.

Steps to Reproduce

Nothing to add to above information.

Important Factoids

No response

References

https://registry.terraform.io/providers/hashicorp/azurerm/3.114.0/docs/resources/security_center_subscription_pricing

@neil-yechenwei
Copy link
Contributor

Thanks for raising this issue. TF doesn't set OnUploadMalwareScanning in the request payload. Seems service API would automatically set it when creating this kind of security center subscription pricing. Suggest filing Azure Support Ticket for this issue.

@phil-holden
Copy link

@neil-yechenwei, is this the API call you're referring to? https://learn.microsoft.com/en-us/azure/defender-for-cloud/defender-for-storage-rest-api-enablement?tabs=enable-subscription

If yes, the request payload needs to explicitly set the extensions enabled property to False otherwise the API will enable the extension.

{
  "properties": {
    "pricingTier": "Standard",
    "subPlan": "DefenderForStorageV2",
    "extensions": [
      {
        "name": "OnUploadMalwareScanning",
        "isEnabled": "False"
      }
    ]
  }
}

I agree this seems to be an issue with the defaults set by the Azure API, but is there something that could be done at the provider level to allow the enabled flag to be set on extensions?

@phil-holden
Copy link

phil-holden commented Oct 6, 2024

To follow up with the response from Microsoft...

=====
Would like to clarify that if no extensions are specified in your API request, both the OnUploadMalwareScanning and SensitiveDataDiscovery extensions are enabled by default. This is designed to ensure that key security features are active unless explicitly disabled.

To disable these extensions, you need to include them in your API request with isEnabled set to False.

=====

@PeterBennink
Copy link
Author

So to summarise the possible ways of solving this, either the API endpoint needs to be changed so that these extensions don't get enabled by default (opt-in vs opt-out, effectively), or the API call Terraform needs to by default include these two extensions with isEnabled set to false. Unless I'm mistaken at least, I don't think this can be changed by just changing the Terraform resource as I defined it in the original issue.

It doesn't seem like Microsoft has any intention of changing the API endpoint, their message indicates that this was done by design. So can the TF provider then be changed so it adds these extensions (with isEnabled=false) to the request by default?

@WodansSon
Copy link
Collaborator

@rcskosir, I do not believe this is a bug as it is by design, to be secure by default. @PeterBennink, as I have just previously stated, Terraform should not be compromising the secure by default initiative by changing the way the API currently works. That said, Terraform should explicitly expose a way for end-users to override the default behavior.

@bubbletroubles
Copy link
Contributor

bubbletroubles commented Dec 12, 2024

I think the issue here is that something is auto-enabled, which incurs customer cost (USD$10/storage account/month). Storage accounts are used in many different ways, and there is no need to auto-enable the OnUploadMalwareScanning capability on all accounts. Sure, if you are receiving content from end users and want to scan it as part of your file upload, sure that makes sense. If you are using it for other purposes (e.g. storing your Terraform State) it may not be required.

There are other examples where Terraform AzureRM provider is not secure by default, e.g. see AzureRM Windows Web App where HTTPS defaults to false.

image

Lastly, this doesn't prevent malicious content in storage accounts. It simply scans it, and if its malicious, gives it metadata to say its malicious. You can still download or interact with the file in the blob. Given this behavior, you need to build in logic to look at the metadata and based on that, work out how to handle the file. Simply enabling the OnUploadMalwareScanning without the corresponding remediation (as documented here), its useless - a false sense of security.

image

@WodansSon
Copy link
Collaborator

WodansSon commented Dec 13, 2024

@bubbletroubles, All of these unsecure values will be updated shortly to be secure by default as we move forward. I also understand that other logic/tooling needs to be implemented to act on the tags, however that said, the value of these fields are, per the service team, set to true by default which is by design. Terraform should not be overriding the default behavior of the API, rather, it should just expose a way to override the default behavior by the end user by explicitly disabling in in the configuration file. As for the customer cost, that should be added in the documentation alerting the customer of that additional cost and explain how to disable it. I think that bit can be fixed with documentation.

@phil-holden
Copy link

phil-holden commented Dec 13, 2024

I think the behavior needs to be clarified in the docs;

https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/security_center_subscription_pricing#extension-1

NOTE:
If an extension is not defined, it will not be enabled.

If we define a DefenderForStorageV2 pricing plan with no extensions defined, the OnUploadMalwareScanning extension gets enabled. However, a second apply of the same configuration disables the extension, so OnUploadMalwareScanning is only enabled when the DefenderForStorageV2 sub plan is first configured.

The MS docs make a similar point that removing the extension block will disable the extension.

https://learn.microsoft.com/en-us/azure/defender-for-cloud/defender-for-storage-infrastructure-as-code-enablement?tabs=enable-subscription#terraform-template

Disabling features:

If you want to turn off the on-upload malware scanning or sensitive data threat detection features, you can remove the corresponding extension block from the Terraform code.

We've moved the management of our pricing plans to the azapi provider which gives us the control we want/need over the extension config, with an explicit enabled property for each extension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants