Correct IP range for Management subnet IP group (#91)

* Correct IP range for Management subnet IP group * Add comment clarifying why forwarded traffic is allowed * Add NetBIOS name resolution port to AD FW rules * Add support for application security groups on session host and Mgmt VM NICs * Register Microsoft.Storage RP in hub sub for imaging * Configure NSGs for AVD and Mgmt hub subnets * Add application security groups for AVD and Mgmt VMs * Follow normal pattern for creating subnet address prefixes
Azure · Jul 31, 2024 · f8968ee · f8968ee
1 parent 652ff1e
commit f8968ee
Show file tree

Hide file tree

Showing 10 changed files with 201 additions and 47 deletions.
diff --git a/research-hub/azure-firewall-rules/ActiveDirectory.jsonc b/research-hub/azure-firewall-rules/ActiveDirectory.jsonc
@@ -20,8 +20,9 @@
           "88", // Kerberos
           "123", // NTP
           "135", // RPC
+          "138", // NetBIOS name resolution
           "389", // LDAP
-          "445", // SMB
+          "445", // SMB, SAM/LSA
           "636", // LDAPS
           "3268-3269", // Global Catalog
           "9389", // ADWS

diff --git a/research-hub/deploy.ps1 b/research-hub/deploy.ps1
@@ -56,8 +56,10 @@ Set-AzContextWrapper -SubscriptionId $TargetSubscriptionId -Environment $Environ
 # Ensure the EncryptionAtHost feature is registered for the current subscription
 # LATER: Do this with a deployment script in Bicep
 Register-AzProviderFeatureWrapper -ProviderNamespace "Microsoft.Compute" -FeatureName "EncryptionAtHost"
+# LATER: Run provider and feature registrations in parallel
+Register-AzResourceProviderWrapper -ProviderNamespace "Microsoft.Storage"
 
-# Remove the module from the session
+# Remove the module from the session (always, even in WhatIf mode)
 Remove-Module AzSubscriptionManagement -WhatIf:$false
 
 [string]$DeploymentName = "ResearchHub-$(Get-Date -Format 'yyyyMMddThhmmssZ' -AsUTC)"

diff --git a/research-hub/hub-modules/management-vm/main.bicep b/research-hub/hub-modules/management-vm/main.bicep
@@ -9,6 +9,7 @@ param vmSize string = 'Standard_B2as_v2'
 
 param diskEncryptionSetId string = ''
 param subnetId string
+param applicationSecurityGroupId string
 
 @secure()
 param vmLocalAdminUsername string
@@ -77,6 +78,13 @@ resource nic 'Microsoft.Network/networkInterfaces@2022-11-01' = {
           subnet: {
             id: subnetId
           }
+          applicationSecurityGroups: !empty(applicationSecurityGroupId)
+            ? [
+                {
+                  id: applicationSecurityGroupId
+                }
+              ]
+            : []
         }
       }
     ]

diff --git a/research-hub/hub-modules/networking/main.bicep b/research-hub/hub-modules/networking/main.bicep
@@ -44,9 +44,50 @@ param tags object
 param deploymentTime string
 param deploymentNameStructure string
 param resourceNamingStructure string
-@description('The resource naming structure to use for IP Group resources. For ease of use when creating firewall rules, it is useful to change the order of the segments. Optiona; defaults to resourceNamingStructure.')
+@description('The resource naming structure to use for IP Group resources. For ease of use when creating firewall rules, it is useful to change the order of the segments. Optional; defaults to resourceNamingStructure.')
 param ipGroupNamingStructure string = resourceNamingStructure
 
