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

Enabling Malware scanning #3768

Closed
migldasilva opened this issue Oct 27, 2023 · 7 comments · Fixed by #3784
Closed

Enabling Malware scanning #3768

migldasilva opened this issue Oct 27, 2023 · 7 comments · Fixed by #3784
Assignees
Labels
question Further information is requested

Comments

@migldasilva
Copy link
Contributor

Description

Hi, If I'm not wrong, malware scanning in AzureTRE relies on on-upload malware scanning, by Microsoft Defender for Storage. This feature must be enabled in AzureTRE, and also in Azure.

During my tests, if on-upload malware scanning is not enable, the import request gets stuck. I mean, a TRE Researcher creates a request, uploads a file, but giving that no malware scanning is enabled on Azure side, the Airlock processor never gets a message saying if the file is OK or not.

Enabling Microsoft Defender for Storage my require a Defender for Storage plan upgrade. Such upgrade can be done subscription-wide, or per storage account. In both cases, it might be done through Azure Portal, Bicep or ARM.

Using Terraform, it'd be possible to enable on-upload malware scanning per Storage Account defining an azurerm_template_deployment object. One candidate storage account for such changes is stalimex<TRE_ENV_NAME>.

Are you aware of how other users are handling this situation? Is there any interested in changing Terraform so that on-upload malware scanning would be enabled for some given storage accounts?

Steps

The steps I have tried are:

  1. Enable Malware scanning on Azure TRE
  2. Enable on-upload malware scanning on the portal, for storage account stalimex<TRE_ENV_NAME>
  3. Create an import request
@migldasilva migldasilva added the question Further information is requested label Oct 27, 2023
@marrobi
Copy link
Member

marrobi commented Oct 27, 2023

We have found another import issue here - #3767

This will likely mean that 50% of airlock import requests fail as there are two IPs for the DNS entry.

"During my tests, if on-upload malware scanning is not enable, the import request gets stuck." Can you confirm you aren't seeing this issue?

If it is not enabled, it should not get stuck.

I've not got much experience with the malware scanning, but @tamirkamara @yuvalyaron @eladiw @anatbal might have some ideas.

@migldasilva
Copy link
Contributor Author

migldasilva commented Oct 27, 2023

Thanks @marrobi , I found that other issue regarding DNS entry just by chance (from time to time I come here to see what's going on). I didn't observe DNS problems during tests with Airlock, but I'll replay them and keep an eye on it.

On the other hand, it seems that I was not clear enough when writing the previous message. My sincere apologies. Indeed, if I don't enable malware scanning in AzureTRE, nothing gets stuck and all the process goes as expected.

By saying "if on-upload malware scanning is not enable, the import request gets stuck.", my idea was to express that malware scanning was not enabled on Azure site (neither on Subscription wide, nor on storage account level). I was moving one small step a time. :)

Thus, once on-upload malware scanning in AzureTRE, and Azure is enabled. Everything goes fine. :)

I'm going to replay the tests and see if DNS problems show up.

@marrobi
Copy link
Member

marrobi commented Oct 27, 2023

Yes, would be good to programmatically enable it based on the environment variable.

@anatbal
Copy link
Contributor

anatbal commented Oct 29, 2023

@marrobi @migldasilva I am willing to start working on enabling it this week, will keep you posted

@anatbal anatbal self-assigned this Oct 29, 2023
@migldasilva
Copy link
Contributor Author

@marrobi I have been trying again to enable Malware scanning, but with no avail. I'm a little bit confused now and have to realize that I wasn't smart enough for previously writing down detailed notes. 🥲

Following the notes I previously prepared, I upgraded only the storage account (SA) stalimexmiguel07dev, and double checked if the configuration was really in place. Here it's mentioned that (1) an Event Grid System Topic should be created for each configured SA, (2) a Data Scanner resource called StorageDataScanner should be enabled on my subscription.

If I'm not mistaken, the following screenshots are show what is expected to happen.

Malware_Scanning_StorageAccount_1

Malware_Scanning_StorageAccount_2

Another step would be choosing an event grid topic for sending scan results. I found this PR #2442 , and it made me think that no addition event grid topic is required. I believe we'd have at most 4 options here:

  1. evgt-airlock-data-deletion-v2-miguel07dev
  2. evgt-airlock-status-changed-v2-miguel07dev
  3. evgt-airlock-step-setup-v2-miguel07dev
  4. evgt-airlock-notification-v2-miguel07dev

Reading this file made me think that evgt-airlock-step-setup-v2-miguel07dev was the event grid topic to be used. After such configurations (following this link), I tried once again to get everything together, and then had no luck.

This screenshot shows some "Publish Failed Events", and the appear after every trial I made.

Malware_Scanning_StorageAccount_3

