JulianHayward / Azure-MG-Sub-Governance-Reporting

Azure Governance Visualizer aka AzGovViz is a PowerShell script that captures Azure Governance related information such as Azure Policy, RBAC (a lot more) by polling Azure ARM, Storage and Microsoft Graph APIs.
MIT License
814 stars 293 forks source link

Multiple tenants experience error while executing Storage Account Analysis with v6.3.4 #218

Closed MarcelHeek closed 6 months ago

MarcelHeek commented 7 months ago

AzGovViz version v6.3.4

CodeRunPlatform GitHub Actions

Describe the bug Multiple tenants throw error processing the XMLSAProperties as can be seen in the screenshot below

Screenshots image

InvalidArgument: Line | 50 | … Write-Host $($saProperties.ForEach({ [char]$_ }) -joi … | ~~~~ | Cannot convert value "<?xml version="1.0" encoding="utf-8"?>1.0falsefalsefalsefalse1.0truetruetrue71.0falsefalsefalsefalse" to type "System.Char". Error: "String must be exactly one character long." Error: Error: The process '/usr/bin/pwsh' failed with exit code 1

Additional context Add any other context about the problem here.

JulianHayward commented 7 months ago

@MarcelHeek is this a new SA (did it work for this one before), any special configuration?

MarcelHeek commented 7 months ago

@JulianHayward

I looked back 1 month ago (for one of the tenants) and the logic was using v6.3.0 and there was no issue at that time and I looked at the activity log of the SA and it was created >2 months ago.

I cannot see anything in the configuration that makes me frown and think of any blocking settings.

JulianHayward commented 7 months ago

@MarcelHeek to my knowledge there were no changes on the SA processing. Does it work with 6.3.0?

kaiaschulz commented 6 months ago

Hey, we are having the same issue in multiple tenants.

We had the last successful run on 09-Dec-2023 02:44:24 (Etc/UTC) and the first failed pipeline on 09-Dec-2023 07:28:56 (Etc/UTC). Since then, all pipelines are failing.

MarcelHeek commented 6 months ago

@JulianHayward I tested with v6.3.0 and the same issue. I must say I have not checked the execution recently due to circumstances, but the latest successful runs were Nov 14th and was while on v6.3.0. So this makes it even more strange.

kaiaschulz commented 6 months ago

It seems that the variable $saProperties has a leading byte order mark U+feff. Depending on the selected encoding, it is invisible. The character U+feff is invisible.

I am using the encoding UTF-8 with BOM.

# $xmlSaProperties = [xml]([string]$saProperties -replace $saProperties.Substring(0, 3)) # Leading character:  (PS version <= 7.3.9)
# $xmlSaProperties = [xml]([string]$saProperties -replace $saProperties.Substring(0, 1)) # Leading character:  or U+feff (PS version >= 7.4.0)
$xmlSaProperties = [xml]($saProperties -replace ($saProperties -replace "<.*")) # Universal fix for all PS versions

It seems that this is related to the newest Powershell version PS Edition: Core; PS Version: 7.4.0. With < 7.4.0 it is still working as expected (e.g. with PS v7.3.9)

PowerShell may have difficulty interpreting XML files that contain a Byte Order Mark (BOM). The [xml] type expects the XML file to start with the XML declaration <?xml ... ?>, and the BOM characters can interfere with the correct interpretation.

@MarcelHeek could you verify this? If this will fix the behavior, I will create a pull request to update the code.

MarcelHeek commented 6 months ago

@kaiaschulz Is it possible for you to put this fix in a branch then I can ref it in my workflow and perform a test. If not possible, I will look into creating a clone when I have some spare time.

kaiaschulz commented 6 months ago

Hey, @MarcelHeek here we go: https://github.com/kaiaschulz/Azure-MG-Sub-Governance-Reporting/blob/kaiaschulz-6.3.5/pwsh/AzGovVizParallel.ps1

MarcelHeek commented 6 months ago

@kaiaschulz The datacollection now succeeds for a subset of our tenants, there is one tenant with similar issue but now from a blobcontainer. Perhaps this needs the same fix?

image

kaiaschulz commented 6 months ago

Hey @MarcelHeek , thanks for testing and providing the feedback.

Have you used my provided code with v6.3.5 or did you just replaced the suggested code snipped? Because I had exactly the same problem after my first try. Afterwards I figured out that the failing script block was exchanged on 2 positions:

# $xmlSaProperties = [xml]([string]$saProperties -replace $saProperties.Substring(0, 3)) # Leading character:  (PS version <= 7.3.9)
# $xmlSaProperties = [xml]([string]$saProperties -replace $saProperties.Substring(0, 1)) # Leading character:  or U+feff (PS version >= 7.4.0)
$xmlSaProperties = [xml]($saProperties -replace ($saProperties -replace "<.*")) # Universal fix for all PS versions

and

# $xmlListContainers = [xml]([string]$listContainers -replace $listContainers.Substring(0, 3)) # Leading character:  (PS version <= 7.3.9)
# $xmlListContainers = [xml]([string]$listContainers -replace $listContainers.Substring(0, 1)) # Leading character:  or U+feff (PS version >= 7.4.0)
$xmlListContainers = [xml]($listContainers -replace ($listContainers -replace "<.*")) # Universal fix for all PS versions

I successfully ran this script v6.3.5 within 2 tenants and also the $xmlListContainers was working as expected.

Otherwise, you might can run the code snipped locally (change the subscription id in line 12) with breakpoint (line 45, 83) and check what is happening? With the comments of lines 44, 46, 82 and 84 you get the output to a string in your clipboard. You can check if the BOM is still existing and causing the issue with [XML]:

# Import-Module AzAPICall #https://www.powershellgallery.com/packages/AzAPICall/1.1.84

#Region initAzAPICall
$parameters4AzAPICallModule = @{
    DebugAzAPICall   = $true
    writeMethod      = 'Warning' #Debug, Error, Host, Information, Output, Progress, Verbose, Warning (default: host)
    debugWriteMethod = 'Warning' #Debug, Error, Host, Information, Output, Progress, Verbose, Warning (default: host)
}
$script:azAPICallConf = initAzAPICall @parameters4AzAPICallModule
#EndRegion initAzAPICall

$scopeId = 'xxxxx-xxx-xxx-xxxxx' # Your subscription id

$currentTask = "Getting Storage Accounts for Subscription: '$($scopeDisplayName)' ('$scopeId') [quotaId:'$subscriptionQuotaId']"
$uri = "$($azAPICallConf['azAPIEndpointUrls'].ARM)/subscriptions/$($scopeId)/providers/Microsoft.Storage/storageAccounts?api-version=2021-09-01"
$method = 'GET'
$storageAccountsSubscriptionResult = AzAPICall -AzAPICallConfiguration $azAPICallConf -uri $uri -method $method -currentTask $currentTask -caller 'CustomDataCollection'

$storageAccounts = [System.Collections.ArrayList]@()
foreach ($storageAccount in $storageAccountsSubscriptionResult) {
    $obj = [System.Collections.ArrayList]@()
    $null = $obj.Add([PSCustomObject]@{
            SA             = $storageAccount
            SAUsedCapacity = $usedCapacity
        })
    $null = $script:storageAccounts.Add($obj)
}

foreach ($storageAccount in $storageAccounts) {
    if ($storageAccount.SA.Properties.primaryEndpoints.blob) {

        $urlServiceProps = "$($storageAccount.SA.Properties.primaryEndpoints.blob)?restype=service&comp=properties"
        $saProperties = AzAPICall -AzAPICallConfiguration $azAPICallConf -uri $urlServiceProps -method 'GET' -listenOn 'Content' -currentTask "$($storageAccount.SA.name) get restype=service&comp=properties" -saResourceGroupName $resourceGroupName -unhandledErrorAction Continue
        if ($saProperties) {
            if ($saProperties -eq 'AuthorizationFailure' -or $saProperties -eq 'AuthorizationPermissionDenied' -or $saProperties -eq 'ResourceUnavailable' -or $saProperties -eq 'AuthorizationPermissionMismatch' ) {
                if ($saProperties -eq 'ResourceUnavailable') {
                    $staticWebsitesState = $saProperties
                }
            }
            else {
                try {
                    # $xmlSaProperties = [xml]([string]$saProperties -replace $saProperties.Substring(0, 3)) # Leading character:  (PS version <= 7.3.9)
                    # $xmlSaProperties = [xml]([string]$saProperties -replace $saProperties.Substring(0, 1)) # Leading character:  or U+feff (PS version >= 7.4.0)
                    # ? $saProperties | Set-Clipboard
                    $xmlSaProperties = [xml]($saProperties -replace ($saProperties -replace "<.*")) # Universal fix for all PS versions
                    # ? $xmlSaProperties | Set-Clipboard

                    if ($xmlSaProperties.StorageServiceProperties.StaticWebsite) {
                        if ($xmlSaProperties.StorageServiceProperties.StaticWebsite.Enabled -eq $true) {
                            $staticWebsitesState = $true
                        }
                        else {
                            $staticWebsitesState = $false
                        }
                    }
                }
                catch {
                    Write-Host "XMLSAPropertiesFailed: Subscription: $($subDetails.displayName) ($subscriptionId) - Storage Account: $($storageAccount.SA.name)"
                    Write-Host $($saProperties.ForEach({ [char]$_ }) -join '') -ForegroundColor Cyan
                }
            }
        }

        $urlCompList = "$($storageAccount.SA.Properties.primaryEndpoints.blob)?comp=list"
        $listContainers = AzAPICall -AzAPICallConfiguration $azAPICallConf -uri $urlCompList -method 'GET' -listenOn 'Content' -currentTask "$($storageAccount.SA.name) get comp=list" -unhandledErrorAction Continue
        if ($listContainers) {
            if ($listContainers -eq 'AuthorizationFailure' -or $listContainers -eq 'AuthorizationPermissionDenied' -or $listContainers -eq 'ResourceUnavailable' -or $listContainers -eq 'AuthorizationPermissionMismatch') {
                if ($listContainers -eq 'ResourceUnavailable') {
                    $listContainersSuccess = $listContainers
                }
                else {
                    $listContainersSuccess = $false
                }
            }
            else {
                $listContainersSuccess = $true
            }

            if ($listContainersSuccess -eq $true) {
                # $xmlListContainers = [xml]([string]$listContainers -replace $listContainers.Substring(0, 3)) # Leading character:  (PS version <= 7.3.9)
                # $xmlListContainers = [xml]([string]$listContainers -replace $listContainers.Substring(0, 1)) # Leading character:  or U+feff (PS version >= 7.4.0)
                # ? $listContainers | Set-Clipboard
                $xmlListContainers = [xml]($listContainers -replace ($listContainers -replace "<.*")) # Universal fix for all PS versions
                # ? $xmlListContainers | Set-Clipboard

                $containersCount = $xmlListContainers.EnumerationResults.Containers.Container.Count

                foreach ($container in $xmlListContainers.EnumerationResults.Containers.Container) {
                    $arrayContainers += $container.Name
                    if ($container.Name -eq '$web' -and $staticWebsitesState) {
                        if ($storageAccount.SA.properties.primaryEndpoints.web) {
                            try {
                                $testStaticWebsiteResponse = Invoke-WebRequest -Uri $storageAccount.SA.properties.primaryEndpoints.web -Method 'HEAD'
                                $webSiteResponds = $true
                            }
                            catch {
                                $webSiteResponds = $false
                            }
                        }
                    }

                    if ($container.Properties.PublicAccess) {
                        if ($container.Properties.PublicAccess -eq 'blob') {
                            $arrayContainersAnonymousBlob += $container.Name
                        }
                        if ($container.Properties.PublicAccess -eq 'container') {
                            $arrayContainersAnonymousContainer += $container.Name
                        }
                    }
                }
            }
        }
    }
}
MarcelHeek commented 6 months ago

@kaiaschulz I used your ref in my workflow, not the snippets. I run this within Github action context with Managed Identities on self-hosted Github runners as security principals for data collection, so setting up something locally for troubleshooting / debugging is not trivial, but I will try to arrange stuff.

MarcelHeek commented 6 months ago

@kaiaschulz Ok, I have some results to share. For a SA that runs without error the variable $saProperties

$saProperties = AzAPICall -AzAPICallConfiguration $azAPICallConf -uri $urlServiceProps -method 'GET' -listenOn 'Content' -currentTask "$($storageAccount.SA.name) get restype=service&comp=properties" -saResourceGroupName $resourceGroupName -unhandledErrorAction Continue

gets populated with

<?xml version="1.0" encoding="utf-8"?><StorageServiceProperties>.............</StorageServiceProperties>

However the SA that generates an error the $saProperties is populated with numbers

image

So typecasting that to XML document format fails.

One of the most obvious differences between the two SA's is that the faulted one is a "general purpose V1" and the other a GPV2 sku.

JulianHayward commented 6 months ago

@kaiaschulz and @MarcelHeek first: thanks a ton for your investment!

I cannot reproduce the issue with the numbers return in any of my tenants (where I think I have all possible variants of SA). According to docs we should not get these numbers.. I am seeing tenants with successful execution with PS7.4.0 let´s dig deeper on the impacted SAs.. May I ask you to test the below (we only require READ permissions) if possible one time with PS7.4.0 and another one with a lower version?

#storageAccount REST ARM
$storageAccountName = 'saName'
$storageAccountResourceGroupName = 'saRG'
$storageAccountSubscriptionId = 'subId' 
$storageAccountARMURI = "https://management.azure.com/subscriptions/$($storageAccountSubscriptionId)/resourcegroups/$($storageAccountResourceGroupName)/providers/Microsoft.Storage/storageAccounts/$($storageAccountName)?api-version=2021-09-01"
$storageAccount = Invoke-AzRestMethod -uri $storageAccountARMURI -Method GET
$storageAccountBlobEndpoint = ($storageAccount.Content | ConvertFrom-Json).properties.primaryEndpoints.blob
Write-Host "`$storageAccountBlobEndpoint: $($storageAccountBlobEndpoint)"

#storageAccount REST Storage
#ref https://learn.microsoft.com/en-us/rest/api/storageservices/get-blob-service-properties
$azContext = Get-AzContext
$tokenRequestEndPoint = 'https://storage.azure.com'
$bearerToken = ([Microsoft.Azure.Commands.Common.Authentication.AzureSession]::Instance.AuthenticationFactory.Authenticate($azContext.Account, $azContext.Environment, $azContext.Tenant.id.ToString(), $null, [Microsoft.Azure.Commands.Common.Authentication.ShowDialog]::Never, $null, $tokenRequestEndPoint)).AccessToken
$Header = @{
    'Content-Type'  = 'application/json';
    'x-ms-version'  = '2021-04-10';
    'Authorization' = "Bearer $($bearerToken)"
}

$uri = "$($storageAccountBlobEndpoint)?restype=service&comp=properties"
$res = Invoke-WebRequest -uri $uri -Headers $Header -Method GET
$res
$res.Content
try {
    $xml = [xml]($res.Content -replace '^.*?<', '<')
    Write-Host 'XML' -ForegroundColor Green
    $xml.gettype().Name
}
catch {
    Write-Host 'failed..' -ForegroundColor DarkRed
    $res.Content.gettype()
}
JulianHayward commented 6 months ago

and if possible try the following for the 'numbers

$byteArray = [byte[]] ($res.Content | convertfrom-json)
[System.Text.Encoding]::UTF8.GetString($byteArray)
kaiaschulz commented 6 months ago

@JulianHayward

$byteArray = [byte[]] ($res.Content | convertfrom-json)

$res.Content 
239
187
191
[..]
115
62

$res.Content.gettype()

IsPublic IsSerial Name                                     BaseType
-------- -------- ----                                     --------
True     True     Byte[]                                   System.Array

$byteArray = [byte[]] ($res.Content | convertfrom-json)
[System.Text.Encoding]::UTF8.GetString($byteArray)

<?xml version="1.0" encoding="utf-8"?>
<StorageServiceProperties><Cors/><DeleteRetentionPolicy><Enabled>false</Enabled><AllowPermanentDelete>false</AllowPermanentDelete></DeleteRetentionPolicy></StorageServiceProperties>

We tested it with Powershell version 7.3.2 and 7.4.0 and the behavior is the same. It seems that Microsoft changed the way of responding to this request. Before it was working without problem, also for these storage accounts. So, this should be independent from the initial problem.

MarcelHeek commented 6 months ago

@JulianHayward I get exactly the same results as @kaiaschulz (tested with PS v7.4.0)

kaiaschulz commented 6 months ago

Hey @MarcelHeek, after a lot of investigation, I have updated my forked repo: https://github.com/kaiaschulz/Azure-MG-Sub-Governance-Reporting/blob/master/pwsh/AzGovVizParallel.ps1

Could you might check if it is running within your environment? In 2 of my tenants it is working without issues.

Thanks in advance.

MarcelHeek commented 6 months ago

@kaiaschulz Nothing less than successful executions on the 4 tenants here. Good job 👏

JulianHayward commented 6 months ago

awesome!

JulianHayward commented 6 months ago

fixed with 6.3.5