+module managementApplicationSecurityGroupModule 'br/public:avm/res/network/application-security-group:0.1.4' = if (deployManagementSubnet) {
+  name: take(replace(deploymentNameStructure, '{rtype}', 'asg-mgmt'), 64)
+  params: {
+    name: replace(replace(resourceNamingStructure, '{subWorkloadName}', 'network'), '{rtype}', 'asg-mgmt')
+    tags: tags
+  }
+}
+
+module avdApplicationSecurityGroupModule 'br/public:avm/res/network/application-security-group:0.1.4' = if (deployAvdSubnet) {
+  name: take(replace(deploymentNameStructure, '{rtype}', 'asg-avd'), 64)
+  params: {
+    name: replace(replace(resourceNamingStructure, '{subWorkloadName}', 'network'), '{rtype}', 'asg-avd')
+    tags: tags
+  }
+}
+
+// Create appropriate security rules for the Management and AVD subnets
+module managementSubnetSecurityRulesModule 'securityRules/managementAndAvdSubnetSecurityRules.bicep' = if (deployManagementSubnet) {
+  name: take(replace(deploymentNameStructure, '{rtype}', 'sr-mgmt'), 64)
+  params: {
+    applicationSecurityGroupId: managementApplicationSecurityGroupModule.outputs.resourceId
+    customDnsIPs: customDnsIPs
+    deploySubnet: deployManagementSubnet
+    domainControllerIPAddresses: domainControllerIPAddresses
+    includeActiveDirectoryFirewallRules: includeActiveDirectoryFirewallRules
+    includeDnsFirewallRules: includeDnsFirewallRules
+  }
+}
+
+module avdSubnetSecurityRulesModule 'securityRules/managementAndAvdSubnetSecurityRules.bicep' = if (deployAvdSubnet) {
+  name: take(replace(deploymentNameStructure, '{rtype}', 'sr-avd'), 64)
+  params: {
+    applicationSecurityGroupId: avdApplicationSecurityGroupModule.outputs.resourceId
+    customDnsIPs: customDnsIPs
+    deploySubnet: deployAvdSubnet
+    domainControllerIPAddresses: domainControllerIPAddresses
+    includeActiveDirectoryFirewallRules: includeActiveDirectoryFirewallRules
+    includeDnsFirewallRules: includeDnsFirewallRules
+  }
+}
+
 /*
  * DEFINE THE RESEARCH HUB VIRTUAL NETWORK'S SUBNETS
  */
@@ -63,16 +104,14 @@ var requiredSubnets = {
     routes: []
     securityRules: []
     delegation: ''
-    order: 4
-    subnetCidr: 27
+    addressPrefix: cidrSubnet(networkAddressSpace, 27, 4) // The fifth /27
   }
   AzureFirewallSubnet: {
     serviceEndpoints: []
     routes: AzureFirewallSubnetRoutes
     //securityRules: [] Azure Firewall does not support NSGs on its subnets
     delegation: ''
-    order: 0
-    subnetCidr: 26
+    addressPrefix: cidrSubnet(networkAddressSpace, 26, 0) // The first /26
   }
 }
 
@@ -85,8 +124,7 @@ var AzureFirewallManagementSubnet = createManagementIPConfiguration
         routes: loadJsonContent('./routes/AzureFirewallNormal.jsonc')
         //securityRules: [] Azure Firewall does not support NSGs on its subnets
         delegation: ''
-        order: 1
-        subnetCidr: 26
+        addressPrefix: cidrSubnet(networkAddressSpace, 26, 1) // The second /26
       }
     }
   : {}
@@ -99,8 +137,7 @@ var AirlockSubnet = deployAirlockSubnet
         routes: [] // Routes through the firewall will be added later
         securityRules: [] // TODO: Allow RDP only from the AVD and Bastion subnets?
         delegation: ''
-        order: 5 // The fourth /27-sized subnet
-        subnetCidr: 27 // There will never be many airlock review virtual machines taking up addresses
+        addressPrefix: cidrSubnet(networkAddressSpace, 27, 5) // The sixth /27
       }
     }
   : {}
@@ -112,8 +149,7 @@ var AzureBastionSubnet = deployBastion
         //routes: [] Bastion doesn't support routes
         securityRules: loadJsonContent('./securityRules/bastion.jsonc')
         delegation: ''