I tried also to use the other topics for receiving scan results, but no luck. For all of them, "Publish Failed Events" were also registered.

The steps for creating, uploading and submitting an import request are the usual ones. Checking the func-airlock-processor-miguel07dev Function App logs, and the content of SA's stalimexmiguel07dev and stalimipmiguel07dev, it's clear that files are being copied from one SA to another. The last lines in the logs are:

2023-10-30T14:26:28.380134246Z: [INFO]  info: Function.BlobCreatedTrigger.User[0]
2023-10-30T14:26:28.380148346Z: [INFO]        Python ServiceBus topic trigger processed message - A new blob was created!.
2023-10-30T14:26:28.380152746Z: [INFO]  info: Function.BlobCreatedTrigger.User[0]
2023-10-30T14:26:28.382517297Z: [INFO]        Python ServiceBus queue trigger processed message: {"topic":"/subscriptions/23f86cef-1234-5678-9123-1234abcdefgh/resourceGroups/rg-miguel07dev/providers/Microsoft.Storage/storageAccounts/stalimipmiguel07dev","subject":"/blobServices/default/containers/73a935ad-1234-5678-9123-1234abcdefgh/blobs/skeletor_danger.png","eventType":"Microsoft.Storage.BlobCreated","id":"fa2142aa-2006-0030-0001-0b3ee106f702","data":{"api":"CopyBlob","clientRequestId":"4f7a35fa-7730-11ee-8c3f-02c289c0dca8","requestId":"0dad644b-901e-0025-713d-0b2952000000","eTag":"0x8DBD9543402830E","contentType":"image/png","contentLength":493999,"blobType":"BlockBlob","url":"https://stalimipmiguel07dev.blob.core.windows.net/73a935ad-1234-5678-9123-1234abcdefgh/skeletor_danger.png","sequencer":"00000000000000000000000000006B5100000000000e8295","storageDiagnostics":{"batchId":"ff18afc5-2006-0030-003d-0b3ee1000000"}},"dataVersion":"","metadataVersion":"1","eventTime":"2023-10-30T14:26:27.6127225Z"}
2023-10-30T14:26:28.382538198Z: [INFO]  info: Function.BlobCreatedTrigger.User[0]
2023-10-30T14:26:28.382543098Z: [INFO]        Malware scanning is enabled. no action to perform.
2023-10-30T14:26:28.382546698Z: [INFO]  MS_FUNCTION_AZURE_MONITOR_EVENT 4,func-airlock-processor-miguel07dev.azurewebsites.net,Microsoft.Web/sites/functions/log,FunctionAppLogs,uksouth,"{'appName':'func-airlock-processor-miguel07dev','roleInstance':'3b6bb1e5c37cce5875b943397ad7a990c10104db547afd972454fc57f6bdadf8','message':'Executed Functions.BlobCreatedTrigger (Succeeded, Id=bb0aab3f-084e-4d86-9c0a-2c4c9844654b, Duration=18ms)','category':'Function.BlobCreatedTrigger','hostVersion':'3.20.1.0','functionInvocationId':'bb0aab3f-084e-4d86-9c0a-2c4c9844654b','functionName':'Functions.BlobCreatedTrigger','hostInstanceId':'7d222c1d-210e-41ac-92e8-79463ea48e66','level':'Information','levelId':2,'processId':1,'eventId':2,'eventName':'FunctionCompleted'}",10/30/2023 14:26:28
2023-10-30T14:26:28.385222355Z: [INFO]  info: Function.BlobCreatedTrigger[2]
2023-10-30T14:26:28.385243756Z: [INFO]        Executed 'Functions.BlobCreatedTrigger' (Succeeded, Id=bb0aab3f-084e-4d86-9c0a-2c4c9844654b, Duration=18ms)

Is there any other step you may realized I have missed?

@anatbal
Copy link
Contributor

anatbal commented Oct 31, 2023

Hello @migldasilva. Since it has been a long time since we enabled malware scanning on your subscription and it was then in Private Preview, a lot of thing has changed. I think your confusing a few concepts - the scanner should be enabled on the in progress storage account rather than the external one. Also, in the previous version, when enabling the scanner it would automatically create an event grid. This version requires us to create one ourself and create the subscriber ourselves. and finally, it requires code change to airlock processor function since the message itself was changed.
My recommendation - in the following days I will release a PR that programmatically solves these issues. It will be much easier to follow and update that way. Does that sound good to you?

@migldasilva
Copy link
Contributor Author

@anatbal Sounds perfect.

Regarding which storage account to use, and following your explanation, I have also observed a funny behavior. When upgrading the in progress storage account, the Event Grid System Topic was not created. Thus I abandoned this option, I tried to stick with the external storage account (no comments added here because it seemed more an Azure support thing, than AzureTRE related).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants