Skip to content

Commit

Permalink
Scan uploaded attachments (#86)
Browse files Browse the repository at this point in the history
* Added virus scan with deployment

* Fix

* Need to add reference to it here too

* defenderForStorageSettings resource name must be current

* Test code

* Remove webhooks path element

* Fix

* Explicitly define as controller

* Add application handler with logic and re-factored controller code some

* Re-factored malware scan controller to better reflect the underlying issue (i.e, the endpoint needs to be able to receive both arrays and objects of EventGridEvent).

* Set data location url in upload attachment

* Fixed tests

* Prefix with webhook because we have no other signifier that it is a webhook endpoint on the API spec level

* Typos

* Scale to zero is annoying when testing.

* Revise publish logic

* Storage connection string should be a connection string, not a key

* Delete test. Duplicate did not occur now when testing. Assume it occurred because more than one subscription was registered when testing last time.

* Remove comment

* Newlines

* Clean-up

* Rename azure storage account parameter name as it is no longer used exclusively for migrations.

* Delete this for now, better for another PR

* Update parameter name too
  • Loading branch information
Ceredron committed Jun 6, 2024
1 parent 7cf2121 commit 69cc9fb
Show file tree
Hide file tree
Showing 31 changed files with 588 additions and 52 deletions.
14 changes: 13 additions & 1 deletion .azure/applications/api/main.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ param platform_base_url string
param sourceKeyVaultName string
@secure()
param keyVaultUrl string

@secure()
param namePrefix string
@secure()
param storageAccountName string

var image = 'ghcr.io/altinn/altinn-correspondence:${imageTag}'
var containerAppName = '${namePrefix}-app'
Expand Down Expand Up @@ -89,5 +90,16 @@ module containerApp '../../modules/containerApp/main.bicep' = {
}
}

module virusScan '../../modules/virusScan/create.bicep' = {
scope: resourceGroup
name: 'virusScan'
params: {
containerAppIngress: containerApp.outputs.containerAppIngress
location: location
namePrefix: namePrefix
storageAccountName: storageAccountName
}
}

output name string = containerApp.outputs.name
output revisionName string = containerApp.outputs.revisionName
1 change: 1 addition & 0 deletions .azure/applications/api/params.bicepparam
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ param environment = readEnvironmentVariable('ENVIRONMENT')
// secrets
param sourceKeyVaultName = readEnvironmentVariable('KEY_VAULT_NAME')
param keyVaultUrl = readEnvironmentVariable('KEY_VAULT_URL')
param storageAccountName = readEnvironmentVariable('STORAGE_ACCOUNT_NAME')
8 changes: 4 additions & 4 deletions .azure/infrastructure/main.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ param environment string
param namePrefix string

@secure()
param migrationsStorageAccountName string
param storageAccountName string

import { Sku as KeyVaultSku } from '../modules/keyvault/create.bicep'
param keyVaultSku KeyVaultSku
Expand Down Expand Up @@ -76,9 +76,9 @@ module postgresql '../modules/postgreSql/create.bicep' = {

module storageAccount '../modules/storageAccount/create.bicep' = {
scope: resourceGroup
name: migrationsStorageAccountName
name: storageAccountName
params: {
storageAccountName: migrationsStorageAccountName
storageAccountName: storageAccountName
location: location
fileshare: 'migrations'
}
Expand All @@ -92,7 +92,7 @@ module containerAppEnv '../modules/containerAppEnvironment/main.bicep' = {
keyVaultName: sourceKeyVaultName
location: location
namePrefix: namePrefix
storageAccountName: migrationsStorageAccountName
storageAccountName: storageAccountName
}
}
output resourceGroupName string = resourceGroup.name
Expand Down
2 changes: 1 addition & 1 deletion .azure/infrastructure/params.bicepparam
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ param correspondencePgAdminPassword = readEnvironmentVariable('CORRESPONDENCE_PG
param tenantId = readEnvironmentVariable('TENANT_ID')
param test_client_id = readEnvironmentVariable('TEST_CLIENT_ID')
param sourceKeyVaultName = readEnvironmentVariable('KEY_VAULT_NAME')
param migrationsStorageAccountName = readEnvironmentVariable('MIGRATION_STORAGE_ACCOUNT_NAME')
param storageAccountName = readEnvironmentVariable('STORAGE_ACCOUNT_NAME')
// SKUs
param keyVaultSku = {
name: 'standard'
Expand Down
13 changes: 8 additions & 5 deletions .azure/modules/containerApp/main.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ param namePrefix string
param image string
param environment string
param platform_base_url string

@secure()
param subscription_id string
@secure()
Expand All @@ -13,7 +12,6 @@ param principal_id string
param keyVaultUrl string
@secure()
param userIdentityClientId string

@secure()
param containerAppEnvId string

Expand All @@ -31,7 +29,7 @@ var containerAppEnvVars = [
{ name: 'ASPNETCORE_ENVIRONMENT', value: environment }
{ name: 'APPLICATIONINSIGHTS_CONNECTION_STRING', secretRef: 'application-insights-connection-string' }
{ name: 'DatabaseOptions__ConnectionString', secretRef: 'correspondence-ado-connection-string' }
{ name: 'AttachmentStorageOptions__ConnectionString', secretRef: 'storage-account-key'}
{ name: 'AttachmentStorageOptions__ConnectionString', secretRef: 'storage-connection-string'}
{ name: 'AzureResourceManagerOptions__SubscriptionId', value: subscription_id }
{ name: 'AzureResourceManagerOptions__Location', value: 'norwayeast' }
{ name: 'AzureResourceManagerOptions__Environment', value: environment }
Expand Down Expand Up @@ -72,14 +70,18 @@ resource containerApp 'Microsoft.App/containerApps@2023-05-01' = {
}
{
identity: principal_id
keyVaultUrl: '${keyVaultUrl}/secrets/storage-account-key'
name: 'storage-account-key'
keyVaultUrl: '${keyVaultUrl}/secrets/storage-connection-string'
name: 'storage-connection-string'
}
]
}

environmentId: containerAppEnvId
template: {
scale: {
minReplicas: 1
maxReplicas: 1
}
containers: [
{
name: 'app'
Expand All @@ -99,3 +101,4 @@ resource containerApp 'Microsoft.App/containerApps@2023-05-01' = {
output name string = containerApp.name
output revisionName string = containerApp.properties.latestRevisionName
output app object = containerApp
output containerAppIngress string = containerApp.properties.configuration.ingress.fqdn
11 changes: 6 additions & 5 deletions .azure/modules/containerAppEnvironment/main.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,15 @@ module containerAppEnvIdSecret '../keyvault/upsertSecret.bicep' = {
}
}

var storageAccountKeySecretName = 'storage-account-key'
module storageAccountKeySecret '../keyvault/upsertSecret.bicep' = {
name: storageAccountKeySecretName
var storageAccountConnectionStringSecretName = 'storage-connection-string'
module storageAccountConnectionStringSecret '../keyvault/upsertSecret.bicep' = {
name: storageAccountConnectionStringSecretName
params: {
destKeyVaultName: keyVaultName
secretName: storageAccountKeySecretName
secretValue: storageAccount.listKeys().keys[0].value
secretName: storageAccountConnectionStringSecretName
secretValue: 'DefaultEndpointsProtocol=https;AccountName=${storageAccountName};AccountKey=${storageAccount.listKeys().keys[0].value};EndpointSuffix=core.windows.net'
}
}


output containerAppEnvironmentId string = containerAppEnvironment.id
48 changes: 48 additions & 0 deletions .azure/modules/virusScan/create.bicep
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
param location string
@secure()
param namePrefix string
@secure()
param storageAccountName string
@secure()
param containerAppIngress string

resource eventgrid_topic 'Microsoft.EventGrid/topics@2022-06-15' = {
name: '${namePrefix}-malware-scan-event-topic'
location: location
}

resource eventgrid_event_subscription 'Microsoft.EventGrid/topics/eventSubscriptions@2022-06-15' = {
name: '${namePrefix}-malware-scan-event-subscription'
parent: eventgrid_topic
properties: {
destination: {
endpointType: 'WebHook'
properties: {
endpointUrl: 'https://${containerAppIngress}/correspondence/api/v1/webhooks/malwarescanresults'
}
}
}
}

resource storageAccount 'Microsoft.Storage/storageAccounts@2023-04-01' existing = {
name: storageAccountName
}

resource malwareScanSettings 'Microsoft.Security/defenderForStorageSettings@2022-12-01-preview' = {
name: 'current'
scope: storageAccount
properties: {
isEnabled: true
malwareScanning: {
onUpload: {
capGBPerMonth: -1
isEnabled: true
}
scanResultsEventGridTopicResourceId: eventgrid_topic.id
}
overrideSubscriptionLevelSettings: true
sensitiveDataDiscovery: {
isEnabled: false
}
}
}
8 changes: 4 additions & 4 deletions .github/actions/migrate-database/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ inputs:
AZURE_NAME_PREFIX:
description: "Prefix for all resources"
required: true
AZURE_MIGRATION_STORAGE_ACCOUNT_NAME:
description: "Name of the storage account for migration files"
AZURE_STORAGE_ACCOUNT_NAME:
description: "Name of the storage account used for attachments and migrations"
required: true

runs:
Expand Down Expand Up @@ -72,8 +72,8 @@ runs:
with:
azcliversion: 2.56.0
inlineScript: |
az storage copy --source ./src/Altinn.Correspondence.Persistence/Migrations/bundle.exe --destination https://${{ inputs.AZURE_MIGRATION_STORAGE_ACCOUNT_NAME }}.file.core.windows.net/migrations --recursive
az storage copy --source ./src/Altinn.Correspondence.API/appsettings.json --destination https://${{ inputs.AZURE_MIGRATION_STORAGE_ACCOUNT_NAME }}.file.core.windows.net/migrations --recursive
az storage copy --source ./src/Altinn.Correspondence.Persistence/Migrations/bundle.exe --destination https://${{ inputs.AZURE_STORAGE_ACCOUNT_NAME }}.file.core.windows.net/migrations --recursive
az storage copy --source ./src/Altinn.Correspondence.API/appsettings.json --destination https://${{ inputs.AZURE_STORAGE_ACCOUNT_NAME }}.file.core.windows.net/migrations --recursive
- name: Start migration job
uses: azure/CLI@v2
Expand Down
4 changes: 4 additions & 0 deletions .github/actions/release-version/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ inputs:
PLATFORM_BASE_URL:
description: "Base url for Altinn platform"
required: true
STORAGE_ACCOUNT_NAME:
description: "Name of the storage account used for attachments"
required: true

runs:
using: "composite"
Expand All @@ -55,6 +58,7 @@ runs:
CLIENT_ID: ${{ inputs.AZURE_CLIENT_ID }}
TENANT_ID: ${{ inputs.AZURE_TENANT_ID }}
PLATFORM_BASE_URL: ${{ inputs.PLATFORM_BASE_URL }}
STORAGE_ACCOUNT_NAME: ${{ inputs.STORAGE_ACCOUNT_NAME }}
with:
scope: subscription
subscriptionId: ${{ inputs.AZURE_SUBSCRIPTION_ID }}
Expand Down
4 changes: 2 additions & 2 deletions .github/actions/update-infrastructure/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ inputs:
AZURE_TEST_ACCESS_CLIENT_ID:
description: "Client ID for the test access service principal"
required: true
AZURE_MIGRATION_STORAGE_ACCOUNT_NAME:
AZURE_STORAGE_ACCOUNT_NAME:
description: "Name of the storage account for migration files"
required: true

Expand Down Expand Up @@ -69,7 +69,7 @@ runs:
TEST_CLIENT_ID: ${{ inputs.AZURE_TEST_ACCESS_CLIENT_ID }}
NAME_PREFIX: ${{ inputs.AZURE_NAME_PREFIX }}
KEY_VAULT_NAME: ${{ inputs.AZURE_ENVIRONMENT_KEY_VAULT_NAME }}
MIGRATION_STORAGE_ACCOUNT_NAME: ${{ inputs.AZURE_MIGRATION_STORAGE_ACCOUNT_NAME }}
STORAGE_ACCOUNT_NAME: ${{ inputs.AZURE_STORAGE_ACCOUNT_NAME }}
with:
scope: subscription
template: ./.azure/infrastructure/main.bicep
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/deploy-to-environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ jobs:
AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }}
AZURE_SUBSCRIPTION_ID: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
AZURE_TEST_ACCESS_CLIENT_ID: ${{ secrets.AZURE_TEST_ACCESS_CLIENT_ID }}
AZURE_MIGRATION_STORAGE_ACCOUNT_NAME: ${{ secrets.AZURE_MIGRATION_STORAGE_ACCOUNT_NAME }}
AZURE_STORAGE_ACCOUNT_NAME: ${{ secrets.AZURE_STORAGE_ACCOUNT_NAME }}

- name: Migrate database
if: ${{ inputs.hasMigrationChanges == 'true' || inputs.hasMigrationChanges == true }}
Expand All @@ -79,7 +79,7 @@ jobs:
AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }}
AZURE_SUBSCRIPTION_ID: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
AZURE_MIGRATION_STORAGE_ACCOUNT_NAME: ${{ secrets.AZURE_MIGRATION_STORAGE_ACCOUNT_NAME }}
AZURE_STORAGE_ACCOUNT_NAME: ${{ secrets.AZURE_STORAGE_ACCOUNT_NAME }}

- name: Release version
if: ${{ inputs.hasBackendChanges == 'true' || inputs.hasBackendChanges == true }}
Expand All @@ -94,4 +94,5 @@ jobs:
PLATFORM_BASE_URL: ${{ secrets.PLATFORM_BASE_URL }}
AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }}
STORAGE_ACCOUNT_NAME: ${{ secrets.AZURE_STORAGE_ACCOUNT_NAME }}