-        order: 3 // The first /26, in the first /24 block
-        subnetCidr: 26 // Minimum for AzureBastionSubnet
+        addressPrefix: cidrSubnet(networkAddressSpace, 26, 3)
       }
     }
   : {}
@@ -124,8 +160,7 @@ var GatewaySubnet = deployVpn && !useRemoteGateway
         routes: []
         // securityRules: [] GatewaySubnet does not support NSGs
         delegation: ''
-        order: 8 // There will already be a /26 for Bastion if enabled, so this becomes the third /27
-        subnetCidr: 27 // Minimum recommended for GatewaySubnet
+        addressPrefix: cidrSubnet(networkAddressSpace, 27, 8) // The ninth /27
       }
     }
   : {}
@@ -135,10 +170,9 @@ var AvdSubnet = deployAvdSubnet
       AvdSubnet: {
         serviceEndpoints: []
         routes: [] // Routes through the firewall will be added later, but we create the route table resource here
-        securityRules: []
+        securityRules: avdSubnetSecurityRulesModule.outputs.securityRules
         delegation: ''
-        order: 9 // The tenth /27
-        subnetCidr: 27
+        addressPrefix: cidrSubnet(networkAddressSpace, 27, 9) // The tenth /27
       }
     }
   : {}
@@ -148,9 +182,8 @@ var ManagementSubnet = deployManagementSubnet
       ManagementSubnet: {
         serviceEndpoints: []
         routes: [] // Routes through the firewall will be added later, but we create the route table resource here
-        securityRules: []
-        order: 11 // The twelfth /27
-        subnetCidr: 27
+        securityRules: managementSubnetSecurityRulesModule.outputs.securityRules
+        addressPrefix: cidrSubnet(networkAddressSpace, 27, 11) // The twelfth /27
       }
     }
   : {}
@@ -166,33 +199,14 @@ var subnets = union(
   ManagementSubnet
 )
 
-/*
- * Calculate the subnet addresses
- */
-
-var actualSubnets = [
-  for (subnet, i) in items(subnets): {
-    // Add a new property addressPrefix to each subnet definition. If addressPrefix property was already defined, it will be respected.
-    '${subnet.key}': union(
-      {
-        // If the subnet specifies its own CIDR size, use it; otherwise, use the default
-        addressPrefix: cidrSubnet(networkAddressSpace, subnet.value.subnetCidr, subnet.value.order)
-      },
-      subnet.value
-    )
-  }
-]
-
-var actualSubnetObject = reduce(actualSubnets, {}, (cur, next) => union(cur, next))
-
 // Create the route tables, network security groups, and virtual network
 module networkModule '../../../shared-modules/networking/main.bicep' = {
   name: replace(deploymentNameStructure, '{rtype}', 'network')
   params: {
     location: location
     deploymentNameStructure: deploymentNameStructure
     namingStructure: resourceNamingStructure
-    subnetDefs: actualSubnetObject
+    subnetDefs: subnets
     vnetAddressPrefixes: [networkAddressSpace]
 
     additionalSubnets: additionalSubnets
@@ -226,7 +240,7 @@ module managementSubnetIPGroupModule '../../../shared-modules/networking/ipGroup
   params: {
     name: replace(ipGroupNamingStructure, '{rtype}', 'ipg-Mgmt_Subnet')
     location: location
-    ipAddresses: [networkModule.outputs.createdSubnets.AzureFirewallManagementSubnet.addressPrefix]
+    ipAddresses: [networkModule.outputs.createdSubnets.ManagementSubnet.addressPrefix]
     tags: tags
   }
   // Cannot simultaneously deploy multiple IP Groups that are already in use by the same firewall
@@ -375,8 +389,12 @@ module allPrivateDnsZonesModule '../dns/allPrivateDnsZones.bicep' =
     }
   }
 
-// TODO: Fix the route tables when the firewall's IP address is known
-
 output createdSubnets object = networkModule.outputs.createdSubnets
 output vNetId string = networkModule.outputs.vNetId
 output fwPrivateIPAddress string = azureFirewallModule.outputs.fwPrIp
+output managementSubnetApplicationSecurityGroupId string = deployManagementSubnet
+  ? managementApplicationSecurityGroupModule.outputs.resourceId
+  : ''
+output avdSubnetApplicationSecurityGroupId string = deployAvdSubnet
+  ? avdApplicationSecurityGroupModule.outputs.resourceId
+  : ''
diff --git a/research-hub/hub-modules/networking/securityRules/managementAndAvdSubnetSecurityRules.bicep b/research-hub/hub-modules/networking/securityRules/managementAndAvdSubnetSecurityRules.bicep
@@ -0,0 +1,54 @@
+param deploySubnet bool
+param includeDnsFirewallRules bool
+param includeActiveDirectoryFirewallRules bool
+
+param customDnsIPs array
+param domainControllerIPAddresses array
+
+param applicationSecurityGroupId string
+
+var dnsSecurityRule = deploySubnet && includeDnsFirewallRules
+  ? [
+      {
+        name: 'Allow_Outbound_DNS'
+        properties: {
+          direction: 'Outbound'
+          priority: 200
+          protocol: '*'
+          access: 'Allow'
+          sourceApplicationSecurityGroups: [
+            {
+              id: applicationSecurityGroupId
+            }
+          ]
+          sourcePortRange: '*'
+          destinationAddressPrefixes: customDnsIPs
+          destinationPortRanges: ['53']
+        }
+      }
+    ]
+  : []
+
+var addcSecurityRule = deploySubnet && includeActiveDirectoryFirewallRules
+  ? [
+      {
+        name: 'Allow_Outbound_ADDC'
+        properties: {
+          direction: 'Outbound'
+          priority: 210
+          protocol: '*' // TCP, UDP, and also allows ICMP echo, which is a benefit
+          access: 'Allow'
+          sourceApplicationSecurityGroups: [
+            {
+              id: applicationSecurityGroupId
+            }
+          ]
+          sourcePortRange: '*'
+          destinationAddressPrefixes: domainControllerIPAddresses
+          destinationPortRanges: ['88', '123', '135', '138', '389', '445', '636', '3268-3269', '9389', '49152-65535']
+        }
+      }
+    ]
+  : []
+
+output securityRules array = union(dnsSecurityRule, addcSecurityRule)
diff --git a/research-hub/main.bicep b/research-hub/main.bicep
@@ -431,7 +431,10 @@ module avdJumpBoxSessionHostModule '../shared-modules/virtualDesktop/sessionHost
     hostPoolToken: avdJumpBoxModule.outputs.hostPoolRegistrationToken
     logonType: logonType
     namingStructure: replace(resourceNamingStructure, '{subWorkloadName}', 'avd')
+
     subnetId: networkModule.outputs.createdSubnets.AvdSubnet.id
+    applicationSecurityGroupId: networkModule.outputs.avdSubnetApplicationSecurityGroupId
+
     vmCount: jumpBoxSessionHostCount
     vmLocalAdminUsername: sessionHostLocalAdminUsername
     vmLocalAdminPassword: sessionHostLocalAdminPassword
@@ -506,6 +509,7 @@ module managementVmModule './hub-modules/management-vm/main.bicep' = if (logonTy
     tags: actualTags
     namingStructure: replace(resourceNamingStructure, '{subWorkloadName}', 'mgmtvm')
     subnetId: networkModule.outputs.createdSubnets.ManagementSubnet.id
+    applicationSecurityGroupId: networkModule.outputs.managementSubnetApplicationSecurityGroupId
 
     vmLocalAdminUsername: sessionHostLocalAdminUsername
     vmLocalAdminPassword: sessionHostLocalAdminPassword

diff --git a/scripts/PowerShell/Modules/AzSubscriptionManagement.psm1 b/scripts/PowerShell/Modules/AzSubscriptionManagement.psm1
@@ -102,8 +102,65 @@ Function Register-AzProviderFeatureWrapper {
     }
 }
 