2 changes: 1 addition & 1 deletion .github/workflows/publish-branch.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@ jobs:
PLATFORM_BASE_URL: ${{ secrets.PLATFORM_BASE_URL }}
AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }}
STORAGE_ACCOUNT_NAME: ${{ secrets.AZURE_MIGRATION_STORAGE_ACCOUNT_NAME }}
STORAGE_ACCOUNT_NAME: ${{ secrets.AZURE_STORAGE_ACCOUNT_NAME }}

2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,4 @@ dotnet ef database update
Formatting of the code base is handled by Dotnet format. [See how to configure it to format-on-save in Visual Studio here.](https://learn.microsoft.com/en-us/community/content/how-to-enforce-dotnet-format-using-editorconfig-github-actions#3---formatting-your-code-locally)

## Deploy
The build and push workflow produces a docker image that is pushed to Github packages. This image is then used by the release action. Read more here: [Readme-infrastructure](/README-infrastructure.md)
The build and push workflow produces a docker image that is pushed to Github packages. This image is then used by the release action. Read more here: [Readme-infrastructure](/README-infrastructure.md)
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net8.0</TargetFramework>
Expand All @@ -9,6 +9,24 @@
<IsTestProject>true</IsTestProject>
</PropertyGroup>

<ItemGroup>
<None Remove="Data\MalwareScanResult_Malicious.json" />
<None Remove="Data\MalwareScanResult_NoThreatFound.json" />
<None Remove="Data\WebHookSubscriptionValidationTest.json" />
</ItemGroup>

<ItemGroup>
<Content Include="Data\MalwareScanResult_Malicious.json">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
<Content Include="Data\MalwareScanResult_NoThreatFound.json">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
<Content Include="Data\WebHookSubscriptionValidationTest.json">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.AspNetCore.Mvc.Testing" Version="8.0.4" />
<PackageReference Include="Microsoft.EntityFrameworkCore.InMemory" Version="8.0.4" />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System.Net;
using System.Net.Http.Json;
using System.Text.Json;
using System.Text.Json.Serialization;
using Altinn.Correspondece.Tests.Factories;
using Altinn.Correspondence.API.Models;
using Altinn.Correspondence.API.Models.Enums;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"data": {
"blobUri": "http://127.0.0.1:10000/devstoreaccount1/attachments/--FILEID--",
"correlationId": "2ee9f258-c96a-4982-9e6e-16b8485d71da",
"eTag": "--ETAGID--",
"scanFinishedTimeUtc": "2023-12-08T08:12:31.9933275Z",
"scanResultDetails": {
"malwareNamesFound": [
"Virus:DOS/EICAR_Test_File"
],
"sha256": "275A021BBFB6489E54D471899F7DB9D1663FC695EC2FE2A2C4538AABF651FD0F"
},
"scanResultType": "Malicious"
},
"dataVersion": "1.0",
"eventTime": "2023-12-08T08:12:31.9939079Z",
"eventType": "Microsoft.Security.MalwareScanningResult",
"id": "2ee9f258-c96a-4982-9e6e-16b8485d71da",
"metadataVersion": "1",
"subject": "storageAccounts/devstoreaccount1/containers/attachments/blobs/--FILEID--",
"topic": "/subscriptions/81cc3a6b-dfdf-49c7-96f0-3efddb159356/resourceGroups/serviceowner-test-0192-991825827-rg/providers/Microsoft.EventGrid/topics/test-broker-defenderresults"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"data": {
"blobUri": "http://127.0.0.1:10000/devstoreaccount1/attachments/--FILEID--",
"correlationId": "21c48159-e5ef-4376-ba96-4f8d6e0f1c7f",
"eTag": "--ETAGID--",
"scanFinishedTimeUtc": "2023-12-08T08:11:44.9457492Z",
"scanResultDetails": null,
"scanResultType": "No threats found"
},
"dataVersion": "1.0",
"eventTime": "2023-12-08T08:11:44.9464641Z",
"eventType": "Microsoft.Security.MalwareScanningResult",
"id": "21c48159-e5ef-4376-ba96-4f8d6e0f1c7f",
"metadataVersion": "1",
"subject": "storageAccounts/devstoreaccount1/containers/attachments/blobs/--FILEID--",
"topic": "/subscriptions/81cc3a6b-dfdf-49c7-96f0-3efddb159356/resourceGroups/serviceowner-test-0192-991825827-rg/providers/Microsoft.EventGrid/topics/test-broker-defenderresults"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[
{
"id": "2d1781af-3a4c-4d7c-bd0c-e34b19da4e66",
"topic": "/subscriptions/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx",
"subject": "",
"data": {
"validationCode": "512d38b6-c7b8-40c8-89fe-f46f9e9622b6",
"validationUrl": "https://www.contoso.com/"
},
"eventType": "Microsoft.EventGrid.SubscriptionValidationEvent",
"eventTime": "2018-01-25T22:12:19.4556811Z",
"metadataVersion": "1",
"dataVersion": "1"
}
]
Loading

0 comments on commit 69cc9fb

Please sign in to comment.