+<#
+    .SYNOPSIS
+    Registers an Azure subscription for a resource provider feature.
+
+    .DESCRIPTION
+    Determines if the specified feature for the specified resource provider namespace is registered. If not, it will register the feature and wait for registration to complete.
+
+    .NOTES
+    The current Azure context will be used to determine the subscription to register the feature in.
+#>
+Function Register-AzResourceProviderWrapper {
+    [CmdletBinding()]
+    param (
+        [Parameter(Mandatory, Position = 1)]
+        [string]$ProviderNamespace
+    )
+
+    # Because this function is in a module, $VerbosePreference doesn't carry over from the caller
+    # See https://stackoverflow.com/a/44902512/816663
+    if (-Not $PSBoundParameters.ContainsKey('Verbose')) {
+        $VerbosePreference = $PSCmdlet.GetVariableValue('VerbosePreference')
+    }
+
+    # Get the feature's current registration state
+    $Provider = Get-AzResourceProvider -ProviderNamespace $ProviderNamespace
+    $AzContext = Get-AzContext
+
+    # If the feature is not registered yet
+    if ($Provider.RegistrationState -ne 'Registered') {
+        Write-Warning "About to register provider '$($Provider.ProviderNamespace)' in subscription '$($AzContext.Subscription.Name)'. Expect a delay while the feature registration is completed."
+        $Status = Register-AzResourceProvider -ProviderNamespace $ProviderNamespace
+
+        if ($Status.RegistrationState -eq 'Registering') {
+            [double]$PercentComplete = 1
+            Write-Progress -Activity "Registering provider '$($Status.ProviderNamespace)'" -Id 0 -PercentComplete $PercentComplete -SecondsRemaining -1
+
+            while ($Status.RegistrationState -eq 'Registering') {
+                Start-Sleep -Seconds 30
+                $Status = Get-AzResourceProvider -ProviderNamespace $ProviderNamespace
+                # Assuming 20 minutes (max); so each 30 seconds is 2.5% complete
+                $PercentComplete += 2.5
+                Write-Progress -Activity "Registering provider '$($Status.ProviderNamespace)'" -Id 0 -PercentComplete $PercentComplete -SecondsRemaining -1
+            }
+
+            $PercentComplete = 100
+            Write-Progress -Activity "Registering provider '$($Status.ProviderNamespace)'" -Id 0 -PercentComplete $PercentComplete -SecondsRemaining 0
+            Write-Information "Provider registration complete."
+        }
+        else {
+            Write-Error "Provider registration failed: $($Status.RegistrationState)"
+        }
+    }
+    else {
+        Write-Verbose "Provider '$($Provider.ProviderNamespace)' is already registered in subscription '$($AzContext.Subscription.Name)'."
+    }
+}
+
 Export-ModuleMember -Function Set-AzContextWrapper
 Export-ModuleMember -Function Register-AzProviderFeatureWrapper
 
 # TODO: Develop module to register resource providers
-#Export-ModuleMember -Function Register-AzSubscriptionResourceProvider
+Export-ModuleMember -Function Register-AzResourceProviderWrapper
diff --git a/shared-modules/networking/peering.bicep b/shared-modules/networking/peering.bicep
@@ -4,9 +4,10 @@ param remoteVNetId string
 param localVNetFriendlyName string
 param remoteVNetFriendlyName string
 
+param allowVirtualNetworkAccess bool = true
+// This should generally be true because this is a spoke that might receive traffic forwarded by the FW from other spokes
 param allowForwardedTraffic bool = true
 param allowGatewayTransit bool = false
-param allowVirtualNetworkAccess bool = true
 param useRemoteGateways bool = false
 
 var peeringName = take('peering-${localVNetFriendlyName}-to-${remoteVNetFriendlyName}', 80)